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

Major version update is auto-merged with target set to minor #452

Closed
2 tasks done
haoliangyu opened this issue Jul 6, 2023 · 16 comments · Fixed by #462
Closed
2 tasks done

Major version update is auto-merged with target set to minor #452

haoliangyu opened this issue Jul 6, 2023 · 16 comments · Fixed by #462
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@haoliangyu
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.x.x

Plugin version

No response

Node.js version

18.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

20

Description

This is similar but not identical to the existing issue #166. I am using the v3 action in my project workflow and the target is set to minor.

I notice that dependabot is able to create a PR with a minor update for a dependency and a major update for its ancestor dependency, like

Screenshot 2023-07-06 at 8 51 16 AM

It is expected that this action would ignore this PR because of the major bump. However, as the screenshot shows, the action actually merges the PR automatically. I wonder if it is an edge case for the action.

Steps to Reproduce

This is from a private repository so I cannot share the PR but code snippet. It is a nodejs (v18) application using the nestjs (v8) framework.

Here is the workflow configuration I am using

  automerge:
    if: ${{ github.event_name == 'pull_request' }}
    # it requires test to finish first
    needs: test
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write
      contents: write
    steps:
      - uses: fastify/github-action-merge-dependabot@v3
        with:
          target: minor

and the PR tends to change the package.json, which is not desired

Screenshot 2023-07-06 at 9 06 12 AM

Here is the raw log of the action execution.

