From 5bc8f4fc8e179a336ec89bf5604f66738e28df14 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Wed, 25 Mar 2026 14:25:41 -0700 Subject: [PATCH] Fix three doc-vs-implementation gaps found during audit 1. DB migration: add CHECK(mode IN ('l4', 'l7')) constraint on the routes.mode column. ARCHITECTURE.md documented this constraint but migration v2 omitted it. Enforces mode validity at the database level in addition to application-level validation. 2. L7 reverse proxy: distinguish timeout errors from connection errors in the ErrorHandler. Backend timeouts now return HTTP 504 Gateway Timeout instead of 502. Uses errors.Is(context.DeadlineExceeded) and net.Error.Timeout() detection. Added isTimeoutError unit tests. 3. Config validation: warn when L4 routes have tls_cert or tls_key set (they are silently ignored). ARCHITECTURE.md documented this warning but config.validate() did not emit it. Uses slog.Warn. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config/config.go | 7 +++++++ internal/db/migrations.go | 2 +- internal/l7/serve.go | 20 +++++++++++++++++++- internal/l7/serve_test.go | 25 +++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 289f7ed..f770a0b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,6 +3,7 @@ package config import ( "crypto/tls" "fmt" + "log/slog" "os" "strings" "time" @@ -198,6 +199,12 @@ func (c *Config) validate() error { i, l.Addr, j, r.Hostname, r.Mode) } + // Warn if L4 routes have cert/key set (they are ignored). + if r.Mode == "l4" && (r.TLSCert != "" || r.TLSKey != "") { + slog.Warn("L4 route has tls_cert/tls_key set (ignored)", + "listener", l.Addr, "hostname", r.Hostname) + } + // L7 routes require TLS cert and key. if r.Mode == "l7" { if r.TLSCert == "" || r.TLSKey == "" { diff --git a/internal/db/migrations.go b/internal/db/migrations.go index ba26697..49cb4b2 100644 --- a/internal/db/migrations.go +++ b/internal/db/migrations.go @@ -96,7 +96,7 @@ func migrate001CreateCoreTables(tx *sql.Tx) error { func migrate002AddL7Fields(tx *sql.Tx) error { stmts := []string{ `ALTER TABLE listeners ADD COLUMN proxy_protocol INTEGER NOT NULL DEFAULT 0`, - `ALTER TABLE routes ADD COLUMN mode TEXT NOT NULL DEFAULT 'l4'`, + `ALTER TABLE routes ADD COLUMN mode TEXT NOT NULL DEFAULT 'l4' CHECK(mode IN ('l4', 'l7'))`, `ALTER TABLE routes ADD COLUMN tls_cert TEXT NOT NULL DEFAULT ''`, `ALTER TABLE routes ADD COLUMN tls_key TEXT NOT NULL DEFAULT ''`, `ALTER TABLE routes ADD COLUMN backend_tls INTEGER NOT NULL DEFAULT 0`, diff --git a/internal/l7/serve.go b/internal/l7/serve.go index 00b5ab6..7f4bfbb 100644 --- a/internal/l7/serve.go +++ b/internal/l7/serve.go @@ -4,6 +4,7 @@ package l7 import ( "context" "crypto/tls" + "errors" "fmt" "log/slog" "net" @@ -132,7 +133,11 @@ func newReverseProxy(route RouteConfig, logger *slog.Logger) (*httputil.ReverseP Transport: transport, ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { logger.Error("reverse proxy error", "backend", route.Backend, "error", err) - w.WriteHeader(http.StatusBadGateway) + if isTimeoutError(err) { + w.WriteHeader(http.StatusGatewayTimeout) + } else { + w.WriteHeader(http.StatusBadGateway) + } }, } @@ -199,6 +204,19 @@ func setForwardingHeaders(r *http.Request, clientAddr netip.AddrPort) { r.Header.Set("X-Real-IP", clientIP) } +// isTimeoutError returns true if the error is a timeout (context deadline +// exceeded or net.Error timeout). +func isTimeoutError(err error) bool { + if errors.Is(err, context.DeadlineExceeded) { + return true + } + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return true + } + return false +} + // singleConnListener is a net.Listener that returns a single connection once, // then blocks until closed. Used to serve HTTP/1.1 on a single TLS connection. type singleConnListener struct { diff --git a/internal/l7/serve_test.go b/internal/l7/serve_test.go index b6d8b11..16c7936 100644 --- a/internal/l7/serve_test.go +++ b/internal/l7/serve_test.go @@ -311,6 +311,31 @@ func TestL7BackendUnreachable(t *testing.T) { } } +func TestIsTimeoutError(t *testing.T) { + // context.DeadlineExceeded is a timeout. + if !isTimeoutError(context.DeadlineExceeded) { + t.Fatal("expected DeadlineExceeded to be a timeout error") + } + + // A net timeout error is a timeout. + netErr := &net.OpError{Op: "dial", Err: &timeoutErr{}} + if !isTimeoutError(netErr) { + t.Fatal("expected net timeout to be a timeout error") + } + + // A regular error is not a timeout. + if isTimeoutError(fmt.Errorf("connection refused")) { + t.Fatal("expected non-timeout error to return false") + } +} + +// timeoutErr implements net.Error with Timeout() = true. +type timeoutErr struct{} + +func (e *timeoutErr) Error() string { return "timeout" } +func (e *timeoutErr) Timeout() bool { return true } +func (e *timeoutErr) Temporary() bool { return false } + func TestL7MultipleRequests(t *testing.T) { certPath, keyPath := testCert(t, "multi.test")