From 1bce603c33add3708b3e369f4839eb61c5591c92 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:12:40 +0100 Subject: [PATCH] fix(multiplayer): bound adapter HTTP calls --- multiplayer-next.md | 2 ++ server/agones/allocation.go | 21 ++++++++++++--------- server/agones/allocation_test.go | 7 +++++++ server/supervisor/supervisor.go | 7 +++++-- server/supervisor/supervisor_test.go | 10 ++++++++++ 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index 0fcec364..a9c00121 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1644,3 +1644,5 @@ Allocated team and slot assignments are now immutable after signed admission. Ma Per-IP API limiting now resolves the client behind the edge gateway instead of charging every player to the gateway's socket address. `X-Forwarded-For` is ignored unless the immediate peer belongs to an explicitly configured `--trusted-proxy-cidrs` range; trusted chains are walked from right to left past known proxies, while malformed/oversized chains fail closed to the immediate peer. The base deployment supplies private/CGNAT/ULA pod ranges under its edge-only ingress NetworkPolicy and calls out that production overlays should narrow them to the actual gateway CIDR. Tests cover spoofing from an untrusted peer, chained proxies, malformed input, invalid configuration, and independent clients behind one gateway. Allocator probes now distinguish process liveness from useful progress. `/healthz` remains live during dependency outages, while `/readyz` starts unavailable and requires a fully successful provider-list, Ready-registration, and worker cycle within `--readiness-max-stale` (30 seconds in the base deployment). The Kubernetes/Agones HTTP path is bounded by `--provider-timeout=10s`, so an unavailable provider cannot leave readiness green indefinitely; startup rejects a freshness window shorter than the poll interval plus provider timeout, and the probe listener has its own header-read deadline. Boundary and HTTP tests cover startup, exact staleness, clock reversal, recovery, method rejection, and metrics coexistence. + +The timeout boundary is enforced inside both network adapters as well as in the production allocator wiring: an `agones.Client` or game-server `Supervisor` constructed without an injected HTTP client now receives a ten-second client rather than Go's unbounded `http.DefaultClient`. This prevents alternate binaries, tests, and future callers from restoring an infinite GameServer, roster, registration, or SDK wait by omission. diff --git a/server/agones/allocation.go b/server/agones/allocation.go index f93c86f5..d9c3bed4 100644 --- a/server/agones/allocation.go +++ b/server/agones/allocation.go @@ -42,6 +42,8 @@ type Client struct { WorkloadTokenTTL time.Duration } +const DefaultHTTPTimeout = 10 * time.Second + type AllocatedServer struct { Allocation domain.Allocation Endpoint string @@ -108,9 +110,7 @@ func (c Client) RecoverAllocation(ctx context.Context, request domain.Allocation if request.AllocationID == "" || request.MatchID == "" || now.IsZero() { return AllocatedServer{}, false, domain.ErrAllocationInput } - if c.HTTP == nil { - c.HTTP = http.DefaultClient - } + c.HTTP = c.httpClient() base, err := c.endpoint() if err != nil { return AllocatedServer{}, false, err @@ -166,9 +166,7 @@ func (c Client) RecoverAllocation(ctx context.Context, request domain.Allocation // allocator registry. Compatibility fields must be present as Fleet labels; // malformed Ready objects fail closed instead of creating selectable capacity. func (c Client) ListReadyServers(ctx context.Context) ([]domain.ReadyServer, error) { - if c.HTTP == nil { - c.HTTP = http.DefaultClient - } + c.HTTP = c.httpClient() base, err := c.endpoint() if err != nil { return nil, err @@ -213,9 +211,7 @@ func readyServerFromGameServer(name string, labels map[string]string) (domain.Re } func (c Client) Allocate(ctx context.Context, request domain.AllocationRequest, labels map[string]string, now time.Time) (AllocatedServer, error) { - if c.HTTP == nil { - c.HTTP = http.DefaultClient - } + c.HTTP = c.httpClient() base, err := c.endpoint() if err != nil { return AllocatedServer{}, err @@ -305,6 +301,13 @@ func (c Client) endpoint() (string, error) { return strings.TrimRight(c.BaseURL, "/"), nil } +func (c Client) httpClient() *http.Client { + if c.HTTP != nil { + return c.HTTP + } + return &http.Client{Timeout: DefaultHTTPTimeout} +} + func selectPort(ports []struct { Name string `json:"name"` Port int `json:"port"` diff --git a/server/agones/allocation_test.go b/server/agones/allocation_test.go index f0f94d59..c4a74758 100644 --- a/server/agones/allocation_test.go +++ b/server/agones/allocation_test.go @@ -17,6 +17,13 @@ func request() domain.AllocationRequest { return domain.AllocationRequest{AllocationID: "allocation-1", MatchID: "match-1", Region: "EU", Build: "build-1", Protocol: 1, Transport: "enet"} } +func TestClientDefaultHTTPTransportHasRequestDeadline(t *testing.T) { + client := (Client{}).httpClient() + if client == http.DefaultClient || client.Timeout != DefaultHTTPTimeout || client.Timeout <= 0 { + t.Fatalf("default HTTP client timeout = %s", client.Timeout) + } +} + func TestAllocateRejectsRankedRequestsWithoutRegisteredArena(t *testing.T) { client := Client{BaseURL: "http://127.0.0.1:1", Namespace: "games"} for _, path := range []string{"", "res://scenes/arena_01_elevated.tscn", "res://forged.tscn"} { diff --git a/server/supervisor/supervisor.go b/server/supervisor/supervisor.go index e61465b9..952c658e 100644 --- a/server/supervisor/supervisor.go +++ b/server/supervisor/supervisor.go @@ -106,7 +106,10 @@ type Supervisor struct { lastGameServer GameServer } -const DefaultDrainGrace = 285 * time.Second +const ( + DefaultDrainGrace = 285 * time.Second + DefaultHTTPTimeout = 10 * time.Second +) func New(config Config) (*Supervisor, error) { if len(config.Command) == 0 || config.Command[0] == "" { @@ -131,7 +134,7 @@ func New(config Config) (*Supervisor, error) { return nil, fmt.Errorf("unsupported transport %q", config.Transport) } if config.HTTPClient == nil { - config.HTTPClient = http.DefaultClient + config.HTTPClient = &http.Client{Timeout: DefaultHTTPTimeout} } if (config.DrainURL == "") != (config.DrainToken == "") { return nil, fmt.Errorf("drain URL and token must be configured together") diff --git a/server/supervisor/supervisor_test.go b/server/supervisor/supervisor_test.go index 32572dd1..b06401e2 100644 --- a/server/supervisor/supervisor_test.go +++ b/server/supervisor/supervisor_test.go @@ -13,6 +13,16 @@ import ( "time" ) +func TestSupervisorDefaultHTTPClientHasRequestDeadline(t *testing.T) { + supervisor, err := New(Config{Command: []string{"game-server"}}) + if err != nil { + t.Fatal(err) + } + if supervisor.client == http.DefaultClient || supervisor.client.Timeout != DefaultHTTPTimeout || supervisor.client.Timeout <= 0 { + t.Fatalf("default HTTP client timeout = %s", supervisor.client.Timeout) + } +} + func TestWithAllocatedConfigOverridesAuthoritativeChildFlags(t *testing.T) { command := []string{ "game-server", "--", "--allocated-mode", "--match-id=stale-match",