Closes part of the 'live Redis failover' gap in §8.46, found by
reproducing a genuine Redis outage (not just an empty/partial cache)
against CandidateProjection.Snapshot with a killed miniredis instance.
CandidateProjection.Snapshot funnelled two different situations into
the same code path: the index erroring outright (Redis unreachable)
and the index coming back empty (ambiguous — a genuinely empty queue,
or a lost keyspace). Both went through Repair, which itself calls
Index.Rebuild — a second Redis round-trip that fails for exactly the
same reason the first one did. The result: a real Redis outage, or
the window during a failover, made Snapshot fail outright even though
PostgreSQL — the documented authoritative source everywhere
(RedisCandidateIndex's own comment, cmd/matcher, cmd/control-plane's
--redis-addr help text all call it a rebuildable/optional
acceleration layer) — was completely healthy. Matchmaking would stop
entirely on a Redis outage despite the architecture explicitly not
requiring that.
Snapshot now falls back to serving Source (PostgreSQL) directly
whenever the index errors OR comes back empty, and only best-effort
attempts to repopulate Redis afterward — that attempt's outcome is
deliberately ignored, since a caller must never be denied service
just because the opportunistic rebuild also hit the same down Redis.
Snapshot still fails when Source itself is unavailable; the fallback
is not unconditional.
Verified: reproduced the bug first (killed-miniredis Snapshot call
failed even though Source was healthy), then fixed it. go build/vet
clean; all pre-existing store-package tests pass unmodified,
including the two live-redis:7-alpine-container tests
(TestRealRedisCandidateIndexUpsertSnapshotRemove,
TestRealRedisCandidateProjectionRepairsAfterFlush, run against a real
container and torn down after). Two new tests cover the fallback
directly (killed miniredis, Source still served, exactly one Source
call) and that the fallback is not unconditional (both Redis and
Source down still fails). Full go test ./... -race clean across
every server package.
Remaining: live matcher-worker-under-load-during-failover integration,
i.e. running the actual matcher process against a real Redis that
goes down mid-run under concurrent load, not just this unit-level
reproduction.
Closes the 'concurrent proposal-recovery expiry races' gap noted in
§8.46. GetProposal (read-side recovery) and RespondToProposal both
run the identical expiry-advance SQL in their own transaction, so any
number of them can observe the same past-expiry proposal at once —
this had never been exercised concurrently, only sequentially (the
existing late-response test drives one call at a time).
TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
races 8 concurrent GetProposal/RespondToProposal calls, each with a
distinct 'now' past the proposal window, against one proposal and
asserts: EXPIRED lands on the proposal and both tickets exactly once,
a PROPOSAL_TIMEOUT penalty lands exactly once per offending player
(not once per racing transaction), and no idempotency row survives a
closed-proposal response. The design already defends against this —
ProposalParticipantExpireSQL only ever flips a still-PENDING row
once, so a losing racer's 'now' can't match
recordProposalTimeoutCooldowns' responded_at filter — this test is
what actually proves that holds under real concurrent load rather
than by inspection.
Verified: real postgres:17-alpine container, go test -tags
integration ./store/... -run
TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
-race -count=3 clean; full -tags integration ./store/... -race run
clean; full non-integration go build/vet/test -race clean across
every server package; container removed after the run.
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.
The last two commits fixed the severe stranding bug in decline and
timeout, but left a real responsiveness gap: cancelling a ticket
directly while it's part of an OPEN proposal used to leave the OTHER
participant waiting out the full response window for something the
system already knew couldn't happen -- their proposal partner just
abandoned the queue. ProposalExpireRequeueSQL eventually rescues them,
but only after the full window elapses, not immediately.
CascadeCancelToOpenProposal runs inside the same transaction as the
cancel itself: if the cancelled ticket belonged to a currently-OPEN
proposal, decline that proposal right now and requeue every other
participant immediately via the same ProposalDeclineRequeueSQL the
decline path already uses. The cancelling player's own ticket
correctly stays CANCELLED, not swept back into the requeue meant for
everyone else (ProposalDeclineRequeueSQL only touches tickets still at
PROPOSED).
Covered by a real PostgreSQL integration test: cancelling one
participant's ticket mid-proposal immediately declines the proposal
and requeues the other participant with a refreshed expiry, while the
cancelling player's own ticket stays CANCELLED. First draft used a
stale expected revision (0) for the cancel call -- CreateProposal's
own QueueTicketProposeSQL already bumps a ticket's revision to 1 when
forming the proposal, caught immediately by actually running the test
against real Postgres rather than assuming. Clean across 5 runs after
the fix, plus the full integration and unit suites.
The timeout sibling of the previous commit's decline fix: a proposal
that simply times out (the 10s window elapses with no unanimous
response) hits ProposalExpireSQL/ProposalParticipantExpireSQL, and
neither of those -- same as the decline path -- ever touched
queue_tickets. Same severe consequence: every participant still
holding a PROPOSED ticket, response pending or already accepted, is
left stranded (invisible to the matcher, blocking a fresh
queue_create, renewable forever by heartbeat) with no automatic way
back into matchmaking. This path is reached from both GetProposal
(the recovery/read boundary -- a client that missed the expiry event
entirely) and RespondToProposal (a response arriving after the
window), so both needed the fix.
ProposalExpireRequeueSQL mirrors ProposalDeclineRequeueSQL, guarded on
state = 'EXPIRED' so it's safe to call unconditionally right after
ProposalExpireSQL: a no-op on a proposal that's still OPEN, and a
no-op on a proposal that was already EXPIRED on a prior pass (nothing
left at PROPOSED to requeue a second time).
Covered by a real PostgreSQL integration test via GetProposal (nobody
ever responds; recovering the proposal well after its window expires
it and must requeue both participants), confirming both tickets land
back at QUEUED with a refreshed expiry and are visible again to
ListQueuedCandidates. Clean across 5 runs, plus the full integration
and unit suites.
Josh Creek