fix: free host-netns :80 for ECS bridge/host tasks via cred-endpoint DNAT (#446)

* fix: free host-netns :80 for ECS bridge/host tasks via cred-endpoint DNAT bridge and host network-mode tasks share the VM host netns where the ecs-agent credential endpoint bound 169.254.170.2:80, so a task binding 0.0.0.0:80 hit EADDRINUSE and crash-looped. Bind the listener on the high proxy port 51679 and DNAT 169.254.170.2:80 to it (OUTPUT for host/bridge task traffic, PREROUTING for the awsvpc credential veth), keeping the dst IP on the endpoint so no route_localnet is needed. Nothing holds a socket on :80, so host/bridge tasks can bind it. e2e runs nginx on :80 in all three network modes; the settle guard catches a regression where a crash-looping :80 bind flaps RUNNING then STOPPED. * test: validate ECS host-netns :80 binding via bridge task The bridge subtest now runs nginx on :80 to prove the cred-endpoint DNAT frees the host netns (pre-fix it crash-looped on EADDRINUSE). host mode keeps a no-port command: it shares the same host netns, so two :80 tasks cannot coexist on one node (one :80 per host, as on AWS), and StopTask does not synchronously reap the prior container's port.
2026-10-15 17:11:30 +00:00 · 2026-06-30 08:59:52 +10:00
parent ab1b1bacf6
commit 52c0ed1e77
3 changed files with 117 additions and 13 deletions
@@ -22,6 +22,10 @@ const (
 	// container credentials; defaultCredEndpointPort is the standard HTTP port.
 	defaultCredEndpointIP   = "169.254.170.2"
 	defaultCredEndpointPort = 80
+	// defaultCredProxyPort is the high port the listener actually binds; a nat
+	// DNAT rule rewrites 169.254.170.2:80 to it so nothing holds a socket on :80
+	// and host/bridge tasks sharing the netns can bind :80 themselves.
+	defaultCredProxyPort = 51679
 	// credRefreshMargin refreshes a cached credential set this long before expiry
 	// so a container never starts a request with a token about to lapse.
 	credRefreshMargin = 5 * time.Minute
@@ -58,6 +62,7 @@ type credEndpoint struct {
 	caPath     string
 	ip         string
 	port       int
+	proxyPort  int
 	run        netCmdRunner

 	// assume mints fresh credentials for a role; defaults to AssumeRole over the
@@ -90,6 +95,7 @@ func newCredEndpoint(provider credentials.CredentialsProvider, region, gatewayUR
 		caPath:     caPath,
 		ip:         ip,
 		port:       port,
+		proxyPort:  defaultCredProxyPort,
 		run:        run,
 		roles:      map[string]string{},
 		cache:      map[string]credEntry{},
@@ -217,16 +223,21 @@ func (c *credEndpoint) client() (*http.Client, error) {
 	return hc, nil
 }

-// Start adds the endpoint IP to a dummy interface in the host netns and serves
-// HTTP on it. A bind or address failure is returned so the caller can log it
-// without aborting register/heartbeat.
+// Start adds the endpoint IP to a dummy interface in the host netns, serves HTTP
+// on the high proxy port, and DNATs the advertised :80 to it. A bind, address,
+// or nat failure is returned so the caller can log it without aborting
+// register/heartbeat.
 func (c *credEndpoint) Start() error {
 	if err := c.addEndpointAddr(); err != nil {
 		return err
 	}
-	ln, err := net.Listen("tcp", net.JoinHostPort(c.ip, strconv.Itoa(c.port)))
+	ln, err := net.Listen("tcp", net.JoinHostPort(c.ip, strconv.Itoa(c.listenPort())))
 	if err != nil {
-		return fmt.Errorf("listen %s:%d: %w", c.ip, c.port, err)
+		return fmt.Errorf("listen %s:%d: %w", c.ip, c.listenPort(), err)
+	}
+	if err := c.addRedirect(); err != nil {
+		_ = ln.Close()
+		return err
 	}
 	c.ln = ln
 	c.srv = &http.Server{Handler: c, ReadHeaderTimeout: 5 * time.Second}
@@ -250,10 +261,60 @@ func (c *credEndpoint) Stop() error {
 		defer cancel()
 		_ = c.srv.Shutdown(ctx)
 	}
+	c.delRedirect()
 	c.delEndpointAddr()
 	return nil
 }

+// listenPort is the port the socket binds. Loopback test mode binds the
+// caller-supplied (ephemeral) port directly; the real endpoint binds the high
+// proxy port and reaches :80 via the DNAT rules.
+func (c *credEndpoint) listenPort() int {
+	if c.ip == "127.0.0.1" {
+		return c.port
+	}
+	return c.proxyPort
+}
+
+// addRedirect installs nat DNAT rules rewriting the advertised :80 to the proxy
+// port, for both locally generated traffic (host/bridge task containers, OUTPUT)
+// and traffic arriving over the awsvpc credential veth (PREROUTING). The dst IP
+// is left on the endpoint IP so no route_localnet handling is needed. Skipped in
+// loopback test mode, which binds the port directly.
+func (c *credEndpoint) addRedirect() error {
+	if c.run == nil || c.ip == "127.0.0.1" {
+		return nil
+	}
+	for _, chain := range []string{"OUTPUT", "PREROUTING"} {
+		if _, err := c.run.Run("iptables", c.natArgs("-C", chain)...); err == nil {
+			continue // rule already present
+		}
+		if _, err := c.run.Run("iptables", c.natArgs("-A", chain)...); err != nil {
+			return fmt.Errorf("add nat redirect %s: %w", chain, err)
+		}
+	}
+	return nil
+}
+
+func (c *credEndpoint) delRedirect() {
+	if c.run == nil || c.ip == "127.0.0.1" {
+		return
+	}
+	for _, chain := range []string{"OUTPUT", "PREROUTING"} {
+		_, _ = c.run.Run("iptables", c.natArgs("-D", chain)...)
+	}
+}
+
+// natArgs builds the iptables nat rule for op (-A/-C/-D) on chain: DNAT the
+// advertised endpoint :80 to the proxy port on the same IP.
+func (c *credEndpoint) natArgs(op, chain string) []string {
+	return []string{
+		"-t", "nat", op, chain,
+		"-d", c.ip, "-p", "tcp", "--dport", strconv.Itoa(c.port),
+		"-j", "DNAT", "--to-destination", net.JoinHostPort(c.ip, strconv.Itoa(c.proxyPort)),
+	}
+}
+
 // addEndpointAddr brings up the dummy interface holding the endpoint IP. Skipped
 // for loopback (tests bind 127.0.0.1 directly, no interface plumbing needed).
 func (c *credEndpoint) addEndpointAddr() error {
@@ -151,6 +151,45 @@ func TestCredEndpoint_StartServesOverLoopback(t *testing.T) {
 	}
 }

+func TestCredEndpoint_RedirectInstallsAndRemovesDNAT(t *testing.T) {
+	// failOn the -C existence check so addRedirect falls through to -A.
+	f := &fakeNetRunner{failOn: "-t nat -C"}
+	c := newCredEndpoint(nil, "us-east-1", "https://gw", "", "169.254.170.2", 80, f)
+	if c.listenPort() != defaultCredProxyPort {
+		t.Fatalf("listenPort = %d, want %d (proxy)", c.listenPort(), defaultCredProxyPort)
+	}
+	if err := c.addRedirect(); err != nil {
+		t.Fatalf("addRedirect: %v", err)
+	}
+	target := "-d 169.254.170.2 -p tcp --dport 80 -j DNAT --to-destination 169.254.170.2:51679"
+	for _, chain := range []string{"-A OUTPUT", "-A PREROUTING"} {
+		if !f.sawAny("iptables -t nat " + chain + " " + target) {
+			t.Errorf("missing add rule for %s; calls=%v", chain, f.joined())
+		}
+	}
+	c.delRedirect()
+	for _, chain := range []string{"-D OUTPUT", "-D PREROUTING"} {
+		if !f.sawAny("iptables -t nat " + chain + " " + target) {
+			t.Errorf("missing delete rule for %s; calls=%v", chain, f.joined())
+		}
+	}
+}
+
+func TestCredEndpoint_RedirectSkippedOnLoopback(t *testing.T) {
+	f := &fakeNetRunner{}
+	c := newCredEndpoint(nil, "us-east-1", "https://gw", "", "127.0.0.1", 0, f)
+	if err := c.addRedirect(); err != nil {
+		t.Fatalf("addRedirect: %v", err)
+	}
+	c.delRedirect()
+	if f.sawAny("iptables") {
+		t.Errorf("loopback mode must not touch iptables; calls=%v", f.joined())
+	}
+	if c.listenPort() != c.port {
+		t.Errorf("loopback listenPort = %d, want caller port %d", c.listenPort(), c.port)
+	}
+}
+
 func TestCredRelativeURI(t *testing.T) {
 	if got := credRelativeURI("abc"); got != "/v2/credentials/abc" {
 		t.Errorf("credRelativeURI = %q", got)
@@ -32,10 +32,14 @@ const (
 	ecsSubnetBCIDR = "10.41.2.0/24"
 	ecsClusterPfx  = "ecs-e2e"
 	ecsInstanceTyp = "t3.small"
-	// awsvpc tasks serve HTTP on 80 (isolated ENI netns) so the ALB target group
-	// health check (GET /) passes. bridge/host tasks share the host netns, where
-	// the agent credential endpoint owns 169.254.170.2:80, so they run a durable
-	// no-port command instead and only assert placement + mode, not serving.
+	// awsvpc and bridge tasks serve nginx on :80. bridge shares the VM host netns
+	// where the agent credential endpoint used to own 169.254.170.2:80; it now
+	// binds :80 cleanly because the endpoint moved to a high proxy port + DNAT, so
+	// the bridge subtest is the regression proof (the settle guard in
+	// waitTaskRunning catches a crash-looping :80 bind that flaps RUNNING then
+	// STOPPED). host mode also shares that netns, so only one host-netns task can
+	// hold :80 at a time (one :80 per host, as on AWS); the host subtest runs a
+	// durable no-port command to assert placement without contending for :80.
 	ecsTaskImage    = "docker.io/library/nginx:1.27-alpine"
 	ecsHostNetnsCmd = "sleep 600"
 )
@@ -86,7 +90,7 @@ func TestECS(t *testing.T) {
 	})

 	t.Run("TaskBridge", func(t *testing.T) {
-		tdArn := registerTaskDef(t, c, fx, "bridge", strings.Fields(ecsHostNetnsCmd))
+		tdArn := registerTaskDef(t, c, fx, "bridge", nil)
 		task := runStandaloneTask(t, c, fx, tdArn, nil)
 		assert.Empty(t, task.Attachments, "bridge task must not allocate an ENI")
 	})
@@ -513,9 +517,9 @@ func waitContainerInstanceActive(t *testing.T, c *harness.AWSClient, cluster, ec
 // --- Task definitions & standalone tasks ----------------------------------

 // registerTaskDef registers a one-container task definition. command overrides
-// the image entrypoint; when set the container exposes no port (used by the
-// host-netns modes to avoid the credential endpoint's :80). When command is nil
-// the container runs nginx and publishes 80 for ALB/awsvpc serving.
+// the image entrypoint and exposes no port (the host subtest uses it so two
+// host-netns tasks don't contend for :80); when command is nil the container
+// runs nginx and publishes 80 for awsvpc/bridge serving.
 func registerTaskDef(t *testing.T, c *harness.AWSClient, fx *ecsFixture, networkMode string, command []string) string {
 	t.Helper()
 	family := fmt.Sprintf("%s-%s", fx.ClusterName, networkMode)