fix(openshift): use controller-manager SA for metrics RBAC#46
Conversation
Signed-off-by: greg pereira <grpereir@redhat.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThe OpenShift overlay's Prometheus RBAC configuration was changed: Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Security observations
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
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 `@config/openshift/prometheus-metrics-reader-binding.yaml`:
- Around line 13-15: The RoleBinding subject for the ServiceAccount named
"controller-manager" is missing the namespace, causing the binding to target the
default namespace; update the subjects block (the ServiceAccount subject) to
include namespace: workload-variant-autoscaler-system so the RoleBinding
correctly authorizes the controller-manager ServiceAccount in the overlay
namespace.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e72df1fd-031d-4fc6-afe7-487d09715ec5
📒 Files selected for processing (3)
config/openshift/kustomization.yamlconfig/openshift/prometheus-metrics-auth-binding.yamlconfig/openshift/prometheus-metrics-reader-binding.yaml
💤 Files with no reviewable changes (1)
- config/openshift/prometheus-metrics-auth-binding.yaml
| subjects: | ||
| - kind: ServiceAccount | ||
| name: controller-manager |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the binding subject explicitly sets namespace and matches overlay namespace.
set -euo pipefail
echo "Checking ServiceAccount subject block:"
awk 'BEGIN{p=0} /subjects:/{p=1} p{print} /namespace:/{if(p){exit}}' config/openshift/prometheus-metrics-reader-binding.yaml
echo
echo "Checking overlay namespace:"
rg -n '^\s*namespace:\s*' config/openshift/kustomization.yamlRepository: opendatahub-io/workload-variant-autoscaler
Length of output: 262
Add namespace to ServiceAccount subject — binding targets wrong namespace (CWE-863: Improper Authorization).
Lines 14–15 define a ServiceAccount subject without namespace. Kubernetes defaults this to the "default" namespace, but the overlay (kustomization.yaml line 32) places the controller-manager ServiceAccount in workload-variant-autoscaler-system. This mismatch prevents the binding from authorizing the intended ServiceAccount, breaking metrics access.
Fix
subjects:
- kind: ServiceAccount
name: controller-manager
+ namespace: workload-variant-autoscaler-system📝 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.
| subjects: | |
| - kind: ServiceAccount | |
| name: controller-manager | |
| subjects: | |
| - kind: ServiceAccount | |
| name: controller-manager | |
| namespace: workload-variant-autoscaler-system |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/openshift/prometheus-metrics-reader-binding.yaml` around lines 13 -
15, The RoleBinding subject for the ServiceAccount named "controller-manager" is
missing the namespace, causing the binding to target the default namespace;
update the subjects block (the ServiceAccount subject) to include namespace:
workload-variant-autoscaler-system so the RoleBinding correctly authorizes the
controller-manager ServiceAccount in the overlay namespace.
Signed-off-by: greg pereira <grpereir@redhat.com>
…est to 8b211cc (opendatahub-io#46) Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com> Co-authored-by: konflux-internal-p02[bot] <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
cc @vivekk16 @pierDipi @zdtsw
More confident about this one, I was able to test with what I think will become these permissions recreated by prefetched-manifests and it resolved my permissions issues
Summary by CodeRabbit