github-action-merge-dependabot icon indicating copy to clipboard operation
github-action-merge-dependabot copied to clipboard

Handle dependency updates without an update-type

Open timbru31 opened this issue 1 year ago • 12 comments

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched existing issues to ensure the issue has not already been raised

Issue

We are using the action @v3.10.1 but see a lot of PRs that are in semver range but those are sadly not merged because (somehow) Dependabot's metadata is outputting update-type: null.

I am not 100% if this is a broader Dependabot related issue or if this action could enhance it's behavior. Interestingly it only affects PRs that Updates the requirements on xx to permit the latest version. (see screenshot attached)

Screenshot 2024-09-02 at 12 35 33

Here is the update from the log:

Run fastify/[email protected]
Run dependabot/fetch-metadata@v1
Parsing Dependabot metadata
Outputting metadata for 1 updated dependency
  outputs.dependency-names: lint-staged
  outputs.dependency-type: direct:development
  outputs.update-type: null
  outputs.directory: /maintenance
  outputs.package-ecosystem: npm_and_yarn
  outputs.target-branch: main
  outputs.previous-version: 
  outputs.new-version: 
  outputs.compatibility-score: 0
  outputs.maintainer-changes: false
  outputs.dependency-group: 
  outputs.alert-state: 
  outputs.ghsa-id: 
  outputs.cvss: 0

The PR is from a private repo, hence I can't link to it.

Cross ref to https://github.com/dependabot/fetch-metadata/issues/499 & https://github.com/dependabot/fetch-metadata/issues/339 As this is open for 1 1/2 years maybe you can have a fallback method which tries to parse the semver information from e.g. the commit message or PR title, too, in case the update-type is null.

If you believe this is outside of this action's scope that is also fine.

timbru31 avatar Sep 02 '24 10:09 timbru31

Can we try updating dependabot/fetch-metadata@v1 to dependabot/fetch-metadata@v2 to see if it fixes the issue?

msgadi avatar Sep 04 '24 07:09 msgadi

@msgadi try

simoneb avatar Sep 04 '24 07:09 simoneb

but fastify/[email protected] is already using dependabot/fetch-metadata@v2. Not sure why in logs it says dependabot/fetch-metadata@v1 !

msgadi avatar Sep 04 '24 08:09 msgadi

let's figure it out :)

simoneb avatar Sep 04 '24 08:09 simoneb

@msgadi because the released version, v3.10.1, did not include the update: https://github.com/fastify/github-action-merge-dependabot/blob/v3.10.1/action.yml#L60

timbru31 avatar Sep 05 '24 14:09 timbru31

v3.10.2 has been published with dependabot/fetch-metadata@v2 update.

@timbru31 Are you still experiencing the same issue with direct dependency updates? If so, could you provide a link to the relevant dependabot and github-action-merge-dependabot action configs?

toomuchdesign avatar Oct 21 '24 08:10 toomuchdesign

Yes the issue remains even with 3.10.2.

I can share some config snippets as the repo is private. If that's not sufficient, I can try a public reproduction repo:

name: 'Dependabot Continuous integration'

on:
  pull_request:
    branches: [main]

env:
  NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

jobs:
  check-dependabot:
    name: 'Dependabot Test Pull Request'
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - name: Checkout
        uses: actions/[email protected]
        with:
          ref: ${{ github.event.pull_request.head.sha }}

      - name: Setup Node.js
        uses: actions/[email protected]
        with:
          node-version-file: '.nvmrc'
          cache: yarn
          registry-url: https://npm.pkg.github.com/

      - name: Install CLI dependencies
        run: yarn install --frozen-lockfile

      - name: Check format
        run: yarn run format:check

      - name: Check lint
        run: yarn run lint:check

      - name: Check TypeScript
        run: yarn run typescript:check

      - name: Run unit tests
        run: yarn test:unit

      - name: Run integration tests
        run: yarn test:integration

      - name: Build CLI
        run: yarn build:prod

  auto-merge:
    name: 'Automatically merge Dependabot upgrades'
    runs-on: ubuntu-latest
    needs: check-dependabot
    permissions:
      pull-requests: write
      contents: write
    steps:
      - name: Automatically merge dependabot upgrades
        uses: fastify/[email protected]
        with:
          target: minor

Config

version: 2
registries:
  npm-github:
    type: npm-registry
    url: https://npm.pkg.github.com
    token: ${{ secrets.DEPENDABOTCICD }}
