fix(webhook): pre-populate Keycloak client-credentials annotation at admission

[huang195]

Summary

First-deploy race between the AuthBridge mutating webhook and the ClientRegistration controller: on a fresh deploy, the webhook runs before the controller has created the per-workload Keycloak credentials Secret, builds the pod with an empty /shared/, and envoy's jwt-validation plugin returns 503 "identity not yet configured (credentials pending)" indefinitely until the user manually deletes the pod.

This PR fixes the race by letting the webhook pre-populate the kagenti.io/keycloak-client-credentials-secret-name annotation on the incoming pod at admission time. The Secret name is a pure function of (namespace, workload) — the webhook can compute it without consulting the controller or the API server. ApplyKeycloakClientCredentialsSecretVolumes then declares a Secret volume with Optional=false, and kubelet holds the pod in ContainerCreating with a FailedMount event until the controller creates the Secret — a standard lazy-resolve pattern.

Observed repro in the field: operator v0.2.0-rc.3 on Kind + Sandbox-based weather agent. Pod log showed the mutator decision "deliveryPaths":"skip","keycloakClientCredentialsSecretName":"" at pod CREATE; ~5 seconds later, "operator client registration applied" with secret=kagenti-keycloak-client-credentials-<hash>. Secret existed, pod did not mount it. kubectl delete pod triggered recreate, mutator saw the annotation (now in the template), mounted correctly. Post-fix: single pod lifecycle, no restart.

Why this is better than today

Scenario	Today	After this PR
Fresh deploy	Pod Running with empty /shared/ → envoy 503s → user deletes pod to "fix" it	Pod ContainerCreating a few seconds → mount resolves → Running; envoy works on first start
Keycloak down / admin creds missing	Pod Running and silently wrong	Pod ContainerCreating with visible FailedMount event — observable, actionable
Legacy sidecar mode (kagenti.io/client-registration-inject=true)	Sidecar populates /shared/	No Secret mount added (workload is ineligible); sidecar still populates /shared/
Tool + injectTools=false	Skipped	Skipped (identical)

Code layout

• internal/clientreg/ (new) — one source of truth for the two load-bearing helpers the controller and the webhook both need:

  • KeycloakClientCredentialsSecretName(namespace, workload) string — deterministic name.
  • SkipReason(labels, injectTools) string / WorkloadWantsOperatorClientReg(labels, injectTools) bool — shared eligibility.
  • Constants (AnnotationKeycloakClientSecretName, label names/values) previously duplicated across packages.

• internal/controller/clientregistration_controller.go — the three functions collapse to one-line wrappers over clientreg. The controller keeps its local constants as aliases for its existing callers. No behavior change.

• internal/webhook/v1alpha1/authbridge_webhook.go — 6 lines of real logic inserted right after deriveWorkloadName, before isAlreadyInjected:

  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)
  }

SPIRE / non-SPIRE

Works for both. KeycloakClientCredentialsSecretName is a pure function of (namespace, workload) — SPIRE only changes what resolveKeycloakClientID (inside the controller) writes as the Secret's content, not the Secret's name. The webhook never needs to know about SPIRE.

Agreement between webhook and controller

Verified that deriveWorkloadName(pod) (webhook) equals the controller's workloadName for all three supported kinds:

Kind	Pod input	deriveWorkloadName	Controller's workloadName
Deployment	GenerateName=<dep>-<hash>- + label pod-template-hash=<hash>	<dep> (strips hash)	dep.Name ✓
StatefulSet	GenerateName=<sts>-	<sts>	sts.Name ✓
Sandbox	Name=<sbx>	<sbx>	sbx.GetName() ✓

Test plan

• go vet ./internal/clientreg/ ./internal/controller/ ./internal/webhook/v1alpha1/ — clean
• golangci-lint run — zero new issues on touched files (pre-existing issues in unrelated files untouched)
• go test ./internal/clientreg/ ./internal/controller/ ./internal/webhook/v1alpha1/ — all pass
• Webhook envtest: 17/17 specs (13 pre-existing + 4 new)
• Manual cluster repro: operator v0.2.0-rc.3 on Kind, Sandbox-based weather agent, confirmed pod went from "Running with empty /shared/, 503s forever" to "briefly ContainerCreating then Running with creds mounted"
• CI green

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>

[pdettori]

Excellent PR — well-motivated race fix with clean architecture (shared clientreg package), strong commit message, and good test coverage. The centralization of naming/eligibility logic eliminates controller-webhook drift by construction. The webhook pre-population + kubelet lazy-mount is a sound pattern.

Two minor test coverage suggestions below (non-blocking).

Areas reviewed: Go, Tests, Cross-file consistency, Security
Commits: 1 commit, signed-off: yes
CI status: All passing except E2E (pending)

[pdettori]

suggestion: Missing test case for the positive tool path — tool workload with injectTools=true. The suite covers gate-off (skip) and legacy-opt-in (skip), but not the eligible-tool scenario. Would strengthen confidence that the webhook correctly pre-populates the annotation for tools when the gate is enabled.

[pdettori]

nit: The test checks that a Secret volume exists with the matching name, but does not assert Optional is false (or nil). Since the PR description highlights Optional=false as the key behavioral mechanism that gives lazy-resolve semantics, asserting it here would document and protect that invariant.