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

ci: don't run e2es on PRs from forks #127

Merged
merged 1 commit into from
Jan 9, 2024
Merged

ci: don't run e2es on PRs from forks #127

merged 1 commit into from
Jan 9, 2024

Conversation

kormide
Copy link
Collaborator

@kormide kormide commented Jan 8, 2024

E2es fail on pull requests from a fork because GitHub Actions doesn't allow access to secrets on the pull_request trigger. pull_request_target is supposed to allow this. I added the label run e2e to tag PRs to allow e2es to be run, the assumption being that we carefully look through the changes before tagging. In addition, outside contributions must be approved.

@cgrindel
Copy link
Member

cgrindel commented Jan 8, 2024

If this ends up being the workflow to allow CI runs from forks, we may want to create a markdown doc that documents this and any other interesting workflows (e.g. release).

@kormide kormide force-pushed the e2e-on-prs branch 2 times, most recently from 958a483 to 84a875c Compare January 9, 2024 02:54
@kormide
Copy link
Collaborator Author

kormide commented Jan 9, 2024

I changed my mind on this. According to this article there are still some security risks associated with the pull_request_target and adding a label approach.

I'm just going to disable e2e tests on PRs from forks, and they can run on the push event after they are merged.

@kormide kormide changed the title ci: runs e2es on pull reuqests when labelled ci: don't run e2es on PRs from forks Jan 9, 2024
@kormide kormide merged commit 414a540 into main Jan 9, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants