Skip to content

Fix: preserve explicitly-set authbridge-config keys per namespace (#433)#434

Merged
pdettori merged 1 commit into
rossoctl:mainfrom
pdettori:fix/preserve-namespace-realm-433
Jun 16, 2026
Merged

Fix: preserve explicitly-set authbridge-config keys per namespace (#433)#434
pdettori merged 1 commit into
rossoctl:mainfrom
pdettori:fix/preserve-namespace-realm-433

Conversation

@pdettori

Copy link
Copy Markdown
Member

Summary

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 had its realm reset to kagenti within one reconcile cycle, so the client-registration controller registered the client in the wrong realm and client_credentials grants returned invalid_client (401). Regression introduced in cdbed3b.

Fixes #433.

Root cause

In the update path of authbridgeconfig_controller.go:

for k, v := range desired {
    cm.Data[k] = v   // clobbers KEYCLOAK_REALM (and every other key)
}

Fix

authbridge-config is a namespace-defaults source (labeled kagenti.io/defaults=true, consumed by defaults_config_reconciler.go / agentruntime_controller.go), and cdbed3b introduced it purely as operator-driven defaulting. Defaults should fill gaps, not override explicit values. On the update path the reconciler now only fills keys that are missing or empty, preserving any existing non-empty value:

for k, v := range desired {
    if existing, ok := cm.Data[k]; ok && existing != "" {
        continue // preserve explicitly-set per-namespace value
    }
    cm.Data[k] = v
}

Unchanged: the create path (a fresh kagenti-enabled namespace still receives full operator-provided defaults) and the ownership/defaults label stamping. So the cdbed3b defaulting behavior is preserved for namespaces that have no config yet.

Tests

The controller previously had no test. Adds authbridgeconfig_controller_test.go (plain testing + fake client, matching clientregistration_controller_test.go) covering:

Case Asserts
Per-namespace realm preserved existing KEYCLOAK_REALM=tx-e2e survives reconcile (the regression)
Create path no CM → created with full platform defaults + labels
Missing/empty-key fill present non-empty key kept; missing & empty keys filled from defaults
Label stamping externally-created CM gets managed-by + kagenti.io/defaults
Skip non-kagenti-enabled namespace → no CM created

The first two/three cases were written failing first (clobber reproduced), then made green by the fix.

ok  github.com/kagenti/operator/internal/controller  (5/5 subtests pass)

gofmt, go vet, and go build ./... clean.

Cross-refs

  • Interim mitigation in kagenti made the token-exchange E2E step non-fatal (kagenti/kagenti #1982); this PR restores the underlying behavior so that suite can go green again.

Assisted-By: Claude Code

@pdettori
pdettori requested a review from a team as a code owner June 16, 2026 02:59
@pdettori pdettori changed the title fix: preserve explicitly-set authbridge-config keys per namespace (#433) Fix: preserve explicitly-set authbridge-config keys per namespace (#433) Jun 16, 2026
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 rossoctl#433

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
@pdettori
pdettori force-pushed the fix/preserve-namespace-realm-433 branch from 5240ce3 to 2072fc7 Compare June 16, 2026 15:33

@esnible esnible left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Summary

Clean, focused fix. The update path now fills only missing/empty keys instead of clobbering every key, so an explicitly-set per-namespace value (e.g. KEYCLOAK_REALM=tx-e2e) survives reconciliation while the create path still gets full operator-provided defaults. This matches Option 1 in #433 and correctly preserves the cdbed3b defaulting behavior for fresh namespaces.

Verified against upstream/main: the diff applies to the pre-fix clobbering loop; LabelNamespaceDefaults == "kagenti.io/defaults", authbridgeConfigCM, and clientRegistrationTestScheme all exist and are used consistently; and authbridge-config is genuinely consumed as a namespace-defaults source (via MatchingLabels{LabelNamespaceDefaults: "true"}). Good first test for a controller that previously had none — the regression case is written to fail on the old behavior.

One behavior trade-off worth noting (non-blocking): once a key is non-empty, the operator can no longer push an updated platform default to it on the update path. That's the intended "defaults fill gaps, don't override" semantics per #433, so this is correct as designed.

Author: pdettori (MEMBER — maintainer)
Areas reviewed: Go (controller + test)
Agent/IDE config (.claude/.vscode): none
Commits: 1 commit, signed-off: yes
CI status: passing (all 16 checks green, incl. E2E + Integration + Unit)

@pdettori
pdettori merged commit 5bdc7a5 into rossoctl:main Jun 16, 2026
16 checks passed
@pdettori
pdettori deleted the fix/preserve-namespace-realm-433 branch June 16, 2026 22:08
varshaprasad96 added a commit to varshaprasad96/kagenti-operator that referenced this pull request Jun 18, 2026
Add missing workloadKind parameter to 10 InjectAuthBridge test calls
introduced by upstream PRs (rossoctl#436 egress enforcement, rossoctl#434 namespace
realm, rossoctl#432 iptables backend) that merged after this branch diverged.

Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AuthbridgeConfigReconciler overwrites per-namespace KEYCLOAK_REALM (regression in cdbed3b) — breaks non-default realms / token-exchange e2e

2 participants