OCPBUGS-65621: add dedicated service account to crb, cvo and version pod#1266
OCPBUGS-65621: add dedicated service account to crb, cvo and version pod#1266ehearne-redhat wants to merge 24 commits intoopenshift:mainfrom
Conversation
WalkthroughThis PR refactors service account configuration for cluster-version-operator and update-payload components by introducing dedicated ServiceAccounts with explicit RBAC bindings, removing the existing default ServiceAccount cluster-admin binding, and updating manifests and code to reference the new service accounts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ehearne-redhat The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
|
/test e2e-aws-ovn-upgrade |
|
/test e2e-aws-ovn-techpreview |
|
/retest |
|
/retest |
|
@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65621, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65621, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65621, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
7 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
1 similar comment
|
/retest |
|
/test e2e-aws-ovn-techpreview |
|
/test okd-scos-images |
install/0000_00_cluster-version-operator_02_service_account.yaml
Outdated
Show resolved
Hide resolved
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRoleBinding | ||
| metadata: | ||
| name: cluster-version-operator-1 |
There was a problem hiding this comment.
Redundant vs. the cluster-version-operator you declare down at the end of this file?
There was a problem hiding this comment.
Actually, this one is interesting. So, in order to get the into-change and out-of-change tests to pass, these two CRBs play different roles.
cluster-version-operator CRB is for the out-of-change test. It looks for this binding so it has the necessary permissions for the default service account to conduct itself.
cluster-version-operator-1 CRB is for the into-change test. This binding ensures the new service account cluster-version-operator has the necessary permissions to conduct itself.
Without cluster-version-operator CRB, the default service account appears to not have the necessary permissions and fail, probably because of the naming of the CRBs. This is important for the out-of-change test to pass.
| namespace: openshift-cluster-version | ||
| roleRef: | ||
| kind: ClusterRole | ||
| name: cluster-admin |
There was a problem hiding this comment.
Update-payload Pod doesn't make Kube API calls at all, so I don't think we need this cluster-version-operator-payload ClusterRoleBinding.
There was a problem hiding this comment.
I've commented it out to test this. If proves true in testing, I'll remove entirely.
| k8s-app: cluster-version-operator | ||
| spec: | ||
| automountServiceAccountToken: false | ||
| serviceAccountName: cluster-version-operator |
There was a problem hiding this comment.
Can you either add this to the bootstrap manifest too, or have a commit message that mentions why we don't need a service account for that bootstrap manifest?
In that vein, you might want to reshuffle your existing commit stack to try and tell the transformation story in a more narrative arc. It is completely fine to take a bunch of commits, if you need more space to talk about each pivot in a series. But at the moment, there are things like fc55fa5, which sounds like useful context to include in a "why I did things this way..." commit message in a commit that adds the new role-bindings. But I don't see a benefit to keeping it completely separate, vs. having a single commit that brings in the finished roll bindings and then explains all the context you need to explain that finished shape.
There was a problem hiding this comment.
I've added it to the bootstrap manifest. I'll wait to see how tests behave before squashing the commits into a narrative commit, or a collection of them depending.
|
/retest |
|
@ehearne-redhat: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What
openshift-cluster-versionnamespace.Why