Restrict secrets RBAC to namespace-scoped Role

[ChughShilpa]

What this PR does / why we need it:
Restrict secrets RBAC to namespace-scoped Role

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Fixes #

Checklist:

• Docs included if any changes are user facing

Summary by CodeRabbit

• Chores
  • Refined permissions structure to restrict secrets access to a dedicated, namespace-scoped role for improved security isolation.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors RBAC configuration for the training operator to reduce privilege scope. The cluster-wide secrets permission is removed from the main ClusterRole and replaced with a new namespace-scoped Role (training-operator-webhook) that grants identical permissions on secrets. A RoleBinding attaches the new Role to the training-operator ServiceAccount. The kustomization.yaml is updated to include both new manifests, and a comment in cert.go documents the change.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Issues

CWE-276 (Incorrect Default Permissions): The new webhook-secret-role grants update and watch verbs on secrets alongside get and list. For a webhook component, verify whether update and watch are actually necessary—webhooks typically only need read access. Reduce permissions to the bare minimum required.

Missing scope verification: Confirm that the namespace-scoped Role is explicitly scoped to the operator's namespace only. The manifest provided shows no namespace field—if this is cluster-wide or inherits incorrect scope, the privilege reduction is negated. Explicitly set metadata.namespace in webhook-secret-role.yaml and verify the RoleBinding references the correct namespace.

Incomplete migration validation: Removing secrets access from the ClusterRole while adding it only to a namespaced Role is a breaking change. Confirm via testing that all webhook operations function correctly with namespace-scoped access only, and that no other components in the cluster depend on the removed ClusterRole permission.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: restricting secrets RBAC from cluster-wide permissions to a namespace-scoped Role.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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 `@manifests/base/rbac/webhook-secret-role-binding.yaml`:
- Line 15: This file is missing a trailing newline at EOF; add a single newline
character at the end of manifests/base/rbac/webhook-secret-role-binding.yaml,
stage the change and commit it so the End-of-File-Fixer modification from
pre-commit is included in the patch and CI stops failing.

In `@manifests/base/rbac/webhook-secret-role.yaml`:
- Line 18: The file manifests/base/rbac/webhook-secret-role.yaml is missing a
trailing newline at EOF; open that file (webhook-secret-role.yaml), add a single
newline character at the end of the file, save, then git add and include that
change in your patch (commit or amend the current commit) so pre-commit’s
End-of-File-Fixer no longer modifies it and CI will pass.
- Around line 11-17: The Role currently grants get/list/update/watch on all
"secrets" in the namespace; restrict it to only the webhook certificate Secret
by adding a resourceNames entry with the exact Secret name used by the rotator
(see the SecretKey / cert rotator in pkg/cert/cert.go), and remove unnecessary
verbs (drop list/watch unless the rotator in pkg/cert/cert.go explicitly needs
them). In short: replace the broad resources: - secrets rule with a secrets rule
that includes resourceNames: - "<webhook-cert-secret-name>" and only the minimal
verbs (likely get and update) required by the cert rotator.

In `@pkg/cert/cert.go`:
- Around line 41-42: The getOperatorNamespace function currently falls back to
the hardcoded "kubeflow" when MY_POD_NAMESPACE is unset—remove that unsafe
fallback and enforce the env var is present by returning/propagating an error
(or logging and exiting) instead; update getOperatorNamespace to check
os.LookupEnv("MY_POD_NAMESPACE") and if missing return an explicit error (or
call log.Fatalf) so callers like the webhook cert rotation cannot proceed
silently with a wrong namespace.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: aa2da576-6b1d-45eb-908a-4f738fb866f9

📥 Commits

Reviewing files that changed from the base of the PR and between b06cbe2 and c291b56.

📒 Files selected for processing (5)

• manifests/base/kustomization.yaml
• manifests/base/rbac/role.yaml
• manifests/base/rbac/webhook-secret-role-binding.yaml
• manifests/base/rbac/webhook-secret-role.yaml
• pkg/cert/cert.go

💤 Files with no reviewable changes (1)

• manifests/base/rbac/role.yaml

[robert-bell]

lgtm