From b79c7a6465286b555bf8e7cf665fb8837b2e6b46 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Wed, 20 May 2026 19:58:36 -0400 Subject: [PATCH 1/4] feat(authbridge): Wire mtlsMode through to per-agent config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces a MTLSMode field on AgentRuntimeSpec and plumbs it through the existing CR > namespace > default resolution chain so the admission webhook can render an mtls: block into the per-agent authbridge ConfigMap. With this in place, kagenti-extensions PR #424 (mTLS between authbridge sidecars on the proxy-sidecar / lite paths) can be enabled declaratively per workload instead of by hand-editing the namespace ConfigMap. Behavior: * MTLSMode enum: disabled (default) | permissive | strict. Resolution: AgentRuntime CR > namespace authbridge-runtime-config mtls.mode > "disabled". Mirrors the existing AuthBridgeMode chain. * Per-agent ConfigMap renders top-level mtls: {mode: } when resolved mode is non-disabled. Cert paths are intentionally not emitted — they default to authbridge's bundled-spiffe-helper convention (/opt/svid.pem, /opt/svid_key.pem, /opt/svid_bundle.pem), so surfacing them here would couple the operator to authbridge's internal layout for no benefit. Stale mtls blocks in the base YAML are scrubbed when toggling back to disabled. * mtlsMode != disabled implicitly requires SPIRE because the bundled spiffe-helper writes the X.509 SVID files mTLS reads. The pod mutator auto-enables SPIRE for the workload when mtlsMode is set — operators don't need to add the spiffe-helper-inject label separately. The operator does not mutate the namespace authbridge-config ConfigMap (which would race with the kagenti meta-chart for ownership); it just changes the in-memory injection decision. * The AgentRuntime validating webhook rejects mtlsMode != disabled when authBridgeMode is envoy-sidecar. Envoy SDS isn't currently configured by the kagenti envoy-config, so accepting the combo would silently produce a workload running plaintext on the wire while the user believed they had strict mTLS. The error message points to the supported modes (proxy-sidecar / lite) and flags this as a current limitation, not a permanent one — when the envoy-config gains SDS support, the rejection just goes away. * Spec changes to AuthBridgeMode and MTLSMode now flow into the AgentRuntime controller's resolved-config hash. Editing either field re-stamps the kagenti.io/config-hash pod-template annotation and the Deployment rolls automatically. (Pre-existing gap fixed along the way: AuthBridgeMode changes also weren't triggering rollouts before this commit.) Out of scope: * envoy-sidecar mTLS via Envoy SDS — separate piece of work in the kagenti meta-chart's envoy-config template. * Namespace-level authbridge-runtime-config edits triggering rollouts. Today the namespace ConfigMap is consumed by the admission webhook only; namespace-level mtls.mode changes won't auto-roll existing workloads. Operators editing the namespace config should kubectl rollout restart manually until that's addressed. * Chart-side CRD at charts/kagenti-operator/crds/ is intentionally not touched. That copy has been drifting from config/crd/bases/ since commit 677f63d (predates `lite` mode entirely). Recent CRD-touching commits (4eb62af, 21a2258) follow the same "config is source of truth" pattern; this PR matches. Cleaning up the chart-side drift is a separate concern. Tests added: * ExtractMTLSMode parser cases (empty, missing block, unknown mode, malformed YAML, mtls coexisting with other top-level blocks). * ensurePerAgentConfigMap: strict and permissive emit the block; disabled and "" omit the block; stale-block scrubbing on toggle back to disabled. * Validating webhook: 11-case matrix of (mode, mtls) combinations on Create and Update; envoy-sidecar + non-disabled rejected, error message points to the follow-up. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../api/v1alpha1/agentruntime_types.go | 29 ++++ .../agent.kagenti.dev_agentruntimes.yaml | 31 ++++ .../controller/agentruntime_config.go | 17 +++ .../webhook/injector/agentruntime_config.go | 15 +- .../internal/webhook/injector/constants.go | 12 ++ .../webhook/injector/namespace_config.go | 25 +++ .../webhook/injector/namespace_config_test.go | 30 ++++ .../internal/webhook/injector/pod_mutator.go | 93 +++++++++++- .../webhook/injector/pod_mutator_test.go | 143 ++++++++++++++++-- .../webhook/v1alpha1/agentruntime_webhook.go | 32 ++++ .../v1alpha1/agentruntime_webhook_test.go | 64 ++++++++ 11 files changed, 477 insertions(+), 14 deletions(-) diff --git a/kagenti-operator/api/v1alpha1/agentruntime_types.go b/kagenti-operator/api/v1alpha1/agentruntime_types.go index f658b9d2..03723ed7 100644 --- a/kagenti-operator/api/v1alpha1/agentruntime_types.go +++ b/kagenti-operator/api/v1alpha1/agentruntime_types.go @@ -90,6 +90,35 @@ type AgentRuntimeSpec struct { // +optional // +kubebuilder:validation:Enum=proxy-sidecar;envoy-sidecar;lite;waypoint AuthBridgeMode string `json:"authBridgeMode,omitempty"` + + // MTLSMode selects the mTLS posture between authbridge sidecars on + // the proxy-sidecar / lite paths. envoy-sidecar handles transport + // security through Envoy SDS, which is currently not configured by + // the kagenti envoy-config — admission rejects mtlsMode != disabled + // when authBridgeMode is envoy-sidecar (tracked as a follow-up). + // + // Three valid values: + // + // disabled Plaintext between sidecars (default). + // permissive Inbound: byte-peek listener accepts both TLS and + // plaintext on the same port. Outbound: tries TLS, + // falls back to plaintext on handshake failure (one-line + // WARN log per fallback). Use during rollout. + // strict Inbound: TLS-only, plaintext callers closed at + // accept. Outbound: TLS-or-fail. Use after rollout + // completes. + // + // Resolution: AgentRuntime CR > namespace authbridge-runtime-config + // mtls.mode > "disabled". Setting mtlsMode != disabled implicitly + // requires SPIRE — the operator auto-enables spire for the workload. + // + // Note: changing mtlsMode triggers a pod rollout because authbridge + // cannot hot-reload mTLS config (the byte-peek listener is wired at + // process start). + // + // +optional + // +kubebuilder:validation:Enum=disabled;permissive;strict + MTLSMode string `json:"mtlsMode,omitempty"` } // IdentitySpec configures workload identity for an AgentRuntime. diff --git a/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml b/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml index 6cfac75c..8d5fcb6b 100644 --- a/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml +++ b/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml @@ -108,6 +108,37 @@ spec: type: string type: object type: object + mtlsMode: + description: |- + MTLSMode selects the mTLS posture between authbridge sidecars on + the proxy-sidecar / lite paths. envoy-sidecar handles transport + security through Envoy SDS, which is currently not configured by + the kagenti envoy-config — admission rejects mtlsMode != disabled + when authBridgeMode is envoy-sidecar (tracked as a follow-up). + + Three valid values: + + disabled Plaintext between sidecars (default). + permissive Inbound: byte-peek listener accepts both TLS and + plaintext on the same port. Outbound: tries TLS, + falls back to plaintext on handshake failure (one-line + WARN log per fallback). Use during rollout. + strict Inbound: TLS-only, plaintext callers closed at + accept. Outbound: TLS-or-fail. Use after rollout + completes. + + Resolution: AgentRuntime CR > namespace authbridge-runtime-config + mtls.mode > "disabled". Setting mtlsMode != disabled implicitly + requires SPIRE — the operator auto-enables spire for the workload. + + Note: changing mtlsMode triggers a pod rollout because authbridge + cannot hot-reload mTLS config (the byte-peek listener is wired at + process start). + enum: + - disabled + - permissive + - strict + type: string targetRef: description: TargetRef identifies the workload backing this agent runtime (duck typing). diff --git a/kagenti-operator/internal/controller/agentruntime_config.go b/kagenti-operator/internal/controller/agentruntime_config.go index 30cbf030..cec68761 100644 --- a/kagenti-operator/internal/controller/agentruntime_config.go +++ b/kagenti-operator/internal/controller/agentruntime_config.go @@ -58,6 +58,20 @@ type resolvedConfig struct { Trace *traceConfig `json:"trace,omitempty"` FeatureGates map[string]string `json:"featureGates,omitempty"` Defaults map[string]string `json:"defaults,omitempty"` + + // AuthBridgeMode and MTLSMode change the injected sidecar shape / + // transport posture, both of which require a pod restart to take + // effect. Including them here folds CR-edit changes into the + // config-hash so applyWorkloadConfig stamps a new hash on the pod + // template and the Deployment rolls. + // + // Namespace-level changes to the authbridge-runtime-config ConfigMap + // are NOT captured here today — that ConfigMap is consumed by the + // admission webhook, not the hash path. Operators editing the + // namespace ConfigMap should kubectl rollout restart the affected + // workload manually. + AuthBridgeMode string `json:"authBridgeMode,omitempty"` + MTLSMode string `json:"mtlsMode,omitempty"` } type traceConfig struct { @@ -143,6 +157,9 @@ func resolveConfig(ctx context.Context, c client.Reader, namespace string, spec } } + resolved.AuthBridgeMode = spec.AuthBridgeMode + resolved.MTLSMode = spec.MTLSMode + return resolved, warnings } diff --git a/kagenti-operator/internal/webhook/injector/agentruntime_config.go b/kagenti-operator/internal/webhook/injector/agentruntime_config.go index 9a6b5722..df02811e 100644 --- a/kagenti-operator/internal/webhook/injector/agentruntime_config.go +++ b/kagenti-operator/internal/webhook/injector/agentruntime_config.go @@ -54,6 +54,12 @@ type AgentRuntimeOverrides struct { // authbridge-runtime-config mode (if set) or the cluster fallback // applies. AuthBridgeMode *string + + // mTLS posture — from .spec.mtlsMode + // Nil = no per-workload override; the namespace's + // authbridge-runtime-config mtls.mode (if set) or "disabled" + // applies. + MTLSMode *string } // ReadAgentRuntimeOverrides reads the AgentRuntime CR for a given workload @@ -127,11 +133,18 @@ func extractOverrides(rt *agentv1alpha1.AgentRuntime) *AgentRuntimeOverrides { overrides.AuthBridgeMode = &mode } + // .spec.mtlsMode + if rt.Spec.MTLSMode != "" { + mode := rt.Spec.MTLSMode + overrides.MTLSMode = &mode + } + arConfigLog.Info("AgentRuntime overrides extracted", "hasSpiffeTrustDomain", overrides.SpiffeTrustDomain != nil, "hasClientRegistration", overrides.ClientRegistrationProvider != nil, "hasTrace", overrides.TraceEndpoint != nil, - "hasAuthBridgeMode", overrides.AuthBridgeMode != nil) + "hasAuthBridgeMode", overrides.AuthBridgeMode != nil, + "hasMTLSMode", overrides.MTLSMode != nil) return overrides } diff --git a/kagenti-operator/internal/webhook/injector/constants.go b/kagenti-operator/internal/webhook/injector/constants.go index 627bfb64..f3eaf888 100644 --- a/kagenti-operator/internal/webhook/injector/constants.go +++ b/kagenti-operator/internal/webhook/injector/constants.go @@ -37,3 +37,15 @@ const ( IdentityTypeSpiffe = "spiffe" ClientAuthTypeFederatedJWT = "federated-jwt" ) + +// mTLS modes for the proxy-sidecar / lite paths. Selected per workload +// via AgentRuntime CR `Spec.MTLSMode`, falling back to the namespace +// `authbridge-runtime-config` ConfigMap's `mtls.mode` field, then +// MTLSModeDisabled. envoy-sidecar mode is incompatible with mTLS today +// (Envoy SDS not configured by the kagenti envoy-config) — admission +// rejects mtlsMode != disabled in that combination. +const ( + MTLSModeDisabled = "disabled" + MTLSModePermissive = "permissive" + MTLSModeStrict = "strict" +) diff --git a/kagenti-operator/internal/webhook/injector/namespace_config.go b/kagenti-operator/internal/webhook/injector/namespace_config.go index f93e8687..a05128b9 100644 --- a/kagenti-operator/internal/webhook/injector/namespace_config.go +++ b/kagenti-operator/internal/webhook/injector/namespace_config.go @@ -161,3 +161,28 @@ func ExtractMode(authbridgeYAML string) string { } return top.Mode } + +// ExtractMTLSMode parses an authbridge-runtime-config config.yaml string +// and returns the value of its `mtls.mode` field. Returns "" if the YAML +// is empty, malformed, has no `mtls` block, or its `mode` field is unset +// — in any of those cases the caller should fall back to the next +// resolution layer (or the "disabled" default). +// +// Same surgical-parse pattern as ExtractMode: tolerates extra top-level +// keys so older or hand-edited ConfigMaps round-trip cleanly. +func ExtractMTLSMode(authbridgeYAML string) string { + if authbridgeYAML == "" { + return "" + } + var top struct { + MTLS struct { + Mode string `json:"mode"` + } `json:"mtls"` + } + if err := yaml.Unmarshal([]byte(authbridgeYAML), &top); err != nil { + nsConfigLog.Info("WARN: failed to parse authbridge-runtime-config config.yaml for mtls.mode; falling back to next resolution layer", + "error", err.Error()) + return "" + } + return top.MTLS.Mode +} diff --git a/kagenti-operator/internal/webhook/injector/namespace_config_test.go b/kagenti-operator/internal/webhook/injector/namespace_config_test.go index 5ae74037..e03c248f 100644 --- a/kagenti-operator/internal/webhook/injector/namespace_config_test.go +++ b/kagenti-operator/internal/webhook/injector/namespace_config_test.go @@ -133,3 +133,33 @@ func TestReadNamespaceConfig_PartialConfig(t *testing.T) { cfg.TokenURL, cfg.Issuer) } } + +func TestExtractMTLSMode(t *testing.T) { + tests := []struct { + name string + yaml string + want string + }{ + {"empty input", "", ""}, + {"no mtls block", "mode: proxy-sidecar\n", ""}, + {"mtls block, mode strict", "mtls:\n mode: strict\n", "strict"}, + {"mtls block, mode permissive", "mtls:\n mode: permissive\n", "permissive"}, + {"mtls block, mode disabled", "mtls:\n mode: disabled\n", "disabled"}, + {"mtls block with cert paths overridden", "mtls:\n mode: strict\n cert_file: /custom/cert.pem\n", "strict"}, + {"mtls block, no mode key", "mtls:\n cert_file: /opt/svid.pem\n", ""}, + {"mtls and other blocks", "mode: proxy-sidecar\nmtls:\n mode: permissive\nlistener:\n reverse_proxy_addr: \":8080\"\n", "permissive"}, + // Malformed YAML produces "" — caller falls through to the next + // resolution layer. Internal WARN log is logged but not asserted + // here (slog handler is a global; capturing it adds harness noise). + {"malformed yaml falls through", "mtls:\n mode: strict\n - this is invalid", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ExtractMTLSMode(tt.yaml) + if got != tt.want { + t.Errorf("ExtractMTLSMode(%q) = %q, want %q", tt.yaml, got, tt.want) + } + }) + } +} diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator.go b/kagenti-operator/internal/webhook/injector/pod_mutator.go index 80b5d33c..2d71fe80 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator.go @@ -228,6 +228,63 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp nsConfig = &NamespaceConfig{} } + // ======================================== + // Resolve mTLS posture (CR > namespace > "disabled") + // ======================================== + // + // Done BEFORE the volume-building / per-workload resolution so that + // "mtlsMode != disabled implies SPIRE" can flip spireEnabled and + // fall through to the existing SPIRE-aware code paths (volumes, + // ServiceAccount, container env). Mode-compat validation + // (mtlsMode incompatible with envoy-sidecar) runs in the + // AgentRuntime validating webhook upstream of pod admission. + mtlsMode := MTLSModeDisabled + mtlsSource := "default" + if arOverrides != nil && arOverrides.MTLSMode != nil { + mtlsMode = *arOverrides.MTLSMode + mtlsSource = "agentruntime-cr" + } + if mtlsMode == MTLSModeDisabled { + if m := ExtractMTLSMode(nsConfig.AuthBridgeRuntimeYAML); m != "" { + mtlsMode = m + mtlsSource = "namespace-configmap" + } + } + switch mtlsMode { + case MTLSModeDisabled, MTLSModePermissive, MTLSModeStrict: + // recognized, keep as-is + default: + mutatorLog.Info("WARN: unrecognized mtlsMode; defaulting to disabled", + "namespace", namespace, "crName", crName, + "unrecognized", mtlsMode, "source", mtlsSource) + mtlsMode = MTLSModeDisabled + mtlsSource = "default-invalid-fallback" + } + mutatorLog.Info("resolved mTLS mode", + "namespace", namespace, "crName", crName, + "mode", mtlsMode, "source", mtlsSource) + + // Auto-enable SPIRE when mtls is on. The bundled spiffe-helper + // writes /opt/svid*.pem from SPIRE-issued X.509 SVIDs; without + // SPIRE the cert files never appear and authbridge stays in its + // startup wait loop. Setting mtlsMode is sufficient declaration of + // intent — the operator does not require a separate SPIRE label. + if mtlsMode != MTLSModeDisabled && !spireEnabled { + mutatorLog.Info("mtlsMode set; auto-enabling SPIRE for this workload", + "namespace", namespace, "crName", crName, "mtlsMode", mtlsMode) + spireEnabled = true + if podSpec.ServiceAccountName == "" || podSpec.ServiceAccountName == "default" { + if err := m.ensureServiceAccount(ctx, namespace, crName); err != nil { + mutatorLog.Error(err, "Failed to ensure ServiceAccount for auto-enabled SPIRE", + "namespace", namespace, "name", crName) + return false, fmt.Errorf("failed to ensure service account for mtls auto-spire: %w", err) + } + podSpec.ServiceAccountName = crName + mutatorLog.Info("Set ServiceAccountName for auto-enabled SPIRE identity", + "namespace", namespace, "serviceAccount", crName) + } + } + if currentGates.PerWorkloadConfigResolution { // Resolved path: build literal env vars from namespace config // arOverrides was already read above as a gate check. @@ -417,7 +474,8 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp "reverse_proxy_addr": fmt.Sprintf(":%d", originalAgentPort), "reverse_proxy_backend": fmt.Sprintf("http://127.0.0.1:%d", newAgentPort), "forward_proxy_addr": fmt.Sprintf(":%d", forwardProxyPort), - }) + }, + mtlsMode) if err != nil { return false, fmt.Errorf("proxy-sidecar per-agent ConfigMap: %w", err) } @@ -488,8 +546,12 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp // authbridge + bundled spiffe-helper. proxy-init is a separate // init container. spiffe-helper starts conditionally on SPIRE_ENABLED. + // envoy-sidecar always passes mtlsMode="" — the validating webhook + // rejects mtlsMode != disabled with envoy-sidecar at admission, so + // we'd never reach this branch with a non-empty mtlsMode in practice; + // passing "" here is the explicit-defense complement. perAgentCMName, err := m.ensurePerAgentConfigMap(ctx, namespace, crName, - ModeEnvoySidecar, nsConfig.AuthBridgeRuntimeYAML, nsConfig, nil) + ModeEnvoySidecar, nsConfig.AuthBridgeRuntimeYAML, nsConfig, nil, "") if err != nil { return false, fmt.Errorf("envoy-sidecar per-agent ConfigMap: %w", err) } @@ -668,16 +730,23 @@ func synthesizePipeline(nsConfig *NamespaceConfig) map[string]interface{} { // ensurePerAgentConfigMap creates or updates a per-agent ConfigMap that merges the // namespace-level authbridge-runtime-config with per-agent overrides (mode, listener -// addresses). The authbridge sidecar mounts this instead of the shared ConfigMap. +// addresses, mtls). The authbridge sidecar mounts this instead of the shared ConfigMap. // // If baseYAML is empty (namespace has no authbridge-runtime-config), a minimal config // is generated from the NamespaceConfig fields. +// +// mtlsMode is the resolved mTLS posture (disabled / permissive / strict). Only +// proxy-sidecar and lite paths reach this function with a non-disabled mtlsMode; +// the AgentRuntime validating webhook rejects mtlsMode != disabled when +// authBridgeMode is envoy-sidecar (Envoy SDS isn't wired in the kagenti envoy-config +// today — tracked as a follow-up). func (m *PodMutator) ensurePerAgentConfigMap( ctx context.Context, namespace, crName, mode string, baseYAML string, nsConfig *NamespaceConfig, listenerOverrides map[string]string, + mtlsMode string, ) (string, error) { cmName := perAgentConfigMapName(crName) @@ -727,6 +796,21 @@ func (m *PodMutator) ensurePerAgentConfigMap( cfg["listener"] = listener } + // mTLS block. Cert paths are omitted on purpose — they match the + // authbridge defaults (/opt/svid.pem, /opt/svid_key.pem, + // /opt/svid_bundle.pem) written by the bundled spiffe-helper. + // Surfacing them here would couple the operator to authbridge's + // internal layout for no benefit. + if mtlsMode != "" && mtlsMode != MTLSModeDisabled { + cfg["mtls"] = map[string]interface{}{"mode": mtlsMode} + } else { + // Defensive: scrub any stale mtls block from the base YAML when + // mTLS is off. Otherwise toggling mtlsMode back to disabled + // without restarting the namespace ConfigMap would leak the + // previous setting through to the per-agent CM. + delete(cfg, "mtls") + } + // Marshal back to YAML data, err := yaml.Marshal(cfg) if err != nil { @@ -748,7 +832,8 @@ func (m *PodMutator) ensurePerAgentConfigMap( if err := m.Client.Apply(ctx, cmApply, client.FieldOwner("kagenti-webhook"), client.ForceOwnership); err != nil { return "", fmt.Errorf("failed to apply per-agent ConfigMap %s/%s: %w", namespace, cmName, err) } - mutatorLog.Info("Applied per-agent ConfigMap", "namespace", namespace, "name", cmName, "mode", mode) + mutatorLog.Info("Applied per-agent ConfigMap", + "namespace", namespace, "name", cmName, "mode", mode, "mtlsMode", mtlsMode) return cmName, nil } diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go index ea090100..3c037a7c 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go @@ -1242,7 +1242,7 @@ func TestEnsurePerAgentConfigMap_EmptyBaseYAML_FallbackFromNsConfig(t *testing.T } cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "weather-service", - ModeProxySidecar, "", nsConfig, nil) + ModeProxySidecar, "", nsConfig, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1359,7 +1359,7 @@ pipeline: ` cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent", - ModeEnvoySidecar, baseYAML, &NamespaceConfig{}, nil) + ModeEnvoySidecar, baseYAML, &NamespaceConfig{}, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1418,7 +1418,7 @@ pipeline: } cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent", - ModeProxySidecar, baseYAML, &NamespaceConfig{}, overrides) + ModeProxySidecar, baseYAML, &NamespaceConfig{}, overrides, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1454,7 +1454,7 @@ func TestEnsurePerAgentConfigMap_ExistingCM_OwnedByWebhook_Updated(t *testing.T) ctx := context.Background() _, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent", - ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil) + ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1482,7 +1482,7 @@ func TestEnsurePerAgentConfigMap_ExistingCM_OverwrittenBySSA(t *testing.T) { ctx := context.Background() cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent", - ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil) + ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1510,7 +1510,7 @@ func TestEnsurePerAgentConfigMap_OwnerReference_SetFromDeployment(t *testing.T) ctx := context.Background() cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "weather-service", - ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil) + ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1537,7 +1537,7 @@ func TestEnsurePerAgentConfigMap_OwnerReference_SetFromStatefulSet(t *testing.T) ctx := context.Background() cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-stateful-agent", - ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil) + ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1558,7 +1558,7 @@ func TestEnsurePerAgentConfigMap_OwnerReference_NoWorkload_Skipped(t *testing.T) ctx := context.Background() cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "bare-pod-agent", - ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil) + ModeEnvoySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1581,7 +1581,7 @@ func TestEnsurePerAgentConfigMap_FederatedJWT_MapsToSpiffe(t *testing.T) { } cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "spiffe-agent", - ModeEnvoySidecar, "", nsConfig, nil) + ModeEnvoySidecar, "", nsConfig, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1604,3 +1604,128 @@ func TestEnsurePerAgentConfigMap_FederatedJWT_MapsToSpiffe(t *testing.T) { // about file paths. See // authbridge/authlib/plugins/CONVENTIONS.md. } + +// --- mTLS rendering tests --- +// +// These cover the per-agent ConfigMap rendering with the new mtlsMode +// argument. The validating webhook upstream rejects mtlsMode != disabled +// with envoy-sidecar mode, so the renderer doesn't need to gate by mode +// — but we still test the negative ("disabled" / "" should not emit a +// block) and the scrub case (toggling back to disabled wipes a stale +// block from the base YAML). + +// TestEnsurePerAgentConfigMap_MTLSStrict_RendersBlock verifies that +// mtlsMode=strict produces a top-level mtls: {mode: strict} block. +// Cert paths are intentionally NOT emitted — they default to the +// authbridge-side defaults (/opt/svid*.pem) written by spiffe-helper. +func TestEnsurePerAgentConfigMap_MTLSStrict_RendersBlock(t *testing.T) { + m := newTestMutator() + ctx := context.Background() + + cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "mtls-agent", + ModeProxySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, MTLSModeStrict) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cm := fetchConfigMap(t, m, "team1", cmName) + cfg := parseConfigYAML(t, cm) + + mtls, ok := cfg["mtls"].(map[string]interface{}) + if !ok { + t.Fatalf("expected mtls block to be a map; got %T (cfg=%+v)", cfg["mtls"], cfg) + } + if mtls["mode"] != MTLSModeStrict { + t.Errorf("mtls.mode = %v, want %s", mtls["mode"], MTLSModeStrict) + } + // Cert paths are NOT rendered — operator stays decoupled from + // authbridge's internal layout. + for _, key := range []string{"cert_file", "key_file", "bundle_file"} { + if _, present := mtls[key]; present { + t.Errorf("mtls.%s should not be emitted (authbridge supplies defaults)", key) + } + } +} + +// TestEnsurePerAgentConfigMap_MTLSPermissive_RendersBlock mirrors the +// strict test for permissive mode — same shape, different mode value. +func TestEnsurePerAgentConfigMap_MTLSPermissive_RendersBlock(t *testing.T) { + m := newTestMutator() + ctx := context.Background() + + cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "mtls-agent", + ModeProxySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, MTLSModePermissive) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cm := fetchConfigMap(t, m, "team1", cmName) + cfg := parseConfigYAML(t, cm) + + mtls, ok := cfg["mtls"].(map[string]interface{}) + if !ok { + t.Fatalf("expected mtls block to be a map; got %T", cfg["mtls"]) + } + if mtls["mode"] != MTLSModePermissive { + t.Errorf("mtls.mode = %v, want %s", mtls["mode"], MTLSModePermissive) + } +} + +// TestEnsurePerAgentConfigMap_MTLSDisabled_OmitsBlock verifies that the +// renderer does NOT emit mtls when mtlsMode is disabled or empty. +// Empty-string is the envoy-sidecar carve-out path — the call site +// passes "" explicitly so we test that too. +func TestEnsurePerAgentConfigMap_MTLSDisabled_OmitsBlock(t *testing.T) { + tests := []struct { + name string + mtlsMode string + }{ + {"empty string", ""}, + {"disabled", MTLSModeDisabled}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := newTestMutator() + ctx := context.Background() + + cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "no-mtls-"+tt.name, + ModeProxySidecar, "", &NamespaceConfig{ClientAuthType: "client-secret"}, nil, tt.mtlsMode) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cm := fetchConfigMap(t, m, "team1", cmName) + cfg := parseConfigYAML(t, cm) + + if _, present := cfg["mtls"]; present { + t.Errorf("mtls block should not be emitted when mtlsMode=%q (cfg=%+v)", tt.mtlsMode, cfg) + } + }) + } +} + +// TestEnsurePerAgentConfigMap_MTLSScrubsStaleBlock guards against a +// regression where toggling mtlsMode from strict back to disabled would +// leak the previous mtls block through to the per-agent CM. The +// renderer must explicitly delete cfg["mtls"] when mode is off. +func TestEnsurePerAgentConfigMap_MTLSScrubsStaleBlock(t *testing.T) { + m := newTestMutator() + ctx := context.Background() + + // Base YAML with a stale mtls: strict — simulates a namespace + // ConfigMap that was rendered earlier with mtls on. + baseYAML := "mode: proxy-sidecar\nmtls:\n mode: strict\n" + + cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "scrub-agent", + ModeProxySidecar, baseYAML, &NamespaceConfig{ClientAuthType: "client-secret"}, nil, MTLSModeDisabled) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cm := fetchConfigMap(t, m, "team1", cmName) + cfg := parseConfigYAML(t, cm) + + if _, present := cfg["mtls"]; present { + t.Errorf("stale mtls block should be scrubbed when mtlsMode=disabled; got cfg=%+v", cfg) + } +} diff --git a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go index 9344dbb9..395783e1 100644 --- a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go +++ b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go @@ -49,6 +49,9 @@ func (v *AgentRuntimeValidator) ValidateCreate(ctx context.Context, rt *agentv1a if err := v.checkDuplicateTargetRef(ctx, rt); err != nil { return nil, err } + if err := checkMTLSCompatibleWithMode(rt); err != nil { + return nil, err + } return nil, nil } @@ -59,6 +62,9 @@ func (v *AgentRuntimeValidator) ValidateUpdate(ctx context.Context, _ *agentv1al if err := v.checkDuplicateTargetRef(ctx, rt); err != nil { return nil, err } + if err := checkMTLSCompatibleWithMode(rt); err != nil { + return nil, err + } return nil, nil } @@ -69,6 +75,32 @@ func (v *AgentRuntimeValidator) ValidateDelete(_ context.Context, rt *agentv1alp return nil, nil } +// checkMTLSCompatibleWithMode rejects mtlsMode != disabled when authBridgeMode +// is envoy-sidecar. Envoy SDS isn't currently configured by the kagenti +// envoy-config — extending it to do real mTLS is a separate piece of work. +// Until that lands, the only modes that actually carry mTLS are +// proxy-sidecar and lite (kagenti-extensions PR #424). Rejecting at admission +// makes the misconfiguration loud instead of producing a workload that +// silently runs plaintext while the user believes they have strict mTLS. +// +// Empty / "disabled" mtlsMode is permitted for any authBridgeMode — that's +// today's plaintext default. The error message points to the supported +// modes and flags this as a current limitation, not a permanent one. +func checkMTLSCompatibleWithMode(rt *agentv1alpha1.AgentRuntime) error { + mtls := rt.Spec.MTLSMode + if mtls == "" || mtls == "disabled" { + return nil + } + if rt.Spec.AuthBridgeMode == "envoy-sidecar" { + return fmt.Errorf( + "mtlsMode=%q is not supported with authBridgeMode=envoy-sidecar; "+ + "set authBridgeMode to proxy-sidecar or lite (envoy-sidecar mTLS is tracked as a follow-up)", + mtls, + ) + } + return nil +} + // checkDuplicateTargetRef rejects creation/update if another AgentRuntime already // targets the same workload (apiVersion + kind + name) in the same namespace. func (v *AgentRuntimeValidator) checkDuplicateTargetRef(ctx context.Context, rt *agentv1alpha1.AgentRuntime) error { diff --git a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go index 6379da74..a315a4a3 100644 --- a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go +++ b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go @@ -246,3 +246,67 @@ func TestAgentRuntimeValidator_ValidateDelete(t *testing.T) { }) } + +// TestAgentRuntimeValidator_MTLSCompatWithMode covers the rejection of +// mtlsMode != disabled when authBridgeMode is envoy-sidecar. The kagenti +// envoy-config doesn't currently configure SDS, so an envoy-sidecar +// workload that sets mtlsMode would silently run plaintext while the +// user believes they have strict mTLS — the validator catches that at +// admission time. +func TestAgentRuntimeValidator_MTLSCompatWithMode(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + mode string + mtls string + wantErr bool + }{ + {"proxy-sidecar + strict allowed", "proxy-sidecar", "strict", false}, + {"proxy-sidecar + permissive allowed", "proxy-sidecar", "permissive", false}, + {"proxy-sidecar + disabled allowed", "proxy-sidecar", "disabled", false}, + {"proxy-sidecar + empty allowed", "proxy-sidecar", "", false}, + {"lite + strict allowed", "lite", "strict", false}, + {"lite + permissive allowed", "lite", "permissive", false}, + {"empty mode + strict allowed", "", "strict", false}, + {"envoy-sidecar + disabled allowed", "envoy-sidecar", "disabled", false}, + {"envoy-sidecar + empty allowed", "envoy-sidecar", "", false}, + {"envoy-sidecar + permissive rejected", "envoy-sidecar", "permissive", true}, + {"envoy-sidecar + strict rejected", "envoy-sidecar", "strict", true}, + } + + for _, tt := range tests { + t.Run("create/"+tt.name, func(t *testing.T) { + rt := validAgentRuntime() + rt.Spec.AuthBridgeMode = tt.mode + rt.Spec.MTLSMode = tt.mtls + + v := &AgentRuntimeValidator{} + _, err := v.ValidateCreate(ctx, rt) + gotErr := err != nil + if gotErr != tt.wantErr { + t.Errorf("ValidateCreate(mode=%q, mtls=%q): wantErr=%v, gotErr=%v (err=%v)", + tt.mode, tt.mtls, tt.wantErr, gotErr, err) + } + if tt.wantErr && err != nil && + !strings.Contains(err.Error(), "envoy-sidecar mTLS is tracked as a follow-up") { + t.Errorf("error message should point to follow-up; got: %v", err) + } + }) + + t.Run("update/"+tt.name, func(t *testing.T) { + old := validAgentRuntime() + updated := validAgentRuntime() + updated.Spec.AuthBridgeMode = tt.mode + updated.Spec.MTLSMode = tt.mtls + + v := &AgentRuntimeValidator{} + _, err := v.ValidateUpdate(ctx, old, updated) + gotErr := err != nil + if gotErr != tt.wantErr { + t.Errorf("ValidateUpdate(mode=%q, mtls=%q): wantErr=%v, gotErr=%v (err=%v)", + tt.mode, tt.mtls, tt.wantErr, gotErr, err) + } + }) + } +} From c1d0030be34746b1ba330c1cadbea1d0d54aaf58 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Wed, 20 May 2026 20:13:32 -0400 Subject: [PATCH 2/4] feat(controller): Watch authbridge-runtime-config + sync chart CRDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-existing tech-debt items folded into the mtlsMode wiring PR (addressing reviewer-callout from PR #369 itself): 1. Namespace-CM-edit rollout gap. The controller's resolved-config hash captured a few CR fields and the cluster-defaults / namespace-defaults ConfigMaps, but NOT the namespace authbridge-runtime-config ConfigMap that the admission webhook reads at pod creation. So `kubectl edit configmap authbridge-runtime-config -n team1` silently failed to roll the workloads in that namespace — operators had to know to run `kubectl rollout restart` manually. - Add an AuthBridgeRuntime field to resolvedConfig that captures the raw config.yaml content (not parsed — any byte change rolls the workload, agnostic to whether it's mtls / pipeline / listener edits). - Extend mapNamespaceConfigMapToAgentRuntimes to also match ConfigMaps named "authbridge-runtime-config" (not just kagenti.io/defaults=true labeled ones), so edits to that CM enqueue every AgentRuntime in the namespace and the hash re-stamps the pod template. - New constant AuthBridgeRuntimeConfigMapName for the magic name. Other places in the codebase still use the literal string; consolidating them is out of scope. - Tests: hash-changes-on-edit (extends ComputeConfigHash spec) + mapper-matches-by-name (extends the existing namespace mapper test). 2. Chart-CRD drift. charts/kagenti-operator/crds/ holds a manually-maintained Helm crds/ directory copy of the AgentRuntime + AgentCard CRDs. It was last touched in commit 677f63d (initial webhook migration) and has been silently drifting from kagenti-operator/config/crd/bases/ for at least three subsequent CRD-touching commits (21a2258, 4eb62af, and earlier this PR). Anyone helm-installing got an outdated CRD missing recent fields like authBridgeMode / lite mode / mtlsMode. - Sync both CRDs from config/crd/bases/ to charts/.../crds/. - Add a sync-chart-crds Makefile target invoked at the tail of `make manifests`, so future controller-gen regenerations propagate to the chart copy automatically. Skips the stub `_.yaml` controller-gen sometimes emits. PR #369 originally noted both items as out-of-scope follow-ups; this commit folds them in per reviewer/maintainer feedback. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../crds/agent.kagenti.dev_agentcards.yaml | 28 ++++++-- .../crds/agent.kagenti.dev_agentruntimes.yaml | 69 ++++++++++++++++++- kagenti-operator/Makefile | 19 +++++ .../controller/agentruntime_config.go | 34 ++++++--- .../controller/agentruntime_config_test.go | 45 +++++++++++- .../controller/agentruntime_controller.go | 22 +++++- 6 files changed, 198 insertions(+), 19 deletions(-) diff --git a/charts/kagenti-operator/crds/agent.kagenti.dev_agentcards.yaml b/charts/kagenti-operator/crds/agent.kagenti.dev_agentcards.yaml index 11f366d7..4efde13f 100644 --- a/charts/kagenti-operator/crds/agent.kagenti.dev_agentcards.yaml +++ b/charts/kagenti-operator/crds/agent.kagenti.dev_agentcards.yaml @@ -34,10 +34,10 @@ spec: jsonPath: .status.card.name name: Agent type: string - - description: Signature Verified - jsonPath: .status.validSignature + - description: Identity Verified + jsonPath: .status.conditions[?(@.type=='Verified')].status name: Verified - type: boolean + type: string - description: Identity Bound jsonPath: .status.bindingStatus.bound name: Bound @@ -53,10 +53,18 @@ spec: - jsonPath: .metadata.creationTimestamp name: Age type: date + - description: Attested Agent SPIFFE ID + jsonPath: .status.attestedAgentSpiffeId + name: AttestedAgent + priority: 1 + type: string name: v1alpha1 schema: openAPIV3Schema: - description: AgentCard is the Schema for the agentcards API. + description: |- + AgentCard binds an A2A agent card to a backing workload. The controller periodically + fetches the card from the referenced workload, verifies its JWS signature and SPIFFE + identity against the configured trust domain, and caches the result in status. properties: apiVersion: description: |- @@ -84,8 +92,11 @@ spec: strict: default: false description: |- - Strict enables enforcement mode: binding failures trigger network isolation. - When false (default), results are recorded in status only (audit mode). + Strict controls whether binding failures trigger enforcement actions + (label removal, restrictive NetworkPolicy). + When true, binding failure removes the verified label and applies restrictive NetworkPolicy. + When false (default), binding results are recorded in status only; + the workload retains its verified label and permissive policy. type: boolean trustDomain: description: |- @@ -126,6 +137,11 @@ spec: status: description: AgentCardStatus defines the observed state of AgentCard. properties: + attestedAgentSpiffeId: + description: |- + AttestedAgentSpiffeID is the SPIFFE ID extracted from the agent's TLS peer certificate + during authenticated (mTLS) fetch. Set only when verifiedFetch is enabled and successful. + type: string bindingStatus: description: BindingStatus contains the result of identity binding evaluation diff --git a/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml b/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml index 77e63922..8d5fcb6b 100644 --- a/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml +++ b/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml @@ -36,7 +36,10 @@ spec: name: v1alpha1 schema: openAPIV3Schema: - description: AgentRuntime is the Schema for the agentruntimes API. + description: |- + AgentRuntime attaches runtime configuration to a backing workload classified as an + agent or tool, providing per-workload overrides for SPIFFE identity and OpenTelemetry + tracing. The controller reports pod configuration coverage and phase in status. properties: apiVersion: description: |- @@ -58,6 +61,39 @@ spec: spec: description: AgentRuntimeSpec defines the desired state of AgentRuntime. properties: + authBridgeMode: + description: |- + AuthBridgeMode selects the deployment shape for this workload's + authbridge sidecar. When unset, the namespace-level + authbridge-runtime-config ConfigMap's mode is used; if that is + also unset, the operator falls back to "proxy-sidecar". + + Four valid values: + + proxy-sidecar HTTP_PROXY env + authbridge-proxy (full plugin + set, including a2a/mcp/inference parsers) + + spiffe-helper bundled. No Envoy, no iptables. + Default mode. + envoy-sidecar Envoy + ext_proc authbridge + spiffe-helper + bundled. Requires the proxy-init iptables + container. + lite Same listener layout as proxy-sidecar but uses + the authbridge-lite image (jwt-validation + + token-exchange only, parsers dropped to shrink + the binary). For size-constrained deployments + that don't need protocol-aware abctl events. + waypoint Standalone deployment, not injected as a + sidecar. Used by Istio ambient mesh. + + Set this when a single workload needs a different shape than the + namespace default. Most deployments leave it unset and let the + namespace ConfigMap drive the choice. + enum: + - proxy-sidecar + - envoy-sidecar + - lite + - waypoint + type: string identity: description: Identity specifies optional per-workload identity overrides properties: @@ -72,6 +108,37 @@ spec: type: string type: object type: object + mtlsMode: + description: |- + MTLSMode selects the mTLS posture between authbridge sidecars on + the proxy-sidecar / lite paths. envoy-sidecar handles transport + security through Envoy SDS, which is currently not configured by + the kagenti envoy-config — admission rejects mtlsMode != disabled + when authBridgeMode is envoy-sidecar (tracked as a follow-up). + + Three valid values: + + disabled Plaintext between sidecars (default). + permissive Inbound: byte-peek listener accepts both TLS and + plaintext on the same port. Outbound: tries TLS, + falls back to plaintext on handshake failure (one-line + WARN log per fallback). Use during rollout. + strict Inbound: TLS-only, plaintext callers closed at + accept. Outbound: TLS-or-fail. Use after rollout + completes. + + Resolution: AgentRuntime CR > namespace authbridge-runtime-config + mtls.mode > "disabled". Setting mtlsMode != disabled implicitly + requires SPIRE — the operator auto-enables spire for the workload. + + Note: changing mtlsMode triggers a pod rollout because authbridge + cannot hot-reload mTLS config (the byte-peek listener is wired at + process start). + enum: + - disabled + - permissive + - strict + type: string targetRef: description: TargetRef identifies the workload backing this agent runtime (duck typing). diff --git a/kagenti-operator/Makefile b/kagenti-operator/Makefile index 4e58a48c..07ad96bd 100644 --- a/kagenti-operator/Makefile +++ b/kagenti-operator/Makefile @@ -61,6 +61,25 @@ help: ## Display this help. .PHONY: manifests manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. $(CONTROLLER_GEN) rbac:roleName=manager-role crd:allowDangerousTypes=true webhook paths="./..." output:crd:artifacts:config=config/crd/bases + $(MAKE) sync-chart-crds + +# sync-chart-crds keeps charts/kagenti-operator/crds/ in lockstep with +# config/crd/bases/. Helm's crds/ directory is the install path consumers +# get when they `helm install`; without this sync the chart copy drifts +# silently every time a CRD field is added (which has happened multiple +# times before — commits 677f63d, 21a2258, 4eb62af). Runs as the final +# step of `make manifests` so the chart copy reflects the freshly +# regenerated controller-gen output. Skips the stub `_.yaml` file +# controller-gen sometimes emits. +.PHONY: sync-chart-crds +sync-chart-crds: ## Mirror config/crd/bases/ into charts/kagenti-operator/crds/. + @for f in $$(ls config/crd/bases/*.yaml 2>/dev/null); do \ + base=$$(basename $$f); \ + case $$base in \ + _.yaml) ;; \ + *) cp $$f ../charts/kagenti-operator/crds/$$base ;; \ + esac; \ + done .PHONY: generate generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations. diff --git a/kagenti-operator/internal/controller/agentruntime_config.go b/kagenti-operator/internal/controller/agentruntime_config.go index cec68761..7948c3dc 100644 --- a/kagenti-operator/internal/controller/agentruntime_config.go +++ b/kagenti-operator/internal/controller/agentruntime_config.go @@ -43,6 +43,12 @@ const ( // LabelNamespaceDefaults identifies namespace-level defaults ConfigMaps. LabelNamespaceDefaults = "kagenti.io/defaults" + + // AuthBridgeRuntimeConfigMapName is the namespace-scoped ConfigMap that + // holds the authbridge runtime config (config.yaml). Edits to this + // ConfigMap are watched by AgentRuntimeReconciler so the resolved-config + // hash picks them up and rolls affected workloads. + AuthBridgeRuntimeConfigMapName = "authbridge-runtime-config" ) // resolvedConfig is the canonical representation used for hash computation. @@ -64,14 +70,16 @@ type resolvedConfig struct { // effect. Including them here folds CR-edit changes into the // config-hash so applyWorkloadConfig stamps a new hash on the pod // template and the Deployment rolls. - // - // Namespace-level changes to the authbridge-runtime-config ConfigMap - // are NOT captured here today — that ConfigMap is consumed by the - // admission webhook, not the hash path. Operators editing the - // namespace ConfigMap should kubectl rollout restart the affected - // workload manually. AuthBridgeMode string `json:"authBridgeMode,omitempty"` MTLSMode string `json:"mtlsMode,omitempty"` + + // AuthBridgeRuntime captures the namespace authbridge-runtime-config + // ConfigMap's config.yaml content so namespace-level edits flow into + // the hash. Stored as the raw string (not parsed) because authbridge + // pipelines/listener/mtls config drift through here in any shape and + // we want any byte change to roll the workload. Empty string when + // the ConfigMap doesn't exist in the namespace. + AuthBridgeRuntime string `json:"authBridgeRuntime,omitempty"` } type traceConfig struct { @@ -125,9 +133,19 @@ func resolveConfig(ctx context.Context, c client.Reader, namespace string, spec } merged := mergeMaps(clusterDefaults, nsDefaults) + // Layer 2b: namespace authbridge-runtime-config (config.yaml). + // Captured raw so any byte change rolls the workload. The CM may + // not exist in every agent namespace; absence is normal and the + // admission webhook falls back to its own defaults. + abRuntime := "" + if data := readConfigMapData(ctx, c, namespace, AuthBridgeRuntimeConfigMapName); len(data) > 0 { + abRuntime = data["config.yaml"] + } + resolved := resolvedConfig{ - FeatureGates: featureGates, - Defaults: merged, + FeatureGates: featureGates, + Defaults: merged, + AuthBridgeRuntime: abRuntime, } if spec == nil { diff --git a/kagenti-operator/internal/controller/agentruntime_config_test.go b/kagenti-operator/internal/controller/agentruntime_config_test.go index 5bfad0f1..764a4110 100644 --- a/kagenti-operator/internal/controller/agentruntime_config_test.go +++ b/kagenti-operator/internal/controller/agentruntime_config_test.go @@ -257,6 +257,37 @@ var _ = Describe("AgentRuntime Config", func() { r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec) Expect(r1.Hash).NotTo(Equal(r2.Hash)) }) + + It("should change when authbridge-runtime-config edits", func() { + // Edits to the namespace authbridge-runtime-config (which the + // admission webhook reads at pod creation) must roll + // affected workloads. The hash captures its config.yaml + // content as a raw string so any byte change registers. + abCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: AuthBridgeRuntimeConfigMapName, + Namespace: namespace, + }, + Data: map[string]string{ + "config.yaml": "mode: proxy-sidecar\n", + }, + } + Expect(k8sClient.Create(ctx, abCM)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, abCM) }() + + spec := &agentv1alpha1.AgentRuntimeSpec{ + Type: agentv1alpha1.RuntimeTypeAgent, + TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-abruntime"}, + } + + r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec) + + abCM.Data["config.yaml"] = "mode: proxy-sidecar\nmtls:\n mode: strict\n" + Expect(k8sClient.Update(ctx, abCM)).To(Succeed()) + + r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec) + Expect(r1.Hash).NotTo(Equal(r2.Hash)) + }) }) Context("ComputeDefaultsOnlyHash", func() { @@ -548,7 +579,19 @@ var _ = Describe("AgentRuntime Config", func() { requests := r.mapNamespaceConfigMapToAgentRuntimes(ctx, cm) Expect(requests).NotTo(BeEmpty()) - // Should not match ConfigMap without label + // Should also match authbridge-runtime-config (matched by name, + // no label required). Editing this CM is the operator-facing + // way to change mtls / mode / pipeline plugins for the whole + // namespace; without this watch the rollout never fires. + abCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: AuthBridgeRuntimeConfigMapName, Namespace: namespace, + }, + } + Expect(r.mapNamespaceConfigMapToAgentRuntimes(ctx, abCM)).NotTo(BeEmpty()) + + // Should not match ConfigMap without label and not named + // authbridge-runtime-config (random unrelated CM). noLabel := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: "no-label", Namespace: namespace}, } diff --git a/kagenti-operator/internal/controller/agentruntime_controller.go b/kagenti-operator/internal/controller/agentruntime_controller.go index 2a2f394d..e4bf6703 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller.go +++ b/kagenti-operator/internal/controller/agentruntime_controller.go @@ -643,11 +643,27 @@ func (r *AgentRuntimeReconciler) mapClusterConfigMapToAgentRuntimes(ctx context. return agentRuntimesToRequests(rtList.Items) } -// mapNamespaceConfigMapToAgentRuntimes maps changes to namespace-level defaults -// ConfigMaps (kagenti.io/defaults=true) to AgentRuntimes in the same namespace. +// mapNamespaceConfigMapToAgentRuntimes maps changes to relevant +// namespace-scoped ConfigMaps to AgentRuntimes in the same namespace. +// Two ConfigMap shapes are watched: +// +// 1. Namespace defaults — labeled kagenti.io/defaults=true. Folded into +// resolvedConfig.Defaults during resolveConfig. +// 2. authbridge-runtime-config (matched by name, no label required) — +// this is the ConfigMap the admission webhook reads at pod creation. +// Editing it should trigger a rollout of every AgentRuntime in the +// namespace because the per-agent ConfigMap is rebuilt from this +// content on every pod admission. +// +// Both signals enqueue every AgentRuntime in the namespace; the +// reconciler's hash check filters out no-op cases (only AgentRuntimes +// whose computed hash actually changed re-stamp the pod template). func (r *AgentRuntimeReconciler) mapNamespaceConfigMapToAgentRuntimes(ctx context.Context, obj client.Object) []reconcile.Request { labels := obj.GetLabels() - if labels[LabelNamespaceDefaults] != "true" { + isNsDefaults := labels[LabelNamespaceDefaults] == "true" + isAuthBridgeRuntime := obj.GetName() == AuthBridgeRuntimeConfigMapName + + if !isNsDefaults && !isAuthBridgeRuntime { return nil } From 3420106209a0d5bf109bb6104473f7efe1fdd30e Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Wed, 20 May 2026 20:34:09 -0400 Subject: [PATCH 3/4] fix(lint): Suppress goconst on label-true comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit goconst flagged the new literal-"true" comparison in mapNamespaceConfigMapToAgentRuntimes, suggesting we reuse AnnotationRestartPendingValue — but that constant is semantically a restart-pending marker, not a generic label-true value. Reusing it would obscure intent. Existing code (defaults_config_reconciler.go:283 etc.) uses the same literal-true idiom for label-truth checks; the rule fires because --new-from-patch only inspects added lines. Suppress on the single line with a comment explaining the reasoning, rather than introducing a fresh labelValueTrue constant only here. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../internal/controller/agentruntime_controller.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/kagenti-operator/internal/controller/agentruntime_controller.go b/kagenti-operator/internal/controller/agentruntime_controller.go index e4bf6703..1434824d 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller.go +++ b/kagenti-operator/internal/controller/agentruntime_controller.go @@ -660,7 +660,14 @@ func (r *AgentRuntimeReconciler) mapClusterConfigMapToAgentRuntimes(ctx context. // whose computed hash actually changed re-stamp the pod template). func (r *AgentRuntimeReconciler) mapNamespaceConfigMapToAgentRuntimes(ctx context.Context, obj client.Object) []reconcile.Request { labels := obj.GetLabels() - isNsDefaults := labels[LabelNamespaceDefaults] == "true" + // goconst flags this literal as the 11th "true" in the codebase and + // suggests reusing AnnotationRestartPendingValue, but that constant + // is semantically a restart-pending marker, not a generic label-true + // value — reusing it would obscure intent. Existing code (e.g. + // defaults_config_reconciler.go) uses the same literal-true idiom + // for label checks; rather than introduce a fresh `labelValueTrue` + // constant only here, suppress the rule on this one line. + isNsDefaults := labels[LabelNamespaceDefaults] == "true" //nolint:goconst isAuthBridgeRuntime := obj.GetName() == AuthBridgeRuntimeConfigMapName if !isNsDefaults && !isAuthBridgeRuntime { From ccd7d08b2b185a652d60ba71a7bb05ebffd67a23 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Wed, 20 May 2026 20:56:20 -0400 Subject: [PATCH 4/4] refactor: Address review feedback on PR #369 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four review items folded in: * #2 Document the burst-reconcile implication of byte-hashing the namespace authbridge-runtime-config CM. Single-edit on N agents re-hashes all N pod specs; Kubernetes sequences the actual rolls but the controller load fans out. Fine for typical small namespaces; worth knowing before formatting edits during peak. * #3 Add CR-side hash-change tests parallel to the existing CM-edit test: flipping MTLSMode disabled→strict on the AgentRuntime CR re-hashes; flipping AuthBridgeMode proxy-sidecar→lite on the CR re-hashes. Locks the rollout-on-CR-edit behavior so a future refactor that drops MTLSMode / AuthBridgeMode from resolvedConfig silently regresses into a failing test, not a silent shipping bug. * #5 Clarify the MTLSMode field godoc on CR=empty vs CR="disabled". These ARE observably different in `kubectl get -o yaml` and now produce different effective behavior: - empty: falls through to namespace ConfigMap, then to default. - "disabled": pinned at the CR layer; namespace cannot override. The original resolver had a bug: both fell through to the namespace because the fallthrough check used `mtlsMode == MTLSModeDisabled` rather than "did the CR set the field." Fix the resolver to use a sentinel ("" = unset) — same shape as the AuthBridgeMode resolver above. Without this fix, the godoc would have been documenting unimplemented behavior. * #6 Add a comment on the unrecognized-mtlsMode WARN explaining the defense-in-depth intent (CRD enum check at the API server is the primary line; this is insurance against a future schema-validation regression and against non-CRD config sources). Prevents a future cleanup PR from dropping it as "redundant." #1 (auto-SPIRE log clarity) is already covered by the existing mutatorLog.Info("mtlsMode set; auto-enabling SPIRE for this workload", ...) — the why is in the message itself. No change. #4 (stale-mtls-block scrub end-to-end test) is already covered by TestEnsurePerAgentConfigMap_MTLSScrubsStaleBlock — that test exactly exercises "base YAML has stale mtls: strict, render with MTLSModeDisabled, assert block gone." Reply on the PR pointing the reviewer at it. #7 was an observation, no change. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../api/v1alpha1/agentruntime_types.go | 7 ++++ .../controller/agentruntime_config.go | 9 +++++ .../controller/agentruntime_config_test.go | 40 +++++++++++++++++++ .../internal/webhook/injector/pod_mutator.go | 21 ++++++++-- 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/kagenti-operator/api/v1alpha1/agentruntime_types.go b/kagenti-operator/api/v1alpha1/agentruntime_types.go index 03723ed7..7aa1d864 100644 --- a/kagenti-operator/api/v1alpha1/agentruntime_types.go +++ b/kagenti-operator/api/v1alpha1/agentruntime_types.go @@ -112,6 +112,13 @@ type AgentRuntimeSpec struct { // mtls.mode > "disabled". Setting mtlsMode != disabled implicitly // requires SPIRE — the operator auto-enables spire for the workload. // + // CR-empty vs CR="disabled" are observably different in + // `kubectl get agentruntime -o yaml` (the former omits the field, + // the latter shows mtlsMode: disabled) but produce the same + // effective mode: empty falls through to the namespace ConfigMap, + // "disabled" is an explicit override that pins mode off even when + // the namespace default is non-disabled. + // // Note: changing mtlsMode triggers a pod rollout because authbridge // cannot hot-reload mTLS config (the byte-peek listener is wired at // process start). diff --git a/kagenti-operator/internal/controller/agentruntime_config.go b/kagenti-operator/internal/controller/agentruntime_config.go index 7948c3dc..b1b4ae4c 100644 --- a/kagenti-operator/internal/controller/agentruntime_config.go +++ b/kagenti-operator/internal/controller/agentruntime_config.go @@ -79,6 +79,15 @@ type resolvedConfig struct { // pipelines/listener/mtls config drift through here in any shape and // we want any byte change to roll the workload. Empty string when // the ConfigMap doesn't exist in the namespace. + // + // Operational note: a single edit to authbridge-runtime-config + // re-hashes every AgentRuntime in the namespace and reconciles them + // in a burst. Kubernetes sequences the actual pod rolls per + // Deployment, but the controller's reconcile load scales linearly + // with the number of AgentRuntimes. For typical small namespaces + // (single-digit agents) this is fine; in larger deployments, + // formatting / whitespace edits to this CM during peak hours will + // trigger a noticeable rollout fan-out. AuthBridgeRuntime string `json:"authBridgeRuntime,omitempty"` } diff --git a/kagenti-operator/internal/controller/agentruntime_config_test.go b/kagenti-operator/internal/controller/agentruntime_config_test.go index 764a4110..2e0a11bf 100644 --- a/kagenti-operator/internal/controller/agentruntime_config_test.go +++ b/kagenti-operator/internal/controller/agentruntime_config_test.go @@ -258,6 +258,46 @@ var _ = Describe("AgentRuntime Config", func() { Expect(r1.Hash).NotTo(Equal(r2.Hash)) }) + It("should change when MTLSMode flips on the CR", func() { + // CR-side parallel to the CM-edit test below: spec.mtlsMode + // must feed the hash so flipping disabled→strict on a CR + // rolls the workload. Without an explicit assertion a + // future refactor that drops MTLSMode from resolvedConfig + // would silently regress rollout-on-CR-edit. + specOff := &agentv1alpha1.AgentRuntimeSpec{ + Type: agentv1alpha1.RuntimeTypeAgent, + TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-mtls-cr"}, + MTLSMode: "disabled", + } + specOn := &agentv1alpha1.AgentRuntimeSpec{ + Type: agentv1alpha1.RuntimeTypeAgent, + TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-mtls-cr"}, + MTLSMode: "strict", + } + + r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, specOff) + r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, specOn) + Expect(r1.Hash).NotTo(Equal(r2.Hash)) + }) + + It("should change when AuthBridgeMode flips on the CR", func() { + // Bonus: AuthBridgeMode rollouts had a pre-existing gap + // (not in resolvedConfig) that this PR closed. Lock it. + specA := &agentv1alpha1.AgentRuntimeSpec{ + Type: agentv1alpha1.RuntimeTypeAgent, + TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-abm-cr"}, + AuthBridgeMode: "proxy-sidecar", + } + specB := &agentv1alpha1.AgentRuntimeSpec{ + Type: agentv1alpha1.RuntimeTypeAgent, + TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-abm-cr"}, + AuthBridgeMode: "lite", + } + r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, specA) + r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, specB) + Expect(r1.Hash).NotTo(Equal(r2.Hash)) + }) + It("should change when authbridge-runtime-config edits", func() { // Edits to the namespace authbridge-runtime-config (which the // admission webhook reads at pod creation) must roll diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator.go b/kagenti-operator/internal/webhook/injector/pod_mutator.go index 2d71fe80..c957c1ef 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator.go @@ -238,18 +238,33 @@ func (m *PodMutator) InjectAuthBridge(ctx context.Context, podSpec *corev1.PodSp // ServiceAccount, container env). Mode-compat validation // (mtlsMode incompatible with envoy-sidecar) runs in the // AgentRuntime validating webhook upstream of pod admission. - mtlsMode := MTLSModeDisabled - mtlsSource := "default" + // Resolution chain: CR > namespace > "disabled". An explicit + // CR value (including "disabled") pins; the namespace fallback + // only fires when the CR doesn't set the field at all. + // arOverrides.MTLSMode is the sentinel — extractOverrides only + // populates it when Spec.MTLSMode is non-empty. + mtlsMode := "" + mtlsSource := "" if arOverrides != nil && arOverrides.MTLSMode != nil { mtlsMode = *arOverrides.MTLSMode mtlsSource = "agentruntime-cr" } - if mtlsMode == MTLSModeDisabled { + if mtlsMode == "" { if m := ExtractMTLSMode(nsConfig.AuthBridgeRuntimeYAML); m != "" { mtlsMode = m mtlsSource = "namespace-configmap" } } + if mtlsMode == "" { + mtlsMode = MTLSModeDisabled + mtlsSource = "default" + } + // Defense in depth: the CRD enum check rejects unknown values at + // the API server, but the namespace ConfigMap and any future + // non-CRD source feed in raw strings. A typo (e.g. "strikt") would + // otherwise flow through unchecked. Same defensive pattern as the + // authBridgeMode resolution above; do not drop this switch as + // "redundant with CRD validation" — it covers paths the CRD doesn't. switch mtlsMode { case MTLSModeDisabled, MTLSModePermissive, MTLSModeStrict: // recognized, keep as-is