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.