OCPBUGS-35387: rbac: Add network resources to cluster-reader role #2826
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
Add read permissions (i.e. get, list, watch) for network-related resources to the cluster-reader ClusterRole aggregation. The resources are: - egressrouters.network.operator.openshift.io - network-attachment-definitions.k8s.cni.cncf.io - networks.operator.openshift.io Fixes: OCPBUGS-35387
mattedallo
force-pushed
the
add-network-rbac-cluster-reader
branch
from
e8f6663 to
93ab506
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/retest |
mattedallo
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@mattedallo: This pull request references Jira Issue OCPBUGS-35387, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In 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 |
openshift-ci-robot
added
jira/valid-bug
|
@mattedallo: This pull request references Jira Issue OCPBUGS-35387, 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: In 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-required |
|
@mattedallo did you check if the cluster-reader role gets the permissions in an OCP cluster with the patch? |
|
Hi @pliurh , Yes I tested it. $ oc get clusterversion
NAME VERSION AVAILABLE PROGRESSING SINCE STATUS
version 4.19.0-0.nightly-2025-10-22-141259 True False 4m58s Cluster version is 4.19.0-0.nightly-2025-10-22-141259
$ oc new-project test-project
Now using project "test-project" on server "https://api.ci-ln-9wmpp2t-76ef8.origin-ci-int-aws.dev.rhcloud.com:6443".
$ oc create sa test-user -n test-project
serviceaccount/test-user created
$ oc auth can-i list network-attachment-definitions.k8s.cni.cncf.io --as=system:serviceaccount:test-project:test-user
no
$ oc auth can-i list egressrouters.network.operator.openshift.io --as=system:serviceaccount:test-project:test-user
no
$ oc auth can-i list networks.operator.openshift.io --as=system:serviceaccount:test-project:test-user
Warning: resource 'networks' is not namespace scoped in group 'operator.openshift.io'
no
$ oc adm policy add-cluster-role-to-user cluster-reader -z test-user -n test-project
clusterrole.rbac.authorization.k8s.io/cluster-reader added: "test-user"
$ oc auth can-i list network-attachment-definitions.k8s.cni.cncf.io --as=system:serviceaccount:test-project:test-user
yes
$ oc auth can-i list egressrouters.network.operator.openshift.io --as=system:serviceaccount:test-project:test-user
yes
$ oc auth can-i list networks.operator.openshift.io --as=system:serviceaccount:test-project:test-user
Warning: resource 'networks' is not namespace scoped in group 'operator.openshift.io'
yes
|
|
/retest |
| - apiGroups: ["network.operator.openshift.io"] | ||
| resources: | ||
| - egressrouters | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: ["operator.openshift.io"] | ||
| resources: | ||
| - networks | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: ["k8s.cni.cncf.io"] | ||
| resources: | ||
| - network-attachment-definitions | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch |
There was a problem hiding this comment.
The bug description mentions other CRDs as well. Shouldn't the other CRDs also be included?
We shall grant permissions to the view default role for network CRDs:
- adminnetworkpolicies.policy.networking.k8s.io
- baselineadminnetworkpolicies.policy.networking.k8s.io
- clusternetworks.network.openshift.io
- egressnetworkpolicies.network.openshift.io
- egressrouters.network.operator.openshift.io
- hostsubnets.network.openshift.io
- netnamespaces.network.openshift.io
- network-attachment-definitions.k8s.cni.cncf.io
- networks.operator.openshift.io
There was a problem hiding this comment.
I left a comment on the bug about this.
I will share it also here, let me know your thoughts :
"""
The following resources from the original ticket were intentionally excluded from the cluster-reader permissions in the main branch commit:
- clusternetworks.network.openshift.io
- egressnetworkpolicies.network.openshift.io
- hostsubnets.network.openshift.io
- netnamespaces.network.openshift.io
Rationale
These resources are deprecated as they are exclusively used by OpenShift SDN, which is no longer supported in current releases. This is documented in the API definitions:
// ClusterNetwork was used by OpenShift SDN.
// DEPRECATED: OpenShift SDN is no longer supported and this object is no longer used in any way by OpenShift.
Codebase Analysis
A thorough analysis of the cluster-network-operator codebase confirms these resources are only referenced in:
- OpenShift SDN implementation code
- pkg/network/openshift_sdn.go - Creates ClusterNetwork objects for SDN
- bindata/network/openshift-sdn/002-rbac.yaml - SDN RBAC permissions
- bindata/network/openshift-sdn/003-rbac-controller.yaml - SDN controller RBAC - Network type migration code (SDN ↔ OVN transitions)
- pkg/controller/operconfig/migration.go - Handles live migration scenarios - Test files
- Various test files in pkg/ directory
No references exist in core OVN-Kubernetes functionality, confirming these resources are not needed for normal OVN-Kubernetes operation.
Backporting Plan
For backports to release-4.16 and earlier versions that still support OpenShift SDN, these resources will be included in the cluster-reader permissions to maintain compatibility with clusters running OpenShift SDN.
Resources Actually Added
The commit correctly added the following non-deprecated network resources to cluster-reader:
- egressrouters.network.operator.openshift.io
- networks.operator.openshift.io
- network-attachment-definitions.k8s.cni.cncf.io
"""
|
/lgtm /cc @kyrtapz |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: arkadeepsen, mattedallo The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThree new read-only RBAC rules were added to the cluster-reader ClusterRole in the OVN-Kubernetes RBAC configuration. The additions grant permissions to access egressrouters, networks, and network-attachment-definitions resources across three API groups. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🔇 Additional comments (1)
Comment |
|
@mattedallo: The following tests failed, say
Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@kyrtapz when you have time please take a look. Thanks |
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: ["operator.openshift.io"] |
There was a problem hiding this comment.
While I am ok with the other two I am not really sure what is the general approach for operator.openshift.io CRDs across OCP.
From what I've seen there are no other operator CRDs added to the cluster-reader.
@pliurh @mattedallo were you able to find that out?
There was a problem hiding this comment.
@kyrtapz Thank you for raising this concern. I've checked the RBAC patterns across OpenShift operators, and I indeed it seems no operator.openshift.io resources are currently exposed to cluster-reader across the entire platform. Our proposed change would be the first exception to this pattern.
Investigation Findings / My understanding
Listing my findings/understanding below, most of them might be trivial for a seasoned Red Hat engineer but helps me validate if my understanding is correct.
Pattern Confirmed Across All Operators
I verified that zero out of 21 operator.openshift.io/v1 resources have aggregate-to-cluster-reader or aggregate-to-view labels.
For comparison, here are operators that do have cluster-reader access, but for different API groups (NOT operator.openshift.io):
- machine-config-operator: Exposes
machineconfiguration.openshift.ioandconfig.openshift.io(source) - cluster-samples-operator: Exposes
samples.operator.openshift.io(different API group) (source) - cluster-config-operator: Exposes only
config.openshift.io(source) - machine-api-operator: Exposes
machine.openshift.io(source)
The Current Architectural Pattern
The consistent pattern across OpenShift is:
- ✅ config.openshift.io → Exposed to cluster-reader (user-facing, high-level configuration)
- ✅ Custom operator API groups (e.g.,
samples.operator.openshift.io,machineconfiguration.openshift.io) → Exposed when appropriate - ❌ operator.openshift.io → NOT exposed to cluster-reader (implementation details, cluster-admin only)
Security Considerations
On the security side, looking at the operator.openshift.io/Network CRD data that would become readable does not seem harmful:
- ✅ No credential/secret fields in the API definition
- ✅ Mostly configuration data (CIDRs, MTU, ports, feature flags)
⚠️ RawCNIConfig is freeform JSON (could be misused, but user error not design flaw AFAIU)⚠️ Infrastructure topology info (collectors, syslog destinations)
Still, that doesn't mean it won't include something that shouldn't be readable by a cluster-reader in the future, especially if the architectural assumption is that this is an internal, non-customer-visible CRD.
How troubleshoot has been made possible so far?
Mostly making use of must-gather
must-gatherwith cluster-admin privileges (already attempts to collect networks.operator.openshift.io)- Offline analysis of must-gather output
Questions
As someone new to the team, I want to make sure I understand the tradeoffs here:
-
Breaking the pattern: Is the debugging benefit sufficient to justify being the first and only
operator.openshift.ioresource exposed to cluster-reader across the platform? -
Security review: Should we assess the risk of exposing implementation details like
RawCNIConfigand in general braking the current pattern? -
Alternatives: Could we instead:
- Enhance
config.openshift.io/Network.Statuswith additional troubleshooting fields? - Create a dedicated troubleshooting API?
- Enhance
I'm happy to pursue whichever direction the team thinks is best.
@pliurh please let me know your thoughts as you might have additional context on this.
There was a problem hiding this comment.
I don't think we shall break the pattern.
5 participants
Add read permissions (i.e. get, list, watch) for network-related resources to the cluster-reader ClusterRole aggregation. The resources are:
Fixes: OCPBUGS-35387