From 15c96022ebb895cb9fa78a9748608bc4516f449c Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 12 May 2026 17:12:00 -0400 Subject: [PATCH 1/3] fix(webhook): pre-populate Keycloak client-credentials annotation at admission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a fresh deploy of a Sandbox-based agent (and the same pattern for fresh Deployment/StatefulSet workloads), the AuthBridge mutating webhook ran before the ClientRegistration controller had produced the per-workload Keycloak client credentials Secret. The webhook saw no annotation on the pod, decided `deliveryPaths: skip`, and built the pod with an emptyDir at /shared/. Moments later the controller created the Secret and patched the owning workload's pod template with the annotation — but that patch does nothing to the already-running pod. The envoy-proxy jwt-validation plugin polled /shared/client-id.txt, timed out, and returned `503 "identity not yet configured (credentials pending)"` on every inbound request until the user manually deleted the pod so the owning controller would recreate it from the updated template. Fix: at admission time, when the workload is eligible for operator-managed client registration (labels say agent, or tool with the injectTools gate on, and not opted into the legacy client-registration sidecar), compute the deterministic Secret name and set the annotation on the pod before sidecar injection runs. The existing ApplyKeycloakClientCredentialsSecretVolumes call then adds a Secret volume with Optional=false. If the Secret does not exist yet, kubelet holds the pod in ContainerCreating with a FailedMount event and retries until the controller creates it — a standard lazy-resolve pattern and strictly better than today's silent 503: the problem is now observable via `kubectl describe pod` instead of hidden behind an "Agent error: 503" in the UI. Code layout: - Extract the shared helpers (KeycloakClientCredentialsSecretName, WorkloadWantsOperatorClientReg, SkipReason) and the constants they share with the controller into a new internal/clientreg package. The controller and webhook now call the same functions, so they cannot drift. The controller keeps thin aliases so local callers are unchanged. - The webhook imports clientreg and calls the two helpers directly. Six new lines of real logic in authbridge_webhook.go, plus an early log line for observability. Tests: - New unit tests in internal/clientreg for the name function and SkipReason coverage (nil labels, agent, tool with/without gate, legacy opt-in). - New envtest cases in authbridge_webhook_test.go covering: eligible agent gets annotation + Secret volume; existing annotation is not overwritten; tool workload with the gate off is not pre-populated; legacy opt-in is not pre-populated. Verified locally: go vet clean on touched packages; golangci-lint clean on touched files; all three affected package test suites pass (17/17 specs in the webhook suite, up from 13). Works for both SPIRE and non-SPIRE configurations because the Secret *name* is a pure function of (namespace, workload) — SPIRE only changes the Secret *contents* (SPIFFE ID vs ns/workload as clientID), not the name the webhook needs to declare at admission time. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- kagenti-operator/internal/clientreg/names.go | 82 +++++++++++++++++ .../internal/clientreg/names_test.go | 69 ++++++++++++++ .../clientregistration_controller.go | 44 +++------ .../webhook/v1alpha1/authbridge_webhook.go | 21 +++++ .../v1alpha1/authbridge_webhook_test.go | 91 +++++++++++++++++++ 5 files changed, 276 insertions(+), 31 deletions(-) create mode 100644 kagenti-operator/internal/clientreg/names.go create mode 100644 kagenti-operator/internal/clientreg/names_test.go diff --git a/kagenti-operator/internal/clientreg/names.go b/kagenti-operator/internal/clientreg/names.go new file mode 100644 index 00000000..13c00165 --- /dev/null +++ b/kagenti-operator/internal/clientreg/names.go @@ -0,0 +1,82 @@ +/* +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 clientreg holds the naming and eligibility logic shared by the AuthBridge mutating +// webhook and the ClientRegistration controller. Both sides must agree on (a) the Secret name +// and (b) whether a workload is eligible for operator-managed Keycloak client registration, +// so the webhook can pre-populate the pod annotation at admission without waiting for the +// controller to run. Centralizing these prevents the two sides from drifting. +package clientreg + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" +) + +const ( + // LabelClientRegistrationInject: when "true", the workload opts into the legacy + // client-registration sidecar and operator-managed registration is skipped. + LabelClientRegistrationInject = "kagenti.io/client-registration-inject" + + // LabelAgentType distinguishes agents from tools; operator-managed registration runs for + // agents unconditionally and for tools only when the injectTools feature gate is on. + LabelAgentType = "kagenti.io/type" + LabelValueAgent = "agent" + // LabelValueTool matches agentv1alpha1.RuntimeTypeTool — kept here as a string to avoid + // importing the API package from this leaf utility. + LabelValueTool = "tool" + + // AnnotationKeycloakClientSecretName is set on workload pod templates (by the controller) + // and pre-populated on new pods (by the webhook) to signal the name of the Secret holding + // Keycloak client credentials. The webhook mounts that Secret into /shared/client-id.txt + // and /shared/client-secret.txt for any container that already mounts the shared-data volume. + AnnotationKeycloakClientSecretName = "kagenti.io/keycloak-client-credentials-secret-name" +) + +// KeycloakClientCredentialsSecretName returns the deterministic name of the Secret the +// ClientRegistration controller produces for (namespace, workload). It is a pure function of +// those inputs only — the webhook can compute it at admission time without consulting the +// API server, so a Secret volume can be declared before the controller has run. Kubelet will +// retry the mount until the Secret appears. +func KeycloakClientCredentialsSecretName(namespace, workload string) string { + sum := sha256.Sum256([]byte(namespace + "\000" + workload + "\000kagenti-keycloak-client-credentials")) + return "kagenti-keycloak-client-credentials-" + hex.EncodeToString(sum[:8]) +} + +// SkipReason returns a non-empty human-readable reason when operator-managed client registration +// should not run for the workload. Empty string means "proceed". Both the controller's +// reconcileOne and the webhook's admission handler use this to stay in lockstep. +func SkipReason(labels map[string]string, injectTools bool) string { + if labels == nil { + return "pod template has no labels" + } + if labels[LabelClientRegistrationInject] == "true" { + return fmt.Sprintf("%s is \"true\" (legacy webhook client-registration sidecar; operator-managed registration disabled for this workload)", LabelClientRegistrationInject) + } + switch labels[LabelAgentType] { + case LabelValueAgent: + return "" + case LabelValueTool: + if !injectTools { + return "kagenti.io/type is tool but cluster injectTools feature gate is disabled" + } + return "" + default: + t := labels[LabelAgentType] + if t == "" { + return "kagenti.io/type label is missing or not agent/tool" + } + return fmt.Sprintf("kagenti.io/type=%q is not agent or tool", t) + } +} + +// WorkloadWantsOperatorClientReg returns true when the workload's labels and the cluster +// injectTools gate both permit operator-managed client registration. +func WorkloadWantsOperatorClientReg(labels map[string]string, injectTools bool) bool { + return SkipReason(labels, injectTools) == "" +} diff --git a/kagenti-operator/internal/clientreg/names_test.go b/kagenti-operator/internal/clientreg/names_test.go new file mode 100644 index 00000000..acbb589d --- /dev/null +++ b/kagenti-operator/internal/clientreg/names_test.go @@ -0,0 +1,69 @@ +/* +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 clientreg + +import "testing" + +func TestKeycloakClientCredentialsSecretName_Deterministic(t *testing.T) { + a := KeycloakClientCredentialsSecretName("team1", "weather-agent") + b := KeycloakClientCredentialsSecretName("team1", "weather-agent") + if a != b { + t.Fatalf("expected deterministic output, got %q and %q", a, b) + } + if a == "" { + t.Fatalf("expected non-empty secret name") + } +} + +func TestKeycloakClientCredentialsSecretName_DistinctByInputs(t *testing.T) { + cases := []struct{ ns, w string }{ + {"team1", "a"}, + {"team1", "b"}, + {"team2", "a"}, + } + seen := map[string]string{} + for _, c := range cases { + got := KeycloakClientCredentialsSecretName(c.ns, c.w) + key := c.ns + "/" + c.w + if prev, ok := seen[got]; ok { + t.Fatalf("collision: %s and %s both produced %s", prev, key, got) + } + seen[got] = key + } +} + +func TestSkipReason(t *testing.T) { + tests := []struct { + name string + labels map[string]string + injectTools bool + wantSkip bool + }{ + {"nil labels", nil, true, true}, + {"empty labels", map[string]string{}, true, true}, + {"legacy sidecar opt-in", map[string]string{LabelClientRegistrationInject: "true", LabelAgentType: LabelValueAgent}, true, true}, + {"agent proceeds", map[string]string{LabelAgentType: LabelValueAgent}, false, false}, + {"tool with gate on proceeds", map[string]string{LabelAgentType: LabelValueTool}, true, false}, + {"tool with gate off skipped", map[string]string{LabelAgentType: LabelValueTool}, false, true}, + {"unknown type skipped", map[string]string{LabelAgentType: "other"}, true, true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + reason := SkipReason(tc.labels, tc.injectTools) + gotSkip := reason != "" + if gotSkip != tc.wantSkip { + t.Fatalf("SkipReason(%v, %v) = %q; wantSkip=%v", tc.labels, tc.injectTools, reason, tc.wantSkip) + } + wants := WorkloadWantsOperatorClientReg(tc.labels, tc.injectTools) + if wants == tc.wantSkip { + t.Fatalf("WorkloadWantsOperatorClientReg disagrees with SkipReason for %v", tc) + } + }) + } +} diff --git a/kagenti-operator/internal/controller/clientregistration_controller.go b/kagenti-operator/internal/controller/clientregistration_controller.go index d1cec7c5..832b8b3d 100644 --- a/kagenti-operator/internal/controller/clientregistration_controller.go +++ b/kagenti-operator/internal/controller/clientregistration_controller.go @@ -9,8 +9,6 @@ package controller import ( "context" - "crypto/sha256" - "encoding/hex" "fmt" "strings" "time" @@ -32,7 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/yaml" - agentv1alpha1 "github.com/kagenti/operator/api/v1alpha1" + "github.com/kagenti/operator/internal/clientreg" "github.com/kagenti/operator/internal/keycloak" ) @@ -44,10 +42,12 @@ const ( // LabelClientRegistrationInject: when not "true", the operator registers the OAuth client and sets // AnnotationKeycloakClientSecretName. Value "true" opts the workload into the legacy webhook // client-registration sidecar; the operator skips registration for that workload. - LabelClientRegistrationInject = "kagenti.io/client-registration-inject" + // Re-exported from internal/clientreg so existing callers keep working. + LabelClientRegistrationInject = clientreg.LabelClientRegistrationInject // AnnotationKeycloakClientSecretName must match kagenti-webhook injector.AnnotationKeycloakClientSecretName. - AnnotationKeycloakClientSecretName = "kagenti.io/keycloak-client-credentials-secret-name" + // Re-exported from internal/clientreg to give both the controller and the webhook one source of truth. + AnnotationKeycloakClientSecretName = clientreg.AnnotationKeycloakClientSecretName ) // ClientRegistrationReconciler registers OAuth clients in Keycloak and patches agent/tool workloads that @@ -303,34 +303,14 @@ func injectKeycloakClientCredentialsAnnotation(template *corev1.PodTemplateSpec, return true } -// keycloakClientCredentialsSkipReason returns a non-empty human-readable reason when this controller should -// not process the workload; empty string means reconcile should continue. +// keycloakClientCredentialsSkipReason and workloadWantsOperatorClientReg delegate to internal/clientreg +// so the controller's reconcile decision and the webhook's admission decision stay in lockstep. func keycloakClientCredentialsSkipReason(labels map[string]string, injectTools bool) string { - if labels == nil { - return "pod template has no labels" - } - if labels[LabelClientRegistrationInject] == "true" { - return fmt.Sprintf("%s is \"true\" (legacy webhook client-registration sidecar; operator-managed registration disabled for this workload)", LabelClientRegistrationInject) - } - switch labels[LabelAgentType] { - case LabelValueAgent: - return "" - case string(agentv1alpha1.RuntimeTypeTool): - if !injectTools { - return "kagenti.io/type is tool but cluster injectTools feature gate is disabled" - } - return "" - default: - t := labels[LabelAgentType] - if t == "" { - return "kagenti.io/type label is missing or not agent/tool" - } - return fmt.Sprintf("kagenti.io/type=%q is not agent or tool", t) - } + return clientreg.SkipReason(labels, injectTools) } func workloadWantsOperatorClientReg(labels map[string]string, injectTools bool) bool { - return keycloakClientCredentialsSkipReason(labels, injectTools) == "" + return clientreg.WorkloadWantsOperatorClientReg(labels, injectTools) } type authbridgeConfig struct { @@ -440,9 +420,11 @@ func resolveKeycloakClientID(namespace, workloadName, serviceAccount string, spi return fmt.Sprintf("spiffe://%s/ns/%s/sa/%s", trustDomain, namespace, sa), nil } +// keycloakClientCredentialsSecretName is a thin alias over the shared helper so call sites in this +// package stay unchanged. The shared implementation lives in internal/clientreg so the AuthBridge +// mutating webhook can compute the same name at admission time. func keycloakClientCredentialsSecretName(namespace, workload string) string { - sum := sha256.Sum256([]byte(namespace + "\000" + workload + "\000kagenti-keycloak-client-credentials")) - return "kagenti-keycloak-client-credentials-" + hex.EncodeToString(sum[:8]) + return clientreg.KeycloakClientCredentialsSecretName(namespace, workload) } func (r *ClientRegistrationReconciler) ensureClientCredentialsSecret(ctx context.Context, owner client.Object, secretName, clientID, clientSecret string) error { diff --git a/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook.go b/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook.go index 45d43a38..9cc75d44 100644 --- a/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook.go +++ b/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook.go @@ -22,6 +22,7 @@ import ( "net/http" "strings" + "github.com/kagenti/operator/internal/clientreg" "github.com/kagenti/operator/internal/webhook/injector" corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -87,6 +88,26 @@ func (w *AuthBridgeWebhook) Handle(ctx context.Context, req admission.Request) a // but GenerateName is set by the owning controller (e.g. "myapp-7d4f8b9c5-"). resourceName := deriveWorkloadName(&pod) + // Pre-populate the Keycloak client-credentials annotation for workloads eligible for + // operator-managed client registration. The ClientRegistration controller produces the + // Secret asynchronously (it calls out to Keycloak), so at first-deploy admission the + // Secret typically does not exist yet. By setting the annotation here and letting + // ApplyKeycloakClientCredentialsSecretVolumes declare the Secret volume with Optional=false, + // kubelet will wait for the Secret to appear and mount it — no pod restart required. + // Without this, the first pod comes up with an empty /shared/ and envoy returns 503 + // "identity not yet configured (credentials pending)" until the user deletes the pod. + if pod.Annotations[injector.AnnotationKeycloakClientSecretName] == "" && + clientreg.WorkloadWantsOperatorClientReg(pod.Labels, w.Mutator.GetFeatureGates().InjectTools) { + if pod.Annotations == nil { + pod.Annotations = map[string]string{} + } + pod.Annotations[injector.AnnotationKeycloakClientSecretName] = + clientreg.KeycloakClientCredentialsSecretName(req.Namespace, resourceName) + authbridgelog.Info("pre-populated Keycloak client credentials annotation", + "namespace", req.Namespace, "name", resourceName, + "secret", pod.Annotations[injector.AnnotationKeycloakClientSecretName]) + } + // Check if already injected (idempotency / reinvocation) if w.isAlreadyInjected(&pod.Spec) { // Reinvocation: sidecars exist but Keycloak Secret mounts may still be diff --git a/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook_test.go b/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook_test.go index 799f88ef..27bca659 100644 --- a/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook_test.go +++ b/kagenti-operator/internal/webhook/v1alpha1/authbridge_webhook_test.go @@ -239,6 +239,97 @@ var _ = Describe("AuthBridge Pod Webhook", func() { Expect(containerNames(pod.Spec.Containers)).To(ContainElement(injector.AuthBridgeContainerName)) }) }) + + // Pre-population of the Keycloak client-credentials annotation ensures that the first pod + // created for an agent workload mounts the operator-produced Secret without having to wait + // for the ClientRegistration controller to patch the workload's pod template and trigger a + // pod recreate. Kubelet resolves the Secret lazily (Optional=false), so the pod simply sits + // in ContainerCreating until the controller creates the Secret. + Context("operator-managed Keycloak credentials: annotation pre-population", func() { + It("sets the annotation for an agent workload missing it", func() { + createAgentRuntime(testNamespace, "prepop-agent") + + pod := newTestPod("prepop-agent", map[string]string{ + "kagenti.io/type": "agent", + "kagenti.io/inject": "enabled", + }) + + err := k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), pod) + Expect(err).NotTo(HaveOccurred()) + + Expect(pod.Annotations).To(HaveKey(injector.AnnotationKeycloakClientSecretName)) + Expect(pod.Annotations[injector.AnnotationKeycloakClientSecretName]). + To(HavePrefix("kagenti-keycloak-client-credentials-")) + + // The webhook should also have declared the Secret volume (lazy-resolved by kubelet). + found := false + for _, v := range pod.Spec.Volumes { + if v.Secret != nil && v.Secret.SecretName == pod.Annotations[injector.AnnotationKeycloakClientSecretName] { + found = true + break + } + } + Expect(found).To(BeTrue(), "expected a Secret volume referencing the pre-populated credentials secret") + }) + + It("does not overwrite an existing annotation", func() { + createAgentRuntime(testNamespace, "prepop-existing") + + const preset = "kagenti-keycloak-client-credentials-deadbeef" + pod := newTestPod("prepop-existing", map[string]string{ + "kagenti.io/type": "agent", + "kagenti.io/inject": "enabled", + }) + pod.Annotations = map[string]string{ + injector.AnnotationKeycloakClientSecretName: preset, + } + + err := k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), pod) + Expect(err).NotTo(HaveOccurred()) + + Expect(pod.Annotations[injector.AnnotationKeycloakClientSecretName]).To(Equal(preset)) + }) + + It("skips tool workloads when the injectTools gate is disabled", func() { + pod := newTestPod("prepop-tool", map[string]string{ + "kagenti.io/type": "tool", + "kagenti.io/inject": "enabled", + }) + + err := k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), pod) + Expect(err).NotTo(HaveOccurred()) + + // No injection happens at all when injectTools is off, so the annotation should not be set. + Expect(pod.Annotations).NotTo(HaveKey(injector.AnnotationKeycloakClientSecretName)) + }) + + It("skips workloads opted into the legacy client-registration sidecar", func() { + createAgentRuntime(testNamespace, "prepop-legacy") + + pod := newTestPod("prepop-legacy", map[string]string{ + "kagenti.io/type": "agent", + "kagenti.io/inject": "enabled", + "kagenti.io/client-registration-inject": "true", + }) + + err := k8sClient.Create(ctx, pod) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), pod) + Expect(err).NotTo(HaveOccurred()) + + Expect(pod.Annotations).NotTo(HaveKey(injector.AnnotationKeycloakClientSecretName)) + }) + }) }) var _ = Describe("deriveWorkloadName", func() { From 76b4b621eae6f1a4386313df33174bc8a155cf54 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 12 May 2026 17:44:15 -0400 Subject: [PATCH 2/3] test(e2e): pre-create Keycloak credentials Secret for authbridge + combined agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit makes the webhook eagerly declare a Secret volume for any pod eligible for operator-managed Keycloak client registration. In prod this succeeds because the ClientRegistration controller produces the Secret shortly after — kubelet lazy-resolves the mount and the pod transitions from ContainerCreating to Running. In the e2e environment there is no real Keycloak and no keycloak-admin-secret, so the controller's reconcile loop can never register a client; the Secret is never produced; the pod sits in ContainerCreating until the test's WaitForDeploymentReady times out. This affected two tests whose pods are eligible and have sidecars actually injected: authbridge-agent and combined-agent. (Other e2e agents either set kagenti.io/inject=disabled, which short-circuits injection before the Secret mount, or omit kagenti.io/type, which makes them ineligible for operator-managed registration.) Pre-create a Secret with the deterministic name the webhook will compute, in each affected namespace, with dummy client-id.txt / client-secret.txt values. These tests exercise sidecar injection shape, not the OAuth flow, so dummy credentials are appropriate — we just need the mount to succeed so the pod can reach Ready. The fixture helper uses clientreg.KeycloakClientCredentialsSecretName so the name stays in lockstep with the webhook and controller — there is no place for these three components to drift apart. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- kagenti-operator/test/e2e/e2e_test.go | 12 +++++++++++ kagenti-operator/test/e2e/fixtures.go | 29 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/kagenti-operator/test/e2e/e2e_test.go b/kagenti-operator/test/e2e/e2e_test.go index bad54c06..6e17bc7d 100644 --- a/kagenti-operator/test/e2e/e2e_test.go +++ b/kagenti-operator/test/e2e/e2e_test.go @@ -364,6 +364,12 @@ var _ = Describe("AuthBridge Injection E2E", Ordered, func() { _, err = utils.KubectlApplyStdin(authBridgeConfigMapFixture(), authBridgeTestNamespace) Expect(err).NotTo(HaveOccurred()) + By("pre-creating Keycloak client credentials Secret (no real Keycloak in e2e)") + _, err = utils.KubectlApplyStdin( + keycloakClientCredentialsSecretFixture(authBridgeTestNamespace, "authbridge-agent"), + authBridgeTestNamespace) + Expect(err).NotTo(HaveOccurred()) + By("creating AgentRuntime CR for authbridge-agent (with retry for webhook readiness)") Eventually(func() error { _, err := utils.KubectlApplyStdin(authBridgeAgentRuntimeFixture(), authBridgeTestNamespace) @@ -1510,6 +1516,12 @@ rules: _, err = utils.KubectlApplyStdin(combinedConfigMapFixture(), combinedTestNamespace) Expect(err).NotTo(HaveOccurred()) + By("pre-creating Keycloak client credentials Secret (no real Keycloak in e2e)") + _, err = utils.KubectlApplyStdin( + keycloakClientCredentialsSecretFixture(combinedTestNamespace, "combined-agent"), + combinedTestNamespace) + Expect(err).NotTo(HaveOccurred()) + By("ensuring kagenti-system namespace exists") cmd = exec.Command("kubectl", "create", "ns", "kagenti-system") _, _ = utils.Run(cmd) diff --git a/kagenti-operator/test/e2e/fixtures.go b/kagenti-operator/test/e2e/fixtures.go index 74cba999..5cf0ce6b 100644 --- a/kagenti-operator/test/e2e/fixtures.go +++ b/kagenti-operator/test/e2e/fixtures.go @@ -16,11 +16,40 @@ limitations under the License. package e2e +import ( + "encoding/base64" + "fmt" + + "github.com/kagenti/operator/internal/clientreg" +) + const testNamespace = "e2e-agentcard-test" const authBridgeTestNamespace = "e2e-authbridge-test" const authBridgeAgentName = "authbridge-agent" const authBridgeAgentCMName = "authbridge-config-" + authBridgeAgentName +// keycloakClientCredentialsSecretFixture returns YAML for a Secret that matches the deterministic +// name the AuthBridge mutating webhook will pre-populate on pods of the given workload. Without it, +// the webhook's eager Secret mount (introduced to eliminate the first-deploy credentials race) would +// keep the pod in ContainerCreating forever in the e2e environment, which has no real Keycloak + no +// admin-secret for the ClientRegistration controller to successfully register clients. The values +// are dummy — these tests exercise injection shape, not OAuth flow. +func keycloakClientCredentialsSecretFixture(namespace, workload string) string { + name := clientreg.KeycloakClientCredentialsSecretName(namespace, workload) + clientID := base64.StdEncoding.EncodeToString([]byte("e2e-" + workload)) + clientSecret := base64.StdEncoding.EncodeToString([]byte("e2e-dummy-secret")) + return fmt.Sprintf(`apiVersion: v1 +kind: Secret +metadata: + name: %s + namespace: %s +type: Opaque +data: + client-id.txt: %s + client-secret.txt: %s +`, name, namespace, clientID, clientSecret) +} + // echoAgentFixture returns YAML for echo-agent Deployment + Service (used by S1, S3). func echoAgentFixture() string { return `apiVersion: apps/v1 From e250a1073db3534563e67edd77c3a2187dcee4c1 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Tue, 12 May 2026 18:04:25 -0400 Subject: [PATCH 3/3] fix(webhook): mount Keycloak client credentials in proxy-sidecar mode The proxy-sidecar branch of InjectAuthBridge returned before reaching the ApplyKeycloakClientCredentialsSecretVolumes call on the envoy-sidecar path. Every pod mutated in proxy-sidecar mode therefore ended up with its Secret volume undeclared: authbridge-proxy polled /shared/client-id.txt forever and rejected every inbound request with: 503 {"error":"upstream.unreachable", "message":"identity not yet configured (credentials pending)", "plugin":"jwt-validation"} Envoy-sidecar mode has always worked because the envoy path reaches line 494 which invokes the helper. Proxy-sidecar mode was silently broken for operator-managed client registration. Invoke the same helper inside the proxy-sidecar branch, right before the final log + return. Both modes now declare the same Secret volume and the same /shared/client-id.txt, /shared/client-secret.txt subPath mounts for any container that already mounts shared-data so authbridge-proxy gets its credentials the moment the ClientRegistration controller produces the Secret. Verified end-to-end on a Kind cluster: switched the weather agent Sandbox to kagenti.io/authbridge-mode=proxy-sidecar, recreated the pod, observed /shared/client-id.txt + /shared/client-secret.txt populated from the kagenti-keycloak-client-credentials- Secret, and the prior 503s disappeared from the authbridge-proxy logs. Tests: TestInjectAuthBridge_ProxySidecarMode_MountsKeycloakCredentials covers the regression (Secret volume present, authbridge-proxy mounts both subpaths). Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../internal/webhook/injector/pod_mutator.go | 9 +++ .../webhook/injector/pod_mutator_test.go | 65 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator.go b/kagenti-operator/internal/webhook/injector/pod_mutator.go index 4cc83d2e..e631bdec 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator.go @@ -413,6 +413,15 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp } } + // Mount operator-managed Keycloak client credentials for any container that uses + // shared-data (authbridge-proxy reads /shared/client-id.txt and /shared/client-secret.txt + // for its jwt-validation + token-exchange plugins). Without this, proxy-sidecar mode + // polls the credential files forever and rejects every inbound request with + // 503 "identity not yet configured (credentials pending)". Envoy-sidecar mode + // already calls this helper further down; the proxy-sidecar branch returns early, + // so it needs its own invocation. + ApplyKeycloakClientCredentialsSecretVolumes(podSpec, annotations) + mutatorLog.Info("proxy-sidecar mode injection complete", "namespace", namespace, "crName", crName, "image", builder.cfg.Images.AuthBridgeLight, diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go index ed16937e..e817df76 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go @@ -746,6 +746,71 @@ func TestInjectAuthBridge_ProxySidecarMode_InjectsCorrectly(t *testing.T) { } } +func TestInjectAuthBridge_ProxySidecarMode_MountsKeycloakCredentials(t *testing.T) { + // Regression: the proxy-sidecar branch used to return before reaching + // ApplyKeycloakClientCredentialsSecretVolumes. That left authbridge-proxy polling + // /shared/client-id.txt forever and returning 503 "identity not yet configured". + m := newTestMutator() + ctx := context.Background() + + podSpec := &corev1.PodSpec{ + ServiceAccountName: "my-agent", + Containers: []corev1.Container{ + {Name: "agent", Image: "my-agent:latest"}, + }, + } + labels := map[string]string{ + KagentiTypeLabel: KagentiTypeAgent, + } + annotations := map[string]string{ + AnnotationAuthBridgeMode: ModeProxySidecar, + AnnotationKeycloakClientSecretName: "kagenti-keycloak-client-credentials-abc12345", + } + + mutated, err := m.InjectAuthBridge(ctx, podSpec, "team1", "my-agent", labels, annotations) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !mutated { + t.Fatal("proxy-sidecar mode should mutate the pod") + } + + // The Secret volume must be declared so kubelet can resolve it. + volFound := false + for _, v := range podSpec.Volumes { + if v.Secret != nil && v.Secret.SecretName == "kagenti-keycloak-client-credentials-abc12345" { + volFound = true + break + } + } + if !volFound { + t.Error("expected a Secret volume for operator-managed Keycloak client credentials; got none") + } + + // The authbridge-proxy container must mount client-id.txt and client-secret.txt + // via subPath so its plugins can read them. + var proxyMounts []corev1.VolumeMount + for _, c := range podSpec.Containers { + if c.Name == AuthBridgeProxyContainerName { + proxyMounts = c.VolumeMounts + break + } + } + haveIDMount, haveSecretMount := false, false + for _, m := range proxyMounts { + if m.MountPath == "/shared/client-id.txt" && m.SubPath == "client-id.txt" { + haveIDMount = true + } + if m.MountPath == "/shared/client-secret.txt" && m.SubPath == "client-secret.txt" { + haveSecretMount = true + } + } + if !haveIDMount || !haveSecretMount { + t.Errorf("authbridge-proxy missing Keycloak credential subPath mounts: id=%v secret=%v", + haveIDMount, haveSecretMount) + } +} + func TestInjectHTTPProxyEnv_DoesNotDuplicate(t *testing.T) { c := &corev1.Container{ Name: "agent",