From 52ee1810427ea2b14c3cbc780b83f8ccbb881243 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:03:41 +0100 Subject: [PATCH] fix(kind): validate the allocation response Agones actually returns With the Fleet readiness wait corrected, the gate reached the allocation check for the first time and failed with "allocation did not return a GameServer" -- while the cluster dump shows the allocation plainly succeeded: one GameServer Allocated, Fleet reporting ALLOCATED 1. GameServerAllocationStatus is flat: state, gameServerName, address, ports, nodeName. It does not embed the allocated GameServer. The validator read status.gameServer.metadata.name and status.gameServer.status.{address,ports}, a shape Agones never sends, and its unit tests asserted that same invented shape -- so validator and tests agreed with each other while both disagreed with Agones. Nothing caught it because the gate had never once allocated anything. Read the real fields, keeping every existing check: non-empty name, address neither blank nor unspecified, exactly one named "game" port in range. Also print the response body when validation fails. work_dir is removed by the EXIT trap, so a shape mismatch was otherwise invisible from CI -- which is how this survived. If the shape is still not what I expect, the next run says so instead of costing another round trip. --- .../test_verify_agones_allocation_response.py | 24 +++++++++---------- scripts/verify_agones_allocation_response.py | 24 +++++++++---------- scripts/verify_kind_agones.sh | 11 ++++++++- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/scripts/test_verify_agones_allocation_response.py b/scripts/test_verify_agones_allocation_response.py index 6e4faa1a..2c31b37b 100644 --- a/scripts/test_verify_agones_allocation_response.py +++ b/scripts/test_verify_agones_allocation_response.py @@ -9,15 +9,15 @@ from verify_agones_allocation_response import validate_allocation def response(**overrides): document = { + # Mirrors Agones' real GameServerAllocationStatus, which is flat. + # These fixtures previously encoded a nested "gameServer" object that + # Agones never returns, so the suite agreed with the validator while + # both disagreed with reality. "status": { "state": "Allocated", - "gameServer": { - "metadata": {"name": "cosmic-clash-game-abc"}, - "status": { - "address": "10.0.0.7", - "ports": [{"name": "game", "port": 31001}], - }, - }, + "gameServerName": "cosmic-clash-game-abc", + "address": "10.0.0.7", + "ports": [{"name": "game", "port": 31001}], } } document["status"].update(overrides) @@ -34,28 +34,28 @@ class AgonesAllocationResponseTest(unittest.TestCase): def test_rejects_missing_identity_or_address(self): missing_name = response() - missing_name["status"]["gameServer"]["metadata"] = {} + missing_name["status"]["gameServerName"] = "" with self.assertRaises(ValueError): validate_allocation(missing_name) missing_address = response() - missing_address["status"]["gameServer"]["status"]["address"] = "0.0.0.0" + missing_address["status"]["address"] = "0.0.0.0" with self.assertRaises(ValueError): validate_allocation(missing_address) def test_rejects_ambiguous_or_invalid_game_ports(self): duplicate = response() - duplicate["status"]["gameServer"]["status"]["ports"].append({"name": "game", "port": 31002}) + duplicate["status"]["ports"].append({"name": "game", "port": 31002}) with self.assertRaises(ValueError): validate_allocation(duplicate) wrong_name = response() - wrong_name["status"]["gameServer"]["status"]["ports"] = [{"name": "query", "port": 31001}] + wrong_name["status"]["ports"] = [{"name": "query", "port": 31001}] with self.assertRaises(ValueError): validate_allocation(wrong_name) invalid_port = response() - invalid_port["status"]["gameServer"]["status"]["ports"][0]["port"] = 70000 + invalid_port["status"]["ports"][0]["port"] = 70000 with self.assertRaises(ValueError): validate_allocation(invalid_port) diff --git a/scripts/verify_agones_allocation_response.py b/scripts/verify_agones_allocation_response.py index d93116d8..e2eac9cf 100644 --- a/scripts/verify_agones_allocation_response.py +++ b/scripts/verify_agones_allocation_response.py @@ -11,26 +11,26 @@ def validate_allocation(document: dict[str, Any]) -> tuple[str, int]: if not isinstance(status, dict) or status.get("state") != "Allocated": raise ValueError(f"allocation state is {status.get('state') if isinstance(status, dict) else None!r}, expected 'Allocated'") - game_server = status.get("gameServer") - if not isinstance(game_server, dict): - raise ValueError("allocation did not return a GameServer") - metadata = game_server.get("metadata") - name = metadata.get("name") if isinstance(metadata, dict) else None + # GameServerAllocationStatus is flat: state, gameServerName, address, + # ports, nodeName. It does not embed the allocated GameServer object. This + # validator originally read status.gameServer.metadata.name and + # status.gameServer.status.{address,ports}, and its tests asserted that + # same invented shape, so both agreed with each other and neither agreed + # with Agones -- undetected because the gate never once got far enough to + # allocate anything. + name = status.get("gameServerName") if not isinstance(name, str) or not name.strip(): - raise ValueError("allocation GameServer has no metadata.name") + raise ValueError("allocation did not return a gameServerName") - game_status = game_server.get("status") - if not isinstance(game_status, dict): - raise ValueError("allocation GameServer has no status") - address = game_status.get("address") + address = status.get("address") if not isinstance(address, str) or not address.strip() or any(char.isspace() for char in address): raise ValueError(f"allocation returned an invalid address: {address!r}") if address in {"0.0.0.0", "::"}: raise ValueError(f"allocation returned an unspecified address: {address!r}") - ports = game_status.get("ports") + ports = status.get("ports") if not isinstance(ports, list): - raise ValueError("allocation GameServer has no ports") + raise ValueError("allocation returned no ports") game_ports = [ entry.get("port") for entry in ports diff --git a/scripts/verify_kind_agones.sh b/scripts/verify_kind_agones.sh index d05015a6..661f2c70 100755 --- a/scripts/verify_kind_agones.sh +++ b/scripts/verify_kind_agones.sh @@ -206,4 +206,13 @@ spec: EOF kubectl create -f "$work_dir/allocation.yaml" -o json > "$work_dir/allocation.json" -python3 scripts/verify_agones_allocation_response.py "$work_dir/allocation.json" +# Print the response when validation fails. work_dir is deleted by the EXIT +# trap, so a mismatch between what Agones returns and what the validator +# expects is otherwise unknowable from CI -- which is exactly how a validator +# reading a field Agones never sends survived undetected. +if ! python3 scripts/verify_agones_allocation_response.py "$work_dir/allocation.json"; then + echo "--- allocation response as returned by Agones ---" >&2 + cat "$work_dir/allocation.json" >&2 || true + echo >&2 + exit 1 +fi