Skip to content

Commit

Permalink
Remove body match requirement from approved reviews
Browse files Browse the repository at this point in the history
If the changes are approved for merge, there is no need to also provide
the /deploy string in the body.

Change-type: patch
Signed-off-by: Kyle Harding <kyle@balena.io>
  • Loading branch information
klutchell committed Dec 9, 2024
1 parent 69ca4dc commit 720c460
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 9 deletions.
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Deploynaut functions as a GitHub App, managing deployment approvals via [custom

### Approval Process

Deployments are approved by submitting a review with the `/deploy` command.
Deployments are approved by submitting an approved review, or a `/deploy` command in a commented review.

#### Deployment Validation

Expand All @@ -30,7 +30,9 @@ Deployments are auto-approved if:

For manual approvals:

- An eligible reviewer submits a [review](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request#submitting-your-review) with the `/deploy` command.
- An eligible reviewer submits a [review](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request#submitting-your-review).
- Commented reviews need to start with `/deploy`.
- Approved reviews do not need to match any string.
- The app approves pending deployments matching the reviewed commit SHA.

#### Eligible Reviewers
Expand All @@ -56,7 +58,7 @@ Key security features include:
1. Developer opens a PR and triggers a deployment.
2. App receives a deployment protection rule event.
3. If not auto-approved, the app comments on the PR with instructions.
4. Eligible reviewer submits a review with `/deploy`.
4. Eligible reviewer submits a commentedreview with `/deploy`.
5. App validates the approval and enables deployment.

## Setup
Expand Down
11 changes: 7 additions & 4 deletions src/handlers/deployment-protection-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import assert from 'assert';

export const instructionalComment =
'One or more environments require approval before deploying workflow runs.\n\n' +
'Maintainers, please inspect changes carefully for improper handling of sensitive information and submit a review with the required string.\n\n' +
'> **Files changed -> Review changes -> Comment** -> `/deploy`';
'Maintainers, please inspect changes carefully for improper handling of secrets or other sensitive information.\n\n' +
'To approve pending deployments, submit an approved review, or a commented review with `/deploy`.';

export async function handleDeploymentProtectionRule(
context: Context,
Expand Down Expand Up @@ -83,13 +83,16 @@ export async function handleDeploymentProtectionRule(
);

// Find an eligible review authored by a different user than the commit author or committer
// Comment reviews need to start with /deploy
// Approved reviews do not need to match any string
const deployReview = reviews.find(
(review) =>
['approved', 'commented'].includes(review.state.toLowerCase()) &&
review.commit_id === deployment.sha &&
review.user.id !== commit.author?.id &&
review.user.id !== commit.committer?.id &&
review.body?.startsWith('/deploy'),
(review.state.toLowerCase() === 'approved' ||
(review.state.toLowerCase() === 'commented' &&
review.body?.startsWith('/deploy'))),
);

if (deployReview) {
Expand Down
7 changes: 6 additions & 1 deletion src/handlers/pull-request-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ export async function handlePullRequestReview(context: Context) {
return;
}

if (!review.body?.startsWith('/deploy')) {
// Comment reviews need to start with /deploy
// Approved reviews do not need to match any string
if (
review.state.toLowerCase() === 'commented' &&
!review.body?.startsWith('/deploy')
) {
context.log.debug('Ignoring unsupported comment');
return;
}
Expand Down
1 change: 0 additions & 1 deletion tests/handlers/deployment-protection-rule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ describe('Deployment Protection Rule Handler', () => {
.reply(200, [
{
commit_id: 'test-sha',
body: '/deploy please',
state: 'APPROVED',
user: { login: 'test-user', id: 789 },
},
Expand Down

0 comments on commit 720c460

Please sign in to comment.