From 76cadca8e35a2fbd9d4a13ec5003d400fd5296bf Mon Sep 17 00:00:00 2001 From: Nguyen Minh Phuc Date: Mon, 20 Jul 2026 15:16:45 +0000 Subject: [PATCH] fix: Kubernetes serialises an empty item list as null, not [] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drift check failed on every tick with: TypeError: 'NoneType' object is not iterable `resp.json().get("items", [])` cannot defend against this. The key IS present, with the value null, so the default never fires. An empty PartialObjectMetadataList comes back as `"items": null`, which is exactly what happens once the release selector matches nothing — the normal state of a cluster with no tenant releases provisioned yet. `or []` handles both shapes. The tests could not have caught this: all of them passed a real empty list, which is the one shape that works. The new test sends `"items": null` as the API server actually sends it, and is control-tested — restoring the old expression fails it with the same TypeError seen in production. Worth recording why this survived review. The API read was verified against a live cluster before shipping, but that cluster had 25 matching release secrets, so the empty case never ran. A measurement on real data proved the fast path worked and said nothing about the path taken when there is no data. --- .../svcforge_core/adapters/helm.py | 6 ++- tests/unit/test_helm_argv.py | 39 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/libs/svcforge_core/svcforge_core/adapters/helm.py b/libs/svcforge_core/svcforge_core/adapters/helm.py index 1ed697a..53f578e 100644 --- a/libs/svcforge_core/svcforge_core/adapters/helm.py +++ b/libs/svcforge_core/svcforge_core/adapters/helm.py @@ -379,7 +379,11 @@ class HelmProvisioner: }, ) resp.raise_for_status() - items: Any = resp.json().get("items", []) + # `or []`, not `.get("items", [])`. Kubernetes serialises an empty list as + # `"items": null`, so the key is present and the default never fires. This + # shipped and failed in production on the first tick that matched no + # releases: TypeError: 'NoneType' object is not iterable. + items: Any = resp.json().get("items") or [] except (httpx.HTTPError, json.JSONDecodeError) as exc: # Raise, do not fall back to helm. The fallback is for "there is no # ServiceAccount here", which is a fact about the environment and is known diff --git a/tests/unit/test_helm_argv.py b/tests/unit/test_helm_argv.py index 8e13723..be6251b 100644 --- a/tests/unit/test_helm_argv.py +++ b/tests/unit/test_helm_argv.py @@ -228,3 +228,42 @@ async def test_no_service_account_falls_back_to_helm(monkeypatch: pytest.MonkeyP await HelmProvisioner(kubeconfig=Path("/dev/null")).list_releases() assert seen and seen[0][:2] == ["helm", "list"] + + +@pytest.mark.asyncio +async def test_api_read_handles_kubernetes_serialising_empty_as_null( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """`"items": null`, not `[]`, is what an empty list looks like on the wire. + + The key is present, so `.get("items", [])` returns None and the default never fires. + This shipped and failed in production on the first tick that matched no releases: + TypeError: 'NoneType' object is not iterable. The earlier tests all passed a real + empty list, which is the one shape that cannot catch it. + """ + cap = _fake_api(monkeypatch, tmp_path, []) + cap["force_null_items"] = True + + class _NullResponse: + def raise_for_status(self) -> None: + return None + + def json(self) -> dict[str, object]: + return {"apiVersion": "meta.k8s.io/v1", "kind": "PartialObjectMetadataList", "items": None} + + class _NullClient: + def __init__(self, **kw: object) -> None: + pass + + async def __aenter__(self) -> _NullClient: + return self + + async def __aexit__(self, *exc: object) -> None: + return None + + async def get(self, url: str, **kw: object) -> _NullResponse: + return _NullResponse() + + monkeypatch.setattr(httpx, "AsyncClient", _NullClient) + + assert await HelmProvisioner().list_releases() == []