fix(deployment): reject TCP readiness when the port relay closes right after accept
- commit
- 7a64a5a70093a602d0ee8a68a5ae87bb018afa99
- author
- brice · · 2026-10-16 20:14 UTC
- parent
- 633b9ae
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +153 -3
M docs/config/apps.md +2 -0
| @@ -166,6 +166,8 @@ A container port declared by both an internal HTTP interface and an externally b | |||
| 166 | 166 | ||
| 167 | 167 | `type = "http"` requires an origin-form `path` beginning with a single `/`. Absolute URLs, authority forms such as `@host:port`, scheme-relative paths, and control characters are rejected at apply time. The probe always dials the declared loopback backend, never follows redirects, ignores environment proxy settings, and is bounded per request and for the whole operation. A public or otherwise published interface keeps this loopback probe. An internal HTTP port is instead probed over the app private network by one bounded, short-lived helper, using HTTP or TCP according to the interface's `[services.<name>.readiness]` type; the internal port is never temporarily published to the host to probe it. | |
| 168 | 168 | ||
| 169 | + | `type = "tcp"` on a published port is ready when a connection to the loopback backend is accepted and then either receives data or stays open for 300ms. A connection closed right after accept counts as not ready: the host port relay of rootless Docker accepts on behalf of a container that does not listen yet. A workload that accepts and closes immediately by design should use `log` readiness instead. | |
| 170 | + | ||
| 169 | 171 | ## TLS | |
| 170 | 172 | ||
| 171 | 173 | TLS applies to public interfaces only; an internal interface never declares `tls`. | |
| @@ -166,6 +166,8 @@ A container port declared by both an internal HTTP interface and an externally b | |||||
| 166 | 166 | ||||
| 167 | `type = "http"` requires an origin-form `path` beginning with a single `/`. Absolute URLs, authority forms such as `@host:port`, scheme-relative paths, and control characters are rejected at apply time. The probe always dials the declared loopback backend, never follows redirects, ignores environment proxy settings, and is bounded per request and for the whole operation. A public or otherwise published interface keeps this loopback probe. An internal HTTP port is instead probed over the app private network by one bounded, short-lived helper, using HTTP or TCP according to the interface's `[services.<name>.readiness]` type; the internal port is never temporarily published to the host to probe it. | 167 | `type = "http"` requires an origin-form `path` beginning with a single `/`. Absolute URLs, authority forms such as `@host:port`, scheme-relative paths, and control characters are rejected at apply time. The probe always dials the declared loopback backend, never follows redirects, ignores environment proxy settings, and is bounded per request and for the whole operation. A public or otherwise published interface keeps this loopback probe. An internal HTTP port is instead probed over the app private network by one bounded, short-lived helper, using HTTP or TCP according to the interface's `[services.<name>.readiness]` type; the internal port is never temporarily published to the host to probe it. | ||
| 168 | 168 | ||||
| 169 | + | `type = "tcp"` on a published port is ready when a connection to the loopback backend is accepted and then either receives data or stays open for 300ms. A connection closed right after accept counts as not ready: the host port relay of rootless Docker accepts on behalf of a container that does not listen yet. A workload that accepts and closes immediately by design should use `log` readiness instead. | |||
| 170 | + | ||||
| 169 | ## TLS | 171 | ## TLS | ||
| 170 | 172 | ||||
| 171 | TLS applies to public interfaces only; an internal interface never declares `tls`. | 173 | TLS applies to public interfaces only; an internal interface never declares `tls`. | ||
M internal/usecase/deployment/probes.go +42 -3
| @@ -3,6 +3,7 @@ package deployment | |||
| 3 | 3 | import ( | |
| 4 | 4 | "bufio" | |
| 5 | 5 | "context" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 7 | 8 | "io" | |
| 8 | 9 | "net" | |
| @@ -42,15 +43,53 @@ func httpGetOnce(ctx context.Context, probeURL, hostAuthority string) (int, erro | |||
| 42 | 43 | return resp.StatusCode, nil | |
| 43 | 44 | } | |
| 44 | 45 | ||
| 45 | - | // tcpDialOnce attempts one TCP connection. | |
| 46 | + | // tcpSettleWindow is how long an accepted TCP probe connection must stay open | |
| 47 | + | // (or deliver data) before the target counts as ready. | |
| 48 | + | const tcpSettleWindow = 300 * time.Millisecond | |
| 49 | + | ||
| 50 | + | // errTCPProbeClosed reports a connection that was accepted and then closed | |
| 51 | + | // within the settle window. | |
| 52 | + | var errTCPProbeClosed = errors.New("connection closed right after accept") | |
| 53 | + | ||
| 54 | + | // tcpDialOnce attempts one TCP readiness check against a published loopback | |
| 55 | + | // bind. A successful connect alone proves nothing: the host-side port relay | |
| 56 | + | // (rootless Docker, docker-proxy) accepts on behalf of the container and then | |
| 57 | + | // closes at once while the workload is not listening yet. So the connection | |
| 58 | + | // must also survive tcpSettleWindow: data from the workload (a banner) or a | |
| 59 | + | // connection still open at the end of the window means ready; EOF or a reset | |
| 60 | + | // means not ready. | |
| 46 | 61 | func tcpDialOnce(ctx context.Context, addr string) error { | |
| 62 | + | return tcpProbeOnce(ctx, addr, tcpSettleWindow) | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | func tcpProbeOnce(ctx context.Context, addr string, settle time.Duration) error { | |
| 47 | 66 | dialer := &net.Dialer{Timeout: 2 * time.Second} | |
| 48 | 67 | conn, err := dialer.DialContext(ctx, "tcp", addr) | |
| 49 | 68 | if err != nil { | |
| 50 | 69 | return err | |
| 51 | 70 | } | |
| 52 | - | _ = conn.Close() | |
| 53 | - | return nil | |
| 71 | + | defer func() { _ = conn.Close() }() | |
| 72 | + | if err := conn.SetReadDeadline(time.Now().Add(settle)); err != nil { | |
| 73 | + | return fmt.Errorf("set probe read deadline: %w", err) | |
| 74 | + | } | |
| 75 | + | stop := context.AfterFunc(ctx, func() { _ = conn.SetReadDeadline(time.Now()) }) | |
| 76 | + | defer stop() | |
| 77 | + | var buf [1]byte | |
| 78 | + | n, err := conn.Read(buf[:]) | |
| 79 | + | if n > 0 { | |
| 80 | + | return nil | |
| 81 | + | } | |
| 82 | + | if ctxErr := ctx.Err(); ctxErr != nil { | |
| 83 | + | return ctxErr | |
| 84 | + | } | |
| 85 | + | var netErr net.Error | |
| 86 | + | if errors.As(err, &netErr) && netErr.Timeout() { | |
| 87 | + | return nil | |
| 88 | + | } | |
| 89 | + | if err == nil || errors.Is(err, io.EOF) { | |
| 90 | + | return errTCPProbeClosed | |
| 91 | + | } | |
| 92 | + | return err | |
| 54 | 93 | } | |
| 55 | 94 | ||
| 56 | 95 | // scanForMarker reports whether the stream contains the marker. | |
| @@ -3,6 +3,7 @@ package deployment | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bufio" | 4 | "bufio" | ||
| 5 | "context" | 5 | "context" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 7 | "io" | 8 | "io" | ||
| 8 | "net" | 9 | "net" | ||
| @@ -42,15 +43,53 @@ func httpGetOnce(ctx context.Context, probeURL, hostAuthority string) (int, erro | |||||
| 42 | return resp.StatusCode, nil | 43 | return resp.StatusCode, nil | ||
| 43 | } | 44 | } | ||
| 44 | 45 | ||||
| 45 | - | // tcpDialOnce attempts one TCP connection. | 46 | + | // tcpSettleWindow is how long an accepted TCP probe connection must stay open |
| 47 | + | // (or deliver data) before the target counts as ready. | |||
| 48 | + | const tcpSettleWindow = 300 * time.Millisecond | |||
| 49 | + | ||||
| 50 | + | // errTCPProbeClosed reports a connection that was accepted and then closed | |||
| 51 | + | // within the settle window. | |||
| 52 | + | var errTCPProbeClosed = errors.New("connection closed right after accept") | |||
| 53 | + | ||||
| 54 | + | // tcpDialOnce attempts one TCP readiness check against a published loopback | |||
| 55 | + | // bind. A successful connect alone proves nothing: the host-side port relay | |||
| 56 | + | // (rootless Docker, docker-proxy) accepts on behalf of the container and then | |||
| 57 | + | // closes at once while the workload is not listening yet. So the connection | |||
| 58 | + | // must also survive tcpSettleWindow: data from the workload (a banner) or a | |||
| 59 | + | // connection still open at the end of the window means ready; EOF or a reset | |||
| 60 | + | // means not ready. | |||
| 46 | func tcpDialOnce(ctx context.Context, addr string) error { | 61 | func tcpDialOnce(ctx context.Context, addr string) error { | ||
| 62 | + | return tcpProbeOnce(ctx, addr, tcpSettleWindow) | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | func tcpProbeOnce(ctx context.Context, addr string, settle time.Duration) error { | |||
| 47 | dialer := &net.Dialer{Timeout: 2 * time.Second} | 66 | dialer := &net.Dialer{Timeout: 2 * time.Second} | ||
| 48 | conn, err := dialer.DialContext(ctx, "tcp", addr) | 67 | conn, err := dialer.DialContext(ctx, "tcp", addr) | ||
| 49 | if err != nil { | 68 | if err != nil { | ||
| 50 | return err | 69 | return err | ||
| 51 | } | 70 | } | ||
| 52 | - | _ = conn.Close() | 71 | + | defer func() { _ = conn.Close() }() |
| 53 | - | return nil | 72 | + | if err := conn.SetReadDeadline(time.Now().Add(settle)); err != nil { |
| 73 | + | return fmt.Errorf("set probe read deadline: %w", err) | |||
| 74 | + | } | |||
| 75 | + | stop := context.AfterFunc(ctx, func() { _ = conn.SetReadDeadline(time.Now()) }) | |||
| 76 | + | defer stop() | |||
| 77 | + | var buf [1]byte | |||
| 78 | + | n, err := conn.Read(buf[:]) | |||
| 79 | + | if n > 0 { | |||
| 80 | + | return nil | |||
| 81 | + | } | |||
| 82 | + | if ctxErr := ctx.Err(); ctxErr != nil { | |||
| 83 | + | return ctxErr | |||
| 84 | + | } | |||
| 85 | + | var netErr net.Error | |||
| 86 | + | if errors.As(err, &netErr) && netErr.Timeout() { | |||
| 87 | + | return nil | |||
| 88 | + | } | |||
| 89 | + | if err == nil || errors.Is(err, io.EOF) { | |||
| 90 | + | return errTCPProbeClosed | |||
| 91 | + | } | |||
| 92 | + | return err | |||
| 54 | } | 93 | } | ||
| 55 | 94 | ||||
| 56 | // scanForMarker reports whether the stream contains the marker. | 95 | // scanForMarker reports whether the stream contains the marker. | ||
A internal/usecase/deployment/probes_tcp_test.go +109 -0
| @@ -0,0 +1,109 @@ | |||
| 1 | + | package deployment | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "net" | |
| 6 | + | "testing" | |
| 7 | + | "time" | |
| 8 | + | ||
| 9 | + | "github.com/stretchr/testify/assert" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // serveTCP accepts connections on loopback and hands each to handle. | |
| 14 | + | func serveTCP(t *testing.T, handle func(net.Conn)) string { | |
| 15 | + | t.Helper() | |
| 16 | + | ln, err := net.Listen("tcp", "127.0.0.1:0") | |
| 17 | + | require.NoError(t, err) | |
| 18 | + | t.Cleanup(func() { _ = ln.Close() }) | |
| 19 | + | go func() { | |
| 20 | + | for { | |
| 21 | + | conn, err := ln.Accept() | |
| 22 | + | if err != nil { | |
| 23 | + | return | |
| 24 | + | } | |
| 25 | + | go handle(conn) | |
| 26 | + | } | |
| 27 | + | }() | |
| 28 | + | return ln.Addr().String() | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | const testSettle = 100 * time.Millisecond | |
| 32 | + | ||
| 33 | + | func TestTCPProbeOnce_RelayAcceptThenCloseIsNotReady(t *testing.T) { | |
| 34 | + | // A host port relay with no listener behind it accepts and closes at once. | |
| 35 | + | addr := serveTCP(t, func(c net.Conn) { _ = c.Close() }) | |
| 36 | + | ||
| 37 | + | err := tcpProbeOnce(context.Background(), addr, testSettle) | |
| 38 | + | ||
| 39 | + | require.ErrorIs(t, err, errTCPProbeClosed) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | func TestTCPProbeOnce_RelayResetIsNotReady(t *testing.T) { | |
| 43 | + | addr := serveTCP(t, func(c net.Conn) { | |
| 44 | + | if tc, ok := c.(*net.TCPConn); ok { | |
| 45 | + | _ = tc.SetLinger(0) // close with RST | |
| 46 | + | } | |
| 47 | + | _ = c.Close() | |
| 48 | + | }) | |
| 49 | + | ||
| 50 | + | err := tcpProbeOnce(context.Background(), addr, testSettle) | |
| 51 | + | ||
| 52 | + | require.Error(t, err) | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | func TestTCPProbeOnce_SilentListenerIsReady(t *testing.T) { | |
| 56 | + | // A workload that never speaks first (Postgres, Redis) keeps the connection open. | |
| 57 | + | release := make(chan struct{}) | |
| 58 | + | t.Cleanup(func() { close(release) }) | |
| 59 | + | addr := serveTCP(t, func(c net.Conn) { | |
| 60 | + | <-release | |
| 61 | + | _ = c.Close() | |
| 62 | + | }) | |
| 63 | + | ||
| 64 | + | start := time.Now() | |
| 65 | + | err := tcpProbeOnce(context.Background(), addr, testSettle) | |
| 66 | + | ||
| 67 | + | require.NoError(t, err) | |
| 68 | + | assert.GreaterOrEqual(t, time.Since(start), testSettle) | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | func TestTCPProbeOnce_BannerIsReadyImmediately(t *testing.T) { | |
| 72 | + | // A workload that speaks first (SSH, SMTP) is ready on its first byte. | |
| 73 | + | addr := serveTCP(t, func(c net.Conn) { | |
| 74 | + | _, _ = c.Write([]byte("SSH-2.0-test\r\n")) | |
| 75 | + | _ = c.Close() | |
| 76 | + | }) | |
| 77 | + | ||
| 78 | + | start := time.Now() | |
| 79 | + | err := tcpProbeOnce(context.Background(), addr, 5*time.Second) | |
| 80 | + | ||
| 81 | + | require.NoError(t, err) | |
| 82 | + | assert.Less(t, time.Since(start), 2*time.Second) | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | func TestTCPProbeOnce_NoListenerFails(t *testing.T) { | |
| 86 | + | ln, err := net.Listen("tcp", "127.0.0.1:0") | |
| 87 | + | require.NoError(t, err) | |
| 88 | + | addr := ln.Addr().String() | |
| 89 | + | require.NoError(t, ln.Close()) | |
| 90 | + | ||
| 91 | + | require.Error(t, tcpProbeOnce(context.Background(), addr, testSettle)) | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | func TestTCPProbeOnce_ContextCancelStopsTheSettleWait(t *testing.T) { | |
| 95 | + | release := make(chan struct{}) | |
| 96 | + | t.Cleanup(func() { close(release) }) | |
| 97 | + | addr := serveTCP(t, func(c net.Conn) { | |
| 98 | + | <-release | |
| 99 | + | _ = c.Close() | |
| 100 | + | }) | |
| 101 | + | ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) | |
| 102 | + | defer cancel() | |
| 103 | + | ||
| 104 | + | start := time.Now() | |
| 105 | + | err := tcpProbeOnce(ctx, addr, 10*time.Second) | |
| 106 | + | ||
| 107 | + | require.ErrorIs(t, err, context.DeadlineExceeded) | |
| 108 | + | assert.Less(t, time.Since(start), 5*time.Second) | |
| 109 | + | } | |
| @@ -0,0 +1,109 @@ | |||||
| 1 | + | package deployment | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "net" | |||
| 6 | + | "testing" | |||
| 7 | + | "time" | |||
| 8 | + | ||||
| 9 | + | "github.com/stretchr/testify/assert" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // serveTCP accepts connections on loopback and hands each to handle. | |||
| 14 | + | func serveTCP(t *testing.T, handle func(net.Conn)) string { | |||
| 15 | + | t.Helper() | |||
| 16 | + | ln, err := net.Listen("tcp", "127.0.0.1:0") | |||
| 17 | + | require.NoError(t, err) | |||
| 18 | + | t.Cleanup(func() { _ = ln.Close() }) | |||
| 19 | + | go func() { | |||
| 20 | + | for { | |||
| 21 | + | conn, err := ln.Accept() | |||
| 22 | + | if err != nil { | |||
| 23 | + | return | |||
| 24 | + | } | |||
| 25 | + | go handle(conn) | |||
| 26 | + | } | |||
| 27 | + | }() | |||
| 28 | + | return ln.Addr().String() | |||
| 29 | + | } | |||
| 30 | + | ||||
| 31 | + | const testSettle = 100 * time.Millisecond | |||
| 32 | + | ||||
| 33 | + | func TestTCPProbeOnce_RelayAcceptThenCloseIsNotReady(t *testing.T) { | |||
| 34 | + | // A host port relay with no listener behind it accepts and closes at once. | |||
| 35 | + | addr := serveTCP(t, func(c net.Conn) { _ = c.Close() }) | |||
| 36 | + | ||||
| 37 | + | err := tcpProbeOnce(context.Background(), addr, testSettle) | |||
| 38 | + | ||||
| 39 | + | require.ErrorIs(t, err, errTCPProbeClosed) | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | func TestTCPProbeOnce_RelayResetIsNotReady(t *testing.T) { | |||
| 43 | + | addr := serveTCP(t, func(c net.Conn) { | |||
| 44 | + | if tc, ok := c.(*net.TCPConn); ok { | |||
| 45 | + | _ = tc.SetLinger(0) // close with RST | |||
| 46 | + | } | |||
| 47 | + | _ = c.Close() | |||
| 48 | + | }) | |||
| 49 | + | ||||
| 50 | + | err := tcpProbeOnce(context.Background(), addr, testSettle) | |||
| 51 | + | ||||
| 52 | + | require.Error(t, err) | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | func TestTCPProbeOnce_SilentListenerIsReady(t *testing.T) { | |||
| 56 | + | // A workload that never speaks first (Postgres, Redis) keeps the connection open. | |||
| 57 | + | release := make(chan struct{}) | |||
| 58 | + | t.Cleanup(func() { close(release) }) | |||
| 59 | + | addr := serveTCP(t, func(c net.Conn) { | |||
| 60 | + | <-release | |||
| 61 | + | _ = c.Close() | |||
| 62 | + | }) | |||
| 63 | + | ||||
| 64 | + | start := time.Now() | |||
| 65 | + | err := tcpProbeOnce(context.Background(), addr, testSettle) | |||
| 66 | + | ||||
| 67 | + | require.NoError(t, err) | |||
| 68 | + | assert.GreaterOrEqual(t, time.Since(start), testSettle) | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | func TestTCPProbeOnce_BannerIsReadyImmediately(t *testing.T) { | |||
| 72 | + | // A workload that speaks first (SSH, SMTP) is ready on its first byte. | |||
| 73 | + | addr := serveTCP(t, func(c net.Conn) { | |||
| 74 | + | _, _ = c.Write([]byte("SSH-2.0-test\r\n")) | |||
| 75 | + | _ = c.Close() | |||
| 76 | + | }) | |||
| 77 | + | ||||
| 78 | + | start := time.Now() | |||
| 79 | + | err := tcpProbeOnce(context.Background(), addr, 5*time.Second) | |||
| 80 | + | ||||
| 81 | + | require.NoError(t, err) | |||
| 82 | + | assert.Less(t, time.Since(start), 2*time.Second) | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | func TestTCPProbeOnce_NoListenerFails(t *testing.T) { | |||
| 86 | + | ln, err := net.Listen("tcp", "127.0.0.1:0") | |||
| 87 | + | require.NoError(t, err) | |||
| 88 | + | addr := ln.Addr().String() | |||
| 89 | + | require.NoError(t, ln.Close()) | |||
| 90 | + | ||||
| 91 | + | require.Error(t, tcpProbeOnce(context.Background(), addr, testSettle)) | |||
| 92 | + | } | |||
| 93 | + | ||||
| 94 | + | func TestTCPProbeOnce_ContextCancelStopsTheSettleWait(t *testing.T) { | |||
| 95 | + | release := make(chan struct{}) | |||
| 96 | + | t.Cleanup(func() { close(release) }) | |||
| 97 | + | addr := serveTCP(t, func(c net.Conn) { | |||
| 98 | + | <-release | |||
| 99 | + | _ = c.Close() | |||
| 100 | + | }) | |||
| 101 | + | ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) | |||
| 102 | + | defer cancel() | |||
| 103 | + | ||||
| 104 | + | start := time.Now() | |||
| 105 | + | err := tcpProbeOnce(ctx, addr, 10*time.Second) | |||
| 106 | + | ||||
| 107 | + | require.ErrorIs(t, err, context.DeadlineExceeded) | |||
| 108 | + | assert.Less(t, time.Since(start), 5*time.Second) | |||
| 109 | + | } | |||