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

Only run CTS issue workflow when opening PRs; run on base branch #597

Merged
merged 3 commits into from
Aug 2, 2024

Conversation

psalz
Copy link
Member

@psalz psalz commented Aug 1, 2024

Turns out the pull_request trigger by default also runs when PRs are updated or re-opened. Change to only run on the opened activity.

Additionally, I forgot that using the pull_request trigger doesn't allow the workflow to access repository secrets when the triggering PR originates from a fork. Thankfully there is a pull_request_target trigger to remedy this, which runs the workflow in the context of the base branch (i.e., changes to the workflow itself aren't executed). This is fine for our use case, as we don't need to access the PR's contents within the workflow.

(Side note: I marked this as editorial to not run the workflow - maybe we need another label for these kinds of infrastructure changes). Edit: I've now limited the workflow to only run when changes are made inside the adoc directory.

Turns out the `pull_request` trigger by default also runs when PRs are
updated or re-opened. Change to only run on the `opened` activity.
@psalz psalz added the editorial Some purely editorial problem label Aug 1, 2024
@psalz psalz removed the editorial Some purely editorial problem label Aug 1, 2024
This allows the workflow to access repository secrets even if the
triggering PR originates from a fork. This has some security
implications, see the comment within the workflow.
@psalz psalz changed the title Only run CTS issue workflow when opening PRs Only run CTS issue workflow when opening PRs; run on base branch Aug 2, 2024
@gmlueck gmlueck added the editorial Some purely editorial problem label Aug 2, 2024
@gmlueck
Copy link
Contributor

gmlueck commented Aug 2, 2024

I marked this "editorial" because it does not change the specification. Therefore, I think it could be merged without WG approval.

@psalz: I'll wait to hear from you before I merge this. Let me know when you think it is ready.

@psalz
Copy link
Member Author

psalz commented Aug 2, 2024

I marked this "editorial" because it does not change the specification. Therefore, I think it could be merged without WG approval.

@psalz: I'll wait to hear from you before I merge this. Let me know when you think it is ready.

Should be good to go!

@gmlueck
Copy link
Contributor

gmlueck commented Aug 2, 2024

Merging as editorial

@gmlueck gmlueck merged commit 976ac9b into SYCL-2020/master Aug 2, 2024
3 checks passed
@psalz psalz deleted the fix-cts-issue-workflow-trigger branch August 2, 2024 13:17
keryell pushed a commit that referenced this pull request Sep 10, 2024
Only run CTS issue workflow when opening PRs; run on base branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Some purely editorial problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants