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

/ok-to-test should trigger GitHub Actions to run #3057

Open
dprotaso opened this issue Jan 31, 2022 · 24 comments
Open

/ok-to-test should trigger GitHub Actions to run #3057

dprotaso opened this issue Jan 31, 2022 · 24 comments
Labels
area/github-actions Issues or PRs related to GitHub Actions lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@dprotaso
Copy link
Member

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

@upodroid
Copy link
Member

upodroid commented Feb 4, 2022

@pseudorandoom
Copy link
Contributor

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

@pseudorandoom
Copy link
Contributor

/assign

@pseudorandoom
Copy link
Contributor

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 ?

@upodroid
Copy link
Member

upodroid commented Feb 9, 2022

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.

@pseudorandoom
Copy link
Contributor

pseudorandoom commented Feb 9, 2022

@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?

@upodroid
Copy link
Member

upodroid commented Feb 9, 2022

I'm fine with either. Try on:workflow_run trigger first and see how that goes.

@dprotaso
Copy link
Member Author

dprotaso commented Feb 9, 2022

Since this is a prow command wouldn't prow have the authority/token to approve the GitHub actions to run?

@pseudorandoom
Copy link
Contributor

It definitely should be possible to do it in Prow but the GitHub actions approach seemed easier to implement for me

@dprotaso
Copy link
Member Author

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

@pseudorandoom
Copy link
Contributor

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 ?

@pseudorandoom
Copy link
Contributor

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

@dprotaso
Copy link
Member Author

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

@dprotaso
Copy link
Member Author

It also looks like the approval needs to occur any time there's a new change (ie. pushed a commit) on the PR.

@krsna-m
Copy link
Contributor

krsna-m commented Feb 17, 2022

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).

@pseudorandoom
Copy link
Contributor

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

@krsna-m
Copy link
Contributor

krsna-m commented Mar 3, 2022

upstream issue: kubernetes/test-infra#25210

@github-actions
Copy link

github-actions bot commented Jun 2, 2022

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 2, 2022
@krsna-m
Copy link
Contributor

krsna-m commented Jun 2, 2022

/remove-lifecycle stale

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 2, 2022
@upodroid upodroid added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/github-actions Issues or PRs related to GitHub Actions labels Jun 13, 2022
@krsna-m
Copy link
Contributor

krsna-m commented Jul 28, 2022

@github-actions
Copy link

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2022
@dprotaso
Copy link
Member Author

dprotaso commented Nov 9, 2022

/lifecycle-frozen

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 10, 2022
@github-actions
Copy link

github-actions bot commented Feb 9, 2023

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 9, 2023
@dprotaso
Copy link
Member Author

dprotaso commented Feb 9, 2023

/lifecycle frozen

@knative-prow knative-prow bot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/github-actions Issues or PRs related to GitHub Actions lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
Status: Ready To Work
Development

No branches or pull requests

4 participants