2023-07-05T21:12:08.1613869Z Requested labels: ubuntu-latest
2023-07-05T21:12:08.1614157Z Job defined at: my-repo/.github/workflows/main.yaml@refs/pull/410/merge
2023-07-05T21:12:08.1614273Z Waiting for a runner to pick up this job...
2023-07-05T21:12:08.3650798Z Job is waiting for a hosted runner to come online.
2023-07-05T21:12:11.3016777Z Job is about to start running on the hosted runner: GitHub Actions 2 (hosted)
2023-07-05T21:12:14.0002294Z Current runner version: '2.305.0'
2023-07-05T21:12:14.0029874Z ##[group]Operating System
2023-07-05T21:12:14.0030526Z Ubuntu
2023-07-05T21:12:14.0030824Z 22.04.2
2023-07-05T21:12:14.0031068Z LTS
2023-07-05T21:12:14.0031390Z ##[endgroup]
2023-07-05T21:12:14.0031760Z ##[group]Runner Image
2023-07-05T21:12:14.0032088Z Image: ubuntu-22.04
2023-07-05T21:12:14.0032410Z Version: 20230702.1.0
2023-07-05T21:12:14.0032931Z Included Software: https://github.com/actions/runner-images/blob/ubuntu22/20230702.1/images/linux/Ubuntu2204-Readme.md
2023-07-05T21:12:14.0033642Z Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu22%2F20230702.1
2023-07-05T21:12:14.0034091Z ##[endgroup]
2023-07-05T21:12:14.0034451Z ##[group]Runner Image Provisioner
2023-07-05T21:12:14.0034803Z 2.0.238.1
2023-07-05T21:12:14.0035064Z ##[endgroup]
2023-07-05T21:12:14.0035813Z ##[group]GITHUB_TOKEN Permissions
2023-07-05T21:12:14.0036388Z Contents: write
2023-07-05T21:12:14.0036671Z Metadata: read
2023-07-05T21:12:14.0037211Z PullRequests: write
2023-07-05T21:12:14.0037652Z ##[endgroup]
2023-07-05T21:12:14.0041370Z Secret source: Dependabot
2023-07-05T21:12:14.0041892Z Prepare workflow directory
2023-07-05T21:12:14.0804118Z Prepare all required actions
2023-07-05T21:12:14.0997236Z Getting action download info
2023-07-05T21:12:14.4378869Z Download action repository 'fastify/github-action-merge-dependabot@v3' (SHA:d37100b180dfd816bb1d7e4fbb544b3c734957a1)
2023-07-05T21:12:15.3413799Z Getting action download info
2023-07-05T21:12:15.6058229Z Download action repository 'dependabot/fetch-metadata@v1' (SHA:c9c4182bf1b97f5224aee3906fd373f6b61b4526)
2023-07-05T21:12:16.4253308Z Download action repository 'actions/github-script@v6' (SHA:d7906e4ad0b1822421a7e6a35d5ca353c962f410)
2023-07-05T21:12:17.4715313Z Complete job name: automerge
2023-07-05T21:12:17.5799012Z ##[group]Run fastify/github-action-merge-dependabot@v3
2023-07-05T21:12:17.5799336Z with:
2023-07-05T21:12:17.5799533Z   target: minor
2023-07-05T21:12:17.5800018Z   github-token: ***
2023-07-05T21:12:17.5800262Z   approve-only: false
2023-07-05T21:12:17.5800486Z   merge-method: squash
2023-07-05T21:12:17.5800720Z   use-github-auto-merge: false
2023-07-05T21:12:17.5800973Z   skip-commit-verification: false
2023-07-05T21:12:17.5801224Z   skip-verification: false
2023-07-05T21:12:17.5801440Z ##[endgroup]
2023-07-05T21:12:17.6077193Z ##[group]Run dependabot/fetch-metadata@v1
2023-07-05T21:12:17.6077464Z with:
2023-07-05T21:12:17.6077697Z   skip-commit-verification: false
2023-07-05T21:12:17.6077951Z   skip-verification: false
2023-07-05T21:12:17.6078389Z   github-token: ***
2023-07-05T21:12:17.6078598Z ##[endgroup]
2023-07-05T21:12:18.3223450Z Parsing Dependabot metadata
2023-07-05T21:12:18.3334099Z ##[group]Outputting metadata for 2 updated dependencies
2023-07-05T21:12:18.3334736Z outputs.dependency-names: webpack, @nestjs/cli
2023-07-05T21:12:18.3335124Z outputs.dependency-type: direct:development
2023-07-05T21:12:18.3335447Z outputs.update-type: null
2023-07-05T21:12:18.3335830Z outputs.directory: /webpack-and-nestjs
2023-07-05T21:12:18.3336198Z outputs.package-ecosystem: npm_and_yarn
2023-07-05T21:12:18.3336510Z outputs.target-branch: master
2023-07-05T21:12:18.3336836Z outputs.previous-version: 
2023-07-05T21:12:18.3337142Z outputs.new-version: 
2023-07-05T21:12:18.3337449Z outputs.compatibility-score: 0
2023-07-05T21:12:18.3337918Z outputs.maintainer-changes: true
2023-07-05T21:12:18.3338208Z outputs.dependency-group: 
2023-07-05T21:12:18.3338482Z outputs.alert-state: 
2023-07-05T21:12:18.3338735Z outputs.ghsa-id: 
2023-07-05T21:12:18.3338957Z outputs.cvss: 0
2023-07-05T21:12:18.3339432Z ##[endgroup]
2023-07-05T21:12:18.3749864Z ##[group]Run actions/github-script@v6
2023-07-05T21:12:18.3750109Z with:
2023-07-05T21:12:18.3750641Z   github-token: ***
2023-07-05T21:12:18.3751619Z   script: const script = require('/home/runner/work/_actions/fastify/github-action-merge-dependabot/v3/dist/index.js')
await script({
  github,
  context,
  inputs: {
  "target": "minor",
  "github-token": "***",
  "exclude": "",
  "approve-only": "false",
  "merge-method": "squash",
  "merge-comment": "",
  "use-github-auto-merge": "false",
  "pr-number": "",
  "skip-commit-verification": "false",
  "skip-verification": "false"
},
  dependabotMetadata: {
    updateType:  '',
    dependencyType:'direct:development',
    dependencyNames: 'webpack, @nestjs/cli',
  }
})

2023-07-05T21:12:18.3752699Z   debug: false
2023-07-05T21:12:18.3752915Z   user-agent: actions/github-script
2023-07-05T21:12:18.3753153Z   result-encoding: json
2023-07-05T21:12:18.3753357Z   retries: 0
2023-07-05T21:12:18.3753587Z   retry-exempt-status-codes: 400,401,403,404,422
2023-07-05T21:12:18.3753822Z ##[endgroup]
2023-07-05T21:12:29.4915673Z Dependabot merge completed
2023-07-05T21:12:29.5108532Z Cleaning up orphan processes

Expected Behavior

No response

@simoneb simoneb added the bug Something isn't working label Jul 27, 2023
@jhonrocha
Copy link
Contributor

I am taking a look on this.

@Eomm
Copy link
Member

Eomm commented Aug 2, 2023

Could @haoliangyu share the PR's title?

The PR updates 2 dependancies: the webpack bump is minior - the other one a major

@simoneb
Copy link
Collaborator

simoneb commented Aug 2, 2023

We're already looking into this @Eomm and we're confident the issue is exactly that. It's an edge case with 2 deps bumped together, one of which is minor and the other major.

@haoliangyu
Copy link
Author

Could @haoliangyu share the PR's title?

The PR updates 2 dependancies: the webpack bump is minior - the other one a major

