Fix Envoy isolated-server readiness and enable e2e tests

.github/workflows/e2e-tests.yml

@@ -57,8 +57,12 @@ jobs:
             label_filter: mcp-protocol
             artifact: e2e-test-results-mcp-protocol
           - title: proxy
-            label_filter: proxy
+            label_filter: "proxy && !isolation"
             artifact: e2e-test-results-proxy
+          - title: network-isolation
+            label_filter: isolation
+            artifact: e2e-test-results-network-isolation
+            test_timeout: 25m
           - title: middleware
             label_filter: 'middleware || stability'
             artifact: e2e-test-results-middleware
@@ -130,10 +134,14 @@ jobs:
           if [ "${{ matrix.label_filter }}" = "vmcp" ]; then
             docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1 &
           fi
-          # the time server is only used by the proxy (streamable-http) test suite
-          if [ "${{ matrix.label_filter }}" = "proxy" ]; then
+          # the time server is only used by the proxy test suite
+          if [ "${{ matrix.label_filter }}" = "proxy && !isolation" ]; then
             docker pull ghcr.io/stacklok/dockyard/uvx/mcp-server-time:2026.1.26 &
           fi
+          # Envoy is only used by the network-isolation test suite
+          if [ "${{ matrix.title }}" = "network-isolation" ]; then
+            docker pull envoyproxy/envoy-distroless:v1.32.3 &
+          fi
           wait
           echo "Pre-pulled images:"
           docker images --format '{{.Repository}}:{{.Tag}}' | grep -E 'osv-mcp|gofetch|egress-proxy|yardstick|mcp-server-time'

pkg/container/docker/envoy.go

@@ -286,12 +286,16 @@ type envoyConnectConfig struct{}
 
 // envoyCluster is an Envoy upstream cluster definition.
 type envoyCluster struct {
-	Name           string               `json:"name"`
-	ConnectTimeout string               `json:"connect_timeout"`
-	LbPolicy       string               `json:"lb_policy,omitempty"`
-	Type           string               `json:"type,omitempty"`
-	ClusterType    *envoyClusterType    `json:"cluster_type,omitempty"`
-	LoadAssignment *envoyLoadAssignment `json:"load_assignment,omitempty"`
+	Name           string `json:"name"`
+	ConnectTimeout string `json:"connect_timeout"`
+	LbPolicy       string `json:"lb_policy,omitempty"`
+	Type           string `json:"type,omitempty"`
+	// DnsLookupFamily restricts DNS resolution to IPv4 only. Without this,
+	// STRICT_DNS clusters attempt AAAA lookups first, which adds latency in
+	// environments where IPv6 is unavailable or times out.
+	DnsLookupFamily string               `json:"dns_lookup_family,omitempty"`
+	ClusterType     *envoyClusterType    `json:"cluster_type,omitempty"`
+	LoadAssignment  *envoyLoadAssignment `json:"load_assignment,omitempty"`
 }
 
 // envoyClusterType is the custom cluster discovery extension (e.g. DFP).
@@ -670,23 +674,24 @@ func buildIngressListener(spec proxySpec, hostPort int) envoyListener {
 }
 
 // ingressDomains returns the virtual host domain list for the ingress listener.
