-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add authorize job which requires externally created PRs to be approve… #883
Conversation
…d while allowing internal team members to run tests without approval
A quick clarification: so the new job |
I think keeping labels is okay (if that is the case) as even for PRs produced by main dev team e2e tests run (and which exact one) may be different depending on the commit. So I think having labels as the instrument to specify what tests exactly to run seems ok. @kylewuolle please confirm the labels approach was untouched ^^. |
Yes that's right so the label is still required in order for the e2e tests to run. So the flow is basically the same other than if the e2e test label is set the authorize job must be approved by a team member. Team members can self approve their own PRs and external users' PRs will need to be approved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kylewuolle I like the approach, but could you please clarify something? So as I understand:
• For PRs created from forked repositories, additional approval from members of the k0rdent team is required
• For unforked PRs (pushed directly to the original kcm project), no additional protection is applied since only team members have permission to push directly to k0rdent.
Am I correct in understanding this? Specifically, I’m curious whether the protection will function correctly for PRs created directly in the repository.
Yes that's right. So unforked PRs will not require approval and forked ones will. Since every team member is able to approve PRs for e2e test runs (including their own) this will not cause issues where team members will need to get someone else to approve a test run. Hopefully that makes sense. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we can try that approach. LGTM
This PR enables the concept of approving externally created PRs for running e2e tests. It is based on a blog post : https://datachain.ai/blog/testing-external-contributions-using-github-actions-secrets
The setup involved here is first we create two environments: external and internal. The external environment will have a deployment protection rule enabled which will require a review from anyone on the team. This PR introduces a job called "authorize" which only runs after approval if the PR originates from a fork. Then, the e2e test jobs are marked as needing the authorize job to complete prior to running. In this way users on the team can still run e2e tests from either forked or unforked PRs since we can approve the job.
Here is an example of the protection in action:
Here's an example of a PR which is not forked:
For forked PRs from team members the flow will be the same as the first example but we can self approve the job run.
Resolves https://github.com/k0rdent/2a-internal/issues/17