-
Notifications
You must be signed in to change notification settings - Fork 887
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
Define top level Permission for ci-schedule workflow #5069
Define top level Permission for ci-schedule workflow #5069
Conversation
Here is the link of the CI Schedule Workflow(ci-schedule.yml) run with modified permissions : |
@aditya7302 refer to https://github.com/aditya7302/karmada/actions/runs/9614970779/workflow, there seems be no restriction on the permissions of ci |
@zhzhuang-zju I am sorry It was mistake I didn't notice I will provide you with another link of the local run with permissions added. |
@zhzhuang-zju here is the updated link of the workflow run with added permissions : |
/lgtm |
.github/workflows/ci-schedule.yml
Outdated
@@ -3,6 +3,10 @@ on: | |||
schedule: | |||
# Run this workflow "At 18:00 UTC on Sunday and Saturday" | |||
- cron: '0 18 * * 0,6' | |||
|
|||
permissions: | |||
actions: write # Required for dynamic workflow control, enabling retrying of job steps in case of failure (actions: nick-fields/retry@v3.0.0). |
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.
@aditya7302 Whether write permission is required for action nick-fields/retry
? Refer to https://github.com/zhzhuang-zju/karmada/actions/runs/9758042445/job/26931766980, when I verify locally, I can enable retrying of job steps without write permission.
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.
@zhzhuang-zju after using the StepSecurity's online tool for this workflow.
The following was the output
name: FOSSA
on:
push:
# Exclude branches created by Dependabot to avoid triggering current workflow
# for PRs initiated by Dependabot.
branches-ignore:
- 'dependabot/**'
permissions: # added using https://github.com/step-security/secure-repo
contents: read
jobs:
fossa:
name: FOSSA
# prevent job running from forked repository, otherwise
# 1. running on the forked repository would fail as missing necessary secret.
# 2. running on the forked repository would use unnecessary GitHub Action time.
if: ${{ github.repository == 'karmada-io/karmada' }}
runs-on: ubuntu-22.04
steps:
- name: Harden Runner
uses: step-security/harden-runner@17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6 # v2.8.1
with:
egress-policy: audit
- name: checkout code
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- name: Run FOSSA scan and upload build data
uses: fossas/fossa-action@f61a4c0c263690f2ddb54b9822a719c25a7b608f # v1.3.1
with:
api-key: ${{secrets.FOSSA_API_KEY}}
I think actions: write permissions is not required.
1bac42c
to
e71813c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5069 +/- ##
=========================================
Coverage ? 28.21%
=========================================
Files ? 632
Lines ? 43568
Branches ? 0
=========================================
Hits ? 12291
Misses ? 30380
Partials ? 897
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
others LGTM, thanks
.github/workflows/ci-schedule.yml
Outdated
@@ -3,6 +3,9 @@ on: | |||
schedule: | |||
# Run this workflow "At 18:00 UTC on Sunday and Saturday" | |||
- cron: '0 18 * * 0,6' | |||
|
|||
permissions: | |||
contents: read # Necessary for fetching the repository contents, enabling actions like actions/checkout@v4 to work properly. |
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.
contents: read # Necessary for fetching the repository contents, enabling actions like actions/checkout@v4 to work properly. | |
contents: read |
there is no need to add comment to read
permissions, if have to, it should also be consistent across workflows
Signed-off-by: aditya7302 <aditya7302@gmail.com>
e71813c
to
25d5f83
Compare
thanks |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zhzhuang-zju The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Part of #5048
Special notes for your reviewer:
Does this PR introduce a user-facing change?: