feat(webhook): Wire envoy-sidecar mTLS through pod mutator + fix svid-output mount#381
Conversation
72c9f78 to
64a9d29
Compare
pdettori
left a comment
There was a problem hiding this comment.
Clean PR — correct Envoy TLS wiring (matches Istio PERMISSIVE/STRICT semantics), well-justified ReadOnly bug fix, and good template-level test coverage. All 16 CI checks pass including E2E.
Two minor suggestions inline, nothing blocking.
| OutboundPort: cfg.Platform.Proxy.Port, | ||
| InboundPort: cfg.Platform.Proxy.InboundProxyPort, | ||
| ExtProcPort: defaultExtProcPort, | ||
| MTLSEnabled: cfg.MTLSMode != "" && cfg.MTLSMode != MTLSModeDisabled, |
There was a problem hiding this comment.
nit: The double-check cfg.MTLSMode != "" && cfg.MTLSMode != MTLSModeDisabled is defensive — worth a one-line comment noting that "" is treated as disabled (since ResolvedConfig defaults it to "" when unset). Prevents a future reader from wondering why both conditions are needed.
| } | ||
| } | ||
| return result | ||
| } |
There was a problem hiding this comment.
suggestion: overrideEnvoyConfigMapInVolumes has no unit test. A small table-driven test (volume found → CM name swapped, not found → list unchanged) would lock in correctness cheaply.
| // + mtls we want template-rendered output, so we deliberately | ||
| // clear EnvoyYAML before rendering. Setting it on the namespace | ||
| // CM was a way to bypass templating; with mtls it's incompatible. | ||
| resolvedForEnvoy := ResolveConfig(currentConfig, nsConfig, arOverrides) |
There was a problem hiding this comment.
nit: The 6-line comment block (lines 595-600) is helpful context but dense — consider condensing to ~2 lines, e.g. "ResolveConfig is cheap/idempotent; we clear EnvoyYAML so RenderEnvoyConfig uses the template path instead of short-circuiting."
…-output mount Adds end-to-end support for Spec.MTLSMode on AgentRuntime CRs targeting authBridgeMode=envoy-sidecar. The kagenti-extensions PR rossoctl#440 demo proved the Envoy data-plane TLS design (tls_inspector + filter_chain_match for inbound, UpstreamTlsContext on a TLS-originating cluster for strict outbound). This wires that into the operator: per-agent envoy-config ConfigMaps render those TLS blocks based on the resolved mtlsMode, and the admission webhook stops rejecting envoy-sidecar + non-disabled mtls. Wiring (the bulk of the diff): * envoy.yaml.tmpl gains conditional TLS blocks gated on MTLSEnabled (permissive: tls_inspector + TLS chain + raw_buffer chain; strict: tls_inspector + TLS chain only) plus a strict-only original_destination_tls cluster with UpstreamTlsContext. SVID files at /opt/svid*.pem. * envoyTemplateData carries MTLSEnabled bool + MTLSMode string; RenderEnvoyConfig populates them from ResolvedConfig. * ResolvedConfig gains MTLSMode + AuthBridgeMode fields, populated via the same CR > namespace ConfigMap > "" chain pod_mutator already implements (so RenderEnvoyConfig reads the resolved value directly). * pod_mutator's envoy-sidecar branch now passes the real mtlsMode to ensurePerAgentConfigMap (was hardcoded "" defensively under the old webhook gate) and, when mtls is on, calls a new ensurePerAgentEnvoyConfigMap that SSAs an envoy-config-<crName> ConfigMap with the rendered envoy.yaml. overrideEnvoyConfigMapInVolumes in volume_builder.go redirects the envoy-config volume to the per-agent CM (mirroring the existing authbridge-runtime override). * checkMTLSCompatibleWithMode no longer rejects envoy-sidecar + non-disabled mtls. The function stays as a hook for future incompatible combinations. Bug fix bundled in: * container_builder.go's BuildEnvoyProxyContainerWithSpireOption was mounting svid-output (/opt) read-only on the envoy-proxy container while BuildProxySidecarContainerWithPorts correctly mounted it RW. The in-process spiffe Provider mirror inside the authbridge-envoy combined image needs to write /opt/svid.pem, /opt/svid_key.pem, /opt/svid_bundle.pem on every SPIRE rotation; under the RO mount it logged "atomicWrite: read-only file system" and Envoy refused to boot with "Invalid path: /opt/svid_bundle.pem". container_builder_test was asserting the bug as the desired behavior; flipped to require RW. Tests: * envoy_template_test.go: three new cases lock in the rendered shape per mode (disabled = no TLS bits; permissive = both chains, no UpstreamTls; strict = TLS-only chain + UpstreamTlsContext + TLS cluster). * agentruntime_webhook_test.go: envoy-sidecar + permissive/strict flipped from rejected to allowed. * container_builder_test.go: assertion flipped (svid-output must be RW). E2E spot-check: operator built and loaded into local kind, AgentRuntime CR with authBridgeMode=envoy-sidecar + mtlsMode=strict applied, the operator created envoy-config-<name> with tls_inspector + DownstreamTlsContext + UpstreamTlsContext + original_destination_tls cluster, and authbridge-config-<name> propagated mtls.mode: strict through the resolution chain. (Pod-level mTLS verification needs SPIRE agent + CSI driver; the local cluster's spire-system namespace had been deleted out-of-band, so end-to-end traffic test is deferred — the kagenti-extensions PR rossoctl#440 demo already verified the same Envoy YAML design works against a SPIRE-enabled cluster.) Companion to kagenti-extensions PR rossoctl#440. Once this lands, that demo's hand-crafted manifests can be replaced (or supplemented) with an operator-injected variant that simply sets Spec.MTLSMode on the AgentRuntime CR. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
64a9d29 to
7e448f8
Compare
Summary
Spec.MTLSModeonAgentRuntimeCRs through to a per-agent rendered Envoy data-plane TLS configuration whenSpec.AuthBridgeMode: envoy-sidecar. Closes the operator-side gap that the kagenti-extensions PR Bug: SharedTrust and MLflow controllers block all reconciliation when RBAC is missing #440 demo had to work around with hand-crafted manifests./optmount on envoy-sidecar's authbridge container was setreadOnly: true(asymmetric with proxy-sidecar's RW), blocking the in-process spiffe Provider mirror from writing/opt/svid*.pemand Envoy from booting.Design
Mode semantics match what the kagenti-extensions PR #440 demo proved at the data-plane level:
mtlsModedisabled(default)permissivetls_inspector+ two filter chains: TLS chain terminates mTLS, raw_buffer chain accepts plaintextstricttls_inspector+ single TLS chain; plaintext rejected at chain matchUpstreamTlsContextonoriginal_destination_tlscluster: TLS-or-failInbound is byte-identical to proxy-sidecar's
tlssniff.Listenersemantics, expressed as Envoy filter chains — matches Istio's PERMISSIVE/STRICT inbound exactly.Outbound is Istio-shaped: no per-connection try-then-fallback (Envoy has no native primitive for it, and Istio itself doesn't do it). Blanket TLS-or-fail in strict mode is practical for the same reason proxy-sidecar's strict mode is: outbound calls that need plaintext (Keycloak / JWKS / external HTTPS) bypass the listener (plugin-side via Go
net/http, external HTTPS via iptables exclusion).Wiring
envoy.yaml.tmpl— adds conditional TLS blocks gated onMTLSEnabled(permissive: tls_inspector + TLS chain + raw_buffer chain; strict: tls_inspector + TLS chain only) plus a strict-onlyoriginal_destination_tlscluster withUpstreamTlsContext. SVID files at/opt/svid*.pem.envoy_template.go—envoyTemplateDatacarriesMTLSEnabled bool+MTLSMode string;RenderEnvoyConfigpopulates fromResolvedConfig.MTLSMode.resolved_config.go— addsMTLSMode+AuthBridgeModefields, populated via the same CR > namespace ConfigMap > "" chainpod_mutatoralready implements (ExtractMode/ExtractMTLSModehelpers).pod_mutator.go— envoy-sidecar branch now passes the resolvedmtlsModetoensurePerAgentConfigMap(was hardcoded""under the old webhook gate). When mtls is non-disabled, calls a newensurePerAgentEnvoyConfigMapthat SSAs anenvoy-config-<crName>ConfigMap with the renderedenvoy.yaml. SSA + OwnerReference machinery mirrors the existingensurePerAgentConfigMap.volume_builder.go— addsoverrideEnvoyConfigMapInVolumes(mirroring the existingoverrideAuthBridgeConfigMapInVolumes) so the envoy-config volume reference points at the per-agent CM.agentruntime_webhook.go—checkMTLSCompatibleWithModeno longer rejects envoy-sidecar + non-disabled mtls. The function stays as a hook for future incompatible combinations.Bug fix
container_builder.go:117— removedReadOnly: trueon thesvid-outputvolumeMount inBuildEnvoyProxyContainerWithSpireOption. Asymmetric withBuildProxySidecarContainerWithPorts(correctly RW). The in-process spiffe Provider mirror inside the authbridge-envoy combined image needs to write SVID files there; under the RO mount it loggedatomicWrite: ... read-only file systemand Envoy refused to boot withInvalid path: /opt/svid_bundle.pem. Historical context: the RO was a copy-paste from the days of an external spiffe-helper writer container that no longer exists.container_builder_test.go— flipped the wrong test assertion. The test currently checksif !vm.ReadOnly { t.Error("svid-output mount should be read-only") }— i.e., it was asserting the bug as the desired behavior. Replaced with the opposite check.Tests
envoy_template_test.go— three new cases lock in the rendered shape per mode (disabled = no TLS bits; permissive = both chains, no UpstreamTls; strict = TLS-only chain + UpstreamTlsContext + TLS cluster).agentruntime_webhook_test.go—envoy-sidecar + permissiveandenvoy-sidecar + strictcases flipped fromwantErr: truetowantErr: false. Drops thestrings.Contains("envoy-sidecar mTLS is tracked as a follow-up")check that's no longer applicable.container_builder_test.go— assertion flipped (svid-output mount must be RW).Test plan
go test -race -count=1 ./internal/webhook/...passes (config + injector + v1alpha1).go test -race -count=1 ./...passes everything excepttest/e2ewhich wants a kind cluster named "kind" and is infra-dependent.go build ./...clean.AgentRuntimeCR withSpec.AuthBridgeMode: envoy-sidecar+Spec.MTLSMode: strictplus its target Deployment. Verified:envoy-config-<name>withtls_inspector+DownstreamTlsContext+UpstreamTlsContext+original_destination_tlscluster (strict-mode shape).authbridge-config-<name>withmtls.mode: strictpropagated through the resolution chain./opt/svid.pem,/opt/svid_key.pem,/opt/svid_bundle.pem— confirming the bug 1 fix is what unblocks Envoy.starting workerscleanly, noInvalid path: /opt/svid_bundle.pemfailures.Follow-up (separate, small PRs, not blocking):
authbridge/demos/mtls/scripts/patch-mtls-config.shto patch the namespaceauthbridge-runtime-configCM instead of the per-agent CM (operator-owned). Fixes the existingmake demo-mtls(proxy-sidecar) verification step.envoy-sidecar + non-disabled mtlsrejection inbackend/app/routers/agents.pyand remove the matching UI lock inImportAgentPage.tsx.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com