Merge pull request #367 from huang195/fix/spire-agent-socket-mount

fix(injector): Mount spire-agent-socket into authbridge containers

Author: huang195

kagenti-operator/internal/webhook/injector/container_builder.go

@@ -18,6 +18,7 @@ package injector
 
 import (
 	"fmt"
+	"path"
 	"strconv"
 	"strings"
 
@@ -123,6 +124,13 @@ func (b *ContainerBuilder) BuildEnvoyProxyContainerWithSpireOption(spireEnabled
 				MountPath: "/etc/spiffe-helper",
 				ReadOnly:  true,
 			},
+			// SPIRE workload-API socket — bundled spiffe-helper dials it.
+			// Path derived from SpiffeConfig.SocketPath (defaults.go).
+			corev1.VolumeMount{
+				Name:      "spire-agent-socket",
+				MountPath: spireSocketDir(b.cfg.Spiffe.SocketPath),
+				ReadOnly:  true,
+			},
 		)
 	}
 
@@ -187,6 +195,19 @@ func spireEnabledStr(b bool) string {
 	return "false"
 }
 
+// spireSocketDir returns the directory portion of the SPIRE workload-API
+// socket path (e.g. "unix:///spiffe-workload-api/spire-agent.sock" →
+// "/spiffe-workload-api"), suitable for use as a container mountPath.
+// Single source of truth: defaults.go's SpiffeConfig.SocketPath.
+func spireSocketDir(socketPath string) string {
+	stripped := strings.TrimPrefix(socketPath, "unix://")
+	dir := path.Dir(stripped)
+	if dir == "." || dir == "/" {
+		return ""
+	}
+	return dir
+}
+
 // BuildProxySidecarContainer creates a combined authbridge container for proxy-sidecar mode.
 // Uses the authbridge image (authbridge-proxy + spiffe-helper bundled, no Envoy).
 // The app uses HTTP_PROXY env vars to route outbound traffic through the forward proxy.
@@ -235,6 +256,13 @@ func (b *ContainerBuilder) BuildProxySidecarContainerWithPorts(spireEnabled bool
 				MountPath: "/etc/spiffe-helper",
 				ReadOnly:  true,
 			},
+			// SPIRE workload-API socket — bundled spiffe-helper dials it.
+			// Path derived from SpiffeConfig.SocketPath (defaults.go).
+			corev1.VolumeMount{
+				Name:      "spire-agent-socket",
+				MountPath: spireSocketDir(b.cfg.Spiffe.SocketPath),
+				ReadOnly:  true,
+			},
 		)
 	}
 

kagenti-operator/internal/webhook/injector/container_builder_test.go

@@ -385,3 +385,101 @@ func TestBuildProxySidecarContainer_SpireEnabled(t *testing.T) {
 		t.Error("svid-output volume mount should be present when SPIRE is enabled")
 	}
 }
+
+// TestBuildEnvoyProxyContainer_SpireEnabled_HasSocketMount asserts that
+// the SPIRE workload-API socket volume is mounted into the envoy-proxy
+// container when SPIRE is on. The bundled spiffe-helper inside the
+// combined image dials this socket; without the mount it sits in a
+// silent dial-loop and never writes /opt/svid*.pem.
+func TestBuildEnvoyProxyContainer_SpireEnabled_HasSocketMount(t *testing.T) {
+	cfg := config.CompiledDefaults()
+	builder := NewContainerBuilder(cfg)
+	container := builder.BuildEnvoyProxyContainerWithSpireOption(true)
+
+	// Derive the expected mount path from SpiffeConfig.SocketPath the
+	// same way the production code does, so a future change to the
+	// canonical SocketPath in defaults.go can't leave this test
+	// asserting against a stale literal.
+	wantPath := spireSocketDir(cfg.Spiffe.SocketPath)
+	if wantPath == "" {
+		t.Fatalf("spireSocketDir(%q) returned empty — defaults must declare a valid socket path", cfg.Spiffe.SocketPath)
+	}
+
+	found := false
+	for _, vm := range container.VolumeMounts {
+		if vm.Name == "spire-agent-socket" {
+			found = true
+			if vm.MountPath != wantPath {
+				t.Errorf("spire-agent-socket mount path = %q, want %q (derived from SpiffeConfig.SocketPath %q)",
+					vm.MountPath, wantPath, cfg.Spiffe.SocketPath)
+			}
+			if !vm.ReadOnly {
+				t.Error("spire-agent-socket mount should be read-only (CSI volume itself is read-only)")
+			}
+			break
+		}
+	}
+	if !found {
+		t.Error("envoy-proxy container missing spire-agent-socket mount when SPIRE is enabled — bundled spiffe-helper can't reach the workload API")
+	}
+}
+
+// TestBuildEnvoyProxyContainer_SpireDisabled_NoSocketMount: with SPIRE
+// off the socket mount must be absent — there's no spiffe-helper to
+// dial the socket, and mounting it would still try to schedule the
+// CSI volume.
+func TestBuildEnvoyProxyContainer_SpireDisabled_NoSocketMount(t *testing.T) {
+	builder := NewContainerBuilder(config.CompiledDefaults())
+	container := builder.BuildEnvoyProxyContainerWithSpireOption(false)
+
+	for _, vm := range container.VolumeMounts {
+		if vm.Name == "spire-agent-socket" {
+			t.Error("envoy-proxy container should NOT have spire-agent-socket mount when SPIRE is disabled")
+		}
+	}
+}
+
+// TestBuildProxySidecarContainer_SpireEnabled_HasSocketMount: same as
+// the envoy-proxy variant but for the proxy-sidecar combined image.
+// The bundled spiffe-helper has the same workload-API requirement.
+func TestBuildProxySidecarContainer_SpireEnabled_HasSocketMount(t *testing.T) {
+	cfg := config.CompiledDefaults()
+	builder := NewContainerBuilder(cfg)
+	container := builder.BuildProxySidecarContainer(true)
+
+	wantPath := spireSocketDir(cfg.Spiffe.SocketPath)
+	if wantPath == "" {
+		t.Fatalf("spireSocketDir(%q) returned empty — defaults must declare a valid socket path", cfg.Spiffe.SocketPath)
+	}
+
+	found := false
+	for _, vm := range container.VolumeMounts {
+		if vm.Name == "spire-agent-socket" {
+			found = true
+			if vm.MountPath != wantPath {
+				t.Errorf("spire-agent-socket mount path = %q, want %q (derived from SpiffeConfig.SocketPath %q)",
+					vm.MountPath, wantPath, cfg.Spiffe.SocketPath)
+			}
+			if !vm.ReadOnly {
+				t.Error("spire-agent-socket mount should be read-only (CSI volume itself is read-only)")
+			}
+			break
+		}
+	}
+	if !found {
+		t.Error("proxy-sidecar container missing spire-agent-socket mount when SPIRE is enabled — bundled spiffe-helper can't reach the workload API")
+	}
+}
+
+// TestBuildProxySidecarContainer_SpireDisabled_NoSocketMount mirrors
+// the envoy-proxy negative test for the proxy-sidecar variant.
+func TestBuildProxySidecarContainer_SpireDisabled_NoSocketMount(t *testing.T) {
+	builder := NewContainerBuilder(config.CompiledDefaults())
+	container := builder.BuildProxySidecarContainer(false)
+
+	for _, vm := range container.VolumeMounts {
+		if vm.Name == "spire-agent-socket" {
+			t.Error("proxy-sidecar container should NOT have spire-agent-socket mount when SPIRE is disabled")
+		}
+	}
+}