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 into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds RHOBS support: README table update and new Changes
Sequence Diagram(s)sequenceDiagram
participant Repo as "Backplane Repo"
participant Hive as "Hive Controller"
participant Cluster as "Managed Cluster"
participant RBAC as "Cluster RBAC Controller"
participant SA as "ServiceAccounts"
Repo->>Hive: Apply SelectorSyncSet (backplane-rhobs)
Hive->>Cluster: Push synced manifests (Namespace, ClusterRole, SubjectPermission)
Cluster->>RBAC: Create Namespace `openshift-backplane-rhobs`
Cluster->>RBAC: Create ClusterRole `backplane-rhobs-regional-cluster-project`
Cluster->>RBAC: Create SubjectPermission binding to `system:serviceaccounts:openshift-backplane-rhobs`
RBAC->>SA: Grant permissions to serviceaccounts in allowed namespaces
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![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
| - apiGroups: | ||
| - monitoring.rhobs.io | ||
| resources: | ||
| - monitoringstacks | ||
| - thanosqueriers | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the group used throughout the repo for RHOBS monitoring resources.
rg -nP --type=yaml -C1 'monitoring\.rhobs(\.io)?'Repository: openshift/managed-cluster-config
Length of output: 4839
🏁 Script executed:
cat -n deploy/backplane/rhobs/rhobs-cluster/10-srep-rhobs-cluster-project.ClusterRole.ymlRepository: openshift/managed-cluster-config
Length of output: 2235
Fix API group and indentation in monitoring.rhobs rule block.
The API group should be monitoring.rhobs (not monitoring.rhobs.io). Throughout the repository, RHOBS resources use apiVersion: monitoring.rhobs/v*, and this incorrect API group will prevent the RBAC rule from granting access to monitoringstacks and thanosqueriers resources.
Additionally, the indentation for this rule block (4-space nested under apiGroups/resources/verbs) is inconsistent with all other rule blocks in the file, which use 2-space indentation.
🔧 Proposed fix
- apiGroups:
- - monitoring.rhobs.io
+ - monitoring.rhobs
resources:
- - monitoringstacks
- - thanosqueriers
+ - monitoringstacks
+ - thanosqueriers
verbs:
- - get
- - list
- - watch
+ - get
+ - list
+ - watch🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@deploy/backplane/rhobs/rhobs-cluster/10-srep-rhobs-cluster-project.ClusterRole.yml`
around lines 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.
There was a problem hiding this comment.
It got fixed in the generated templates, but can you also fix the indents here? As coderabbit suggested
There was a problem hiding this comment.
🧩 Analysis chain
Let me check the current state of the source file.
🏁 Script executed:
#!/bin/bash
cat -n deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml
Length of output: 2452
@bmeng, the API group is already fixed to monitoring.rhobs in the source file. Only the indentation (lines 19–37) still needs normalizing. Here's the fix:
- apiGroups:
- - monitoring.rhobs
+ - monitoring.rhobs
resources:
- - alertmanagerconfigs
- - alertmanagers
- - monitoringstacks
- - podmonitors
- - probes
- - prometheusagents
- - prometheuses
- - prometheusrules
- - scrapeconfigs
- - servicemonitors
- - thanosqueriers
- - thanosrulers
- - uiplugins
+ - alertmanagerconfigs
+ - alertmanagers
+ - monitoringstacks
+ - podmonitors
+ - probes
+ - prometheusagents
+ - prometheuses
+ - prometheusrules
+ - scrapeconfigs
+ - servicemonitors
+ - thanosqueriers
+ - thanosrulers
+ - uiplugins
verbs:
- - get
- - list
- - watch
+ - get
+ - list
+ - watchThis aligns the monitoring.rhobs rule block with the 2-space style used by all other rule blocks in the file.
| 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
| - 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? |
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
force-pushed
the
rhobs-rbac-backplane
branch
from
25c30a0 to
ea6736d
Compare
|
[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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/00-osd-managed-cluster-config-integration.yaml.tmpl`:
- Around line 30198-30199: Update the SubjectPermission's namespace regexes:
change namespacesAllowedRegex from (^rhobs.*) to a stricter ^rhobs-.* to only
match namespaces prefixed with "rhobs-"; then remove or replace the redundant
namespacesDeniedRegex (currently set to openshift-backplane-cluster-admin) since
it cannot match anything allowed by the tightened allowed regex—either drop that
denied entry or replace it with a meaningful exclusion if a specific sensitive
rhobs-* sub-namespace must be denied.
In `@hack/00-osd-managed-cluster-config-production.yaml.tmpl`:
- Around line 30197-30199: The namespacesAllowedRegex currently uses
'(^rhobs.*)' which also matches names like 'rhobsfoo' and includes unnecessary
parentheses; update the source template
deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml to use a stricter anchored
pattern such as '^rhobs-' (or '^rhobs-.*' if a trailing match is desired) for
the namespacesAllowedRegex field so only namespaces with the 'rhobs-' prefix are
permitted; verify the change near the SubjectPermission that references
clusterRoleName backplane-rhobs-regional-cluster-project and ensure the
generated hack/00-osd-managed-cluster-config-production.yaml.tmpl reflects the
updated regex after regeneration.
- Around line 30116-30127: The rule block under apiGroup "monitoring.coreos.com"
incorrectly includes the resource "thanosqueriers"; remove "thanosqueriers" from
the resources list in the ClusterRole entry so that the monitoring.coreos.com
block only contains valid resources (alertmanagers, prometheuses,
prometheusrules, servicemonitors), and instead ensure "thanosqueriers" remains
only under the monitoring.rhobs block; make this change in the source
ClusterRole template
(deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml) so it
propagates to hack/00-osd-managed-cluster-config-*.yaml.tmpl on re-render.
In `@hack/00-osd-managed-cluster-config-stage.yaml.tmpl`:
- Around line 30079-30201: The ClusterRole contains cluster-scoped rules that
won't be granted by the SubjectPermission's namespace-scoped permissions and the
namespacesAllowedRegex is too broad; update
deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml to either add a
clusterPermissions block that grants the ClusterRole's cluster-scoped read
rights (for rbac.authorization.k8s.io clusterroles/clusterrolebindings and
apiextensions.k8s.io/customresourcedefinitions) or remove those cluster-scoped
rules from deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml so
the ClusterRole only contains namespace-scoped rules, and change
namespacesAllowedRegex from (^rhobs.*) to (^rhobs-.*) to strictly match rhobs-
prefixed namespaces.
🪄 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: Enterprise
Run ID: dd3f1ea8-13b2-43e6-a9b2-c6b1435bbee5
📒 Files selected for processing (8)
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.yamlhack/00-osd-managed-cluster-config-integration.yaml.tmplhack/00-osd-managed-cluster-config-production.yaml.tmplhack/00-osd-managed-cluster-config-stage.yaml.tmpl
✅ Files skipped from review due to trivial changes (4)
- deploy/backplane/rhobs/config.yaml
- deploy/backplane/README.md
- deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml
- deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/backplane/rhobs/00-rhobs.namespace.yml
| namespacesAllowedRegex: (^rhobs.*) | ||
| namespacesDeniedRegex: openshift-backplane-cluster-admin |
There was a problem hiding this comment.
Tighten namespacesAllowedRegex and reconsider the redundant namespacesDeniedRegex.
Two minor concerns on the SubjectPermission:
namespacesAllowedRegex: (^rhobs.*)is only anchored at the start with no separator afterrhobs, so it would also match unintended names likerhobsomethingorrhobsystem. Per the PR description ("namespaces prefixed withrhobs-"), consider^rhobs-.*to bind the role only to the intended RHOBS namespaces.namespacesDeniedRegex: openshift-backplane-cluster-admincannot match anything already restricted by the allowed regex (no namespace can both start withrhobsand beopenshift-backplane-cluster-admin), so it's effectively a no-op. It looks copy-pasted from the ODF SubjectPermission directly above (line 30049). Either drop it or replace it with something meaningful (e.g., explicitly excluding any sensitive sub-namespace, if applicable).
Note: this template is generated; the root-cause edit should land in deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml and the template can then be regenerated.
📝 Proposed fix (apply at the source manifest)
permissions:
- clusterRoleName: backplane-rhobs-regional-cluster-project
- namespacesAllowedRegex: (^rhobs.*)
- namespacesDeniedRegex: openshift-backplane-cluster-admin
+ namespacesAllowedRegex: ^rhobs-.*📝 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.
| namespacesAllowedRegex: (^rhobs.*) | |
| namespacesDeniedRegex: openshift-backplane-cluster-admin | |
| namespacesAllowedRegex: ^rhobs-.* |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/00-osd-managed-cluster-config-integration.yaml.tmpl` around lines 30198
- 30199, Update the SubjectPermission's namespace regexes: change
namespacesAllowedRegex from (^rhobs.*) to a stricter ^rhobs-.* to only match
namespaces prefixed with "rhobs-"; then remove or replace the redundant
namespacesDeniedRegex (currently set to openshift-backplane-cluster-admin) since
it cannot match anything allowed by the tightened allowed regex—either drop that
denied entry or replace it with a meaningful exclusion if a specific sensitive
rhobs-* sub-namespace must be denied.
| - 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
# Confirm that thanosqueriers does NOT appear under monitoring.coreos.com in any source manifest,
# and confirm where the rule originates so the fix can be applied at the root cause.
fd -t f -e yml -e yaml . deploy/backplane/rhobs
echo "---- ClusterRole source ----"
cat deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml 2>/dev/null
echo "---- Search for thanosqueriers occurrences across templates and source ----"
rg -n --type=yaml -C2 'thanosqueriers'
echo "---- Verify parallel stage/integration templates contain the same dead rule ----"
rg -nP -C2 '\bthanosqueriers\b' hack/00-osd-managed-cluster-config-stage.yaml.tmpl hack/00-osd-managed-cluster-config-integration.yaml.tmpl 2>/dev/nullRepository: openshift/managed-cluster-config
Length of output: 40812
Remove thanosqueriers from the monitoring.coreos.com rule — it does not exist in that API group.
The resource thanosqueriers is not part of the monitoring.coreos.com API group (per Prometheus Operator). It is correctly listed under monitoring.rhobs at lines 30108–30111, but its presence in the monitoring.coreos.com block below is a copy-paste error and authorizes nothing.
This file is generated from deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml. The source must be corrected, and the same fix will propagate to hack/00-osd-managed-cluster-config-integration.yaml.tmpl and hack/00-osd-managed-cluster-config-stage.yaml.tmpl on re-render.
Suggested fix
- apiGroups:
- monitoring.coreos.com
resources:
- alertmanagers
- prometheuses
- prometheusrules
- - thanosqueriers
- servicemonitors
verbs:
- get
- list
- watchDo not add thanosrulers unless confirmed with RHOBS — it lives under monitoring.thanos.io, not monitoring.coreos.com.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/00-osd-managed-cluster-config-production.yaml.tmpl` around lines 30116 -
30127, The rule block under apiGroup "monitoring.coreos.com" incorrectly
includes the resource "thanosqueriers"; remove "thanosqueriers" from the
resources list in the ClusterRole entry so that the monitoring.coreos.com block
only contains valid resources (alertmanagers, prometheuses, prometheusrules,
servicemonitors), and instead ensure "thanosqueriers" remains only under the
monitoring.rhobs block; make this change in the source ClusterRole template
(deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml) so it
propagates to hack/00-osd-managed-cluster-config-*.yaml.tmpl on re-render.
| - clusterRoleName: backplane-rhobs-regional-cluster-project | ||
| namespacesAllowedRegex: (^rhobs.*) | ||
| namespacesDeniedRegex: openshift-backplane-cluster-admin |
There was a problem hiding this comment.
Tighten namespacesAllowedRegex to a strict rhobs- prefix.
(^rhobs.*) matches any namespace whose name simply starts with the literal rhobs — e.g., rhobs, rhobsfoo, rhobsy-test — not just the rhobs--prefixed namespaces described in the PR description. The outer parentheses are also unnecessary. Recommend anchoring on the hyphen so unintended namespaces created on a managed cluster cannot inherit these permissions.
This file is generated; apply the fix in deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml so it propagates to the integration/stage/production templates consistently.
🛠️ Proposed fix
- clusterRoleName: backplane-rhobs-regional-cluster-project
- namespacesAllowedRegex: (^rhobs.*)
+ namespacesAllowedRegex: ^rhobs-.*
namespacesDeniedRegex: openshift-backplane-cluster-admin📝 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.
| - clusterRoleName: backplane-rhobs-regional-cluster-project | |
| namespacesAllowedRegex: (^rhobs.*) | |
| namespacesDeniedRegex: openshift-backplane-cluster-admin | |
| - clusterRoleName: backplane-rhobs-regional-cluster-project | |
| namespacesAllowedRegex: ^rhobs-.* | |
| namespacesDeniedRegex: openshift-backplane-cluster-admin |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/00-osd-managed-cluster-config-production.yaml.tmpl` around lines 30197 -
30199, The namespacesAllowedRegex currently uses '(^rhobs.*)' which also matches
names like 'rhobsfoo' and includes unnecessary parentheses; update the source
template deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml to use a stricter
anchored pattern such as '^rhobs-' (or '^rhobs-.*' if a trailing match is
desired) for the namespacesAllowedRegex field so only namespaces with the
'rhobs-' prefix are permitted; verify the change near the SubjectPermission that
references clusterRoleName backplane-rhobs-regional-cluster-project and ensure
the generated hack/00-osd-managed-cluster-config-production.yaml.tmpl reflects
the updated regex after regeneration.
| - apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole | ||
| metadata: | ||
| name: backplane-rhobs-regional-cluster-project | ||
| rules: | ||
| - apiGroups: | ||
| - monitoring.thanos.io | ||
| resources: | ||
| - thanoscompacts | ||
| - thanosqueries | ||
| - thanosreceives | ||
| - thanosrulers | ||
| - thanosstores | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - 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 | ||
| - apiGroups: | ||
| - loki.grafana.com | ||
| resources: | ||
| - lokistacks | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - route.openshift.io | ||
| resources: | ||
| - routes | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - template.openshift.io | ||
| resources: | ||
| - templates | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - rbac.authorization.k8s.io | ||
| resources: | ||
| - clusterroles | ||
| - clusterrolebindings | ||
| - roles | ||
| - rolebindings | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - apiextensions.k8s.io | ||
| resources: | ||
| - customresourcedefinitions | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - '' | ||
| resources: | ||
| - configmaps | ||
| - services | ||
| - serviceaccounts | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiGroups: | ||
| - apps | ||
| resources: | ||
| - deployments | ||
| - statefulsets | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch | ||
| - apiVersion: managed.openshift.io/v1alpha1 | ||
| kind: SubjectPermission | ||
| metadata: | ||
| name: backplane-rhobs-regional-cluster | ||
| namespace: openshift-rbac-permissions | ||
| spec: | ||
| permissions: | ||
| - clusterRoleName: backplane-rhobs-regional-cluster-project | ||
| namespacesAllowedRegex: (^rhobs.*) | ||
| namespacesDeniedRegex: openshift-backplane-cluster-admin | ||
| subjectKind: Group | ||
| subjectName: system:serviceaccounts:openshift-backplane-rhobs |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm SubjectPermission semantics: does `permissions` create RoleBindings only?
# And does `clusterPermissions` exist on the CRD as the way to grant cluster-scoped access?
fd -t f 'subjectpermission' -i
rg -nP -C3 '\b(permissions|clusterPermissions)\b' --iglob '*.yaml' --iglob '*.yml' -g 'deploy/**'
# Sanity-check other SubjectPermission definitions in this repo for prior art on cluster-scoped grants.
rg -nP -C2 '^\s*clusterPermissions:' --iglob '*.yml' --iglob '*.yaml'
# Inspect any vendored/operator type definitions if present.
rg -nP -C3 'type\s+SubjectPermissionSpec\b'Repository: openshift/managed-cluster-config
Length of output: 50389
🌐 Web query:
openshift rbac-permissions-operator SubjectPermission permissions vs clusterPermissions semantics
💡 Result:
In the OpenShift RBAC Permissions Operator (rbac-permissions-operator), the SubjectPermission custom resource (CR) defines two distinct fields for granting permissions to a subject (specified by subjectKind and subjectName, e.g., a Group): - clusterPermissions: A list of cluster role names. These result in the creation of ClusterRoleBindings, granting the specified ClusterRoles to the subject cluster-wide (across all namespaces). This provides cluster-scoped or non-namespaced permissions. - permissions: A list of objects, each specifying a clusterRoleName along with optional namespacesAllowedRegex and namespacesDeniedRegex. These result in the creation of RoleBindings in matching namespaces (filtered by the regexes), granting the specified ClusterRole only within those specific namespaces. The key semantic difference is scope: clusterPermissions apply globally via ClusterRoleBindings, while permissions apply selectively to namespaces via RoleBindings. This allows fine-grained control, e.g., granting cluster-wide access for administrative tasks but restricting project-level access to user namespaces (excluding system ones like openshift-). Example from official operator docs: spec: subjectKind: Group subjectName: dedicated-admins clusterPermissions: - dedicated-admins-cluster # -> ClusterRoleBinding permissions: - clusterRoleName: dedicated-admins-project namespacesAllowedRegex: "." namespacesDeniedRegex: "(^kube-.|^openshift.|...)" # -> RoleBindings in allowed NS only The operator's SubjectPermission controller watches for changes to these CRs and reconciles the corresponding bindings accordingly.
Citations:
- 1: https://pkg.go.dev/github.com/openshift/rbac-permissions-operator
- 2: https://github.com/openshift/rbac-permissions-operator
🏁 Script executed:
#!/bin/bash
# Inspect the source SubjectPermission file for rhobs
cat -n deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml
echo "---"
echo "Searching for actual rhobs* namespace definitions in the repo:"
rg -nP 'name:\s+(rhobs|rhobs-)' --iglob '*.yml' --iglob '*.yaml' | head -20Repository: openshift/managed-cluster-config
Length of output: 654
🏁 Script executed:
#!/bin/bash
# Check other SubjectPermission patterns for how regex is used elsewhere
echo "=== Sample of namespacesAllowedRegex patterns from repo ==="
rg -nP 'namespacesAllowedRegex:' --iglob '*.yml' --iglob '*.yaml' -A 1 | head -40Repository: openshift/managed-cluster-config
Length of output: 4306
Cluster-scoped rules in ClusterRole won't be effective via this SubjectPermission, and the namespace regex is broader than the stated intent.
Two RBAC concerns in this generated template. Fixes must be applied to the source manifests under deploy/backplane/rhobs/ and the template regenerated.
-
Cluster-scoped rules have no effect here.
TheSubjectPermissiondeclares onlypermissions(lines 30196–30199), which the rbac-permissions-operator translates intoRoleBindings—namespace-scoped only. Rules in the ClusterRole targeting cluster-scoped resources (rbac.authorization.k8s.io/{clusterroles,clusterrolebindings}andapiextensions.k8s.io/customresourcedefinitions) cannot be granted through RoleBindings and will be silently ineffective. To grant cluster-scoped read access, add aclusterPermissionsblock to the SubjectPermission (indeploy/backplane/rhobs/20-rhobs.SubjectPermission.yml); otherwise, remove these cluster-scoped rules from the ClusterRole (deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml) to avoid misleading authors. -
namespacesAllowedRegex: (^rhobs.*)matches more than "rhobs-" prefixed namespaces.
This pattern matches "rhobs", "rhobsfoo", "rhobs-anything"—any string starting with "rhobs". If the intent is strictly namespaces with therhobs-prefix, use(^rhobs-.*). Change this indeploy/backplane/rhobs/20-rhobs.SubjectPermission.yml.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/00-osd-managed-cluster-config-stage.yaml.tmpl` around lines 30079 -
30201, The ClusterRole contains cluster-scoped rules that won't be granted by
the SubjectPermission's namespace-scoped permissions and the
namespacesAllowedRegex is too broad; update
deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml to either add a
clusterPermissions block that grants the ClusterRole's cluster-scoped read
rights (for rbac.authorization.k8s.io clusterroles/clusterrolebindings and
apiextensions.k8s.io/customresourcedefinitions) or remove those cluster-scoped
rules from deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml so
the ClusterRole only contains namespace-scoped rules, and change
namespacesAllowedRegex from (^rhobs.*) to (^rhobs-.*) to strictly match rhobs-
prefixed namespaces.
|
@bmeng just resurrecting this PR, CI is green now. |
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole | ||
| metadata: | ||
| name: backplane-rhobs-regional-cluster-project |
There was a problem hiding this comment.
The suffix project is a bit misleading, can we just change the role name to backplane-rhobs-regional-cluster or backplane-rhobs-regional-cluster-role?
| namespace: openshift-rbac-permissions | ||
| spec: | ||
| permissions: | ||
| - clusterRoleName: backplane-rhobs-regional-cluster-project |
| permissions: | ||
| - clusterRoleName: backplane-rhobs-regional-cluster-project | ||
| namespacesAllowedRegex: "(^rhobs.*)" | ||
| namespacesDeniedRegex: openshift-backplane-cluster-admin |
There was a problem hiding this comment.
This namespacesDeniedRegex is not needed actually, as the above allowedRegex only includes the rhobs* namespaces
There was a problem hiding this comment.
I agree :) but it's a backplane-rule:
cluster-role-suffix: deploy/backplane/rhobs/10-rhobs-cluster-project.ClusterRole.yml: ClusterRole 'backplane-rhobs-regional-cluster-role' has suffix different from its containing file
subject-permission-deny-cluster-admin: deploy/backplane/rhobs/20-rhobs.SubjectPermission.yml: SubjectPermission 'backplane-rhobs-regional-role' has permission(s) which do not explicitly deny clusterRole(s) 'backplane-rhobs-regional-cluster-project' access to 'openshift-backplane-cluster-admin'| - apiGroups: | ||
| - monitoring.rhobs.io | ||
| resources: | ||
| - monitoringstacks | ||
| - thanosqueriers | ||
| verbs: | ||
| - get | ||
| - list | ||
| - watch |
There was a problem hiding this comment.
It got fixed in the generated templates, but can you also fix the indents here? As coderabbit suggested
moadz
force-pushed
the
rhobs-rbac-backplane
branch
from
ea6736d to
8d44256
Compare
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
8d44256 to
3ea12a0
Compare
|
@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