Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions kagenti-operator/internal/clientreg/names.go
Original file line number Diff line number Diff line change
@@ -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) == ""
}
69 changes: 69 additions & 0 deletions kagenti-operator/internal/clientreg/names_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ package controller

import (
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
"strings"
"time"
Expand All @@ -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"
)

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions kagenti-operator/internal/webhook/injector/pod_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
65 changes: 65 additions & 0 deletions kagenti-operator/internal/webhook/injector/pod_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading