NETOBSERV-229 CI: create pre-merge images #100
Conversation
jotak
requested review from
mariomac,
eranra,
OlivierCazade,
memodi and
jpinsonneau
|
FYI I've partly tested here: https://github.com/jotak/test-actions/actions |
|
reviewers: as explained here: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ there is a security risk when we try to build / push images per PR. I think the workflow I'm suggesting is ok, but let me know if you have concerns. |
Actually, the site mentions maybe a better way to achieve what I want: using labels such as "safe to test": Maybe it's better than an explicit whitelist; however it means that we should never ever allow label creation publicly, so there's also some risk. |
.github/workflows/push_image_pr.yml
Outdated
| - name: validate user | ||
| id: validate_user | ||
| run: | | ||
| allowed=(${{ env.ALLOWED_USERS }}) |
There was a problem hiding this comment.
@jotak as you suggest, I think that using a label might be more "clean" also I suggest triggering this also if someone is adding a level to a PR (is this possible?) .... this will allow working on a PR and if needed just tag the pr with "push_image" and this will cause in this action to start creating images for the PR
There was a problem hiding this comment.
We also need to make sure that only specific users will be able to add the labels to the PR
There was a problem hiding this comment.
Ok, I've changed to use a label .. it's cleaner and shorter. So the new workflow is:
- A PR is opened
- Maintainer makes sure this PR is safe to test (no risk of attack / stealing secret)
- He adds the "ok-to-test" label
- That triggers a build and push image to quay
- If the PR is modified, the "ok-to-test" label is automatically removed. Then GOTO 2.
Also, I've modified the shortlived image creation to stick with what we said on FLP previously.
There was a problem hiding this comment.
of course this is not a mandatory workflow, we can continue to work as today, it's only something to follow if tester / QE wants an easy way to get the image
There was a problem hiding this comment.
BTW, setting a label requires write permission on the repo, so it should be safe
https://issues.redhat.com/browse/NETOBSERV-229 New github action workflow to build and push images per pull request. Some security consideration: pushing images requires QUAY_SECRET to be used, which isn't available in the 'pull_request' trigger, so we have to use 'pull_request_target' instead. This trigger uses the base branch HEAD, rather than the PR's HEAD to execute the workflow, to prevent attacks stealing secrets. As an additional measure, we inculde a user validation check, so that only allowed users (maintainers) have their PR image built.
Label is also automatically removed when a change is detected
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
https://issues.redhat.com/browse/NETOBSERV-229
New github action workflow to build and push images per pull request.
Some security consideration: pushing images requires QUAY_SECRET to be used, which isn't
available in the 'pull_request' trigger, so we have to use
'pull_request_target' instead. This trigger uses the base branch HEAD,
rather than the PR's HEAD to execute the workflow, to prevent attacks
stealing secrets. Still, it could be vulnerable to attacks stealing secret from the build itself. So as an additional measure, we inculde a user validation check, so that only allowed users (maintainers) have their PR image built.