Skip to content

fix(openshift): use controller-manager SA for metrics RBAC #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zdtsw merged 2 commits into from
Mar 29, 2026
Merged

fix(openshift): use controller-manager SA for metrics RBAC #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3551c43
fix(openshift): use controller-manager SA for metrics RBAC
Gregory-Pereira Mar 27, 2026
d2cf699
fix(kube): use controller-manager SA for metrics RBAC
Gregory-Pereira Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/openshift/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ resources:
- ../default
- cluster-monitoring-view-binding.yaml
- metrics-reader-token.yaml
- prometheus-metrics-auth-binding.yaml
- prometheus-metrics-reader-binding.yaml

patches:
- path: configmap-patch.yaml
Expand Down
14 changes: 0 additions & 14 deletions config/openshift/prometheus-metrics-auth-binding.yaml
View file
Open in desktop

This file was deleted.

15 changes: 15 additions & 0 deletions config/openshift/prometheus-metrics-reader-binding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Grant GET /metrics permission to the controller-manager SA whose token is
# used by Prometheus (via authorization.credentials) to scrape the WVA
# metrics endpoint. The WVA metrics endpoint performs a SubjectAccessReview
# checking this permission.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: workload-variant-autoscaler-ocp-prometheus-metrics-reader
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: metrics-reader
subjects:
- kind: ServiceAccount
name: controller-manager
Comment on lines +13 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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.yaml

Repository: 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.

Suggested change
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.

1 change: 1 addition & 0 deletions config/rbac/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ resources:
- metrics_auth_role_binding.yaml
- prometheus_metrics_auth_role_binding.yaml
- metrics_reader_role.yaml
- metrics_reader_role_binding.yaml
# EPP metrics reader service account with minimal privileges
- epp_metrics_service_account.yaml
- epp_metrics_reader_role.yaml
Expand Down
21 changes: 21 additions & 0 deletions config/rbac/metrics_reader_role_binding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Grant GET /metrics permission to the Prometheus SA so the
# SubjectAccessReview performed by the WVA metrics endpoint passes.
# This complements metrics_auth_role_binding.yaml (which grants
# tokenreview/SAR permissions to the controller-manager) and
# prometheus_metrics_auth_role_binding.yaml (which grants the
# Prometheus SA authentication permissions).
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: metrics-reader-rolebinding
labels:
app.kubernetes.io/name: workload-variant-autoscaler
app.kubernetes.io/managed-by: kustomize
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: metrics-reader
subjects:
- kind: ServiceAccount
name: kube-prometheus-stack-prometheus
namespace: workload-variant-autoscaler-monitoring
Loading