Don't use author_association for self-hosted vs public runner decision. #14718
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using this has two draw-backs for us. 1. MEMBER applies to _anyone in the org_, not just members/commiters to this repo 2. The value of this setting depends upon the user's "visiblity" in the org. I.e. if they hide their membership of the org, the author_association will show up as "CONTRIBUTOR" instead. Both of these combined mean we should instead use an alternative list. Airflow committers is a scret that contains a list of GH user ids, such as `["ashb", "..."]` etc.
Member
Author
|
Hmmm. |
Member
Author
|
Damn, It seems you can't reference secrets nor env in the |
Member
Author
|
Trying to include a hard-coded list in the ci.yaml (it's verifired/checked again on the runner so is "safe") fails too:
Damn, not sure this is actually possible :/ |
Member
Author
|
|
|
The Workflow run is cancelling this PR. Building image for the PR has been cancelled |
kaxil
approved these changes
Mar 11, 2021
Member
Author
|
Down to at least one hard-coded list now. |
ashb
commented
Mar 11, 2021
Member
Author
|
There, working, and not too horrible/duplictative. I would have liked to be able to use secrets, but sadly not possible. |
Member
Author
|
Jobs after the first are running on self-hosted runners, don't need to wait on tests. |
potiuk
reviewed
Mar 11, 2021
|
|
||
| env: | ||
|
|
||
| AIRFLOW_COMMITERS: ${{ secrets.AIRFLOW_COMMITERS }} |
Member
There was a problem hiding this comment.
I guess we can remove this one? That turned out to be not working for forked PRs I believe?
potiuk
added a commit
to potiuk/airflow
that referenced
this pull request
Mar 12, 2021
The change apache#14718 by mistake left the 'self-hosted" runs-on in case of push or schedule. This caused failures on non-apache repositories.
potiuk
added a commit
that referenced
this pull request
Mar 12, 2021
The change #14718 by mistake left the 'self-hosted" runs-on in case of push or schedule. This caused failures on non-apache repositories.
potiuk
pushed a commit
that referenced
this pull request
Mar 23, 2021
…n. (#14718) Using this has two draw-backs for us. 1. MEMBER applies to _anyone in the org_, not just members/commiters to this repo 2. The value of this setting depends upon the user's "visiblity" in the org. I.e. if they hide their membership of the org, the author_association will show up as "CONTRIBUTOR" instead. Both of these combined mean we should instead use an alternative list. We can't use a secret as the `secrets.` context is not available in the runs-on stanza, so we have to have a hard-coded list in the workflow file :( This is as secure as the runner still checks the author against it's own list. (cherry picked from commit 4213487)
potiuk
added a commit
that referenced
this pull request
Mar 23, 2021
ashb
added a commit
that referenced
this pull request
Apr 15, 2021
…n. (#14718) Using this has two draw-backs for us. 1. MEMBER applies to _anyone in the org_, not just members/commiters to this repo 2. The value of this setting depends upon the user's "visiblity" in the org. I.e. if they hide their membership of the org, the author_association will show up as "CONTRIBUTOR" instead. Both of these combined mean we should instead use an alternative list. We can't use a secret as the `secrets.` context is not available in the runs-on stanza, so we have to have a hard-coded list in the workflow file :( This is as secure as the runner still checks the author against it's own list. (cherry picked from commit 4213487)
ashb
pushed a commit
that referenced
this pull request
Apr 15, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using this has two draw-backs for us.
Both of these combined mean we should instead use an alternative list. Airflow committers is a scret that contains a list of GH user ids, such as
["ashb", "..."]etc.