Define top level Permission for ci-schedule-compatibility workflow #5068

aditya7302 · 2024-06-20T07:26:36Z

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?:

NONE

zhzhuang-zju · 2024-06-21T01:51:08Z

@aditya7302 goog job~
Since this ci is executed periodically and is not running this time, have you tried to see if you can run this workflow properly with modified permissions?

aditya7302 · 2024-06-21T06:00:49Z

@zhzhuang-zju I am trying to run this workflow on my local to test it but I couldn't figure it out.

zhzhuang-zju · 2024-06-21T06:18:09Z

@zhzhuang-zju I am trying to run this workflow on my local to test it but I couldn't figure it out.

You can post the link to your local test.

aditya7302 · 2024-06-21T12:40:04Z

Here is the link of the APIServer compatibility workflow(ci-schedule-compatibility.yaml) run with modified permissions : https://github.com/aditya7302/karmada/actions/runs/9611101767

RainbowMango · 2024-06-24T06:18:08Z

/assign @zhzhuang-zju

zhzhuang-zju · 2024-06-25T02:03:00Z

~~/lgtm~~
cc @RainbowMango

zhzhuang-zju · 2024-06-26T02:19:12Z

.github/workflows/ci-schedule-compatibility.yaml

+  deployments: write # Needed to manage deployments for setting up the test environment.
+  statuses: read # Necessary to check the status of deployments before running tests.


@aditya7302 Can you explain the reason for adding these two permissions?

@zhzhuang-zju
deployments: write ->This permission is necessary to manage deployments for setting up the test environment. We need to update the images which involves setting up new images. This requires write access to deployments.

- name: change kube-apiserver and kube-controller-manager version run: | kubectl --kubeconfig=${HOME}/.kube/karmada.config --context=karmada-host \ set image deployment/karmada-apiserver -nkarmada-system \ karmada-apiserver=registry.k8s.io/kube-apiserver:${{ matrix.kubeapiserver-version }} kubectl --kubeconfig=${HOME}/.kube/karmada.config --context=karmada-host \ set image deployment/karmada-kube-controller-manager -nkarmada-system \ kube-controller-manager=registry.k8s.io/kube-controller-manager:${{ matrix.kubeapiserver-version }}

statuses: read ->This permission is essential to check the status of deployments before running tests.

kubectl --kubeconfig=${HOME}/.kube/karmada.config --context=karmada-host \ rollout status deployment/karmada-kube-controller-manager -nkarmada-system --timeout=5m kubectl --kubeconfig=${HOME}/.kube/karmada.config --context=karmada-host \ rollout status deployment/karmada-apiserver -nkarmada-system --timeout=5m

Refer to https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs, permission deployment and statused are not actually a k8s concept. We might don't need those two permissions. Can you remove those two permissions and try it out？

@zhzhuang-zju here is the APIServer compatibility workflow run without deployments: write and statuses: read permissions : https://github.com/aditya7302/karmada/actions/runs/9694680710.

I've tried this locally and had similar results to yours, a couple of jobs would fail. Might have to configure out the reason for that

@aditya7302 It looks like it's all failing at step change kube-apiserver and kube-controller-manager version. @liangyuanpeng is trying to remove that step, so we can wait for his part of the work to be done before defining top level permission for ci -schedule-compatibility workflow, what do you think?

I also think that some jobs are failing at step change kube-apiserver and kube-controller-manager version. I will wait for @liangyuanpeng to remove this step and until then I will look for other good-first and help-wanted issues to work on.

This is pending with #4929 before open a PR to working for update remove the step change kube-apiserver and kube-controller-manager version

#5142 is merged and please go ahead!

kindly ping @aditya7302
It seems @zhzhuang-zju is suggesting removing some permissions as per #5068 (comment).

zhzhuang-zju · 2024-08-01T12:29:49Z

kindly ping @aditya7302

Signed-off-by: aditya7302 <aditya7302@gmail.com>

RainbowMango · 2024-08-06T04:08:29Z

Helped rebase and addressed the comments. Please take another look. @zhzhuang-zju

