From c859bb854d75d7d497784619acf058446876991b Mon Sep 17 00:00:00 2001 From: SShogun Date: Sat, 10 Oct 2026 19:49:37 +0530 Subject: [PATCH 1/6] feat(dataplane): add static HTTP proxy --- README.md | 13 +- cmd/gatehouse-data/main.go | 42 ++- docs/Gatehouse_PLAN.md | 15 +- internal/dataplane/proxy.go | 160 +++++++++ internal/dataplane/proxy_test.go | 538 ++++++++++++++++++++++++++++++ internal/dataplane/server.go | 139 ++++++++ internal/dataplane/server_test.go | 104 ++++++ 7 files changed, 989 insertions(+), 22 deletions(-) create mode 100644 internal/dataplane/proxy.go create mode 100644 internal/dataplane/proxy_test.go create mode 100644 internal/dataplane/server.go create mode 100644 internal/dataplane/server_test.go diff --git a/README.md b/README.md index 94d27ac..9b1b2e6 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ A production-minded Layer 7 gateway in Go, designed around protocol correctness, -> **Project state: M0 — repository, contracts, and CI foundation.** The current code has process lifecycle scaffolding and generated Protobuf bindings. It does **not** yet accept network traffic, route requests, store configuration, or publish runtime configuration. This README separates the intended system from what runs today. +> **Project state: M1 complete — static HTTP proxy and cancellation.** The data plane forwards traffic to one configured upstream; M1 tests cover forwarding semantics, informational responses, cancellation cleanup, streaming memory, hop-by-hop headers, and graceful drain. Dynamic routing, configuration storage, and publication are not implemented yet. [What exists](#what-works-today) · [Run checks](#try-the-current-foundation) · [Target architecture](#target-architecture) · [Roadmap](#roadmap) · [Docs](#repository-map) @@ -33,13 +33,14 @@ Its eventual boundary is deliberately split: The current milestone is intentionally small. The repository provides: - separate Go command entry points for `gatehouse-control` and `gatehouse-data`; -- SIGINT/SIGTERM handling, root cancellation, and bounded shutdown scaffolding; +- a static HTTP reverse proxy in `gatehouse-data`, with bounded server defaults and graceful shutdown; +- SIGINT/SIGTERM handling, root cancellation, and bounded shutdown coordination; - a `gatehouse.v1` Protobuf workspace with committed Go and TypeScript generated output; - Buf linting, compatibility checks, and generated-code drift checks; - an executable compatibility fixture that proves Buf rejects a removed field and accepts a compatible addition; - separate CI jobs for contracts, static analysis, unit tests, race tests, and vulnerability scanning. -The command binaries currently wait for a shutdown signal. They do not open listeners or implement gateway behavior yet. The Protobuf package is a contract workspace, not a completed application API. +The data-plane binary listens on `127.0.0.1:8080` and forwards every request to `http://127.0.0.1:8081` by default. Set `GATEHOUSE_LISTEN_ADDR` and `GATEHOUSE_UPSTREAM_URL` to override these in-process static settings. The control-plane binary remains a lifecycle scaffold. The Protobuf package is a contract workspace, not a completed application API. ## Target architecture @@ -86,13 +87,15 @@ make verify `make verify` runs formatting checks, `go vet`, Staticcheck, Buf lint and breaking checks, the negative compatibility fixture, generated-code drift detection, module verification, unit tests, race tests, and `govulncheck`. -### Run the lifecycle scaffold +### Run the static data-plane proxy ```bash +GATEHOUSE_LISTEN_ADDR=127.0.0.1:8080 \ +GATEHOUSE_UPSTREAM_URL=http://127.0.0.1:8081 \ go run ./cmd/gatehouse-data ``` -The process waits for SIGINT (`Ctrl+C`) or SIGTERM, then follows the lifecycle shutdown path. It does not bind a port or proxy requests yet. The control-plane scaffold can be started with `go run ./cmd/gatehouse-control` and behaves the same way at this milestone. +The proxy preserves the request method, path, end-to-end headers, and body; forwards response status, end-to-end headers, and body; strips hop-by-hop headers; propagates request cancellation; and streams response chunks without waiting for the full upstream body. It is a single static upstream, with no route matching or retries. SIGINT (`Ctrl+C`) and SIGTERM stop new requests and drain in-flight requests within the shutdown timeout. The control-plane scaffold can be started with `go run ./cmd/gatehouse-control` and currently waits for a shutdown signal. ## The proof bar diff --git a/cmd/gatehouse-data/main.go b/cmd/gatehouse-data/main.go index e1e07c7..b3a5172 100644 --- a/cmd/gatehouse-data/main.go +++ b/cmd/gatehouse-data/main.go @@ -3,9 +3,13 @@ package main import ( "context" "fmt" + "log/slog" "os" + "os/signal" + "syscall" "time" + "github.com/SShogun/GateHouse/internal/dataplane" "github.com/SShogun/GateHouse/internal/lifecycle" ) @@ -19,14 +23,32 @@ func main() { } func run() error { - return lifecycle.RunWithSignals(shutdownTimeout, - func(ctx context.Context) error { - <-ctx.Done() - return nil - }, - func(context.Context) error { - // The data plane owns no listener or upstream pool until later milestones. - return nil - }, - ) + signalCtx, stopSignals := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stopSignals() + + logger := slog.New(slog.NewJSONHandler(os.Stdout, nil)) + proxy, err := dataplane.NewProxy(dataplane.Config{ + UpstreamURL: envOrDefault("GATEHOUSE_UPSTREAM_URL", "http://127.0.0.1:8081"), + }, logger) + if err != nil { + return fmt.Errorf("configure data-plane proxy: %w", err) + } + server, err := dataplane.NewServer(proxy, dataplane.ServerConfig{ + Address: envOrDefault("GATEHOUSE_LISTEN_ADDR", "127.0.0.1:8080"), + }) + if err != nil { + return fmt.Errorf("configure data-plane listener: %w", err) + } + if err := server.Start(); err != nil { + return err + } + logger.Info("data plane listening", "address", server.Addr(), "upstream", envOrDefault("GATEHOUSE_UPSTREAM_URL", "http://127.0.0.1:8081")) + return lifecycle.Run(signalCtx, shutdownTimeout, server.Wait, server.Shutdown) +} + +func envOrDefault(key, fallback string) string { + if value := os.Getenv(key); value != "" { + return value + } + return fallback } diff --git a/docs/Gatehouse_PLAN.md b/docs/Gatehouse_PLAN.md index a4cbcb1..d30847e 100644 --- a/docs/Gatehouse_PLAN.md +++ b/docs/Gatehouse_PLAN.md @@ -224,13 +224,14 @@ Do not add retries yet. ## Gate M1 ```text -[ ] HTTP proxy semantic test GREEN -[ ] cancel reaches upstream GREEN -[ ] streaming-first-byte test GREEN -[ ] huge streamed response does not scale memory with response size -[ ] hop-by-hop header tests GREEN -[ ] graceful shutdown unary request test GREEN -[ ] race + leak checks GREEN +[x] HTTP proxy semantic test GREEN +[x] cancel reaches upstream GREEN +[x] streaming-first-byte test GREEN +[x] huge streamed response does not scale memory with response size (sampled peak HeapAlloc: 8 MiB → 128 MiB body, growth under 24 MiB) +[x] hop-by-hop header tests GREEN +[x] structured failure outcome logged when an upstream response aborts +[x] graceful shutdown in-flight request test GREEN +[x] race + leak checks GREEN (16 concurrent cancellations; active count 16→0 and goroutines return within baseline +4) ``` --- diff --git a/internal/dataplane/proxy.go b/internal/dataplane/proxy.go new file mode 100644 index 0000000..823cf9d --- /dev/null +++ b/internal/dataplane/proxy.go @@ -0,0 +1,160 @@ +// Package dataplane contains the static data-plane HTTP proxy used by M1. +package dataplane + +import ( + "errors" + "fmt" + "log/slog" + "net/http" + "net/http/httputil" + "net/textproto" + "net/url" + "strings" + "sync/atomic" + "time" +) + +// Config describes the single static upstream used by the M1 data plane. +type Config struct { + UpstreamURL string +} + +// Proxy forwards all requests to one configured upstream without buffering +// response bodies. It is intentionally not a route or cluster abstraction. +type Proxy struct { + upstream *httputil.ReverseProxy + transport *http.Transport + logger *slog.Logger + active atomic.Int64 +} + +// NewProxy creates a reverse proxy for a static HTTP or HTTPS upstream. +func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { + target, err := url.Parse(cfg.UpstreamURL) + if err != nil { + return nil, fmt.Errorf("parse upstream URL: %w", err) + } + if (target.Scheme != "http" && target.Scheme != "https") || target.Host == "" { + return nil, errors.New("upstream URL must be an absolute http or https URL") + } + if target.User != nil || target.RawQuery != "" || target.Fragment != "" || (target.Path != "" && target.Path != "/") { + return nil, errors.New("upstream URL must contain only a scheme and host") + } + if logger == nil { + logger = slog.Default() + } + + transport := http.DefaultTransport.(*http.Transport).Clone() + p := &Proxy{logger: logger, transport: transport} + p.upstream = &httputil.ReverseProxy{ + Rewrite: func(request *httputil.ProxyRequest) { + request.SetURL(target) + request.SetXForwarded() + }, + FlushInterval: -1, + Transport: transport, + ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { + p.logger.Warn("upstream request failed", "method", r.Method, "error", err) + http.Error(w, "bad gateway", http.StatusBadGateway) + }, + } + return p, nil +} + +// ActiveRequests reports the number of requests currently being proxied. +func (p *Proxy) ActiveRequests() int64 { return p.active.Load() } + +// CloseIdleConnections releases pooled upstream connections after shutdown. +func (p *Proxy) CloseIdleConnections() { p.transport.CloseIdleConnections() } + +// ServeHTTP proxies one request and records a bounded structured outcome. +func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { + p.active.Add(1) + defer p.active.Add(-1) + started := time.Now() + recorder := &statusRecorder{ResponseWriter: w} + defer func() { + if recovered := recover(); recovered != nil { + status := recorder.status + if status == 0 { + status = http.StatusOK + } + p.logger.Warn("proxy request aborted", + "method", r.Method, + "status", status, + "outcome", "failure", + "error", fmt.Sprint(recovered), + "duration", time.Since(started), + ) + panic(recovered) + } + status := recorder.status + if status == 0 { + status = http.StatusOK + } + p.logger.Info("proxy request complete", + "method", r.Method, + "status", status, + "outcome", "success", + "duration", time.Since(started), + ) + }() + p.upstream.ServeHTTP(recorder, r) +} + +type statusRecorder struct { + http.ResponseWriter + status int +} + +func (w *statusRecorder) Unwrap() http.ResponseWriter { return w.ResponseWriter } + +func (w *statusRecorder) WriteHeader(status int) { + if w.status != 0 { + return + } + if status >= 100 && status < 200 && status != http.StatusSwitchingProtocols { + stripHopByHopHeaders(w.Header()) + w.ResponseWriter.WriteHeader(status) + return + } + w.status = status + w.ResponseWriter.WriteHeader(status) +} + +func stripHopByHopHeaders(header http.Header) { + for _, value := range header.Values("Connection") { + for _, field := range strings.Split(value, ",") { + if field = textproto.TrimString(field); field != "" { + header.Del(field) + } + } + } + for _, field := range [...]string{ + "Connection", + "Proxy-Connection", + "Keep-Alive", + "Proxy-Authenticate", + "Proxy-Authorization", + "Te", + "Trailer", + "Transfer-Encoding", + "Upgrade", + } { + header.Del(field) + } +} + +func (w *statusRecorder) Write(body []byte) (int, error) { + if w.status == 0 { + w.WriteHeader(http.StatusOK) + } + return w.ResponseWriter.Write(body) +} + +func (w *statusRecorder) Flush() { + if w.status == 0 { + w.WriteHeader(http.StatusOK) + } + _ = http.NewResponseController(w.ResponseWriter).Flush() +} diff --git a/internal/dataplane/proxy_test.go b/internal/dataplane/proxy_test.go new file mode 100644 index 0000000..bf75072 --- /dev/null +++ b/internal/dataplane/proxy_test.go @@ -0,0 +1,538 @@ +package dataplane + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/http/httptrace" + "net/textproto" + "runtime" + "strings" + "sync" + "testing" + "time" +) + +func TestProxyForwardsRequestAndResponse(t *testing.T) { + type receivedRequest struct { + method string + path string + header string + body string + } + gotRequest := make(chan receivedRequest, 1) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read upstream request body: %v", err) + } + gotRequest <- receivedRequest{ + method: r.Method, + path: r.URL.RequestURI(), + header: r.Header.Get("X-Request-ID"), + body: string(body), + } + w.Header().Set("X-Upstream", "present") + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, "upstream response") + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + req, err := http.NewRequest(http.MethodPatch, gateway.URL+"/widgets/42?view=full", strings.NewReader("payload")) + if err != nil { + t.Fatal(err) + } + req.Header.Set("X-Request-ID", "request-123") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + + if resp.StatusCode != http.StatusCreated { + t.Errorf("response status = %d, want %d", resp.StatusCode, http.StatusCreated) + } + if got := resp.Header.Get("X-Upstream"); got != "present" { + t.Errorf("response X-Upstream = %q, want present", got) + } + if got := string(body); got != "upstream response" { + t.Errorf("response body = %q, want upstream response", got) + } + + got := <-gotRequest + if got.method != http.MethodPatch || got.path != "/widgets/42?view=full" || got.header != "request-123" || got.body != "payload" { + t.Errorf("upstream request = %+v, want PATCH /widgets/42?view=full, X-Request-ID request-123, body payload", got) + } +} + +func TestProxyAbortLogsFailureOutcome(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "100") + _, _ = io.WriteString(w, "x") + })) + defer upstream.Close() + + var logs bytes.Buffer + logger := slog.New(slog.NewJSONHandler(&logs, nil)) + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, logger) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + resp, requestErr := http.Get(gateway.URL + "/truncated") + if resp != nil { + _, requestErr = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + } + if err := waitForActiveRequests(proxy, 0, 2*time.Second); err != nil { + t.Fatal(err) + } + if requestErr == nil { + t.Fatal("client read a complete response despite upstream truncation") + } + + var record map[string]any + if err := json.Unmarshal(logs.Bytes(), &record); err != nil { + t.Fatalf("decode structured request outcome %q: %v", logs.String(), err) + } + if record["outcome"] != "failure" && record["outcome"] != "abort" { + t.Fatalf("structured request outcome = %v, want failure or abort; log = %s", record["outcome"], logs.String()) + } +} + +func TestProxyForwardsInformationalAndFinalResponses(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Link", "; rel=preload") + w.Header().Set("Connection", "X-Interim-Hop, Keep-Alive") + w.Header().Set("X-Interim-Hop", "must-not-reach-client") + w.Header().Set("Keep-Alive", "timeout=5") + w.WriteHeader(http.StatusEarlyHints) + w.Header().Set("X-Final", "present") + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, "created") + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + type interimResponse struct { + link string + connection string + hopHeader string + keepAlive string + } + earlyHints := make(chan interimResponse, 1) + trace := &httptrace.ClientTrace{ + Got1xxResponse: func(code int, header textproto.MIMEHeader) error { + if code == http.StatusEarlyHints { + earlyHints <- interimResponse{ + link: header.Get("Link"), + connection: header.Get("Connection"), + hopHeader: header.Get("X-Interim-Hop"), + keepAlive: header.Get("Keep-Alive"), + } + } + return nil + }, + } + ctx := httptrace.WithClientTrace(context.Background(), trace) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, gateway.URL+"/early-hints", nil) + if err != nil { + t.Fatal(err) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusCreated { + t.Errorf("final status = %d, want %d", resp.StatusCode, http.StatusCreated) + } + if got := resp.Header.Get("X-Final"); got != "present" { + t.Errorf("final X-Final header = %q, want present", got) + } + if string(body) != "created" { + t.Errorf("final response body = %q, want created", body) + } + select { + case got := <-earlyHints: + if got.link != "; rel=preload" { + t.Errorf("103 Link header = %q", got.link) + } + if got.connection != "" || got.hopHeader != "" || got.keepAlive != "" { + t.Errorf("103 contains hop-by-hop headers: %+v", got) + } + case <-time.After(2 * time.Second): + t.Error("client did not receive upstream 103 Early Hints") + } +} + +func TestProxyPropagatesClientCancellation(t *testing.T) { + started := make(chan struct{}) + canceled := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(started) + <-r.Context().Done() + close(canceled) + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, gateway.URL+"/blocked", nil) + if err != nil { + t.Fatal(err) + } + clientDone := make(chan struct{}) + go func() { + defer close(clientDone) + resp, err := http.DefaultClient.Do(req) + if err == nil { + _ = resp.Body.Close() + } + }() + + select { + case <-started: + case <-time.After(2 * time.Second): + t.Fatal("upstream did not receive request") + } + cancel() + select { + case <-canceled: + case <-time.After(2 * time.Second): + t.Fatal("upstream request context was not canceled") + } + select { + case <-clientDone: + case <-time.After(2 * time.Second): + t.Fatal("client request did not finish after cancellation") + } + if err := waitForActiveRequests(proxy, 0, 2*time.Second); err != nil { + t.Error(err) + } +} + +func TestConcurrentCancellationsLeaveNoActiveRequestLeak(t *testing.T) { + const requestCount = 16 + baselineGoroutines := runtime.NumGoroutine() + started := make(chan struct{}, requestCount) + canceled := make(chan struct{}, requestCount) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + started <- struct{}{} + <-r.Context().Done() + canceled <- struct{}{} + })) + upstreamClosed := false + defer func() { + if !upstreamClosed { + upstream.Close() + } + }() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + gatewayClosed := false + defer func() { + if !gatewayClosed { + gateway.Close() + } + }() + clientTransport := &http.Transport{} + client := &http.Client{Transport: clientTransport} + defer clientTransport.CloseIdleConnections() + defer proxy.CloseIdleConnections() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + clientResults := make(chan struct{}, requestCount) + for range requestCount { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, gateway.URL+"/cancel", nil) + if err != nil { + t.Fatal(err) + } + go func() { + resp, err := client.Do(req) + if err == nil { + _ = resp.Body.Close() + } + clientResults <- struct{}{} + }() + } + for range requestCount { + select { + case <-started: + case <-time.After(2 * time.Second): + t.Fatal("not all concurrent requests reached upstream") + } + } + if err := waitForActiveRequests(proxy, requestCount, 2*time.Second); err != nil { + t.Fatal(err) + } + cancel() + for range requestCount { + select { + case <-canceled: + case <-time.After(2 * time.Second): + t.Fatal("not all upstream requests observed cancellation") + } + select { + case <-clientResults: + case <-time.After(2 * time.Second): + t.Fatal("not all canceled client requests finished") + } + } + if err := waitForActiveRequests(proxy, 0, 2*time.Second); err != nil { + t.Fatal(err) + } + clientTransport.CloseIdleConnections() + proxy.CloseIdleConnections() + gateway.Close() + gatewayClosed = true + upstream.Close() + upstreamClosed = true + if err := waitForGoroutinesAtMost(baselineGoroutines+4, 3*time.Second); err != nil { + t.Fatal(err) + } +} + +func TestProxyStreamsBeforeUpstreamCompletes(t *testing.T) { + upstreamDone := make(chan struct{}) + allowFinish := make(chan struct{}) + var finishOnce sync.Once + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, "first chunk\n") + w.(http.Flusher).Flush() + <-allowFinish + _, _ = io.WriteString(w, "last chunk\n") + close(upstreamDone) + })) + defer upstream.Close() + defer finishOnce.Do(func() { close(allowFinish) }) + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + resp, err := http.Get(gateway.URL + "/stream") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + line, err := readLine(resp.Body) + if err != nil { + t.Fatalf("read first streamed chunk: %v", err) + } + if line != "first chunk\n" { + t.Fatalf("first streamed chunk = %q, want %q", line, "first chunk\n") + } + select { + case <-upstreamDone: + t.Fatal("upstream finished before the first chunk was observed") + default: + } + finishOnce.Do(func() { close(allowFinish) }) + _, _ = io.Copy(io.Discard, resp.Body) +} + +func TestProxyLargeResponsePeakHeapDoesNotScaleWithBodySize(t *testing.T) { + const chunkSize = 32 << 10 + chunk := strings.Repeat("x", chunkSize) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + size := int64(8 << 20) + if r.URL.Path == "/large" { + size = 128 << 20 + } + for sent := int64(0); sent < size; sent += chunkSize { + if _, err := io.WriteString(w, chunk); err != nil { + return + } + } + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + measurePeak := func(path string, wantBytes int64) uint64 { + t.Helper() + runtime.GC() + var before runtime.MemStats + runtime.ReadMemStats(&before) + peak := before.HeapAlloc + stop := make(chan struct{}) + samplerStarted := make(chan struct{}) + samplerDone := make(chan struct{}) + go func() { + defer close(samplerDone) + ticker := time.NewTicker(time.Millisecond) + defer ticker.Stop() + close(samplerStarted) + for { + select { + case <-stop: + return + case <-ticker.C: + var current runtime.MemStats + runtime.ReadMemStats(¤t) + if current.HeapAlloc > peak { + peak = current.HeapAlloc + } + } + } + }() + <-samplerStarted + + resp, err := http.Get(gateway.URL + path) + if err != nil { + close(stop) + <-samplerDone + t.Fatal(err) + } + count, copyErr := io.Copy(io.Discard, resp.Body) + closeErr := resp.Body.Close() + close(stop) + <-samplerDone + if copyErr != nil { + t.Fatal(copyErr) + } + if closeErr != nil { + t.Fatal(closeErr) + } + if count != wantBytes { + t.Fatalf("streamed bytes for %s = %d, want %d", path, count, wantBytes) + } + var after runtime.MemStats + runtime.ReadMemStats(&after) + if after.HeapAlloc > peak { + peak = after.HeapAlloc + } + return peak - before.HeapAlloc + } + + smallPeak := measurePeak("/small", 8<<20) + largePeak := measurePeak("/large", 128<<20) + const allowedGrowth = 24 << 20 + t.Logf("peak heap above baseline: 8 MiB body=%d bytes, 128 MiB body=%d bytes", smallPeak, largePeak) + if largePeak > smallPeak+allowedGrowth { + t.Fatalf("peak heap grew from %d bytes for 8 MiB to %d bytes for 128 MiB; growth exceeds %d-byte allowance", smallPeak, largePeak, allowedGrowth) + } +} + +func TestProxyStripsHopByHopRequestHeaders(t *testing.T) { + gotConnectionTokenHeader := make(chan string, 1) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotConnectionTokenHeader <- r.Header.Get("X-Hop-By-Hop") + w.Header().Set("Connection", "X-Upstream-Hop") + w.Header().Set("X-Upstream-Hop", "must-not-reach-client") + w.WriteHeader(http.StatusNoContent) + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + req, err := http.NewRequest(http.MethodGet, gateway.URL+"/headers", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Connection", "X-Hop-By-Hop") + req.Header.Set("X-Hop-By-Hop", "must-not-forward") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if got := <-gotConnectionTokenHeader; got != "" { + t.Errorf("upstream received hop-by-hop header value %q", got) + } + if got := resp.Header.Get("X-Upstream-Hop"); got != "" { + t.Errorf("client received hop-by-hop response header value %q", got) + } +} + +func readLine(r io.Reader) (string, error) { + var line strings.Builder + var one [1]byte + for { + if _, err := r.Read(one[:]); err != nil { + return line.String(), err + } + line.WriteByte(one[0]) + if one[0] == '\n' { + return line.String(), nil + } + } +} + +func waitForActiveRequests(proxy *Proxy, want int64, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for proxy.ActiveRequests() != want && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if got := proxy.ActiveRequests(); got != want { + return fmt.Errorf("active requests = %d, want %d", got, want) + } + return nil +} + +func waitForGoroutinesAtMost(limit int, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for { + runtime.GC() + if got := runtime.NumGoroutine(); got <= limit { + return nil + } else if time.Now().After(deadline) { + return fmt.Errorf("goroutines after cleanup = %d, want at most %d", got, limit) + } + time.Sleep(25 * time.Millisecond) + } +} diff --git a/internal/dataplane/server.go b/internal/dataplane/server.go new file mode 100644 index 0000000..31e133b --- /dev/null +++ b/internal/dataplane/server.go @@ -0,0 +1,139 @@ +package dataplane + +import ( + "context" + "errors" + "fmt" + "net" + "net/http" + "sync" + "time" +) + +const ( + defaultReadHeaderTimeout = 5 * time.Second + defaultIdleTimeout = 90 * time.Second + defaultMaxHeaderBytes = 1 << 20 +) + +// ServerConfig contains listener and HTTP server resource limits. WriteTimeout +// is intentionally omitted because it would break long-lived streamed bodies. +type ServerConfig struct { + Address string + ReadHeaderTimeout time.Duration + IdleTimeout time.Duration + MaxHeaderBytes int +} + +// Server owns the listener and drains active HTTP requests on shutdown. +type Server struct { + mu sync.Mutex + handler http.Handler + cfg ServerConfig + server *http.Server + listener net.Listener + serveErr chan error + started bool +} + +// NewServer validates and configures a server but does not bind its address. +func NewServer(handler http.Handler, cfg ServerConfig) (*Server, error) { + if handler == nil { + return nil, errors.New("HTTP handler is nil") + } + if cfg.Address == "" { + return nil, errors.New("server address is required") + } + if cfg.ReadHeaderTimeout <= 0 { + cfg.ReadHeaderTimeout = defaultReadHeaderTimeout + } + if cfg.IdleTimeout <= 0 { + cfg.IdleTimeout = defaultIdleTimeout + } + if cfg.MaxHeaderBytes <= 0 { + cfg.MaxHeaderBytes = defaultMaxHeaderBytes + } + return &Server{handler: handler, cfg: cfg}, nil +} + +// Start binds the configured listener and starts accepting HTTP requests. +func (s *Server) Start() error { + s.mu.Lock() + defer s.mu.Unlock() + if s.started { + return errors.New("HTTP server already started") + } + listener, err := net.Listen("tcp", s.cfg.Address) + if err != nil { + return fmt.Errorf("listen on %s: %w", s.cfg.Address, err) + } + s.listener = listener + s.server = &http.Server{ + Handler: s.handler, + ReadHeaderTimeout: s.cfg.ReadHeaderTimeout, + IdleTimeout: s.cfg.IdleTimeout, + MaxHeaderBytes: s.cfg.MaxHeaderBytes, + } + s.serveErr = make(chan error, 1) + s.started = true + go func(server *http.Server, listener net.Listener, result chan<- error) { + result <- server.Serve(listener) + }(s.server, listener, s.serveErr) + return nil +} + +// Addr returns the actual listener address after Start, including an assigned +// port when Address used port zero. +func (s *Server) Addr() string { + s.mu.Lock() + defer s.mu.Unlock() + if s.listener == nil { + return "" + } + return s.listener.Addr().String() +} + +// Wait returns when the server stops or ctx is canceled. A normal HTTP server +// shutdown is reported as nil. +func (s *Server) Wait(ctx context.Context) error { + if ctx == nil { + return errors.New("wait context is nil") + } + s.mu.Lock() + started, result := s.started, s.serveErr + s.mu.Unlock() + if !started { + return errors.New("HTTP server has not started") + } + select { + case err := <-result: + if errors.Is(err, http.ErrServerClosed) { + return nil + } + return err + case <-ctx.Done(): + return nil + } +} + +// Shutdown stops accepting new requests and waits for active requests to +// finish within ctx. If the deadline expires, it forcibly closes connections. +func (s *Server) Shutdown(ctx context.Context) error { + if ctx == nil { + return errors.New("shutdown context is nil") + } + s.mu.Lock() + server, started := s.server, s.started + s.mu.Unlock() + if !started { + return errors.New("HTTP server has not started") + } + err := server.Shutdown(ctx) + if err != nil { + err = errors.Join(err, server.Close()) + } + if closer, ok := s.handler.(interface{ CloseIdleConnections() }); ok { + closer.CloseIdleConnections() + } + return err +} diff --git a/internal/dataplane/server_test.go b/internal/dataplane/server_test.go new file mode 100644 index 0000000..c9e897a --- /dev/null +++ b/internal/dataplane/server_test.go @@ -0,0 +1,104 @@ +package dataplane + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" +) + +func TestServerGracefullyDrainsInFlightRequest(t *testing.T) { + upstreamStarted := make(chan struct{}) + releaseUpstream := make(chan struct{}) + var releaseOnce sync.Once + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(upstreamStarted) + <-releaseUpstream + _, _ = io.WriteString(w, "drained response") + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + server, err := NewServer(proxy, ServerConfig{Address: "127.0.0.1:0"}) + if err != nil { + t.Fatal(err) + } + if err := server.Start(); err != nil { + t.Fatal(err) + } + defer func() { + releaseOnce.Do(func() { close(releaseUpstream) }) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _ = server.Shutdown(ctx) + }() + + clientResult := make(chan error, 1) + go func() { + resp, err := http.Get("http://" + server.Addr() + "/drain") + if err != nil { + clientResult <- err + return + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err == nil && string(body) != "drained response" { + err = fmt.Errorf("response body = %q, want drained response", body) + } + clientResult <- err + }() + + select { + case <-upstreamStarted: + case <-time.After(2 * time.Second): + t.Fatal("upstream did not receive in-flight request") + } + shutdownResult := make(chan error, 1) + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + shutdownResult <- server.Shutdown(ctx) + }() + select { + case err := <-shutdownResult: + t.Fatalf("server shutdown returned before in-flight request drained: %v", err) + case <-time.After(25 * time.Millisecond): + } + releaseOnce.Do(func() { close(releaseUpstream) }) + + select { + case err := <-clientResult: + if err != nil { + t.Fatalf("in-flight client request failed: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("in-flight client request did not finish") + } + select { + case err := <-shutdownResult: + if err != nil { + t.Fatalf("server shutdown failed: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("server did not finish graceful shutdown") + } +} + +func TestServerConfigRejectsMissingAddress(t *testing.T) { + _, err := NewProxy(Config{UpstreamURL: "http://127.0.0.1:1"}, nil) + if err != nil { + t.Fatal(err) + } + _, err = NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), ServerConfig{}) + if err == nil || !strings.Contains(err.Error(), "address") { + t.Fatalf("NewServer() error = %v, want missing address error", err) + } +} From 8459ceca17496224f222cfb515e84b0338af3653 Mon Sep 17 00:00:00 2001 From: SShogun Date: Sat, 10 Oct 2026 19:56:52 +0530 Subject: [PATCH 2/6] feat(router): add deterministic routing and cluster health Add immutable route compilation with precedence, ambiguity checks, and fuzz coverage. Add endpoint selection, health thresholds, and cancellable probe scheduling. This is a partial M2 checkpoint; listener configuration, telemetry integration, scheduler jitter, and remaining M2 gates are not included. --- internal/cluster/cluster.go | 186 ++++++++++++++ internal/cluster/cluster_test.go | 152 ++++++++++++ internal/cluster/scheduler.go | 122 +++++++++ internal/cluster/scheduler_test.go | 62 +++++ internal/router/router.go | 382 +++++++++++++++++++++++++++++ internal/router/router_test.go | 241 ++++++++++++++++++ 6 files changed, 1145 insertions(+) create mode 100644 internal/cluster/cluster.go create mode 100644 internal/cluster/cluster_test.go create mode 100644 internal/cluster/scheduler.go create mode 100644 internal/cluster/scheduler_test.go create mode 100644 internal/router/router.go create mode 100644 internal/router/router_test.go diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go new file mode 100644 index 0000000..a075b89 --- /dev/null +++ b/internal/cluster/cluster.go @@ -0,0 +1,186 @@ +// Package cluster provides endpoint health tracking and deterministic selection. +package cluster + +import ( + "errors" + "fmt" + "sync" +) + +var ( + ErrEmptyCluster = errors.New("cluster must contain at least one endpoint") + ErrNoHealthyEndpoints = errors.New("cluster has no healthy enabled endpoints") + ErrUnknownEndpoint = errors.New("unknown endpoint") + ErrInvalidConfig = errors.New("invalid cluster configuration") + ErrSelectionSource = errors.New("weighted selection requires a source") +) + +// Policy identifies how an eligible endpoint is selected. +type Policy string + +const ( + RoundRobin Policy = "round_robin" + Weighted Policy = "weighted_round_robin" +) + +// HealthState is the local runtime state of an endpoint. Configured endpoints +// start Unknown; selection accepts Unknown until the unhealthy threshold is crossed. +type HealthState string + +const ( + Unknown HealthState = "unknown" + Healthy HealthState = "healthy" + Unhealthy HealthState = "unhealthy" +) + +// Endpoint is immutable cluster configuration; health is maintained separately. +type Endpoint struct { + ID string + Address string + Weight uint32 + Disabled bool +} + +// HealthThresholds define consecutive probe results required to change state. +type HealthThresholds struct { + Healthy uint32 + Unhealthy uint32 +} + +// Config is validated once at construction. Endpoint configuration is copied. +type Config struct { + Endpoints []Endpoint + Health HealthThresholds +} + +// SelectionSource supplies a deterministic draw in [0,n) for weighted selection. +type SelectionSource interface { + Uint64n(n uint64) uint64 +} + +type endpointHealth struct { + state HealthState + successes uint32 + failures uint32 +} + +// Cluster owns mutable health and selection cursor state for one config snapshot. +type Cluster struct { + mu sync.Mutex + endpoints []Endpoint + health map[string]*endpointHealth + thresholds HealthThresholds + cursor uint64 +} + +// New validates and copies endpoint configuration. Zero health thresholds default to 1. +func New(cfg Config) (*Cluster, error) { + if len(cfg.Endpoints) == 0 { + return nil, ErrEmptyCluster + } + if cfg.Health.Healthy == 0 { + cfg.Health.Healthy = 1 + } + if cfg.Health.Unhealthy == 0 { + cfg.Health.Unhealthy = 1 + } + seen := make(map[string]struct{}, len(cfg.Endpoints)) + endpoints := append([]Endpoint(nil), cfg.Endpoints...) + for _, ep := range endpoints { + if ep.ID == "" || ep.Weight == 0 { + return nil, fmt.Errorf("%w: endpoint ID and positive weight are required", ErrInvalidConfig) + } + if _, ok := seen[ep.ID]; ok { + return nil, fmt.Errorf("%w: duplicate endpoint ID %q", ErrInvalidConfig, ep.ID) + } + seen[ep.ID] = struct{}{} + } + health := make(map[string]*endpointHealth, len(endpoints)) + for _, ep := range endpoints { + health[ep.ID] = &endpointHealth{state: Unknown} + } + return &Cluster{endpoints: endpoints, health: health, thresholds: cfg.Health}, nil +} + +// Select returns one enabled endpoint that is not known Unhealthy. Unknown +// endpoints remain eligible until the unhealthy threshold is crossed. If all +// endpoints are unavailable, it returns ErrNoHealthyEndpoints. Weighted +// selection draws proportionally to configured positive weights using source; +// the source is consulted only for that policy. +func (c *Cluster) Select(policy Policy, source SelectionSource) (Endpoint, error) { + c.mu.Lock() + defer c.mu.Unlock() + eligible := make([]Endpoint, 0, len(c.endpoints)) + for _, ep := range c.endpoints { + if !ep.Disabled && c.health[ep.ID].state != Unhealthy { + eligible = append(eligible, ep) + } + } + if len(eligible) == 0 { + return Endpoint{}, ErrNoHealthyEndpoints + } + switch policy { + case RoundRobin: + ep := eligible[c.cursor%uint64(len(eligible))] + c.cursor++ + return ep, nil + case Weighted: + if source == nil { + return Endpoint{}, ErrSelectionSource + } + var total uint64 + for _, ep := range eligible { + total += uint64(ep.Weight) + } + draw := source.Uint64n(total) % total + for _, ep := range eligible { + weight := uint64(ep.Weight) + if draw < weight { + return ep, nil + } + draw -= weight + } + panic("unreachable weighted selection") + default: + return Endpoint{}, fmt.Errorf("%w: unknown selection policy %q", ErrInvalidConfig, policy) + } +} + +// MarkResult applies one consecutive active health-check result. +func (c *Cluster) MarkResult(id string, success bool) error { + c.mu.Lock() + defer c.mu.Unlock() + h, ok := c.health[id] + if !ok { + return fmt.Errorf("%w: %q", ErrUnknownEndpoint, id) + } + if success { + h.failures = 0 + if h.successes < c.thresholds.Healthy { + h.successes++ + } + if h.successes >= c.thresholds.Healthy { + h.state = Healthy + } + return nil + } + h.successes = 0 + if h.failures < c.thresholds.Unhealthy { + h.failures++ + } + if h.failures >= c.thresholds.Unhealthy { + h.state = Unhealthy + } + return nil +} + +// Health returns the current health state for an endpoint. +func (c *Cluster) Health(id string) (HealthState, error) { + c.mu.Lock() + defer c.mu.Unlock() + h, ok := c.health[id] + if !ok { + return "", fmt.Errorf("%w: %q", ErrUnknownEndpoint, id) + } + return h.state, nil +} diff --git a/internal/cluster/cluster_test.go b/internal/cluster/cluster_test.go new file mode 100644 index 0000000..1a929e1 --- /dev/null +++ b/internal/cluster/cluster_test.go @@ -0,0 +1,152 @@ +package cluster + +import ( + "errors" + "testing" +) + +type sequenceSource struct { + values []uint64 + index int +} + +func (s *sequenceSource) Uint64n(n uint64) uint64 { + v := s.values[s.index] % n + s.index++ + return v +} + +func TestNewRejectsEmptyCluster(t *testing.T) { + if _, err := New(Config{}); !errors.Is(err, ErrEmptyCluster) { + t.Fatalf("New(Config{}) error = %v, want ErrEmptyCluster", err) + } +} + +func TestRoundRobinCyclesEnabledUnknownEndpoints(t *testing.T) { + c, err := New(Config{Endpoints: []Endpoint{ + {ID: "a", Weight: 1}, + {ID: "disabled", Weight: 1, Disabled: true}, + {ID: "b", Weight: 1}, + }}) + if err != nil { + t.Fatal(err) + } + got := []string{} + for range 4 { + ep, err := c.Select(RoundRobin, nil) + if err != nil { + t.Fatal(err) + } + got = append(got, ep.ID) + } + want := []string{"a", "b", "a", "b"} + for i := range want { + if got[i] != want[i] { + t.Fatalf("selection %v, want %v", got, want) + } + } +} + +func TestWeightedSelectionUsesConfiguredWeights(t *testing.T) { + c, err := New(Config{Endpoints: []Endpoint{ + {ID: "light", Weight: 1}, + {ID: "heavy", Weight: 3}, + }}) + if err != nil { + t.Fatal(err) + } + source := &sequenceSource{values: []uint64{0, 1, 2, 3}} + got := make([]string, 4) + for i := range got { + ep, err := c.Select(Weighted, source) + if err != nil { + t.Fatal(err) + } + got[i] = ep.ID + } + want := []string{"light", "heavy", "heavy", "heavy"} + for i := range want { + if got[i] != want[i] { + t.Fatalf("selection %v, want %v", got, want) + } + } +} + +func TestUnknownEndpointsRemainEligibleUntilUnhealthyThreshold(t *testing.T) { + c, err := New(Config{ + Endpoints: []Endpoint{{ID: "api", Weight: 1}}, + Health: HealthThresholds{Healthy: 2, Unhealthy: 2}, + }) + if err != nil { + t.Fatal(err) + } + state, err := c.Health("api") + if err != nil || state != Unknown { + t.Fatalf("initial health = %q, %v; want Unknown", state, err) + } + if ep, err := c.Select(RoundRobin, nil); err != nil || ep.ID != "api" { + t.Fatalf("initial selection = %q, %v; want api", ep.ID, err) + } + if err := c.MarkResult("api", false); err != nil { + t.Fatal(err) + } + if ep, err := c.Select(RoundRobin, nil); err != nil || ep.ID != "api" { + t.Fatalf("selection before unhealthy threshold = %q, %v; want api", ep.ID, err) + } + if err := c.MarkResult("api", false); err != nil { + t.Fatal(err) + } + if _, err := c.Select(RoundRobin, nil); !errors.Is(err, ErrNoHealthyEndpoints) { + t.Fatalf("selection after unhealthy threshold error = %v, want ErrNoHealthyEndpoints", err) + } +} + +func TestSelectionExcludesDisabledAndUnhealthyEndpoints(t *testing.T) { + c, err := New(Config{Endpoints: []Endpoint{ + {ID: "down", Weight: 100}, + {ID: "disabled", Weight: 100, Disabled: true}, + {ID: "up", Weight: 1}, + }}) + if err != nil { + t.Fatal(err) + } + if err := c.MarkResult("down", false); err != nil { + t.Fatal(err) + } + for _, policy := range []Policy{RoundRobin, Weighted} { + for i := 0; i < 20; i++ { + ep, err := c.Select(policy, &sequenceSource{values: []uint64{999}}) + if err != nil { + t.Fatal(err) + } + if ep.ID != "up" { + t.Fatalf("%s selected %q, want only non-unhealthy enabled endpoint up", policy, ep.ID) + } + } + } +} + +func TestHealthTransitionsRequireConfiguredThresholds(t *testing.T) { + c, err := New(Config{Endpoints: []Endpoint{{ID: "api", Weight: 1}}, + Health: HealthThresholds{Unhealthy: 2, Healthy: 2}, + }) + if err != nil { + t.Fatal(err) + } + checkHealth := func(want HealthState) { + t.Helper() + got, err := c.Health("api") + if err != nil || got != want { + t.Fatalf("health = %q, %v; want %q", got, err, want) + } + } + checkHealth(Unknown) + _ = c.MarkResult("api", false) + checkHealth(Unknown) + _ = c.MarkResult("api", false) + checkHealth(Unhealthy) + _ = c.MarkResult("api", true) + checkHealth(Unhealthy) + _ = c.MarkResult("api", true) + checkHealth(Healthy) +} diff --git a/internal/cluster/scheduler.go b/internal/cluster/scheduler.go new file mode 100644 index 0000000..3a97c20 --- /dev/null +++ b/internal/cluster/scheduler.go @@ -0,0 +1,122 @@ +package cluster + +import ( + "context" + "errors" + "sync" + "time" +) + +var ( + ErrInvalidSchedule = errors.New("schedule requires an ID, positive interval, and probe") + ErrDuplicateSchedule = errors.New("schedule already exists") + ErrUnknownSchedule = errors.New("unknown schedule") + ErrSchedulerStopped = errors.New("scheduler is stopped") +) + +// Probe performs one health check and must return promptly when ctx is canceled. +type Probe func(ctx context.Context) error + +type scheduledProbe struct { + cancel context.CancelFunc + done chan struct{} +} + +// Scheduler owns at most one periodic worker per registered ID. +type Scheduler struct { + mu sync.Mutex + entries map[string]*scheduledProbe + stopped bool + stopDone chan struct{} +} + +func NewScheduler() *Scheduler { + return &Scheduler{entries: make(map[string]*scheduledProbe), stopDone: make(chan struct{})} +} + +// Add starts an immediate probe followed by periodic probes. Duplicate IDs fail. +func (s *Scheduler) Add(id string, interval time.Duration, probe Probe) error { + if id == "" || interval <= 0 || probe == nil { + return ErrInvalidSchedule + } + s.mu.Lock() + defer s.mu.Unlock() + if s.stopped { + return ErrSchedulerStopped + } + if _, exists := s.entries[id]; exists { + return ErrDuplicateSchedule + } + ctx, cancel := context.WithCancel(context.Background()) + entry := &scheduledProbe{cancel: cancel, done: make(chan struct{})} + s.entries[id] = entry + go func() { + defer close(entry.done) + for { + _ = probe(ctx) + timer := time.NewTimer(interval) + select { + case <-ctx.Done(): + timer.Stop() + return + case <-timer.C: + } + if ctx.Err() != nil { + return + } + } + }() + return nil +} + +// Remove cancels a registered worker and waits for it to finish. +func (s *Scheduler) Remove(id string) error { + s.mu.Lock() + entry, ok := s.entries[id] + s.mu.Unlock() + if !ok { + return ErrUnknownSchedule + } + entry.cancel() + <-entry.done + s.mu.Lock() + if s.entries[id] == entry { + delete(s.entries, id) + } + s.mu.Unlock() + return nil +} + +// Count reports registered schedules, including workers that are stopping. +func (s *Scheduler) Count() int { + s.mu.Lock() + defer s.mu.Unlock() + return len(s.entries) +} + +// Stop cancels and joins every worker. It is safe to call more than once. +func (s *Scheduler) Stop() { + s.mu.Lock() + if s.stopped { + done := s.stopDone + s.mu.Unlock() + <-done + return + } + s.stopped = true + entries := make([]*scheduledProbe, 0, len(s.entries)) + for _, entry := range s.entries { + entries = append(entries, entry) + } + s.mu.Unlock() + for _, entry := range entries { + entry.cancel() + } + for _, entry := range entries { + <-entry.done + } + s.mu.Lock() + clear(s.entries) + close(s.stopDone) + s.mu.Unlock() +} diff --git a/internal/cluster/scheduler_test.go b/internal/cluster/scheduler_test.go new file mode 100644 index 0000000..2687add --- /dev/null +++ b/internal/cluster/scheduler_test.go @@ -0,0 +1,62 @@ +package cluster + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestSchedulerRemoveCancelsAndJoinsProbe(t *testing.T) { + started := make(chan struct{}) + finished := make(chan struct{}) + scheduler := NewScheduler() + if err := scheduler.Add("api", time.Hour, func(ctx context.Context) error { + close(started) + <-ctx.Done() + close(finished) + return ctx.Err() + }); err != nil { + t.Fatal(err) + } + <-started + if err := scheduler.Remove("api"); err != nil { + t.Fatal(err) + } + select { + case <-finished: + default: + t.Fatal("Remove returned before the probe stopped") + } + if got := scheduler.Count(); got != 0 { + t.Fatalf("scheduler count = %d, want 0", got) + } + if err := scheduler.Remove("api"); !errors.Is(err, ErrUnknownSchedule) { + t.Fatalf("second Remove error = %v, want ErrUnknownSchedule", err) + } +} + +func TestSchedulerStopCancelsAllProbes(t *testing.T) { + started := make(chan struct{}, 2) + finished := make(chan struct{}, 2) + scheduler := NewScheduler() + for _, id := range []string{"one", "two"} { + if err := scheduler.Add(id, time.Hour, func(ctx context.Context) error { + started <- struct{}{} + <-ctx.Done() + finished <- struct{}{} + return ctx.Err() + }); err != nil { + t.Fatal(err) + } + } + <-started + <-started + scheduler.Stop() + if got := scheduler.Count(); got != 0 { + t.Fatalf("scheduler count = %d, want 0", got) + } + if len(finished) != 2 { + t.Fatalf("finished probes = %d, want 2", len(finished)) + } +} diff --git a/internal/router/router.go b/internal/router/router.go new file mode 100644 index 0000000..e1b1b3e --- /dev/null +++ b/internal/router/router.go @@ -0,0 +1,382 @@ +// Package router compiles immutable route configuration into a deterministic matcher. +package router + +import ( + "errors" + "fmt" + "net" + "net/http" + "sort" + "strings" +) + +// PathType describes whether a route path is an exact path or a byte prefix. +type PathType uint8 + +const ( + PathExact PathType = iota + 1 + PathPrefix +) + +// Route is one immutable routing rule. Host may be empty (any host), an exact +// hostname, or a single-label wildcard such as "*.example.com". Configured +// paths and Request.Path are compared byte-for-byte; callers should pass the +// escaped request path when escaped path distinctions matter. No path cleaning, +// decoding, or slash collapsing is performed. +type Route struct { + ID string + Host string + Path string + PathType PathType + Methods []string + Headers map[string]string + ClusterID string +} + +// Config contains routes to compile. Cluster IDs are supplied separately to +// Compile so callers can validate references against their cluster inventory. +type Config struct { + Routes []Route +} + +// Request contains the matching fields already extracted from an HTTP request. +// Host may include a port; it is lower-cased and the port is removed for match. +type Request struct { + Host string + Path string + Method string + Headers http.Header +} + +// Matcher is an immutable compiled route set safe for concurrent matching. +type Matcher struct { + routes []Route +} + +// Compile validates route configuration and cluster references, then creates +// a deterministic matcher. Equal rules are permitted and resolved by lexical +// route ID; overlapping rules with equal precedence but different selectors +// are rejected as ambiguous. +func Compile(config Config, clusterIDs []string) (*Matcher, error) { + clusters := make(map[string]struct{}, len(clusterIDs)) + for _, id := range clusterIDs { + if id == "" { + return nil, errors.New("cluster ID must not be empty") + } + clusters[id] = struct{}{} + } + + routes := make([]Route, len(config.Routes)) + ids := make(map[string]struct{}, len(config.Routes)) + for i, route := range config.Routes { + if route.ID == "" { + return nil, fmt.Errorf("route %d: ID must not be empty", i) + } + if _, exists := ids[route.ID]; exists { + return nil, fmt.Errorf("route %q: duplicate ID", route.ID) + } + ids[route.ID] = struct{}{} + if route.ClusterID == "" { + return nil, fmt.Errorf("route %q: cluster ID must not be empty", route.ID) + } + if _, exists := clusters[route.ClusterID]; !exists { + return nil, fmt.Errorf("route %q: unknown cluster %q", route.ID, route.ClusterID) + } + if route.PathType != PathExact && route.PathType != PathPrefix { + return nil, fmt.Errorf("route %q: invalid path type", route.ID) + } + host, err := normalizePatternHost(route.Host) + if err != nil { + return nil, fmt.Errorf("route %q: %w", route.ID, err) + } + route.Host = host + route.Methods, err = normalizeMethods(route.Methods) + if err != nil { + return nil, fmt.Errorf("route %q: %w", route.ID, err) + } + route.Headers, err = normalizeHeaders(route.Headers) + if err != nil { + return nil, fmt.Errorf("route %q: %w", route.ID, err) + } + routes[i] = route + } + + for i := range routes { + for j := i + 1; j < len(routes); j++ { + if ambiguous(routes[i], routes[j]) { + return nil, fmt.Errorf("routes %q and %q have unresolved equal-precedence overlap", routes[i].ID, routes[j].ID) + } + } + } + sort.Slice(routes, func(i, j int) bool { return routes[i].ID < routes[j].ID }) + return &Matcher{routes: routes}, nil +} + +// Match returns the highest-precedence matching route. A nil Matcher behaves +// as an empty matcher, keeping malformed call paths panic-free. +func (m *Matcher) Match(request Request) (Route, bool) { + if m == nil { + return Route{}, false + } + var winner Route + found := false + for _, route := range m.routes { + if !routeMatches(route, request) { + continue + } + if !found || routePrecedes(route, winner) { + winner, found = route, true + } + } + if !found { + return Route{}, false + } + return cloneRoute(winner), true +} + +func cloneRoute(route Route) Route { + route.Methods = append([]string(nil), route.Methods...) + if route.Headers != nil { + headers := make(map[string]string, len(route.Headers)) + for name, value := range route.Headers { + headers[name] = value + } + route.Headers = headers + } + return route +} + +func routeMatches(route Route, request Request) bool { + host := normalizeRequestHost(request.Host) + if !hostMatches(route.Host, host) { + return false + } + if route.PathType == PathExact && request.Path != route.Path { + return false + } + if route.PathType == PathPrefix && !strings.HasPrefix(request.Path, route.Path) { + return false + } + if len(route.Methods) > 0 && !contains(route.Methods, strings.ToUpper(request.Method)) { + return false + } + for name, value := range route.Headers { + if request.Headers.Get(name) != value { + return false + } + } + return true +} + +func routePrecedes(a, b Route) bool { + if hostRank(a.Host) != hostRank(b.Host) { + return hostRank(a.Host) > hostRank(b.Host) + } + if a.PathType != b.PathType { + return a.PathType == PathExact + } + if len(a.Path) != len(b.Path) { + return len(a.Path) > len(b.Path) + } + if (len(a.Methods) > 0) != (len(b.Methods) > 0) { + return len(a.Methods) > 0 + } + if (len(a.Headers) > 0) != (len(b.Headers) > 0) { + return len(a.Headers) > 0 + } + return a.ID < b.ID +} + +func hostRank(host string) int { + switch { + case host == "": + return 0 + case strings.HasPrefix(host, "*."): + return 1 + default: + return 2 + } +} + +func hostMatches(pattern, host string) bool { + if pattern == "" { + return true + } + if !strings.HasPrefix(pattern, "*.") { + return pattern == host + } + suffix := pattern[1:] // includes the leading dot + if !strings.HasSuffix(host, suffix) { + return false + } + label := strings.TrimSuffix(host, suffix) + return label != "" && !strings.Contains(label, ".") +} + +func normalizePatternHost(host string) (string, error) { + if host == "" { + return "", nil + } + host = strings.ToLower(host) + if strings.ContainsAny(host, "/\\ \t\r\n") || strings.Contains(host, ":") { + return "", errors.New("host pattern must be a hostname without a port") + } + if strings.HasPrefix(host, "*.") { + if strings.Count(host, "*") != 1 || len(host) <= 2 || strings.Contains(host[2:], "*") || strings.HasPrefix(host[2:], ".") { + return "", errors.New("invalid single-label wildcard host") + } + return host, nil + } + if strings.Contains(host, "*") || strings.HasPrefix(host, ".") || strings.HasSuffix(host, ".") { + return "", errors.New("invalid exact host") + } + return host, nil +} + +func normalizeRequestHost(host string) string { + host = strings.TrimSpace(strings.ToLower(host)) + if h, _, err := net.SplitHostPort(host); err == nil { + return strings.Trim(h, "[]") + } + return strings.Trim(host, "[]") +} + +func normalizeMethods(methods []string) ([]string, error) { + if len(methods) == 0 { + return nil, nil + } + out := make([]string, 0, len(methods)) + seen := map[string]struct{}{} + for _, method := range methods { + method = strings.ToUpper(method) + if method == "" { + return nil, errors.New("method constraint must not be empty") + } + if _, exists := seen[method]; !exists { + out = append(out, method) + seen[method] = struct{}{} + } + } + sort.Strings(out) + return out, nil +} + +func normalizeHeaders(headers map[string]string) (map[string]string, error) { + if len(headers) == 0 { + return nil, nil + } + out := make(map[string]string, len(headers)) + for name, value := range headers { + canonical := http.CanonicalHeaderKey(name) + if canonical == "" || strings.ContainsAny(canonical, " \t\r\n:") { + return nil, fmt.Errorf("invalid header constraint name %q", name) + } + if _, exists := out[canonical]; exists { + return nil, fmt.Errorf("duplicate header constraint %q", canonical) + } + out[canonical] = value + } + return out, nil +} + +func ambiguous(a, b Route) bool { + if !sameRank(a, b) || !routesOverlap(a, b) { + return false + } + if !sameSelectors(a, b) { + return true + } + return a.ClusterID != b.ClusterID +} + +func sameRank(a, b Route) bool { + return hostRank(a.Host) == hostRank(b.Host) && a.PathType == b.PathType && len(a.Path) == len(b.Path) && + (len(a.Methods) > 0) == (len(b.Methods) > 0) && (len(a.Headers) > 0) == (len(b.Headers) > 0) +} + +func routesOverlap(a, b Route) bool { + if !hostsOverlap(a.Host, b.Host) || !pathsOverlap(a, b) || !methodsOverlap(a.Methods, b.Methods) { + return false + } + for name, value := range a.Headers { + if other, exists := b.Headers[name]; exists && other != value { + return false + } + } + return true +} + +func hostsOverlap(a, b string) bool { + if a == "" || b == "" || a == b { + return true + } + if strings.HasPrefix(a, "*.") { + return hostMatches(a, b) || strings.HasPrefix(b, "*.") && hostMatches(b, strings.TrimPrefix(a, "*.")) + } + if strings.HasPrefix(b, "*.") { + return hostMatches(b, a) + } + return false +} + +func pathsOverlap(a, b Route) bool { + if a.PathType == PathExact { + return pathMatches(b, a.Path) + } + if b.PathType == PathExact { + return pathMatches(a, b.Path) + } + return strings.HasPrefix(a.Path, b.Path) || strings.HasPrefix(b.Path, a.Path) +} + +func pathMatches(route Route, path string) bool { + if route.PathType == PathExact { + return path == route.Path + } + return strings.HasPrefix(path, route.Path) +} + +func methodsOverlap(a, b []string) bool { + if len(a) == 0 || len(b) == 0 { + return true + } + for _, method := range a { + if contains(b, method) { + return true + } + } + return false +} + +func sameSelectors(a, b Route) bool { + if a.Host != b.Host || a.Path != b.Path || a.PathType != b.PathType || !equalStrings(a.Methods, b.Methods) || len(a.Headers) != len(b.Headers) { + return false + } + for name, value := range a.Headers { + if b.Headers[name] != value { + return false + } + } + return true +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func contains(values []string, value string) bool { + for _, candidate := range values { + if candidate == value { + return true + } + } + return false +} diff --git a/internal/router/router_test.go b/internal/router/router_test.go new file mode 100644 index 0000000..2a4ae53 --- /dev/null +++ b/internal/router/router_test.go @@ -0,0 +1,241 @@ +package router + +import ( + "net/http" + "sync" + "testing" +) + +func TestCompileRoutePrecedence(t *testing.T) { + tests := []struct { + name string + routes []Route + input Request + want string + }{ + { + name: "exact host beats wildcard host", + routes: []Route{ + {ID: "wild", Host: "*.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "exact", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + }, + input: Request{Host: "API.EXAMPLE.COM:8443", Path: "/x"}, want: "exact", + }, + { + name: "wildcard matches one label only", + routes: []Route{ + {ID: "wild", Host: "*.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + }, + input: Request{Host: "a.b.example.com", Path: "/x"}, want: "", + }, + { + name: "exact path beats prefix", + routes: []Route{ + {ID: "prefix", Host: "api.example.com", Path: "/v1", PathType: PathPrefix, ClusterID: "c"}, + {ID: "exact", Host: "api.example.com", Path: "/v1", PathType: PathExact, ClusterID: "c"}, + }, + input: Request{Host: "api.example.com", Path: "/v1"}, want: "exact", + }, + { + name: "longer prefix beats shorter", + routes: []Route{ + {ID: "short", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "long", Host: "api.example.com", Path: "/v1", PathType: PathPrefix, ClusterID: "c"}, + }, + input: Request{Host: "api.example.com", Path: "/v1/users"}, want: "long", + }, + { + name: "method constraint beats unconstrained", + routes: []Route{ + {ID: "any", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "get", Host: "api.example.com", Path: "/", PathType: PathPrefix, Methods: []string{"GET"}, ClusterID: "c"}, + }, + input: Request{Host: "api.example.com", Path: "/x", Method: "GET"}, want: "get", + }, + { + name: "header constraint beats unconstrained", + routes: []Route{ + {ID: "any", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "json", Host: "api.example.com", Path: "/", PathType: PathPrefix, Headers: map[string]string{"Content-Type": "application/json"}, ClusterID: "c"}, + }, + input: Request{Host: "api.example.com", Path: "/x", Headers: http.Header{"Content-Type": []string{"application/json"}}}, want: "json", + }, + { + name: "equal rules for same cluster use lexical route id", + routes: []Route{ + {ID: "z-route", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "a-route", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + }, + input: Request{Host: "api.example.com", Path: "/x"}, want: "a-route", + }, + { + name: "path comparison preserves escapes and repeated slashes", + routes: []Route{ + {ID: "escaped", Host: "api.example.com", Path: "/a%2Fb", PathType: PathExact, ClusterID: "c"}, + }, + input: Request{Host: "api.example.com", Path: "/a%2Fb"}, want: "escaped", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + matcher, err := Compile(Config{Routes: tt.routes}, []string{"c"}) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + got, ok := matcher.Match(tt.input) + if tt.want == "" { + if ok { + t.Fatalf("Match() = %q, want no route", got.ID) + } + return + } + if !ok || got.ID != tt.want { + t.Fatalf("Match() = (%q, %v), want (%q, true)", got.ID, ok, tt.want) + } + }) + } +} + +func TestMatchReturnedRouteCannotMutateMatcher(t *testing.T) { + route := Route{ + ID: "json-get", + Host: "api.example.com", + Path: "/", + PathType: PathPrefix, + Methods: []string{"GET"}, + Headers: map[string]string{"Content-Type": "application/json"}, + ClusterID: "c", + } + matcher, err := Compile(Config{Routes: []Route{route}}, []string{"c"}) + if err != nil { + t.Fatalf("Compile() error = %v", err) + } + request := Request{ + Host: "api.example.com", + Path: "/items", + Method: "GET", + Headers: http.Header{"Content-Type": []string{"application/json"}}, + } + returned, ok := matcher.Match(request) + if !ok { + t.Fatal("Match() found no route") + } + returned.Methods[0] = "POST" + returned.Headers["Content-Type"] = "text/plain" + + got, ok := matcher.Match(request) + if !ok || got.ID != "json-get" { + t.Fatalf("mutation of returned route changed later matching: (%q, %v)", got.ID, ok) + } + + var wg sync.WaitGroup + wg.Add(1) + start := make(chan struct{}) + go func() { + defer wg.Done() + <-start + for i := 0; i < 1000; i++ { + returned.Methods[0] = "POST" + returned.Headers["Content-Type"] = "text/plain" + returned.Methods[0] = "GET" + returned.Headers["Content-Type"] = "application/json" + } + }() + close(start) + for i := 0; i < 1000; i++ { + if _, ok := matcher.Match(request); !ok { + t.Fatal("concurrent mutation of returned route changed matching") + } + } + wg.Wait() +} + +func TestCompileRejectsEquivalentSelectorsWithConflictingClusters(t *testing.T) { + routes := []Route{ + {ID: "blue", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "blue-cluster"}, + {ID: "green", Host: "api.example.com", Path: "/", PathType: PathPrefix, ClusterID: "green-cluster"}, + } + if _, err := Compile(Config{Routes: routes}, []string{"blue-cluster", "green-cluster"}); err == nil { + t.Fatal("Compile() error = nil, want conflicting destination rejection") + } +} + +func TestCompileRejectsAmbiguousRoutes(t *testing.T) { + tests := []struct { + name string + routes []Route + }{ + { + name: "intersecting header constraints at same rank", + routes: []Route{ + {ID: "a", Host: "api.example.com", Path: "/", PathType: PathPrefix, Methods: []string{"GET"}, Headers: map[string]string{"X-A": "1"}, ClusterID: "a"}, + {ID: "b", Host: "api.example.com", Path: "/", PathType: PathPrefix, Methods: []string{"GET"}, Headers: map[string]string{"X-B": "2"}, ClusterID: "b"}, + }, + }, + { + name: "intersecting method constraints at same rank", + routes: []Route{ + {ID: "a", Host: "api.example.com", Path: "/", PathType: PathPrefix, Methods: []string{"GET", "POST"}, ClusterID: "a"}, + {ID: "b", Host: "api.example.com", Path: "/", PathType: PathPrefix, Methods: []string{"GET", "PUT"}, ClusterID: "b"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := Compile(Config{Routes: tt.routes}, []string{"a", "b"}); err == nil { + t.Fatal("Compile() error = nil, want ambiguous route rejection") + } + }) + } +} + +func TestCompileRejectsUnknownClusterReference(t *testing.T) { + _, err := Compile(Config{Routes: []Route{{ID: "r", Path: "/", PathType: PathPrefix, ClusterID: "missing"}}}, nil) + if err == nil { + t.Fatal("Compile() error = nil, want missing cluster rejection") + } +} + +func FuzzRouteMatch(f *testing.F) { + f.Add("api.example.com", "/v1/users", "GET", "application/json") + f.Add("API.EXAMPLE.COM:443", "/a%2Fb//../c", "POST", "text/plain") + f.Add("x.example.com", "", "", "") + f.Fuzz(func(t *testing.T, host, path, method, contentType string) { + // Keep fuzz inputs bounded so this property stays a matcher test rather + // than a memory/CPU stress test. + if len(host) > 256 || len(path) > 1024 || len(method) > 32 || len(contentType) > 128 { + t.Skip() + } + routes := []Route{ + {ID: "fallback", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "host", Host: "*.example.com", Path: "/", PathType: PathPrefix, ClusterID: "c"}, + {ID: "path", Host: "*.example.com", Path: "/v1", PathType: PathPrefix, ClusterID: "c"}, + {ID: "exact", Host: "api.example.com", Path: "/v1", PathType: PathExact, Methods: []string{"GET"}, Headers: map[string]string{"Content-Type": "application/json"}, ClusterID: "c"}, + } + matcher, err := Compile(Config{Routes: routes}, []string{"c"}) + if err != nil { + t.Fatalf("valid seed config rejected: %v", err) + } + req := Request{Host: host, Path: path, Method: method, Headers: http.Header{"Content-Type": []string{contentType}}} + first, firstOK := matcher.Match(req) + second, secondOK := matcher.Match(req) + if firstOK != secondOK || first.ID != second.ID { + t.Fatalf("nondeterministic Match(): (%q,%v) then (%q,%v)", first.ID, firstOK, second.ID, secondOK) + } + if firstOK { + winnerMatches := false + for _, route := range routes { + if routeMatches(route, req) && routePrecedes(route, first) { + t.Fatalf("winner %q loses precedence to matching route %q", first.ID, route.ID) + } + if route.ID == first.ID && routeMatches(route, req) { + winnerMatches = true + } + } + if !winnerMatches { + t.Fatalf("winner %q does not match request", first.ID) + } + } + }) +} From 365cc464b1c70b6d71b138a04da7360ecf8f55ae Mon Sep 17 00:00:00 2001 From: SShogun Date: Sat, 10 Oct 2026 20:50:14 +0530 Subject: [PATCH 3/6] fix(proxy): report upstream transport failures --- README.md | 4 +-- docs/Gatehouse_DECISIONS.md | 4 +-- docs/Gatehouse_TESTING_CI.md | 8 +++++ go.mod | 2 +- internal/dataplane/proxy.go | 18 +++++++++- internal/dataplane/proxy_test.go | 60 ++++++++++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 9b1b2e6..762d419 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A production-minded Layer 7 gateway in Go, designed around protocol correctness, safe configuration rollouts, and predictable failure behavior. -**Go:** 1.26.8 · **Contracts:** Protobuf · **CI:** [GitHub Actions](https://github.com/SShogun/GateHouse/actions/workflows/ci.yml) +**Go:** 1.26.9 · **Contracts:** Protobuf · **CI:** [GitHub Actions](https://github.com/SShogun/GateHouse/actions/workflows/ci.yml) @@ -72,7 +72,7 @@ The critical separation: the control plane owns durable configuration and public ### Requirements - Git -- Go 1.26.8 (the `go.mod` toolchain directive is authoritative) +- Go 1.26.9 (the `go.mod` Go version is authoritative) - Buf CLI 1.73.0 - `make` - Network access on the first verification run to download pinned analysis tools and Buf generators diff --git a/docs/Gatehouse_DECISIONS.md b/docs/Gatehouse_DECISIONS.md index a5ab4e5..f74079d 100644 --- a/docs/Gatehouse_DECISIONS.md +++ b/docs/Gatehouse_DECISIONS.md @@ -380,6 +380,6 @@ An unauthenticated developer mode may bind loopback only and must be explicit. The Go module path follows the canonical repository identity: `github.com/SShogun/GateHouse`. -Since `gatehouse.v1` currently defines no application messages and the project is pre-release (M0), the `go_package` option in `gatehouse.v1` has been updated directly to match the new canonical repository path. +Since `gatehouse.v1` defined no application messages and the project was pre-release (M0), the `go_package` option in `gatehouse.v1` was updated directly to match the new canonical repository path. -To satisfy Buf's strict `FILE` compatibility rules during this zero-consumer migration, a narrowly scoped `breaking.ignore_only` exception for `FILE_SAME_GO_PACKAGE` on `gatehouse.proto` is temporarily applied in `buf.yaml`. This exception must be removed immediately after merging, once the new main commit becomes the compatibility baseline. +The zero-consumer migration completed in M0. The temporary `FILE_SAME_GO_PACKAGE` exception was removed in commit `0d366eb`; `buf.yaml` now enforces strict `FILE` compatibility. No temporary exception remains. diff --git a/docs/Gatehouse_TESTING_CI.md b/docs/Gatehouse_TESTING_CI.md index 7c0006e..895a424 100644 --- a/docs/Gatehouse_TESTING_CI.md +++ b/docs/Gatehouse_TESTING_CI.md @@ -774,3 +774,11 @@ known quarantined tests (ideally none) ``` A green badge without the ability to reproduce commands locally is insufficient. + +## Historical M0 completion evidence + +M0 was verified by PR #4 at commit `408be0505f48f096ae1af0cd42d5279229531f72` on 2026-10-10. The [GitHub Actions run](https://github.com/SShogun/GateHouse/actions/runs/38056682768) reports `contracts`, `static`, `unit`, `race`, and `security` all successful. That commit used Go `1.26.8` from `go.mod` and Buf `1.73.0` from the CI workflow. This is historical evidence for the M0 skeleton; later toolchain security findings are tracked against M1/M2. + +## M1 local verification evidence + +The M1 fixes were verified in the dirty `codex/m1-static-http-proxy` checkout based on `c859bb8` on 2026-10-10. The project `make verify` passed with the Go `1.26.9` toolchain: vet, staticcheck, Buf lint and compatibility fixtures, generated-file cleanliness, module verification, unit tests, race tests, and govulncheck (`No vulnerabilities found`). The focused RED showed a failing `RoundTripper` response being logged as success; the fix records transport failure per request. A separate regression confirms a valid upstream HTTP 502 remains a successful proxy response. The current uncommitted M1 state has no exact-head GitHub CI result; remote verification requires the approved commit/push checkpoint. diff --git a/go.mod b/go.mod index 75438de..8d7de33 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module github.com/SShogun/GateHouse -go 1.26.8 +go 1.26.9 require google.golang.org/protobuf v1.36.11 diff --git a/internal/dataplane/proxy.go b/internal/dataplane/proxy.go index 823cf9d..f96f911 100644 --- a/internal/dataplane/proxy.go +++ b/internal/dataplane/proxy.go @@ -2,6 +2,7 @@ package dataplane import ( + "context" "errors" "fmt" "log/slog" @@ -28,6 +29,12 @@ type Proxy struct { active atomic.Int64 } +type requestOutcomeKey struct{} + +type requestOutcome struct { + failure bool +} + // NewProxy creates a reverse proxy for a static HTTP or HTTPS upstream. func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { target, err := url.Parse(cfg.UpstreamURL) @@ -54,6 +61,9 @@ func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { FlushInterval: -1, Transport: transport, ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { + if outcome, ok := r.Context().Value(requestOutcomeKey{}).(*requestOutcome); ok { + outcome.failure = true + } p.logger.Warn("upstream request failed", "method", r.Method, "error", err) http.Error(w, "bad gateway", http.StatusBadGateway) }, @@ -72,6 +82,8 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { p.active.Add(1) defer p.active.Add(-1) started := time.Now() + outcome := &requestOutcome{} + r = r.WithContext(context.WithValue(r.Context(), requestOutcomeKey{}, outcome)) recorder := &statusRecorder{ResponseWriter: w} defer func() { if recovered := recover(); recovered != nil { @@ -92,10 +104,14 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { if status == 0 { status = http.StatusOK } + loggedOutcome := "success" + if outcome.failure { + loggedOutcome = "failure" + } p.logger.Info("proxy request complete", "method", r.Method, "status", status, - "outcome", "success", + "outcome", loggedOutcome, "duration", time.Since(started), ) }() diff --git a/internal/dataplane/proxy_test.go b/internal/dataplane/proxy_test.go index bf75072..93bd7ce 100644 --- a/internal/dataplane/proxy_test.go +++ b/internal/dataplane/proxy_test.go @@ -513,6 +513,66 @@ func readLine(r io.Reader) (string, error) { } } +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestProxyTransportErrorLogsFailureOutcome(t *testing.T) { + var logs bytes.Buffer + proxy, err := NewProxy(Config{UpstreamURL: "http://upstream.invalid"}, slog.New(slog.NewJSONHandler(&logs, nil))) + if err != nil { + t.Fatal(err) + } + proxy.upstream.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { + return nil, fmt.Errorf("dial failed") + }) + defer proxy.CloseIdleConnections() + + response := httptest.NewRecorder() + proxy.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/transport-error", nil)) + if response.Code != http.StatusBadGateway { + t.Fatalf("response status = %d, want %d", response.Code, http.StatusBadGateway) + } + logLines := strings.Split(strings.TrimSpace(logs.String()), "\n") + var record map[string]any + if err := json.Unmarshal([]byte(logLines[len(logLines)-1]), &record); err != nil { + t.Fatalf("decode structured request outcome %q: %v", logs.String(), err) + } + if record["outcome"] != "failure" { + t.Fatalf("transport error outcome = %v, want failure; log = %s", record["outcome"], logs.String()) + } +} + +func TestProxyUpstreamBadGatewayIsSuccessfulResponse(t *testing.T) { + var logs bytes.Buffer + proxy, err := NewProxy(Config{UpstreamURL: "http://upstream.invalid"}, slog.New(slog.NewJSONHandler(&logs, nil))) + if err != nil { + t.Fatal(err) + } + proxy.upstream.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusBadGateway, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("upstream 502")), + Request: httptest.NewRequest(http.MethodGet, "http://upstream.invalid/", nil), + }, nil + }) + defer proxy.CloseIdleConnections() + + response := httptest.NewRecorder() + proxy.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/upstream-502", nil)) + if response.Code != http.StatusBadGateway { + t.Fatalf("response status = %d, want %d", response.Code, http.StatusBadGateway) + } + var record map[string]any + if err := json.Unmarshal(logs.Bytes(), &record); err != nil { + t.Fatalf("decode structured request outcome %q: %v", logs.String(), err) + } + if record["outcome"] != "success" { + t.Fatalf("upstream 502 outcome = %v, want success; log = %s", record["outcome"], logs.String()) + } +} + func waitForActiveRequests(proxy *Proxy, want int64, timeout time.Duration) error { deadline := time.Now().Add(timeout) for proxy.ActiveRequests() != want && time.Now().Before(deadline) { From fc41624dcd8ec9ab17f09e97b532de893f982c43 Mon Sep 17 00:00:00 2001 From: SShogun Date: Sat, 10 Oct 2026 20:50:21 +0530 Subject: [PATCH 4/6] feat(router): add deterministic routing and health selection --- README.md | 15 +- cmd/gatehouse-data/main.go | 35 +- docs/Gatehouse_DECISIONS.md | 4 +- docs/Gatehouse_PLAN.md | 16 +- docs/Gatehouse_TESTING_CI.md | 22 + go.mod | 2 +- internal/cluster/cluster.go | 44 +- internal/cluster/cluster_test.go | 84 +++- internal/cluster/scheduler.go | 57 ++- internal/cluster/scheduler_test.go | 124 +++++ internal/config/config.go | 144 ++++++ internal/config/config_test.go | 94 ++++ internal/dataplane/proxy.go | 18 +- internal/dataplane/proxy_test.go | 60 +++ internal/gateway/gateway.go | 275 +++++++++++ internal/gateway/gateway_test.go | 426 ++++++++++++++++++ internal/router/router.go | 105 ++++- internal/router/router_test.go | 162 ++++++- .../fuzz/FuzzRouteMatch/12587131861cbc78 | 5 + .../fuzz/FuzzRouteMatch/3ef996ad05a4b442 | 5 + 20 files changed, 1622 insertions(+), 75 deletions(-) create mode 100644 internal/config/config.go create mode 100644 internal/config/config_test.go create mode 100644 internal/gateway/gateway.go create mode 100644 internal/gateway/gateway_test.go create mode 100644 internal/router/testdata/fuzz/FuzzRouteMatch/12587131861cbc78 create mode 100644 internal/router/testdata/fuzz/FuzzRouteMatch/3ef996ad05a4b442 diff --git a/README.md b/README.md index 9b1b2e6..92df7ae 100644 --- a/README.md +++ b/README.md @@ -6,11 +6,11 @@ A production-minded Layer 7 gateway in Go, designed around protocol correctness, safe configuration rollouts, and predictable failure behavior. -**Go:** 1.26.8 · **Contracts:** Protobuf · **CI:** [GitHub Actions](https://github.com/SShogun/GateHouse/actions/workflows/ci.yml) +**Go:** 1.26.9 · **Contracts:** Protobuf · **CI:** [GitHub Actions](https://github.com/SShogun/GateHouse/actions/workflows/ci.yml) -> **Project state: M1 complete — static HTTP proxy and cancellation.** The data plane forwards traffic to one configured upstream; M1 tests cover forwarding semantics, informational responses, cancellation cleanup, streaming memory, hop-by-hop headers, and graceful drain. Dynamic routing, configuration storage, and publication are not implemented yet. +> **Project state: M2 implementation present — static routing, clusters, and health.** The gateway can compile in-memory listener, route, and cluster configuration, match requests deterministically, select eligible endpoints, and run optional HTTP health checks. The `gatehouse-data` command still builds a single default route and cluster from environment variables. M3 configuration storage and dynamic publication/reload are not implemented. [What exists](#what-works-today) · [Run checks](#try-the-current-foundation) · [Target architecture](#target-architecture) · [Roadmap](#roadmap) · [Docs](#repository-map) @@ -30,17 +30,18 @@ Its eventual boundary is deliberately split: ## What works today -The current milestone is intentionally small. The repository provides: +The current implementation provides: - separate Go command entry points for `gatehouse-control` and `gatehouse-data`; -- a static HTTP reverse proxy in `gatehouse-data`, with bounded server defaults and graceful shutdown; +- a static HTTP reverse proxy and compiled in-memory route/cluster runtime, with bounded server defaults and graceful shutdown; +- deterministic route matching, endpoint selection, and optional scheduled HTTP health checks; - SIGINT/SIGTERM handling, root cancellation, and bounded shutdown coordination; - a `gatehouse.v1` Protobuf workspace with committed Go and TypeScript generated output; - Buf linting, compatibility checks, and generated-code drift checks; - an executable compatibility fixture that proves Buf rejects a removed field and accepts a compatible addition; - separate CI jobs for contracts, static analysis, unit tests, race tests, and vulnerability scanning. -The data-plane binary listens on `127.0.0.1:8080` and forwards every request to `http://127.0.0.1:8081` by default. Set `GATEHOUSE_LISTEN_ADDR` and `GATEHOUSE_UPSTREAM_URL` to override these in-process static settings. The control-plane binary remains a lifecycle scaffold. The Protobuf package is a contract workspace, not a completed application API. +The data-plane binary listens on `127.0.0.1:8080` and routes requests to `http://127.0.0.1:8081` by default. Set `GATEHOUSE_LISTEN_ADDR` and `GATEHOUSE_UPSTREAM_URL` to override the listener and the sole endpoint configured by this command. The gateway/config packages support richer static in-memory route and cluster configurations, but the command does not load them from a file or control plane. The control-plane binary remains a lifecycle scaffold. The Protobuf package is a contract workspace, not a completed application API. ## Target architecture @@ -72,7 +73,7 @@ The critical separation: the control plane owns durable configuration and public ### Requirements - Git -- Go 1.26.8 (the `go.mod` toolchain directive is authoritative) +- Go 1.26.9 (the `go.mod` Go version is authoritative) - Buf CLI 1.73.0 - `make` - Network access on the first verification run to download pinned analysis tools and Buf generators @@ -95,7 +96,7 @@ GATEHOUSE_UPSTREAM_URL=http://127.0.0.1:8081 \ go run ./cmd/gatehouse-data ``` -The proxy preserves the request method, path, end-to-end headers, and body; forwards response status, end-to-end headers, and body; strips hop-by-hop headers; propagates request cancellation; and streams response chunks without waiting for the full upstream body. It is a single static upstream, with no route matching or retries. SIGINT (`Ctrl+C`) and SIGTERM stop new requests and drain in-flight requests within the shutdown timeout. The control-plane scaffold can be started with `go run ./cmd/gatehouse-control` and currently waits for a shutdown signal. +The default command configuration matches all requests to one cluster with one upstream endpoint. The gateway runtime also supports deterministic static route matching and cluster endpoint selection, with optional scheduled HTTP health checks when configured in memory. The proxy preserves the request method, path, end-to-end headers, and body; forwards response status, end-to-end headers, and body; strips hop-by-hop headers; propagates request cancellation; and streams response chunks without waiting for the full upstream body. Retries and dynamic configuration publication/reload are not implemented. SIGINT (`Ctrl+C`) and SIGTERM stop new requests and drain in-flight requests within the shutdown timeout. The control-plane scaffold can be started with `go run ./cmd/gatehouse-control` and currently waits for a shutdown signal. ## The proof bar diff --git a/cmd/gatehouse-data/main.go b/cmd/gatehouse-data/main.go index b3a5172..efb196c 100644 --- a/cmd/gatehouse-data/main.go +++ b/cmd/gatehouse-data/main.go @@ -2,15 +2,21 @@ package main import ( "context" + "errors" "fmt" "log/slog" + "net/url" "os" "os/signal" "syscall" "time" + "github.com/SShogun/GateHouse/internal/cluster" + "github.com/SShogun/GateHouse/internal/config" "github.com/SShogun/GateHouse/internal/dataplane" + "github.com/SShogun/GateHouse/internal/gateway" "github.com/SShogun/GateHouse/internal/lifecycle" + "github.com/SShogun/GateHouse/internal/router" ) const shutdownTimeout = 10 * time.Second @@ -27,23 +33,36 @@ func run() error { defer stopSignals() logger := slog.New(slog.NewJSONHandler(os.Stdout, nil)) - proxy, err := dataplane.NewProxy(dataplane.Config{ - UpstreamURL: envOrDefault("GATEHOUSE_UPSTREAM_URL", "http://127.0.0.1:8081"), + listenAddress := envOrDefault("GATEHOUSE_LISTEN_ADDR", "127.0.0.1:8080") + upstreamAddress := envOrDefault("GATEHOUSE_UPSTREAM_URL", "http://127.0.0.1:8081") + upstream, err := url.Parse(upstreamAddress) + if err != nil { + return fmt.Errorf("parse GATEHOUSE_UPSTREAM_URL: %w", err) + } + clusterID, endpointID, routeID := "default", "default", "default" + runtime, err := gateway.New(config.Config{ + Listener: config.Listener{Address: listenAddress}, + Routes: []router.Route{{ID: routeID, Path: "/", PathType: router.PathPrefix, ClusterID: clusterID}}, + Clusters: []config.Cluster{{ID: clusterID, Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: endpointID, Address: upstream.String(), Weight: 1}}}}, }, logger) if err != nil { - return fmt.Errorf("configure data-plane proxy: %w", err) + return fmt.Errorf("configure data-plane gateway: %w", err) } - server, err := dataplane.NewServer(proxy, dataplane.ServerConfig{ - Address: envOrDefault("GATEHOUSE_LISTEN_ADDR", "127.0.0.1:8080"), - }) + server, err := dataplane.NewServer(runtime, config.Listener{Address: listenAddress}) if err != nil { + _ = runtime.Close() return fmt.Errorf("configure data-plane listener: %w", err) } if err := server.Start(); err != nil { + _ = runtime.Close() return err } - logger.Info("data plane listening", "address", server.Addr(), "upstream", envOrDefault("GATEHOUSE_UPSTREAM_URL", "http://127.0.0.1:8081")) - return lifecycle.Run(signalCtx, shutdownTimeout, server.Wait, server.Shutdown) + logger.Info("data plane listening", "address", server.Addr(), "upstream", upstreamAddress) + return lifecycle.Run(signalCtx, shutdownTimeout, server.Wait, func(ctx context.Context) error { + serverErr := server.Shutdown(ctx) + gatewayErr := runtime.Close() + return errors.Join(serverErr, gatewayErr) + }) } func envOrDefault(key, fallback string) string { diff --git a/docs/Gatehouse_DECISIONS.md b/docs/Gatehouse_DECISIONS.md index a5ab4e5..f74079d 100644 --- a/docs/Gatehouse_DECISIONS.md +++ b/docs/Gatehouse_DECISIONS.md @@ -380,6 +380,6 @@ An unauthenticated developer mode may bind loopback only and must be explicit. The Go module path follows the canonical repository identity: `github.com/SShogun/GateHouse`. -Since `gatehouse.v1` currently defines no application messages and the project is pre-release (M0), the `go_package` option in `gatehouse.v1` has been updated directly to match the new canonical repository path. +Since `gatehouse.v1` defined no application messages and the project was pre-release (M0), the `go_package` option in `gatehouse.v1` was updated directly to match the new canonical repository path. -To satisfy Buf's strict `FILE` compatibility rules during this zero-consumer migration, a narrowly scoped `breaking.ignore_only` exception for `FILE_SAME_GO_PACKAGE` on `gatehouse.proto` is temporarily applied in `buf.yaml`. This exception must be removed immediately after merging, once the new main commit becomes the compatibility baseline. +The zero-consumer migration completed in M0. The temporary `FILE_SAME_GO_PACKAGE` exception was removed in commit `0d366eb`; `buf.yaml` now enforces strict `FILE` compatibility. No temporary exception remains. diff --git a/docs/Gatehouse_PLAN.md b/docs/Gatehouse_PLAN.md index d30847e..d56529c 100644 --- a/docs/Gatehouse_PLAN.md +++ b/docs/Gatehouse_PLAN.md @@ -287,15 +287,17 @@ Implement: ## Gate M2 ```text -[ ] full route precedence matrix GREEN -[ ] fuzz smoke GREEN -[ ] health scheduler add/remove leak test GREEN -[ ] endpoint selection never chooses known-unhealthy endpoint when healthy choice exists -[ ] config with empty/missing cluster rejected -[ ] 10k route-match benchmark recorded -[ ] race GREEN under concurrent requests + health transitions +[x] full route precedence matrix GREEN +[x] fuzz smoke GREEN +[x] health scheduler add/remove leak test GREEN +[x] endpoint selection never chooses known-unhealthy endpoint when healthy choice exists +[x] config with empty/missing cluster rejected +[x] 10k route-match benchmark recorded +[x] race GREEN under concurrent requests + health transitions ``` +M2 acceptance evidence is recorded in `Gatehouse_TESTING_CI.md`. + --- # 7. Milestone 3 — Control plane, revisions, and atomic hot reload diff --git a/docs/Gatehouse_TESTING_CI.md b/docs/Gatehouse_TESTING_CI.md index 7c0006e..c01dba3 100644 --- a/docs/Gatehouse_TESTING_CI.md +++ b/docs/Gatehouse_TESTING_CI.md @@ -774,3 +774,25 @@ known quarantined tests (ideally none) ``` A green badge without the ability to reproduce commands locally is insufficient. + +## Historical M0 completion evidence + +M0 was verified by PR #4 at commit `408be0505f48f096ae1af0cd42d5279229531f72` on 2026-10-10. The [GitHub Actions run](https://github.com/SShogun/GateHouse/actions/runs/38056682768) reports `contracts`, `static`, `unit`, `race`, and `security` all successful. That commit used Go `1.26.8` from `go.mod` and Buf `1.73.0` from the CI workflow. This is historical evidence for the M0 skeleton; later toolchain security findings are tracked against M1/M2. + +## M1 local verification evidence + +The M1 fixes were verified in the dirty `codex/m1-static-http-proxy` checkout based on `c859bb8` on 2026-10-10. The project `make verify` passed with Go `1.26.9`: vet, staticcheck, Buf lint and compatibility fixtures, generated-file cleanliness, module verification, unit tests, race tests, and govulncheck (`No vulnerabilities found`). The focused RED showed a failing `RoundTripper` response being logged as success; the fix records transport failure per request. A separate regression confirms a valid upstream HTTP 502 remains a successful proxy response. The current uncommitted M1 state has no exact-head GitHub CI result; remote verification requires the approved commit/push checkpoint. + +## M2 local verification evidence + +The M2 implementation, the integrated M1 proxy fix, and tests were verified in a dirty worktree based on `8459cec` on 2026-10-10. After the final independent-audit fixes, `GOCACHE=/tmp/gatehouse-gocache GOMODCACHE=/tmp/gatehouse-modcache make verify` passed using the module-pinned Go `1.26.9`, staticcheck `v0.7.0`, and govulncheck `v1.8.0`. The gate covered vet, staticcheck, Buf lint, breaking-fixture rejection and compatible-fixture acceptance, generated-file cleanliness, module checks, shuffled unit tests, race tests, and vulnerability scanning (`No vulnerabilities found`). + +M2-specific evidence: + +- The header-constraint regression RED showed missing and repeated empty values matching an empty-value constraint, and malformed `X@Bad` being accepted. The focused router suite now covers missing, present-empty, wrong, and repeated values, plus malformed names. +- A 3-second `FuzzRouteMatch` smoke run completed 374,404 executions. Its expected winner is computed by an independent oracle, not the production matcher or comparator. +- `BenchmarkMatch10KRoutes` recorded 239.4 ns/op, 32 B/op, and 1 alloc/op on linux/amd64 with Go `1.27.1-X:nodwarf5`. This benchmark runtime differs from the pinned Go `1.26.9` used for `make verify`. +- Behavioral RED/GREEN tests cover smooth weighted round-robin cycle counts, deterministic source tie-breaking, active health transitions and exclusion, health probe cancellation, configured timeout bounds, scheduler concurrency limits, and concurrent requests during health-state changes. +- The independent Sol review found and the final race run now covers startup health probes against a fully built cluster map (128 health-checked clusters); the gateway defers starting workers until runtime construction completes. Additional regressions cover incomplete 2xx health bodies, escaped health-path preservation on the HTTP wire, health paths containing query/fragment delimiters, ambiguous routes with distinct empty-valued headers, malformed listener addresses, and route/cluster IDs in abort telemetry. The fuzz oracle's seed corpus includes a bracketed host without a port as well as a host with a port, and the oracle covers both normalization forms independently. + +The dirty M2 worktree includes the same M1 `RoundTripper` outcome fix as the main M1 checkout, with regression coverage for both transport errors and valid upstream 502 responses. This evidence is local to the uncommitted worktree based on `8459cec`; GitHub CI has not run against these current M2 changes. The PR branch remains pending the user-approved commit and push checkpoint. diff --git a/go.mod b/go.mod index 75438de..8d7de33 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module github.com/SShogun/GateHouse -go 1.26.8 +go 1.26.9 require google.golang.org/protobuf v1.36.11 diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go index a075b89..bc8e0f4 100644 --- a/internal/cluster/cluster.go +++ b/internal/cluster/cluster.go @@ -66,11 +66,13 @@ type endpointHealth struct { // Cluster owns mutable health and selection cursor state for one config snapshot. type Cluster struct { - mu sync.Mutex - endpoints []Endpoint - health map[string]*endpointHealth - thresholds HealthThresholds - cursor uint64 + mu sync.Mutex + endpoints []Endpoint + health map[string]*endpointHealth + thresholds HealthThresholds + cursor uint64 + weightedCurrent map[string]int64 + weightedKey string } // New validates and copies endpoint configuration. Zero health thresholds default to 1. @@ -105,8 +107,8 @@ func New(cfg Config) (*Cluster, error) { // Select returns one enabled endpoint that is not known Unhealthy. Unknown // endpoints remain eligible until the unhealthy threshold is crossed. If all // endpoints are unavailable, it returns ErrNoHealthyEndpoints. Weighted -// selection draws proportionally to configured positive weights using source; -// the source is consulted only for that policy. +// selection uses smooth weighted round robin. source breaks exact score ties, +// making tie outcomes deterministic and injectable by callers. func (c *Cluster) Select(policy Policy, source SelectionSource) (Endpoint, error) { c.mu.Lock() defer c.mu.Unlock() @@ -128,19 +130,31 @@ func (c *Cluster) Select(policy Policy, source SelectionSource) (Endpoint, error if source == nil { return Endpoint{}, ErrSelectionSource } - var total uint64 + var total int64 + key := "" for _, ep := range eligible { - total += uint64(ep.Weight) + total += int64(ep.Weight) + key += fmt.Sprintf("%d:%s:%d;", len(ep.ID), ep.ID, ep.Weight) } - draw := source.Uint64n(total) % total + if key != c.weightedKey { + c.weightedKey = key + c.weightedCurrent = make(map[string]int64, len(eligible)) + } + var best int64 + candidates := make([]Endpoint, 0, len(eligible)) for _, ep := range eligible { - weight := uint64(ep.Weight) - if draw < weight { - return ep, nil + c.weightedCurrent[ep.ID] += int64(ep.Weight) + if len(candidates) == 0 || c.weightedCurrent[ep.ID] > best { + best = c.weightedCurrent[ep.ID] + candidates = candidates[:0] + candidates = append(candidates, ep) + } else if c.weightedCurrent[ep.ID] == best { + candidates = append(candidates, ep) } - draw -= weight } - panic("unreachable weighted selection") + selected := candidates[source.Uint64n(uint64(len(candidates)))%uint64(len(candidates))] + c.weightedCurrent[selected.ID] -= total + return selected, nil default: return Endpoint{}, fmt.Errorf("%w: unknown selection policy %q", ErrInvalidConfig, policy) } diff --git a/internal/cluster/cluster_test.go b/internal/cluster/cluster_test.go index 1a929e1..f65149c 100644 --- a/internal/cluster/cluster_test.go +++ b/internal/cluster/cluster_test.go @@ -64,7 +64,7 @@ func TestWeightedSelectionUsesConfiguredWeights(t *testing.T) { } got[i] = ep.ID } - want := []string{"light", "heavy", "heavy", "heavy"} + want := []string{"heavy", "heavy", "light", "heavy"} for i := range want { if got[i] != want[i] { t.Fatalf("selection %v, want %v", got, want) @@ -72,6 +72,88 @@ func TestWeightedSelectionUsesConfiguredWeights(t *testing.T) { } } +func TestWeightedRoundRobinHasDeterministicWeightCycles(t *testing.T) { + config := Config{Endpoints: []Endpoint{ + {ID: "a", Weight: 5}, + {ID: "b", Weight: 1}, + }} + first, err := New(config) + if err != nil { + t.Fatal(err) + } + second, err := New(config) + if err != nil { + t.Fatal(err) + } + source := &sequenceSource{values: make([]uint64, 24)} + secondSource := &sequenceSource{values: make([]uint64, 24)} + gotA, gotB := 0, 0 + for i := 0; i < 24; i++ { + left, err := first.Select(Weighted, source) + if err != nil { + t.Fatal(err) + } + right, err := second.Select(Weighted, secondSource) + if err != nil { + t.Fatal(err) + } + if left.ID != right.ID { + t.Fatalf("selection %d differs between identical clusters: %q vs %q", i, left.ID, right.ID) + } + if left.ID == "a" { + gotA++ + } else if left.ID == "b" { + gotB++ + } else { + t.Fatalf("unexpected endpoint %q", left.ID) + } + } + if gotA != 20 || gotB != 4 { + t.Fatalf("24 selections = a:%d b:%d, want a:20 b:4", gotA, gotB) + } +} + +func TestWeightedRoundRobinUsesSourceForTiesAndPreservesCycleWeights(t *testing.T) { + config := Config{Endpoints: []Endpoint{{ID: "a", Weight: 1}, {ID: "b", Weight: 1}}} + selectFirst := func(values []uint64) []string { + t.Helper() + c, err := New(config) + if err != nil { + t.Fatal(err) + } + source := &sequenceSource{values: values} + got := make([]string, 6) + for i := range got { + endpoint, err := c.Select(Weighted, source) + if err != nil { + t.Fatal(err) + } + got[i] = endpoint.ID + } + return got + } + fromA := selectFirst([]uint64{0, 0, 0, 0, 0, 0}) + fromB := selectFirst([]uint64{1, 1, 1, 1, 1, 1}) + replayA := selectFirst([]uint64{0, 0, 0, 0, 0, 0}) + if fromA[0] != "a" || fromB[0] != "b" { + t.Fatalf("source tie choices = %q and %q, want a and b", fromA[0], fromB[0]) + } + for i := range fromA { + if fromA[i] != replayA[i] { + t.Fatalf("same source replay differs: %v vs %v", fromA, replayA) + } + } + for label, sequence := range map[string][]string{"source 0": fromA, "source 1": fromB} { + counts := map[string]int{} + for _, id := range sequence { + counts[id]++ + } + if counts["a"] != 3 || counts["b"] != 3 { + t.Fatalf("%s six-selection cycle = %v, want a:3 b:3", label, counts) + } + } +} + func TestUnknownEndpointsRemainEligibleUntilUnhealthyThreshold(t *testing.T) { c, err := New(Config{ Endpoints: []Endpoint{{ID: "api", Weight: 1}}, diff --git a/internal/cluster/scheduler.go b/internal/cluster/scheduler.go index 3a97c20..28864e7 100644 --- a/internal/cluster/scheduler.go +++ b/internal/cluster/scheduler.go @@ -3,6 +3,7 @@ package cluster import ( "context" "errors" + "math/rand" "sync" "time" ) @@ -22,16 +23,46 @@ type scheduledProbe struct { done chan struct{} } +// SchedulerOptions bounds active checks and controls probe timing. A nil Jitter +// selects a random delay within 10% of the configured interval on either side. +type SchedulerOptions struct { + ProbeTimeout time.Duration + MaxConcurrent int + Jitter func(time.Duration) time.Duration +} + // Scheduler owns at most one periodic worker per registered ID. type Scheduler struct { - mu sync.Mutex - entries map[string]*scheduledProbe - stopped bool - stopDone chan struct{} + mu sync.Mutex + entries map[string]*scheduledProbe + stopped bool + stopDone chan struct{} + options SchedulerOptions + semaphore chan struct{} } func NewScheduler() *Scheduler { - return &Scheduler{entries: make(map[string]*scheduledProbe), stopDone: make(chan struct{})} + return NewSchedulerWithOptions(SchedulerOptions{}) +} + +// NewSchedulerWithOptions creates a scheduler with bounded, cancelable probes. +func NewSchedulerWithOptions(options SchedulerOptions) *Scheduler { + if options.ProbeTimeout <= 0 { + options.ProbeTimeout = 5 * time.Second + } + if options.MaxConcurrent <= 0 { + options.MaxConcurrent = 16 + } + if options.Jitter == nil { + options.Jitter = func(interval time.Duration) time.Duration { + span := int64(interval / 10) + if span == 0 { + return 0 + } + return time.Duration(rand.Int63n(2*span+1) - span) + } + } + return &Scheduler{entries: make(map[string]*scheduledProbe), stopDone: make(chan struct{}), options: options, semaphore: make(chan struct{}, options.MaxConcurrent)} } // Add starts an immediate probe followed by periodic probes. Duplicate IDs fail. @@ -53,8 +84,20 @@ func (s *Scheduler) Add(id string, interval time.Duration, probe Probe) error { go func() { defer close(entry.done) for { - _ = probe(ctx) - timer := time.NewTimer(interval) + select { + case s.semaphore <- struct{}{}: + case <-ctx.Done(): + return + } + probeCtx, probeCancel := context.WithTimeout(ctx, s.options.ProbeTimeout) + _ = probe(probeCtx) + probeCancel() + <-s.semaphore + delay := interval + s.options.Jitter(interval) + if delay <= 0 { + delay = interval + } + timer := time.NewTimer(delay) select { case <-ctx.Done(): timer.Stop() diff --git a/internal/cluster/scheduler_test.go b/internal/cluster/scheduler_test.go index 2687add..fa83784 100644 --- a/internal/cluster/scheduler_test.go +++ b/internal/cluster/scheduler_test.go @@ -3,6 +3,7 @@ package cluster import ( "context" "errors" + "sync/atomic" "testing" "time" ) @@ -60,3 +61,126 @@ func TestSchedulerStopCancelsAllProbes(t *testing.T) { t.Fatalf("finished probes = %d, want 2", len(finished)) } } + +func TestSchedulerCancelsProbeWhenRemovedDuringJitterDelay(t *testing.T) { + started := make(chan struct{}) + finished := make(chan struct{}) + scheduler := NewScheduler() + if err := scheduler.Add("api", time.Hour, func(ctx context.Context) error { + select { + case <-started: + default: + close(started) + } + <-ctx.Done() + close(finished) + return ctx.Err() + }); err != nil { + t.Fatal(err) + } + <-started + if err := scheduler.Remove("api"); err != nil { + t.Fatal(err) + } + select { + case <-finished: + default: + t.Fatal("Remove returned before cancellation completed") + } + if scheduler.Count() != 0 { + t.Fatalf("schedule leaked after Remove: count=%d", scheduler.Count()) + } +} + +func TestSchedulerProbeReceivesConfiguredTimeout(t *testing.T) { + deadlineSeen := make(chan time.Time, 1) + canceled := make(chan struct{}, 1) + scheduler := NewSchedulerWithOptions(SchedulerOptions{ + ProbeTimeout: 20 * time.Millisecond, + MaxConcurrent: 1, + Jitter: func(time.Duration) time.Duration { return 0 }, + }) + if err := scheduler.Add("api", time.Hour, func(ctx context.Context) error { + deadline, ok := ctx.Deadline() + if !ok { + return errors.New("probe context has no timeout") + } + deadlineSeen <- deadline + <-ctx.Done() + canceled <- struct{}{} + return ctx.Err() + }); err != nil { + t.Fatal(err) + } + select { + case <-deadlineSeen: + case <-time.After(time.Second): + t.Fatal("probe did not start") + } + select { + case <-canceled: + case <-time.After(time.Second): + t.Fatal("probe context was not canceled at timeout") + } + if err := scheduler.Remove("api"); err != nil { + t.Fatal(err) + } +} + +func TestSchedulerBoundsConcurrentProbes(t *testing.T) { + const limit = 2 + var active atomic.Int32 + var maximum atomic.Int32 + started := make(chan struct{}, 4) + release := make(chan struct{}, 4) + scheduler := NewSchedulerWithOptions(SchedulerOptions{ + ProbeTimeout: time.Minute, + MaxConcurrent: limit, + Jitter: func(time.Duration) time.Duration { return 0 }, + }) + for _, id := range []string{"one", "two", "three", "four"} { + if err := scheduler.Add(id, time.Hour, func(ctx context.Context) error { + current := active.Add(1) + for seen := maximum.Load(); current > seen && !maximum.CompareAndSwap(seen, current); seen = maximum.Load() { + } + started <- struct{}{} + defer active.Add(-1) + select { + case <-release: + case <-ctx.Done(): + } + return nil + }); err != nil { + t.Fatal(err) + } + } + for i := 0; i < limit; i++ { + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("expected two probes to start") + } + } + select { + case <-started: + t.Fatal("scheduler exceeded configured probe concurrency") + default: + } + for i := 0; i < limit; i++ { + release <- struct{}{} + } + for i := 0; i < limit; i++ { + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("queued probe did not start after capacity was released") + } + } + scheduler.Stop() + if got := maximum.Load(); got > limit { + t.Fatalf("maximum concurrent probes = %d, want <= %d", got, limit) + } + if got := active.Load(); got != 0 { + t.Fatalf("active probes after Stop = %d, want 0", got) + } +} diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 0000000..d5eb493 --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,144 @@ +// Package config defines the static listener, route, and cluster configuration +// accepted by Gatehouse. +package config + +import ( + "errors" + "fmt" + "net" + "net/url" + "strconv" + "strings" + "time" + + "github.com/SShogun/GateHouse/internal/cluster" + "github.com/SShogun/GateHouse/internal/dataplane" + "github.com/SShogun/GateHouse/internal/router" +) + +// Listener configures the HTTP listener and its server resource limits. +type Listener = dataplane.ServerConfig + +// Cluster configures one named group of upstream endpoints. +type Cluster struct { + ID string + Policy cluster.Policy + Endpoints []cluster.Endpoint + Health cluster.HealthThresholds + HealthCheck *HealthCheck +} + +// HealthCheck enables scheduled HTTP health probes when attached to a cluster. +type HealthCheck struct { + Path string + Interval time.Duration + Timeout time.Duration +} + +// Config is one complete data-plane configuration snapshot. +type Config struct { + Listener Listener + Routes []router.Route + Clusters []Cluster +} + +// Validate checks cross-reference and component-level configuration rules. +// It does not bind the listener or create runtime health-check resources. +func Validate(cfg Config) error { + if cfg.Listener.Address == "" { + return errors.New("listener address is required") + } + if err := validateListenerAddress(cfg.Listener.Address); err != nil { + return fmt.Errorf("listener address: %w", err) + } + if len(cfg.Clusters) == 0 { + return errors.New("at least one cluster is required") + } + + clusterIDs := make([]string, 0, len(cfg.Clusters)) + seenClusters := make(map[string]struct{}, len(cfg.Clusters)) + for i, configured := range cfg.Clusters { + if configured.ID == "" { + return fmt.Errorf("cluster %d: ID must not be empty", i) + } + if _, exists := seenClusters[configured.ID]; exists { + return fmt.Errorf("cluster %q: duplicate ID", configured.ID) + } + seenClusters[configured.ID] = struct{}{} + clusterIDs = append(clusterIDs, configured.ID) + + if configured.Policy != cluster.RoundRobin && configured.Policy != cluster.Weighted { + return fmt.Errorf("cluster %q: invalid selection policy %q", configured.ID, configured.Policy) + } + if len(configured.Endpoints) == 0 { + return fmt.Errorf("cluster %q: at least one endpoint is required", configured.ID) + } + for j, endpoint := range configured.Endpoints { + if endpoint.Address == "" { + return fmt.Errorf("cluster %q endpoint %d: address is required", configured.ID, j) + } + if err := validateEndpointAddress(endpoint.Address); err != nil { + return fmt.Errorf("cluster %q endpoint %q: %w", configured.ID, endpoint.ID, err) + } + } + if _, err := cluster.New(cluster.Config{Endpoints: configured.Endpoints, Health: configured.Health}); err != nil { + return fmt.Errorf("cluster %q: %w", configured.ID, err) + } + if check := configured.HealthCheck; check != nil { + if err := validateHealthCheck(*check); err != nil { + return fmt.Errorf("cluster %q health check: %w", configured.ID, err) + } + } + } + + if _, err := router.Compile(router.Config{Routes: cfg.Routes}, clusterIDs); err != nil { + return fmt.Errorf("routes: %w", err) + } + return nil +} + +func validateEndpointAddress(address string) error { + parsed, err := url.Parse(address) + if err != nil { + return fmt.Errorf("invalid endpoint address: %w", err) + } + if (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Host == "" { + return errors.New("endpoint address must be an absolute http or https URL") + } + if parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" || (parsed.Path != "" && parsed.Path != "/") { + return errors.New("endpoint address must contain only a scheme and host") + } + return nil +} + +func validateListenerAddress(address string) error { + if strings.TrimSpace(address) != address { + return errors.New("must not contain surrounding whitespace") + } + host, port, err := net.SplitHostPort(address) + if err != nil { + return fmt.Errorf("must be host:port: %w", err) + } + if strings.ContainsAny(host, " \t\r\n") { + return errors.New("host must not contain whitespace") + } + portNumber, err := strconv.Atoi(port) + if err != nil || portNumber < 0 || portNumber > 65535 { + return errors.New("port must be a number between 0 and 65535") + } + return nil +} + +func validateHealthCheck(check HealthCheck) error { + parsed, err := url.ParseRequestURI(check.Path) + if err != nil || parsed.IsAbs() || parsed.Host != "" || parsed.Path == "" || parsed.Path[0] != '/' || parsed.RawQuery != "" || parsed.Fragment != "" || strings.ContainsAny(check.Path, "?#") { + return errors.New("path must be an absolute URL path") + } + if check.Interval <= 0 { + return errors.New("interval must be positive") + } + if check.Timeout <= 0 { + return errors.New("timeout must be positive") + } + return nil +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..ce2bfe2 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,94 @@ +package config + +import ( + "testing" + "time" + + "github.com/SShogun/GateHouse/internal/cluster" + "github.com/SShogun/GateHouse/internal/dataplane" + "github.com/SShogun/GateHouse/internal/router" +) + +func validConfig() Config { + return Config{ + Listener: dataplane.ServerConfig{Address: "127.0.0.1:8080"}, + Clusters: []Cluster{{ + ID: "backend", Policy: cluster.RoundRobin, + Endpoints: []cluster.Endpoint{{ID: "primary", Address: "http://127.0.0.1:9000", Weight: 1}}, + }}, + Routes: []router.Route{{ID: "root", Path: "/", PathType: router.PathPrefix, ClusterID: "backend"}}, + } +} + +func TestValidateAcceptsOptionalHealthCheck(t *testing.T) { + cfg := validConfig() + cfg.Clusters[0].HealthCheck = &HealthCheck{Path: "/ready", Interval: time.Second, Timeout: 250 * time.Millisecond} + if err := Validate(cfg); err != nil { + t.Fatalf("Validate() error = %v", err) + } +} + +func TestValidateRejectsInvalidHealthCheck(t *testing.T) { + tests := []struct { + name string + check HealthCheck + }{ + {"relative path", HealthCheck{Path: "ready", Interval: time.Second, Timeout: time.Second}}, + {"path with query", HealthCheck{Path: "/ready?x=1", Interval: time.Second, Timeout: time.Second}}, + {"path with fragment", HealthCheck{Path: "/ready#ok", Interval: time.Second, Timeout: time.Second}}, + {"empty path", HealthCheck{Path: "", Interval: time.Second, Timeout: time.Second}}, + {"zero interval", HealthCheck{Path: "/ready", Timeout: time.Second}}, + {"negative interval", HealthCheck{Path: "/ready", Interval: -time.Second, Timeout: time.Second}}, + {"zero timeout", HealthCheck{Path: "/ready", Interval: time.Second}}, + {"negative timeout", HealthCheck{Path: "/ready", Interval: time.Second, Timeout: -time.Second}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := validConfig() + cfg.Clusters[0].HealthCheck = &tt.check + if err := Validate(cfg); err == nil { + t.Fatal("Validate() error = nil, want health-check validation error") + } + }) + } +} + +func TestValidateAcceptsMinimalCompleteConfig(t *testing.T) { + if err := Validate(validConfig()); err != nil { + t.Fatalf("Validate() error = %v", err) + } +} + +func TestValidateRejectsInvalidSemantics(t *testing.T) { + tests := []struct { + name string + mutate func(*Config) + }{ + {"empty listener address", func(c *Config) { c.Listener.Address = "" }}, + {"malformed listener address", func(c *Config) { c.Listener.Address = "not-an-address" }}, + {"listener port out of range", func(c *Config) { c.Listener.Address = "127.0.0.1:70000" }}, + {"no clusters", func(c *Config) { c.Clusters = nil }}, + {"empty cluster id", func(c *Config) { c.Clusters[0].ID = "" }}, + {"duplicate cluster id", func(c *Config) { c.Clusters = append(c.Clusters, c.Clusters[0]) }}, + {"empty endpoints", func(c *Config) { c.Clusters[0].Endpoints = nil }}, + {"empty endpoint id", func(c *Config) { c.Clusters[0].Endpoints[0].ID = "" }}, + {"empty endpoint address", func(c *Config) { c.Clusters[0].Endpoints[0].Address = "" }}, + {"non-absolute endpoint address", func(c *Config) { c.Clusters[0].Endpoints[0].Address = "backend:9000" }}, + {"endpoint base path", func(c *Config) { c.Clusters[0].Endpoints[0].Address = "http://backend:9000/api" }}, + {"zero endpoint weight", func(c *Config) { c.Clusters[0].Endpoints[0].Weight = 0 }}, + {"duplicate endpoint id", func(c *Config) { c.Clusters[0].Endpoints = append(c.Clusters[0].Endpoints, c.Clusters[0].Endpoints[0]) }}, + {"invalid policy", func(c *Config) { c.Clusters[0].Policy = "random" }}, + {"empty route id", func(c *Config) { c.Routes[0].ID = "" }}, + {"duplicate route id", func(c *Config) { c.Routes = append(c.Routes, c.Routes[0]) }}, + {"unresolved route cluster", func(c *Config) { c.Routes[0].ClusterID = "missing" }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := validConfig() + tt.mutate(&cfg) + if err := Validate(cfg); err == nil { + t.Fatal("Validate() error = nil, want validation error") + } + }) + } +} diff --git a/internal/dataplane/proxy.go b/internal/dataplane/proxy.go index 823cf9d..f96f911 100644 --- a/internal/dataplane/proxy.go +++ b/internal/dataplane/proxy.go @@ -2,6 +2,7 @@ package dataplane import ( + "context" "errors" "fmt" "log/slog" @@ -28,6 +29,12 @@ type Proxy struct { active atomic.Int64 } +type requestOutcomeKey struct{} + +type requestOutcome struct { + failure bool +} + // NewProxy creates a reverse proxy for a static HTTP or HTTPS upstream. func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { target, err := url.Parse(cfg.UpstreamURL) @@ -54,6 +61,9 @@ func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { FlushInterval: -1, Transport: transport, ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { + if outcome, ok := r.Context().Value(requestOutcomeKey{}).(*requestOutcome); ok { + outcome.failure = true + } p.logger.Warn("upstream request failed", "method", r.Method, "error", err) http.Error(w, "bad gateway", http.StatusBadGateway) }, @@ -72,6 +82,8 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { p.active.Add(1) defer p.active.Add(-1) started := time.Now() + outcome := &requestOutcome{} + r = r.WithContext(context.WithValue(r.Context(), requestOutcomeKey{}, outcome)) recorder := &statusRecorder{ResponseWriter: w} defer func() { if recovered := recover(); recovered != nil { @@ -92,10 +104,14 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { if status == 0 { status = http.StatusOK } + loggedOutcome := "success" + if outcome.failure { + loggedOutcome = "failure" + } p.logger.Info("proxy request complete", "method", r.Method, "status", status, - "outcome", "success", + "outcome", loggedOutcome, "duration", time.Since(started), ) }() diff --git a/internal/dataplane/proxy_test.go b/internal/dataplane/proxy_test.go index bf75072..93bd7ce 100644 --- a/internal/dataplane/proxy_test.go +++ b/internal/dataplane/proxy_test.go @@ -513,6 +513,66 @@ func readLine(r io.Reader) (string, error) { } } +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestProxyTransportErrorLogsFailureOutcome(t *testing.T) { + var logs bytes.Buffer + proxy, err := NewProxy(Config{UpstreamURL: "http://upstream.invalid"}, slog.New(slog.NewJSONHandler(&logs, nil))) + if err != nil { + t.Fatal(err) + } + proxy.upstream.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { + return nil, fmt.Errorf("dial failed") + }) + defer proxy.CloseIdleConnections() + + response := httptest.NewRecorder() + proxy.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/transport-error", nil)) + if response.Code != http.StatusBadGateway { + t.Fatalf("response status = %d, want %d", response.Code, http.StatusBadGateway) + } + logLines := strings.Split(strings.TrimSpace(logs.String()), "\n") + var record map[string]any + if err := json.Unmarshal([]byte(logLines[len(logLines)-1]), &record); err != nil { + t.Fatalf("decode structured request outcome %q: %v", logs.String(), err) + } + if record["outcome"] != "failure" { + t.Fatalf("transport error outcome = %v, want failure; log = %s", record["outcome"], logs.String()) + } +} + +func TestProxyUpstreamBadGatewayIsSuccessfulResponse(t *testing.T) { + var logs bytes.Buffer + proxy, err := NewProxy(Config{UpstreamURL: "http://upstream.invalid"}, slog.New(slog.NewJSONHandler(&logs, nil))) + if err != nil { + t.Fatal(err) + } + proxy.upstream.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusBadGateway, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("upstream 502")), + Request: httptest.NewRequest(http.MethodGet, "http://upstream.invalid/", nil), + }, nil + }) + defer proxy.CloseIdleConnections() + + response := httptest.NewRecorder() + proxy.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/upstream-502", nil)) + if response.Code != http.StatusBadGateway { + t.Fatalf("response status = %d, want %d", response.Code, http.StatusBadGateway) + } + var record map[string]any + if err := json.Unmarshal(logs.Bytes(), &record); err != nil { + t.Fatalf("decode structured request outcome %q: %v", logs.String(), err) + } + if record["outcome"] != "success" { + t.Fatalf("upstream 502 outcome = %v, want success; log = %s", record["outcome"], logs.String()) + } +} + func waitForActiveRequests(proxy *Proxy, want int64, timeout time.Duration) error { deadline := time.Now().Add(timeout) for proxy.ActiveRequests() != want && time.Now().Before(deadline) { diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go new file mode 100644 index 0000000..4bb92f2 --- /dev/null +++ b/internal/gateway/gateway.go @@ -0,0 +1,275 @@ +// Package gateway connects the compiled route matcher to cluster selection and +// reusable endpoint proxies for one static data-plane snapshot. +package gateway + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "strconv" + "sync" + "time" + + "github.com/SShogun/GateHouse/internal/cluster" + "github.com/SShogun/GateHouse/internal/config" + "github.com/SShogun/GateHouse/internal/dataplane" + "github.com/SShogun/GateHouse/internal/router" +) + +// Options supplies dependencies that need deterministic control in runtime +// tests. Nil values select their standard library defaults. +type Options struct { + ProbeClient *http.Client +} + +type endpointRuntime struct { + proxy *dataplane.Proxy +} + +type clusterRuntime struct { + config config.Cluster + selection *cluster.Cluster + endpoints map[string]*endpointRuntime +} + +// Gateway owns a compiled route set, cluster health state, endpoint proxies, +// and the active health-check scheduler. +type Gateway struct { + routes *router.Matcher + clusters map[string]*clusterRuntime + logger *slog.Logger + scheduler *cluster.Scheduler + probeHTTP *http.Client + closeOnce sync.Once +} + +const ( + defaultProbeTimeout = 5 * time.Second + maxProbeConcurrency = 16 +) + +// New validates and constructs the data-plane runtime. +func New(cfg config.Config, logger *slog.Logger) (*Gateway, error) { + return NewWithOptions(cfg, logger, Options{}) +} + +// NewWithOptions validates and constructs a gateway with injectable probe HTTP. +func NewWithOptions(cfg config.Config, logger *slog.Logger, options Options) (*Gateway, error) { + if err := config.Validate(cfg); err != nil { + return nil, fmt.Errorf("validate gateway config: %w", err) + } + if logger == nil { + logger = slog.Default() + } + clusterIDs := make([]string, 0, len(cfg.Clusters)) + for _, item := range cfg.Clusters { + clusterIDs = append(clusterIDs, item.ID) + } + routes, err := router.Compile(router.Config{Routes: cfg.Routes}, clusterIDs) + if err != nil { + return nil, fmt.Errorf("compile gateway routes: %w", err) + } + probeTimeout := defaultProbeTimeout + for _, configured := range cfg.Clusters { + if configured.HealthCheck != nil && configured.HealthCheck.Timeout > probeTimeout { + probeTimeout = configured.HealthCheck.Timeout + } + } + g := &Gateway{ + routes: routes, clusters: make(map[string]*clusterRuntime, len(cfg.Clusters)), + logger: logger, + scheduler: cluster.NewSchedulerWithOptions(cluster.SchedulerOptions{ + ProbeTimeout: probeTimeout, MaxConcurrent: maxProbeConcurrency, + }), + probeHTTP: options.ProbeClient, + } + if g.probeHTTP == nil { + g.probeHTTP = http.DefaultClient + } + type scheduledHealthCheck struct { + id string + interval time.Duration + probe cluster.Probe + } + var schedules []scheduledHealthCheck + for _, configured := range cfg.Clusters { + selection, err := cluster.New(cluster.Config{Endpoints: configured.Endpoints, Health: configured.Health}) + if err != nil { + g.Close() + return nil, fmt.Errorf("create cluster %q: %w", configured.ID, err) + } + state := &clusterRuntime{config: configured, selection: selection, endpoints: make(map[string]*endpointRuntime, len(configured.Endpoints))} + g.clusters[configured.ID] = state + for _, ep := range configured.Endpoints { + proxy, err := dataplane.NewProxy(dataplane.Config{UpstreamURL: ep.Address}, logger) + if err != nil { + g.Close() + return nil, fmt.Errorf("create proxy for cluster %q endpoint %q: %w", configured.ID, ep.ID, err) + } + state.endpoints[ep.ID] = &endpointRuntime{proxy: proxy} + if configured.HealthCheck != nil { + clusterID, endpointID, health := configured.ID, ep.ID, *configured.HealthCheck + address := ep.Address + schedules = append(schedules, scheduledHealthCheck{ + id: scheduleID(clusterID, endpointID), interval: health.Interval, + probe: func(parent context.Context) error { + return g.checkEndpoint(parent, clusterID, endpointID, address, health) + }, + }) + } + } + } + // Start workers only after the cluster map and every endpoint proxy are + // complete; Add runs its first probe immediately on another goroutine. + for _, schedule := range schedules { + if err := g.scheduler.Add(schedule.id, schedule.interval, schedule.probe); err != nil { + g.Close() + return nil, fmt.Errorf("schedule health check %q: %w", schedule.id, err) + } + } + return g, nil +} + +func scheduleID(clusterID, endpointID string) string { + return strconv.Itoa(len(clusterID)) + ":" + clusterID + strconv.Itoa(len(endpointID)) + ":" + endpointID +} + +func (g *Gateway) checkEndpoint(parent context.Context, clusterID, endpointID, address string, health config.HealthCheck) error { + ctx, cancel := context.WithTimeout(parent, health.Timeout) + defer cancel() + target, err := url.Parse(address) + if err == nil { + var probePath *url.URL + probePath, err = url.ParseRequestURI(health.Path) + if err == nil { + target.Path = probePath.Path + target.RawPath = probePath.RawPath + } + target.RawQuery = "" + target.Fragment = "" + } + success := false + if err == nil { + request, requestErr := http.NewRequestWithContext(ctx, http.MethodGet, target.String(), nil) + if requestErr == nil { + response, doErr := g.probeHTTP.Do(request) + if doErr == nil { + _, copyErr := io.Copy(io.Discard, response.Body) + _ = response.Body.Close() + success = copyErr == nil && ctx.Err() == nil && response.StatusCode >= 200 && response.StatusCode < 300 + } + } + } + markErr := g.MarkResult(clusterID, endpointID, success) + if err != nil { + return err + } + if markErr != nil { + return markErr + } + if !success { + return errors.New("health probe failed") + } + return nil +} + +// MarkResult applies a probe outcome. It is also a deterministic seam for +// health integrations that deliver results from outside the HTTP scheduler. +func (g *Gateway) MarkResult(clusterID, endpointID string, success bool) error { + state, ok := g.clusters[clusterID] + if !ok { + return fmt.Errorf("unknown cluster %q", clusterID) + } + return state.selection.MarkResult(endpointID, success) +} + +// ServeHTTP matches a request, selects an eligible endpoint, and forwards it. +func (g *Gateway) ServeHTTP(w http.ResponseWriter, r *http.Request) { + started := time.Now() + route, matched := g.routes.Match(router.Request{Host: r.Host, Path: r.URL.EscapedPath(), Method: r.Method, Headers: r.Header}) + fields := []any{"method", r.Method, "route_id", "", "cluster_id", "", "endpoint_id", ""} + if !matched { + http.NotFound(w, r) + g.logRequest(fields, http.StatusNotFound, started) + return + } + fields[3] = route.ID + fields[5] = route.ClusterID + clusterState := g.clusters[route.ClusterID] + endpoint, err := clusterState.selection.Select(clusterState.config.Policy, stableSelectionSource{}) + if err != nil { + http.Error(w, "service unavailable", http.StatusServiceUnavailable) + g.logRequest(fields, http.StatusServiceUnavailable, started) + return + } + fields[7] = endpoint.ID + response := &telemetryWriter{ResponseWriter: w} + func() { + defer func() { + if recovered := recover(); recovered != nil { + status := response.status + g.logRequest(fields, status, started) + panic(recovered) + } + }() + clusterState.endpoints[endpoint.ID].proxy.ServeHTTP(response, r) + }() + status := response.status + if status == 0 { + status = http.StatusOK + } + g.logRequest(fields, status, started) +} + +func (g *Gateway) logRequest(fields []any, status int, started time.Time) { + g.logger.Info("gateway request complete", append(fields, + "status", status, "duration", time.Since(started), + )...) +} + +type stableSelectionSource struct{} + +func (stableSelectionSource) Uint64n(n uint64) uint64 { return 0 } + +type telemetryWriter struct { + http.ResponseWriter + status int +} + +func (w *telemetryWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter } + +func (w *telemetryWriter) WriteHeader(status int) { + if w.status != 0 { + return + } + if status >= 100 && status < 200 && status != http.StatusSwitchingProtocols { + w.ResponseWriter.WriteHeader(status) + return + } + w.status = status + w.ResponseWriter.WriteHeader(status) +} + +func (w *telemetryWriter) Write(body []byte) (int, error) { + if w.status == 0 { + w.WriteHeader(http.StatusOK) + } + return w.ResponseWriter.Write(body) +} + +// Close stops and joins health checks, then releases idle proxy connections. +func (g *Gateway) Close() error { + g.closeOnce.Do(func() { + g.scheduler.Stop() + for _, state := range g.clusters { + for _, endpoint := range state.endpoints { + endpoint.proxy.CloseIdleConnections() + } + } + }) + return nil +} diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go new file mode 100644 index 0000000..31013ee --- /dev/null +++ b/internal/gateway/gateway_test.go @@ -0,0 +1,426 @@ +package gateway + +import ( + "bytes" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/SShogun/GateHouse/internal/cluster" + "github.com/SShogun/GateHouse/internal/config" + "github.com/SShogun/GateHouse/internal/router" +) + +func TestGatewayRoutesAndLogsStableIDs(t *testing.T) { + upstreamA := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = io.WriteString(w, "a") })) + defer upstreamA.Close() + upstreamB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = io.WriteString(w, "b") })) + defer upstreamB.Close() + var logs bytes.Buffer + logger := slog.New(slog.NewJSONHandler(&logs, nil)) + gw, err := New(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{ + {ID: "route-a", Path: "/a", PathType: router.PathPrefix, ClusterID: "cluster-a"}, + {ID: "route-b", Path: "/b", PathType: router.PathPrefix, ClusterID: "cluster-b"}, + }, + Clusters: []config.Cluster{ + {ID: "cluster-a", Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: "endpoint-a", Address: upstreamA.URL, Weight: 1}}}, + {ID: "cluster-b", Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: "endpoint-b", Address: upstreamB.URL, Weight: 1}}}, + }, + }, logger) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + + rec := httptest.NewRecorder() + gw.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "http://gateway/b/resource", nil)) + if rec.Code != http.StatusOK || rec.Body.String() != "b" { + t.Fatalf("response = %d %q", rec.Code, rec.Body.String()) + } + for _, id := range []string{"route-b", "cluster-b", "endpoint-b"} { + if !strings.Contains(logs.String(), `"`+map[string]string{"route-b": "route_id", "cluster-b": "cluster_id", "endpoint-b": "endpoint_id"}[id]+`":"`+id+`"`) { + t.Errorf("structured telemetry missing %s: %s", id, logs.String()) + } + } +} + +func TestValidUpstream502IsNotLabeledAsGatewayFailure(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "upstream error", http.StatusBadGateway) + })) + defer upstream.Close() + var logs bytes.Buffer + gw, err := New(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{{ID: "r", Path: "/", PathType: router.PathPrefix, ClusterID: "c"}}, + Clusters: []config.Cluster{{ID: "c", Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: "e", Address: upstream.URL, Weight: 1}}}}, + }, slog.New(slog.NewJSONHandler(&logs, nil))) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + + rec := httptest.NewRecorder() + gw.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "http://gateway/", nil)) + if rec.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502", rec.Code) + } + for _, line := range strings.Split(logs.String(), "\n") { + if strings.Contains(line, `"msg":"gateway request complete"`) && strings.Contains(line, `"outcome":"failure"`) { + t.Fatalf("valid upstream status was mislabeled as a gateway failure: %s", line) + } + } +} + +func TestAbortedRequestLogRetainsRouteAndClusterIDs(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "100") + _, _ = io.WriteString(w, "x") + })) + defer upstream.Close() + var logs bytes.Buffer + gw, err := New(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{{ID: "route-abort", Path: "/", PathType: router.PathPrefix, ClusterID: "cluster-abort"}}, + Clusters: []config.Cluster{{ID: "cluster-abort", Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: "endpoint-abort", Address: upstream.URL, Weight: 1}}}}, + }, slog.New(slog.NewJSONHandler(&logs, nil))) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + handlerDone := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer close(handlerDone) + gw.ServeHTTP(w, r) + })) + defer server.Close() + response, err := http.Get(server.URL + "/") + if err != nil { + t.Fatal(err) + } + _, bodyErr := io.Copy(io.Discard, response.Body) + _ = response.Body.Close() + if bodyErr == nil { + t.Fatal("truncated upstream body was read as a complete response") + } + select { + case <-handlerDone: + case <-time.After(time.Second): + t.Fatal("gateway handler did not finish after abort") + } + for _, want := range []string{`"msg":"gateway request complete"`, `"route_id":"route-abort"`, `"cluster_id":"cluster-abort"`, `"endpoint_id":"endpoint-abort"`} { + if !strings.Contains(logs.String(), want) { + t.Errorf("abort telemetry missing %s: %s", want, logs.String()) + } + } +} + +func TestGatewayRejectsInvalidConfig(t *testing.T) { + _, err := New(config.Config{}, nil) + if err == nil { + t.Fatal("New accepted config with no clusters") + } +} + +func TestHealthProbeSchedulerDoesNotPreemptConfiguredTimeout(t *testing.T) { + observed := make(chan time.Duration, 1) + client := &http.Client{Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + deadline, ok := r.Context().Deadline() + if !ok { + t.Error("probe request has no deadline") + } else { + observed <- time.Until(deadline) + } + return &http.Response{StatusCode: http.StatusNoContent, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("")), Request: r}, nil + })} + gw, err := NewWithOptions(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Clusters: []config.Cluster{{ID: "c", Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: "e", Address: "http://upstream.test", Weight: 1}}, HealthCheck: &config.HealthCheck{Path: "/health", Interval: time.Hour, Timeout: 8 * time.Second}}}, + }, nil, Options{ProbeClient: client}) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + select { + case remaining := <-observed: + if remaining <= 5*time.Second { + t.Fatalf("scheduler deadline %s preempts configured 8s timeout", remaining) + } + case <-time.After(time.Second): + t.Fatal("scheduled probe did not execute") + } +} + +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return fn(r) } + +func TestGatewayCloseStopsHealthScheduler(t *testing.T) { + probeStarted := make(chan struct{}) + probeDone := make(chan struct{}) + probe := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(probeStarted) + <-r.Context().Done() + close(probeDone) + })) + defer probe.Close() + gw, err := NewWithOptions(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{{ID: "r", Path: "/", PathType: router.PathPrefix, ClusterID: "c"}}, + Clusters: []config.Cluster{{ID: "c", Policy: cluster.RoundRobin, Endpoints: []cluster.Endpoint{{ID: "e", Address: probe.URL, Weight: 1}}, HealthCheck: &config.HealthCheck{Path: "/health", Interval: time.Hour, Timeout: time.Hour}}}, + }, nil, Options{ProbeClient: probe.Client()}) + if err != nil { + t.Fatal(err) + } + select { + case <-probeStarted: + case <-time.After(time.Second): + t.Fatal("probe did not start") + } + if err := gw.Close(); err != nil { + t.Fatal(err) + } + select { + case <-probeDone: + case <-time.After(time.Second): + t.Fatal("scheduler did not cancel probe") + } +} + +func TestUnhealthyEndpointIsExcludedAfterProbe(t *testing.T) { + probeStarted := make(chan struct{}, 1) + bad := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/health" { + select { + case probeStarted <- struct{}{}: + default: + } + http.Error(w, "down", http.StatusServiceUnavailable) + return + } + _, _ = io.WriteString(w, "bad") + })) + defer bad.Close() + good := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, _ = io.WriteString(w, "good") })) + defer good.Close() + gw, err := NewWithOptions(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{{ID: "r", Path: "/", PathType: router.PathPrefix, ClusterID: "c"}}, + Clusters: []config.Cluster{{ID: "c", Policy: cluster.RoundRobin, Health: cluster.HealthThresholds{Unhealthy: 1}, Endpoints: []cluster.Endpoint{{ID: "bad", Address: bad.URL, Weight: 1}, {ID: "good", Address: good.URL, Weight: 1}}, HealthCheck: &config.HealthCheck{Path: "/health", Interval: time.Hour, Timeout: time.Second}}}, + }, nil, Options{ProbeClient: bad.Client()}) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + select { + case <-probeStarted: + case <-time.After(time.Second): + t.Fatal("health probe did not execute") + } + deadline := time.Now().Add(time.Second) + for { + state, err := gw.clusters["c"].selection.Health("bad") + if err != nil { + t.Fatal(err) + } + if state == cluster.Unhealthy { + break + } + if time.Now().After(deadline) { + t.Fatalf("failing configured probe did not mark endpoint unhealthy; state=%s", state) + } + time.Sleep(time.Millisecond) + } + + rec := httptest.NewRecorder() + gw.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "http://gateway/", nil)) + if rec.Body.String() != "good" { + t.Fatalf("selected unhealthy endpoint: response %q", rec.Body.String()) + } +} + +func TestIncompleteSuccessfulHealthResponseMarksEndpointUnhealthy(t *testing.T) { + bad := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/health" { + w.Header().Set("Content-Length", "100") + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "short") + return + } + _, _ = io.WriteString(w, "bad") + })) + defer bad.Close() + good := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/health" { + w.WriteHeader(http.StatusNoContent) + return + } + _, _ = io.WriteString(w, "good") + })) + defer good.Close() + gw, err := New(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{{ID: "r", Path: "/", PathType: router.PathPrefix, ClusterID: "c"}}, + Clusters: []config.Cluster{{ID: "c", Policy: cluster.RoundRobin, Health: cluster.HealthThresholds{Unhealthy: 1}, Endpoints: []cluster.Endpoint{ + {ID: "bad", Address: bad.URL, Weight: 1}, {ID: "good", Address: good.URL, Weight: 1}, + }, HealthCheck: &config.HealthCheck{Path: "/health", Interval: time.Hour, Timeout: time.Second}}}, + }, nil) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + + deadline := time.Now().Add(time.Second) + for { + state, err := gw.clusters["c"].selection.Health("bad") + if err != nil { + t.Fatal(err) + } + if state == cluster.Unhealthy { + break + } + if time.Now().After(deadline) { + t.Fatalf("incomplete 200 health response left endpoint %s", state) + } + time.Sleep(time.Millisecond) + } + recorder := httptest.NewRecorder() + gw.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "http://gateway/", nil)) + if recorder.Body.String() != "good" { + t.Fatalf("selected endpoint after incomplete probe = %q, want good", recorder.Body.String()) + } +} + +func TestConcurrentRequestsAndHealthTransitions(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, "ok") + })) + defer upstream.Close() + gw, err := New(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Routes: []router.Route{{ID: "r", Path: "/", PathType: router.PathPrefix, ClusterID: "c"}}, + Clusters: []config.Cluster{{ID: "c", Policy: cluster.RoundRobin, Health: cluster.HealthThresholds{Healthy: 1, Unhealthy: 1}, Endpoints: []cluster.Endpoint{ + {ID: "a", Address: upstream.URL, Weight: 1}, {ID: "b", Address: upstream.URL, Weight: 1}, + }}}, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + + const requestCount = 200 + var wg sync.WaitGroup + wg.Add(requestCount + 1) + for i := 0; i < requestCount; i++ { + go func() { + defer wg.Done() + response := httptest.NewRecorder() + gw.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "http://gateway/", nil)) + if response.Code != http.StatusOK { + t.Errorf("concurrent request status = %d, want 200", response.Code) + } + }() + } + go func() { + defer wg.Done() + for i := 0; i < requestCount; i++ { + if err := gw.MarkResult("c", "a", i%2 == 0); err != nil { + t.Errorf("concurrent health transition: %v", err) + return + } + } + }() + wg.Wait() +} + +func TestGatewayHealthProbeStartupUsesCompleteClusterMap(t *testing.T) { + const clusterCount = 128 + checked := make(chan struct{}, clusterCount) + client := &http.Client{Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + checked <- struct{}{} + return &http.Response{StatusCode: http.StatusNoContent, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("")), Request: r}, nil + })} + cfg := config.Config{Listener: config.Listener{Address: "127.0.0.1:0"}} + for i := 0; i < clusterCount; i++ { + clusterID := fmt.Sprintf("cluster-%03d", i) + cfg.Clusters = append(cfg.Clusters, config.Cluster{ + ID: clusterID, Policy: cluster.RoundRobin, + Endpoints: []cluster.Endpoint{{ID: "endpoint", Address: "http://upstream.test", Weight: 1}}, + Health: cluster.HealthThresholds{Healthy: 1, Unhealthy: 1}, + HealthCheck: &config.HealthCheck{Path: "/health", Interval: time.Hour, Timeout: time.Second}, + }) + } + gw, err := NewWithOptions(cfg, slog.New(slog.NewTextHandler(io.Discard, nil)), Options{ProbeClient: client}) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + for i := 0; i < clusterCount; i++ { + select { + case <-checked: + case <-time.After(3 * time.Second): + t.Fatalf("only %d of %d initial health probes ran", i, clusterCount) + } + } + deadline := time.NewTimer(3 * time.Second) + defer deadline.Stop() + ticker := time.NewTicker(time.Millisecond) + defer ticker.Stop() + for { + allHealthy := true + for _, state := range gw.clusters { + health, err := state.selection.Health("endpoint") + if err != nil || health != cluster.Healthy { + allHealthy = false + break + } + } + if allHealthy { + break + } + select { + case <-ticker.C: + case <-deadline.C: + t.Fatal("not all clusters became healthy after their initial probes") + } + } +} + +func TestGatewayHealthProbePreservesEscapedPath(t *testing.T) { + requestURI := make(chan string, 1) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestURI <- r.RequestURI + w.WriteHeader(http.StatusNoContent) + })) + defer upstream.Close() + + gw, err := New(config.Config{ + Listener: config.Listener{Address: "127.0.0.1:0"}, + Clusters: []config.Cluster{{ + ID: "c", Policy: cluster.RoundRobin, + Endpoints: []cluster.Endpoint{{ID: "e", Address: upstream.URL, Weight: 1}}, + HealthCheck: &config.HealthCheck{Path: "/ready%2Fdeep", Interval: time.Hour, Timeout: time.Second}, + }}, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatal(err) + } + defer gw.Close() + + select { + case got := <-requestURI: + if got != "/ready%2Fdeep" { + t.Fatalf("health probe RequestURI = %q, want %q", got, "/ready%2Fdeep") + } + case <-time.After(3 * time.Second): + t.Fatal("initial health probe did not reach the upstream") + } +} diff --git a/internal/router/router.go b/internal/router/router.go index e1b1b3e..7944c66 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -50,7 +50,13 @@ type Request struct { // Matcher is an immutable compiled route set safe for concurrent matching. type Matcher struct { - routes []Route + root *pathNode +} + +type pathNode struct { + children map[byte]*pathNode + prefix map[string][]Route + exact map[string][]Route } // Compile validates route configuration and cluster references, then creates @@ -109,7 +115,33 @@ func Compile(config Config, clusterIDs []string) (*Matcher, error) { } } sort.Slice(routes, func(i, j int) bool { return routes[i].ID < routes[j].ID }) - return &Matcher{routes: routes}, nil + root := &pathNode{} + for _, route := range routes { + node := root + for i := 0; i < len(route.Path); i++ { + if node.children == nil { + node.children = make(map[byte]*pathNode) + } + if node.children[route.Path[i]] == nil { + node.children[route.Path[i]] = &pathNode{} + } + node = node.children[route.Path[i]] + } + index := node.prefix + if route.PathType == PathExact { + index = node.exact + } + if index == nil { + index = make(map[string][]Route) + if route.PathType == PathExact { + node.exact = index + } else { + node.prefix = index + } + } + index[route.Host] = append(index[route.Host], route) + } + return &Matcher{root: root}, nil } // Match returns the highest-precedence matching route. A nil Matcher behaves @@ -120,13 +152,42 @@ func (m *Matcher) Match(request Request) (Route, bool) { } var winner Route found := false - for _, route := range m.routes { - if !routeMatches(route, request) { - continue + host := normalizeRequestHost(request.Host) + method := strings.ToUpper(request.Method) + visit := func(node *pathNode, exact bool) { + index := node.prefix + if exact { + index = node.exact + } + if index == nil { + return + } + check := func(routes []Route) { + for _, route := range routes { + if routeSelectorsMatch(route, host, request.Path, method, request.Headers) && (!found || routePrecedes(route, winner)) { + winner, found = route, true + } + } } - if !found || routePrecedes(route, winner) { - winner, found = route, true + check(index[""]) + check(index[host]) + for i := 0; i < len(host); i++ { + if host[i] == '.' { + check(index["*."+host[i+1:]]) + } + } + } + node := m.root + visit(node, false) + for i := 0; i < len(request.Path); i++ { + node = node.children[request.Path[i]] + if node == nil { + break } + visit(node, false) + } + if node != nil { + visit(node, true) } if !found { return Route{}, false @@ -146,22 +207,22 @@ func cloneRoute(route Route) Route { return route } -func routeMatches(route Route, request Request) bool { - host := normalizeRequestHost(request.Host) +func routeSelectorsMatch(route Route, host, path, method string, headers http.Header) bool { if !hostMatches(route.Host, host) { return false } - if route.PathType == PathExact && request.Path != route.Path { + if route.PathType == PathExact && path != route.Path { return false } - if route.PathType == PathPrefix && !strings.HasPrefix(request.Path, route.Path) { + if route.PathType == PathPrefix && !strings.HasPrefix(path, route.Path) { return false } - if len(route.Methods) > 0 && !contains(route.Methods, strings.ToUpper(request.Method)) { + if len(route.Methods) > 0 && !contains(route.Methods, method) { return false } for name, value := range route.Headers { - if request.Headers.Get(name) != value { + values, exists := headers[name] + if !exists || len(values) != 1 || values[0] != value { return false } } @@ -268,7 +329,7 @@ func normalizeHeaders(headers map[string]string) (map[string]string, error) { out := make(map[string]string, len(headers)) for name, value := range headers { canonical := http.CanonicalHeaderKey(name) - if canonical == "" || strings.ContainsAny(canonical, " \t\r\n:") { + if !validHeaderName(name) { return nil, fmt.Errorf("invalid header constraint name %q", name) } if _, exists := out[canonical]; exists { @@ -279,6 +340,19 @@ func normalizeHeaders(headers map[string]string) (map[string]string, error) { return out, nil } +func validHeaderName(name string) bool { + if name == "" { + return false + } + for i := 0; i < len(name); i++ { + c := name[i] + if !(c >= '0' && c <= '9' || c >= 'A' && c <= 'Z' || c >= 'a' && c <= 'z' || strings.ContainsRune("!#$%&'*+-.^_`|~", rune(c))) { + return false + } + } + return true +} + func ambiguous(a, b Route) bool { if !sameRank(a, b) || !routesOverlap(a, b) { return false @@ -353,7 +427,8 @@ func sameSelectors(a, b Route) bool { return false } for name, value := range a.Headers { - if b.Headers[name] != value { + other, exists := b.Headers[name] + if !exists || other != value { return false } } diff --git a/internal/router/router_test.go b/internal/router/router_test.go index 2a4ae53..ae85a8a 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -1,7 +1,10 @@ package router import ( + "fmt" + "net" "net/http" + "strings" "sync" "testing" ) @@ -190,6 +193,16 @@ func TestCompileRejectsAmbiguousRoutes(t *testing.T) { } } +func TestCompileRejectsOverlappingRoutesWithDifferentEmptyHeaderSelectors(t *testing.T) { + routes := []Route{ + {ID: "a", Path: "/", PathType: PathPrefix, Headers: map[string]string{"X-A": ""}, ClusterID: "c"}, + {ID: "b", Path: "/", PathType: PathPrefix, Headers: map[string]string{"X-B": ""}, ClusterID: "c"}, + } + if _, err := Compile(Config{Routes: routes}, []string{"c"}); err == nil { + t.Fatal("Compile() accepted overlapping, different header selectors") + } +} + func TestCompileRejectsUnknownClusterReference(t *testing.T) { _, err := Compile(Config{Routes: []Route{{ID: "r", Path: "/", PathType: PathPrefix, ClusterID: "missing"}}}, nil) if err == nil { @@ -197,9 +210,38 @@ func TestCompileRejectsUnknownClusterReference(t *testing.T) { } } +func TestHeaderConstraintRequiresExactlyOnePresentValue(t *testing.T) { + m, err := Compile(Config{Routes: []Route{{ID: "r", Path: "/", PathType: PathPrefix, Headers: map[string]string{"X-Mode": ""}, ClusterID: "c"}}}, []string{"c"}) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + values []string + want bool + }{ + {"missing", nil, false}, {"present empty", []string{""}, true}, {"wrong", []string{"other"}, false}, {"repeated", []string{"", ""}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + _, ok := m.Match(Request{Path: "/", Headers: http.Header{"X-Mode": tc.values}}) + if ok != tc.want { + t.Fatalf("Match() ok=%v, want %v", ok, tc.want) + } + }) + } +} + +func TestCompileRejectsMalformedHeaderTokenName(t *testing.T) { + _, err := Compile(Config{Routes: []Route{{ID: "r", Path: "/", PathType: PathPrefix, Headers: map[string]string{"X@Bad": "x"}, ClusterID: "c"}}}, []string{"c"}) + if err == nil { + t.Fatal("Compile() accepted malformed HTTP header token name") + } +} + func FuzzRouteMatch(f *testing.F) { f.Add("api.example.com", "/v1/users", "GET", "application/json") f.Add("API.EXAMPLE.COM:443", "/a%2Fb//../c", "POST", "text/plain") + f.Add("[api.example.com]", "/v1", "GET", "application/json") f.Add("x.example.com", "", "", "") f.Fuzz(func(t *testing.T, host, path, method, contentType string) { // Keep fuzz inputs bounded so this property stays a matcher test rather @@ -223,19 +265,117 @@ func FuzzRouteMatch(f *testing.F) { if firstOK != secondOK || first.ID != second.ID { t.Fatalf("nondeterministic Match(): (%q,%v) then (%q,%v)", first.ID, firstOK, second.ID, secondOK) } - if firstOK { - winnerMatches := false - for _, route := range routes { - if routeMatches(route, req) && routePrecedes(route, first) { - t.Fatalf("winner %q loses precedence to matching route %q", first.ID, route.ID) - } - if route.ID == first.ID && routeMatches(route, req) { - winnerMatches = true - } + // Independent oracle: spell out each selector and the documented ordering + // directly; do not share production predicates with the assertion. + var expected string + for _, route := range routes { + if !oracleMatches(route, req) { + continue } - if !winnerMatches { - t.Fatalf("winner %q does not match request", first.ID) + if expected == "" || oraclePrecedes(route, routeByID(routes, expected)) { + expected = route.ID } } + if firstOK != (expected != "") || firstOK && first.ID != expected { + t.Fatalf("Match()=(%q,%v), independent oracle wants %q", first.ID, firstOK, expected) + } }) } + +func routeByID(routes []Route, id string) Route { + for _, r := range routes { + if r.ID == id { + return r + } + } + return Route{} +} + +func oracleMatches(r Route, q Request) bool { + host := strings.ToLower(strings.TrimSpace(q.Host)) + if withoutPort, _, err := net.SplitHostPort(host); err == nil { + host = withoutPort + } + // Independently mirror the public host contract: outer IPv6-style brackets + // are ignored whether or not a port is present. + host = strings.Trim(host, "[]") + if strings.Contains(r.Host, "*.") { + suffix := strings.TrimPrefix(r.Host, "*") + prefix := strings.TrimSuffix(host, suffix) + if !strings.HasSuffix(host, suffix) || prefix == "" || strings.Contains(prefix, ".") { + return false + } + } else if r.Host != "" && r.Host != host { + return false + } + if r.PathType == PathExact && q.Path != r.Path || r.PathType == PathPrefix && !strings.HasPrefix(q.Path, r.Path) { + return false + } + if len(r.Methods) > 0 { + found := false + for _, m := range r.Methods { + if m == strings.ToUpper(q.Method) { + found = true + } + } + if !found { + return false + } + } + if len(r.Headers) > 0 { + v := q.Headers["Content-Type"] + want, ok := r.Headers["Content-Type"] + if !ok || len(v) != 1 || v[0] != want { + return false + } + } + return true +} + +func oraclePrecedes(a, b Route) bool { + rank := func(h string) int { + if h == "" { + return 0 + } + if strings.HasPrefix(h, "*.") { + return 1 + } + return 2 + } + if rank(a.Host) != rank(b.Host) { + return rank(a.Host) > rank(b.Host) + } + if a.PathType != b.PathType { + return a.PathType == PathExact + } + if len(a.Path) != len(b.Path) { + return len(a.Path) > len(b.Path) + } + if (len(a.Methods) > 0) != (len(b.Methods) > 0) { + return len(a.Methods) > 0 + } + if (len(a.Headers) > 0) != (len(b.Headers) > 0) { + return len(a.Headers) > 0 + } + return a.ID < b.ID +} + +func BenchmarkMatch10KRoutes(b *testing.B) { + routes := make([]Route, 10000) + for i := range routes { + routes[i] = Route{ID: fmt.Sprintf("r%05d", i), Host: fmt.Sprintf("h%d.example.com", i), Path: "/api/", PathType: PathPrefix, ClusterID: "c"} + } + routes[9999].Host = "target.example.com" + m, err := Compile(Config{Routes: routes}, []string{"c"}) + if err != nil { + b.Fatal(err) + } + q := Request{Host: "target.example.com", Path: "/api/items", Method: "GET"} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, ok := m.Match(q); !ok { + b.Fatal("no route") + } + } +} diff --git a/internal/router/testdata/fuzz/FuzzRouteMatch/12587131861cbc78 b/internal/router/testdata/fuzz/FuzzRouteMatch/12587131861cbc78 new file mode 100644 index 0000000..d8c946b --- /dev/null +++ b/internal/router/testdata/fuzz/FuzzRouteMatch/12587131861cbc78 @@ -0,0 +1,5 @@ +go test fuzz v1 +string("..eXAmple.Com") +string("/") +string("0") +string("0") diff --git a/internal/router/testdata/fuzz/FuzzRouteMatch/3ef996ad05a4b442 b/internal/router/testdata/fuzz/FuzzRouteMatch/3ef996ad05a4b442 new file mode 100644 index 0000000..f6c3f81 --- /dev/null +++ b/internal/router/testdata/fuzz/FuzzRouteMatch/3ef996ad05a4b442 @@ -0,0 +1,5 @@ +go test fuzz v1 +string("0.EXAMPLE.COM:") +string("/") +string("0") +string("0") From 4e597872511a6c2ae6ef7dabefe86ce81ca3c53c Mon Sep 17 00:00:00 2001 From: SShogun Date: Sat, 10 Oct 2026 20:56:49 +0530 Subject: [PATCH 5/6] docs: record integrated M2 verification evidence --- docs/Gatehouse_TESTING_CI.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Gatehouse_TESTING_CI.md b/docs/Gatehouse_TESTING_CI.md index 1cffa95..a258d9b 100644 --- a/docs/Gatehouse_TESTING_CI.md +++ b/docs/Gatehouse_TESTING_CI.md @@ -795,4 +795,4 @@ M2-specific evidence: - Behavioral RED/GREEN tests cover smooth weighted round-robin cycle counts, deterministic source tie-breaking, active health transitions and exclusion, health probe cancellation, configured timeout bounds, scheduler concurrency limits, and concurrent requests during health-state changes. - The independent Sol review found and the final race run now covers startup health probes against a fully built cluster map (128 health-checked clusters); the gateway defers starting workers until runtime construction completes. Additional regressions cover incomplete 2xx health bodies, escaped health-path preservation on the HTTP wire, health paths containing query/fragment delimiters, ambiguous routes with distinct empty-valued headers, malformed listener addresses, and route/cluster IDs in abort telemetry. The fuzz oracle's seed corpus includes a bracketed host without a port as well as a host with a port, and the oracle covers both normalization forms independently. -The M2 worktree includes the same M1 `RoundTripper` outcome fix and regression tests as the M1 branch. The M2 PR targets the M1 branch; after branch integration, final pinned verification and exact-head remote CI results are recorded below. +The M2 branch now includes M1 commit `365cc464b1c70b6d71b138a04da7360ecf8f55ae` through merge commit `572da01`. Final pinned verification passed on the integrated tree. Exact-head GitHub CI status is tracked on PR #6. From c37b019e3ff7ede6e8efc749f8c24b86688c027d Mon Sep 17 00:00:00 2001 From: SShogun Date: Sun, 11 Oct 2026 00:12:03 +0530 Subject: [PATCH 6/6] fix: address proxy safety and protocol review findings --- README.md | 2 + cmd/gatehouse-data/main.go | 5 +- cmd/gatehouse-data/main_test.go | 15 + docs/Gatehouse_PR5_REVIEW.md | 123 +++ internal/config/config.go | 3 + internal/config/config_test.go | 2 + internal/dataplane/development_listener.go | 34 + internal/dataplane/proxy.go | 23 +- internal/dataplane/proxy_test.go | 137 +++- internal/dataplane/resource_limits_test.go | 845 +++++++++++++++++++++ internal/dataplane/server.go | 264 ++++++- internal/dataplane/server_test.go | 93 ++- 12 files changed, 1524 insertions(+), 22 deletions(-) create mode 100644 cmd/gatehouse-data/main_test.go create mode 100644 docs/Gatehouse_PR5_REVIEW.md create mode 100644 internal/dataplane/development_listener.go create mode 100644 internal/dataplane/resource_limits_test.go diff --git a/README.md b/README.md index 92df7ae..8581d01 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,8 @@ go run ./cmd/gatehouse-data The default command configuration matches all requests to one cluster with one upstream endpoint. The gateway runtime also supports deterministic static route matching and cluster endpoint selection, with optional scheduled HTTP health checks when configured in memory. The proxy preserves the request method, path, end-to-end headers, and body; forwards response status, end-to-end headers, and body; strips hop-by-hop headers; propagates request cancellation; and streams response chunks without waiting for the full upstream body. Retries and dynamic configuration publication/reload are not implemented. SIGINT (`Ctrl+C`) and SIGTERM stop new requests and drain in-flight requests within the shutdown timeout. The control-plane scaffold can be started with `go run ./cmd/gatehouse-control` and currently waits for a shutdown signal. +The development command accepts only literal loopback listener addresses, such as `127.0.0.1:8080` or `[::1]:8080`; wildcard addresses, non-loopback IPs, and hostnames are rejected. Authentication and TLS remain M6 work. Static defaults admit at most 128 concurrent HTTP handlers per listener, bound individual request-body read waits to 30 seconds, and bound upstream response-header waits to 30 seconds and headers to 1 MiB. Excess requests receive HTTP 503 without a queue. Active uploads and response streams retain streaming behavior; total request deadlines and route policies remain M5 work. See the [PR #5 review record](docs/Gatehouse_PR5_REVIEW.md) for dispositions and verification evidence. + ## The proof bar GateHouse treats correctness as observable behavior. The current test foundation proves lifecycle ordering and cancellation, and the contract fixture runs Buf against both a breaking and a compatible revision. As request-handling milestones arrive, the proof suite grows with the behavior: real HTTP clients and upstreams, cancellation propagation, streaming, race/leak checks, fault injection, and bounded-resource evidence. diff --git a/cmd/gatehouse-data/main.go b/cmd/gatehouse-data/main.go index efb196c..24413ec 100644 --- a/cmd/gatehouse-data/main.go +++ b/cmd/gatehouse-data/main.go @@ -29,11 +29,14 @@ func main() { } func run() error { + listenAddress := envOrDefault("GATEHOUSE_LISTEN_ADDR", "127.0.0.1:8080") + if err := dataplane.ValidateDevelopmentListenAddress(listenAddress); err != nil { + return fmt.Errorf("validate development listener: %w", err) + } signalCtx, stopSignals := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stopSignals() logger := slog.New(slog.NewJSONHandler(os.Stdout, nil)) - listenAddress := envOrDefault("GATEHOUSE_LISTEN_ADDR", "127.0.0.1:8080") upstreamAddress := envOrDefault("GATEHOUSE_UPSTREAM_URL", "http://127.0.0.1:8081") upstream, err := url.Parse(upstreamAddress) if err != nil { diff --git a/cmd/gatehouse-data/main_test.go b/cmd/gatehouse-data/main_test.go new file mode 100644 index 0000000..0d9c521 --- /dev/null +++ b/cmd/gatehouse-data/main_test.go @@ -0,0 +1,15 @@ +package main + +import ( + "strings" + "testing" +) + +func TestRunRejectsExternallyReachableDevelopmentListenerBeforeStarting(t *testing.T) { + t.Setenv("GATEHOUSE_LISTEN_ADDR", "0.0.0.0:8080") + t.Setenv("GATEHOUSE_UPSTREAM_URL", "not a URL") + err := run() + if err == nil || !strings.Contains(err.Error(), "loopback") { + t.Fatalf("run() error = %v, want loopback listener rejection before upstream setup", err) + } +} diff --git a/docs/Gatehouse_PR5_REVIEW.md b/docs/Gatehouse_PR5_REVIEW.md new file mode 100644 index 0000000..b47ffd5 --- /dev/null +++ b/docs/Gatehouse_PR5_REVIEW.md @@ -0,0 +1,123 @@ +# PR #5 Sourcery review follow-through + +This record distinguishes the bot's findings from their verified dispositions. +The review started from M1 commit `365cc46` and the dependent M2 commit +`4e59787`. Follow-up changes are local until the user approves commit and push; +the existing GitHub threads are not manually resolved by this record. + +## Findings and corrections + +| Finding | Disposition | +| --- | --- | +| [Externally reachable unauthenticated listener](https://github.com/SShogun/GateHouse/pull/5#discussion_r4237918690) | Confirmed configuration exposure. The development command validates a literal loopback IP and numeric port before constructing its runtime. Wildcard addresses, non-loopback IPs, and hostnames are rejected. The reusable server remains configurable for later authenticated deployment. | +| [Unbounded waits and concurrent work](https://github.com/SShogun/GateHouse/pull/5#discussion_r4237918693) | Confirmed missing static safeguards. Listener admission, individual request-body read waits, upstream response-header waits, and response-header size are bounded. Rejected and unread-body cleanup must also be bounded because net/http can drain the original body outside the handler. | +| [Missing Hijacker forwarding](https://github.com/SShogun/GateHouse/pull/5#discussion_r4237918697) | False positive for the pinned Go 1.26.9 runtime. ReverseProxy calls ResponseController.Hijack, which follows statusRecorder.Unwrap. A real bidirectional HTTP 101 characterization test verifies this behavior without another forwarding wrapper. | +| [Failure outcomes logged as success](https://github.com/SShogun/GateHouse/pull/5#discussion_r4237918701) | Already fixed in `365cc46` and resolved by Sourcery. The correction and its existing regressions are preserved. | +| [Connection-header coverage gap](https://github.com/SShogun/GateHouse/pull/5#discussion_r4237918705) | Confirmed test gap. The upstream request and raw client response are checked for Connection and its nominated headers. Controlled leak mutations demonstrate that the assertions reject regressions. | +| [Shutdown timing assumption](https://github.com/SShogun/GateHouse/pull/5#discussion_r4237918707) | Confirmed test gap. A bounded listener-closure barrier replaces the 25 ms assumption; a blocked-request deadline test rejects a server that closes immediately instead of draining. | + +## Static defaults and scope + +| Limit | Default | +| --- | --- | +| Concurrent HTTP handlers per listener | 128; excess work receives HTTP 503 without a queue | +| Individual request-body read wait | 30 seconds | +| Upstream response-header wait after writing the request | 30 seconds | +| Upstream response-header size | 1 MiB | + +Zero values use these defaults; negative values for the new resource settings +are rejected. Request-body waiting limits are not a total upload deadline. +Response bodies retain progressive streaming without a write or client-wide +timeout. These static controls do not promise a total request deadline or bound +all upstream backpressure. + +The listener admission safeguard is added earlier than the policy milestone as +part of the authorized review correction. M5 still owns total/per-attempt +deadlines, route admission, retries, budgets, and circuit breakers. M6 still owns +authentication and TLS/mTLS. None of those milestone gates is marked complete +by this correction, and these development commands do not accept public binds. + +## Verification + +Final integrated `GOCACHE=/tmp/gatehouse-gocache make verify` passed with exit +code 0 on both corrected trees using Go 1.26.9, Staticcheck v0.7.0 and +govulncheck v1.8.0. The gate includes formatting, vet, Staticcheck, Buf lint and +breaking/fixture checks, clean generation, module verification, shuffled unit +tests, shuffled race tests, and vulnerability scanning. No vulnerabilities +were found. M1 was verified in the primary checkout; M2 was verified in the +source-identical normal clone described below. Their baseline commits remain +`365cc46` and `4e59787`; corrections are uncommitted. + +Focused protocol tests pass with Go 1.26.9. Controlled request-header, +response-header, and no-drain mutations fail at the intended assertions. These +are mutation/coverage proofs rather than claims that the existing proxy leaked +headers or lacked graceful shutdown. + +The pinned source proof for upgrades is in Go 1.26.9's +`net/http/httputil/reverseproxy.go:853-854` and +`net/http/responsecontroller.go:66-75`. + +The independent Luna review identified a new regression introduced by the +request-body limiter: generic body-bearing handlers lost direct Hijacker +support, and deferred cleanup could change a connection's deadline after +ownership transferred to the handler. The limiter now preserves the interface +and tracks ownership. Real retained-connection tests cover direct and +ResponseController hijacks. The follow-up also covers repeated hijacking and +restoring the body-read deadline when an attempted transfer fails. +The reviewer confirmed those bounded corrections without repeating the test +runs and reported no remaining blocker within the reviewed ownership cases. + +## Process deviations + +The initial static-safety API/helper and behavior were implemented before the +first focused RED. Subsequent body-cleanup and overload failures had real +assertion-level RED/GREEN evidence, but they do not retroactively establish +test-first ordering for every initial change. The concurrent-read case was +characterization coverage rather than a failing RED. The overload RED record +was copied from observed tool output; it is not a directly captured shell log. + +One protocol test command ran while another worker's shared package scaffold +was incomplete and failed to compile. That was a coordination failure, not a +behavioral RED. Protocol characterization and mutations then ran in an +isolated copy of the exact M1 baseline. The external engineering lessons record +the required compile barrier and per-behavior RED checkpoint. + +An initial full M1 gate passed before the independent review's hijack correction; +it is not evidence for the final source. Subsequent full checks stopped at +Staticcheck because two new tests deferred connection cleanup before checking +the dial error. Those warnings were corrected before final verification. + +The pinned Go toolchain's VCS discovery failed in the M2 worktree in this +environment: it did not recognize the worktree's `.git` file and attempted +`git status` in the sandbox's synthetic `/tmp/.git`. M2 verification therefore +uses a normal local clone at the same baseline, with identical working source +and the same `main` reference. VCS stamping and verification gates remain +enabled. This environment failure is not a behavioral RED. + +## Local evidence index + +Raw logs are retained outside the repository under `/tmp`; they are local +session evidence, not portable CI artifacts. + +| Evidence | Log | +| --- | --- | +| Static limit focused GREEN | `/tmp/gatehouse-pr5-focused-green.log` | +| Protocol characterization GREEN | `/tmp/gatehouse-pr5-protocol-green.log` | +| Request/response header mutations | `/tmp/gatehouse-pr5-protocol-request-mutant.log`, `/tmp/gatehouse-pr5-protocol-header-mutant.log` | +| Shutdown GREEN and no-drain mutation | `/tmp/gatehouse-pr5-protocol-shutdown-green.log`, `/tmp/gatehouse-pr5-protocol-shutdown-mutant.log` | +| M2 resource config RED/GREEN | `/tmp/gatehouse-pr5-m2-config-red.log`, `/tmp/gatehouse-pr5-m2-config-green.log` | +| Initial connection ownership RED/GREEN and race | `/tmp/gatehouse-pr5-hijack-red.log`, `/tmp/gatehouse-pr5-hijack-green.log`, `/tmp/gatehouse-pr5-hijack-race.log` | +| Failed/repeated hijack RED/GREEN and race | `/tmp/gatehouse-pr5-hijack-edge-red.log`, `/tmp/gatehouse-pr5-hijack-edge-green.log`, `/tmp/gatehouse-pr5-hijack-edge-race.log` | +| Final M1 full gate | `/tmp/gatehouse-pr5-m1-verify.log` | +| Final M2 full gate | `/tmp/gatehouse-pr5-m2-verify.log` | + +The existing remote heads still have successful CI: [M1 `365cc46`](https://github.com/SShogun/GateHouse/actions/runs/38063221638) +and [M2 `4e59787`](https://github.com/SShogun/GateHouse/actions/runs/38063661407). +Those runs cover the previous commits, not these uncommitted corrections. +Merge readiness still requires approved delivery and CI for the new heads. + +The failed/repeated-hijack RED predates the final test's strengthened barrier +on read-deadline installation. The revised test passes normally and with the +race detector; the earlier transition was not replayed after that test-only +revision. This distinction is preserved instead of claiming a new historical +RED. diff --git a/internal/config/config.go b/internal/config/config.go index d5eb493..d0e725d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -51,6 +51,9 @@ func Validate(cfg Config) error { if err := validateListenerAddress(cfg.Listener.Address); err != nil { return fmt.Errorf("listener address: %w", err) } + if err := cfg.Listener.Validate(); err != nil { + return fmt.Errorf("listener resource limits: %w", err) + } if len(cfg.Clusters) == 0 { return errors.New("at least one cluster is required") } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ce2bfe2..b9751c2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -67,6 +67,8 @@ func TestValidateRejectsInvalidSemantics(t *testing.T) { {"empty listener address", func(c *Config) { c.Listener.Address = "" }}, {"malformed listener address", func(c *Config) { c.Listener.Address = "not-an-address" }}, {"listener port out of range", func(c *Config) { c.Listener.Address = "127.0.0.1:70000" }}, + {"negative listener concurrency", func(c *Config) { c.Listener.MaxConcurrentRequests = -1 }}, + {"negative body read idle timeout", func(c *Config) { c.Listener.RequestBodyReadIdleTimeout = -time.Second }}, {"no clusters", func(c *Config) { c.Clusters = nil }}, {"empty cluster id", func(c *Config) { c.Clusters[0].ID = "" }}, {"duplicate cluster id", func(c *Config) { c.Clusters = append(c.Clusters, c.Clusters[0]) }}, diff --git a/internal/dataplane/development_listener.go b/internal/dataplane/development_listener.go new file mode 100644 index 0000000..7445f78 --- /dev/null +++ b/internal/dataplane/development_listener.go @@ -0,0 +1,34 @@ +package dataplane + +import ( + "errors" + "fmt" + "net" + "strconv" +) + +// ValidateDevelopmentListenAddress permits only literal loopback IPs with a +// numeric TCP port. It is intended for unauthenticated development listeners. +func ValidateDevelopmentListenAddress(address string) error { + host, port, err := net.SplitHostPort(address) + if err != nil { + return fmt.Errorf("development listener must be a literal loopback IP and numeric port: %w", err) + } + if port == "" { + return errors.New("development listener port is required") + } + for _, digit := range port { + if digit < '0' || digit > '9' { + return errors.New("development listener port must be numeric") + } + } + portNumber, err := strconv.Atoi(port) + if err != nil || portNumber < 0 || portNumber > 65535 { + return errors.New("development listener port must be between 0 and 65535") + } + ip := net.ParseIP(host) + if ip == nil || !(ip.To4() != nil && ip.IsLoopback()) && !ip.Equal(net.IPv6loopback) { + return errors.New("development listener address must be a literal loopback IP") + } + return nil +} diff --git a/internal/dataplane/proxy.go b/internal/dataplane/proxy.go index f96f911..fe5f894 100644 --- a/internal/dataplane/proxy.go +++ b/internal/dataplane/proxy.go @@ -15,9 +15,16 @@ import ( "time" ) +const ( + defaultResponseHeaderTimeout = 30 * time.Second + defaultMaxResponseHeaderBytes = 1 << 20 +) + // Config describes the single static upstream used by the M1 data plane. type Config struct { - UpstreamURL string + UpstreamURL string + ResponseHeaderTimeout time.Duration + MaxResponseHeaderBytes int64 } // Proxy forwards all requests to one configured upstream without buffering @@ -37,6 +44,18 @@ type requestOutcome struct { // NewProxy creates a reverse proxy for a static HTTP or HTTPS upstream. func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { + if cfg.ResponseHeaderTimeout < 0 { + return nil, errors.New("response header timeout cannot be negative") + } + if cfg.MaxResponseHeaderBytes < 0 { + return nil, errors.New("max response header bytes cannot be negative") + } + if cfg.ResponseHeaderTimeout == 0 { + cfg.ResponseHeaderTimeout = defaultResponseHeaderTimeout + } + if cfg.MaxResponseHeaderBytes == 0 { + cfg.MaxResponseHeaderBytes = defaultMaxResponseHeaderBytes + } target, err := url.Parse(cfg.UpstreamURL) if err != nil { return nil, fmt.Errorf("parse upstream URL: %w", err) @@ -52,6 +71,8 @@ func NewProxy(cfg Config, logger *slog.Logger) (*Proxy, error) { } transport := http.DefaultTransport.(*http.Transport).Clone() + transport.ResponseHeaderTimeout = cfg.ResponseHeaderTimeout + transport.MaxResponseHeaderBytes = cfg.MaxResponseHeaderBytes p := &Proxy{logger: logger, transport: transport} p.upstream = &httputil.ReverseProxy{ Rewrite: func(request *httputil.ProxyRequest) { diff --git a/internal/dataplane/proxy_test.go b/internal/dataplane/proxy_test.go index 93bd7ce..4b95dad 100644 --- a/internal/dataplane/proxy_test.go +++ b/internal/dataplane/proxy_test.go @@ -1,12 +1,14 @@ package dataplane import ( + "bufio" "bytes" "context" "encoding/json" "fmt" "io" "log/slog" + "net" "net/http" "net/http/httptest" "net/http/httptrace" @@ -464,9 +466,16 @@ func TestProxyLargeResponsePeakHeapDoesNotScaleWithBodySize(t *testing.T) { } func TestProxyStripsHopByHopRequestHeaders(t *testing.T) { - gotConnectionTokenHeader := make(chan string, 1) + type receivedHeaders struct { + connection string + token string + } + gotHeaders := make(chan receivedHeaders, 1) upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotConnectionTokenHeader <- r.Header.Get("X-Hop-By-Hop") + gotHeaders <- receivedHeaders{ + connection: r.Header.Get("Connection"), + token: r.Header.Get("X-Hop-By-Hop"), + } w.Header().Set("Connection", "X-Upstream-Hop") w.Header().Set("X-Upstream-Hop", "must-not-reach-client") w.WriteHeader(http.StatusNoContent) @@ -480,22 +489,128 @@ func TestProxyStripsHopByHopRequestHeaders(t *testing.T) { gateway := httptest.NewServer(proxy) defer gateway.Close() - req, err := http.NewRequest(http.MethodGet, gateway.URL+"/headers", nil) + conn, err := net.DialTimeout("tcp", strings.TrimPrefix(gateway.URL, "http://"), 2*time.Second) + if err != nil { + t.Fatalf("connect to gateway: %v", err) + } + defer conn.Close() + if err := conn.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { + t.Fatalf("set gateway connection deadline: %v", err) + } + if _, err := fmt.Fprintf(conn, "GET /headers HTTP/1.1\r\nHost: gateway\r\nConnection: X-Hop-By-Hop\r\nX-Hop-By-Hop: must-not-forward\r\n\r\n"); err != nil { + t.Fatalf("write request to gateway: %v", err) + } + reader := bufio.NewReader(conn) + statusLine, err := reader.ReadString('\n') + if err != nil { + t.Fatalf("read response status line: %v", err) + } + statusFields := strings.Fields(statusLine) + if len(statusFields) < 2 || statusFields[1] != "204" { + t.Fatalf("response status line = %q, want 204", statusLine) + } + responseHeaders, err := textproto.NewReader(reader).ReadMIMEHeader() + if err != nil { + t.Fatalf("read response headers: %v", err) + } + if got := <-gotHeaders; got.connection != "" || got.token != "" { + t.Errorf("upstream received hop-by-hop request headers: Connection=%q X-Hop-By-Hop=%q", got.connection, got.token) + } + if got := responseHeaders.Get("Connection"); got != "" { + t.Errorf("client received hop-by-hop Connection header %q", got) + } + if got := responseHeaders.Get("X-Upstream-Hop"); got != "" { + t.Errorf("client received hop-by-hop response header value %q", got) + } +} + +func TestProxyPreservesBidirectionalProtocolUpgrade(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.EqualFold(r.Header.Get("Connection"), "Upgrade") || r.Header.Get("Upgrade") != "echo" { + t.Errorf("upstream upgrade headers = Connection %q, Upgrade %q", r.Header.Get("Connection"), r.Header.Get("Upgrade")) + } + hijacker, ok := w.(http.Hijacker) + if !ok { + t.Error("upstream ResponseWriter does not support hijacking") + return + } + conn, rw, err := hijacker.Hijack() + if err != nil { + t.Errorf("hijack upstream connection: %v", err) + return + } + defer conn.Close() + if err := conn.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { + t.Errorf("set upstream connection deadline: %v", err) + return + } + if _, err := rw.WriteString("HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: echo\r\n\r\nupstream-ready"); err != nil { + t.Errorf("write upgrade response: %v", err) + return + } + if err := rw.Flush(); err != nil { + t.Errorf("flush upgrade response: %v", err) + return + } + message := make([]byte, len("client-message")) + if _, err := io.ReadFull(rw, message); err != nil { + t.Errorf("read upgraded client data: %v", err) + return + } + if _, err := conn.Write(message); err != nil { + t.Errorf("write upgraded response data: %v", err) + } + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) if err != nil { t.Fatal(err) } - req.Header.Set("Connection", "X-Hop-By-Hop") - req.Header.Set("X-Hop-By-Hop", "must-not-forward") - resp, err := http.DefaultClient.Do(req) + defer proxy.CloseIdleConnections() + gateway := httptest.NewServer(proxy) + defer gateway.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, gateway.URL+"/upgrade", nil) if err != nil { t.Fatal(err) } - _ = resp.Body.Close() - if got := <-gotConnectionTokenHeader; got != "" { - t.Errorf("upstream received hop-by-hop header value %q", got) + req.Header.Set("Connection", "Upgrade") + req.Header.Set("Upgrade", "echo") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("perform upgrade request: %v", err) + } + if resp.StatusCode != http.StatusSwitchingProtocols { + _ = resp.Body.Close() + t.Fatalf("upgrade response status = %d, want %d", resp.StatusCode, http.StatusSwitchingProtocols) } - if got := resp.Header.Get("X-Upstream-Hop"); got != "" { - t.Errorf("client received hop-by-hop response header value %q", got) + body, ok := resp.Body.(io.ReadWriteCloser) + if !ok { + _ = resp.Body.Close() + t.Fatalf("upgrade response body type %T does not support bidirectional I/O", resp.Body) + } + defer body.Close() + const upstreamMessage = "upstream-ready" + gotUpstream := make([]byte, len(upstreamMessage)) + if _, err := io.ReadFull(body, gotUpstream); err != nil { + t.Fatalf("read upgraded upstream data: %v", err) + } + if string(gotUpstream) != upstreamMessage { + t.Fatalf("upgraded upstream data = %q, want %q", gotUpstream, upstreamMessage) + } + const clientMessage = "client-message" + if _, err := io.WriteString(body, clientMessage); err != nil { + t.Fatalf("write upgraded client data: %v", err) + } + gotEcho := make([]byte, len(clientMessage)) + if _, err := io.ReadFull(body, gotEcho); err != nil { + t.Fatalf("read upgraded echo: %v", err) + } + if string(gotEcho) != clientMessage { + t.Fatalf("upgraded echo = %q, want %q", gotEcho, clientMessage) } } diff --git a/internal/dataplane/resource_limits_test.go b/internal/dataplane/resource_limits_test.go new file mode 100644 index 0000000..ec75465 --- /dev/null +++ b/internal/dataplane/resource_limits_test.go @@ -0,0 +1,845 @@ +package dataplane + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" +) + +func TestDevelopmentListenAddressRequiresLiteralLoopbackAndNumericPort(t *testing.T) { + for _, address := range []string{"127.0.0.1:0", "127.200.1.2:65535", "[::1]:8080"} { + if err := ValidateDevelopmentListenAddress(address); err != nil { + t.Errorf("ValidateDevelopmentListenAddress(%q) = %v, want success", address, err) + } + } + for _, address := range []string{"0.0.0.0:8080", "[::]:8080", "192.0.2.1:8080", "localhost:8080", "127.0.0.1:http", "127.0.0.1:-1", "127.0.0.1:65536", "127.0.0.1", "127.0.0.1:80:90"} { + if err := ValidateDevelopmentListenAddress(address); err == nil { + t.Errorf("ValidateDevelopmentListenAddress(%q) succeeded, want rejection", address) + } + } +} + +func TestServerResourceLimitDefaultsAndRejectsNegativeValues(t *testing.T) { + server, err := NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), ServerConfig{Address: "127.0.0.1:0"}) + if err != nil { + t.Fatal(err) + } + if server.cfg.MaxConcurrentRequests != 128 { + t.Errorf("MaxConcurrentRequests = %d, want 128", server.cfg.MaxConcurrentRequests) + } + if server.cfg.RequestBodyReadIdleTimeout != 30*time.Second { + t.Errorf("RequestBodyReadIdleTimeout = %s, want 30s", server.cfg.RequestBodyReadIdleTimeout) + } + for _, cfg := range []ServerConfig{ + {Address: "127.0.0.1:0", MaxConcurrentRequests: -1}, + {Address: "127.0.0.1:0", RequestBodyReadIdleTimeout: -time.Second}, + } { + if _, err := NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg); err == nil { + t.Errorf("NewServer(%+v) succeeded, want negative resource limit rejection", cfg) + } + } +} + +func TestProxyHeaderLimitDefaultsAndRejectsNegativeValues(t *testing.T) { + proxy, err := NewProxy(Config{UpstreamURL: "http://127.0.0.1:1"}, nil) + if err != nil { + t.Fatal(err) + } + if proxy.transport.ResponseHeaderTimeout != 30*time.Second { + t.Errorf("ResponseHeaderTimeout = %s, want 30s", proxy.transport.ResponseHeaderTimeout) + } + if proxy.transport.MaxResponseHeaderBytes != 1<<20 { + t.Errorf("MaxResponseHeaderBytes = %d, want 1MiB", proxy.transport.MaxResponseHeaderBytes) + } + for _, cfg := range []Config{ + {UpstreamURL: "http://127.0.0.1:1", ResponseHeaderTimeout: -time.Second}, + {UpstreamURL: "http://127.0.0.1:1", MaxResponseHeaderBytes: -1}, + } { + if _, err := NewProxy(cfg, nil); err == nil { + t.Errorf("NewProxy(%+v) succeeded, want negative limit rejection", cfg) + } + } +} + +func TestServerTimesOutAnIdleRequestBodyRead(t *testing.T) { + server, client := startResourceLimitServer(t, ServerConfig{RequestBodyReadIdleTimeout: 100 * time.Millisecond}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "body read failed", http.StatusRequestTimeout) + return + } + w.WriteHeader(http.StatusNoContent) + })) + defer client.CloseIdleConnections() + conn, err := net.Dial("tcp", server.Addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(2 * time.Second)) + if _, err := io.WriteString(conn, "POST / HTTP/1.1\r\nHost: local\r\nContent-Length: 5\r\n\r\nx"); err != nil { + t.Fatal(err) + } + line, err := bufio.NewReader(conn).ReadString('\n') + if err != nil { + t.Fatal(err) + } + if !strings.Contains(line, "408 Request Timeout") { + t.Fatalf("response status line = %q, want HTTP 408 after idle body read", line) + } +} + +func TestServerAllowsProgressiveBodyLongerThanOneIdleInterval(t *testing.T) { + server, client := startResourceLimitServer(t, ServerConfig{RequestBodyReadIdleTimeout: 120 * time.Millisecond}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + _, _ = w.Write(body) + })) + defer client.CloseIdleConnections() + pipeReader, pipeWriter := io.Pipe() + request, err := http.NewRequest(http.MethodPost, "http://"+server.Addr()+"/", pipeReader) + if err != nil { + t.Fatal(err) + } + result := make(chan error, 1) + go func() { + response, err := client.Do(request) + if err != nil { + result <- err + return + } + defer response.Body.Close() + got, err := io.ReadAll(response.Body) + if err == nil && string(got) != "progressive" { + err = fmt.Errorf("response body = %q", got) + } + result <- err + }() + for _, part := range []string{"pro", "gre", "ssi", "ve"} { + time.Sleep(70 * time.Millisecond) + if _, err := io.WriteString(pipeWriter, part); err != nil { + t.Fatal(err) + } + } + _ = pipeWriter.Close() + select { + case err := <-result: + if err != nil { + t.Fatalf("progressive upload failed: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("progressive upload did not finish") + } +} + +func TestServerRejectsConcurrentRequestImmediatelyAndRecoversSlot(t *testing.T) { + entered := make(chan struct{}, 2) + release := make(chan struct{}) + server, client := startResourceLimitServer(t, ServerConfig{MaxConcurrentRequests: 1}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + entered <- struct{}{} + if r.URL.Path == "/error" { + http.Error(w, "failed", http.StatusInternalServerError) + return + } + <-release + _, _ = io.WriteString(w, "ok") + })) + defer client.CloseIdleConnections() + first := make(chan *http.Response, 1) + firstErr := make(chan error, 1) + go func() { + resp, err := client.Get("http://" + server.Addr() + "/hold") + if err != nil { + firstErr <- err + return + } + first <- resp + }() + select { + case <-entered: + case <-time.After(time.Second): + t.Fatal("first request did not enter handler") + } + started := time.Now() + second, err := client.Get("http://" + server.Addr() + "/second") + if err != nil { + t.Fatal(err) + } + _ = second.Body.Close() + if second.StatusCode != http.StatusServiceUnavailable || time.Since(started) > 300*time.Millisecond { + t.Fatalf("second request status=%d elapsed=%s, want immediate 503", second.StatusCode, time.Since(started)) + } + close(release) + select { + case err := <-firstErr: + t.Fatal(err) + case resp := <-first: + _ = resp.Body.Close() + case <-time.After(time.Second): + t.Fatal("first request did not finish") + } + resp, err := client.Get("http://" + server.Addr() + "/error") + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusInternalServerError { + t.Fatalf("error request status=%d, want 500", resp.StatusCode) + } + resp, err = client.Get("http://" + server.Addr() + "/after-error") + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("request after handler error status=%d, want 200", resp.StatusCode) + } +} + +func TestServerReleasesConcurrentSlotAfterCancellationAndPanic(t *testing.T) { + started := make(chan struct{}, 1) + finished := make(chan struct{}, 1) + server, client := startResourceLimitServer(t, ServerConfig{MaxConcurrentRequests: 1}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/cancel": + started <- struct{}{} + <-r.Context().Done() + finished <- struct{}{} + case "/panic": + panic("test panic") + default: + w.WriteHeader(http.StatusNoContent) + } + })) + defer client.CloseIdleConnections() + ctx, cancel := context.WithCancel(context.Background()) + request, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://"+server.Addr()+"/cancel", nil) + if err != nil { + t.Fatal(err) + } + canceled := make(chan struct{}) + go func() { + _, _ = client.Do(request) + close(canceled) + }() + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("cancel handler did not start") + } + cancel() + select { + case <-finished: + case <-time.After(time.Second): + t.Fatal("cancel handler did not exit") + } + select { + case <-canceled: + case <-time.After(time.Second): + t.Fatal("canceled client request did not return") + } + resp, err := client.Get("http://" + server.Addr() + "/panic") + if err == nil { + _ = resp.Body.Close() + } + resp, err = client.Get("http://" + server.Addr() + "/after-panic") + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("status after panic = %d, want 204", resp.StatusCode) + } +} + +func TestServerReleasesConcurrentSlotWhenUpgradeHandlerReturns(t *testing.T) { + server, client := startResourceLimitServer(t, ServerConfig{MaxConcurrentRequests: 1}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/upgrade" { + w.WriteHeader(http.StatusNoContent) + return + } + hijacker, ok := w.(http.Hijacker) + if !ok { + t.Error("response writer does not support hijacking") + return + } + conn, rw, err := hijacker.Hijack() + if err != nil { + t.Errorf("hijack connection: %v", err) + return + } + _, _ = io.WriteString(rw, "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: test\r\n\r\n") + _ = rw.Flush() + _ = conn.Close() + })) + defer client.CloseIdleConnections() + conn, err := net.Dial("tcp", server.Addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if _, err := io.WriteString(conn, "GET /upgrade HTTP/1.1\r\nHost: local\r\nConnection: Upgrade\r\nUpgrade: test\r\n\r\n"); err != nil { + t.Fatal(err) + } + status, err := bufio.NewReader(conn).ReadString('\n') + if err != nil || !strings.Contains(status, "101 Switching Protocols") { + t.Fatalf("upgrade status line=%q err=%v", status, err) + } + resp, err := client.Get("http://" + server.Addr() + "/after-upgrade") + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("status after upgrade = %d, want 204", resp.StatusCode) + } +} + +func TestServerResourceLimitsPreserveProxyUpgradeTunnel(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hijacker, ok := w.(http.Hijacker) + if !ok { + t.Error("upstream response writer does not support hijacking") + return + } + conn, rw, err := hijacker.Hijack() + if err != nil { + t.Errorf("upstream hijack: %v", err) + return + } + defer conn.Close() + if _, err := io.WriteString(rw, "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: echo\r\n\r\nready"); err != nil { + t.Errorf("write upgrade response: %v", err) + return + } + if err := rw.Flush(); err != nil { + t.Errorf("flush upgrade response: %v", err) + return + } + message := make([]byte, len("ping")) + if _, err := io.ReadFull(rw, message); err != nil { + t.Errorf("read upgraded request data: %v", err) + return + } + _, _ = conn.Write(message) + })) + defer upstream.Close() + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + server, client := startResourceLimitServer(t, ServerConfig{MaxConcurrentRequests: 1}, proxy) + defer client.CloseIdleConnections() + conn, err := net.Dial("tcp", server.Addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(2 * time.Second)) + if _, err := io.WriteString(conn, "GET /upgrade HTTP/1.1\r\nHost: local\r\nConnection: Upgrade\r\nUpgrade: echo\r\n\r\n"); err != nil { + t.Fatal(err) + } + reader := bufio.NewReader(conn) + status, err := reader.ReadString('\n') + if err != nil || !strings.Contains(status, "101 Switching Protocols") { + t.Fatalf("upgrade response status=%q err=%v", status, err) + } + for { + line, err := reader.ReadString('\n') + if err != nil { + t.Fatal(err) + } + if line == "\r\n" { + break + } + } + ready := make([]byte, len("ready")) + if _, err := io.ReadFull(reader, ready); err != nil || string(ready) != "ready" { + t.Fatalf("upstream greeting=%q err=%v", ready, err) + } + if _, err := io.WriteString(conn, "ping"); err != nil { + t.Fatal(err) + } + echo := make([]byte, len("ping")) + if _, err := io.ReadFull(reader, echo); err != nil || string(echo) != "ping" { + t.Fatalf("upgrade echo=%q err=%v", echo, err) + } +} + +func TestBodyBearingHandlerRetainsDirectHijackConnection(t *testing.T) { + retained := make(chan net.Conn, 1) + server, client := startResourceLimitServer(t, ServerConfig{}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hijacker, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "hijacker unavailable", http.StatusInternalServerError) + return + } + conn, rw, err := hijacker.Hijack() + if err != nil { + return + } + if _, err := io.WriteString(rw, "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: body\r\n\r\n"); err != nil { + _ = conn.Close() + return + } + _ = rw.Flush() + retained <- conn + go echoHijackedMessage(conn, rw) + })) + defer client.CloseIdleConnections() + conn, err := dialBodyBearingUpgrade(t, server.Addr()) + if err != nil { + if conn != nil { + _ = conn.Close() + } + t.Fatal(err) + } + defer conn.Close() + retainedConn := awaitHijackedConn(t, retained) + defer retainedConn.Close() + assertHijackedEcho(t, conn) +} + +func TestResponseControllerHijackDoesNotInheritBodyDeadline(t *testing.T) { + retained := make(chan net.Conn, 1) + server, client := startResourceLimitServer(t, ServerConfig{RequestBodyReadIdleTimeout: 100 * time.Millisecond}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, rw, err := http.NewResponseController(w).Hijack() + if err != nil { + t.Errorf("ResponseController.Hijack: %v", err) + return + } + if _, err := io.WriteString(rw, "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: body\r\n\r\n"); err != nil { + _ = conn.Close() + t.Errorf("write upgrade response: %v", err) + return + } + _ = rw.Flush() + retained <- conn + go echoHijackedMessage(conn, rw) + })) + defer client.CloseIdleConnections() + conn, err := dialBodyBearingUpgrade(t, server.Addr()) + if err != nil { + if conn != nil { + _ = conn.Close() + } + t.Fatal(err) + } + defer conn.Close() + retainedConn := awaitHijackedConn(t, retained) + defer retainedConn.Close() + assertHijackedEcho(t, conn) +} + +func TestRepeatedHijackKeepsTransferredConnectionOwned(t *testing.T) { + retained := make(chan net.Conn, 1) + secondHijack := make(chan error, 1) + server, client := startResourceLimitServer(t, ServerConfig{RequestBodyReadIdleTimeout: 100 * time.Millisecond}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + controller := http.NewResponseController(w) + conn, rw, err := controller.Hijack() + if err != nil { + t.Errorf("first ResponseController.Hijack: %v", err) + return + } + _, _, err = controller.Hijack() + secondHijack <- err + if _, err := io.WriteString(rw, "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: body\r\n\r\n"); err != nil { + _ = conn.Close() + return + } + _ = rw.Flush() + retained <- conn + go echoHijackedMessage(conn, rw) + })) + defer client.CloseIdleConnections() + conn, err := dialBodyBearingUpgrade(t, server.Addr()) + if err != nil { + if conn != nil { + _ = conn.Close() + } + t.Fatal(err) + } + defer conn.Close() + retainedConn := awaitHijackedConn(t, retained) + defer retainedConn.Close() + select { + case err := <-secondHijack: + if !errors.Is(err, http.ErrHijacked) { + t.Fatalf("second Hijack error = %v, want http.ErrHijacked", err) + } + case <-time.After(time.Second): + t.Fatal("second Hijack did not return") + } + assertHijackedEcho(t, conn) +} + +func TestFailedHijackRestoresDeadlineForBlockedBodyRead(t *testing.T) { + const idleTimeout = 150 * time.Millisecond + readResult := make(chan error, 1) + readObserved := make(chan error, 1) + readDeadlineInstalled := make(chan struct{}, 1) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + go func() { + _, err := io.ReadFull(r.Body, make([]byte, 4)) + readResult <- err + }() + select { + case <-readDeadlineInstalled: + case <-time.After(time.Second): + t.Error("body read did not install its idle deadline") + return + } + _, _, err := http.NewResponseController(w).Hijack() + if err == nil { + t.Error("Hijack succeeded through failing test writer") + } + select { + case readErr := <-readResult: + readObserved <- readErr + case <-time.After(500 * time.Millisecond): + readObserved <- errors.New("blocked body read did not retain its idle deadline after failed Hijack") + } + }) + limited := newResourceLimitHandler(handler, 1, idleTimeout) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + limited.ServeHTTP(failingHijackResponseWriter{ResponseWriter: w, readDeadlineInstalled: readDeadlineInstalled}, r) + })) + defer server.Close() + conn, err := net.Dial("tcp", strings.TrimPrefix(server.URL, "http://")) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(2 * time.Second)) + if _, err := io.WriteString(conn, "POST / HTTP/1.1\r\nHost: local\r\nContent-Length: 4\r\n\r\n"); err != nil { + t.Fatal(err) + } + select { + case err := <-readObserved: + var netErr net.Error + if !errors.As(err, &netErr) || !netErr.Timeout() { + t.Fatalf("blocked body read error = %v, want idle timeout", err) + } + case <-time.After(2 * time.Second): + t.Fatal("handler did not observe stalled body read timeout") + } +} + +type failingHijackResponseWriter struct { + http.ResponseWriter + readDeadlineInstalled chan struct{} +} + +func (w failingHijackResponseWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter } + +func (w failingHijackResponseWriter) SetReadDeadline(deadline time.Time) error { + err := http.NewResponseController(w.ResponseWriter).SetReadDeadline(deadline) + if err == nil && !deadline.IsZero() { + select { + case w.readDeadlineInstalled <- struct{}{}: + default: + } + } + return err +} + +func (w failingHijackResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + return nil, nil, errors.New("injected Hijack failure") +} + +func dialBodyBearingUpgrade(t *testing.T, address string) (net.Conn, error) { + t.Helper() + conn, err := net.Dial("tcp", address) + if err != nil { + return nil, err + } + _ = conn.SetDeadline(time.Now().Add(time.Second)) + if _, err := io.WriteString(conn, "POST /upgrade HTTP/1.1\r\nHost: local\r\nContent-Length: 4\r\nConnection: Upgrade\r\nUpgrade: body\r\n\r\n"); err != nil { + _ = conn.Close() + return nil, err + } + reader := bufio.NewReader(conn) + status, err := reader.ReadString('\n') + if err != nil { + return conn, err + } + if !strings.Contains(status, "101 Switching Protocols") { + return conn, fmt.Errorf("upgrade status line = %q, want 101 Switching Protocols", status) + } + for { + line, err := reader.ReadString('\n') + if err != nil { + return conn, err + } + if line == "\r\n" { + break + } + } + return conn, nil +} + +func awaitHijackedConn(t *testing.T, retained <-chan net.Conn) net.Conn { + t.Helper() + select { + case conn := <-retained: + return conn + case <-time.After(time.Second): + t.Fatal("handler did not retain hijacked connection") + return nil + } +} + +func assertHijackedEcho(t *testing.T, conn net.Conn) { + t.Helper() + if _, err := io.WriteString(conn, "ping"); err != nil { + t.Fatalf("write to retained hijacked connection: %v", err) + } + echo := make([]byte, len("ping")) + if _, err := io.ReadFull(conn, echo); err != nil || string(echo) != "ping" { + t.Fatalf("retained hijack echo=%q err=%v", echo, err) + } +} + +func echoHijackedMessage(conn net.Conn, rw *bufio.ReadWriter) { + message := make([]byte, len("ping")) + if _, err := io.ReadFull(rw, message); err != nil { + return + } + _, _ = conn.Write(message) +} + +func TestRejectedRequestBodyDoesNotDelayOverloadResponse(t *testing.T) { + entered := make(chan struct{}) + release := make(chan struct{}) + var releaseOnce sync.Once + server, client := startResourceLimitServer(t, ServerConfig{ + MaxConcurrentRequests: 1, + RequestBodyReadIdleTimeout: 5 * time.Second, + }, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/hold" { + close(entered) + <-release + } + w.WriteHeader(http.StatusNoContent) + })) + defer client.CloseIdleConnections() + defer releaseOnce.Do(func() { close(release) }) + firstResponse := make(chan error, 1) + go func() { + resp, err := client.Get("http://" + server.Addr() + "/hold") + if resp != nil { + _ = resp.Body.Close() + } + firstResponse <- err + }() + select { + case <-entered: + case <-time.After(time.Second): + t.Fatal("first request did not occupy the only slot") + } + conn, err := net.Dial("tcp", server.Addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(time.Second)) + if _, err := io.WriteString(conn, "POST /overload HTTP/1.1\r\nHost: local\r\nContent-Length: 4\r\n\r\nx"); err != nil { + t.Fatal(err) + } + response, err := http.ReadResponse(bufio.NewReader(conn), &http.Request{Method: http.MethodPost}) + if err != nil { + t.Fatalf("read overload response before sending remaining body: %v", err) + } + _, _ = io.Copy(io.Discard, response.Body) + _ = response.Body.Close() + if response.StatusCode != http.StatusServiceUnavailable { + t.Fatalf("overload status = %d, want 503", response.StatusCode) + } + if _, err := conn.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { + t.Fatalf("overload connection read error = %v, want EOF after incomplete body", err) + } + releaseOnce.Do(func() { close(release) }) + select { + case err := <-firstResponse: + if err != nil { + t.Fatalf("first request failed: %v", err) + } + case <-time.After(time.Second): + t.Fatal("first request did not finish after release") + } +} + +func TestEarlyResponseWithUnreadStalledBodyIsBounded(t *testing.T) { + server, client := startResourceLimitServer(t, ServerConfig{RequestBodyReadIdleTimeout: 150 * time.Millisecond}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + defer client.CloseIdleConnections() + conn, err := net.Dial("tcp", server.Addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(time.Second)) + started := time.Now() + if _, err := io.WriteString(conn, "POST /early HTTP/1.1\r\nHost: local\r\nContent-Length: 4\r\n\r\nx"); err != nil { + t.Fatal(err) + } + response, err := http.ReadResponse(bufio.NewReader(conn), &http.Request{Method: http.MethodPost}) + if err != nil { + t.Fatalf("read early response with stalled body: %v", err) + } + _ = response.Body.Close() + if response.StatusCode != http.StatusNoContent { + t.Fatalf("early response status = %d, want 204", response.StatusCode) + } + if elapsed := time.Since(started); elapsed > 500*time.Millisecond { + t.Fatalf("early response took %s with unread stalled body, want under 500ms", elapsed) + } + if _, err := conn.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { + t.Fatalf("early-response connection read error = %v, want EOF after incomplete body", err) + } +} + +func TestEarlyResponseKeepsDeadlineWhileConcurrentBodyReadFinishes(t *testing.T) { + readStarted := make(chan struct{}) + readFinished := make(chan error, 1) + server, client := startResourceLimitServer(t, ServerConfig{RequestBodyReadIdleTimeout: 5 * time.Second}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + go func() { + close(readStarted) + _, err := io.Copy(io.Discard, r.Body) + readFinished <- err + }() + <-readStarted + time.Sleep(50 * time.Millisecond) + w.WriteHeader(http.StatusNoContent) + })) + defer client.CloseIdleConnections() + conn, err := net.Dial("tcp", server.Addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(time.Second)) + if _, err := io.WriteString(conn, "POST /early HTTP/1.1\r\nHost: local\r\nContent-Length: 4\r\n\r\nx"); err != nil { + t.Fatal(err) + } + response, err := http.ReadResponse(bufio.NewReader(conn), &http.Request{Method: http.MethodPost}) + if err != nil { + t.Fatalf("read early response while body Read is blocked: %v", err) + } + _, _ = io.Copy(io.Discard, response.Body) + _ = response.Body.Close() + if response.StatusCode != http.StatusNoContent { + t.Fatalf("early response status = %d, want 204", response.StatusCode) + } + select { + case <-readFinished: + case <-time.After(time.Second): + t.Fatal("concurrent request-body read did not stop after response") + } +} + +func TestProxyHeaderTimeoutReturnsBadGatewayAndReleasesRequest(t *testing.T) { + started := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(started) + <-r.Context().Done() + })) + defer upstream.Close() + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL, ResponseHeaderTimeout: 100 * time.Millisecond}, nil) + if err != nil { + t.Fatal(err) + } + response := httptest.NewRecorder() + proxy.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "http://gateway/slow", nil)) + if response.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502", response.Code) + } + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("upstream request did not start") + } + if got := proxy.ActiveRequests(); got != 0 { + t.Fatalf("active requests = %d, want 0 after header timeout", got) + } +} + +func TestProxyHeaderTimeoutDoesNotLimitResponseBodyAfterHeaders(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + time.Sleep(180 * time.Millisecond) + _, _ = io.WriteString(w, "late body") + })) + defer upstream.Close() + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL, ResponseHeaderTimeout: 100 * time.Millisecond}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + resp, err := http.Get(gateway.URL) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil || string(body) != "late body" { + t.Fatalf("body=%q err=%v, want late body after headers", body, err) + } +} + +func TestProxyRejectsOversizedUpstreamResponseHeaders(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Large", strings.Repeat("x", (1<<20)+1)) + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + gateway := httptest.NewServer(proxy) + defer gateway.Close() + resp, err := http.Get(gateway.URL) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("status = %d, want 502 for oversized upstream headers", resp.StatusCode) + } +} + +func startResourceLimitServer(t *testing.T, overrides ServerConfig, handler http.Handler) (*Server, *http.Client) { + t.Helper() + overrides.Address = "127.0.0.1:0" + server, err := NewServer(handler, overrides) + if err != nil { + t.Fatal(err) + } + if err := server.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _ = server.Shutdown(ctx) + }) + return server, &http.Client{Timeout: 3 * time.Second} +} diff --git a/internal/dataplane/server.go b/internal/dataplane/server.go index 31e133b..a6d99d7 100644 --- a/internal/dataplane/server.go +++ b/internal/dataplane/server.go @@ -1,9 +1,11 @@ package dataplane import ( + "bufio" "context" "errors" "fmt" + "io" "net" "net/http" "sync" @@ -11,18 +13,33 @@ import ( ) const ( - defaultReadHeaderTimeout = 5 * time.Second - defaultIdleTimeout = 90 * time.Second - defaultMaxHeaderBytes = 1 << 20 + defaultReadHeaderTimeout = 5 * time.Second + defaultIdleTimeout = 90 * time.Second + defaultMaxHeaderBytes = 1 << 20 + defaultMaxConcurrentRequests = 128 + defaultRequestBodyReadIdleTimeout = 30 * time.Second ) // ServerConfig contains listener and HTTP server resource limits. WriteTimeout // is intentionally omitted because it would break long-lived streamed bodies. type ServerConfig struct { - Address string - ReadHeaderTimeout time.Duration - IdleTimeout time.Duration - MaxHeaderBytes int + Address string + ReadHeaderTimeout time.Duration + IdleTimeout time.Duration + MaxHeaderBytes int + MaxConcurrentRequests int + RequestBodyReadIdleTimeout time.Duration +} + +// Validate rejects invalid resource-limit values. Zero values use defaults. +func (cfg ServerConfig) Validate() error { + if cfg.MaxConcurrentRequests < 0 { + return errors.New("max concurrent requests cannot be negative") + } + if cfg.RequestBodyReadIdleTimeout < 0 { + return errors.New("request body read idle timeout cannot be negative") + } + return nil } // Server owns the listener and drains active HTTP requests on shutdown. @@ -44,6 +61,9 @@ func NewServer(handler http.Handler, cfg ServerConfig) (*Server, error) { if cfg.Address == "" { return nil, errors.New("server address is required") } + if err := cfg.Validate(); err != nil { + return nil, err + } if cfg.ReadHeaderTimeout <= 0 { cfg.ReadHeaderTimeout = defaultReadHeaderTimeout } @@ -53,6 +73,12 @@ func NewServer(handler http.Handler, cfg ServerConfig) (*Server, error) { if cfg.MaxHeaderBytes <= 0 { cfg.MaxHeaderBytes = defaultMaxHeaderBytes } + if cfg.MaxConcurrentRequests == 0 { + cfg.MaxConcurrentRequests = defaultMaxConcurrentRequests + } + if cfg.RequestBodyReadIdleTimeout == 0 { + cfg.RequestBodyReadIdleTimeout = defaultRequestBodyReadIdleTimeout + } return &Server{handler: handler, cfg: cfg}, nil } @@ -69,7 +95,7 @@ func (s *Server) Start() error { } s.listener = listener s.server = &http.Server{ - Handler: s.handler, + Handler: newResourceLimitHandler(s.handler, s.cfg.MaxConcurrentRequests, s.cfg.RequestBodyReadIdleTimeout), ReadHeaderTimeout: s.cfg.ReadHeaderTimeout, IdleTimeout: s.cfg.IdleTimeout, MaxHeaderBytes: s.cfg.MaxHeaderBytes, @@ -82,6 +108,228 @@ func (s *Server) Start() error { return nil } +type resourceLimitHandler struct { + handler http.Handler + active chan struct{} + bodyReadTimeout time.Duration +} + +func newResourceLimitHandler(handler http.Handler, maxConcurrent int, bodyReadTimeout time.Duration) http.Handler { + return &resourceLimitHandler{ + handler: handler, + active: make(chan struct{}, maxConcurrent), + bodyReadTimeout: bodyReadTimeout, + } +} + +func (h *resourceLimitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + var body *idleTimedRequestBody + if h.bodyReadTimeout > 0 && r.Body != nil && r.Body != http.NoBody { + body = &idleTimedRequestBody{ReadCloser: r.Body, controller: http.NewResponseController(w), timeout: h.bodyReadTimeout} + r.Body = body + } + select { + case h.active <- struct{}{}: + defer func() { + if body != nil { + body.finishHandler(w) + } + <-h.active + }() + default: + if body != nil { + body.reject(w) + } else if r.ProtoMajor == 1 { + w.Header().Set("Connection", "close") + } + http.Error(w, "server busy", http.StatusServiceUnavailable) + return + } + if body == nil { + h.handler.ServeHTTP(w, r) + return + } + wrappedWriter := &requestBodyDeadlineWriter{ResponseWriter: w, body: body} + if _, ok := w.(http.Hijacker); ok { + h.handler.ServeHTTP(&hijackableRequestBodyDeadlineWriter{requestBodyDeadlineWriter: wrappedWriter}, r) + return + } + h.handler.ServeHTTP(wrappedWriter, r) +} + +type idleTimedRequestBody struct { + io.ReadCloser + controller *http.ResponseController + timeout time.Duration + mu sync.Mutex + sawEOF bool + finished bool + activeReads int + hijacking bool + hijacked bool +} + +func (body *idleTimedRequestBody) Read(p []byte) (int, error) { + body.mu.Lock() + if body.finished || body.hijacking || body.hijacked { + body.mu.Unlock() + return 0, http.ErrBodyReadAfterClose + } + if err := body.controller.SetReadDeadline(time.Now().Add(body.timeout)); err != nil { + body.mu.Unlock() + return 0, err + } + body.activeReads++ + body.mu.Unlock() + n, err := body.ReadCloser.Read(p) + body.mu.Lock() + body.activeReads-- + if err == io.EOF { + body.sawEOF = true + } + if !body.hijacking && !body.hijacked { + switch { + case body.finished && !body.sawEOF: + _ = body.controller.SetReadDeadline(time.Now()) + case body.activeReads > 0: + _ = body.controller.SetReadDeadline(time.Now().Add(body.timeout)) + default: + body.clearDeadline() + } + } + body.mu.Unlock() + return n, err +} + +func (body *idleTimedRequestBody) Close() error { + body.mu.Lock() + var deadlineErr error + if body.hijacking || body.hijacked { + body.mu.Unlock() + return nil + } + if !body.finished { + deadlineErr = body.controller.SetReadDeadline(time.Now().Add(body.timeout)) + } + body.mu.Unlock() + closeErr := body.ReadCloser.Close() + body.mu.Lock() + if !body.finished && !body.hijacking && !body.hijacked { + body.clearDeadline() + } + body.mu.Unlock() + return errors.Join(deadlineErr, closeErr) +} + +func (body *idleTimedRequestBody) reject(w http.ResponseWriter) { + w.Header().Set("Connection", "close") + body.mu.Lock() + if body.hijacking || body.hijacked { + body.mu.Unlock() + return + } + body.finished = true + _ = body.controller.SetReadDeadline(time.Now()) + body.mu.Unlock() +} + +func (body *idleTimedRequestBody) finishHandler(w http.ResponseWriter) { + body.mu.Lock() + if body.hijacking || body.hijacked { + body.mu.Unlock() + return + } + sawEOF := body.sawEOF + if sawEOF { + body.clearDeadline() + body.finished = true + body.mu.Unlock() + return + } + w.Header().Set("Connection", "close") + body.finished = true + _ = body.controller.SetReadDeadline(time.Now()) + body.mu.Unlock() +} + +func (body *idleTimedRequestBody) clearDeadline() { + _ = body.controller.SetReadDeadline(time.Time{}) +} + +type requestBodyDeadlineWriter struct { + http.ResponseWriter + body *idleTimedRequestBody +} + +func (w *requestBodyDeadlineWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter } + +func (w *requestBodyDeadlineWriter) WriteHeader(status int) { + if status >= http.StatusOK || status == http.StatusSwitchingProtocols { + w.body.finishHandler(w.ResponseWriter) + } + w.ResponseWriter.WriteHeader(status) +} + +func (w *requestBodyDeadlineWriter) Write(p []byte) (int, error) { + w.body.finishHandler(w.ResponseWriter) + return w.ResponseWriter.Write(p) +} + +func (w *requestBodyDeadlineWriter) Flush() { + w.body.finishHandler(w.ResponseWriter) + _ = http.NewResponseController(w.ResponseWriter).Flush() +} + +type hijackableRequestBodyDeadlineWriter struct { + *requestBodyDeadlineWriter +} + +func (w *hijackableRequestBodyDeadlineWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + if !w.body.beginHijack() { + return nil, nil, http.ErrHijacked + } + conn, rw, err := http.NewResponseController(w.ResponseWriter).Hijack() + w.body.finishHijack(err == nil) + if err != nil { + return nil, nil, err + } + return conn, rw, nil +} + +func (body *idleTimedRequestBody) beginHijack() bool { + body.mu.Lock() + defer body.mu.Unlock() + if body.hijacking || body.hijacked { + return false + } + body.hijacking = true + return true +} + +func (body *idleTimedRequestBody) finishHijack(succeeded bool) { + body.mu.Lock() + body.hijacking = false + if succeeded { + body.hijacked = true + body.clearDeadline() + body.mu.Unlock() + return + } + if body.hijacked { + body.mu.Unlock() + return + } + switch { + case body.finished && !body.sawEOF: + _ = body.controller.SetReadDeadline(time.Now()) + case body.activeReads > 0: + _ = body.controller.SetReadDeadline(time.Now().Add(body.timeout)) + default: + body.clearDeadline() + } + body.mu.Unlock() +} + // Addr returns the actual listener address after Start, including an assigned // port when Address used port zero. func (s *Server) Addr() string { diff --git a/internal/dataplane/server_test.go b/internal/dataplane/server_test.go index c9e897a..8c46777 100644 --- a/internal/dataplane/server_test.go +++ b/internal/dataplane/server_test.go @@ -2,6 +2,7 @@ package dataplane import ( "context" + "errors" "fmt" "io" "net/http" @@ -62,6 +63,11 @@ func TestServerGracefullyDrainsInFlightRequest(t *testing.T) { t.Fatal("upstream did not receive in-flight request") } shutdownResult := make(chan error, 1) + listenerClosed := make(chan error, 1) + go func() { + _, err := server.listener.Accept() + listenerClosed <- err + }() go func() { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() @@ -70,7 +76,12 @@ func TestServerGracefullyDrainsInFlightRequest(t *testing.T) { select { case err := <-shutdownResult: t.Fatalf("server shutdown returned before in-flight request drained: %v", err) - case <-time.After(25 * time.Millisecond): + case err := <-listenerClosed: + if err == nil { + t.Fatal("listener accepted an unexpected connection while shutdown started") + } + case <-time.After(2 * time.Second): + t.Fatal("server shutdown did not close listener") } releaseOnce.Do(func() { close(releaseUpstream) }) @@ -92,6 +103,86 @@ func TestServerGracefullyDrainsInFlightRequest(t *testing.T) { } } +func TestServerShutdownHonorsDeadlineWhileRequestBlocked(t *testing.T) { + upstreamStarted := make(chan struct{}) + releaseUpstream := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(upstreamStarted) + <-releaseUpstream + _, _ = io.WriteString(w, "late response") + })) + defer upstream.Close() + + proxy, err := NewProxy(Config{UpstreamURL: upstream.URL}, nil) + if err != nil { + t.Fatal(err) + } + server, err := NewServer(proxy, ServerConfig{Address: "127.0.0.1:0"}) + if err != nil { + t.Fatal(err) + } + if err := server.Start(); err != nil { + t.Fatal(err) + } + defer func() { + close(releaseUpstream) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _ = server.Shutdown(ctx) + }() + + clientResult := make(chan error, 1) + go func() { + resp, err := http.Get("http://" + server.Addr() + "/deadline") + if err == nil { + _ = resp.Body.Close() + } + clientResult <- err + }() + select { + case <-upstreamStarted: + case <-time.After(2 * time.Second): + t.Fatal("upstream did not receive in-flight request") + } + + listenerClosed := make(chan error, 1) + go func() { + _, err := server.listener.Accept() + listenerClosed <- err + }() + shutdownResult := make(chan error, 1) + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + shutdownResult <- server.Shutdown(ctx) + }() + select { + case err := <-listenerClosed: + if err == nil { + t.Fatal("listener accepted an unexpected connection while shutdown started") + } + case <-time.After(2 * time.Second): + t.Fatal("server shutdown did not close listener") + } + + select { + case err := <-shutdownResult: + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("Shutdown() error = %v, want context deadline exceeded while request is blocked", err) + } + case <-time.After(2 * time.Second): + t.Fatal("server shutdown did not return after its deadline") + } + select { + case err := <-clientResult: + if err == nil { + t.Fatal("in-flight request unexpectedly completed before upstream was released") + } + case <-time.After(2 * time.Second): + t.Fatal("forced shutdown did not end the in-flight client request") + } +} + func TestServerConfigRejectsMissingAddress(t *testing.T) { _, err := NewProxy(Config{UpstreamURL: "http://127.0.0.1:1"}, nil) if err != nil {