Fix incorrect "fork check" condition in validate_pr workflow#2161
Fix incorrect "fork check" condition in validate_pr workflow#2161
validate_pr workflow#2161Conversation
c076e4f to
cfcddc2
Compare
cfcddc2 to
d36f598
Compare
| validate-pr: | ||
| name: build | ||
| if: github.repository_owner == 'elastic' # this action will fail if executed from a fork | ||
| if: github.event.pull_request.head.repo.owner.login == 'elastic' # this action will fail if executed from a fork |
There was a problem hiding this comment.
I'm not familiar with the github object model. This test is because the action checks out elastic/clients-flight-recorder which is a private repo. So the condition should test that the user running the action is a member of the elastic org. I don't think this is what this new expression will test?
There was a problem hiding this comment.
Correct. If the workflow is triggered from a fork, the secrets context is not available and in consequence no token/PAT is supplied to the checkout action which ultimately causes the workflow to fail.
The condition seems to be intended to work around that problem by disabling the workflow if it was triggered from a fork.
The expression used in the original condition however does not work for that purpose, because github.repository_owner always evaluates to the BASE/target repository (elastic/elasticsearch-specification in this case). This for example happened in #2160.
To check the HEAD/source branch instead, we have to use github.event.pull_request.head.repo.owner.login. This expression would evaluate to e.g. flobernd if I trigger this workflow from a fork (e.g. flobernd/elasticsearch-specification).
There was a problem hiding this comment.
Probably this condition is still not the perfect one to use, because even a fork from inside the elastic organization (e.g. elastic/elasticsearch-specification-clone) might not have access to the secrets context. However there might be a special rule for "inside organization" forks that I don't remember right now.
This expression:
elasticsearch-specification/.github/workflows/validate-pr.yml
Line 18 in 9eff69a
always evaluates to
trueasgithub.repository_ownerrefers to the "target" repository forpull_requesttriggers.It should instead check
github.event.pull_request.head.repo.owner.login.Related: #2160