zhzhuang-zju · 2024-08-06T04:09:27Z

thanks~
/lgtm

codecov-commenter · 2024-08-06T04:21:49Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.39%. Comparing base (4124ba8) to head (284b9de).
Report is 2 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5068      +/-   ##
==========================================
+ Coverage   28.38%   28.39%   +0.01%     
==========================================
  Files         632      632              
  Lines       43798    43798              
==========================================
+ Hits        12431    12436       +5     
+ Misses      30462    30459       -3     
+ Partials      905      903       -2

Flag	Coverage Δ
unittests	`28.39% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

RainbowMango

/approve

karmada-bot · 2024-08-06T06:38:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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:

~~.github/workflows/OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jun 20, 2024

karmada-bot requested review from chaosi-zju and jwcesign June 20, 2024 07:26

karmada-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 20, 2024

zhzhuang-zju mentioned this pull request Jun 21, 2024

[Umbrella] Enhancement of Karmada maturity based on Clomonitor check sets #5048

Open

24 tasks

karmada-bot assigned zhzhuang-zju Jun 24, 2024

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2024

zhzhuang-zju reviewed Jun 26, 2024

View reviewed changes

Define top level Permission for ci-schedule-compatibility workflow

284b9de

Signed-off-by: aditya7302 <aditya7302@gmail.com>

RainbowMango force-pushed the ci-schedule-compatibility-token-permission branch from bce8c21 to 284b9de Compare August 6, 2024 04:07

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2024

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2024

RainbowMango approved these changes Aug 6, 2024

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 6, 2024

karmada-bot merged commit 19f3529 into karmada-io:master Aug 6, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define top level Permission for ci-schedule-compatibility workflow #5068

Define top level Permission for ci-schedule-compatibility workflow #5068

aditya7302 commented Jun 20, 2024 •

edited

Loading

zhzhuang-zju commented Jun 21, 2024

aditya7302 commented Jun 21, 2024

zhzhuang-zju commented Jun 21, 2024

aditya7302 commented Jun 21, 2024 •

edited

Loading

RainbowMango commented Jun 24, 2024

zhzhuang-zju commented Jun 25, 2024 •

edited

Loading

zhzhuang-zju Jun 26, 2024

aditya7302 Jun 26, 2024

zhzhuang-zju Jun 27, 2024

aditya7302 Jun 27, 2024

zhzhuang-zju Jun 29, 2024

zhzhuang-zju Jun 29, 2024

aditya7302 Jun 29, 2024

liangyuanpeng Jul 3, 2024 •

edited

Loading

liangyuanpeng Jul 8, 2024

RainbowMango Jul 16, 2024

zhzhuang-zju commented Aug 1, 2024

RainbowMango commented Aug 6, 2024

zhzhuang-zju commented Aug 6, 2024

codecov-commenter commented Aug 6, 2024

RainbowMango left a comment

karmada-bot commented Aug 6, 2024

		deployments: write # Needed to manage deployments for setting up the test environment.
		statuses: read # Necessary to check the status of deployments before running tests.

Define top level Permission for ci-schedule-compatibility workflow #5068

Define top level Permission for ci-schedule-compatibility workflow #5068

Conversation

aditya7302 commented Jun 20, 2024 • edited Loading

zhzhuang-zju commented Jun 21, 2024

aditya7302 commented Jun 21, 2024

zhzhuang-zju commented Jun 21, 2024

aditya7302 commented Jun 21, 2024 • edited Loading

RainbowMango commented Jun 24, 2024

zhzhuang-zju commented Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liangyuanpeng Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhzhuang-zju commented Aug 1, 2024

RainbowMango commented Aug 6, 2024

zhzhuang-zju commented Aug 6, 2024

codecov-commenter commented Aug 6, 2024

Codecov Report

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Aug 6, 2024

aditya7302 commented Jun 20, 2024 •

edited

Loading

aditya7302 commented Jun 21, 2024 •

edited

Loading

zhzhuang-zju commented Jun 25, 2024 •

edited

Loading

liangyuanpeng Jul 3, 2024 •

edited

Loading