From 1a964b18fa2ff11fbde3deacc10b19f7ff5c95c5 Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:54:44 -0700 Subject: [PATCH] Check the type a CRD declares, not just presence and shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walker asked whether every required property was present, whether any property was excess, and whether list identities were unique. It never asked whether a value carried the type the CRD declares — and YAML decides that for you. That gap has a live cost. portal's Platform declared minACU: 0.5 maxACU: 4 which reads as obviously correct and is not: the CRD types both as STRINGS, because Kubernetes serialises fractional quantities that way. The API server refused the whole object: spec.datastores[0].relational.minACU: Invalid value: "number": ... in body must be of type string: "number" Every property was present, none was excess, no list repeated an identity — so this walker had already declared that manifest admissible, and the install failed applying it after the cluster and its substrate were built. The check covers scalars and containers, with two details that matter: - a Python bool is an int, so `true` would otherwise satisfy `integer`. It is tested for first, in both directions. - the message reports the value's OWN type in `Invalid value`, the way the API server does, so it can be searched for verbatim after a failed apply. A number where a string is wanted also says to quote it, because that is the fix and it is not obvious from the type names alone. Twelve self-test cases: a number and an integer where a string is wanted, the same value quoted, a string where an integer is wanted, a bool against integer and against boolean. Every hand-written Platform, Tenant, BudgetPolicy, ModelGateway, AgentFleet and EvalSuite in the org still passes with the rule on — thirty-one documents — so it adds no false positives. --- scripts/check-platform-crs.py | 111 +++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/scripts/check-platform-crs.py b/scripts/check-platform-crs.py index e15c26a..01f699c 100755 --- a/scripts/check-platform-crs.py +++ b/scripts/check-platform-crs.py @@ -29,6 +29,11 @@ - every `required` property must be present, at every level - no property may be absent from the schema (the API server prunes it, so a field set here has never reached a cluster) + - every value must carry the type the CRD declares. YAML decides that for you, + and it is why `minACU: 0.5` is rejected where `minACU: "0.5"` is admitted — + Kubernetes serialises fractional quantities as strings + - a list declared `x-kubernetes-list-type: map` or `set` must hold unique + entries, because a duplicate is a hard rejection of the whole object The version comes from the appset rather than from `latest` deliberately. The question is not "is this manifest valid against the newest CRDs" but "is it @@ -187,8 +192,92 @@ def check_list_uniqueness(value, schema, path, kind, source, problems): seen[identity] = i +# OpenAPI type -> the Python types a YAML load produces for it. +# +# bool before int deliberately: in Python `True` is an int, so an unquoted `true` would +# satisfy `integer` and a genuine type error would pass. +_JSON_TYPES = { + "string": (str,), + "integer": (int,), + "number": (int, float), + "boolean": (bool,), + "object": (dict,), + "array": (list,), +} + + +def check_type(value, schema, path, kind, source, problems): + """The type the CRD declares is enforced at admission, and YAML decides it for you. + + This is the half a required-and-pruning walker does not have, and it is the half that + bites hardest on numbers. `minACU: 0.5` reads as a YAML float; the CRD declares + minACU a STRING, because Kubernetes serialises fractional quantities as strings. The + manifest looks obviously correct, every property is present, none is excess, and the + API server rejects the whole object: + + spec.datastores[0].relational.minACU: Invalid value: "number": + ... in body must be of type string: "number" + + Only scalars and containers are checked here — the recursion below already walks into + objects and arrays, and a mistyped one is reported by this rule before it descends. + """ + want = schema.get("type") + allowed = _JSON_TYPES.get(want) + if allowed is None or value is None: + return + # A bool is an int in Python; nothing else may borrow that. + if isinstance(value, bool) != (want == "boolean"): + if want != "boolean" and isinstance(value, bool): + problems.append( + f"{source}: {kind} {path} is a boolean and the CRD declares {want} — the API " + f"server rejects the object with `{path.lstrip('.')}: Invalid value`" + ) + return + if not isinstance(value, allowed): + got = _json_type_name(value) + hint = "" + if want == "string" and isinstance(value, (int, float)): + hint = ( + " — quote it. YAML makes an unquoted number a number, and Kubernetes " + "serialises fractional quantities as strings" + ) + # Phrased as the API server phrases it, including reporting the value's OWN type in + # `Invalid value` rather than the wanted one, so the message can be searched for + # verbatim after a failed apply. + problems.append( + f"{source}: {kind} {path} is {_article(got)} {got} and the CRD declares {want}{hint}. The API " + f"server rejects the whole object with `{path.lstrip('.')}: Invalid value: " + f'"{got}": ... in body must be of type {want}: "{got}"`, so nothing is applied' + ) + + +def _article(word: str) -> str: + return "an" if word[:1] in "aeiou" else "a" + + +def _json_type_name(value) -> str: + """The OpenAPI type name for a value, which is what the API server reports. + + bool first: a bool is an int in Python, and calling one an integer here would print a + message that does not match what kubectl said. + """ + if isinstance(value, bool): + return "boolean" + if isinstance(value, str): + return "string" + if isinstance(value, int): + return "integer" + if isinstance(value, float): + return "number" + if isinstance(value, dict): + return "object" + if isinstance(value, list): + return "array" + return type(value).__name__ + + def walk(value, schema, path, kind, source, problems): - """Required present, nothing excess, list identities unique — arrays transparent. + """Required present, nothing excess, types right, list identities unique. Stops descending wherever the schema declines to describe the shape (x-kubernetes-preserve-unknown-fields, or an object with no properties), @@ -197,6 +286,8 @@ def walk(value, schema, path, kind, source, problems): if not isinstance(schema, dict): return + check_type(value, schema, path, kind, source, problems) + if isinstance(value, list): check_list_uniqueness(value, schema, path, kind, source, problems) items = schema.get("items") @@ -332,6 +423,14 @@ def self_test() -> int: "items": {"properties": {"group": {}, "name": {}}}, }, "finalizers": {"x-kubernetes-list-type": "set", "items": {}}, + "acu": { + "type": "object", + "properties": { + "minACU": {"type": "string"}, + "retention": {"type": "integer"}, + "paused": {"type": "boolean"}, + }, + }, "ordered": {"x-kubernetes-list-type": "atomic", "items": {"properties": {"name": {}}}}, }, } @@ -362,6 +461,16 @@ def self_test() -> int: # An unset key is a value: two entries omitting it collide on . ("unset key participates in the identity", {"routes": [{"group": "g"}, {"group": "g"}]}, 1), ("duplicate set member", {"finalizers": ["a", "a"]}, 1), + # The live failure this rule was added for: a CRD string carrying a YAML number. + # Every property present, none excess, and the API server refuses the object. + ("a number where the CRD wants a string", {"acu": {"minACU": 0.5}}, 1), + ("an integer where the CRD wants a string", {"acu": {"minACU": 4}}, 1), + ("the same value quoted", {"acu": {"minACU": "0.5"}}, 0), + ("a string where the CRD wants an integer", {"acu": {"retention": "7"}}, 1), + ("an integer where the CRD wants an integer", {"acu": {"retention": 7}}, 0), + # A bool is an int in Python and must not satisfy `integer`. + ("a boolean where the CRD wants an integer", {"acu": {"retention": True}}, 1), + ("a boolean where the CRD wants a boolean", {"acu": {"paused": True}}, 0), ("distinct set members", {"finalizers": ["a", "b"]}, 0), # atomic imposes no uniqueness — flagging it would be a false positive that # teaches operators to work around the gate.