Define top level Permission for ci-schedule-compatibility workflow #5068
Conversation
karmada-bot
added
the
kind/cleanup
karmada-bot
added
the
size/XS
|
@aditya7302 goog job~ |
|
@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. |
|
Here is the link of the APIServer compatibility workflow(ci-schedule-compatibility.yaml) run with modified permissions : |
|
/assign @zhzhuang-zju |
|
|
| deployments: write # Needed to manage deployments for setting up the test environment. | ||
| statuses: read # Necessary to check the status of deployments before running tests. |
There was a problem hiding this comment.
@aditya7302 Can you explain the reason for adding these two permissions?
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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？
There was a problem hiding this comment.
@zhzhuang-zju here is the APIServer compatibility workflow run without deployments: write and statuses: read permissions : https://github.com/aditya7302/karmada/actions/runs/9694680710.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This is pending with #4929 before open a PR to working for update remove the step change kube-apiserver and kube-controller-manager version
There was a problem hiding this comment.
#5142 is merged and please go ahead!
There was a problem hiding this comment.
kindly ping @aditya7302
It seems @zhzhuang-zju is suggesting removing some permissions as per #5068 (comment).
|
kindly ping @aditya7302 |
Signed-off-by: aditya7302 <aditya7302@gmail.com>
RainbowMango
force-pushed
the
ci-schedule-compatibility-token-permission
branch
from
bce8c21
to
284b9de
Compare
|
Helped rebase and addressed the comments. Please take another look. @zhzhuang-zju |
|
thanks~ |
|
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 #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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
[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:
Approvers can indicate their approval by writing |
karmada-bot
added
the
approved
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?: