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

chore: simplify auto approve mechanism #17264

Merged
merged 3 commits into from
Nov 1, 2021

Conversation

iliapolo
Copy link
Contributor

@iliapolo iliapolo commented Nov 1, 2021

Currently, PR's are auto approved if they either:

  1. Contain the pr/auto-approve label.
  2. Created by dependabot
  3. Created by aws-cdk-automation

This is somewhat convoluted, and complicates the responsibility of the auto-approve workflow.
In addition, this makes it impossible to formulate a single GitHub query to lookup all automated PR's that we expect to be approved and merged without human intervention.

This PR switches to a simpler mechanism, by which the auto-approve workflow will only approve PR's that contain the appropriate label, forcing all PR creators to add the label if they wish to be auto-approved.

This means we can now use a simple label:pr/auto-approve query to find all those automated PR's.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Nov 1, 2021

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Nov 1, 2021
github.event.pull_request.user.login == 'dependabot[bot]'
|| github.event.pull_request.user.login == 'dependabot-preview[bot]'
|| (contains(github.event.pull_request.labels.*.name, 'pr/auto-approve')
&& github.event.pull_request.user.login == 'aws-cdk-automation')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this condition was removed because PR's created by aws-cdk-automation already contain the pr/auto-approve label. For example:

#17223

@iliapolo iliapolo requested a review from a team November 1, 2021 16:52

jobs:
auto-approve:
if: github.event.pull_request.user.login == 'dependabot[bot]' || github.event.pull_request.user.login == 'dependabot-preview[bot]'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be possible to configure dependabot to add that label on its pull requests so this workflow shouldn't be required

Copy link
Contributor Author

@iliapolo iliapolo Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not possible because security related PR's are not coupled with any configuration file, and are controlled by the Dependabot security updates setting in the security analysis page.

Note that dependabot right now is only explicitly configured to upgrade github actions:

version: 2
updates:
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "weekly"
labels:
- "pr/auto-approve"
open-pull-requests-limit: 5

It does however still create security upgrade PR's: #16421

We decided to keep creating these PR's as a fallback to when our own dependency upgrade PR's is misbehaving. Unfortunately those security related PR's are not configurable, from what I could gather.

@iliapolo iliapolo requested a review from eladb November 1, 2021 18:14
@mergify
Copy link
Contributor

mergify bot commented Nov 1, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 415e4a1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 606a2d3 into master Nov 1, 2021
@mergify mergify bot deleted the epolon/add-auto-approve-label-to-dependabot branch November 1, 2021 22:15
@mergify
Copy link
Contributor

mergify bot commented Nov 1, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify bot pushed a commit to aws/jsii that referenced this pull request Nov 2, 2021
Complementary of aws/aws-cdk#17264

Related to cdklabs/cdk-ops#1759

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
Currently, PR's are auto approved if they either:

1. Contain the `pr/auto-approve` label.
2. Created by `dependabot`
3. Created by `aws-cdk-automation`

This is somewhat convoluted, and complicates the responsibility of the `auto-approve` workflow. 
In addition, this makes it impossible to formulate a single GitHub query to lookup all automated PR's that we expect to be approved and merged without human intervention. 

This PR switches to a simpler mechanism, by which the `auto-approve` workflow will **only** approve PR's that contain the appropriate label, forcing all PR creators to add the label if they wish to be auto-approved.

This means we can now use a simple `label:pr/auto-approve` query to find all those automated PR's.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants