fix: read keycloak-admin-secret from kagenti-system namespace

[Alan-Cha]

Summary

Addresses security issue in #320 by changing the client registration controller to read Keycloak admin credentials from the kagenti-system namespace instead of from each agent namespace.

Security Improvement

Before:

• Operator read keycloak-admin-secret from agent namespaces (team1, team2, etc.)
• If an agent namespace is compromised → attacker gains full Keycloak realm admin access
• Can create/delete users, realms, clients, roles

After:

• Operator reads keycloak-admin-secret from kagenti-system namespace only
• Agent namespaces have no access to Keycloak admin credentials
• Compromised agent namespace = no admin access

Changes

• ✅ Add operatorNamespace constant set to "kagenti-system"
• ✅ Update secret read in clientregistration_controller.go to use operator namespace
• ✅ Update struct and inline comments to clarify secret location
• ✅ Add security rationale in comments

RBAC

No RBAC changes needed - the operator already has ClusterRole permissions for cluster-wide secret access (confirmed in issue description).

Related PRs

Deployment sequence is critical:

1. FIRST: Merge and deploy kagenti/kagenti#1422

   • Creates keycloak-admin-secret in kagenti-system
   • Removes secret from agent namespaces

2. THEN: Merge and deploy this PR

   • Operator switches to reading from kagenti-system

⚠️ Do not merge this PR until rossoctl/rossoctl#1422 is deployed, otherwise the operator will fail to find the secret.

Manual Cleanup

After both PRs are deployed, manually delete old secrets from agent namespaces:

kubectl delete secret keycloak-admin-secret -n team1
kubectl delete secret keycloak-admin-secret -n team2
# ... for each agent namespace

Testing

• Verify operator starts successfully with secret in kagenti-system
• Deploy test agent and verify OAuth client registration works
• Check operator logs for successful registration
• Verify no errors about missing secret

Related Issues

• Closes: Operator should read keycloak-admin-secret from kagenti-system, not agent namespaces #320
• Related: fix: move Keycloak admin secret to kagenti-system and remove client-registration sidecar references rossoctl#1422 (Helm chart changes)
• Long-term: Eliminate admin credentials from client registration using SPIFFE-based DCR rossoctl#1421 (SPIFFE DCR to eliminate admin credentials)

🤖 Generated with Claude Code

[akram]

Instead of hardcoding it we should use the current namespace.
The common practice is indeed to use kagenti-system often replacing the system placeholder.

But, in downstream products, sometimes it is chosen to keep an project agnostic namespace.

[Alan-Cha]

This makes sense

[Alan-Cha]

✅ Updated to address feedback - the operator now dynamically detects its namespace instead of hardcoding kagenti-system.

Changes:

• Added OperatorNamespace field to the reconciler struct
• Added getOperatorNamespace() helper that reads from /var/run/secrets/kubernetes.io/serviceaccount/namespace
• Falls back to kagenti-system if the file cannot be read (e.g., during local development)
• Logs the detected namespace at startup for visibility

This makes the code compatible with downstream deployments that use different namespace conventions.

[Alan-Cha]

📝 Further simplified based on feedback - now uses the standard POD_NAMESPACE environment variable pattern.

What changed:

• ✅ Replaced file reading logic with simple os.Getenv("POD_NAMESPACE")
• ✅ Added POD_NAMESPACE to the manager deployment using Kubernetes downward API
• ✅ Follows the same pattern already used elsewhere in the codebase (agentcard-signer)

Much cleaner implementation:

func getOperatorNamespace() string {
    if ns := os.Getenv("POD_NAMESPACE"); ns != "" {
        return ns
    }
    setupLog.Info("POD_NAMESPACE not set, using default", "default", "kagenti-system")
    return "kagenti-system"
}

The deployment now automatically injects the namespace via downward API. 🎉

[Alan-Cha]

✅ Simplified client registration flags based on feedback from kagenti#1422.

Changes

Removed confusing dual-flag setup:

• ❌ Removed --enable-operator-client-registration (was defaulted to false)
• ✅ --enable-client-registration now controls operator-based registration only

Before (confusing):

• --enable-client-registration=true → global toggle for feature
• --enable-operator-client-registration=false → choose operator vs sidecar

After (simplified):

• --enable-client-registration=true → operator-based registration enabled
• --enable-client-registration=false → no registration

Sidecar removal:

• Webhook now passes false to disable legacy client-registration sidecar injection
• Only operator-based registration is supported

This makes the configuration much clearer and removes the confusing interaction between two flags.

[esnible]

I realize you didn't modify kagenti-operator/internal/controller/clientregistration_controller_test.go. Claude complains that the unit test fails, and I see the same thing. We don't want to merge with failing unit tests.

cd kagenti-operator
make test

--- FAIL: TestClientRegistrationReconciler_Reconcile (0.04s)
    --- FAIL: TestClientRegistrationReconciler_Reconcile/happy_path_registers_client_patches_deployment_and_creates_secret (0.00s)
        clientregistration_controller_test.go:358: got ({false 30s <nil>}, <nil>), want (zero Result, nil)
Running Suite: Controller Suite - /Users/snible/src/kagenti-operator/kagenti-operator/internal/controller

[Alan-Cha]

✅ Fixed unit tests - addressing the test failure reported in review.

Changes

Updated clientregistration_controller_test.go to match the new behavior:

• Added clientRegistrationTestOperatorNS constant for operator namespace
• Set OperatorNamespace field when creating test reconcilers
• Create keycloak-admin-secret in operator namespace instead of agent namespace

Test Results

All tests now pass:

=== RUN   TestClientRegistrationReconciler_Reconcile
=== RUN   TestClientRegistrationReconciler_Reconcile/happy_path_registers_client_patches_deployment_and_creates_secret
--- PASS: TestClientRegistrationReconciler_Reconcile (0.06s)
    --- PASS: TestClientRegistrationReconciler_Reconcile/happy_path_registers_client_patches_deployment_and_creates_secret (0.01s)
PASS
ok  	github.com/kagenti/operator/internal/controller	1.056s

The fix correctly models the new behavior: operator reads admin secret from its own namespace, not from agent namespaces.

[Alan-Cha]

✅ Terminology consistency - changed 'operator-based' to 'operator-managed'

Updated to use consistent terminology with the rest of the codebase:

• Flag description
• Log message

The term 'operator-managed' is already used throughout the operator code (precedence.go, pod_mutator.go, clientregistration_controller.go, etc.)

[akram]

@Alan-Cha I have tested your branch and it works as described now. Keycloak admin creds are taken only from kagenti-system and they are not required nor installed anymore in other namespace.

While doing my tests; I found an issue fixed by #324 . Should we include it in this PR ?

edit: I did my tests with keycloak 26.6.0.

[pdettori]

Akram's review feedback was addressed: namespace is no longer hardcoded — the operator now dynamically detects its namespace via POD_NAMESPACE (downward API), with kagenti-system as a fallback default. This supports downstream products that use a different namespace.