updates:
  - package-ecosystem: 'npm'
    directory: '/'
    schedule:
      interval: 'daily'
    registries:
      - npm-github
    commit-message:
      prefix: 'build(deps)'
      prefix-development: 'build(deps-dev)'
    ignore:
      - dependency-name: 'react'
    open-pull-requests-limit: 100
    versioning-strategy: 'increase'

  - package-ecosystem: 'npm'
    directory: '/maintenance'
    schedule:
      interval: 'daily'
    registries:
      - npm-github
    commit-message:
      prefix: 'build(deps)'
      prefix-development: 'build(deps-dev)'
    ignore:
      - dependency-name: 'styled-components'
      - dependency-name: 'react'
      - dependency-name: 'react-dom'
      - dependency-name: '@types/react-dom'
      - dependency-name: '@types/react'
      - dependency-name: '@types/styled-components'
    open-pull-requests-limit: 100
    versioning-strategy: 'increase'

  - package-ecosystem: 'npm'
    directory: '/maintenance/api'
    schedule:
      interval: 'daily'
    registries:
      - npm-github
    commit-message:
      prefix: 'build(deps)'
      prefix-development: 'build(deps-dev)'
    open-pull-requests-limit: 100
    versioning-strategy: 'increase'

  - package-ecosystem: 'github-actions'
    directory: '/'
    schedule:
      interval: 'daily'
    commit-message:
      prefix: 'build(deps)'
      prefix-development: 'build(deps-dev)'
    open-pull-requests-limit: 100

job-logs.txt

timbru31 avatar Oct 21 '24 09:10 timbru31

The provided log seems to refer to a package dependency being hosted on a private registry.

  • is it hosted on NPM/private or another registry? (I don't want to disclose any private info)
  • can you get similar failures with other NPM packages with direct:production or direct:development dependency type?

toomuchdesign avatar Oct 21 '24 09:10 toomuchdesign

Both internal packages (using the GH registry) and public packages are affected. Also direct:development updates are affected, too. (e.g. eslint updates)

timbru31 avatar Oct 21 '24 10:10 timbru31

@kieranswhite can you take a look at this one please?

simoneb avatar Oct 22 '24 16:10 simoneb

After having a read through this issue, issues in dependabot fetch-metadata and dependabot-core I feel like this is most likely out of scope with this product (as you alluded to in the original comment).

There is an open issue in dependabot core as well as the issues you linked in this issue referring to problems with update-type not being set properly for a variety of reasons.

I feel like it would be possible for us to add some sort of override/default here for merging the PRs without a defined update type but this seems risky, as without the update type we don't know the extent of the changes (major,minor,patch..etc) and merging PRs without a definitive update type could result in unintended consequences.

I noticed there were some PRs in fetch-metadata which attempts to resolve the update-type if not set in the original commit message but this also doesn't seem ideal as it's really something that dependabot-core should be doing in my opinion.

Please let me know if you are happy for us to close the issue off based on it being out of scope or if you feel differently please respond and we can discuss things further :)

Thanks 😄

kieranswhite avatar Oct 24 '24 09:10 kieranswhite

Thanks everyone for looking into this. This is my current understanding:

  • in some cases, fetch-metadata will return an invalid update-type
  • in such cases, this action doesn't try to merge the PR and fails with "Semver bump '' is invalid!", but this should happen only when the target input is set to anything other than 'any', because in that case I believe the action will merge regardless (which I believe makes sense). See here

We may try to infer the update type from somewhere else. We used to do this before we started using fetch-metadata. I don't like it and I would like to avoid it. I don't see other solutions unfortunately. If you're using this action to bump only up to a certain semver bump and we can't detect what bump type it is, the sensible thing to do in my opinion is to fail and not do anything.

simoneb avatar Oct 24 '24 11:10 simoneb

One interesting addition:

For one repo we've switched from pull_request_target to pull_request and it magically started working. Update type was populated outputs.update-type: version-update:semver-minor

timbru31 avatar Dec 11 '24 10:12 timbru31

Interesting, thanks for the follow up @timbru31!

Meanwhile we were discussing internally what to do with this issue, and at the moment I believe there is nothing that we can reasonably do. So I'm going to close it as there is no sensible solution in user space.

simoneb avatar Dec 11 '24 10:12 simoneb