Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle dependency updates without an update-type #644

Open
2 tasks done
timbru31 opened this issue Sep 2, 2024 · 12 comments
Open
2 tasks done

Handle dependency updates without an update-type #644

timbru31 opened this issue Sep 2, 2024 · 12 comments

Comments

@timbru31
Copy link

timbru31 commented Sep 2, 2024

Prerequisites

  • I have written a descriptive issue title
  • 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 dependabot/fetch-metadata#499 & dependabot/fetch-metadata#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.

@msgadi
Copy link

msgadi commented Sep 4, 2024

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

@simoneb
Copy link
Collaborator

simoneb commented Sep 4, 2024

@msgadi try

@msgadi
Copy link

msgadi commented Sep 4, 2024

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

@simoneb
Copy link
Collaborator

simoneb commented Sep 4, 2024

let's figure it out :)

@timbru31
Copy link
Author

timbru31 commented Sep 5, 2024

@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

@toomuchdesign
Copy link
Contributor

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?

@timbru31
Copy link
Author

timbru31 commented Oct 21, 2024

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

@toomuchdesign
Copy link
Contributor

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?

@timbru31
Copy link
Author

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

@simoneb
Copy link
Collaborator

simoneb commented Oct 22, 2024

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

@kieranswhite
Copy link

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 😄

@simoneb
Copy link
Collaborator

simoneb commented Oct 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants