feat(webhook): operator-managed Keycloak client credentials by akram · Pull Request #262 · kagenti/kagenti-extensions

akram · 2026-03-27T12:39:51Z

Summary

Coordinates with kagenti-operator for workloads that opt out of webhook-injected client registration (kagenti.io/client-registration-inject: "false").
requires:

oc patch deployment weather-service  --type merge \
   -p '{"spec": { "template": { "metadata": { "labels": {"kagenti.io/client-registration-inject": "false" }}}}}'

requires: kagenti/kagenti-operator#247

Changes

Mount Keycloak OAuth2 client id/secret from a namespace Secret named in pod annotation kagenti.io/keycloak-client-credentials-secret-name (volume name matches Secret: kagenti-keycloak-client-credentials-<id>).
Replace operator_clientreg with keycloak_client_credentials helpers: ApplyKeycloakClientCredentialsSecretVolumes, NeedsKeycloakClientCredentialsVolumePatch, AnnotationKeycloakClientSecretName.
Admission logging for Pod processing and credential delivery path (Keycloak Secret mount vs sidecar vs combined authbridge); logger keycloak-client-registration.
Reinvocation path patches missing mounts when annotation appears after first injection.

Testing

go test ./internal/webhook/injector/... (unit); deploy webhook + operator together for integration.

fixes kagenti/kagenti#397

cwiklik

Review Summary

Clean webhook-side implementation of the operator-managed Keycloak client credentials contract. The code correctly mounts operator-provisioned Secrets into shared-data containers, handles reinvocation for late annotations, and logs credential delivery paths. The annotation constant matches the operator PR (kagenti/kagenti-operator#247) exactly. Idempotency is well-handled throughout.

Must-fix: Made-with: Cursor trailer in 2 of 3 commits — should be Assisted-By: per kagenti conventions. Same issue flagged on the paired operator PR.

Areas reviewed: Go, Tests
Commits: 3 commits, all signed-off: yes
CI status: DCO passing
Cross-repo consistency: Annotation kagenti.io/keycloak-client-credentials-secret-name matches operator PR #247 ✓

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials.go

kagenti-webhook/internal/webhook/v1alpha1/authbridge_webhook.go

kagenti-webhook/internal/webhook/injector/pod_mutator.go

kagenti-webhook/internal/webhook/v1alpha1/authbridge_webhook.go

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials.go

…gistration When workloads opt out of webhook-injected client-registration, the kagenti-operator reconciler registers OAuth clients and annotates the pod template with the secret name. The mutating webhook now skips injecting the client-registration container in that case while still injecting AuthBridge sidecars, and wires the operator-provided secret for mounts. Signed-off-by: Akram <akram.benaissi@gmail.com>

- Replace operator_clientreg with keycloak_client_credentials (annotation, volume naming) - Add keycloak-client-registration logger; align deliveryPaths labels with Keycloak terminology Assisted-By: Cursor Signed-off-by: Akram <akram.benaissi@gmail.com>

Use AnnotationKeycloakClientSecretName, NeedsKeycloakClientCredentialsVolumePatch, ApplyKeycloakClientCredentialsSecretVolumes, and updated log field names. Assisted-By: Cursor Signed-off-by: Akram <akram.benaissi@gmail.com>

Signed-off-by: Akram <akram.benaissi@gmail.com>

Alan-Cha · 2026-03-27T15:54:11Z

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials.go

+// keycloakClientCredentialsVolumeName is the in-pod Volume name for the Secret that holds Keycloak
+// client credentials. It matches the Secret name (kagenti-keycloak-client-credentials-<uniq-id>) so the
+// volume is unique and stable for that Secret.
+func keycloakClientCredentialsVolumeName(secretName string) string {
+	return secretName
+}


What is the purpose of this function?

Alan-Cha

This PR successfully implements the webhook-side coordination for operator-managed Keycloak client registration. The code quality is excellent: idempotent, well-tested, secure (ReadOnly SubPath mounts), and provides good observability. The prior review issue (Made-with: Cursor → Assisted-By: Cursor) has been fixed ✅.

Remaining issues:

All 4 commits and PR title are missing emoji prefixes (Kagenti convention)
Final commit message is too generic

Prior review issues:
✅ Assisted-By: Cursor trailer fixed (was Made-with: Cursor)

Areas reviewed: Go, Tests, Security, Cross-repo consistency
Commits: 4 commits, all signed-off ✅, all missing emoji prefixes ❌
CI status: DCO passing ✅
Code quality: Excellent - idempotent, well-tested, secure mounts, good observability

Alan-Cha · 2026-03-27T17:44:27Z

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials.go

+// keys client-id.txt and client-secret.txt. The webhook mounts them for every container that uses
+// the shared-data volume (same paths as the client-registration sidecar: /shared/client-id.txt,
+// /shared/client-secret.txt).
+const AnnotationKeycloakClientSecretName = "kagenti.io/keycloak-client-credentials-secret-name"


✅ PRAISE: Good use of a constant for the annotation name - ensures consistency with operator PR #247 and prevents typos across the codebase.

Alan-Cha · 2026-03-27T17:44:27Z

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials.go

+
+// NeedsKeycloakClientCredentialsVolumePatch reports whether the pod still needs Secret volume mounts for
+// operator-managed Keycloak client credentials (e.g. after webhook reinvocation when sidecars were injected first).
+func NeedsKeycloakClientCredentialsVolumePatch(podSpec *corev1.PodSpec, annotations map[string]string) bool {


✅ PRAISE: Excellent idempotency check! NeedsKeycloakClientCredentialsVolumePatch correctly handles reinvocation scenarios where the annotation appears after initial sidecar injection.

Alan-Cha · 2026-03-27T17:44:27Z

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials.go

+	return false
+}
+
+func appendSubPathMount(c *corev1.Container, vol, subPath, mountPath string) {


✅ PRAISE: appendSubPathMount correctly implements idempotent mounting - checks for existing mounts before appending. This prevents duplicate volume mounts on reinvocation.

Alan-Cha · 2026-03-27T17:44:27Z

kagenti-webhook/internal/webhook/injector/keycloak_client_credentials_test.go

+		t.Fatalf("mounts: %#v", c.VolumeMounts)
+	}
+
+	ApplyKeycloakClientCredentialsSecretVolumes(spec, ann) // idempotent


✅ PRAISE: Testing idempotency explicitly (calling ApplyKeycloakClientCredentialsSecretVolumes twice and verifying volume count stays at 1) is excellent practice.

Alan-Cha · 2026-03-27T17:44:27Z

kagenti-webhook/internal/webhook/v1alpha1/authbridge_webhook.go

+	if alreadyInjected {
+		// Reinvocation / upgrade: sidecars exist but operator-managed registration Secret mounts may
+		// still be missing (annotation added after first injection pass).
+		if injector.NeedsKeycloakClientCredentialsVolumePatch(&pod.Spec, pod.Annotations) {


✅ PRAISE: The reinvocation logic is well-implemented - handles the case where the operator annotation appears after initial injection. The verbose logging (V(1).Info) will help debugging without cluttering normal logs.

Alan-Cha · 2026-03-27T17:44:27Z

kagenti-webhook/internal/webhook/injector/pod_mutator.go

+// logClientRegistrationPaths records how Keycloak client credentials are delivered for this Pod:
+// Secret mounted from annotation (kagenti-operator), legacy kagenti-client-registration sidecar, or combined authbridge.
+// deliveryPaths is comma-separated short tokens: operator-secret, combined-authbridge, sidecar, skip.
+func logClientRegistrationPaths(namespace, crName string, labels map[string]string, combinedSidecar bool, decision InjectionDecision, annotations map[string]string) {


✅ PRAISE: logClientRegistrationPaths provides excellent observability into how credentials are delivered (operator-secret vs sidecar vs combined vs skip). The comma-separated deliveryPaths field makes it easy to track multiple concurrent delivery methods.

Alan-Cha · 2026-03-27T17:47:37Z

The PR looks good to me. I will approve for now but I will do some more testing in the mean time

Alan-Cha · 2026-03-27T20:16:03Z

I asked Claude to help me test a number of scenarios

7 Test Scenarios Executed:

✅ Baseline Webhook Injection - Verified all 4 sidecars injected without opt-out label
✅ Opt-Out Label Behavior - Verified client-registration sidecar correctly omitted
✅ RBAC Fix Validation - Fixed missing permissions, operator reconciled successfully
✅ Operator Secret Creation - Real Keycloak credentials created in Secret
✅ Rolling Update Trigger - Annotation change created new ReplicaSet
✅ Webhook Secret Mounting - Secret mounted at correct paths with subPath
⚠️ Sidecar Image Availability - Images loaded, pods blocked only by missing SPIRE (expected)
Pass Rate: 100% for integration testing (6/6 core scenarios + 1 partial due to infrastructure)

Key Validation Points:

Opt-out label prevents sidecar injection ✅
Operator registers OAuth clients with Keycloak ✅
Operator creates Secrets with real credentials ✅
Webhook reads annotation and mounts Secret ✅
Volume mounts use correct subPath for individual files ✅
Both PRs work together seamlessly ✅

akram force-pushed the feature/operator-managed-client-registration branch from c510134 to 019144b Compare March 27, 2026 12:41

akram mentioned this pull request Mar 27, 2026

feat(operator): operator-managed Keycloak client registration kagenti/kagenti-operator#247

Open

cwiklik requested changes Mar 27, 2026

View reviewed changes

akram added 4 commits March 27, 2026 15:50

Address review comments

8a25d04

Signed-off-by: Akram <akram.benaissi@gmail.com>

akram force-pushed the feature/operator-managed-client-registration branch from 019144b to 8a25d04 Compare March 27, 2026 14:51

Alan-Cha reviewed Mar 27, 2026

View reviewed changes

Alan-Cha requested changes Mar 27, 2026

View reviewed changes

Alan-Cha approved these changes Mar 27, 2026

View reviewed changes

Conversation

akram commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Related

Testing

Uh oh!

cwiklik left a comment

Choose a reason for hiding this comment

Review Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha left a comment

Choose a reason for hiding this comment

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alan-Cha commented Mar 27, 2026

Uh oh!

Alan-Cha commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

akram commented Mar 27, 2026 •

edited

Loading