From f3544ddeb0835424a69b4eabe33acc63141c770a Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 7 May 2026 10:25:25 -0400 Subject: [PATCH] feat(webhook): Emit per-plugin authbridge config schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to kagenti-extensions PR #378, which replaced authbridge's top-level `inbound:` / `outbound:` / `identity:` / `bypass:` / `routes:` YAML blocks with per-plugin config under `pipeline.*.plugins[].config`. The new binary rejects any config whose pipeline is empty (which is what yaml.v3's silent drop of unknown keys produces when the top-level blocks are removed), so the webhook had to stop synthesizing the old shape. What changed in pod_mutator.go: ensurePerAgentConfigMap's fallback-synthesis branch is rewritten. Previously: if baseYAML was missing any of `inbound`, `outbound`, `identity`, or `bypass`, synthesize each from NamespaceConfig values. Now: if baseYAML has no `pipeline:` section at all, synthesize one with jwt-validation (inbound) and token-exchange (outbound) entries whose configs carry Issuer / Keycloak URL / realm / default policy / identity type from NamespaceConfig. File-path defaults (audience_file, client_id_file, client_secret_file, jwt_svid_path) are no longer emitted by the webhook — the authbridge plugin applies them from its own convention layer at Configure time (see authbridge/authlib/plugins/CONVENTIONS.md). This keeps the webhook schema-agnostic about paths and reduces duplication. When baseYAML already has a `pipeline:` section (the post-migration Kagenti Helm chart emits it), synthesis is skipped entirely. Only mode + listener overrides layer on top. The Helm chart owns plugin config contents; the webhook owns per-agent mode selection. Tests: - TestEnsurePerAgentConfigMap_EmptyBaseYAML_FallbackFromNsConfig: asserts the synthesized pipeline has the correct plugin names and config values. Navigates pipeline..plugins[] via a new pluginConfigAt helper to keep assertions compact. - TestEnsurePerAgentConfigMap_BaseYAML_PreservesExistingFields: baseYAML rewritten to new shape; asserts plugin config is preserved verbatim. - TestEnsurePerAgentConfigMap_ListenerOverrides_Merged: baseYAML rewritten to new shape; listener assertions unchanged (listener is top-level in both schemas). - TestEnsurePerAgentConfigMap_FederatedJWT_MapsToSpiffe: asserts identity.type = spiffe under token-exchange. Dropped assertions for file-path defaults (those moved into the plugin). E2E fixtures: - test/e2e/fixtures.go: two hardcoded ConfigMaps with old-shape YAML rewritten to new per-plugin shape. Image tags: none bumped. The operator references authbridge images as `:latest` throughout — once PR #378 merges and CI publishes new `:latest`, operator CI picks it up. The version-pinning (and atomic authbridge+operator cutover) lives in the kagenti Helm chart PR that comes next. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../internal/webhook/injector/pod_mutator.go | 135 ++++++++++----- .../webhook/injector/pod_mutator_test.go | 160 ++++++++++++------ kagenti-operator/test/e2e/fixtures.go | 58 +++---- 3 files changed, 221 insertions(+), 132 deletions(-) diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator.go b/kagenti-operator/internal/webhook/injector/pod_mutator.go index 82acb681..590546e0 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator.go @@ -562,6 +562,75 @@ func perAgentConfigMapName(crName string) string { return "authbridge-config-" + crName } +// synthesizePipeline builds the per-plugin pipeline section that +// maps 1:1 to NamespaceConfig (env-var-style authbridge-config +// values). Used only when the namespace's authbridge-runtime-config +// ConfigMap has no `pipeline:` of its own — typically a demo or +// operator-managed namespace that bypassed the Kagenti Helm chart. +// +// The synthesized shape matches what plugins expect: +// - jwt-validation.config.issuer from NamespaceConfig.Issuer +// (rest of the plugin's config comes from its defaults — +// audience_file=/shared/client-id.txt, bypass_paths=standard +// probes, jwks_url derived from issuer). +// - token-exchange.config with Keycloak URL/realm, default_policy, +// and identity block keyed off ClientAuthType. File paths fall +// through to plugin defaults so operators don't have to +// boilerplate them. +// +// Empty NamespaceConfig fields are not emitted — the plugin's own +// defaults apply. That matches the minimum-viable config shown in +// authbridge/cmd/authbridge/README.md. +func synthesizePipeline(nsConfig *NamespaceConfig) map[string]interface{} { + jwtCfg := map[string]interface{}{} + if nsConfig.Issuer != "" { + jwtCfg["issuer"] = nsConfig.Issuer + } + + tokenCfg := map[string]interface{}{} + if nsConfig.KeycloakURL != "" { + tokenCfg["keycloak_url"] = nsConfig.KeycloakURL + } + if nsConfig.KeycloakRealm != "" { + tokenCfg["keycloak_realm"] = nsConfig.KeycloakRealm + } + if nsConfig.DefaultOutboundPolicy != "" { + tokenCfg["default_policy"] = nsConfig.DefaultOutboundPolicy + } + // Identity: only set type (file paths default per-plugin). Spiffe + // mode carries over the jwt_svid_path explicitly because that + // default lives in the plugin only when the operator actually + // selected spiffe — the Helm chart reads ClientAuthType to pick. + if nsConfig.ClientAuthType != "" { + identity := map[string]interface{}{} + if nsConfig.ClientAuthType == ClientAuthTypeFederatedJWT { + identity["type"] = IdentityTypeSpiffe + } else { + identity["type"] = nsConfig.ClientAuthType + } + tokenCfg["identity"] = identity + } + + return map[string]interface{}{ + "inbound": map[string]interface{}{ + "plugins": []interface{}{ + map[string]interface{}{ + "name": "jwt-validation", + "config": jwtCfg, + }, + }, + }, + "outbound": map[string]interface{}{ + "plugins": []interface{}{ + map[string]interface{}{ + "name": "token-exchange", + "config": tokenCfg, + }, + }, + }, + } +} + // 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. @@ -587,53 +656,25 @@ func (m *PodMutator) ensurePerAgentConfigMap( } } - // If the base was empty or missing key fields, populate from NamespaceConfig - if cfg["inbound"] == nil && nsConfig != nil { - inbound := map[string]interface{}{} - if nsConfig.Issuer != "" { - inbound["issuer"] = nsConfig.Issuer - } - if len(inbound) > 0 { - cfg["inbound"] = inbound - } - } - if cfg["outbound"] == nil && nsConfig != nil { - outbound := map[string]interface{}{} - if nsConfig.KeycloakURL != "" { - outbound["keycloak_url"] = nsConfig.KeycloakURL - } - if nsConfig.KeycloakRealm != "" { - outbound["keycloak_realm"] = nsConfig.KeycloakRealm - } - if nsConfig.DefaultOutboundPolicy != "" { - outbound["default_policy"] = nsConfig.DefaultOutboundPolicy - } - if len(outbound) > 0 { - cfg["outbound"] = outbound - } - } - if cfg["identity"] == nil && nsConfig != nil && nsConfig.ClientAuthType != "" { - identity := map[string]interface{}{ - "client_id_file": "/shared/client-id.txt", - "client_secret_file": "/shared/client-secret.txt", - } - if nsConfig.ClientAuthType == ClientAuthTypeFederatedJWT { - identity["type"] = IdentityTypeSpiffe - identity["jwt_svid_path"] = "/opt/jwt_svid.token" - } else { - identity["type"] = nsConfig.ClientAuthType - } - cfg["identity"] = identity - } - if cfg["bypass"] == nil { - cfg["bypass"] = map[string]interface{}{ - "inbound_paths": []string{ - "/.well-known/*", - "/healthz", - "/readyz", - "/livez", - }, - } + // If the base YAML has no `pipeline:` section, synthesize one + // from NamespaceConfig. Happens in two cases: + // + // 1. baseYAML was empty (namespace has no authbridge-runtime- + // config ConfigMap at all). + // 2. baseYAML was present but stale pre-migration shape — the + // parse succeeded but yielded top-level `inbound:` / + // `outbound:` / etc., which the authbridge binary now + // rejects at Validate time. Any top-level key the parser + // found is left alone and ignored; the synthesized + // `pipeline:` is what authbridge actually reads. + // + // When the base YAML already has `pipeline:` (Kagenti Helm chart + // emits it), this branch is skipped and we only layer mode + + // listener overrides on top — the chart owns the plugin config + // contents. See authbridge/authlib/plugins/CONVENTIONS.md for + // the per-plugin config schema. + if cfg["pipeline"] == nil && nsConfig != nil { + cfg["pipeline"] = synthesizePipeline(nsConfig) } // Override mode diff --git a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go index 864abc59..30e46bb2 100644 --- a/kagenti-operator/internal/webhook/injector/pod_mutator_test.go +++ b/kagenti-operator/internal/webhook/injector/pod_mutator_test.go @@ -1088,23 +1088,30 @@ func TestEnsurePerAgentConfigMap_EmptyBaseYAML_FallbackFromNsConfig(t *testing.T t.Errorf("mode = %v, want %s", cfg["mode"], ModeProxySidecar) } - inbound, _ := cfg["inbound"].(map[string]interface{}) - if inbound == nil || inbound["issuer"] != "http://keycloak:8080/realms/kagenti" { - t.Errorf("inbound.issuer = %v, want http://keycloak:8080/realms/kagenti", inbound) + // Synthesized pipeline: jwt-validation inbound, token-exchange + // outbound. Plugin-level defaults (audience_file, bypass_paths, + // identity file paths) are not emitted by the webhook — the + // authbridge binary applies them from its own convention layer + // when it reads this config. See + // authbridge/authlib/plugins/CONVENTIONS.md. + jwtCfg := pluginConfigAt(t, cfg, "inbound", "jwt-validation") + if got, want := jwtCfg["issuer"], "http://keycloak:8080/realms/kagenti"; got != want { + t.Errorf("jwt-validation.config.issuer = %v, want %v", got, want) } - outbound, _ := cfg["outbound"].(map[string]interface{}) - if outbound == nil || outbound["keycloak_url"] != "http://keycloak:8080" { - t.Errorf("outbound.keycloak_url = %v, want http://keycloak:8080", outbound) + tokCfg := pluginConfigAt(t, cfg, "outbound", "token-exchange") + if got, want := tokCfg["keycloak_url"], "http://keycloak:8080"; got != want { + t.Errorf("token-exchange.config.keycloak_url = %v, want %v", got, want) } - - identity, _ := cfg["identity"].(map[string]interface{}) - if identity == nil || identity["type"] != "client-secret" { - t.Errorf("identity.type = %v, want client-secret", identity) + if got, want := tokCfg["keycloak_realm"], "kagenti"; got != want { + t.Errorf("token-exchange.config.keycloak_realm = %v, want %v", got, want) } - - if cfg["bypass"] == nil { - t.Error("expected default bypass paths") + if got, want := tokCfg["default_policy"], "passthrough"; got != want { + t.Errorf("token-exchange.config.default_policy = %v, want %v", got, want) + } + identity, _ := tokCfg["identity"].(map[string]interface{}) + if identity == nil || identity["type"] != "client-secret" { + t.Errorf("token-exchange.config.identity.type = %v, want client-secret", identity) } // managedBy label @@ -1113,25 +1120,63 @@ func TestEnsurePerAgentConfigMap_EmptyBaseYAML_FallbackFromNsConfig(t *testing.T } } +// pluginConfigAt navigates pipeline..plugins[].config +// and returns the config map. Fails the test if the path is missing +// or the shape is unexpected. Keeps assertions in tests compact. +func pluginConfigAt(t *testing.T, cfg map[string]interface{}, direction, pluginName string) map[string]interface{} { + t.Helper() + pipeline, ok := cfg["pipeline"].(map[string]interface{}) + if !ok { + t.Fatalf("expected pipeline section, got %v", cfg["pipeline"]) + } + dir, ok := pipeline[direction].(map[string]interface{}) + if !ok { + t.Fatalf("expected pipeline.%s section", direction) + } + plugins, ok := dir["plugins"].([]interface{}) + if !ok || len(plugins) == 0 { + t.Fatalf("expected pipeline.%s.plugins list, got %v", direction, dir["plugins"]) + } + for _, raw := range plugins { + entry, ok := raw.(map[string]interface{}) + if !ok { + continue + } + if entry["name"] == pluginName { + cfg, _ := entry["config"].(map[string]interface{}) + return cfg + } + } + t.Fatalf("plugin %q not found under pipeline.%s.plugins", pluginName, direction) + return nil +} + func TestEnsurePerAgentConfigMap_BaseYAML_PreservesExistingFields(t *testing.T) { m := newTestMutator() ctx := context.Background() + // baseYAML uses the per-plugin schema the Kagenti Helm chart + // emits post-migration. When pipeline: is already present, the + // webhook must not touch plugin config — only mode + listener + // overrides layer on top. baseYAML := ` mode: envoy-sidecar -inbound: - issuer: "http://custom-issuer" -outbound: - keycloak_url: "http://custom-keycloak:8080" - keycloak_realm: "custom-realm" -identity: - type: spiffe - jwt_svid_path: "/opt/jwt_svid.token" - client_id_file: "/shared/client-id.txt" - client_secret_file: "/shared/client-secret.txt" -bypass: - inbound_paths: - - "/custom-path" +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "http://custom-issuer" + bypass_paths: + - "/custom-path" + outbound: + plugins: + - name: token-exchange + config: + keycloak_url: "http://custom-keycloak:8080" + keycloak_realm: "custom-realm" + identity: + type: spiffe ` cmName, err := m.ensurePerAgentConfigMap(ctx, "team1", "my-agent", @@ -1148,21 +1193,20 @@ bypass: t.Errorf("mode = %v, want %s", cfg["mode"], ModeEnvoySidecar) } - // Existing fields preserved (not overwritten by fallback) - inbound, _ := cfg["inbound"].(map[string]interface{}) - if inbound["issuer"] != "http://custom-issuer" { - t.Errorf("inbound.issuer = %v, should be preserved from base YAML", inbound["issuer"]) + // Existing plugin config preserved (not overwritten by fallback) + jwtCfg := pluginConfigAt(t, cfg, "inbound", "jwt-validation") + if jwtCfg["issuer"] != "http://custom-issuer" { + t.Errorf("jwt-validation.config.issuer = %v, should be preserved from base YAML", jwtCfg["issuer"]) } - - identity, _ := cfg["identity"].(map[string]interface{}) - if identity["type"] != IdentityTypeSpiffe { - t.Errorf("identity.type = %v, should be preserved from base YAML", identity["type"]) + paths, _ := jwtCfg["bypass_paths"].([]interface{}) + if len(paths) != 1 || paths[0] != "/custom-path" { + t.Errorf("bypass_paths = %v, should be preserved from base YAML", paths) } - bypass, _ := cfg["bypass"].(map[string]interface{}) - paths, _ := bypass["inbound_paths"].([]interface{}) - if len(paths) != 1 || paths[0] != "/custom-path" { - t.Errorf("bypass paths = %v, should be preserved from base YAML", paths) + tokCfg := pluginConfigAt(t, cfg, "outbound", "token-exchange") + identity, _ := tokCfg["identity"].(map[string]interface{}) + if identity["type"] != IdentityTypeSpiffe { + t.Errorf("token-exchange.config.identity.type = %v, should be preserved from base YAML", identity["type"]) } } @@ -1172,15 +1216,20 @@ func TestEnsurePerAgentConfigMap_ListenerOverrides_Merged(t *testing.T) { baseYAML := ` mode: envoy-sidecar -inbound: - issuer: "http://issuer" -outbound: - keycloak_url: "http://keycloak:8080" - keycloak_realm: "kagenti" -identity: - type: client-secret - client_id_file: "/shared/client-id.txt" - client_secret_file: "/shared/client-secret.txt" +pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "http://issuer" + outbound: + plugins: + - name: token-exchange + config: + keycloak_url: "http://keycloak:8080" + keycloak_realm: "kagenti" + identity: + type: client-secret ` overrides := map[string]string{ @@ -1361,17 +1410,18 @@ func TestEnsurePerAgentConfigMap_FederatedJWT_MapsToSpiffe(t *testing.T) { cm := fetchConfigMap(t, m, "team1", cmName) cfg := parseConfigYAML(t, cm) - identity, _ := cfg["identity"].(map[string]interface{}) + tokCfg := pluginConfigAt(t, cfg, "outbound", "token-exchange") + identity, _ := tokCfg["identity"].(map[string]interface{}) if identity == nil { - t.Fatal("expected identity section") + t.Fatal("expected identity block under token-exchange config") } if identity["type"] != IdentityTypeSpiffe { t.Errorf("identity.type = %v, want spiffe (federated-jwt should map to spiffe)", identity["type"]) } - if identity["jwt_svid_path"] != "/opt/jwt_svid.token" { - t.Errorf("identity.jwt_svid_path = %v, want /opt/jwt_svid.token", identity["jwt_svid_path"]) - } - if identity["client_id_file"] != "/shared/client-id.txt" { - t.Errorf("identity.client_id_file = %v, want /shared/client-id.txt", identity["client_id_file"]) - } + // Note: the webhook no longer emits default credential file + // paths (client_id_file, client_secret_file, jwt_svid_path). + // The authbridge plugin applies those defaults itself from its + // own convention layer — keeping the webhook schema-agnostic + // about file paths. See + // authbridge/authlib/plugins/CONVENTIONS.md. } diff --git a/kagenti-operator/test/e2e/fixtures.go b/kagenti-operator/test/e2e/fixtures.go index 7cff4712..74cba999 100644 --- a/kagenti-operator/test/e2e/fixtures.go +++ b/kagenti-operator/test/e2e/fixtures.go @@ -908,21 +908,20 @@ metadata: data: config.yaml: | mode: envoy-sidecar - inbound: - issuer: "https://keycloak.example.com/realms/test" - outbound: - token_url: "https://keycloak.example.com/realms/test/protocol/openid-connect/token" - default_policy: "passthrough" - identity: - type: client-secret - client_id_file: "/shared/client-id.txt" - client_secret_file: "/shared/client-secret.txt" - bypass: - inbound_paths: - - "/.well-known/*" - - "/healthz" - - "/readyz" - - "/livez" + pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "https://keycloak.example.com/realms/test" + outbound: + plugins: + - name: token-exchange + config: + token_url: "https://keycloak.example.com/realms/test/protocol/openid-connect/token" + default_policy: "passthrough" + identity: + type: client-secret ` } @@ -1308,21 +1307,20 @@ metadata: data: config.yaml: | mode: envoy-sidecar - inbound: - issuer: "https://keycloak.example.com/realms/test" - outbound: - token_url: "https://keycloak.example.com/realms/test/protocol/openid-connect/token" - default_policy: "passthrough" - identity: - type: client-secret - client_id_file: "/shared/client-id.txt" - client_secret_file: "/shared/client-secret.txt" - bypass: - inbound_paths: - - "/.well-known/*" - - "/healthz" - - "/readyz" - - "/livez" + pipeline: + inbound: + plugins: + - name: jwt-validation + config: + issuer: "https://keycloak.example.com/realms/test" + outbound: + plugins: + - name: token-exchange + config: + token_url: "https://keycloak.example.com/realms/test/protocol/openid-connect/token" + default_policy: "passthrough" + identity: + type: client-secret ` }