From 2072fc732acc3e72dccc9de60c36f80abd7f1d54 Mon Sep 17 00:00:00 2001 From: Paolo Dettori Date: Mon, 15 Jun 2026 22:58:37 -0400 Subject: [PATCH] fix: preserve explicitly-set authbridge-config keys per namespace AuthbridgeConfigReconciler unconditionally overwrote every key of an existing authbridge-config ConfigMap with platform-global defaults on each reconcile, silently discarding any explicitly-set per-namespace value such as KEYCLOAK_REALM. A workload in a namespace using a non-default realm (e.g. tx-e2e) had its realm reset to "kagenti" within one reconcile cycle, so the client-registration controller registered its client in the wrong realm and client_credentials grants returned invalid_client (401). Regression introduced in cdbed3b. authbridge-config is a namespace-defaults source (kagenti.io/defaults= true), so defaults should fill gaps, not override explicit values. On the update path, only fill keys that are missing or empty; preserve any existing non-empty value. The create path (fresh kagenti-enabled namespace gets full operator-provided defaults) and label stamping are unchanged. Adds authbridgeconfig_controller_test.go (the controller previously had no test) covering: per-namespace realm preservation, full-defaults creation, missing/empty-key fill, label stamping on externally-created ConfigMaps, and the non-kagenti-enabled skip. Fixes #433 Assisted-By: Claude (Anthropic AI) Signed-off-by: Paolo Dettori --- .../controller/authbridgeconfig_controller.go | 7 + .../authbridgeconfig_controller_test.go | 187 ++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 kagenti-operator/internal/controller/authbridgeconfig_controller_test.go diff --git a/kagenti-operator/internal/controller/authbridgeconfig_controller.go b/kagenti-operator/internal/controller/authbridgeconfig_controller.go index effcbdee..9a7c6f4b 100644 --- a/kagenti-operator/internal/controller/authbridgeconfig_controller.go +++ b/kagenti-operator/internal/controller/authbridgeconfig_controller.go @@ -111,7 +111,14 @@ func (r *AuthbridgeConfigReconciler) Reconcile(ctx context.Context, req ctrl.Req if cm.Data == nil { cm.Data = make(map[string]string) } + // Fill in defaults only for keys that are missing or empty; preserve any + // explicitly-set per-namespace value (e.g. KEYCLOAK_REALM). This ConfigMap + // is a namespace-defaults source (kagenti.io/defaults=true), so defaults + // must not clobber a value provided by an operator/admin/CI. See issue #433. for k, v := range desired { + if existing, ok := cm.Data[k]; ok && existing != "" { + continue + } cm.Data[k] = v } if updateErr := r.Update(ctx, cm); updateErr != nil { diff --git a/kagenti-operator/internal/controller/authbridgeconfig_controller_test.go b/kagenti-operator/internal/controller/authbridgeconfig_controller_test.go new file mode 100644 index 00000000..4fe8161d --- /dev/null +++ b/kagenti-operator/internal/controller/authbridgeconfig_controller_test.go @@ -0,0 +1,187 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +*/ + +package controller + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const authbridgeConfigTestNamespace = "tx-e2e" + +// authbridgeTestPlatform returns a platform config whose default realm is the +// platform-global "kagenti" — i.e. the value that would clobber a per-namespace +// override if the reconciler overwrote existing keys. +func authbridgeTestPlatform() AuthbridgeConfigPlatform { + return AuthbridgeConfigPlatform{ + KeycloakAdminSecretNamespace: "keycloak", + KeycloakRealm: "kagenti", + KeycloakPublicURL: "https://keycloak.example", + } +} + +func kagentiEnabledNamespace(name string) *corev1.Namespace { + return &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{labelKagentiEnabled: "true"}, + }, + } +} + +func TestAuthbridgeConfigReconciler_Reconcile(t *testing.T) { + ctx := context.Background() + req := ctrl.Request{NamespacedName: types.NamespacedName{Name: authbridgeConfigTestNamespace}} + + getCM := func(t *testing.T, c client.Client) *corev1.ConfigMap { + t.Helper() + cm := &corev1.ConfigMap{} + if err := c.Get(ctx, client.ObjectKey{Namespace: authbridgeConfigTestNamespace, Name: authbridgeConfigCM}, cm); err != nil { + t.Fatalf("get authbridge-config: %v", err) + } + return cm + } + + cases := []struct { + name string + objs []client.Object + check func(t *testing.T, c client.Client) + }{ + { + // Regression for #433: an explicitly-set per-namespace realm must + // survive reconciliation rather than being reset to the platform default. + name: "preserves explicitly-set KEYCLOAK_REALM on existing ConfigMap", + objs: []client.Object{ + kagentiEnabledNamespace(authbridgeConfigTestNamespace), + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authbridgeConfigCM, + Namespace: authbridgeConfigTestNamespace, + }, + Data: map[string]string{ + "KEYCLOAK_REALM": "tx-e2e", + "KEYCLOAK_URL": "https://keycloak.example", + "CLIENT_AUTH_TYPE": "client-secret", + }, + }, + }, + check: func(t *testing.T, c client.Client) { + cm := getCM(t, c) + if got := cm.Data["KEYCLOAK_REALM"]; got != "tx-e2e" { + t.Fatalf("KEYCLOAK_REALM = %q, want %q (explicit per-namespace value must be preserved)", got, "tx-e2e") + } + }, + }, + { + name: "creates ConfigMap with platform defaults when none exists", + objs: []client.Object{ + kagentiEnabledNamespace(authbridgeConfigTestNamespace), + }, + check: func(t *testing.T, c client.Client) { + cm := getCM(t, c) + if got := cm.Data["KEYCLOAK_REALM"]; got != "kagenti" { + t.Fatalf("KEYCLOAK_REALM = %q, want platform default %q", got, "kagenti") + } + if got := cm.Data["KEYCLOAK_URL"]; got == "" { + t.Fatalf("KEYCLOAK_URL should be populated from platform defaults, got empty") + } + if cm.Labels["app.kubernetes.io/managed-by"] != "kagenti-operator" { + t.Fatalf("missing managed-by label, got %v", cm.Labels) + } + if cm.Labels[LabelNamespaceDefaults] != "true" { + t.Fatalf("missing %s label, got %v", LabelNamespaceDefaults, cm.Labels) + } + }, + }, + { + name: "fills missing/empty keys without overwriting present ones", + objs: []client.Object{ + kagentiEnabledNamespace(authbridgeConfigTestNamespace), + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authbridgeConfigCM, + Namespace: authbridgeConfigTestNamespace, + }, + Data: map[string]string{ + "KEYCLOAK_REALM": "tx-e2e", // explicit, non-empty -> keep + "CLIENT_AUTH_TYPE": "", // empty -> should be filled + }, + }, + }, + check: func(t *testing.T, c client.Client) { + cm := getCM(t, c) + if got := cm.Data["KEYCLOAK_REALM"]; got != "tx-e2e" { + t.Fatalf("KEYCLOAK_REALM = %q, want %q (preserved)", got, "tx-e2e") + } + if got := cm.Data["KEYCLOAK_URL"]; got == "" { + t.Fatalf("KEYCLOAK_URL (missing key) should be filled with default, got empty") + } + if got := cm.Data["CLIENT_AUTH_TYPE"]; got != "client-secret" { + t.Fatalf("CLIENT_AUTH_TYPE = %q, want empty value filled with default %q", got, "client-secret") + } + }, + }, + { + name: "stamps ownership labels on an externally-created ConfigMap", + objs: []client.Object{ + kagentiEnabledNamespace(authbridgeConfigTestNamespace), + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authbridgeConfigCM, + Namespace: authbridgeConfigTestNamespace, + }, + Data: map[string]string{"KEYCLOAK_REALM": "tx-e2e"}, + }, + }, + check: func(t *testing.T, c client.Client) { + cm := getCM(t, c) + if cm.Labels["app.kubernetes.io/managed-by"] != "kagenti-operator" { + t.Fatalf("missing managed-by label, got %v", cm.Labels) + } + if cm.Labels[LabelNamespaceDefaults] != "true" { + t.Fatalf("missing %s label, got %v", LabelNamespaceDefaults, cm.Labels) + } + }, + }, + { + name: "skips namespace without kagenti-enabled label", + objs: []client.Object{ + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: authbridgeConfigTestNamespace}}, + }, + check: func(t *testing.T, c client.Client) { + cm := &corev1.ConfigMap{} + err := c.Get(ctx, client.ObjectKey{Namespace: authbridgeConfigTestNamespace, Name: authbridgeConfigCM}, cm) + if err == nil { + t.Fatalf("expected no authbridge-config to be created for non-kagenti-enabled namespace") + } + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + scheme := clientRegistrationTestScheme(t) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.objs...).Build() + r := &AuthbridgeConfigReconciler{ + Client: c, + Platform: authbridgeTestPlatform(), + } + if _, err := r.Reconcile(ctx, req); err != nil { + t.Fatalf("Reconcile: %v", err) + } + tc.check(t, c) + }) + } +}