Skip to content

[SREP-3267] - fix rbac to apply to backplane users.#500

Open
Mhodesty wants to merge 2 commits intoopenshift:masterfrom
Mhodesty:SREP-3267-fix-rbac
Open

[SREP-3267] - fix rbac to apply to backplane users.#500
Mhodesty wants to merge 2 commits intoopenshift:masterfrom
Mhodesty:SREP-3267-fix-rbac

Conversation

@Mhodesty
Copy link
Copy Markdown
Contributor

With PKO replacing OLM, these CRD-generated ClusterRoles are no longer created, causing backplane users to lose view access to clusterurlmonitors and routemonitors without elevation.

Changes

  • Added deploy_pko/ClusterRole-route-monitor-operator-dedicated-admins-cluster.yaml with:
    • rbac.authorization.k8s.io/aggregate-to-view: "true" - restores backplane access via view aggregation
    • managed.openshift.io/aggregate-to-dedicated-admins: "cluster" - restores dedicated-admin access

Test plan

  • Manually applied ClusterRole to test cluster
  • Verified backplane user can oc get clusterurlmonitors without elevation
  • Verified backplane user can oc get routemonitors without elevation

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c5c55f2-957f-4c8d-b139-e950759910ba

📥 Commits

Reviewing files that changed from the base of the PR and between 70eaae1 and d93a7f4.

📒 Files selected for processing (1)
  • hack/olm-registry/olm-artifacts-template.yaml
✅ Files skipped from review due to trivial changes (1)
  • hack/olm-registry/olm-artifacts-template.yaml

Walkthrough

Added RBAC aggregation label to a ClusterRole resource enabling view role group aggregation. Changed SelectorSyncSet resourceApplyMode from Sync to Upsert for resource reconciliation behavior in OLM registry template configuration.

Changes

Cohort / File(s) Summary
RBAC Configuration
deploy_pko/ClusterRole-route-monitor-operator-dedicated-admins-cluster.yaml
Added label rbac.authorization.k8s.io/aggregate-to-view: "true" to ClusterRole metadata to enable RBAC aggregation to the view role group.
OLM Registry Template
hack/olm-registry/olm-artifacts-template.yaml
Changed SelectorSyncSet spec field resourceApplyMode from Sync to Upsert, altering resource reconciliation behavior from strict sync to upsert semantics.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot requested review from Tessg22 and anispate March 27, 2026 17:55
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 27, 2026
@clcollins
Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2026
@Mhodesty
Copy link
Copy Markdown
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.26%. Comparing base (a7cd7f3) to head (d93a7f4).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #500   +/-   ##
=======================================
  Coverage   56.26%   56.26%           
=======================================
  Files          31       31           
  Lines        2851     2851           
=======================================
  Hits         1604     1604           
  Misses       1137     1137           
  Partials      110      110           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2026
@Mhodesty
Copy link
Copy Markdown
Contributor Author

/remove-hold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2026
@clcollins
Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 27, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clcollins, Mhodesty

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 27, 2026

New changes are detected. LGTM label has been removed.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/pko/clusterpackage-direct.yaml`:
- Around line 12-13: The IMAGE_DIGEST parameter is declared required but never
used; either remove the IMAGE_DIGEST parameter or update the manifest to
reference it in the pod image field: modify the spec.image (where PKO_IMAGE is
used) to include the digest (e.g., use the pattern ${PKO_IMAGE}@${IMAGE_DIGEST})
so IMAGE_DIGEST is consumed, or delete the IMAGE_DIGEST parameter declaration if
the digest is not needed; update any related documentation or parameter lists
accordingly.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e8f234a8-ac33-4792-887e-d52ef3ffbf9b

📥 Commits

Reviewing files that changed from the base of the PR and between 600d7c4 and 70eaae1.

📒 Files selected for processing (2)
  • hack/olm-registry/olm-artifacts-template.yaml
  • hack/pko/clusterpackage-direct.yaml

Comment on lines +12 to +13
- name: IMAGE_DIGEST
required: true
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

IMAGE_DIGEST is required but unused.

The IMAGE_DIGEST parameter is marked as required but is not referenced anywhere in the template objects. Either remove this parameter or update the spec.image to use the digest (e.g., ${PKO_IMAGE}@${IMAGE_DIGEST} if that's the intended format).

Proposed fix (if parameter is unnecessary)
-  - name: IMAGE_DIGEST
-    required: true
📝 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.

Suggested change
- name: IMAGE_DIGEST
required: true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hack/pko/clusterpackage-direct.yaml` around lines 12 - 13, The IMAGE_DIGEST
parameter is declared required but never used; either remove the IMAGE_DIGEST
parameter or update the manifest to reference it in the pod image field: modify
the spec.image (where PKO_IMAGE is used) to include the digest (e.g., use the
pattern ${PKO_IMAGE}@${IMAGE_DIGEST}) so IMAGE_DIGEST is consumed, or delete the
IMAGE_DIGEST parameter declaration if the digest is not needed; update any
related documentation or parameter lists accordingly.

@Mhodesty Mhodesty force-pushed the SREP-3267-fix-rbac branch from 70eaae1 to 4c11dfc Compare March 27, 2026 19:11
@Mhodesty Mhodesty force-pushed the SREP-3267-fix-rbac branch from 4c11dfc to d93a7f4 Compare March 27, 2026 19:42
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 27, 2026

@Mhodesty: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@Mhodesty
Copy link
Copy Markdown
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants