github/ci: Switch prechecks to pull_request_target and fix#30126
github/ci: Switch prechecks to pull_request_target and fix#30126phlax merged 4 commits intoenvoyproxy:mainfrom
Conversation
phlax
changed the title
Signed-off-by: Ryan Northey <ryan@synca.io>
|
checking further - our policy currently discourages pull_request_targets but doesnt forbid - so i dont think update is necessary just now |
phlax
changed the title
| name: Checkout Envoy repository | ||
| with: | ||
| fetch-depth: ${{ ! (inputs.check_mobile_run || inputs.trusted) && 1 || 0 }} | ||
| fetch-depth: ${{ ! (inputs.check_mobile_run || ! startsWith(github.event_name, 'pull_request')) && 1 || 0 }} |
There was a problem hiding this comment.
How about the event triggered by the trusted bots?
There was a problem hiding this comment.
the condition is a bit convoluted - but it would match - ie not startswith pull_request == fetch_depth=0
There was a problem hiding this comment.
open to suggestions on how to make it clearer - with these pseudo ternaries your options are a bit limited
There was a problem hiding this comment.
Sorry, what I actually want to say is how about the event triggered by the untrusted bots. Seems that events also be treated as trusted.
There was a problem hiding this comment.
i see - at this point we havent derived the trusted state and the repo hasnt been checked out
re untrusted bots - they should never happen - but in the case they do i think this does the right thing
what we dont want is for them to be able to check out a specified commit
in this case they will get the head of the branch, in the next step they will be deemed untrusted, and the step after that they wont be able to rewind the commit
There was a problem hiding this comment.
just realizing tho - this is still not quite right - it needs to call the env action a second time i think
imagining a ci run to actually release, where the branch has been reopened by the time this gets triggered (ie not this actual workflow - but others that reuse _env.yml etc)
in this case it will trigger the run, checkout HEAD, derive env vars, but because the branch is reopened it will see the version as being dev - so i think we need to call the env action a second time with the correct commit
There was a problem hiding this comment.
hmm - that scheme wont quite work - but we need something like that
There was a problem hiding this comment.
k - updated with a bit of rearrangement - hopefully should work and will get/set correct commits at right times
|
pushing past unrelated coverage fail Code coverage for source/common/config is lower than limit of 95.4 (95.3) |
…y#30126) Signed-off-by: Ryan Northey <ryan@synca.io>
…y#30126) Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: phlax <phlax@users.noreply.github.com>
Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: phlax <phlax@users.noreply.github.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
2 participants
Currently the code checks for
inputs.trustedbefore that var has been derived/set - this fixes that issueIt also switches the github deps/prechecks wf to
pull_request_target- this will require an update in our ci policy, but is a necessary step to enabling RBE for these workflowsCommit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]