-
Notifications
You must be signed in to change notification settings - Fork 146
GitHub Actions workflow updates #2471
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
Changes from all commits
eb13606
599cb69
a8a8fed
236f8fc
3d9b512
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,11 +31,18 @@ on: | |
| - reopened | ||
| - synchronize | ||
|
|
||
| # Disable permissions for all available scopes by default. | ||
| # Any needed permissions should be configured at the job level. | ||
| permissions: {} | ||
|
|
||
| jobs: | ||
| php-test-plugins: | ||
| name: 'PHP ${{ matrix.php }} / WP ${{ matrix.wp }}' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 20 | ||
| permissions: | ||
| contents: read # Required to clone the repo. | ||
| actions: write # Required by styfle/cancel-workflow-action to cancel prior runs. | ||
|
desrosj marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unrelated to this, but need to audit third-party actions here.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @westonruter
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thelovekesh Good call out. I will resolve my review comment stating the same.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've opened #2472 to address this. |
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,16 +19,24 @@ on: | |
| env: | ||
| LABELS: ${{ join( github.event.pull_request.labels.*.name, ' ' ) }} | ||
|
|
||
| # Disable permissions for all available scopes by default. | ||
| # Any needed permissions should be configured at the job level. | ||
| permissions: {} | ||
|
|
||
| jobs: | ||
| check-type-label: | ||
| name: Check [Type] Label | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| permissions: {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe redundant given permission are already disabled on workflow level?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion, it's good to leave this and be explicit just in case the workflow-level permissions are changed unintentionally/unknowingly. @johnbillion what do you think?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the idea here is to be as explicit as possible. Defence in depth, etc.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good |
||
| steps: | ||
| - if: contains( env.LABELS, '[Type]' ) == false | ||
| run: exit 1 | ||
| check-milestone: | ||
| name: Check Milestone | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| permissions: {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
| steps: | ||
| - if: github.event.pull_request.milestone == null && contains( env.LABELS, 'no milestone' ) == false | ||
| run: exit 1 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
safe to keep but not required for public repos.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With
permissions: {}being set at the workflow level, I think the idea here is to be explicit instead of relying on an implicit fallback.It's also good to have just in case the workflow's overall permissions change for some reason, or someone creates a private fork or mirror (which does happen for some of the important repositories in the WordPress org).
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it also facilitates contributors running the workflows on a private fork. See: