feat(repo settings): approve build mechanism for pull_request events
#328
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #328 +/- ##
==========================================
+ Coverage 96.35% 96.38% +0.03%
==========================================
Files 62 62
Lines 6859 6920 +61
==========================================
+ Hits 6609 6670 +61
Misses 181 181
Partials 69 69
|
| // ApproveOnce defines the CI strategy of having a repo administrator approve | ||
| // all builds triggered from an outside contributor if this is their first time contributing. | ||
| ApproveOnce = "first-time" |
There was a problem hiding this comment.
A note on this: I am adding this constant as more of a To-Do. I don't have actual functionality for this coded up, but would create a follow-up story to implement this at a later point.
Co-authored-by: Jacob Floyd <cognifloyd@gmail.com>
|
Slotting for |
There was a problem hiding this comment.
a couple of comments
consider other forms of approval?
i wonder if we want to decouple this from fork builds more (at the least the bit attached to the repo object) and maybe push that aspect to the value(s) you can provide. reason is to avoid potential issues later when we want to (maybe?) support other types of approval? for example, maybe we want to support approval for tag builds as these are often associated with prod deploys.
on the other hand, if you wanted this functionality, i think you can kind of get there with deployments today by utilizing https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment#required-reviewers . that is, send a deployment request via Vela, and then it hangs out in GitHub until someone approves. once approved it sends off the webhook for the deployment to Vela. i haven't tested this, tbh.
some folks also get more creative to get to similar functionality, like https://trstringer.com/github-actions-manual-approval/ for github actions, for example.
track approver
i think there would be additional interest to track who approved a build (and maybe when?) from audit perspective. should we attach to build?
any thoughts on those?
|
@wass3rw3rk Included auditing and put the fork terminology in the constants rather than the field name with latest commit. Let me know what you think! |
DESCRIPTION
Inspired by this behavior that GH Actions is able to do.
This functionality would protect our user base from untrusted sources executing pipelines without approval. It is also much more tenable to provide this capability rather than show warnings like this.
This security enhancement was made possible by the temporary storage of compiled build objects (main PR for that). Using this, we can put builds on the ice box while they await approval.
The below diagrams illustrate this process (note the two DB icons are the same database, just duplicated for readability):
BEFORE
AFTER
MISC
As an aside, this will close all these open PRs for trying to differentiate PR events from forks:
Closes #271
Closes go-vela/server#727
Closes go-vela/ui#593