From 2cc00ba39cd9effbb03774a95f6c28d237418ac9 Mon Sep 17 00:00:00 2001 From: yeikel Date: Mon, 24 Apr 2023 08:52:21 -0400 Subject: [PATCH] feat: add option to skip-verification When enabled, both author and commit verification are disabled Closes #378 --- README.md | 23 +++++----- action.yml | 9 +++- dist/index.js | 10 ++-- package-lock.json | 2 +- src/action.js | 7 +-- src/util.js | 1 + test/action.test.js | 108 ++++++++++++++++++++++++++++++++++++++++++++ test/util.test.js | 4 ++ 8 files changed, 143 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index f3ec61a9..85bb51e2 100644 --- a/README.md +++ b/README.md @@ -20,17 +20,18 @@ Error: Resource not accessible by integration ## Inputs -| input | required | default | description | -|----------------------------|----------|--------------------------|-----------------------------------------------------------------| -| `github-token` | No | `${{github.token}}` | A GitHub token. | -| `exclude` | No | | A comma or semicolon separated value of packages that you don't want to auto-merge and would like to manually review to decide whether to upgrade or not. | -| `approve-only` | No | `false` | If `true`, the PR is only approved but not merged. | -| `merge-method` | No | `squash` | The merge method you would like to use (squash, merge, rebase). | -| `merge-comment` | No | `''` | An arbitrary message that you'd like to comment on the PR after it gets auto-merged. This is only useful when you're receiving too much of noise in email and would like to filter mails for PRs that got automatically merged. | -| `use-github-auto-merge` | No | `false` | If `true`, the PR is marked as auto-merge and will be merged by GitHub when status checks are satisfied.

_NOTE_: This feature only works when all of the following conditions are met.
- The repository enables auto-merge.
- The pull request base must have a branch protection rule.
- The pull request's status checks are not yet satisfied.

Refer to [the official document](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request) about GitHub auto-merge. | -| `target` | No | `any` | A flag to only auto-merge updates based on Semantic Versioning.
Possible options are: `major, premajor, minor, preminor, patch, prepatch, prerelease, any`.

