The just-landed signed workload token embedded (allocation_id, match_id,
server_id) as claims. That doesn't actually work for its intended delivery
channel: the token is meant to be requested as a GameServerAllocation
annotation in the SAME request that asks Agones to pick a server, so at mint
time the allocator knows allocation_id (it generates it) but not yet which
server_id Agones will return -- server_id only exists in Agones's response,
after the annotation request has already been sent. Embedding it was simply
not possible for the real caller this was built for; only the (allocator ->
signed_token) unit tests and hand-constructed integration tests happened to
supply it directly, masking the gap.
Fixes it by having the token bind only allocation_id (the one identifier
actually known at mint time) plus expiry. match_id/server_id are resolved at
verify time from the durable allocations table via the new
store.AllocationBindingByAllocationID, keyed by allocation_id -- which the
allocator already records immediately after Agones responds. This is
strictly stronger, not just a workaround: a caller can no longer claim any
match/server pairing at all, even one that happens to be internally
consistent -- the binding returned is entirely durable-record-derived.
Verified: server/workload's unit tests updated for the new two-field claim
shape; server/api's Postgres integration suite gains
TestWorkloadVerifierFromSignedTokenNeverTrustsCallerSuppliedBinding (two
distinct real allocations each resolve to their own, and only their own,
match/server pairing) replacing the now-inapplicable mismatched-triple test.
Full `go build ./... && go vet ./... && gofmt -l . && go test ./... -race`
and `go test -tags integration ./... -race` both clean; the api integration
suite re-run 3x clean against a live postgres:17-alpine container.
WorkloadVerify (api.Service.WorkloadVerify) was permanently unwired: both
/v1/servers/{id}/register and /v1/servers/{id}/result always 503, because
the only design considered so far was verifying a Kubernetes-projected
service-account JWT (server/workload/jwt.go), which needs a live cluster's
TokenReview/JWKS endpoint to validate against safely -- something this
sandbox cannot do without guessing at a trust boundary.
The API layer doesn't actually require that specific mechanism. serverMutation
only compares WorkloadBinding.ServerID and .MatchID (server/api/service.go);
AdvanceServerRegistration only uses .MatchID/.ServerID/.AllocationID. Nothing
downstream needs Namespace/ServiceAcct/PodUID/GameServerUID populated.
This adds a self-contained alternative: a short-lived, HMAC-signed token the
control plane mints and verifies with a secret only it holds (server/workload/
signed_token.go), the same trust model domain.SessionStore already uses for
player sessions elsewhere in this codebase. It needs no cluster to verify --
signature + expiry is fully self-contained and unit-testable.
The design's soundness rests on the delivery channel, not the crypto: the
token is meant to reach the allocated GameServer via the same Agones
GameServerAllocation annotation channel allocation.go already uses for
match-id/allocation-id, readable only by that pod's own local SDK sidecar. A
caller presenting this token has already proven, via that channel, that it is
the pod Agones allocated. (Wiring the actual annotation delivery -- extending
agones.Client.Allocate and the supervisor's token source -- is a separate,
follow-up change; this commit lands the verification core it depends on.)
store.AllocationBindingStillValid adds defense-in-depth on top of signature
and expiry: it cross-checks the token's claims against the durable
allocations table (append-only, never leaves 'ALLOCATED'), so a validly-signed
token naming an allocation that was never recorded -- or a real allocation id
paired with a mismatched match/server -- is still rejected.
api.WorkloadVerifierFromSignedToken wires the two together and is now plugged
into cmd/control-plane (new --workload-secret / COSMIC_CLASH_WORKLOAD_SECRET
flag; a startup warning is logged if it's left unset, since the route then
stays 503 exactly as before) and cmd/testkit-api (fixed test secret, since
that binary is test-only already).
Verified: new unit tests in server/workload (signature tamper, wrong secret,
expiry boundary, malformed input) and a new Postgres integration suite in
server/api (real allocation row, real signed token, acceptance / unknown-
allocation rejection / mismatched-triple rejection / the previously-503
Service.WorkloadVerify field itself) -- both run clean with -race across
multiple passes against a live postgres:17-alpine container. Full
`go build ./... && go vet ./... && gofmt -l . && go test ./... -race` and
`go test -tags integration ./... -race` both clean.
Same discovery pattern as ResultSubmitter/SessionIssuer, one level
deeper: api.Service.rankedProfile and .profile both only ever read
from an in-memory RankedProfiles map with no durable-store equivalent
at all -- not "adapter exists but unwired" this time, there was no
adapter. Every real request to GET /v1/profile/ranked or /api/v1/profile
always 404'd regardless of a player's actual rating.
Add RankedProfileProvider (an interface, not a struct-literal adapter
this time) and store.PostgresRankedProfiles reading the ratings table;
Service.rankedProfileFor prefers it when set and falls back to the
map otherwise, so every existing test/direct Service literal keeps
compiling and passing unchanged. A missing ratings row maps to the
exact same (zero value, false, nil) the map lookup already produced,
preserving existing not-found semantics rather than reinterpreting
them. LastSeasonID/SeasonHistory are deliberately left unset -- the
ratings table has no season pointer, and reconstructing history needs
its own query and display semantics, not bundled in here speculatively.
Wired into both cmd/control-plane and cmd/testkit-api. Verified
against real PostgreSQL via curl: a fresh identity's ranked profile
correctly 404s through the real adapter (same behavior as before,
now for a real reason instead of an empty map).
Wire the same Service.Log hook added for the server register/result
routes into queue create/heartbeat/cancel and proposal accept/decline:
log the resulting state on success (queue_create, queue_heartbeat,
queue_cancel, proposal_response) or 'rejected' on a domain error,
using only the ticket/proposal ID and outcome -- never the domain
error text itself, which isn't documented as credential-free.
Read-only routes (queue GET, proposal GET, assignment fetch) and the
early availability/not-found rejections that return before reaching
the domain call are deliberately not logged in this pass.
Covered by a new end-to-end test driving real create/heartbeat/cancel
and an accept followed by a stale-revision accept (fenced for real by
the domain layer behind proposalBackendSpy, unlike the dumb queue
spy), asserting the exact sequence of events logged.
server/observability existed fully unit-tested but was imported by
nothing outside its own package -- no HTTP handler ever called it, so
its credential redaction protected zero real log output. Wire it into
Service via an optional Log field (nil-safe, so every existing Service
literal keeps compiling unchanged) and call it from the two
workload-authenticated server routes -- register and result -- at
every outcome: unauthorized, rejected, conflict and success. Wire
cmd/control-plane to actually emit those events as JSON lines on
stderr.
Add a secret canary test that drives both routes end to end with
realistic bearer-token and result-nonce values and asserts neither
literal secret appears anywhere in what Service.Log actually received
-- a stronger claim than the existing observability unit test, which
only proves redact() strips a synthetic value under a denylisted key
name. redact() is still key-name-based, not content-based: a future
call site that logs a secret under an unlisted key name would not be
caught by this test or by redact() itself, only by the same discipline
applied here of never putting raw request/token bytes into Fields.
Queue, proposal and assignment mutation routes are not wired yet.
Add POST /v1/servers/{id}/register (and its /api/v1 contract alias),
authenticated by the same workload binding as the result route. A
game server reports its protocol version and image digest and asks
to advance ALLOCATING -> PROCESS_READY -> ASSIGNMENT_READY; the store
boundary (AdvanceServerRegistration) does this as one idempotent
SERIALIZABLE transaction that also advances every participant's queue
ticket, and gates the final transition on every participant having a
live, unexpired assignment.
Adversarial review of the surrounding routing turned up a pre-existing
bug: contractServerMutation rejected any path containing '/', so the
already-documented /api/v1/servers/{id}/result route (and this new
/register route) 404'd for every real caller despite being declared
in the OpenAPI contract. Fix it to delegate shape validation to
serverMutation, matching how contractQueueMutation handles its own
two-segment paths, and add a regression test covering both contract
routes end to end.
Josh Creek