Skip to content

feat(proxy): multiplex HTTP and Postgres on one port when their ports coincide - #56

Merged
andybons merged 2 commits into
mainfrom
single-port-multiplexing
Jul 15, 2026
Merged

feat(proxy): multiplex HTTP and Postgres on one port when their ports coincide#56
andybons merged 2 commits into
mainfrom
single-port-multiplexing

Conversation

@andybons

Copy link
Copy Markdown
Contributor

What

Serves the HTTP/CONNECT proxy and the Postgres data plane on a single listener when postgres.port == proxy.port (and the same host), so one GCP TCP Proxy load balancer — one forwarding rule, one backend service, one health check, one PROXY config — can front both planes. A TCP Proxy LB is L4 and can't route the two apart itself, so gatekeeper demultiplexes them.

Design

Trigger — no flag. Multiplexing is expressed by the ports coinciding, mirroring how Traefik/Caddy attach multiple protocols to one entrypoint (no multiplex: true switch). Different ports → two listeners, exactly as today. port: 0 (OS-assigned) never triggers it — two ephemeral binds aren't "the same port."

Mechanism — hand-rolled cmux-style, no new dependency. One accept loop owns the real (optionally proxyproto-wrapped) listener. Each accepted conn is peeked in its own goroutine (never the accept loop — preserves the don't-block-Accept discipline), classified, and pushed onto one of two in-memory virtual listeners. http.Server.Serve and PostgresServer.StartListener run unmodified on them.

Classification. Positive-match Postgres (SSLRequest / GSSENCRequest / v3 StartupMessage), everything else → HTTP. A short read can't match (requires 8 bytes with a zero at offset 4, which no HTTP/TLS opening has). A misroute lands on the wrong parser and fails — it can't bypass a control, since each plane still authenticates after demux.

PROXY protocol. The wrapper stays outermost; the header is stripped before classification and the advertised client IP reaches both planes. One shared listener ⇒ one setting: equal ports require proxy.proxy_protocol == postgres.proxy_protocol (fatal error on mismatch).

Review

Opus-reviewed (SHIP): mutation-checked the classifier and byte-replay tests non-vacuous, verified the accept loop never blocks on a slow client, byte-replay is exact through both planes, shutdown drains without goroutine/fd leaks, and demux only routes (auth unchanged). One doc/code wording mismatch it flagged (host match is string equality, not address resolution) is corrected in the config reference.

Tests

Test-first: classifier unit tests incl. adversarial inputs (short read, garbage, length-only, all HTTP verbs, h2 preface, GSSENC), shared-port E2E (real CONNECT+TLS interception AND real Postgres handshake on one port), PROXY-on-shared-port for both planes, silent-client-doesn't-block-Accept, backlog-full-drops. Live-verified end-to-end. go test -race ./..., vet, gofmt clean. Distinct-port path is the original code moved verbatim — all pre-existing tests untouched and green.

CHANGELOG v0.19.0.

… coincide

Ships as v0.19.0. When postgres.port equals proxy.port (and their hosts
resolve to the same address), gatekeeper now multiplexes the HTTP/CONNECT
proxy and the Postgres data-plane listener onto one real listener instead
of binding two — no new config flag; the port equality itself is the
declaration. Distinct ports (today's default, and every existing config)
keep the two-listener path byte-for-byte unchanged.

Mechanism: proxy.Demux (proxy/demux.go) owns the one real listener and
classifies each accepted connection by its first 8 bytes in its own
goroutine — never in the shared accept loop, so a silent or slow client
can't stall Accept for other connections. Classification positively
matches Postgres startup signatures (SSLRequest, GSSENCRequest, v3
StartupMessage) and defaults everything else to HTTP, mirroring Caddy-L4's
postgres matcher. The sniffed bytes are replayed on the classified
connection (mirroring proxyProtoLogConn's hold-then-replay pattern) before
it's pushed onto one of two in-memory virtual listeners; http.Server.Serve
and PostgresServer.StartListener then run completely unmodified against
those.

Config validation (resolveListenTopology, config.go) runs at New() and
Start() so the two can never disagree: equal ports with different hosts,
or with proxy.proxy_protocol != postgres.proxy_protocol, are fatal
startup errors — a shared listener has exactly one PROXY protocol
setting, owned by proxy.proxy_protocol. Port 0 on both sides never
triggers multiplexing, since two independent ephemeral binds aren't "the
same port" — this preserves every existing test and config that leaves
the port unset on both listeners.

Docs: config reference ("Sharing one listener with the HTTP proxy"),
load-balancer guide ("Single load balancer, one shared port", including
health-check behavior on the shared listener), a concepts note, and a
commented example in examples/gatekeeper-postgres.yaml.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor
📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 5m 8s —— View job


Review: single-port multiplexing (proxy/demux.go)

  • Gather context (read diff, changed files)
  • Review proxy/demux.go (classifier, accept loop, virtual listeners)
  • Review gatekeeper.go wiring changes
  • Review config.go topology validation
  • Review tests for soundness (non-vacuous)
  • Post findings

Summary

This is a well-scoped, well-documented change. I traced the full lifecycle carefully:

  • Classifier (classifyPrefix/isPostgresStartup) — the byte offsets for SSLRequest (80877103 = 0x04D2162F), GSSENCRequest (80877104 = 0x04D2162F+1 → 0x04D21630), and v3 StartupMessage (0x00030000 at bytes 4–7) are correct, and no legitimate HTTP method/h2 preface can collide (HTTP opens with printable ASCII, never a 0x00 byte at offset 4).
  • PROXY protocol ordering — verified against proxy/proxyproto.go: proxyProtoLogConn's header parse is lazy on first Read, so sniffProtocol's io.ReadFull naturally strips the PROXY header before the classification bytes are read. Order is correct.
  • demuxConn.Raw() / underlyingTCPConn — I initially suspected a double-unwrap bug (underlyingTCPConn in postgres.go only unwraps one Raw() layer), but demuxConn.Raw() itself recurses into the inner Raw() rather than returning c.Conn directly, so a single external .Raw() call correctly reaches the real *net.TCPConn through both demuxConn → proxyProtoLogConn → proxyproto.Conn layers. Keep-alive setup is unaffected.
  • Shutdown/close sequencingDemux.StopAccepting vs Close, and virtualListener.push/Close under the same mutex, are race-free: a push after closed is set always observes it and the caller closes the conn itself. The "straggler" connection that's mid-classification when shutdown begins (sniff deadline up to 10s) isn't tracked by either downstream server's drain, but it self-closes once classification completes and finds its target virtualListener already closed — no fd leak, just a slightly delayed close after Stop()'s 5s deadline. Consistent with the documented backlog/drop semantics.
  • resolveListenTopology — port-0 exclusion, host-string-equality (not resolution), and the proxy_protocol mismatch check are all correct and match the docs precisely. Test coverage in config_topology_test.go pins the real cases (equal ports, different hosts, mismatched proxy_protocol, both-zero) with meaningful assertions, not vacuous checks.
  • No credential/content leaks — the demux only ever logs "http"/"postgres" or a generic read-error reason, never bytes.

Finding

One inline comment on proxy/demux.go (accept-loop resilience): Demux.acceptLoop exits permanently on any Accept() error, including transient ones (e.g. fd exhaustion). Previously this pattern already existed in PostgresServer.acceptLoop, but it only risked the Postgres plane — the HTTP plane ran through http.Server.Serve's own real-listener accept loop, which tolerates transient errors. After this change, in multiplexed mode Demux.acceptLoop is the only thing calling Accept() on the real socket, so a transient error now takes down accept for both planes until process restart. Worth a bounded retry/backoff on transient errors given the widened blast radius. Not a security issue — a reliability/availability one.

Note on verification

I was not able to run go build/go vet/go test in this session (tool invocations for those commands required approval that wasn't available in this automated context). The review above is based on careful static reading of proxy/demux.go, proxy/postgres.go, proxy/proxyproto.go, gatekeeper.go, config.go, and the test files (demux_test.go, config_topology_test.go, gatekeeper_multiplex_test.go), cross-checked against the PR's own stated test plan. If CI is green, that plus this review should give good confidence in the change.

Comment thread proxy/demux.go
…f exiting

Demux.acceptLoop is the sole caller of Accept on the real socket in
multiplex mode — http.Server.Serve only ever sees the virtual listener,
which never surfaces an OS-level error, so http.Server's own accept-retry
loop can neither see nor recover from a transient failure on the real
socket. The loop previously returned on ANY Accept error, so one transient
error (EMFILE/ENFILE under fd exhaustion, ECONNABORTED — realistic for a
proxy holding many long-lived CONNECT tunnels and Postgres relays)
permanently killed accept for BOTH planes until process restart. Before
multiplexing, http.Server.Serve tolerated exactly these errors, so this
silently removed the HTTP plane's prior resilience.

Mirror net/http.Server.Serve's accept-error handling: exit cleanly only
when the demux is shutting down (Close/StopAccepting set the closed flag
before closing the listener), otherwise back off with a capped exponential
delay (5ms doubling to a 1s cap, logged at WARN) and retry while the
listener is live, resetting the delay after a successful Accept. Unlike
net/http, the retry is not gated on the deprecated, unreliable
net.Error.Temporary(): any error while the listener is open is retried; a
genuinely dead-but-unclosed listener retries once per second forever with a
WARN each time — the same visible, capped pathological case net/http
tolerates, not a zero-delay spin.

PostgresServer.acceptLoop (proxy/postgres.go) has the same
unconditional-exit pattern, but its blast radius is unchanged by this PR
(non-multiplex Postgres plane only), so it is left out of scope here.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @andybons's task in 58s —— View job


Review: transient Accept-error fix (commit 9535760)

  • Gather context (diff since last review, new commit)
  • Verify the transient-error retry fix in proxy/demux.go
  • Check for regressions / new issues introduced by the fix
  • Post findings

Summary

Commit 9535760 directly addresses the availability finding from my previous review (comment): Demux.acceptLoop previously exited permanently on any Accept() error, which — since it's now the sole caller of Accept on the real socket in multiplex mode — meant a single transient error (EMFILE/ENFILE, ECONNABORTED) killed accept for both planes.

The fix (proxy/demux.go:287-336):

  • Exits cleanly and silently only when d.closed is already set (i.e. StopAccepting/Close initiated shutdown) — correct, since that flag is set before the listener is closed, so the resulting error is gatekeeper's own doing, not a real fault.
  • Otherwise applies a capped exponential backoff (5ms → 1s cap, doubling each attempt), logs at WARN, and retries — mirroring net/http.Server.Serve's historical tempDelay handling.
  • Resets backoff to 0 after a successful Accept, so a transient burst doesn't leave the loop artificially slow afterward.
  • Deliberately does not gate on net.Error.Temporary() (correctly noted as deprecated/unreliable) — any error while the listener is open is treated as retryable. The documented tradeoff (a truly dead-but-unclosed listener retries forever at a 1s cadence with a WARN each time) is reasonable and matches the PR's own description of the risk.

Checked for regressions:

  • Shutdown latency: since the retry loop only checks d.closed after an Accept call returns, a shutdown initiated mid-time.Sleep(backoff) waits out the remainder of that sleep (max 1s) before observing the closed flag. That's within the previously-reviewed 5s shutdown deadline and consistent with net/http's own behavior — not a concern.
  • No spin risk: backoff always resets from 0 → 5ms → ... rather than continuing to climb across unrelated error bursts, since it's reset on any successful Accept.
  • Logging: only logs the error value and computed delay, no connection data or credentials — consistent with the codebase's logging conventions.

Tests (proxy/demux_test.go) are well-targeted and non-vacuous:

  • TestDemux_AcceptLoopRetriesTransientErrors scripts 3 transient failures before a real connection and asserts the connection is still dispatched and its bytes replayed correctly — this fails under the old code (loop would exit after the first error, connection never accepted, test times out).
  • TestDemux_AcceptLoopExitsCleanlyOnClose asserts Accept is called exactly once after Close() (not spin-retried) and that no error/retry log line is emitted on a clean shutdown — this would fail if the closed-listener error were treated as transient instead of terminal.

No new issues found in this change. The fix is correctly scoped and doesn't touch PostgresServer.acceptLoop, which the PR description and commit message both correctly note is out of scope here (its blast radius is unchanged by this PR, since it only ever served the non-multiplexed Postgres path).

Note on verification

As with the previous review pass, I was not able to run go build/go vet/go test in this session — the Bash tool calls for those commands required approval that wasn't granted in this automated context. This review is based on careful static reading of the diff (proxy/demux.go, proxy/demux_test.go, CHANGELOG.md) and cross-referencing against the prior review's finding and the PR's stated test plan. If CI is green, that plus this review should give good confidence in the fix.

@andybons
andybons merged commit 4e4e6eb into main Jul 15, 2026
2 checks passed
@andybons
andybons deleted the single-port-multiplexing branch July 15, 2026 22:51
andybons added a commit that referenced this pull request Jul 15, 2026
…nstead of exiting (#57)

PostgresServer.acceptLoop had the identical unconditional-exit-on-any-
Accept-error bug that PR #56 fixed in Demux.acceptLoop, and was
intentionally left out of that PR's scope. This is the sibling fix:
acceptLoop now mirrors Demux.acceptLoop (and, in turn, net/http.Server.Serve)
by retrying any non-shutdown Accept error with a capped exponential backoff
(reusing demux.go's demuxAcceptRetryBaseDelay/demuxAcceptRetryMaxDelay
constants so the two loops can't drift), logging at WARN, and only exiting
cleanly on an intentional Stop/Shutdown.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant