From a32d3cf3da1d9995d4127197a34e1809a9ddd050 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 14 Jul 2026 17:43:51 +0300 Subject: [PATCH 1/3] fix: derive the extends merge policy from each key's validator resolve_extends runs ahead of validate(), so a merge that normalizes a value decides what the gate then sees. The policy was a hand-maintained pair of key sets, and it had drifted from the gate in both directions. It laundered invalid input into valid: ulimits has no list form and the gate refuses one, but _merge_map coerced `ulimits: ["nofile=2"]` into {"nofile": "2"} before the gate ran. Same for every list-only key given a bare scalar -- cap_add, devices, security_opt, group_add, volumes, secrets, configs all refuse a scalar standalone and all accepted one via extends. And it refused valid input: extra_hosts has a list form, accepted standalone, but rejected on merge. One rule now: a merge may normalize a value only through a form that key actually has. Registry keys run spec.validate on both sides before spec.merge, so the merge's accepted forms ARE the gate's, by construction. Structural normalizers are narrowed to the forms Compose defines -- scalar only for tmpfs/env_file, list form only for environment/extra_hosts/depends_on. extra_hosts gets a real normalizer: its list form is colon-separated, so pairs_to_mapping (which splits on '=') would have produced a single {'host:ip': None} key. Splits on the first colon, so IPv6 survives. A parametrized test pins the invariant for every mergeable key: a form the gate refuses standalone must be refused through extends too. Verified to go red without the fix. --- architecture/supported-subset.md | 40 +++--- compose2pod/extends.py | 68 +++++++-- .../2026-07-14.05-extends-merge-policy.md | 113 +++++++++++++++ tests/test_extends.py | 136 +++++++++++++++++- 4 files changed, 327 insertions(+), 30 deletions(-) create mode 100644 planning/changes/2026-07-14.05-extends-merge-policy.md diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index ad25dba..395b18a 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -234,26 +234,32 @@ slot (`architecture/glossary.md`). `entrypoint` (argv replaced wholesale, never concatenated) — also covers unknown keys, which `validate()` then rejects downstream exactly as it would without `extends`. - - **Normalization before merge:** list-form `environment` and list-form - `depends_on` (a bare service-name list) are normalized to mappings - before the mapping-merge; scalar-form `tmpfs`/`env_file` are normalized - to a one-element list before the concatenation. + - **Normalization before merge — a merge never widens the gate.** A merged + side is normalized only through a form that key *actually has*: list-form + `environment`, `extra_hosts` and `depends_on` become mappings before the + mapping-merge, and scalar-form `tmpfs`/`env_file` become a one-element + list before the concatenation. Nothing else is coerced. This matters + because `resolve_extends` runs **ahead of** `validate()`: normalizing a + form a key does not have would hand the gate a document it would have + refused standalone. So `ulimits` and `healthcheck` (no list form) refuse a + list, and every list-only key (`cap_add`, `cap_drop`, `security_opt`, + `devices`, `group_add`, `volumes`, `secrets`, `configs`) refuses a bare + scalar — exactly as each does outside `extends`. The accepted forms are + read from each key's own validator (`spec.validate` runs on both sides + before `spec.merge`), so the merge policy cannot drift from the gate. + `extra_hosts`' list form is `host:ip` — colon-separated, split on the + *first* colon so an IPv6 address survives (`myhost:::1` → + `{myhost: ::1}`). - **Refused loudly:** cross-file `extends: {file: ..., service: ...}`; a bare-string (or any other non-mapping) `extends`; an unrecognized key under `extends` other than `service`; a non-string `service`; a `service` - naming one that doesn't exist; and a merge across incompatible forms (a - mapping-merge key that is neither a mapping nor list-form - `environment`/`depends_on`; a sequence-concatenate key that is neither a - list nor a scalar string). -- **Divergences from Compose:** `environment`/`depends_on` accept list form - directly in `extends.py`'s own merge (`_as_mapping`); `extra_hosts`/ - `healthcheck` do not — list form on a merged side is refused for those two. - `labels`/`annotations`/`ulimits` instead route through their `SERVICE_KEYS` - merge policy (`_merge_map`), which *does* coerce list form, via the same - `pairs_to_mapping` normalizer `environment` uses — both paths reject a - non-string list element identically, since it's the one function they - share. Short-form `volumes` are concatenated rather than merged by target - path; podman resolves duplicate mounts at run time. Referenced resources + naming one that doesn't exist; and a merge across incompatible forms — a + side whose form that key does not have, which raises with the key's own + validator message (`'cap_add' must be a list`), the same message the value + produces standalone. +- **Divergences from Compose:** short-form `volumes` are concatenated rather + than merged by target path; podman resolves duplicate mounts at run time. + Referenced resources (top-level `volumes`, `networks`, `secrets`, `configs`) are not auto-imported — as in Compose, the extending service must declare what it needs. diff --git a/compose2pod/extends.py b/compose2pod/extends.py index 2ad5394..91500b0 100644 --- a/compose2pod/extends.py +++ b/compose2pod/extends.py @@ -3,16 +3,23 @@ from typing import Any from compose2pod.exceptions import UnsupportedComposeError -from compose2pod.keys import SERVICE_KEYS, concat_list, pairs_to_mapping +from compose2pod.keys import SERVICE_KEYS, as_list, pairs_to_mapping # Merge policy for keys with a SERVICE_KEYS KeySpec comes from spec.merge (see # _merge below); these two sets cover only the remaining structural keys, which -# have no KeySpec. Unifying structural-key merge policy is deferred — see -# decisions/2026-07-12-reject-structural-key-registry.md's revisit trigger. +# have no KeySpec. Both categories obey one rule: a merge may normalize a value +# only through a form that key actually has, so it can never accept a shape the +# gate would reject standalone. _STRUCTURAL_MERGE_KEYS = {"environment", "extra_hosts", "healthcheck", "depends_on"} _STRUCTURAL_CONCAT_KEYS = {"secrets", "configs", "volumes", "tmpfs", "env_file"} +# The only concat keys Compose gives a bare-string form. The gate accepts a +# scalar for exactly these two and requires a list for the rest, so only these +# two may be normalized scalar -> [scalar] on a merged side; normalizing any +# other would accept a shape the gate refuses standalone. +_SCALAR_FORM_KEYS = {"tmpfs", "env_file"} + def _extends_target(name: str, ext: Any) -> str: # noqa: ANN401 - Compose values are untyped """Return the referenced service name, after refusing cross-file and malformed forms.""" @@ -39,19 +46,54 @@ def _extends_target(name: str, ext: Any) -> str: # noqa: ANN401 - Compose value return service +def _extra_hosts_to_mapping(name: str, value: list[Any]) -> dict[str, Any]: + """Normalize list-form `extra_hosts` ('host:ip') to a mapping. + + Separated by a colon, not '=', so `pairs_to_mapping` would mangle the whole + entry into a single `{'host:ip': None}` key. Split on the *first* colon only: + an IPv6 address is itself full of them (`myhost:::1` -> `{'myhost': '::1'}`). + """ + result: dict[str, Any] = {} + for item in value: + if not isinstance(item, str) or ":" not in item: + msg = f"service {name!r}: extra_hosts entries must be 'host:ip' strings" + raise UnsupportedComposeError(msg) + host, _sep, address = item.partition(":") + result[host] = address + return result + + +def _as_concat_list(name: str, key: str, value: Any) -> list[Any]: # noqa: ANN401 - Compose values are untyped + """Normalize a structural concat key's value to a list, without widening the gate. + + `keys.as_list` turns a bare string into a one-element list, which is right + for `tmpfs`/`env_file` (Compose gives them a scalar form and the gate accepts + one) and wrong for `volumes`/`secrets`/`configs`, where the gate requires a + list -- normalizing there would let `extends` accept a scalar the same + document would be refused for standalone. + """ + if isinstance(value, str) and key not in _SCALAR_FORM_KEYS: + msg = f"service {name!r}: '{key}' must be a list" + raise UnsupportedComposeError(msg) + return as_list(name, key, value) + + def _as_mapping(name: str, key: str, value: Any) -> dict[str, Any]: # noqa: ANN401 - Compose values are untyped """Normalize a structural mapping-merge key's value to a mapping. - Deliberately stricter than `keys.pairs_to_mapping`: list form is accepted - only for `environment` and `depends_on`, the two keys Compose actually - defines a list form for. `extra_hosts`/`healthcheck` in list form on a - merged side are refused as an incompatible form rather than coerced. + List form is accepted for exactly the keys Compose defines one for -- + `environment`, `extra_hosts`, `depends_on` -- each through the normalizer + that key's own list form actually needs. `healthcheck` has no list form, so + a list is refused rather than coerced: a merge must not accept a shape the + gate would reject standalone (see `_merge`). """ if isinstance(value, dict): return value if isinstance(value, list): if key == "environment": return pairs_to_mapping(name, key, value) + if key == "extra_hosts": + return _extra_hosts_to_mapping(name, value) if key == "depends_on": for dep in value: if not isinstance(dep, str): @@ -68,11 +110,21 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, for key, local_val in local.items(): spec = SERVICE_KEYS.get(key) if key in base and spec is not None and spec.merge is not None: + # A merge must never *widen* what the gate accepts. `resolve_extends` + # runs ahead of `validate()`, so a normalizing merge (list -> mapping) + # can launder a form the key does not have into one it does: + # `ulimits: ["nofile=2"]` is refused standalone, but coercing it here + # would produce a valid-looking `{"nofile": "2"}` that then sails + # through the gate. Checking each side with the key's own validator + # first derives the merge's accepted forms from the gate's, so the + # two cannot drift apart. + spec.validate(name, key, base[key]) + spec.validate(name, key, local_val) merged[key] = spec.merge(name, key, base[key], local_val) elif key in base and key in _STRUCTURAL_MERGE_KEYS: merged[key] = {**_as_mapping(name, key, base[key]), **_as_mapping(name, key, local_val)} elif key in base and key in _STRUCTURAL_CONCAT_KEYS: - merged[key] = concat_list(name, key, base[key], local_val) + merged[key] = _as_concat_list(name, key, base[key]) + _as_concat_list(name, key, local_val) else: merged[key] = local_val return merged diff --git a/planning/changes/2026-07-14.05-extends-merge-policy.md b/planning/changes/2026-07-14.05-extends-merge-policy.md new file mode 100644 index 0000000..415dd2c --- /dev/null +++ b/planning/changes/2026-07-14.05-extends-merge-policy.md @@ -0,0 +1,113 @@ +--- +summary: An extends merge may normalize a value only through a form that key actually has, derived from the key's own validator, so a merge can no longer launder a shape the gate refuses standalone (ulimits/cap_add/volumes scalars and lists) nor refuse one it accepts (list-form extra_hosts). +--- + +# Design: Derive the extends merge policy from each key's validator + +## Summary + +`resolve_extends` runs **ahead of** `validate()`. A merge that *normalizes* a +value (list → mapping, scalar → list) therefore decides what the gate will +subsequently see — and if it normalizes a form the key does not actually have, +it turns an invalid document into a valid one. That is what it was doing. + +One rule now governs every mergeable key: **a merge may normalize a value only +through a form that key actually has.** The accepted forms are derived from the +key's own validator rather than from a hand-maintained list, so the two cannot +drift. + +## Motivation + +The merge policy had drifted out of step with the gate in *both* directions. + +**It laundered invalid input into valid** (the serious half). `ulimits` has no +list form in Compose, and the gate refuses one: + +``` +$ validate({"app": {"ulimits": ["nofile=2"]}}) -> 'ulimits' must be a mapping +``` + +But arriving through `extends`, `_merge_map` coerced that list into +`{"nofile": "2"}` *before* the gate ran, and the document sailed through. The +same held for every list-only key given a bare scalar — `cap_add`, `devices`, +`security_opt`, `group_add`, `volumes`, `secrets`, `configs` all refuse a scalar +standalone, and all silently accepted one via `extends`. + +**And it refused valid input.** `extra_hosts` *does* have a list form +(`- "host:1.2.3.4"`), accepted standalone — but a merge rejected it as an +"incompatible form". + +The asymmetry was not cosmetic: it was two bugs pointing in opposite directions, +both caused by the policy being an enumeration maintained by hand instead of +being derived from what each key actually accepts. + +## Design + +**Registry keys** (`SERVICE_KEYS`): `_merge` runs `spec.validate` on *both* sides +before `spec.merge`. The merge's accepted forms are therefore exactly the gate's, +by construction. `ulimits: [...]` and `cap_add: "SCALAR"` now raise — with the +validator's own message (`'cap_add' must be a list`), which is also the message +the same value produces standalone. + +**Structural keys** (no `KeySpec`): the two normalizers are narrowed to the forms +Compose actually defines. + +- `_as_concat_list` normalizes `scalar → [scalar]` only for `tmpfs` and + `env_file` — the only concat keys with a bare-string form, and the only two the + gate accepts a scalar for. `volumes`/`secrets`/`configs` must stay lists. +- `_as_mapping` accepts list form for `environment`, `extra_hosts`, and + `depends_on` — each through the normalizer that key's list form actually needs. + `healthcheck` has no list form, so a list is still refused. + +`extra_hosts` gets a real normalizer: its list form is `host:ip`, **colon**- +separated, so routing it through `pairs_to_mapping` (which splits on `=`) would +have silently produced a single `{'host:ip': None}` key. It splits on the *first* +colon only, so an IPv6 address survives (`myhost:::1` → `{'myhost': '::1'}`). + +No structural-key registry — `decisions/2026-07-12` stands. This derives policy +from validators that already exist; it does not build a dispatch table. + +## The invariant, pinned + +A parametrized test asserts, for **every** mergeable key, that a form the gate +refuses standalone is also refused through `extends`. That is the property the +enumeration kept violating, and it now fails loudly rather than drifting. It is +verified to go red without the fix (on `ulimits`). + +## Non-goals + +- Not validating the whole service pre-merge (the heavier structural option). + Deriving per-key from the validator closes the class with a much smaller + change, and the invariant test guards the seam. +- Not changing what the gate itself accepts. Every shape valid standalone before + this change is still valid; every shape invalid standalone is now *also* + invalid through `extends`. + +## Behavior change + +Three previously-accepted merges now raise, all of which produced documents the +gate would have refused standalone: + +- `ulimits` in list form on a merged side +- any list-only key (`cap_add`, `devices`, `security_opt`, `group_add`, + `volumes`, `secrets`, `configs`) given a bare scalar on a merged side +- (unchanged, still refused) `healthcheck` in list form + +One previously-refused merge now works: list-form `extra_hosts`. + +Two error messages improved: they now come from the key's validator +(`'cap_add' must be a list`) rather than the vaguer `cannot merge 'cap_add' +across incompatible forms`, matching what the same value yields standalone. + +## Testing + +`just test-ci` at 100%. The invariant test above is the centrepiece; new tests +cover the `extra_hosts` list form (including IPv6 colon-safety and a +missing-colon refusal) and the scalar-form narrowing. + +## Risk + +- **A user relying on the laundering** — e.g. a base with `cap_add: "NET_ADMIN"` + (scalar) — now gets an error. This is the intended correction: that document + was already invalid without `extends`, and the tool was accepting it only by + accident of merge order. The error names the key and the required form. diff --git a/tests/test_extends.py b/tests/test_extends.py index 51021ec..196ec7d 100644 --- a/tests/test_extends.py +++ b/tests/test_extends.py @@ -1,9 +1,10 @@ -from typing import Any +from typing import Any, ClassVar import pytest from compose2pod.exceptions import UnsupportedComposeError from compose2pod.extends import resolve_extends +from compose2pod.parsing import validate def _services(doc: dict[str, Any]) -> dict[str, Any]: @@ -89,17 +90,21 @@ def test_incompatible_labels_form_is_refused(self) -> None: "web": {"extends": {"service": "base"}, "labels": 5}, } } - with pytest.raises(UnsupportedComposeError, match="cannot merge 'labels' across incompatible forms"): + with pytest.raises(UnsupportedComposeError, match="'labels' must be a list or mapping"): resolve_extends(doc) - def test_cap_add_scalar_normalized_before_concat(self) -> None: + def test_cap_add_scalar_form_is_refused_not_normalized(self) -> None: + # Compose has no scalar form for cap_add and the gate refuses one, so a + # merge must not quietly normalize it into a list -- that would let + # `extends` accept a document that is invalid on its own. doc = { "services": { "base": {"image": "x", "cap_add": "NET_ADMIN"}, "web": {"extends": {"service": "base"}, "cap_add": ["SYS_TIME"]}, } } - assert _services(doc)["web"]["cap_add"] == ["NET_ADMIN", "SYS_TIME"] + with pytest.raises(UnsupportedComposeError, match="'cap_add' must be a list"): + resolve_extends(doc) def test_depends_on_list_and_map_forms_merge(self) -> None: doc = { @@ -214,7 +219,7 @@ def test_incompatible_sequence_form_is_refused(self) -> None: "web": {"extends": {"service": "base"}, "cap_add": {"bad": "shape"}}, } } - with pytest.raises(UnsupportedComposeError, match="cannot merge 'cap_add' across incompatible forms"): + with pytest.raises(UnsupportedComposeError, match="'cap_add' must be a list"): resolve_extends(doc) def test_extends_non_dict_base_defers_to_validate(self) -> None: @@ -334,3 +339,124 @@ def test_structural_concat_merges_scalar_and_list_forms(self) -> None: } merged = resolve_extends(doc) assert merged["services"]["app"]["env_file"] == ["base.env", "local.env"] + + +class TestMergeNeverWidensTheGate: + """`extends` must not turn a form the gate refuses into one it accepts. + + `resolve_extends` runs ahead of `validate()`, so a normalizing merge can + launder an invalid shape into a valid one. This pins the invariant for every + mergeable key at once, so a new key cannot quietly leak the same way. + """ + + # A form each mergeable key does NOT have, paired with a valid base value. + HOSTILE: ClassVar[dict[str, tuple[Any, Any]]] = { + "ulimits": ({"nofile": 1}, ["nofile=2"]), # no list form in Compose + "healthcheck": ({"test": ["CMD", "x"]}, ["x"]), # no list form + "labels": ({"a": "1"}, 5), + "annotations": ({"a": "1"}, 5), + "environment": ({"A": "1"}, 5), + "extra_hosts": ({"h": "1.1.1.1"}, 5), + "cap_add": (["NET_ADMIN"], {"bad": "shape"}), + "devices": (["/dev/fuse"], {"bad": "shape"}), + "volumes": (["./a:/a"], {"bad": "shape"}), + "tmpfs": (["/scratch/a"], {"bad": "shape"}), + "env_file": (["a.env"], 5), + } + + @pytest.mark.parametrize("key", sorted(HOSTILE)) + def test_form_refused_standalone_is_refused_through_extends(self, key: str) -> None: + base_val, bad_val = self.HOSTILE[key] + + # The gate refuses this form on a plain service... + with pytest.raises(UnsupportedComposeError): + validate({"services": {"app": {"image": "x", key: bad_val}}}) + + # ...so it must not become acceptable by arriving through `extends`. + doc = { + "services": { + "base": {"image": "x", key: base_val}, + "app": {"extends": {"service": "base"}, key: bad_val}, + } + } + with pytest.raises(UnsupportedComposeError): + validate(resolve_extends(doc)) + + +class TestExtraHostsListFormMerge: + """extra_hosts has a list form in Compose, so a merge normalizes it -- correctly.""" + + def test_list_form_merges_with_map_form(self) -> None: + doc = { + "services": { + "base": {"image": "x", "extra_hosts": {"db": "1.1.1.1"}}, + "web": {"extends": {"service": "base"}, "extra_hosts": ["cache:2.2.2.2"]}, + } + } + assert resolve_extends(doc)["services"]["web"]["extra_hosts"] == {"db": "1.1.1.1", "cache": "2.2.2.2"} + + def test_local_wins_on_collision(self) -> None: + doc = { + "services": { + "base": {"image": "x", "extra_hosts": {"db": "1.1.1.1"}}, + "web": {"extends": {"service": "base"}, "extra_hosts": ["db:9.9.9.9"]}, + } + } + assert resolve_extends(doc)["services"]["web"]["extra_hosts"] == {"db": "9.9.9.9"} + + def test_ipv6_address_keeps_its_colons(self) -> None: + # Splitting on the *first* colon only. pairs_to_mapping would have split on + # '=' and produced a single {'myhost:::1': None} key. + doc = { + "services": { + "base": {"image": "x", "extra_hosts": {"a": "1.1.1.1"}}, + "web": {"extends": {"service": "base"}, "extra_hosts": ["myhost:::1"]}, + } + } + assert resolve_extends(doc)["services"]["web"]["extra_hosts"] == {"a": "1.1.1.1", "myhost": "::1"} + + def test_entry_without_a_colon_is_refused(self) -> None: + doc = { + "services": { + "base": {"image": "x", "extra_hosts": {"a": "1.1.1.1"}}, + "web": {"extends": {"service": "base"}, "extra_hosts": ["no-colon-here"]}, + } + } + with pytest.raises(UnsupportedComposeError, match="extra_hosts entries must be 'host:ip' strings"): + resolve_extends(doc) + + def test_non_string_entry_is_refused(self) -> None: + doc = { + "services": { + "base": {"image": "x", "extra_hosts": {"a": "1.1.1.1"}}, + "web": {"extends": {"service": "base"}, "extra_hosts": [5]}, + } + } + with pytest.raises(UnsupportedComposeError, match="extra_hosts entries must be 'host:ip' strings"): + resolve_extends(doc) + + +class TestStructuralConcatScalarForm: + """Only tmpfs/env_file have a bare-string form; the rest must stay lists.""" + + def test_volumes_scalar_form_is_refused(self) -> None: + # The gate refuses `volumes: "./a:/a"` standalone, so a merge must not + # normalize it into a list either. + doc = { + "services": { + "base": {"image": "x", "volumes": ["./a:/a"]}, + "web": {"extends": {"service": "base"}, "volumes": "./b:/b"}, + } + } + with pytest.raises(UnsupportedComposeError, match="'volumes' must be a list"): + resolve_extends(doc) + + def test_tmpfs_scalar_form_still_normalizes(self) -> None: + # Compose does give tmpfs a scalar form, and the gate accepts one. + doc = { + "services": { + "base": {"image": "x", "tmpfs": "/scratch/base"}, + "web": {"extends": {"service": "base"}, "tmpfs": ["/scratch/local"]}, + } + } + assert resolve_extends(doc)["services"]["web"]["tmpfs"] == ["/scratch/base", "/scratch/local"] From 34275c7b217ca63a98188d9627b6c98bbd5c8674 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 14 Jul 2026 17:56:44 +0300 Subject: [PATCH 2/3] fix: attribute merge errors to the right service; make the invariant test real The base-side spec.validate reported the base's malformed value under the extending service's name. _merge now takes base_name and attributes each side's failure to the service the value belongs to. The parametrized invariant test was 10/11 vacuous: it paired the list-only keys with a dict, which the old code already refused, instead of the bare scalar that actually laundered. It now uses the laundering shape, covers all 17 mergeable keys, and asserts its own completeness so a new key cannot be added without a case. Against the pre-fix code it goes red on nine keys; before, only one. --- architecture/supported-subset.md | 19 +++-- compose2pod/extends.py | 17 ++-- .../2026-07-14.05-extends-merge-policy.md | 31 +++++--- tests/test_extends.py | 78 ++++++++++++++++--- 4 files changed, 112 insertions(+), 33 deletions(-) diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 395b18a..941c44b 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -244,9 +244,13 @@ slot (`architecture/glossary.md`). refused standalone. So `ulimits` and `healthcheck` (no list form) refuse a list, and every list-only key (`cap_add`, `cap_drop`, `security_opt`, `devices`, `group_add`, `volumes`, `secrets`, `configs`) refuses a bare - scalar — exactly as each does outside `extends`. The accepted forms are - read from each key's own validator (`spec.validate` runs on both sides - before `spec.merge`), so the merge policy cannot drift from the gate. + scalar — exactly as each does outside `extends`. For a **registry** key the + accepted forms are read from the key's own validator (`spec.validate` runs + on both sides before `spec.merge`), so those cannot drift from the gate by + construction. The **structural** keys have no `KeySpec`, so their accepted + forms are declared in `extends.py` (`_STRUCTURAL_*`, `_SCALAR_FORM_KEYS`) + and kept honest by a test asserting, for every mergeable key, that a form + the gate refuses standalone is refused through `extends` too. `extra_hosts`' list form is `host:ip` — colon-separated, split on the *first* colon so an IPv6 address survives (`myhost:::1` → `{myhost: ::1}`). @@ -254,9 +258,12 @@ slot (`architecture/glossary.md`). bare-string (or any other non-mapping) `extends`; an unrecognized key under `extends` other than `service`; a non-string `service`; a `service` naming one that doesn't exist; and a merge across incompatible forms — a - side whose form that key does not have, which raises with the key's own - validator message (`'cap_add' must be a list`), the same message the value - produces standalone. + side whose form that key does not have. A registry key raises with its own + validator's message (`'cap_add' must be a list`), the same message the value + produces standalone; a structural key raises `cannot merge '' across + incompatible forms`. Either way the error names the service the offending + value belongs to — the base, when it is the base's value that is malformed, + not the service extending it. - **Divergences from Compose:** short-form `volumes` are concatenated rather than merged by target path; podman resolves duplicate mounts at run time. Referenced resources diff --git a/compose2pod/extends.py b/compose2pod/extends.py index 91500b0..986f470 100644 --- a/compose2pod/extends.py +++ b/compose2pod/extends.py @@ -104,8 +104,13 @@ def _as_mapping(name: str, key: str, value: Any) -> dict[str, Any]: # noqa: ANN raise UnsupportedComposeError(msg) -def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, Any]: - """Merge `local` onto `base` per key category: mapping-merge, sequence-concat, else override.""" +def _merge(base: dict[str, Any], local: dict[str, Any], name: str, base_name: str) -> dict[str, Any]: + """Merge `local` onto `base` per key category: mapping-merge, sequence-concat, else override. + + `name` is the extending service, `base_name` the one it extends: each side's + failures are reported against the service the offending value actually + belongs to. + """ merged: dict[str, Any] = dict(base) for key, local_val in local.items(): spec = SERVICE_KEYS.get(key) @@ -118,13 +123,13 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, # through the gate. Checking each side with the key's own validator # first derives the merge's accepted forms from the gate's, so the # two cannot drift apart. - spec.validate(name, key, base[key]) + spec.validate(base_name, key, base[key]) spec.validate(name, key, local_val) merged[key] = spec.merge(name, key, base[key], local_val) elif key in base and key in _STRUCTURAL_MERGE_KEYS: - merged[key] = {**_as_mapping(name, key, base[key]), **_as_mapping(name, key, local_val)} + merged[key] = {**_as_mapping(base_name, key, base[key]), **_as_mapping(name, key, local_val)} elif key in base and key in _STRUCTURAL_CONCAT_KEYS: - merged[key] = _as_concat_list(name, key, base[key]) + _as_concat_list(name, key, local_val) + merged[key] = _as_concat_list(base_name, key, base[key]) + _as_concat_list(name, key, local_val) else: merged[key] = local_val return merged @@ -167,7 +172,7 @@ def resolve(name: str) -> Any: # noqa: ANN401 - Compose values are untyped if not isinstance(base, dict): resolved[name] = local return local - merged = _merge(base, local, name) + merged = _merge(base, local, name, base_name) resolved[name] = merged return merged diff --git a/planning/changes/2026-07-14.05-extends-merge-policy.md b/planning/changes/2026-07-14.05-extends-merge-policy.md index 415dd2c..1608627 100644 --- a/planning/changes/2026-07-14.05-extends-merge-policy.md +++ b/planning/changes/2026-07-14.05-extends-merge-policy.md @@ -12,9 +12,11 @@ subsequently see — and if it normalizes a form the key does not actually have, it turns an invalid document into a valid one. That is what it was doing. One rule now governs every mergeable key: **a merge may normalize a value only -through a form that key actually has.** The accepted forms are derived from the -key's own validator rather than from a hand-maintained list, so the two cannot -drift. +through a form that key actually has.** For registry keys the accepted forms are +derived from the key's own validator, so those cannot drift from the gate by +construction. Structural keys have no `KeySpec`, so theirs stay declared in +`extends.py` — kept honest by a test that asserts the invariant for *every* +mergeable key and fails if a new one is added without a case. ## Motivation @@ -70,9 +72,15 @@ from validators that already exist; it does not build a dispatch table. ## The invariant, pinned A parametrized test asserts, for **every** mergeable key, that a form the gate -refuses standalone is also refused through `extends`. That is the property the -enumeration kept violating, and it now fails loudly rather than drifting. It is -verified to go red without the fix (on `ulimits`). +refuses standalone is also refused through `extends`, using the shape that +actually laundered (a bare scalar for the list-only keys, a list for the +mapping-only ones) rather than merely any invalid value. A companion test +asserts the table covers every mergeable key, so a new key cannot be added +without a case — the drift this change exists to end. + +Run against the pre-fix `extends.py` it goes red on nine keys (`cap_add`, +`cap_drop`, `configs`, `devices`, `group_add`, `secrets`, `security_opt`, +`ulimits`, `volumes`), which is the laundering it now prevents. ## Non-goals @@ -95,9 +103,14 @@ gate would have refused standalone: One previously-refused merge now works: list-form `extra_hosts`. -Two error messages improved: they now come from the key's validator -(`'cap_add' must be a list`) rather than the vaguer `cannot merge 'cap_add' -across incompatible forms`, matching what the same value yields standalone. +Registry-key merge errors now come from the key's own validator (`'cap_add' +must be a list`) rather than the vaguer `cannot merge 'cap_add' across +incompatible forms`, matching what the same value yields standalone. Structural +keys keep the `cannot merge ...` message. + +Merge errors also now name the service the offending value belongs to: a +malformed value in the *base* is reported against the base, not against the +service extending it. ## Testing diff --git a/tests/test_extends.py b/tests/test_extends.py index 196ec7d..b8550c5 100644 --- a/tests/test_extends.py +++ b/tests/test_extends.py @@ -3,7 +3,8 @@ import pytest from compose2pod.exceptions import UnsupportedComposeError -from compose2pod.extends import resolve_extends +from compose2pod.extends import _STRUCTURAL_CONCAT_KEYS, _STRUCTURAL_MERGE_KEYS, resolve_extends +from compose2pod.keys import SERVICE_KEYS from compose2pod.parsing import validate @@ -345,44 +346,97 @@ class TestMergeNeverWidensTheGate: """`extends` must not turn a form the gate refuses into one it accepts. `resolve_extends` runs ahead of `validate()`, so a normalizing merge can - launder an invalid shape into a valid one. This pins the invariant for every - mergeable key at once, so a new key cannot quietly leak the same way. + launder an invalid shape into a valid one. Each case below uses the shape + that actually laundered -- a bare scalar for the list-only keys, a list for + the mapping-only keys -- not merely any invalid value. """ - # A form each mergeable key does NOT have, paired with a valid base value. + # key -> (valid base value, a form that key does NOT have) HOSTILE: ClassVar[dict[str, tuple[Any, Any]]] = { - "ulimits": ({"nofile": 1}, ["nofile=2"]), # no list form in Compose - "healthcheck": ({"test": ["CMD", "x"]}, ["x"]), # no list form + # Mapping-only: Compose gives these no list form. + "ulimits": ({"nofile": 1}, ["nofile=2"]), + "healthcheck": ({"test": ["CMD", "x"]}, ["x"]), + # List-only: Compose gives these no scalar form. `as_list` used to + # normalize a bare string into [string], laundering it past the gate. + "cap_add": (["NET_ADMIN"], "SYS_TIME"), + "cap_drop": (["ALL"], "NET_RAW"), + "security_opt": (["label=disable"], "seccomp=unconfined"), + "devices": (["/dev/fuse"], "/dev/null"), + "group_add": (["1000"], "2000"), + "volumes": (["./a:/a"], "./b:/b"), + "secrets": (["s"], "s"), + "configs": (["c"], "c"), + # Map-or-list keys: a scalar is neither. "labels": ({"a": "1"}, 5), "annotations": ({"a": "1"}, 5), "environment": ({"A": "1"}, 5), "extra_hosts": ({"h": "1.1.1.1"}, 5), - "cap_add": (["NET_ADMIN"], {"bad": "shape"}), - "devices": (["/dev/fuse"], {"bad": "shape"}), - "volumes": (["./a:/a"], {"bad": "shape"}), - "tmpfs": (["/scratch/a"], {"bad": "shape"}), + "depends_on": ({"db": {}}, 5), + # Scalar IS a valid form for these two, so a non-scalar non-list is used. + "tmpfs": (["/scratch/a"], 5), "env_file": (["a.env"], 5), } + def test_table_covers_every_mergeable_key(self) -> None: + # Guards the guard: a new mergeable key cannot be added without a case + # here, which is exactly how the merge policy drifted from the gate. + mergeable = {key for key, spec in SERVICE_KEYS.items() if spec.merge is not None} + mergeable |= _STRUCTURAL_MERGE_KEYS | _STRUCTURAL_CONCAT_KEYS + assert set(self.HOSTILE) == mergeable + + # Top-level definitions the store keys need, so the only thing that can + # refuse them is the shape -- not an "unknown secret/config" lookup failure. + TOP_LEVEL: ClassVar[dict[str, dict[str, Any]]] = { + "secrets": {"secrets": {"s": {"environment": "E"}}}, + "configs": {"configs": {"c": {"content": "x"}}}, + } + @pytest.mark.parametrize("key", sorted(HOSTILE)) def test_form_refused_standalone_is_refused_through_extends(self, key: str) -> None: base_val, bad_val = self.HOSTILE[key] + top = self.TOP_LEVEL.get(key, {}) # The gate refuses this form on a plain service... with pytest.raises(UnsupportedComposeError): - validate({"services": {"app": {"image": "x", key: bad_val}}}) + validate({"services": {"app": {"image": "x", key: bad_val}}, **top}) # ...so it must not become acceptable by arriving through `extends`. doc = { "services": { "base": {"image": "x", key: base_val}, "app": {"extends": {"service": "base"}, key: bad_val}, - } + }, + **top, } with pytest.raises(UnsupportedComposeError): validate(resolve_extends(doc)) +class TestMergeErrorAttribution: + """A merge error names the service the offending value actually belongs to.""" + + def test_bad_value_in_the_base_blames_the_base(self) -> None: + # `web`'s own cap_add is a valid list; the base's is the malformed one. + doc = { + "services": { + "base": {"image": "x", "cap_add": "NET_ADMIN"}, + "web": {"extends": {"service": "base"}, "cap_add": ["SYS_TIME"]}, + } + } + with pytest.raises(UnsupportedComposeError, match="service 'base': 'cap_add' must be a list"): + resolve_extends(doc) + + def test_bad_value_in_the_extending_service_blames_it(self) -> None: + doc = { + "services": { + "base": {"image": "x", "cap_add": ["NET_ADMIN"]}, + "web": {"extends": {"service": "base"}, "cap_add": "SYS_TIME"}, + } + } + with pytest.raises(UnsupportedComposeError, match="service 'web': 'cap_add' must be a list"): + resolve_extends(doc) + + class TestExtraHostsListFormMerge: """extra_hosts has a list form in Compose, so a merge normalizes it -- correctly.""" From c3693a0ed542e600d27af7da84b5cc06043c3ef0 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 14 Jul 2026 18:06:33 +0300 Subject: [PATCH 3/3] docs: correct the normalization list and attribution claim; pin structural attribution The Extends section claimed 'nothing else is coerced' while omitting list-form labels/annotations, which are legitimately normalized through their own validator-approved list form. The 'error always names the owning service' claim had one reachable exception: spec.merge only receives the extending service's name, so a null ulimits in the base is reported against the child. Stated rather than overclaimed. Adds tests pinning base-side attribution on both structural paths (_as_mapping, _as_concat_list), which were correct but unpinned. --- architecture/supported-subset.md | 19 +++++++++++-------- tests/test_extends.py | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 941c44b..3647c1a 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -236,11 +236,12 @@ slot (`architecture/glossary.md`). as it would without `extends`. - **Normalization before merge — a merge never widens the gate.** A merged side is normalized only through a form that key *actually has*: list-form - `environment`, `extra_hosts` and `depends_on` become mappings before the - mapping-merge, and scalar-form `tmpfs`/`env_file` become a one-element - list before the concatenation. Nothing else is coerced. This matters - because `resolve_extends` runs **ahead of** `validate()`: normalizing a - form a key does not have would hand the gate a document it would have + `environment`, `labels`, `annotations`, `extra_hosts` and `depends_on` + become mappings before the mapping-merge, and scalar-form `tmpfs`/`env_file` + become a one-element list before the concatenation. Nothing else is + coerced. This matters because `resolve_extends` runs **ahead of** + `validate()`: normalizing a form a key does not have would hand the gate a + document it would have refused standalone. So `ulimits` and `healthcheck` (no list form) refuse a list, and every list-only key (`cap_add`, `cap_drop`, `security_opt`, `devices`, `group_add`, `volumes`, `secrets`, `configs`) refuses a bare @@ -261,9 +262,11 @@ slot (`architecture/glossary.md`). side whose form that key does not have. A registry key raises with its own validator's message (`'cap_add' must be a list`), the same message the value produces standalone; a structural key raises `cannot merge '' across - incompatible forms`. Either way the error names the service the offending - value belongs to — the base, when it is the base's value that is malformed, - not the service extending it. + incompatible forms`. A malformed *form* is reported against the service the + value belongs to — the base, when it is the base's value at fault, not the + service extending it. (One exception: an explicit `null` on a mergeable key + is valid standalone but refused on merge, and a null in the base is reported + against the extending service.) - **Divergences from Compose:** short-form `volumes` are concatenated rather than merged by target path; podman resolves duplicate mounts at run time. Referenced resources diff --git a/tests/test_extends.py b/tests/test_extends.py index b8550c5..15d063a 100644 --- a/tests/test_extends.py +++ b/tests/test_extends.py @@ -436,6 +436,28 @@ def test_bad_value_in_the_extending_service_blames_it(self) -> None: with pytest.raises(UnsupportedComposeError, match="service 'web': 'cap_add' must be a list"): resolve_extends(doc) + def test_structural_concat_base_value_blames_the_base(self) -> None: + # `_as_concat_list` path, not the registry one. + doc = { + "services": { + "base": {"image": "x", "volumes": "./a:/a"}, + "web": {"extends": {"service": "base"}, "volumes": ["./b:/b"]}, + } + } + with pytest.raises(UnsupportedComposeError, match="service 'base': 'volumes' must be a list"): + resolve_extends(doc) + + def test_structural_mapping_base_value_blames_the_base(self) -> None: + # `_as_mapping` path: healthcheck has no list form. + doc = { + "services": { + "base": {"image": "x", "healthcheck": ["bad"]}, + "web": {"extends": {"service": "base"}, "healthcheck": {"test": ["CMD", "x"]}}, + } + } + with pytest.raises(UnsupportedComposeError, match="service 'base': cannot merge 'healthcheck'"): + resolve_extends(doc) + class TestExtraHostsListFormMerge: """extra_hosts has a list form in Compose, so a merge normalizes it -- correctly."""