diff --git a/charts/kagenti-operator/templates/manager/manager.yaml b/charts/kagenti-operator/templates/manager/manager.yaml index 787b1b27..afe0ac7f 100644 --- a/charts/kagenti-operator/templates/manager/manager.yaml +++ b/charts/kagenti-operator/templates/manager/manager.yaml @@ -65,13 +65,19 @@ spec: - {{ .Values.controllerManager.container.cmd }} image: {{ .Values.controllerManager.container.image.repository }}:{{ .Values.controllerManager.container.image.tag }} imagePullPolicy: {{ .Values.controllerManager.container.image.pullPolicy | default "IfNotPresent" }} - {{- if .Values.controllerManager.container.env }} env: + # POD_NAMESPACE is used by the client registration controller to locate + # keycloak-admin-secret in the operator's namespace + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + {{- if .Values.controllerManager.container.env }} {{- range $key, $value := .Values.controllerManager.container.env }} - name: {{ $key }} value: {{ $value | quote }} {{- end }} - {{- end }} + {{- end }} livenessProbe: {{- toYaml .Values.controllerManager.container.livenessProbe | nindent 12 }} readinessProbe: diff --git a/kagenti-operator/cmd/main.go b/kagenti-operator/cmd/main.go index d17cdf75..ebf4feb0 100644 --- a/kagenti-operator/cmd/main.go +++ b/kagenti-operator/cmd/main.go @@ -68,6 +68,17 @@ func init() { // +kubebuilder:scaffold:scheme } +// getOperatorNamespace returns the namespace the operator is running in. +// Reads from POD_NAMESPACE environment variable (set via downward API in deployment), +// falling back to kagenti-system if not set. +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" +} + // nolint:gocyclo func main() { var metricsAddr string @@ -86,7 +97,6 @@ func main() { var requireA2ASignature bool var signatureAuditMode bool var enforceNetworkPolicies bool - var enableOperatorClientRegistration bool var enableMLflow bool var spireTrustDomain string @@ -114,7 +124,7 @@ func main() { flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") flag.BoolVar(&enableClientRegistration, "enable-client-registration", true, - "If set, AuthBridge webhook will register tool clients in Keycloak") + "Enable operator-managed Keycloak client registration for agent/tool workloads") flag.StringVar(&configPath, "config-path", "/etc/kagenti/config.yaml", "Path to platform config file") flag.StringVar(&featureGatesPath, "feature-gates-path", "/etc/kagenti/feature-gates/feature-gates.yaml", "Path to feature gates config file") @@ -124,9 +134,6 @@ func main() { "When true, log signature verification failures but don't block (use for rollout)") flag.BoolVar(&enforceNetworkPolicies, "enforce-network-policies", false, "Create NetworkPolicies to restrict traffic for agents with unverified signatures") - flag.BoolVar(&enableOperatorClientRegistration, "enable-operator-client-registration", false, - "Reconcile Keycloak client registration for agent/tool workloads unless "+ - "kagenti.io/client-registration-inject=true (legacy sidecar)") flag.BoolVar(&enableMLflow, "enable-mlflow", false, "Enable MLflow experiment tracking integration") @@ -380,11 +387,15 @@ func main() { setupLog.Info("MLflow experiment tracking controller enabled") } - if enableOperatorClientRegistration { + if enableClientRegistration { + operatorNS := getOperatorNamespace() + setupLog.Info("Client registration controller will read keycloak-admin-secret from operator namespace", + "namespace", operatorNS) if err = (&controller.ClientRegistrationReconciler{ Client: mgr.GetClient(), APIReader: mgr.GetAPIReader(), Scheme: mgr.GetScheme(), + OperatorNamespace: operatorNS, SpireTrustDomain: spireTrustDomain, KeycloakAdminTokenCache: &keycloak.CachedAdminTokenProvider{}, }).SetupWithManager(mgr); err != nil { @@ -414,10 +425,12 @@ func main() { // AuthBridge sidecar injection webhook if authBridgeWebhooksEnabled() { + // Pass false to disable legacy client-registration sidecar injection. + // Client registration is now handled by the operator controller. podMutator := injector.NewPodMutator( mgr.GetClient(), mgr.GetAPIReader(), - enableClientRegistration, + false, // disableLegacyClientRegistrationSidecar configLoader.Get, featureGateLoader.Get, ) diff --git a/kagenti-operator/internal/controller/clientregistration_controller.go b/kagenti-operator/internal/controller/clientregistration_controller.go index 66aa1818..3c274f4b 100644 --- a/kagenti-operator/internal/controller/clientregistration_controller.go +++ b/kagenti-operator/internal/controller/clientregistration_controller.go @@ -35,7 +35,7 @@ import ( "github.com/kagenti/operator/internal/keycloak" ) -// Well-known namespace resources (same contract as kagenti-webhook injector). +// Well-known namespace resources. const ( authbridgeConfigConfigMap = "authbridge-config" keycloakAdminSecret = "keycloak-admin-secret" @@ -55,11 +55,16 @@ const ( // never reference a missing Secret; the webhook mounts the Secret for injected sidecars that use shared-data. type ClientRegistrationReconciler struct { client.Client - // APIReader reads authbridge-config and keycloak-admin-secret from the API server. Those objects - // are not in the manager's ConfigMap cache (see cmd/main.go cache.ByObject for ConfigMap). + // APIReader reads authbridge-config from agent namespaces and keycloak-admin-secret from + // the operator's namespace from the API server. Those objects are not in the manager's + // ConfigMap cache (see cmd/main.go cache.ByObject for ConfigMap). APIReader client.Reader Scheme *runtime.Scheme + // OperatorNamespace is the namespace where the operator is running and where keycloak-admin-secret + // is located. This is dynamically detected from the operator's service account. + OperatorNamespace string + SpireTrustDomain string // KeycloakAdminTokenCache caches admin password-grant tokens by Keycloak URL and credentials to // avoid a token request on every reconcile. If nil, PasswordGrantToken is used without caching. @@ -163,10 +168,13 @@ func (r *ClientRegistrationReconciler) reconcileOne( return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } + // Read keycloak-admin-secret from the operator's namespace, not from agent namespace. + // This prevents Keycloak admin credentials from being replicated to every agent namespace, + // which would be a security risk if an agent namespace is compromised. adminSecret := &corev1.Secret{} - if err := r.uncachedReader().Get(ctx, types.NamespacedName{Namespace: ns, Name: keycloakAdminSecret}, adminSecret); err != nil { + if err := r.uncachedReader().Get(ctx, types.NamespacedName{Namespace: r.OperatorNamespace, Name: keycloakAdminSecret}, adminSecret); err != nil { if apierrors.IsNotFound(err) { - logger.Info("waiting for keycloak-admin-secret", "namespace", ns) + logger.Info("waiting for keycloak-admin-secret", "namespace", r.OperatorNamespace) return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } return ctrl.Result{}, err diff --git a/kagenti-operator/internal/controller/clientregistration_controller_test.go b/kagenti-operator/internal/controller/clientregistration_controller_test.go index 9acb183f..eb8fbce5 100644 --- a/kagenti-operator/internal/controller/clientregistration_controller_test.go +++ b/kagenti-operator/internal/controller/clientregistration_controller_test.go @@ -24,6 +24,7 @@ import ( const ( clientRegistrationTestNamespace = "test-ns" + clientRegistrationTestOperatorNS = "operator-ns" clientRegistrationTestDeploymentName = "my-dep" ) @@ -196,6 +197,10 @@ func authbridgeConfigMapForTest(ns, keycloakURL string) *corev1.ConfigMap { } } +// keycloakAdminSecretForTest creates a test keycloak-admin-secret. +// The secret should be created in the operator namespace (clientRegistrationTestOperatorNS), +// NOT in agent namespaces. This matches the production behavior where admin credentials +// are restricted to the operator namespace for security. func keycloakAdminSecretForTest(ns string) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -308,11 +313,13 @@ func TestClientRegistrationReconciler_Reconcile(t *testing.T) { wantRequeue: requeue, }, { - name: "missing keycloak admin secret waits with requeue", + name: "missing keycloak admin secret in operator namespace waits with requeue", objs: []client.Object{ clusterFeatureGatesConfigMap(true), testDeploymentForClientReg(), authbridgeConfigMapForTest(clientRegistrationTestNamespace, "https://keycloak.example"), + // Note: keycloak-admin-secret intentionally NOT created in operator namespace + // to test the missing secret path }, wantRequeue: requeue, }, @@ -322,7 +329,11 @@ func TestClientRegistrationReconciler_Reconcile(t *testing.T) { t.Run(tc.name, func(t *testing.T) { scheme := clientRegistrationTestScheme(t) c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.objs...).Build() - r := &ClientRegistrationReconciler{Client: c, Scheme: scheme} + r := &ClientRegistrationReconciler{ + Client: c, + Scheme: scheme, + OperatorNamespace: clientRegistrationTestOperatorNS, + } res, err := r.Reconcile(ctx, req) if err != nil { t.Fatalf("Reconcile: %v", err) @@ -350,9 +361,14 @@ func TestClientRegistrationReconciler_Reconcile(t *testing.T) { clusterFeatureGatesConfigMap(true), dep, authbridgeConfigMapForTest(clientRegistrationTestNamespace, srv.URL), - keycloakAdminSecretForTest(clientRegistrationTestNamespace), + // keycloak-admin-secret created in OPERATOR namespace (not agent namespace) + keycloakAdminSecretForTest(clientRegistrationTestOperatorNS), ).Build() - r := &ClientRegistrationReconciler{Client: c, Scheme: scheme} + r := &ClientRegistrationReconciler{ + Client: c, + Scheme: scheme, + OperatorNamespace: clientRegistrationTestOperatorNS, + } res, err := r.Reconcile(ctx, req) if err != nil || res != (ctrl.Result{}) { t.Fatalf("got (%v, %v), want (zero Result, nil)", res, err)