mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-16 00:32:21 +00:00
feat(multiplayer): implement WorkloadVerify without a Kubernetes trust boundary
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.
This commit is contained in:
@@ -27,6 +27,7 @@ func main() {
|
||||
redisAddr := flag.String("redis-addr", os.Getenv("COSMIC_CLASH_REDIS_ADDR"), "optional Redis address for the candidate projection")
|
||||
redisPrefix := flag.String("redis-prefix", envOrDefault("COSMIC_CLASH_REDIS_PREFIX", "cosmic-clash"), "Redis key prefix")
|
||||
redisTTL := flag.Duration("redis-ttl", 60*time.Second, "TTL for transient candidate projection entries")
|
||||
workloadSecret := flag.String("workload-secret", os.Getenv("COSMIC_CLASH_WORKLOAD_SECRET"), "HMAC secret for control-plane-issued workload tokens (see workload/signed_token.go); server registration/result submission return 503 until this is set")
|
||||
flag.Parse()
|
||||
if *role != "api" {
|
||||
fatalf("unsupported role %q (only api is implemented)", *role)
|
||||
@@ -57,7 +58,10 @@ func main() {
|
||||
defer redisClient.Close()
|
||||
candidateIndex = store.RedisCandidateIndex{Client: redisClient, Prefix: *redisPrefix, TTL: *redisTTL}
|
||||
}
|
||||
server := &http.Server{Addr: *listen, Handler: newAPIHandler(db, candidateIndex), ReadHeaderTimeout: 5 * time.Second}
|
||||
if *workloadSecret == "" {
|
||||
fmt.Fprintln(os.Stderr, "control-plane: warning: --workload-secret / COSMIC_CLASH_WORKLOAD_SECRET is unset; server registration and result submission will return 503")
|
||||
}
|
||||
server := &http.Server{Addr: *listen, Handler: newAPIHandler(db, *workloadSecret, candidateIndex), ReadHeaderTimeout: 5 * time.Second}
|
||||
serveErr := make(chan error, 1)
|
||||
go func() { serveErr <- server.ListenAndServe() }()
|
||||
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
|
||||
@@ -76,7 +80,7 @@ func main() {
|
||||
}
|
||||
}
|
||||
|
||||
func newAPIHandler(db *sql.DB, indexes ...api.CandidateIndex) http.Handler {
|
||||
func newAPIHandler(db *sql.DB, workloadSecret string, indexes ...api.CandidateIndex) http.Handler {
|
||||
var candidateIndex api.CandidateIndex
|
||||
if len(indexes) > 0 {
|
||||
candidateIndex = indexes[0]
|
||||
@@ -93,6 +97,7 @@ func newAPIHandler(db *sql.DB, indexes ...api.CandidateIndex) http.Handler {
|
||||
Assignment: api.AssignmentProviderFromStore(db),
|
||||
CandidateIndex: candidateIndex,
|
||||
ProbeRecorder: store.PostgresQueue{DB: db},
|
||||
WorkloadVerify: api.WorkloadVerifierFromSignedToken([]byte(workloadSecret), db),
|
||||
Now: func() time.Time { return time.Now().UTC() },
|
||||
Log: logEvent,
|
||||
}).Handler()
|
||||
|
||||
@@ -9,24 +9,25 @@ import (
|
||||
func TestAPIHandlerExposesHealthWithoutDatabase(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodGet, "/healthz", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
newAPIHandler(nil).ServeHTTP(rec, req)
|
||||
newAPIHandler(nil, "").ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("health status = %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestServerRoutesRequireWorkloadVerifyToBeWired pins a real, known gap
|
||||
// rather than leaving it silent: newAPIHandler wires ServerRegistrar and
|
||||
// ResultSubmitter, but never a WorkloadVerify -- and Service.serverMutation
|
||||
// treats a nil WorkloadVerify as fatal for BOTH the register and result
|
||||
// routes, regardless of whether their own dependency is present. So today,
|
||||
// in the actual running binary, POST /v1/servers/{id}/register and
|
||||
// /v1/servers/{id}/result both always 503, independent of a real database or
|
||||
// real request. This test should start failing (and be updated, not
|
||||
// deleted) the day a real WorkloadVerify is wired -- that's the intended
|
||||
// signal, not a bug in the test.
|
||||
// TestServerRoutesRequireWorkloadVerifyToBeWired pins the deployment
|
||||
// misconfiguration case: newAPIHandler wires ServerRegistrar and
|
||||
// ResultSubmitter, but WorkloadVerifierFromSignedToken deliberately returns
|
||||
// nil whenever the secret or the database is missing (see
|
||||
// api.WorkloadVerifierFromSignedToken) rather than silently accepting every
|
||||
// caller. Service.serverMutation treats a nil WorkloadVerify as fatal for
|
||||
// BOTH the register and result routes. This test should start failing (and
|
||||
// be updated, not deleted) the day this path stops 503ing with an empty
|
||||
// secret and a nil database -- that's the intended signal, not a bug in the
|
||||
// test. See TestWorkloadVerifierFromSignedTokenAcceptsARealAllocation in the
|
||||
// api package's Postgres integration suite for the wired, working path.
|
||||
func TestServerRoutesRequireWorkloadVerifyToBeWired(t *testing.T) {
|
||||
handler := newAPIHandler(nil)
|
||||
handler := newAPIHandler(nil, "")
|
||||
for _, path := range []string{"/v1/servers/server-1/register", "/v1/servers/server-1/result"} {
|
||||
req := httptest.NewRequest(http.MethodPost, path, nil)
|
||||
req.Header.Set("Idempotency-Key", "regression-pin-key-123456")
|
||||
|
||||
Reference in New Issue
Block a user