The PR title is Bump webpack and @nestjs/cli.

@simoneb
Copy link
Collaborator

simoneb commented Aug 2, 2023

We're not using the PR title any longer @Eomm , we're using the native fetch-metadata action.

@jhonrocha
Copy link
Contributor

jhonrocha commented Aug 2, 2023

@haoliangyu I am not able to reproduce this issue. I tried using this snippet:

"dependencies": {
    "@nestjs/cli": "^8.2.8",
    "webpack": "^5.73.0"
  }

But I got 2 different PRs from Dependabot:
image

Can you confirm this is still happening? If so, can you share a reproducible snippet for it?

@haoliangyu
Copy link
Author

@jhonrocha See haoliangyu/dependabot-minor-version-update-demo#1

It only install the @nestjs/cli as

"dependencies": {
    "@nestjs/cli": "^8.2.8"
}

and webpack is the implicit dependency from @nestjs/cli.

@jhonrocha
Copy link
Contributor

@haoliangyu Thanks for that! I just tried it and I see the single PR with 2 updates from Dependabot as well.
However, the automerge action worked as expected and didn't merged the PR. Here is what I got:

Run fastify/github-action-merge-dependabot@v3
  with:
    target: minor
    github-token: ***
    approve-only: false
    merge-method: squash
    use-github-auto-merge: false
    skip-commit-verification: false
    skip-verification: false
Run dependabot/fetch-metadata@v1
Parsing Dependabot metadata
Outputting metadata for 1 updated dependency
Run actions/github-script@v[6](https://github.com/jhonrocha/test-dependabot/actions/runs/5744423046/job/15570672528#step:2:6)
Warning: Semver bump is higher than allowed in TARGET.
Tried to do a 'version-update:semver-major' update but the max allowed is 'version-update:semver-minor'

Here is a link to the action: https://github.com/jhonrocha/test-dependabot/actions/runs/5744423046/job/15570672528

@haoliangyu
Copy link
Author

@jhonrocha Please review this auto-merged PR haoliangyu/dependabot-minor-version-update-demo#1 again. The workflow configuration is the same as your test repository.

However, your test-dependabot example is incomplete since the repository doesn't include a package-lock.json file that helps dependabot locates the implicit webpack dependency.

@climba03003
Copy link
Member

climba03003 commented Aug 3, 2023

I am sure it is the problem on dependabot/fetch-metadata.
Whenever the commit message cannot be parsed, it is actually fallback to empty string.
https://github.com/dependabot/fetch-metadata/blob/06df9f85d617223bcd75c54e4307a229ac09ad01/src/dependabot/update_metadata.ts#L77-L94

For advice, we can add empty inside the propriety list to see if it helps.

const updateTypesPriority = [
updateTypes.patch,
updateTypes.minor,
updateTypes.major,
updateTypes.any,
]

@haoliangyu In order to helps troubleshoot more easily, can you add dependabot/fetch-metadata to your action and allow it to post the related information as comment?
https://github.com/dependabot/fetch-metadata

@haoliangyu
Copy link
Author

haoliangyu commented Aug 3, 2023

@climba03003 This is the raw log of the workflow run, which contains the dependabot/fetch-metadata action output:

2023-08-03T00:44:36.9920160Z ##[group]Run dependabot/fetch-metadata@v1
2023-08-03T00:44:36.9920532Z with:
2023-08-03T00:44:36.9920860Z   skip-commit-verification: false
2023-08-03T00:44:36.9921240Z   skip-verification: false
2023-08-03T00:44:36.9921820Z   github-token: ***
2023-08-03T00:44:36.9922112Z ##[endgroup]
2023-08-03T00:44:37.7113504Z Parsing Dependabot metadata
2023-08-03T00:44:37.7232030Z ##[group]Outputting metadata for 2 updated dependencies
2023-08-03T00:44:37.7233040Z outputs.dependency-names: webpack, @nestjs/cli
2023-08-03T00:44:37.7233576Z outputs.dependency-type: direct:production
2023-08-03T00:44:37.7234039Z outputs.update-type: null
2023-08-03T00:44:37.7234498Z outputs.directory: /webpack-and-nestjs
2023-08-03T00:44:37.7234998Z outputs.package-ecosystem: npm_and_yarn
2023-08-03T00:44:37.7235448Z outputs.target-branch: main
2023-08-03T00:44:37.7235906Z outputs.previous-version: 
2023-08-03T00:44:37.7236352Z outputs.new-version: 
2023-08-03T00:44:37.7236777Z outputs.compatibility-score: 0
2023-08-03T00:44:37.7237245Z outputs.maintainer-changes: true
2023-08-03T00:44:37.7237693Z outputs.dependency-group: 
2023-08-03T00:44:37.7238079Z outputs.alert-state: 
2023-08-03T00:44:37.7238441Z outputs.ghsa-id: 
2023-08-03T00:44:37.7238764Z outputs.cvss: 0
2023-08-03T00:44:37.7239757Z ##[endgroup]

You can see that the outputs.update-type is null.

For reference, this is the full raw log.
2023-08-03T00:44:27.6236236Z Requested labels: ubuntu-latest
2023-08-03T00:44:27.6236493Z Job defined at: haoliangyu/dependabot-minor-version-update-demo/.github/workflows/main.yaml@refs/pull/1/merge
2023-08-03T00:44:27.6236667Z Waiting for a runner to pick up this job...
2023-08-03T00:44:27.8873882Z Job is waiting for a hosted runner to come online.
2023-08-03T00:44:31.0604449Z Job is about to start running on the hosted runner: GitHub Actions 2 (hosted)
2023-08-03T00:44:34.4942947Z Current runner version: '2.307.1'
2023-08-03T00:44:34.4974873Z ##[group]Operating System
2023-08-03T00:44:34.4975517Z Ubuntu
2023-08-03T00:44:34.4975902Z 22.04.2
2023-08-03T00:44:34.4976249Z LTS
2023-08-03T00:44:34.4976649Z ##[endgroup]
2023-08-03T00:44:34.4977054Z ##[group]Runner Image
2023-08-03T00:44:34.4977507Z Image: ubuntu-22.04
2023-08-03T00:44:34.4977856Z Version: 20230728.3.0
2023-08-03T00:44:34.4978517Z Included Software: https://github.com/actions/runner-images/blob/ubuntu22/20230728.3/images/linux/Ubuntu2204-Readme.md
2023-08-03T00:44:34.4979379Z Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu22%2F20230728.3
2023-08-03T00:44:34.4979891Z ##[endgroup]
2023-08-03T00:44:34.4980329Z ##[group]Runner Image Provisioner
2023-08-03T00:44:34.4980754Z 2.0.264.1
2023-08-03T00:44:34.4981064Z ##[endgroup]
2023-08-03T00:44:34.4981993Z ##[group]GITHUB_TOKEN Permissions
2023-08-03T00:44:34.4982659Z Contents: write
2023-08-03T00:44:34.4983056Z Metadata: read
2023-08-03T00:44:34.4983735Z PullRequests: write
2023-08-03T00:44:34.4984243Z ##[endgroup]
2023-08-03T00:44:34.4988597Z Secret source: Dependabot
2023-08-03T00:44:34.4989305Z Prepare workflow directory
2023-08-03T00:44:34.5924892Z Prepare all required actions
2023-08-03T00:44:34.6201239Z Getting action download info
2023-08-03T00:44:34.9097297Z Download action repository 'fastify/github-action-merge-dependabot@v3' (SHA:d37100b180dfd816bb1d7e4fbb544b3c734957a1)
2023-08-03T00:44:35.6578742Z Getting action download info
2023-08-03T00:44:35.8278461Z Download action repository 'dependabot/fetch-metadata@v1' (SHA:c9c4182bf1b97f5224aee3906fd373f6b61b4526)
2023-08-03T00:44:36.3373610Z Download action repository 'actions/github-script@v6' (SHA:d7906e4ad0b1822421a7e6a35d5ca353c962f410)
2023-08-03T00:44:36.8348072Z Complete job name: automerge
2023-08-03T00:44:36.9567191Z ##[group]Run fastify/github-action-merge-dependabot@v3
2023-08-03T00:44:36.9567673Z with:
2023-08-03T00:44:36.9567945Z   target: minor
2023-08-03T00:44:36.9568590Z   github-token: ***
2023-08-03T00:44:36.9568940Z   approve-only: false
2023-08-03T00:44:36.9569257Z   merge-method: squash
2023-08-03T00:44:36.9569589Z   use-github-auto-merge: false
2023-08-03T00:44:36.9569982Z   skip-commit-verification: false
2023-08-03T00:44:36.9570357Z   skip-verification: false
2023-08-03T00:44:36.9570669Z ##[endgroup]
2023-08-03T00:44:36.9920160Z ##[group]Run dependabot/fetch-metadata@v1
2023-08-03T00:44:36.9920532Z with:
2023-08-03T00:44:36.9920860Z   skip-commit-verification: false
2023-08-03T00:44:36.9921240Z   skip-verification: false
2023-08-03T00:44:36.9921820Z   github-token: ***
2023-08-03T00:44:36.9922112Z ##[endgroup]
2023-08-03T00:44:37.7113504Z Parsing Dependabot metadata
2023-08-03T00:44:37.7232030Z ##[group]Outputting metadata for 2 updated dependencies
2023-08-03T00:44:37.7233040Z outputs.dependency-names: webpack, @nestjs/cli
2023-08-03T00:44:37.7233576Z outputs.dependency-type: direct:production
2023-08-03T00:44:37.7234039Z outputs.update-type: null
2023-08-03T00:44:37.7234498Z outputs.directory: /webpack-and-nestjs
2023-08-03T00:44:37.7234998Z outputs.package-ecosystem: npm_and_yarn
2023-08-03T00:44:37.7235448Z outputs.target-branch: main
2023-08-03T00:44:37.7235906Z outputs.previous-version: 
2023-08-03T00:44:37.7236352Z outputs.new-version: 
2023-08-03T00:44:37.7236777Z outputs.compatibility-score: 0
2023-08-03T00:44:37.7237245Z outputs.maintainer-changes: true
2023-08-03T00:44:37.7237693Z outputs.dependency-group: 
2023-08-03T00:44:37.7238079Z outputs.alert-state: 
2023-08-03T00:44:37.7238441Z outputs.ghsa-id: 
2023-08-03T00:44:37.7238764Z outputs.cvss: 0
2023-08-03T00:44:37.7239757Z ##[endgroup]
2023-08-03T00:44:37.7700144Z ##[group]Run actions/github-script@v6
2023-08-03T00:44:37.7700490Z with:
2023-08-03T00:44:37.7701070Z   github-token: ***
2023-08-03T00:44:37.7703199Z   script: const script = require('/home/runner/work/_actions/fastify/github-action-merge-dependabot/v3/dist/index.js')
await script({
  github,
  context,
  inputs: {
  "target": "minor",
  "github-token": "***",
  "exclude": "",
  "approve-only": "false",
  "merge-method": "squash",
  "merge-comment": "",
  "use-github-auto-merge": "false",
  "pr-number": "",
  "skip-commit-verification": "false",
  "skip-verification": "false"
},
  dependabotMetadata: {
    updateType:  '',
    dependencyType:'direct:production',
    dependencyNames: 'webpack, @nestjs/cli',
  }
})

2023-08-03T00:44:37.7704889Z   debug: false
2023-08-03T00:44:37.7705200Z   user-agent: actions/github-script
2023-08-03T00:44:37.7705553Z   result-encoding: json
2023-08-03T00:44:37.7705849Z   retries: 0
2023-08-03T00:44:37.7706188Z   retry-exempt-status-codes: 400,401,403,404,422
2023-08-03T00:44:37.7706539Z ##[endgroup]
2023-08-03T00:44:40.1906333Z Dependabot merge completed
2023-08-03T00:44:40.2250431Z Cleaning up orphan processes

@climba03003
Copy link
Member

You can see that the outputs.update-type is null.

Thanks, this is exactly the problem.

null resolved to -1 priority which is always lower than major.
That means whenever parsing error, it will merge the PR.

@climba03003 climba03003 added the good first issue Good for newcomers label Aug 3, 2023
@haoliangyu
Copy link
Author

Thanks for identifying the source. I think this should be the problematic part of this repo.

if (
TARGET !== updateTypes.any &&
updateTypesPriority.indexOf(updateType) >
updateTypesPriority.indexOf(TARGET)
) {
core.setOutput(MERGE_STATUS_KEY, MERGE_STATUS.skippedBumpHigherThanTarget)
logWarning(`Semver bump is higher than allowed in TARGET.
Tried to do a '${updateType}' update but the max allowed is '${TARGET}'`)
return
}

Because the updateType is an unrecognized string, the updateTypesPriority.indexOf(updateType) will always return -1 and fail the check. As a defensive approach, it would be nice to have an extra check to skip auto-merge if the update type is not recognized.

This is the related issue at the upstream dependabot/fetch-metadata#339.

@jhonrocha
Copy link
Contributor

What if we invert the order? Check it here #462 .

@jhonrocha
Copy link
Contributor

We have fixed handling the invalid update-type provided by the fetch-metadata action. Now we should not merge on this edge cases where dependabot doesn't set the update-type, unless you use target: any.
This is a workaround for the bug dependabot/fetch-metadata#339.

@github-actions
Copy link

🎉 This issue has been resolved in version 3.9.1 🎉

The release is available on:

Your optic bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
5 participants