Add backplane RBAC for RHOBS team access to regional clusters#2708
Add backplane RBAC for RHOBS team access to regional clusters#2708moadz wants to merge 1 commit intoopenshift:masterfrom
Conversation
WalkthroughAdds RBAC and deployment configuration for RHOBS regional clusters: a new namespace, selector-based deployment config, a ClusterRole granting read/list/watch permissions on monitoring and infra resources, a SubjectPermission binding that ClusterRole to RHOBS serviceaccounts, and README updates listing the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: moadz 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@deploy/backplane/rhobs/rhobs-cluster/10-srep-rhobs-cluster-project.ClusterRole.yml`:
- Around line 18-26: Update the RBAC rule block for monitoring resources: change
the apiGroups entry from "monitoring.rhobs.io" to "monitoring.rhobs" and
normalize indentation to 2 spaces for the apiGroups/resources/verbs block so it
matches other rule blocks; target the rule that lists resources
"monitoringstacks" and "thanosqueriers" and ensure the apiGroups, resources, and
verbs keys are aligned with the 2-space style used elsewhere.
In `@deploy/backplane/rhobs/rhobs-cluster/20-srep.SubjectPermission.yml`:
- Line 4: The resource metadata.name is stale: change the SubjectPermission
resource name from "backplane-srep-rhobs-regional-cluster" to
"backplane-rhobs-regional-cluster" to match the bound service-account group
"openshift-backplane-rhobs" and ClusterRole
"backplane-rhobs-regional-cluster-project"; update any internal references if
present and also consider renaming the file from "20-srep.SubjectPermission.yml"
to "20-rhobs.SubjectPermission.yml" so filename and resource name are
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2de97ae8-a176-4281-8d74-c92196a84179
📒 Files selected for processing (4)
deploy/backplane/README.mddeploy/backplane/rhobs/rhobs-cluster/10-srep-rhobs-cluster-project.ClusterRole.ymldeploy/backplane/rhobs/rhobs-cluster/20-srep.SubjectPermission.ymldeploy/backplane/rhobs/rhobs-cluster/config.yaml
| apiVersion: managed.openshift.io/v1alpha1 | ||
| kind: SubjectPermission | ||
| metadata: | ||
| name: backplane-srep-rhobs-regional-cluster |
There was a problem hiding this comment.
Stale srep in resource name; rename for consistency with the new RHOBS subject.
metadata.name is backplane-srep-rhobs-regional-cluster, but this SubjectPermission binds the openshift-backplane-rhobs service-account group (not srep) to the backplane-rhobs-regional-cluster-project ClusterRole. Keeping srep in the name looks like a copy-paste leftover from deploy/backplane/srep/hypershift/rhobs-cluster/20-srep.SubjectPermission.yml and will likely collide conceptually with that existing resource. Consider renaming to match.
🧹 Proposed fix
metadata:
- name: backplane-srep-rhobs-regional-cluster
+ name: backplane-rhobs-regional-cluster
namespace: openshift-rbac-permissionsConsider also renaming the file from 20-srep.SubjectPermission.yml to 20-rhobs.SubjectPermission.yml for the same reason.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: backplane-srep-rhobs-regional-cluster | |
| metadata: | |
| name: backplane-rhobs-regional-cluster | |
| namespace: openshift-rbac-permissions |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/backplane/rhobs/rhobs-cluster/20-srep.SubjectPermission.yml` at line
4, The resource metadata.name is stale: change the SubjectPermission resource
name from "backplane-srep-rhobs-regional-cluster" to
"backplane-rhobs-regional-cluster" to match the bound service-account group
"openshift-backplane-rhobs" and ClusterRole
"backplane-rhobs-regional-cluster-project"; update any internal references if
present and also consider renaming the file from "20-srep.SubjectPermission.yml"
to "20-rhobs.SubjectPermission.yml" so filename and resource name are
consistent.
moadz
force-pushed
the
rhobs-rbac-backplane
branch
2 times, most recently
from
0a598dc to
37addfb
Compare
typeid
left a comment
typeid
left a comment
There was a problem hiding this comment.
Let's fix the file and namespace names as commented, then this looks good to me.
moadz
force-pushed
the
rhobs-rbac-backplane
branch
from
37addfb to
1512070
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml`:
- Around line 75-88: The ClusterRole in this file mixes cluster-scoped rules
(resources: clusterroles, clusterrolebindings, customresourcedefinitions) with a
namespaced SubjectPermission that uses namespacesAllowedRegex, which is invalid;
split out a new ClusterRole containing only the cluster-scoped resources
(clusterroles, clusterrolebindings, customresourcedefinitions) and remove those
resources from the existing ClusterRole, then bind the new ClusterRole via an
explicit clusterPermissions entry on the SubjectPermission (follow the binding
pattern used in 50-dedicated-admins.SubjectPermission.yaml) so namespace
restrictions remain on the remaining namespaced rules and cluster-scoped rules
are granted at cluster scope.
- Around line 18-49: The RBAC rules currently mix Prometheus Operator resources
into the monitoring.rhobs apiGroup; update the ClusterRole so Prometheus
Operator-owned resources (alertmanagers, podmonitors, prometheuses,
prometheusrules, servicemonitors) are listed under apiGroups:
["monitoring.coreos.com"] and RHOBS-specific resources (monitoringstacks,
uiplugins and any RHOBS-owned variants) remain under apiGroups:
["monitoring.rhobs"]; remove resources that are not actually deployed here
(alertmanagerconfigs, probes, prometheusagents, scrapeconfigs, thanosrulers) and
verify which apiGroup owns thanosqueriers in your environment before assigning
it (leave it in monitoring.rhobs only if RHOBS owns it).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 967858b9-6fe6-4d5c-9a90-a62b202a06a8
📒 Files selected for processing (5)
deploy/backplane/README.mddeploy/backplane/rhobs/00-rhobs.namespace.ymldeploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.ymldeploy/backplane/rhobs/20-rhobs.SubjectPermission.ymldeploy/backplane/rhobs/config.yaml
✅ Files skipped from review due to trivial changes (4)
- deploy/backplane/README.md
- deploy/backplane/rhobs/00-rhobs.namespace.yml
- deploy/backplane/rhobs/config.yaml
- deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml
| - apiGroups: | ||
| - monitoring.rhobs | ||
| resources: | ||
| - alertmanagerconfigs | ||
| - alertmanagers | ||
| - monitoringstacks | ||
| - podmonitors | ||
| - probes | ||
| - prometheusagents | ||
| - prometheuses | ||
| - prometheusrules | ||
| - scrapeconfigs | ||
| - servicemonitors | ||
| - thanosqueriers | ||
| - thanosrulers | ||
| - uiplugins | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - monitoring.coreos.com | ||
| resources: | ||
| - alertmanagers | ||
| - prometheuses | ||
| - prometheusrules | ||
| - thanosqueriers | ||
| - servicemonitors | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect CRD/API-version evidence for the resources in the monitoring RBAC rules.
# Expectation: Prometheus Operator resources resolve to monitoring.coreos.com; RHOBS resources resolve to monitoring.rhobs.
rg -nP --type=yaml -C3 \
'apiVersion:\s*(monitoring\.coreos\.com|monitoring\.rhobs|observability\.openshift\.io|console\.openshift\.io)/|name:\s*(alertmanagerconfigs|alertmanagers|podmonitors|probes|prometheusagents|prometheuses|prometheusrules|scrapeconfigs|servicemonitors|thanosrulers|thanosqueriers|monitoringstacks|uiplugins)\.'Repository: openshift/managed-cluster-config
Length of output: 37244
Correct API group ownership for Prometheus Operator and RHOBS resources.
Resources are currently misplaced across API groups. Verification confirms that prometheusrules, podmonitors, and servicemonitors actually exist in both monitoring.coreos.com and monitoring.rhobs, but the current RBAC rule incorrectly bundles Prometheus Operator resources under monitoring.rhobs. This breaks RBAC enforcement for read-only access to resources that belong under their correct API groups.
Recommended fix:
- Move Prometheus Operator resources (
alertmanagers,podmonitors,prometheuses,prometheusrules,servicemonitors) tomonitoring.coreos.com. - Keep RHOBS-specific resources (
monitoringstacks,uiplugins) and any RHOBS-owned variants of shared resources undermonitoring.rhobs. - Note:
thanosqueriersrequires clarification—verify which API group actually owns this resource in your environment; evidence suggests it may only exist inmonitoring.rhobs. - Remove resources not actually deployed in this repository (e.g.,
alertmanagerconfigs,probes,prometheusagents,scrapeconfigs,thanosrulersif they are not in use).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml` around lines
18 - 49, The RBAC rules currently mix Prometheus Operator resources into the
monitoring.rhobs apiGroup; update the ClusterRole so Prometheus Operator-owned
resources (alertmanagers, podmonitors, prometheuses, prometheusrules,
servicemonitors) are listed under apiGroups: ["monitoring.coreos.com"] and
RHOBS-specific resources (monitoringstacks, uiplugins and any RHOBS-owned
variants) remain under apiGroups: ["monitoring.rhobs"]; remove resources that
are not actually deployed here (alertmanagerconfigs, probes, prometheusagents,
scrapeconfigs, thanosrulers) and verify which apiGroup owns thanosqueriers in
your environment before assigning it (leave it in monitoring.rhobs only if RHOBS
owns it).
| - rbac.authorization.k8s.io | ||
| resources: | ||
| - clusterroles | ||
| - clusterrolebindings | ||
| - roles | ||
| - rolebindings | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - apiextensions.k8s.io | ||
| resources: | ||
| - customresourcedefinitions |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find how SubjectPermission models cluster-scoped access elsewhere.
# Expectation: Cluster-scoped resources should use a distinct cluster-wide permission mechanism, not only namespace regex bindings.
rg -nP --type=yaml -C6 \
'kind:\s*SubjectPermission|clusterPermissions|namespacesAllowedRegex|clusterroles|clusterrolebindings|customresourcedefinitions'Repository: openshift/managed-cluster-config
Length of output: 50389
🏁 Script executed:
# Find the RHOBS ClusterRole file
fd -t f "rhobs" | head -20Repository: openshift/managed-cluster-config
Length of output: 394
🏁 Script executed:
# Look for the actual file and RHOBS SubjectPermission
rg -l "rhobs" --type=yaml | grep -i subjectRepository: openshift/managed-cluster-config
Length of output: 206
🏁 Script executed:
# Search for references to "rhobs-cluster-project" ClusterRole
rg "rhobs-cluster-project" --type=yaml -A5 -B5Repository: openshift/managed-cluster-config
Length of output: 58
🏁 Script executed:
# View the RHOBS SubjectPermission to see how rhobs ClusterRole is used
cat deploy/backplane/rhobs/20-rhobs.SubjectPermission.ymlRepository: openshift/managed-cluster-config
Length of output: 502
🏁 Script executed:
# View the complete RHOBS ClusterRole to see all rules
cat deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.ymlRepository: openshift/managed-cluster-config
Length of output: 1750
Cluster-scoped resources must not be mixed with namespace-scoped access rules in this ClusterRole.
The ClusterRole includes clusterroles, clusterrolebindings, and customresourcedefinitions—all cluster-scoped resources—yet the SubjectPermission binding restricts access using namespacesAllowedRegex: "(^rhobs.*)". This creates an unresolvable RBAC conflict: either the binding becomes a RoleBinding (rendering cluster-scoped rules inaccessible) or a ClusterRoleBinding (making the namespace restriction meaningless).
Separate these cluster-scoped resources into a distinct ClusterRole bound through an explicit clusterPermissions field, following the pattern used elsewhere in the codebase (e.g., 50-dedicated-admins.SubjectPermission.yaml).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml` around lines
75 - 88, The ClusterRole in this file mixes cluster-scoped rules (resources:
clusterroles, clusterrolebindings, customresourcedefinitions) with a namespaced
SubjectPermission that uses namespacesAllowedRegex, which is invalid; split out
a new ClusterRole containing only the cluster-scoped resources (clusterroles,
clusterrolebindings, customresourcedefinitions) and remove those resources from
the existing ClusterRole, then bind the new ClusterRole via an explicit
clusterPermissions entry on the SubjectPermission (follow the binding pattern
used in 50-dedicated-admins.SubjectPermission.yaml) so namespace restrictions
remain on the remaining namespaced rules and cluster-scoped rules are granted at
cluster scope.
|
@moadz can you help check the pre-sumbit job failure? |
Introduces a new backplane role for the RHOBS team to get read-only access to RHOBS regional clusters. This includes a ClusterRole scoped to Thanos, Loki, monitoring, and core Kubernetes resources, a SubjectPermission binding it to rhobs-prefixed namespaces, and a SelectorSyncSet config targeting clusters labeled as RHOBS regional.
moadz
force-pushed
the
rhobs-rbac-backplane
branch
from
1512070 to
25c30a0
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml (2)
18-49:⚠️ Potential issue | 🟠 MajorFinish separating Prometheus Operator resources from RHOBS resources.
This still mixes likely
monitoring.coreos.comresources into themonitoring.rhobsrule, while themonitoring.coreos.comrule omits common Prometheus Operator resources such aspodmonitors,alertmanagerconfigs,probes,prometheusagents,scrapeconfigs, andthanosrulers. RBAC requires the exact API group/resource pair, so these entries can silently fail to grant the intended read access.Keep RHOBS-owned resources under
monitoring.rhobs, and put Prometheus Operator resources undermonitoring.coreos.com; verifythanosqueriersbefore leaving it in either group.#!/bin/bash # Description: Verify API group ownership for the monitoring resources referenced by this ClusterRole. # Expectation: Resources served by monitoring.coreos.com should be granted under monitoring.coreos.com; # RHOBS-specific resources should remain under monitoring.rhobs. rg -nP --type=yaml -C3 \ 'apiVersion:\s*(monitoring\.coreos\.com|monitoring\.rhobs|monitoring\.thanos\.io)/|kind:\s*(AlertmanagerConfig|Alertmanager|MonitoringStack|PodMonitor|Probe|PrometheusAgent|Prometheus|PrometheusRule|ScrapeConfig|ServiceMonitor|ThanosQuerier|ThanosRuler|UIPlugin)\b'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml` around lines 18 - 49, The ClusterRole currently mixes apiGroups; split the resource lists so that resources owned by RHOBS remain under apiGroup "monitoring.rhobs" and Prometheus Operator resources are under "monitoring.coreos.com": in the ClusterRole resource block, ensure "monitoring.rhobs" contains RHOBS-specific entries (e.g., alertmanagerconfigs, monitoringstacks, prometheusagents, uiplugins and any RHOBS-only scrapeconfigs/probes if applicable) and that "monitoring.coreos.com" contains Prometheus Operator entries (e.g., alertmanagers, prometheuses, prometheusrules, servicemonitors, podmonitors, probes, scrapeconfigs, thanosrulers as appropriate); verify ownership of thanosqueriers and place it under the correct apiGroup, then keep verbs (get/list/watch) the same for each apiGroup block.
75-88:⚠️ Potential issue | 🟠 MajorSplit cluster-scoped resources out of the namespace-scoped role.
clusterroles,clusterrolebindings, andcustomresourcedefinitionsare cluster-scoped, butdeploy/backplane/rhobs/20-rhobs.SubjectPermission.ymlbinds this role through namespace-scopedpermissionswithnamespacesAllowedRegex. Those cluster-scoped rules will not be granted by namespace RoleBindings.Move these resources to a separate ClusterRole bound via a cluster-scoped SubjectPermission mechanism, or remove them if RHOBS only needs namespace-scoped access.
#!/bin/bash # Description: Confirm this ClusterRole is only referenced by namespace-scoped SubjectPermission permissions, # and inspect existing cluster-scoped SubjectPermission patterns in the repo. cat deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml rg -nP --type=yaml -C8 \ 'clusterPermissions:|namespacesAllowedRegex:|clusterRoleName:\s*backplane-rhobs-regional-cluster-project|kind:\s*SubjectPermission'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml` around lines 75 - 88, The ClusterRole currently lists cluster-scoped rules (resources: clusterroles, clusterrolebindings, customresourcedefinitions) which cannot be granted via namespace RoleBindings; extract those rules into a separate ClusterRole (e.g., create a new ClusterRole containing only resources: clusterroles, clusterrolebindings, customresourcedefinitions) and remove them from the namespace-scoped role (the existing role name referenced as backplane-rhobs-regional-cluster-project), then bind that new ClusterRole with a cluster-scoped SubjectPermission (or remove the cluster-scoped rules if RHOBS only needs namespace access); also update or verify the SubjectPermission that currently references the namespace-scoped binding (the file that defines the permissions bound via namespacesAllowedRegex) to point to the new ClusterRole or to no longer reference cluster-scoped rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml`:
- Around line 18-49: The ClusterRole currently mixes apiGroups; split the
resource lists so that resources owned by RHOBS remain under apiGroup
"monitoring.rhobs" and Prometheus Operator resources are under
"monitoring.coreos.com": in the ClusterRole resource block, ensure
"monitoring.rhobs" contains RHOBS-specific entries (e.g., alertmanagerconfigs,
monitoringstacks, prometheusagents, uiplugins and any RHOBS-only
scrapeconfigs/probes if applicable) and that "monitoring.coreos.com" contains
Prometheus Operator entries (e.g., alertmanagers, prometheuses, prometheusrules,
servicemonitors, podmonitors, probes, scrapeconfigs, thanosrulers as
appropriate); verify ownership of thanosqueriers and place it under the correct
apiGroup, then keep verbs (get/list/watch) the same for each apiGroup block.
- Around line 75-88: The ClusterRole currently lists cluster-scoped rules
(resources: clusterroles, clusterrolebindings, customresourcedefinitions) which
cannot be granted via namespace RoleBindings; extract those rules into a
separate ClusterRole (e.g., create a new ClusterRole containing only resources:
clusterroles, clusterrolebindings, customresourcedefinitions) and remove them
from the namespace-scoped role (the existing role name referenced as
backplane-rhobs-regional-cluster-project), then bind that new ClusterRole with a
cluster-scoped SubjectPermission (or remove the cluster-scoped rules if RHOBS
only needs namespace access); also update or verify the SubjectPermission that
currently references the namespace-scoped binding (the file that defines the
permissions bound via namespacesAllowedRegex) to point to the new ClusterRole or
to no longer reference cluster-scoped rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 05e38e1a-a2b5-440f-9335-cc4d96750def
📒 Files selected for processing (5)
deploy/backplane/README.mddeploy/backplane/rhobs/00-rhobs.namespace.ymldeploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.ymldeploy/backplane/rhobs/20-rhobs.SubjectPermission.ymldeploy/backplane/rhobs/config.yaml
✅ Files skipped from review due to trivial changes (2)
- deploy/backplane/rhobs/config.yaml
- deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- deploy/backplane/README.md
- deploy/backplane/rhobs/00-rhobs.namespace.yml
Succeeds locally - i've rebased to try rerunning it. |
|
@moadz: The following test 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. |
3 participants
What type of PR is this?
(bug/feature/cleanup/documentation)
Adding new backplane role.
What this PR does / why we need it?
Introduces a new backplane role for the RHOBS team to get read-only
access to RHOBS regional clusters. This includes a ClusterRole scoped
to Thanos, Loki, monitoring, and core Kubernetes resources, a
SubjectPermission binding it to rhobs-prefixed namespaces, and a
SelectorSyncSet config targeting clusters labeled as RHOBS regional.
Summary by CodeRabbit
New Features
Documentation
Chores