From ce30bcc32259475e52f5379d8c6ba14199162583 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 14 Jul 2026 19:33:15 +0300 Subject: [PATCH 1/3] fix: match Docker's null-value policy compose2pod's treatment of an explicitly-null service value was ad hoc -- an accident of which keys happened to get a hand-written validator. Measured against `docker compose config` (v5.1.2) key by key, it diverged on 13, in both directions: entrypoint: compose2pod refused, Docker accepts environment, env_file, volumes, compose2pod accepted, Docker refuses tmpfs, healthcheck, depends_on, networks, hostname, container_name, secrets, configs, pull_policy, ulimits The split had no design behind it: `labels:` (null) was refused while `environment:` (null) was accepted, purely because one routes through validate_map and the other had a validator that early-returned on None. Docker's rule is exact: an explicit null is accepted for command, entrypoint and deploy -- where it means "not specified" -- and refused everywhere else. That is now one check at the gate, not 13 per-key decisions. extends treats a null side as "not specified", so the other side's value survives: a null in the extending service inherits the base's value (verified byte-for-byte against docker compose config), a null in the base takes the child's, and a null on both survives resolution for the gate to refuse. That restores the invariant in the accepting direction -- a document valid standalone stays valid through extends. Breaking: a bare `environment:` / `volumes:` / etc now raises. Such a document is not valid Compose; it only ever worked here, and it silently did nothing. --- architecture/supported-subset.md | 21 +++- compose2pod/extends.py | 9 +- compose2pod/keys.py | 5 +- compose2pod/parsing.py | 31 +++++- .../2026-07-14.07-null-value-policy.md | 101 ++++++++++++++++++ tests/test_extends.py | 47 ++++++++ tests/test_parsing.py | 79 +++++++++++--- 7 files changed, 271 insertions(+), 22 deletions(-) create mode 100644 planning/changes/2026-07-14.07-null-value-policy.md diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 2699d7d..23abb43 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -86,6 +86,15 @@ redundant only when reached through `validate()`. Each entry in `services` must itself be a mapping; any other shape (a bare string, a list, ...) raises. +**An explicitly-null value raises**, for every service key except `command`, +`entrypoint` and `deploy` — the three where Compose gives a null a meaning +("not specified") and `docker compose config` accepts one. A bare +`environment:` with its contents deleted is a mistake, not an instruction to +emit nothing, so it is refused rather than silently dropped +(`_reject_null_values`). One rule, taken from Docker's own verdict key by key, +rather than a per-key decision to keep in sync. `x-` extension keys are exempt: +their contents are arbitrary user payload compose2pod never reads. + - **Supported:** `image`, `build`, `command`, `entrypoint`, `environment`, `env_file`, `volumes`, `healthcheck`, `depends_on`, `networks`, `hostname`, `container_name`, `tmpfs`, `secrets`, `configs`, plus the declarative @@ -264,9 +273,15 @@ slot (`architecture/glossary.md`). produces standalone; a structural key raises `cannot merge '' across 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.) + service extending it. +- **A null side means "not specified".** An explicitly-null value on either + side of a merge is not a form at all, so the other side's value survives: a + null in the extending service inherits the base's value (what Docker does), + and a null in the base takes the extending service's. A null on *both* sides + survives resolution and the gate then refuses it — as Docker refuses a null + that no inheritance overwrote. This is what makes the invariant hold in both + directions: a value the gate accepts standalone is accepted through + `extends`, and one it refuses standalone is refused through `extends`. - **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 c4acced..30aaae8 100644 --- a/compose2pod/extends.py +++ b/compose2pod/extends.py @@ -115,7 +115,14 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str, base_name: st merged: dict[str, Any] = dict(base) 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: + if key in base and (local_val is None or base[key] is None): + # A null side means "not specified", so the other side's value + # survives -- what Docker does, and what keeps a document valid + # through `extends` if it is valid standalone. A null on *both* + # sides survives resolution, and the gate refuses it (as Docker + # refuses a null no inheritance overwrote). + merged[key] = base[key] if local_val is None else local_val + elif 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: diff --git a/compose2pod/keys.py b/compose2pod/keys.py index 59426a8..46b8407 100644 --- a/compose2pod/keys.py +++ b/compose2pod/keys.py @@ -271,8 +271,9 @@ def _emit_pull_policy(value: Any) -> list[Token]: # noqa: ANN401 - Compose valu def _validate_ulimits(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped YAML/JSON - if value is None: - return + # No `value is None` escape: the gate refuses a null `ulimits:` outright + # (`parsing._reject_null_values`), as Docker does, so a null reaching here + # is a wrong shape like any other. if not isinstance(value, dict): msg = f"service {name!r}: '{key}' must be a mapping" raise UnsupportedComposeError(msg) diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index c8ef4b6..3055ae6 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -13,6 +13,10 @@ SUPPORTED_SERVICE_KEYS = set(SERVICE_KEYS) | STRUCTURAL_KEYS IGNORED_SERVICE_KEYS = {"ports", "restart", "stdin_open", "tty", "stop_signal", "stop_grace_period", "profiles"} +# The only service keys Docker tolerates an explicit null on, where it means +# "not specified" (measured against `docker compose config`). Every other key +# with a bare `key:` is refused -- see `_reject_null_values`. +NULL_ALLOWED_KEYS = {"command", "entrypoint", "deploy"} SUPPORTED_HEALTHCHECK_KEYS = {"test", "interval", "timeout", "retries", "start_period"} _HEALTHCHECK_SCALAR_KEYS = ("timeout", "retries", "start_period") SUPPORTED_TOP_LEVEL_KEYS = {"services", "version", "name", "networks", "volumes", "secrets", "configs"} @@ -103,9 +107,12 @@ def _validate_argv_list(name: str, key: str, value: list[Any]) -> None: def _validate_entrypoint(name: str, svc: dict[str, Any]) -> None: """Check the structural entrypoint key's form (it is not a registry key).""" - if "entrypoint" not in svc: + entrypoint = svc.get("entrypoint") + if entrypoint is None: + # An explicit null means "not specified", as it does for its sibling + # `command` and as Docker accepts for both. `entrypoint_tokens` emits + # nothing for it. return - entrypoint = svc["entrypoint"] if not isinstance(entrypoint, str | list): msg = f"service {name!r}: 'entrypoint' must be a string or list" raise UnsupportedComposeError(msg) @@ -155,6 +162,25 @@ def _validate_env_file(name: str, svc: dict[str, Any]) -> None: _validate_string_or_string_list(name, "env_file", svc.get("env_file")) +def _reject_null_values(name: str, svc: dict[str, Any]) -> None: + """Refuse an explicitly-null service value, everywhere Docker refuses one. + + Measured against `docker compose config`: it tolerates an explicit null on + exactly `command`, `entrypoint` and `deploy` -- where a null means "not + specified" -- and refuses one on every other service key, because a bare + `environment:` is almost always a deleted-contents mistake rather than an + intent. Emitting nothing for it silently would be exactly the dropped + behavior this gate exists to catch. + + One rule rather than a per-key decision, so the policy cannot drift back + into an enumeration. `x-` keys are arbitrary user payload and are exempt. + """ + for key, value in svc.items(): + if value is None and key not in NULL_ALLOWED_KEYS and not key.startswith("x-"): + msg = f"service {name!r}: '{key}' must not be null (Compose refuses a bare '{key}:')" + raise UnsupportedComposeError(msg) + + def _validate_service(name: str, svc: Any) -> list[str]: # noqa: ANN401 - Compose values are untyped """Validate one service; returns warnings, raises UnsupportedComposeError.""" if not isinstance(svc, dict): @@ -165,6 +191,7 @@ def _validate_service(name: str, svc: Any) -> list[str]: # noqa: ANN401 - Compo # contract as a module entry point -- belt-and-braces, not load-bearing # only by luck of the current call graph. require_string_keys(f"service {name!r}", svc) + _reject_null_values(name, svc) warnings: list[str] = [] for key in sorted(svc): if key.startswith("x-"): diff --git a/planning/changes/2026-07-14.07-null-value-policy.md b/planning/changes/2026-07-14.07-null-value-policy.md new file mode 100644 index 0000000..82a2205 --- /dev/null +++ b/planning/changes/2026-07-14.07-null-value-policy.md @@ -0,0 +1,101 @@ +--- +summary: An explicit null service value is refused everywhere Docker refuses it (all keys but command/entrypoint/deploy), accepted where Docker accepts it, and treated as "not specified" by an extends merge so the other side's value survives. +--- + +# Design: Match Docker's null-value policy + +## Summary + +compose2pod's treatment of an explicitly-null service value (`environment:` with +nothing after it) was ad hoc — an accident of which keys happened to get a +hand-written validator. It diverged from Docker on **13 keys**, in both +directions, and `extends` refused a null that the gate accepted, so a document +valid on its own became invalid through inheritance. + +One rule, taken from Docker: **an explicit null is refused for every service key +except `command`, `entrypoint`, and `deploy`.** An `extends` merge treats a null +side as "not specified", so the other side's value survives. + +## Motivation + +Measured against `docker compose config` (v5.1.2), key by key. Docker accepts an +explicit null for exactly three service keys — `command`, `entrypoint`, `deploy` +— and refuses it for every other one. compose2pod disagreed on 13: + +| | compose2pod | Docker | +|---|---|---| +| `entrypoint` | refuses | **accepts** | +| `environment`, `env_file`, `volumes`, `tmpfs`, `healthcheck`, `depends_on`, `networks`, `hostname`, `container_name`, `secrets`, `configs`, `pull_policy`, `ulimits` | accepts | **refuses** | + +The split had no design behind it: `labels:` (null) was refused while +`environment:` (null) was accepted, purely because `labels` routes through +`validate_map` and `environment` had a validator that early-returned on `None`. + +Two consequences: + +- **A bare key is silently ignored.** `environment:` with its contents deleted + emits no `-e` flags and says nothing. Docker refuses it, because it is nearly + always a mistake. Quietly dropping it is the failure this project's gate + exists to prevent. +- **`extends` refused what the gate accepted.** Every mergeable key that + tolerated a null standalone raised `cannot merge ... across incompatible + forms` when that null met a real value on the other side — so a valid document + became invalid by being inherited from. + +Docker's own `extends` does not do this: a null in the extending service is +simply inherited over. Verified — a child declaring `environment:` and +`volumes:` as nulls comes out of `docker compose config` with the base's values +intact. + +## Design + +**The gate** grows one check in `_validate_service`, ahead of the per-key +validators: a service key present with a `None` value raises, unless it is one +of `command`/`entrypoint`/`deploy` (the three Docker allows, where null means +"not specified") or an `x-` extension field (arbitrary user payload). One rule, +derived from Docker, rather than 13 per-key decisions to keep in sync. + +`entrypoint` stops refusing a null, matching Docker and its sibling `command` +(which already accepts one). + +**The merge** treats a null side as absent, so the other side's value survives: + +- null in the extending service → the base's value is inherited (Docker's + behavior). +- null in the base → the extending service's value wins. +- null on both → null survives resolution and the gate refuses it, exactly as + Docker refuses a null that no inheritance overwrites. + +This is what makes the invariant hold in both directions: a form the gate accepts +standalone is accepted through `extends`, and a form it refuses standalone is +refused through `extends` (`2026-07-14.05`). + +## Non-goals + +- Not changing what a null *means* where Docker allows it: `command:`, + `entrypoint:`, and `deploy:` still mean "not specified" and emit nothing. +- Not touching null handling inside a value (`environment: {KEY: null}` is a + host-passthrough and stays exactly as it is). This is about a null *key value*. + +## Behavior change + +**Breaking, deliberately.** A document with a bare `environment:`, `volumes:`, +`healthcheck:`, `depends_on:`, `secrets:`, `configs:`, `ulimits:`, `env_file:`, +`tmpfs:`, `networks:`, `hostname:`, `container_name:` or `pull_policy:` now +raises where it previously ran. Such a document is **not valid Compose** — +`docker compose config` rejects it — so it only ever worked here by accident, +and it silently did nothing. + +One over-rejection is lifted: `entrypoint:` (null) is now accepted, as Docker +accepts it. + +## Testing + +`just test-ci` at 100%. A parametrized test pins the policy for every service +key against the measured Docker verdict, so the rule cannot drift back into an +enumeration. Plus the `extends` inherit-over-null cases in both directions. + +## Risk + +- **A user with a bare key gets an error.** Intended: the key did nothing, and + Docker refuses it. The message names the key and says a null is not allowed. diff --git a/tests/test_extends.py b/tests/test_extends.py index 4e85373..145b7c8 100644 --- a/tests/test_extends.py +++ b/tests/test_extends.py @@ -557,3 +557,50 @@ def test_tmpfs_scalar_form_still_normalizes(self) -> None: } } assert resolve_extends(doc)["services"]["web"]["tmpfs"] == ["/scratch/base", "/scratch/local"] + + +class TestNullIsNotSpecified: + """A null side means 'not specified', so the other side's value survives (as Docker does).""" + + def test_null_in_the_extending_service_inherits_the_base(self) -> None: + # `docker compose config` on the same document keeps the base's value. + doc = { + "services": { + "base": {"image": "x", "environment": {"A": "1"}, "volumes": ["./a:/a"]}, + "web": {"extends": {"service": "base"}, "environment": None, "volumes": None}, + } + } + web = resolve_extends(doc)["services"]["web"] + assert web["environment"] == {"A": "1"} + assert web["volumes"] == ["./a:/a"] + + def test_null_in_the_base_takes_the_extending_service_value(self) -> None: + doc = { + "services": { + "base": {"image": "x", "environment": None}, + "web": {"extends": {"service": "base"}, "environment": {"B": "2"}}, + } + } + assert resolve_extends(doc)["services"]["web"]["environment"] == {"B": "2"} + + def test_null_on_both_sides_survives_and_the_gate_refuses_it(self) -> None: + doc = { + "services": { + "base": {"image": "x", "environment": None}, + "web": {"extends": {"service": "base"}, "environment": None}, + } + } + merged = resolve_extends(doc) + assert merged["services"]["web"]["environment"] is None + with pytest.raises(UnsupportedComposeError, match="'environment' must not be null"): + validate(merged) + + def test_a_document_valid_standalone_stays_valid_through_extends(self) -> None: + # The invariant the null handling exists to restore, in the accepting direction. + doc = { + "services": { + "base": {"image": "x", "command": ["run"]}, + "web": {"extends": {"service": "base"}, "command": None}, + } + } + assert validate(resolve_extends(doc)) == [] diff --git a/tests/test_parsing.py b/tests/test_parsing.py index e7647dc..dd55f6d 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -94,9 +94,6 @@ def test_single_slash_string_volumes_rejected_at_gate(self) -> None: with pytest.raises(UnsupportedComposeError, match=r"'volumes' must be a list"): validate({"services": {"app": {"image": "x", "volumes": "/"}}}) - def test_null_volumes_is_accepted(self) -> None: - assert validate({"services": {"app": {"image": "x", "volumes": None}}}) == [] - def test_no_services_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="no services"): validate({"services": {}}) @@ -354,12 +351,6 @@ def test_ulimits_boolean_soft_hard_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="'soft' and 'hard' must be int or str"): validate({"services": {"app": {"image": "x", "ulimits": {"nofile": {"soft": True, "hard": 100}}}}}) - def test_pull_policy_null_is_accepted(self) -> None: - assert validate({"services": {"app": {"image": "x", "pull_policy": None}}}) == [] - - def test_ulimits_null_is_accepted(self) -> None: - assert validate({"services": {"app": {"image": "x", "ulimits": None}}}) == [] - def test_non_mapping_healthcheck_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="healthcheck must be a mapping"): validate({"services": {"app": {"image": "x", "healthcheck": ["CMD", "true"]}}}) @@ -442,7 +433,6 @@ def test_tmpfs_list_with_non_string_entry_rejected_at_gate(self) -> None: def test_tmpfs_string_and_list_of_strings_still_accepted(self) -> None: assert validate({"services": {"app": {"image": "x", "tmpfs": "/tmp"}}}) == [] # noqa: S108 assert validate({"services": {"app": {"image": "x", "tmpfs": ["/tmp", "/run"]}}}) == [] # noqa: S108 - assert validate({"services": {"app": {"image": "x", "tmpfs": None}}}) == [] def test_non_string_hostname_raises_at_gate(self) -> None: with pytest.raises(UnsupportedComposeError, match="hostname must be a string"): @@ -519,9 +509,6 @@ def test_string_environment_rejected_at_gate(self) -> None: with pytest.raises(UnsupportedComposeError, match=r"'environment' must be a list or mapping"): validate({"services": {"app": {"image": "x", "environment": "FOO=bar"}}}) - def test_null_environment_is_accepted(self) -> None: - assert validate({"services": {"app": {"image": "x", "environment": None}}}) == [] - def test_environment_list_of_mapping_rejected_at_gate(self) -> None: # The commonest Compose YAML slip: `- KEY: value` (list + mapping) # instead of `- KEY=value`. Used to be silently accepted and emit the @@ -562,7 +549,6 @@ def test_non_string_non_list_env_file_rejected_at_gate(self) -> None: def test_string_and_list_env_file_are_accepted(self) -> None: assert validate({"services": {"app": {"image": "x", "env_file": "tests.env"}}}) == [] assert validate({"services": {"app": {"image": "x", "env_file": ["a.env", "b.env"]}}}) == [] - assert validate({"services": {"app": {"image": "x", "env_file": None}}}) == [] def test_env_file_list_with_non_string_entry_rejected_at_gate(self) -> None: # Used to reach emit and crash with TypeError: argument should be a str or an os.PathLike object. @@ -909,3 +895,68 @@ def test_non_string_key_nested_in_a_list_is_caught_by_the_sweep_itself(self) -> } with pytest.raises(UnsupportedComposeError, match=r"service 'app'\.secrets: key 1 must be a string"): validate(compose) + + +class TestNullValuePolicy: + """An explicit null is refused everywhere Docker refuses it. + + Measured against `docker compose config` (v5.1.2), key by key: Docker accepts + an explicit null for exactly `command`, `entrypoint` and `deploy` -- where it + means "not specified" -- and refuses it for every other service key. A bare + `environment:` is nearly always a deleted-contents mistake, and silently + emitting nothing for it is the failure this gate exists to prevent. + """ + + # The only service keys Docker tolerates an explicit null on. + NULL_OK = ("command", "entrypoint", "deploy") + + NULL_REFUSED = ( + "environment", + "env_file", + "volumes", + "tmpfs", + "healthcheck", + "depends_on", + "networks", + "hostname", + "container_name", + "secrets", + "configs", + "pull_policy", + "ulimits", + "user", + "working_dir", + "platform", + "init", + "read_only", + "privileged", + "group_add", + "cap_add", + "cap_drop", + "security_opt", + "devices", + "labels", + "annotations", + "extra_hosts", + "dns", + "sysctls", + "mem_limit", + "cpus", + ) + + @pytest.mark.parametrize("key", NULL_OK) + def test_null_accepted_where_docker_accepts_it(self, key: str) -> None: + assert validate({"services": {"app": {"image": "x", key: None}}}) == [] + + @pytest.mark.parametrize("key", NULL_REFUSED) + def test_null_refused_where_docker_refuses_it(self, key: str) -> None: + with pytest.raises(UnsupportedComposeError, match=f"'{key}' must not be null"): + validate({"services": {"app": {"image": "x", key: None}}}) + + def test_null_extension_field_is_accepted(self) -> None: + # `x-` blocks are arbitrary user payload; compose2pod never reads them. + assert validate({"services": {"app": {"image": "x", "x-anything": None}}}) == [] + + def test_absent_key_is_not_a_null_key(self) -> None: + # The check is about a key present with no value, not a key that is missing. + assert validate({"services": {"app": {"image": "x"}}}) == [] From 80d4bbaf08eb610f23fb45231722ed5dd9d4c06c Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 14 Jul 2026 19:46:07 +0300 Subject: [PATCH 2/3] fix: a null command/entrypoint resets, it does not inherit The first pass applied "a null side means not specified, so the other side's value survives" uniformly. Docker splits: a child's null `environment:`/ `volumes:`/`deploy:`/... inherits the base's value, but a child's null `command:`/`entrypoint:` ERASES it -- the image's own default runs. Verified against docker compose config: the child's rendered config has no command key. So the uniform rule regressed `command`, which main had right: the merged service silently ran the base's argv instead of the image default. The reset set is {command, entrypoint}, not the null-allowed set -- `deploy` tolerates a null and inherits. The suite passed identically with and without the bug, because nothing asserted the merged command's VALUE. The new tests assert at emit level: a reset command must not put the base's argv in the podman-run line. --- architecture/supported-subset.md | 17 +++--- compose2pod/extends.py | 25 ++++++-- .../2026-07-14.07-null-value-policy.md | 11 ++-- tests/test_extends.py | 59 +++++++++++++++++++ 4 files changed, 94 insertions(+), 18 deletions(-) diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 23abb43..8e4a03e 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -274,14 +274,15 @@ slot (`architecture/glossary.md`). 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. -- **A null side means "not specified".** An explicitly-null value on either - side of a merge is not a form at all, so the other side's value survives: a - null in the extending service inherits the base's value (what Docker does), - and a null in the base takes the extending service's. A null on *both* sides - survives resolution and the gate then refuses it — as Docker refuses a null - that no inheritance overwrote. This is what makes the invariant hold in both - directions: a value the gate accepts standalone is accepted through - `extends`, and one it refuses standalone is refused through `extends`. +- **A null in the extending service means "not specified"** — the base's value + is inherited, as Docker does. **Except `command` and `entrypoint`, where a + null is a *reset*:** Docker erases the inherited value so the image's own + default runs, and so does compose2pod. (`deploy` tolerates a null and + inherits, so the reset set is not simply "the keys that allow a null".) + A null in the *base* cannot be observed in a document that passes the gate — + the base is itself a service, so `_reject_null_values` refuses its null + first, exactly as Docker does; the merge handles it only to report the error + against the base rather than 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 30aaae8..f2f4166 100644 --- a/compose2pod/extends.py +++ b/compose2pod/extends.py @@ -20,6 +20,13 @@ # other would accept a shape the gate refuses standalone. _SCALAR_FORM_KEYS = {"tmpfs", "env_file"} +# The keys where an explicit null in the extending service RESETS the inherited +# value rather than meaning "not specified". Docker erases a child's null +# `command:`/`entrypoint:` so the image's own default runs, while a child's null +# `environment:`/`volumes:`/`deploy:`/... inherits the base's value -- so this is +# not "the keys that tolerate a null" (`deploy` tolerates one and inherits). +_RESET_ON_NULL_KEYS = {"command", "entrypoint"} + 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.""" @@ -115,12 +122,18 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str, base_name: st merged: dict[str, Any] = dict(base) for key, local_val in local.items(): spec = SERVICE_KEYS.get(key) - if key in base and (local_val is None or base[key] is None): - # A null side means "not specified", so the other side's value - # survives -- what Docker does, and what keeps a document valid - # through `extends` if it is valid standalone. A null on *both* - # sides survives resolution, and the gate refuses it (as Docker - # refuses a null no inheritance overwrote). + if key in base and local_val is None and key in _RESET_ON_NULL_KEYS: + # An explicit null `command:`/`entrypoint:` in the extending service + # is a *reset*, not an omission: Docker erases the inherited value and + # the image's own default runs. (`deploy:` tolerates a null too but + # inherits, so this is not simply "the keys that allow a null".) + merged[key] = None + elif key in base and (local_val is None or base[key] is None): + # Every other null side means "not specified", so the other side's + # value survives -- what Docker does, and what keeps a document valid + # through `extends` if it is valid standalone. A null on *both* sides + # survives resolution, and the gate refuses it (as Docker refuses a + # null that no inheritance overwrote). merged[key] = base[key] if local_val is None else local_val elif 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` diff --git a/planning/changes/2026-07-14.07-null-value-policy.md b/planning/changes/2026-07-14.07-null-value-policy.md index 82a2205..100bb34 100644 --- a/planning/changes/2026-07-14.07-null-value-policy.md +++ b/planning/changes/2026-07-14.07-null-value-policy.md @@ -87,13 +87,16 @@ raises where it previously ran. Such a document is **not valid Compose** — and it silently did nothing. One over-rejection is lifted: `entrypoint:` (null) is now accepted, as Docker -accepts it. +accepts it — and in an `extends` chain it resets the inherited entrypoint rather +than inheriting it, again matching Docker. ## Testing -`just test-ci` at 100%. A parametrized test pins the policy for every service -key against the measured Docker verdict, so the rule cannot drift back into an -enumeration. Plus the `extends` inherit-over-null cases in both directions. +`just test-ci` at 100%. A parametrized test pins the gate's verdict against Docker's measured one for +34 service keys (the supported set plus the ignored keys), so the rule cannot +drift back into an enumeration. Emit-level tests pin the `command`/`entrypoint` +reset — a merged null must not emit the base's argv — since asserting only that +the document validates would green-light the wrong behavior. ## Risk diff --git a/tests/test_extends.py b/tests/test_extends.py index 145b7c8..75c1381 100644 --- a/tests/test_extends.py +++ b/tests/test_extends.py @@ -2,6 +2,7 @@ import pytest +from compose2pod.emit import EmitOptions, emit_script from compose2pod.exceptions import UnsupportedComposeError from compose2pod.extends import _STRUCTURAL_CONCAT_KEYS, _STRUCTURAL_MERGE_KEYS, resolve_extends from compose2pod.keys import SERVICE_KEYS @@ -604,3 +605,61 @@ def test_a_document_valid_standalone_stays_valid_through_extends(self) -> None: } } assert validate(resolve_extends(doc)) == [] + + +class TestNullResetsCommandAndEntrypoint: + """A null `command`/`entrypoint` in the extending service ERASES the base's. + + Docker splits here: `environment:`/`volumes:`/`deploy:` null in a child inherit + the base's value, but `command:`/`entrypoint:` null erase it -- the image's own + default runs. Verified against `docker compose config`: the child's rendered + config has no `command` key at all. + """ + + def _script(self, doc: dict) -> str: + options = EmitOptions( + target="web", + ci_image="ci:latest", + command="", + pod="test-pod", + project_dir=".", + artifacts=[], + allow_exit_codes=[], + ) + return emit_script(compose=resolve_extends(doc), options=options) + + def test_null_command_erases_the_inherited_command(self) -> None: + doc = { + "services": { + "base": {"image": "alpine", "command": ["run", "base"]}, + "web": {"extends": {"service": "base"}, "command": None}, + } + } + assert resolve_extends(doc)["services"]["web"]["command"] is None + # The image's own default must run -- the base's argv must not be emitted. + # (Assert on the podman-run line, not the whole script: "podman run" contains "run".) + run_line = next(line for line in self._script(doc).splitlines() if "--name test-pod-web" in line) + assert '"run"' not in run_line, run_line + assert '"base"' not in run_line, run_line + + def test_null_entrypoint_erases_the_inherited_entrypoint(self) -> None: + doc = { + "services": { + "base": {"image": "alpine", "entrypoint": ["/bin/sh", "-c"]}, + "web": {"extends": {"service": "base"}, "entrypoint": None}, + } + } + assert resolve_extends(doc)["services"]["web"]["entrypoint"] is None + assert "--entrypoint" not in self._script(doc) + + def test_null_deploy_still_inherits(self) -> None: + # deploy allows a null too, but Docker INHERITS it -- so the reset set is + # {command, entrypoint}, not "every key that tolerates a null". + doc = { + "services": { + "base": {"image": "alpine", "deploy": {"resources": {"limits": {"memory": "100M"}}}}, + "web": {"extends": {"service": "base"}, "deploy": None}, + } + } + assert resolve_extends(doc)["services"]["web"]["deploy"] == {"resources": {"limits": {"memory": "100M"}}} + assert "--memory" in self._script(doc) From 0235caaf9480bca23b7a3e82da57f2c84c0b3b8b Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 14 Jul 2026 19:55:36 +0300 Subject: [PATCH 3/3] fix: make the null guards coherent; correct the base-null doc claim _validate_ulimits raised on a null while _validate_pull_policy returned clean -- two registry keys disagreeing about the same value through the same seam. Null policy belongs in one place, the gate, so neither shape validator special-cases it now: a null reaching a validator is a wrong shape like any other. The emitters stay null-tolerant as defense-in-depth, since run_flags is reachable without the gate and PULL_POLICY_MAP[None] would be a raw KeyError. The architecture doc claimed a base-side null "cannot be observed in a document that passes the gate". False for exactly the three keys that paragraph is about: the gate allows a null command/entrypoint/deploy, so a base with `command:` and a child that sets one is valid, reaches the merge, and the child's value wins -- as Docker does. --- architecture/supported-subset.md | 9 +++++---- compose2pod/keys.py | 9 ++++++++- planning/changes/2026-07-14.07-null-value-policy.md | 7 +++++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 8e4a03e..c4c91d3 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -279,10 +279,11 @@ slot (`architecture/glossary.md`). null is a *reset*:** Docker erases the inherited value so the image's own default runs, and so does compose2pod. (`deploy` tolerates a null and inherits, so the reset set is not simply "the keys that allow a null".) - A null in the *base* cannot be observed in a document that passes the gate — - the base is itself a service, so `_reject_null_values` refuses its null - first, exactly as Docker does; the merge handles it only to report the error - against the base rather than the service extending it. + A null in the *base* means "not specified" the same way, so the extending + service's value wins. This is reachable for exactly the three keys the gate + allows a null on: a base with `command:` and a child that sets one gives the + child's, as Docker does. For every other key the base's null is refused by + the gate first (the base is a service too), so the merge never sees 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/keys.py b/compose2pod/keys.py index 46b8407..1e1cbe2 100644 --- a/compose2pod/keys.py +++ b/compose2pod/keys.py @@ -260,13 +260,20 @@ def extra_host_entries(value: list[Any] | dict[str, Any]) -> list[tuple[str, str def _validate_pull_policy(name: str, key: str, value: Any) -> None: # noqa: ANN401 - Compose values are untyped - if value is not None and (not isinstance(value, str) or value not in PULL_POLICY_MAP): + # No `value is None` escape, matching `_validate_ulimits`: the gate refuses a + # null `pull_policy:` outright (`parsing._reject_null_values`), as Docker + # does, so a null reaching a *shape* validator is a wrong shape like any + # other. Null policy lives in one place -- the gate -- not in each validator. + if not isinstance(value, str) or value not in PULL_POLICY_MAP: allowed = "/".join(PULL_POLICY_MAP) msg = f"service {name!r}: unsupported {key} {value!r} (use {allowed})" raise UnsupportedComposeError(msg) def _emit_pull_policy(value: Any) -> list[Token]: # noqa: ANN401 - Compose values are untyped + # The emitters stay null-tolerant as defense-in-depth: they are reachable + # from `run_flags` without the gate, and `PULL_POLICY_MAP[None]` would be a + # raw KeyError rather than a clean refusal. return ["--pull", PULL_POLICY_MAP[value]] if value is not None else [] diff --git a/planning/changes/2026-07-14.07-null-value-policy.md b/planning/changes/2026-07-14.07-null-value-policy.md index 100bb34..9f73a09 100644 --- a/planning/changes/2026-07-14.07-null-value-policy.md +++ b/planning/changes/2026-07-14.07-null-value-policy.md @@ -93,8 +93,11 @@ than inheriting it, again matching Docker. ## Testing `just test-ci` at 100%. A parametrized test pins the gate's verdict against Docker's measured one for -34 service keys (the supported set plus the ignored keys), so the rule cannot -drift back into an enumeration. Emit-level tests pin the `command`/`entrypoint` +34 service keys, so the rule cannot drift back into an enumeration. It is a +sample, not the whole key universe (56): the ignored keys, `image`/`build` and +the numeric resource keys are not in the table, though each was checked against +Docker by hand and behaves correctly -- the gate's rule is uniform, so the +sample is what pins it. Emit-level tests pin the `command`/`entrypoint` reset — a merged null must not emit the base's argv — since asserting only that the document validates would green-light the wrong behavior.