test-infra
test-infra copied to clipboard
knative
`/ok-to-test` should trigger GitHub Actions to run
For non-members GitHub Actions don't run. They need approval from a member or maybe admin https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks
Similarly prow gates running prow jobs for non-members until an /ok-to-test comment that adds an ok-to-test label.
It would be great when we comment /ok-to-test it also approves the GitHub Actions workflows to run
cc @shinigambit @kvmware
There is an API for this.
https://docs.github.com/en/rest/reference/actions#approve-a-workflow-run-for-a-fork-pull-request
I think this could be accomplished by a new GitHub action that checks for the addition of the ok-to-test tag and copy the action to the necessary repos via knobots rather that relying on Prow for this
I'm making good progress on this here but I've hit a roadblock because at every attempt to approve I get the following message
{"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/reference/actions#approve-a-workflow-run-for-a-fork-pull-request"}
I think it's possible that the token in my fork doesn't have the appropriate permissions but the one used in the actual knative project would work. Should I create it on a repo to try it out @dprotaso @upodroid ?
For security reasons, the token available for the PR from the fork is read-only to avoid bad actors making changes.
https://github.community/t/token-permissions-for-forks-once-again/16468/5 https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
You'll need to wrap it around a prow job with a privileged github oauth token.
@upodroid the post you linked is very informative but also suggests using an on:workflow_run action to circumvent the token limitation. Do you feel strongly in favor of Prow or is the approach in the security article acceptable to you?
I'm fine with either. Try on:workflow_run trigger first and see how that goes.
Since this is a prow command wouldn't prow have the authority/token to approve the GitHub actions to run?
It definitely should be possible to do it in Prow but the GitHub actions approach seemed easier to implement for me
GitHub actions approach seemed easier to implement for me
But now we have to worry about propagating this action everywhere we want to use this feature. I think it's just simpler to maintain this across many repos if it's a prow plugin vs. a GitHub action
That's fair, I'm not sure if a Prow job would do, seems a bit easier than making a plugin. Mind if I try that option first @dprotaso ?
Consensus in the productivity WG meeting is to open an issue in upstream Prow and have Prow take care of approving the GitHub workflow runs, let's explore that first @dprotaso I'm opening the issue so we can have the Prow folks input
Sounds good. To clarify I wasn't suggesting a job but it being part of the plugin. The same way that you implemented /retest to restart failed actions
It also looks like the approval needs to occur any time there's a new change (ie. pushed a commit) on the PR.
We discussed this issue in Productivity WG meeting and we came to the consensus that the /ok-to-test for github actions should be feature parity with what Prow is currently doing (which the current PR is not doing).
It also looks like the approval needs to occur any time there's a new change (ie. pushed a commit) on the PR.
That's a bit worrying because we would have to make at least one GitHub API call for every commit for every user unless somehow Prow or GitHub actions has some context on who is a new contributor, then we could approve runs only for those users. If that's not an option we should be aware of the GitHub token usage related to #2962
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.
This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.