-// When Inbound.AllowHost is configured those entries are used; otherwise a
-// wildcard ("*") is returned so all hostnames are accepted.
-func ingressDomains(spec proxySpec) []string {
-	if spec.Permissions != nil && spec.Permissions.Inbound != nil &&
-		len(spec.Permissions.Inbound.AllowHost) > 0 {
-		return spec.Permissions.Inbound.AllowHost
-	}
+// Always returns a wildcard: the Inbound.AllowHost list contains bare hostnames
+// (e.g. "localhost", "127.0.0.1") but the transparent proxy sends a Host header
+// with a port suffix ("127.0.0.1:22354"), which Envoy would not match against
+// a bare hostname. The inbound access restriction is instead enforced by the
+// host-side port binding to 127.0.0.1, which already limits the ingress to
+// local connections only.
+func ingressDomains(_ proxySpec) []string {
 	return []string{"*"}
 }
 
 // buildIngressCluster returns the STRICT_DNS upstream cluster for the ingress
 // listener, pointing at spec.WorkloadName:spec.UpstreamPort.
 func buildIngressCluster(spec proxySpec) envoyCluster {
 	return envoyCluster{
-		Name:           ingressClusterName,
-		ConnectTimeout: "10s",
-		Type:           "STRICT_DNS",
+		Name:            ingressClusterName,
+		ConnectTimeout:  "10s",
+		Type:            "STRICT_DNS",
+		DnsLookupFamily: "V4_ONLY",
 		LoadAssignment: &envoyLoadAssignment{
 			ClusterName: ingressClusterName,
 			Endpoints: []envoyEndpoint{
@@ -733,8 +738,12 @@ func writeEnvoyBootstrap(b envoyBootstrap) (string, error) {
 		_ = os.Remove(created)
 		return "", fmt.Errorf("failed to write envoy bootstrap: %w", err)
 	}
-	// 0600: only the owner can read — the file may contain network topology.
-	if err := tmpFile.Chmod(0o600); err != nil {
+	// 0o644: world-readable so the Envoy distroless container (UID 101) can
+	// read the bind-mounted file. On Linux Docker Engine, strict POSIX
+	// permissions apply — 0o600 prevents the container user from reading the
+	// file, causing Envoy to crash-loop. The bootstrap contains no secrets
+	// (only network topology: hostnames, ports, RBAC rules).
+	if err := tmpFile.Chmod(0o644); err != nil {
 		_ = os.Remove(created)
 		return "", fmt.Errorf("failed to set envoy bootstrap file permissions: %w", err)
 	}
@@ -752,13 +761,28 @@ type envoyProxy struct {
 	client *Client
 }
 
-// SetupEgress implements networkProxy for the Envoy backend. Envoy consolidates
-// egress and ingress into a single container, so this creates that container
-// (both listeners) before the MCP container. Envoy's ingress upstream is a
-// STRICT_DNS cluster that resolves the MCP container lazily and keeps retrying,
-// so — unlike squid's cache_peer — pre-MCP creation is safe. The reserved
-// ingress port is carried back in egressResult for SetupIngress to return.
-func (e *envoyProxy) SetupEgress(ctx context.Context, spec proxySpec) (egressResult, error) {
+// SetupEgress implements networkProxy for the Envoy backend.
+//
+// Only the egress proxy env vars are returned here — no container is created
+// yet. Container creation is deferred to SetupIngress (which runs after
+// createMcpContainer) so that the MCP container's hostname is resolvable the
+// moment the Envoy STRICT_DNS ingress cluster first probes it. Creating Envoy
+// before the MCP container caused the ingress cluster to cache a negative DNS
+// response on Linux Docker Engine, preventing the server from ever becoming
+// ready within the readiness window (see #5922).
+func (*envoyProxy) SetupEgress(_ context.Context, spec proxySpec) (egressResult, error) {
+	egressContainerName := fmt.Sprintf("%s-egress", spec.WorkloadName)
+	return egressResult{EnvVars: addEgressEnvVars(nil, egressContainerName)}, nil
+}
+
+// SetupIngress implements networkProxy for the Envoy backend.
+//
+// Creates the Envoy container after the MCP container exists, with both the
+// egress forward-proxy listener and (for non-stdio transports) the ingress
+// reverse-proxy listener. Running after createMcpContainer ensures the
+// STRICT_DNS upstream cluster resolves the MCP hostname on the first probe,
+// avoiding the Linux Docker Engine readiness failure described in #5922.
+func (e *envoyProxy) SetupIngress(ctx context.Context, spec proxySpec, _ egressResult) (int, error) {
 	egressContainerName := fmt.Sprintf("%s-egress", spec.WorkloadName)
 
 	bootstrap := envoyBootstrap{
@@ -780,7 +804,7 @@ func (e *envoyProxy) SetupEgress(ctx context.Context, spec proxySpec) (egressRes
 	if spec.TransportType != "stdio" && spec.UpstreamPort > 0 {
 		port, err := networking.FindOrUsePort(spec.UpstreamPort + 1)
 		if err != nil {
-			return egressResult{}, fmt.Errorf("failed to find ingress port: %w", err)
+			return 0, fmt.Errorf("failed to find ingress port: %w", err)
 		}
 		ingressPort = port
 		bootstrap.StaticResources.Listeners = append(
@@ -795,11 +819,8 @@ func (e *envoyProxy) SetupEgress(ctx context.Context, spec proxySpec) (egressRes
 
 	configPath, err := writeEnvoyBootstrap(bootstrap)
 	if err != nil {
-		return egressResult{}, err // already wrapped with context
+		return 0, err
 	}
-	// On success the file must persist for the container's read-only bind mount,
-	// so it can't be unconditionally deferred away. Remove it only if setup fails
-	// past this point, so a failed deploy doesn't orphan a bootstrap in TempDir.
 	success := false
 	defer func() {
 		if !success {
@@ -812,12 +833,9 @@ func (e *envoyProxy) SetupEgress(ctx context.Context, spec proxySpec) (egressRes
 	slog.Debug("setting up envoy container", "name", egressContainerName, "image", envoyImage)
 
 	if err := e.client.imageManager.PullImage(ctx, envoyImage); err != nil {
-		// Fall back to a locally-present image; only proceed if it actually
-		// exists (ImageExists returns (false, nil) when absent, so the bool
-		// must be checked, not just the error).
 		exists, inspectErr := e.client.imageManager.ImageExists(ctx, envoyImage)
 		if inspectErr != nil || !exists {
-			return egressResult{}, fmt.Errorf("failed to pull envoy image: %w", err)
+			return 0, fmt.Errorf("failed to pull envoy image: %w", err)
 		}
 		//nolint:gosec // G706: envoy image name from config
 		slog.Debug("envoy image exists locally, continuing despite pull failure", "image", envoyImage)
@@ -827,18 +845,14 @@ func (e *envoyProxy) SetupEgress(ctx context.Context, spec proxySpec) (egressRes
 	lb.AddStandardLabels(envoyLabels, egressContainerName, egressContainerName, "stdio", 80)
 	envoyLabels[ToolhiveAuxiliaryWorkloadLabel] = LabelValueTrue
 
-	config := &container.Config{
+	containerConfig := &container.Config{
 		Image:  envoyImage,
 		Cmd:    []string{"-c", "/etc/envoy/envoy.json"},
 		Labels: envoyLabels,
 	}
 
 	mounts := []runtime.Mount{
-		{
-			Source:   configPath,
-			Target:   "/etc/envoy/envoy.json",
-			ReadOnly: true,
-		},
+		{Source: configPath, Target: "/etc/envoy/envoy.json", ReadOnly: true},
 	}
 
 	var exposedPorts map[string]struct{}
@@ -856,36 +870,24 @@ func (e *envoyProxy) SetupEgress(ctx context.Context, spec proxySpec) (egressRes
 		Mounts:      convertMounts(mounts),
 		NetworkMode: container.NetworkMode("bridge"),
 		SecurityOpt: []string{"label:disable"},
-		// Envoy distroless runs as nonroot and needs no capabilities; drop all.
-		CapDrop: []string{"ALL"},
+		CapDrop:     []string{"ALL"},
 		RestartPolicy: container.RestartPolicy{
 			Name: "unless-stopped",
 		},
 	}
 	if portBindings != nil {
 		if err := setupPortBindings(hostConfig, portBindings); err != nil {
-			return egressResult{}, fmt.Errorf("failed to setup port bindings: %w", err)
+			return 0, fmt.Errorf("failed to setup port bindings: %w", err)
 		}
 	}
-	if err := setupExposedPorts(config, exposedPorts); err != nil {
-		return egressResult{}, fmt.Errorf("failed to setup exposed ports: %w", err)
+	if err := setupExposedPorts(containerConfig, exposedPorts); err != nil {
+		return 0, fmt.Errorf("failed to setup exposed ports: %w", err)
 	}
 
-	if _, err := e.client.createContainer(ctx, egressContainerName, config, hostConfig, spec.Endpoints); err != nil {
-		return egressResult{}, fmt.Errorf("failed to create envoy container: %w", err)
+	if _, err := e.client.createContainer(ctx, egressContainerName, containerConfig, hostConfig, spec.Endpoints); err != nil {
+		return 0, fmt.Errorf("failed to create envoy container: %w", err)
 	}
 
-	success = true // keep the bootstrap file; the container bind-mounts it
-	return egressResult{
-		EnvVars:     addEgressEnvVars(nil, egressContainerName),
-		ingressPort: ingressPort,
-	}, nil
-}
-
-// SetupIngress implements networkProxy for the Envoy backend. Envoy already
-// created its ingress listener as part of the single container in SetupEgress,
-// so there is nothing more to create here — it simply returns the ingress port
-// reserved in SetupEgress (0 for stdio / UpstreamPort==0).
-func (*envoyProxy) SetupIngress(_ context.Context, _ proxySpec, egress egressResult) (int, error) {
-	return egress.ingressPort, nil
+	success = true
+	return ingressPort, nil
 }

pkg/container/docker/envoy_test.go

@@ -407,7 +407,11 @@ func TestBuildIngressListener_PortAndHostGating(t *testing.T) {
 			wantHostPortBound: 18080,
 		},
 		{
-			name: "inbound AllowHost restricts virtual host domains",
+			// Inbound.AllowHost is intentionally ignored for Envoy's ingress virtual
+			// host domains. The transparent proxy rewrites Host to include the port
+			// ("127.0.0.1:19090"), which would not match a bare hostname list. The
+			// inbound access restriction is enforced by the 127.0.0.1 port binding.
+			name: "inbound AllowHost is ignored — ingress always uses wildcard domain",
 			spec: proxySpec{
 				WorkloadName:  "svc",
 				UpstreamPort:  9090,
@@ -421,7 +425,7 @@ func TestBuildIngressListener_PortAndHostGating(t *testing.T) {
 			hostPort:          19090,
 			wantUpstreamRef:   "svc",
 			wantHostPortBound: 19090,
-			wantDomains:       []string{"app.example.com"},
+			wantDomains:       []string{`"*"`},
 		},
 		{
 			// No inbound AllowHost → wildcard virtual host. This is safe because
@@ -540,10 +544,12 @@ func TestWriteEnvoyBootstrap_FileMode(t *testing.T) {
 	info, err := os.Stat(path)
 	require.NoError(t, err)
 
-	// Mode must be 0600 — not 0644 — so that other processes cannot read the
-	// bootstrap config (which may contain sensitive socket addresses).
-	assert.Equal(t, os.FileMode(0o600), info.Mode().Perm(),
-		"bootstrap file must be written at mode 0600")
+	// Mode must be 0644: world-readable so the Envoy distroless container
+	// (UID 101) can read the bind-mounted file on Linux Docker Engine, where
+	// strict POSIX permissions prevent a different UID from reading 0600 files.
+	// The bootstrap contains no secrets, so world-readable is safe.
+	assert.Equal(t, os.FileMode(0o644), info.Mode().Perm(),
+		"bootstrap file must be written at mode 0644")
 
 	// File must contain valid JSON that deserializes back into envoyBootstrap.
 	data, err := os.ReadFile(path)
@@ -783,16 +789,20 @@ func TestEnvoyProxy_SetupOrchestration(t *testing.T) {
 				Endpoints:     map[string]*network.EndpointSettings{},
 			}
 
+			// SetupEgress must NOT create the container — it only returns env vars.
+			// Container creation is deferred to SetupIngress so the MCP hostname
+			// resolves on first probe (see #5922).
 			egress, err := e.SetupEgress(t.Context(), spec)
 			require.NoError(t, err)
-			assert.Equal(t, "app-egress", createdName, "envoy container must reuse the -egress name")
+			assert.Empty(t, createdName, "SetupEgress must not create the container")
 			assert.Equal(t, "http://app-egress:3128", egress.EnvVars["HTTP_PROXY"])
 
+			// SetupIngress creates the container and returns the ingress port.
 			ingressPort, err := e.SetupIngress(t.Context(), spec, egress)
 			require.NoError(t, err)
+			assert.Equal(t, "app-egress", createdName, "SetupIngress must create the -egress container")
 			if tt.wantIngress {
 				assert.Positive(t, ingressPort, "non-stdio must reserve an ingress port")
-				assert.Equal(t, egress.ingressPort, ingressPort, "SetupIngress must return the port reserved in SetupEgress")
 			} else {
 				assert.Zero(t, ingressPort, "stdio must not reserve an ingress port")
 			}

pkg/container/docker/networkproxy.go

@@ -74,16 +74,12 @@ type proxySpec struct {
 }
 
 // egressResult is the output of a successful SetupEgress call. It is passed to
-// SetupIngress so a consolidated backend can carry state (e.g. a reserved
-// ingress port) forward without holding per-workload state on the shared proxy.
+// SetupIngress so backends can carry any state set up during egress forward
+// without holding per-workload state on the shared proxy.
 type egressResult struct {
 	// EnvVars contains environment variables that must be merged into the MCP
 	// container's environment (e.g. HTTP_PROXY, HTTPS_PROXY).
 	EnvVars map[string]string
-	// ingressPort is the host-side ingress port reserved by a consolidated
-	// backend (envoy) when it created its container in SetupEgress. Per-container
-	// backends (squid) leave it 0 and bind the ingress port later in SetupIngress.
-	ingressPort int
 }
 
 // newNetworkProxy reads the TOOLHIVE_NETWORK_PROXY environment variable and

test/e2e/network_isolation_envoy_test.go

@@ -8,10 +8,6 @@ package e2e_test
 // as the Squid suite in network_isolation_test.go — egress allowlist, inbound
 // Host gating, docker-gateway deny/allow — plus Envoy-specific guards such as
 // the route-timeout regression test.
-//
-// All tests are currently skipped pending resolution of #5922 (Envoy isolated
-// server does not reach ready state on Linux Docker Engine). Remove the
-// BeforeEach(Skip(...)) call to activate them once #5922 is fixed.
 
 import (
 	"context"
@@ -39,11 +35,6 @@ var _ = Describe("NetworkIsolationEnvoy", Label("proxy", "network", "isolation",
 	)
 
 	BeforeEach(func() {
-		// TODO(#5922): Remove this skip once Envoy isolated-server readiness on
-		// Linux Docker Engine is fixed. The tests are correct; the backend is not
-		// yet ready on that platform.
-		Skip("Envoy isolated-server readiness on Linux Docker Engine is broken — see #5922")
-
 		config = e2e.NewTestConfig()
 
 		var err error
@@ -85,7 +76,7 @@ var _ = Describe("NetworkIsolationEnvoy", Label("proxy", "network", "isolation",
 		runArgs = append(runArgs, "fetch")
 
 		envoyRun(config, runArgs...).ExpectSuccess()
-		Expect(e2e.WaitForMCPServer(config, serverName, 120*time.Second)).
+		Expect(e2e.WaitForMCPServer(config, serverName, 180*time.Second)).
 			To(Succeed(), "Envoy server should be running within 120 seconds")
 		return serverName
 	}
@@ -157,7 +148,7 @@ var _ = Describe("NetworkIsolationEnvoy", Label("proxy", "network", "isolation",
 			runArgs = append(runArgs, "--permission-profile", profilePath, "fetch")
 			envoyRun(config, runArgs...).ExpectSuccess()
 
-			Expect(e2e.WaitForMCPServer(config, serverName, 120*time.Second)).To(Succeed())
+			Expect(e2e.WaitForMCPServer(config, serverName, 180*time.Second)).To(Succeed())
 			serverURL, err := e2e.GetMCPServerURL(config, serverName)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(e2e.WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second)).To(Succeed())
@@ -250,7 +241,9 @@ var _ = Describe("NetworkIsolationEnvoy", Label("proxy", "network", "isolation",
 			target := fmt.Sprintf("http://%s:%d/", dockerBridgeGatewayIP(), port)
 			server := startFetchServer("slow", "", "--allow-docker-gateway")
 
-			result := fetchThrough(server, target, 30*time.Second)
+			// 60s: 16s upstream sleep + generous MCP client round-trip headroom.
+			// The 30s default was too tight under CI infrastructure load.
+			result := fetchThrough(server, target, 60*time.Second)
 			if result.IsError {
 				Skip("docker bridge gateway is not routable to the host in this environment")
 			}

test/e2e/run_tests.sh

@@ -59,6 +59,7 @@ cd "$(dirname "$0")"
 # Build ginkgo command with conditional GitHub output flag
 GINKGO_CMD="ginkgo run --timeout=\"$TEST_TIMEOUT\""
 GINKGO_CMD="$GINKGO_CMD --junit-report=junit-report.xml --output-dir=."
+GINKGO_CMD="$GINKGO_CMD --silence-skips"
 if [ -n "$GITHUB_ACTIONS" ]; then
     echo -e "${GREEN}✓${NC} GitHub Actions detected, enabling GitHub output format"
     GINKGO_CMD="$GINKGO_CMD --github-output --vv"