From ca70568fad6f55fc82ff9ee23f743648f079ca40 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:50:01 +0100 Subject: [PATCH] fix(agones): make the kind gate's Agones lifecycle actually work Several independent causes, all of which had to be right before the Fleet could reach Ready. The supervisor pointed --sdk-base-url at 127.0.0.1:9357, which is the Agones sidecar's gRPC port; its HTTP surface is 9358, and that is what AGONES_SDK_HTTP_PORT carries and what agones_sdk.gd reads. An HTTP client against the gRPC port could never have worked, in kind or in production. The supervisor also treated the sidecar's first incomplete /gameserver response as fatal. The sidecar accepts requests before the controller populates status.address and status.ports, so this produced a restart loop precisely during normal Agones startup. It now polls until the endpoint is assigned or ReadyTimeout elapses. server_boot.gd started ServerControl and the Agones SDK only under --allocated-mode, but the kind smoke deliberately strips that flag, so nothing served the readiness probe and the GameServer could never become Ready. Lifecycle now keys on AGONES_SDK_HTTP_PORT, which Agones injects into every managed container, while allocation and roster semantics stay tied to --allocated-mode. The SDK node is added to the tree non-deferred, since start_health() creates a Timer immediately. Fleet: Agones assigns its own SDK service account and masks that token from the game container while keeping it for the injected sidecar, so the manifest must not pin serviceAccountName or automountServiceAccountToken. Godot stores user:// under HOME, so HOME points at the writable runtime volume to keep the root filesystem read-only, and fsGroup makes that volume writable for the non-root user. Namespace: Agones' Dynamic port policy injects a hostPort, which both the baseline and restricted Pod Security Standards forbid, so the workload namespace enforces privileged while continuing to audit and warn against restricted. NetworkPolicy: the injected sidecar reaches the Kubernetes API over HTTPS, and NetworkPolicy applies to the whole Pod rather than to the container whose token was masked. The kind runner creates the namespace before Helm so Agones can install its per-namespace SDK RBAC, scopes gameservers.namespaces to it, forces the allocator and ping Services to ClusterIP because LoadBalancer ingress never becomes ready in plain kind, and labels the node so the production Fleet's on-demand/zone constraints are exercised rather than edited out of the rendered manifest. --- Game/scripts/server_boot.gd | 38 +++++++++++++-------- Game/scripts/server_control.gd | 7 ++-- deploy/k8s/base/fleet.yaml | 13 +++++-- deploy/k8s/base/namespace.yaml | 7 ++-- deploy/k8s/base/network-policies.yaml | 6 ++++ deploy/k8s/base/service-accounts.yaml | 7 ---- scripts/verify_kind_agones.sh | 25 ++++++++++++-- server/security/test_fleet_manifests.py | 26 +++++++++++--- server/security/test_kubernetes_policies.py | 6 ++-- server/supervisor/supervisor.go | 26 +++++++++++++- server/supervisor/supervisor_test.go | 38 +++++++++++++++++++++ 11 files changed, 159 insertions(+), 40 deletions(-) diff --git a/Game/scripts/server_boot.gd b/Game/scripts/server_boot.gd index 41deca3a..4d049209 100644 --- a/Game/scripts/server_boot.gd +++ b/Game/scripts/server_boot.gd @@ -73,6 +73,29 @@ func _ready() -> void: printerr("cosmic-clash-server: allocated transport '%s' is not supported by this build" % assigned_transport) get_tree().quit(1) return + # Agones injects its HTTP port into every managed game-server container. + # Keep lifecycle readiness and health active in the reduced kind smoke even + # though that environment intentionally omits allocation/roster semantics. + var agones_managed := not OS.get_environment("AGONES_SDK_HTTP_PORT").is_empty() + if allocated_mode or agones_managed: + _control = ServerControlScript.new() + _control.name = "ServerControl" + _control.drain_requested.connect(_on_drain_requested) + _control.initial_connect_ready.connect(_on_initial_connect_ready) + get_tree().root.add_child.call_deferred(_control) + var control_err := _control.start(int(config.get_value("readiness-port")), OS.get_environment(String(config.get_value("drain-token-env")))) + if control_err != OK: + printerr("cosmic-clash-server: refusing to start with invalid readiness control port") + get_tree().quit(1) + return + if agones_managed: + _agones = AgonesSDKScript.new() + _agones.name = "AgonesSDK" + # Health creates and starts a Timer immediately, so the SDK node must be + # in the tree before start_health() runs. + get_tree().root.add_child(_agones) + if _agones.configure_from_environment(): + _agones.start_health() if allocated_mode: var roster_file := String(config.get_value("join-authorisations-file")) var key_file := String(config.get_value("join-authorisations-key-file")) @@ -88,25 +111,10 @@ func _ready() -> void: printerr("cosmic-clash-server: refusing to start with invalid join-authorisations-file") get_tree().quit(1) return - _control = ServerControlScript.new() # An allocated process owns exactly the roster issued for this match. # Never let the general-purpose direct-server default (one player) start # an allocated match with only a partial assignment admitted. config.values["min-players"] = required_min_players(true, roster_tokens.size(), int(config.get_value("min-players"))) - _control.name = "ServerControl" - _control.drain_requested.connect(_on_drain_requested) - _control.initial_connect_ready.connect(_on_initial_connect_ready) - get_tree().root.add_child.call_deferred(_control) - var control_err := _control.start(int(config.get_value("readiness-port")), OS.get_environment(String(config.get_value("drain-token-env")))) - if control_err != OK: - printerr("cosmic-clash-server: refusing to start with invalid readiness control port") - get_tree().quit(1) - return - _agones = AgonesSDKScript.new() - _agones.name = "AgonesSDK" - get_tree().root.add_child.call_deferred(_agones) - if _agones.configure_from_environment(): - _agones.start_health() _connection_leases = ConnectionLeaseClientScript.new() _connection_leases.name = "ConnectionLeases" var lease_url := OS.get_environment("COSMIC_CLASH_CONTROL_PLANE_URL") diff --git a/Game/scripts/server_control.gd b/Game/scripts/server_control.gd index 532c9a17..ca06c852 100644 --- a/Game/scripts/server_control.gd +++ b/Game/scripts/server_control.gd @@ -1,9 +1,10 @@ class_name ServerControl extends Node -# Small loopback HTTP control surface for allocated servers. The Go supervisor -# uses GET /ready as the explicit process-ready probe and POST /drain during a -# controlled termination. Direct/community servers do not start this node. +# Small loopback HTTP control surface for lifecycle-managed servers. The Go +# supervisor uses GET /ready as the explicit process-ready probe and POST +# /drain during a controlled termination. Direct/community servers outside +# Agones do not start this node. signal drain_requested signal initial_connect_ready diff --git a/deploy/k8s/base/fleet.yaml b/deploy/k8s/base/fleet.yaml index ca92eda6..1581d5d1 100644 --- a/deploy/k8s/base/fleet.yaml +++ b/deploy/k8s/base/fleet.yaml @@ -43,19 +43,22 @@ spec: labelSelector: matchLabels: app.kubernetes.io/name: game-server - serviceAccountName: match-server - automountServiceAccountToken: false + # Leave serviceAccountName unset: Agones assigns its SDK account and + # masks that account's token from this public game-server container, + # while retaining it in the injected SDK sidecar that needs API access. securityContext: runAsNonRoot: true runAsUser: 10001 runAsGroup: 10001 + fsGroup: 10001 + fsGroupChangePolicy: OnRootMismatch seccompProfile: type: RuntimeDefault containers: - name: game-server image: ghcr.io/cosmic-clash/game-server@sha256:0000000000000000000000000000000000000000000000000000000000000000 args: - - --sdk-base-url=http://127.0.0.1:9357 + - --sdk-base-url=http://127.0.0.1:9358 - --ready-url=http://127.0.0.1:7780/ready - --drain-url=http://127.0.0.1:7780/drain - --initial-connect-ready-url=http://127.0.0.1:7780/initial-connect-ready @@ -86,6 +89,10 @@ spec: - --join-authorisations-key-file=/run/secrets/cosmic-clash/join-signing-keys.json - --readiness-port=7780 env: + # Godot stores user:// beneath HOME. Point it at the writable + # runtime volume while retaining a read-only root filesystem. + - name: HOME + value: /run/cosmic-clash - name: COSMIC_CLASH_SERVER_ID valueFrom: fieldRef: diff --git a/deploy/k8s/base/namespace.yaml b/deploy/k8s/base/namespace.yaml index 2e40d676..aeda8b9e 100644 --- a/deploy/k8s/base/namespace.yaml +++ b/deploy/k8s/base/namespace.yaml @@ -3,7 +3,10 @@ kind: Namespace metadata: name: cosmic-clash labels: - pod-security.kubernetes.io/enforce: restricted + # Agones' Dynamic port policy injects a hostPort into every GameServer + # Pod. Kubernetes' built-in baseline and restricted policies both forbid + # host ports, so this workload namespace must enforce privileged while + # continuing to surface restricted-policy deviations in audit and warnings. + pod-security.kubernetes.io/enforce: privileged pod-security.kubernetes.io/audit: restricted pod-security.kubernetes.io/warn: restricted - diff --git a/deploy/k8s/base/network-policies.yaml b/deploy/k8s/base/network-policies.yaml index 17222c60..337f24b7 100644 --- a/deploy/k8s/base/network-policies.yaml +++ b/deploy/k8s/base/network-policies.yaml @@ -90,6 +90,12 @@ spec: ports: - protocol: TCP port: 8080 + # The injected Agones SDK sidecar updates its GameServer through the + # kubernetes.default HTTPS Service. Its token is masked from the public + # game-server container by Agones, but NetworkPolicy applies to the Pod. + - ports: + - protocol: TCP + port: 443 - ports: - protocol: UDP port: 53 diff --git a/deploy/k8s/base/service-accounts.yaml b/deploy/k8s/base/service-accounts.yaml index e02605fc..7cc2c8b8 100644 --- a/deploy/k8s/base/service-accounts.yaml +++ b/deploy/k8s/base/service-accounts.yaml @@ -7,13 +7,6 @@ automountServiceAccountToken: false --- apiVersion: v1 kind: ServiceAccount -metadata: - name: match-server - namespace: cosmic-clash -automountServiceAccountToken: false ---- -apiVersion: v1 -kind: ServiceAccount metadata: name: allocator namespace: cosmic-clash diff --git a/scripts/verify_kind_agones.sh b/scripts/verify_kind_agones.sh index 79ea44c4..a1cc07c2 100755 --- a/scripts/verify_kind_agones.sh +++ b/scripts/verify_kind_agones.sh @@ -34,6 +34,8 @@ dump_cluster_state() { for ns in agones-system cosmic-clash; do echo "=== namespace ${ns}: pods ===" >&2 kubectl -n "$ns" get pods -o wide >&2 2>&1 || true + echo "=== namespace ${ns}: services ===" >&2 + kubectl -n "$ns" get services -o wide >&2 2>&1 || true # Events explain scheduling/image/probe failures that pod status alone # does not: FailedScheduling, ImagePullBackOff, readiness probe errors. echo "=== namespace ${ns}: recent events ===" >&2 @@ -98,15 +100,22 @@ fi kind create cluster --name "$cluster_name" --image "$kind_node_image" --wait 120s kind load docker-image "$game_server_image" --name "$cluster_name" +# Agones creates its SDK service account and namespaced RBAC in each configured +# GameServer namespace. The namespace must therefore exist before Helm runs. +kubectl apply -f deploy/k8s/base/namespace.yaml + helm repo add agones https://agones.dev/chart/stable >/dev/null helm repo update >/dev/null # Agones 1.49 otherwise requests 10,100 MiB of ephemeral storage for both its # controller and extensions pods, which exceeds a default single-node kind -# cluster before the Fleet can be exercised. These are smoke-only bounds; -# production resource sizing remains deployment-owned. +# cluster before the Fleet can be exercised. Its allocator and ping Services +# also default to LoadBalancer, whose ingress never becomes ready in plain kind. +# These are smoke-only bounds; production sizing and exposure remain +# deployment-owned. helm upgrade --install agones agones/agones \ --namespace agones-system --create-namespace \ --version "$agones_version" \ + --set 'gameservers.namespaces[0]=cosmic-clash' \ --set agones.crds.cleanup.enabled=true \ --set agones.controller.replicas=1 \ --set agones.controller.resources.requests.ephemeral-storage=128Mi \ @@ -115,6 +124,9 @@ helm upgrade --install agones agones/agones \ --set agones.extensions.resources.requests.ephemeral-storage=128Mi \ --set agones.extensions.resources.limits.ephemeral-storage=512Mi \ --set agones.allocator.replicas=1 \ + --set agones.allocator.service.serviceType=ClusterIP \ + --set agones.ping.http.serviceType=ClusterIP \ + --set agones.ping.udp.serviceType=ClusterIP \ --wait --timeout 5m kubectl wait --for=condition=available deployment/agones-controller \ @@ -122,6 +134,14 @@ kubectl wait --for=condition=available deployment/agones-controller \ kubectl wait --for=condition=available deployment/agones-allocator \ -n agones-system --timeout=180s +# The production Fleet only schedules on explicitly on-demand, zoned nodes. +# Give the disposable node equivalent labels so this gate exercises those +# constraints instead of rewriting them out of the rendered Fleet. +kubectl label nodes --all \ + cosmic-clash.io/capacity-type=on-demand \ + topology.kubernetes.io/zone=kind-smoke \ + --overwrite + # The base Fleet intentionally carries a release-time digest placeholder. For # this isolated run only, replace that exact placeholder with the image loaded # into kind. No repository manifest is modified and no mutable image is used @@ -139,7 +159,6 @@ sed -e "s|ghcr.io/cosmic-clash/game-server@sha256:${zero_digest}|$game_server_im -e '/- --allocated-mode$/d' \ deploy/k8s/base/fleet.yaml > "$work_dir/fleet.yaml" -kubectl apply -f deploy/k8s/base/namespace.yaml kubectl -n cosmic-clash create secret generic cosmic-clash-game-server \ --from-literal=drain-token=kind-smoke-drain-token \ --from-literal=join-signing-keys.json='{"kind-smoke-key":"a2luZC1zbW9rZS1zaWduaW5nLWtleQ=="}' \ diff --git a/server/security/test_fleet_manifests.py b/server/security/test_fleet_manifests.py index d2b4d0ac..acce6c65 100644 --- a/server/security/test_fleet_manifests.py +++ b/server/security/test_fleet_manifests.py @@ -18,11 +18,19 @@ class FleetManifestTest(unittest.TestCase): "protocol: UDP", "containerPort: 7777", "replicas: 2", ): self.assertIn(label, fleet) - for hardening in ("runAsNonRoot: true", "automountServiceAccountToken: false", "readOnlyRootFilesystem: true", "allowPrivilegeEscalation: false"): + for hardening in ( + "runAsNonRoot: true", "readOnlyRootFilesystem: true", + "allowPrivilegeEscalation: false", "fsGroup: 10001", + "name: HOME", "value: /run/cosmic-clash", + ): self.assertIn(hardening, fleet) + # Agones must assign its SDK service account so it can keep the token + # available to its injected sidecar while masking it from the game. + self.assertNotIn("serviceAccountName:", fleet) + self.assertNotIn("automountServiceAccountToken:", fleet) for runtime in ( "ghcr.io/cosmic-clash/game-server@sha256:", - "--sdk-base-url=http://127.0.0.1:9357", + "--sdk-base-url=http://127.0.0.1:9358", "--control-plane-url=http://control-plane.cosmic-clash.svc.cluster.local:8080", "--protocol-version=1", "/opt/cosmic-clash/CosmicClashServer.x86_64", @@ -88,13 +96,23 @@ class FleetManifestTest(unittest.TestCase): base = self.read("base/kustomization.yaml") for field in ("kind: Service", "name: control-plane", "port: 8080", "targetPort: http"): self.assertIn(field, service) - for field in ("name: game-server-allowed-egress", "app.kubernetes.io/name: game-server", "port: 8080"): - self.assertIn(field, network) + game_server_egress = network.split("name: game-server-allowed-egress", 1)[-1].split("---", 1)[0] + for field in ("app.kubernetes.io/name: game-server", "port: 8080", "port: 443"): + self.assertIn(field, game_server_egress) self.assertIn("control-plane-service.yaml", base) def test_kind_runner_is_explicitly_separate_from_production_roster_flow(self): runner = (ROOT / "scripts/verify_kind_agones.sh").read_text() self.assertIn("Agones lifecycle smoke", runner) + self.assertIn("gameservers.namespaces[0]=cosmic-clash", runner) + for service in ( + "agones.allocator.service.serviceType=ClusterIP", + "agones.ping.http.serviceType=ClusterIP", + "agones.ping.udp.serviceType=ClusterIP", + ): + self.assertIn(service, runner) + self.assertIn("cosmic-clash.io/capacity-type=on-demand", runner) + self.assertIn("topology.kubernetes.io/zone=kind-smoke", runner) self.assertIn("--control-plane-url=", runner) self.assertIn("--allocated-mode", runner) validator = (ROOT / "scripts/verify_agones_allocation_response.py").read_text() diff --git a/server/security/test_kubernetes_policies.py b/server/security/test_kubernetes_policies.py index 49faada0..149ee6d0 100644 --- a/server/security/test_kubernetes_policies.py +++ b/server/security/test_kubernetes_policies.py @@ -10,10 +10,12 @@ class KubernetesPolicyTest(unittest.TestCase): def read(self, name): return (BASE / name).read_text() - def test_namespace_enforces_restricted_pod_security(self): + def test_namespace_allows_agones_host_ports_and_audits_restricted(self): namespace = self.read("namespace.yaml") - for key in ("enforce", "audit", "warn"): + self.assertIn("pod-security.kubernetes.io/enforce: privileged", namespace) + for key in ("audit", "warn"): self.assertIn(f"pod-security.kubernetes.io/{key}: restricted", namespace) + self.assertIn("Agones' Dynamic port policy", namespace) def test_workload_is_non_root_immutable_and_unprivileged(self): deployment = self.read("control-plane-deployment.yaml") diff --git a/server/supervisor/supervisor.go b/server/supervisor/supervisor.go index 8f746339..4671f0ce 100644 --- a/server/supervisor/supervisor.go +++ b/server/supervisor/supervisor.go @@ -199,7 +199,7 @@ func (s *Supervisor) Start(ctx context.Context) error { env := append([]string(nil), os.Environ()...) env = append(env, s.config.Environment...) if s.config.SDKBaseURL != "" { - port, address, err := s.assignedEndpoint(ctx) + port, address, err := s.waitAssignedEndpoint(ctx) if err != nil { return err } @@ -269,6 +269,30 @@ func (s *Supervisor) Start(ctx context.Context) error { return nil } +// waitAssignedEndpoint covers the short interval between the SDK sidecar +// accepting requests and the GameServer controller populating status.address +// and status.ports. Treating the sidecar's first incomplete response as fatal +// creates a restart loop precisely while Agones is finishing normal startup. +func (s *Supervisor) waitAssignedEndpoint(ctx context.Context) (int, string, error) { + deadline := time.NewTimer(s.config.ReadyTimeout) + defer deadline.Stop() + var lastErr error + for { + port, address, err := s.assignedEndpoint(ctx) + if err == nil { + return port, address, nil + } + lastErr = err + select { + case <-ctx.Done(): + return 0, "", ctx.Err() + case <-deadline.C: + return 0, "", fmt.Errorf("assigned endpoint timed out: %w", lastErr) + case <-time.After(s.config.PollInterval): + } + } +} + func (s *Supervisor) signalInitialConnectReady(ctx context.Context) error { if s.config.AdmissionURL == "" { return nil diff --git a/server/supervisor/supervisor_test.go b/server/supervisor/supervisor_test.go index 2ada2f29..6569e9af 100644 --- a/server/supervisor/supervisor_test.go +++ b/server/supervisor/supervisor_test.go @@ -165,6 +165,44 @@ func TestAllocatedStartInjectsDynamicEndpointAndCallsReadyAfterProbe(t *testing. } } +func TestAllocatedStartWaitsForAgonesToAssignEndpoint(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/gameserver": + requests++ + if requests == 1 { + _, _ = w.Write([]byte(`{"status":{}}`)) + return + } + _, _ = w.Write([]byte(`{"status":{"address":"203.0.113.9","ports":[{"name":"game","port":31001}]}}`)) + case "/ready-probe", "/ready": + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + s, err := New(Config{ + Command: []string{"/bin/sh", "-c", "true"}, SDKBaseURL: server.URL, + ReadyURL: server.URL + "/ready-probe", ReadyTimeout: time.Second, + PollInterval: time.Millisecond, + }) + if err != nil { + t.Fatal(err) + } + if err := s.Start(context.Background()); err != nil { + t.Fatal(err) + } + if err := s.Wait(); err != nil { + t.Fatal(err) + } + if requests < 2 { + t.Fatalf("gameserver requests = %d, want at least 2", requests) + } +} + func TestAllocatedStartMaterializesWorkloadAuthenticatedRosterBeforeChild(t *testing.T) { rosterPath := filepath.Join(t.TempDir(), "join-roster.json") sdk := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {