-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-48770: [CI] Add missing permissions declaration to workflows #48771
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
Conversation
|
|
|
|
||
| permissions: | ||
| actions: read | ||
| contents: read |
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.
Required by actions/checkout
| workflow_call: | ||
|
|
||
| permissions: | ||
| actions: read |
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.
Required by archery ci report-email and archery ci report-chat
| type: string | ||
|
|
||
| permissions: | ||
| contents: read |
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.
Required by actions/checkout
| value: ${{ jobs.check-labels.outputs.force }} | ||
|
|
||
| permissions: | ||
| contents: read |
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.
Required by actions/checkout
Required by gh pr view
|
Hmm... Why extra CI jobs aren't run...? Anyway, could you add this? diff --git a/.github/workflows/cpp_extra.yml b/.github/workflows/cpp_extra.yml
index 612175e60f..4b2290d077 100644
--- a/.github/workflows/cpp_extra.yml
+++ b/.github/workflows/cpp_extra.yml
@@ -26,6 +26,7 @@ on:
- '.dockerignore'
- '.github/workflows/check_labels.yml'
- '.github/workflows/cpp_extra.yml'
+ - '.github/workflows/cpp_windows.yml'
- '.github/workflows/report_ci.yml'
- 'ci/conda_env_*'
- 'ci/docker/**'
@@ -47,6 +48,7 @@ on:
- '.dockerignore'
- '.github/workflows/check_labels.yml'
- '.github/workflows/cpp_extra.yml'
+ - '.github/workflows/cpp_windows.yml'
- '.github/workflows/report_ci.yml'
- 'ci/conda_env_*'
- 'ci/docker/**' |
|
Sure! |
|
Let me try to run them in this PR .. one sec ... |
This reverts commit a54ab72.
|
Tests passed at a54ab72. Should be good to have a look! |
kou
left a comment
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.
+1
But it seems that paths: doesn't work as expected...
|
Let me open another quick PR and see if this is triggered. |
…48778) ### Rationale for this change Workflows `cpp_extra.yml`, `r_extra.yml` and `package_linux.yml` call reusable workflows (`check_labels.yml` and `report_ci.yml`) that require specific permissions. When #48771 added explicit permissions to these reusable workflows, the calling workflows were not updated to give those permissions. This caused `startup_failure` errors when these workflows were triggered on pull requests. Here are example failures: https://github.com/apache/arrow/actions/runs/20809257825 and https://github.com/apache/arrow/actions/runs/20803198596 ### What changes are included in this PR? Added missing permissions to the workflows ### Are these changes tested? Yes, I tested them within this PR. ### Are there any user-facing changes? No, dev-only. * GitHub Issue: #48780 Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit fdeac0b. There weren't enough matching historic benchmark results to make a call on whether there were regressions. The full Conbench report has more details. |
Rationale for this change
Adds missing
permissions:declaration to workflows, following the security best practices (see also #35708).What changes are included in this PR?
Adds workflow-level permissions to workflows
Are these changes tested?
Tested in a54ab72
Are there any user-facing changes?
No, dev-only.