For more details on how semantic version difference is calculated please see [semver](https://www.npmjs.com/package/semver) package.

If you set a value other than `any`, PRs that are not semantic version compliant are skipped. An example of a non-semantic version is a commit hash when using git submodules.| -| `pr-number` | No | | A pull request number, only required if triggered from a workflow_dispatch event. Typically this would be triggered by a script running in a separate CI provider. See [Trigger action from workflow_dispatch event](#trigger-action-from-workflow_dispatch-event) example. | -| `skip-commit-verification` | No | `false` | If `true`, then the action will not expect the commits to have a verification signature. It is required to set this to `true` in GitHub Enterprise Server. | +| input | required | default | description | +|----------------------------|----------|---------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `github-token` | No | `${{github.token}}` | A GitHub token. | +| `exclude` | No | | A comma or semicolon separated value of packages that you don't want to auto-merge and would like to manually review to decide whether to upgrade or not. | +| `approve-only` | No | `false` | If `true`, the PR is only approved but not merged. | +| `merge-method` | No | `squash` | The merge method you would like to use (squash, merge, rebase). | +| `merge-comment` | No | `''` | An arbitrary message that you'd like to comment on the PR after it gets auto-merged. This is only useful when you're receiving too much of noise in email and would like to filter mails for PRs that got automatically merged. | +| `use-github-auto-merge` | No | `false` | If `true`, the PR is marked as auto-merge and will be merged by GitHub when status checks are satisfied.

_NOTE_: This feature only works when all of the following conditions are met.
- The repository enables auto-merge.
- The pull request base must have a branch protection rule.
- The pull request's status checks are not yet satisfied.

Refer to [the official document](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request) about GitHub auto-merge. | +| `target` | No | `any` | A flag to only auto-merge updates based on Semantic Versioning.
Possible options are: `major, premajor, minor, preminor, patch, prepatch, prerelease, any`.

For more details on how semantic version difference is calculated please see [semver](https://www.npmjs.com/package/semver) package.

If you set a value other than `any`, PRs that are not semantic version compliant are skipped. An example of a non-semantic version is a commit hash when using git submodules. | +| `pr-number` | No | | A pull request number, only required if triggered from a workflow_dispatch event. Typically this would be triggered by a script running in a separate CI provider. See [Trigger action from workflow_dispatch event](#trigger-action-from-workflow_dispatch-event) example. | +| `skip-commit-verification` | No | `false` | If `true`, then the action will not expect the commits to have a verification signature. It is required to set this to `true` in GitHub Enterprise Server. | +| `skip-verification` | No | `false` | If true, the action will not validate the user or the commit verification status | ## Examples diff --git a/action.yml b/action.yml index 35d46178..71c3a276 100644 --- a/action.yml +++ b/action.yml @@ -36,6 +36,10 @@ inputs: description: 'If true, then the action will not expect the commits to have a verification signature. It is required to set this to true in GitHub Enterprise Server' required: false default: false + skip-verification: + type: boolean + description: 'If true, the action will not validate the user or the commit verification status' + default: false runs: using: 'composite' @@ -43,12 +47,13 @@ runs: - name: Fetch metadata id: dependabot-metadata uses: dependabot/fetch-metadata@v1 - if: ${{ github.event_name == 'pull_request' && github.actor == 'dependabot[bot]' }} + if: github.event_name == 'pull_request' && (github.actor == 'dependabot[bot]' || skip-verification == 'true') with: skip-commit-verification: ${{ inputs.skip-commit-verification }} + skip-verification : ${{ inputs.skip-verification }} - name: Merge/approve PR uses: actions/github-script@v6 - if: ${{ github.event_name == 'pull_request' && github.actor == 'dependabot[bot]' }} + if: github.event_name == 'pull_request' && (github.actor == 'dependabot[bot]' || skip-verification == 'true') with: github-token: ${{ inputs.github-token }} script: | diff --git a/dist/index.js b/dist/index.js index d79aed2c..76b214ad 100644 --- a/dist/index.js +++ b/dist/index.js @@ -2803,6 +2803,7 @@ module.exports = async function run({ TARGET, PR_NUMBER, SKIP_COMMIT_VERIFICATION, + SKIP_VERIFICATION, } = getInputs(inputs) try { @@ -2820,16 +2821,16 @@ module.exports = async function run({ const pr = pull_request || (await client.getPullRequest(PR_NUMBER)) const isDependabotPR = pr.user.login === dependabotAuthor - if (!isDependabotPR) { + if (!isDependabotPR && !SKIP_VERIFICATION) { return logWarning('Not a dependabot PR, skipping.') } const commits = await client.getPullRequestCommits(pr.number) - if (!commits.every(commit => commit.author?.login === dependabotAuthor)) { + if (!commits.every(commit => commit.author?.login === dependabotAuthor) && !SKIP_VERIFICATION) { return logWarning('PR contains non dependabot commits, skipping.') } - if (!SKIP_COMMIT_VERIFICATION) { + if (!SKIP_COMMIT_VERIFICATION && !SKIP_VERIFICATION) { try { verifyCommits(commits) } catch { @@ -3132,6 +3133,7 @@ exports.getInputs = inputs => { TARGET: mapUpdateType(inputs['target']), PR_NUMBER: inputs['pr-number'], SKIP_COMMIT_VERIFICATION: /true/i.test(inputs['skip-commit-verification']), + SKIP_VERIFICATION: /true/i.test(inputs['skip-verification']), } } @@ -3280,7 +3282,7 @@ module.exports = require("util"); /***/ ((module) => { "use strict"; -module.exports = JSON.parse('{"name":"github-action-merge-dependabot","version":"3.6.4","description":"A GitHub action to automatically merge and approve Dependabot pull requests","main":"src/index.js","scripts":{"build":"ncc build src/index.js","lint":"eslint .","test":"tap test/**.test.js","prepare":"husky install"},"author":{"name":"Salman Mitha","email":"SalmanMitha@gmail.com"},"contributors":["Simone Busoli "],"license":"MIT","repository":{"type":"git","url":"git+https://github.com/fastify/github-action-merge-dependabot.git"},"bugs":{"url":"https://github.com/fastify/github-action-merge-dependabot/issues"},"homepage":"https://github.com/fastify/github-action-merge-dependabot#readme","dependencies":{"@actions/core":"^1.9.1","@actions/github":"^5.1.1","actions-toolkit":"github:nearform/actions-toolkit","gitdiff-parser":"^0.3.1","semver":"^7.4.0"},"devDependencies":{"@vercel/ncc":"^0.36.1","eslint":"^8.38.0","eslint-config-prettier":"^8.8.0","eslint-plugin-prettier":"^4.2.1","husky":"^8.0.3","prettier":"^2.8.7","proxyquire":"^2.1.3","sinon":"^15.0.3","tap":"^16.3.4"}}'); +module.exports = JSON.parse('{"name":"github-action-merge-dependabot","version":"3.6.4","description":"A GitHub action to automatically merge and approve Dependabot pull requests","main":"src/index.js","scripts":{"build":"ncc build src/index.js","lint":"eslint .","test":"tap test/**.test.js","prepare":"husky install"},"author":{"name":"Salman Mitha","email":"SalmanMitha@gmail.com"},"contributors":["Simone Busoli "],"license":"MIT","repository":{"type":"git","url":"git+https://github.com/fastify/github-action-merge-dependabot.git"},"bugs":{"url":"https://github.com/fastify/github-action-merge-dependabot/issues"},"homepage":"https://github.com/fastify/github-action-merge-dependabot#readme","dependencies":{"@actions/core":"^1.9.1","@actions/github":"^5.1.1","actions-toolkit":"github:nearform/actions-toolkit","gitdiff-parser":"^0.3.1","semver":"^7.5.0"},"devDependencies":{"@vercel/ncc":"^0.36.1","eslint":"^8.39.0","eslint-config-prettier":"^8.8.0","eslint-plugin-prettier":"^4.2.1","husky":"^8.0.3","prettier":"^2.8.8","proxyquire":"^2.1.3","sinon":"^15.0.4","tap":"^16.3.4"}}'); /***/ }) diff --git a/package-lock.json b/package-lock.json index 7ba8710d..ba6a00b2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6363,7 +6363,7 @@ "actions-toolkit": { "version": "git+ssh://git@github.com/nearform/actions-toolkit.git#a262b95f7b335fea39c74d48e6a9c4c882240065", "integrity": "sha512-6XEaQXE/yian7ZiRR8/2Xa71sBXCxERr/V3nfHfDAc85Wffr2vptbqXDR8o5EBGpXkPtugDxNg4zG4JwUHNrhA==", - "from": "actions-toolkit@github:nearform/actions-toolkit#a262b95f7b335fea39c74d48e6a9c4c882240065", + "from": "actions-toolkit@github:nearform/actions-toolkit", "requires": { "@actions/core": "^1.10.0" } diff --git a/src/action.js b/src/action.js index b198f1a0..eed3d451 100644 --- a/src/action.js +++ b/src/action.js @@ -32,6 +32,7 @@ module.exports = async function run({ TARGET, PR_NUMBER, SKIP_COMMIT_VERIFICATION, + SKIP_VERIFICATION, } = getInputs(inputs) try { @@ -49,16 +50,16 @@ module.exports = async function run({ const pr = pull_request || (await client.getPullRequest(PR_NUMBER)) const isDependabotPR = pr.user.login === dependabotAuthor - if (!isDependabotPR) { + if (!isDependabotPR && !SKIP_VERIFICATION) { return logWarning('Not a dependabot PR, skipping.') } const commits = await client.getPullRequestCommits(pr.number) - if (!commits.every(commit => commit.author?.login === dependabotAuthor)) { + if (!commits.every(commit => commit.author?.login === dependabotAuthor) && !SKIP_VERIFICATION) { return logWarning('PR contains non dependabot commits, skipping.') } - if (!SKIP_COMMIT_VERIFICATION) { + if (!SKIP_COMMIT_VERIFICATION && !SKIP_VERIFICATION) { try { verifyCommits(commits) } catch { diff --git a/src/util.js b/src/util.js index e4a3f62b..4852f12f 100644 --- a/src/util.js +++ b/src/util.js @@ -47,5 +47,6 @@ exports.getInputs = inputs => { TARGET: mapUpdateType(inputs['target']), PR_NUMBER: inputs['pr-number'], SKIP_COMMIT_VERIFICATION: /true/i.test(inputs['skip-commit-verification']), + SKIP_VERIFICATION: /true/i.test(inputs['skip-verification']), } } diff --git a/test/action.test.js b/test/action.test.js index e46b42b3..baed3ddc 100644 --- a/test/action.test.js +++ b/test/action.test.js @@ -206,6 +206,38 @@ for (const prCommitsStub of prCommitsStubs) { }) } +for (const prCommitsStub of prCommitsStubs) { + tap.test('should NOT skip PR with non dependabot commit when skip-verification is enabled', async () => { + const PR_NUMBER = Math.random() + const { action, stubs } = buildStubbedAction({ + payload: { + pull_request: { + user: { + login: BOT_NAME, + }, + number: PR_NUMBER, + }, + }, + inputs: { + 'skip-verification': true, + }, + }) + stubs.prCommitsStub.resolves([prCommitsStub]) + + await action() + + sinon.assert.calledWithExactly( + stubs.logStub.logInfo, + 'Dependabot merge completed' + ) + sinon.assert.calledOnce(stubs.prCommitsStub) + sinon.assert.calledOnce(stubs.approveStub) + sinon.assert.calledOnce(stubs.mergeStub) + }) +} + + + tap.test( 'should skip PR if dependabot commit signatures cannot be verified', async () => { @@ -280,6 +312,82 @@ tap.test( } ) +tap.test( + 'should review and merge even if commit signatures cannot be verified when skip-verification is enabled', + async () => { + const PR_NUMBER = Math.random() + const { action, stubs } = buildStubbedAction({ + payload: { + pull_request: { + user: { + login: BOT_NAME, + }, + number: PR_NUMBER, + }, + }, + inputs: { + 'skip-verification': true, + }, + }) + + stubs.prCommitsStub.resolves([ + { + author: { + login: 'dependabot[bot]', + }, + }, + ]) + + await action() + + sinon.assert.calledWithExactly( + stubs.logStub.logInfo, + 'Dependabot merge completed' + ) + sinon.assert.notCalled(stubs.coreStub.setFailed) + sinon.assert.calledOnce(stubs.approveStub) + sinon.assert.calledOnce(stubs.mergeStub) + } +) + +tap.test( + 'should review and merge even the user is not dependabot when skip-verification is enabled', + async () => { + const PR_NUMBER = Math.random() + const { action, stubs } = buildStubbedAction({ + payload: { + pull_request: { + user: { + login: BOT_NAME, + }, + number: PR_NUMBER, + }, + }, + inputs: { + 'skip-verification': true, + }, + }) + + stubs.prCommitsStub.resolves([ + { + author: { + login: 'myCustomUser', + }, + }, + ]) + + await action() + + sinon.assert.calledWithExactly( + stubs.logStub.logInfo, + 'Dependabot merge completed' + ) + sinon.assert.notCalled(stubs.coreStub.setFailed) + sinon.assert.calledOnce(stubs.approveStub) + sinon.assert.calledOnce(stubs.mergeStub) + } +) + tap.test('should ignore excluded package', async () => { const PR_NUMBER = Math.random() const { action, stubs } = buildStubbedAction({ diff --git a/test/util.test.js b/test/util.test.js index 785a1ed9..8ec6e3cb 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -39,6 +39,10 @@ const BOOLEAN_INPUTS = [ input: 'skip-commit-verification', key: 'SKIP_COMMIT_VERIFICATION', }, + { + input: 'skip-verification', + key: 'SKIP_VERIFICATION', + }, ] tap.test('getInputs', async t => {