Skip to content

Commit eb7c3a9

Browse files
authored
fix: refuse a null wherever docker compose refuses one (#61)
compose2pod is a drop-in replacement for docker compose on rootless runners: the file it converts is the file the developer runs locally. That makes accepting-what-Docker-refuses actively harmful rather than merely lax -- the document is already broken in the real workflow, so converting it anyway means CI goes green on a file docker compose would not run. A false green is worse than a hard error. So the test is not "does this null drop behavior?" but "would docker compose run this file at all?". Eight positions failed it, accepted here and refused by docker compose config, each emitting nothing: healthcheck.test / .interval / .timeout / .retries / .start_period deploy.resources / .limits / .reservations top-level networks: / volumes: / secrets: / configs: This reverses changes/2026-07-13.10's null-scalar ruling (a null healthcheck timeout/retries/start_period meant "unset"). That ruling was taken on the stated premise that it matched docker compose config. It does not -- Docker refuses all three. The premise was wrong, so the conclusion goes with it. A null INSIDE a value is untouched and still accepted, because Docker accepts it too: environment: {KEY: null} is host-passthrough, labels: {KEY: null} an empty label.
1 parent 8ba9033 commit eb7c3a9

6 files changed

Lines changed: 212 additions & 31 deletions

File tree

architecture/supported-subset.md

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,19 @@ emit nothing, so it is refused rather than silently dropped
9595
rather than a per-key decision to keep in sync. `x-` extension keys are exempt:
9696
their contents are arbitrary user payload compose2pod never reads.
9797

98+
The same rule holds **wherever a null can appear** — the healthcheck sub-keys,
99+
`deploy.resources` and its `limits`/`reservations`, and the top-level
100+
`networks:`/`volumes:`/`secrets:`/`configs:` blocks. compose2pod is a drop-in
101+
replacement for `docker compose` on rootless runners: the file it converts is
102+
the file the developer runs locally. So a document `docker compose` will not run
103+
must not pass green here — accepting a null it refuses would emit a script for a
104+
file that is already broken upstream, turning a hard error into a false green.
105+
Parity on *refusal* is what the drop-in role demands; it is not parity for its
106+
own sake, and the package keeps its documented divergences elsewhere. A null
107+
*inside* a value is a different thing and stays accepted, because Docker accepts
108+
it too: `environment: {KEY: null}` is host-passthrough, `labels: {KEY: null}` an
109+
empty label.
110+
98111
- **Supported:** `image`, `build`, `command`, `entrypoint`, `environment`,
99112
`env_file`, `volumes`, `healthcheck`, `depends_on`, `networks`, `hostname`,
100113
`container_name`, `tmpfs`, `secrets`, `configs`, plus the declarative
@@ -424,13 +437,15 @@ exception in this group, validated as an actual bool like
424437
which has no sub-second resolution, so `"500ms"` and `0` both poll once a
425438
second.
426439
- **`timeout`, `retries`, `start_period`:** each must be a number (int or
427-
float), a string, or `null` when present. A mapping or list raises
428-
rather than reaching its `--health-*` flag as a literal Python `repr()`.
429-
**An explicit `null` scalar means unset** — its `--health-*` flag is
430-
omitted entirely, keyed off the *value*, not key presence, so `timeout:
431-
null` and an omitted `timeout` behave identically. This matches `docker
432-
compose config`, and is the same treatment this package already gives a
433-
null `environment`/`volumes`/`command` value elsewhere.
440+
float) or a string. A mapping or list raises rather than reaching its
441+
`--health-*` flag as a literal Python `repr()`.
442+
- **A null raises in every healthcheck position**`test`, `interval`,
443+
`timeout`, `retries`, `start_period` — because `docker compose config`
444+
refuses each. A bare `test:` would silently drop the healthcheck entirely;
445+
a bare `timeout:` drops nothing on its own, but the document carrying it is
446+
one `docker compose` will not run, and compose2pod is a drop-in replacement
447+
for it — so passing CI green on such a file would be a false green. An
448+
*omitted* key is a different thing and stays fine: podman's default applies.
434449
- **Extension fields:** any `x-`-prefixed healthcheck key is accepted and
435450
ignored silently.
436451
- Everything else raises.

compose2pod/parsing.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,21 @@
2323
DEPENDS_ON_CONDITIONS = {"service_started", "service_healthy", "service_completed_successfully"}
2424

2525

26+
def _reject_null_healthcheck_values(name: str, healthcheck: dict[str, Any]) -> None:
27+
"""Refuse a null in any healthcheck position -- `docker compose config` refuses each.
28+
29+
A bare `test:` would silently drop the healthcheck entirely. A bare `timeout:`
30+
drops nothing by itself, but the document carrying it is one `docker compose`
31+
will not run, and compose2pod is a drop-in replacement for it -- so emitting a
32+
script for such a file would turn a hard error into a false green. An *omitted*
33+
key is a different thing and stays fine: podman's default applies.
34+
"""
35+
for key in ("test", "interval", *_HEALTHCHECK_SCALAR_KEYS):
36+
if key in healthcheck and healthcheck[key] is None:
37+
msg = f"service {name!r}: healthcheck {key!r} must not be null"
38+
raise UnsupportedComposeError(msg)
39+
40+
2641
def _validate_service_healthcheck(name: str, svc: dict[str, Any]) -> None:
2742
"""Check healthcheck is a mapping with supported keys and a parseable interval."""
2843
healthcheck = svc.get("healthcheck")
@@ -42,6 +57,7 @@ def _validate_service_healthcheck(name: str, svc: dict[str, Any]) -> None:
4257
if key not in SUPPORTED_HEALTHCHECK_KEYS:
4358
msg = f"service {name!r}: unsupported healthcheck key '{key}'"
4459
raise UnsupportedComposeError(msg)
60+
_reject_null_healthcheck_values(name, healthcheck)
4561
if "interval" in healthcheck:
4662
interval_seconds(healthcheck["interval"])
4763
if "test" in healthcheck:
@@ -50,7 +66,7 @@ def _validate_service_healthcheck(name: str, svc: dict[str, Any]) -> None:
5066
# again for the actual --health-cmd value at emit time).
5167
health_cmd(healthcheck["test"])
5268
for key in _HEALTHCHECK_SCALAR_KEYS:
53-
if key in healthcheck and healthcheck[key] is not None and not is_number(healthcheck[key]):
69+
if key in healthcheck and not is_number(healthcheck[key]):
5470
msg = f"service {name!r}: healthcheck {key!r} must be a number or string"
5571
raise UnsupportedComposeError(msg)
5672

@@ -400,6 +416,18 @@ def _validate_depends_on(services: dict[str, Any]) -> None:
400416
raise UnsupportedComposeError(msg)
401417

402418

419+
def _reject_null_top_level_blocks(compose: dict[str, Any]) -> None:
420+
"""Refuse a bare top-level block -- `docker compose config` refuses each.
421+
422+
`services` has its own message ("no services defined"); `version`/`name` are
423+
scalars, not blocks.
424+
"""
425+
for key in ("networks", "volumes", "secrets", "configs"):
426+
if key in compose and compose[key] is None:
427+
msg = f"top-level {key!r} must not be null"
428+
raise UnsupportedComposeError(msg)
429+
430+
403431
def validate(compose: dict[str, Any]) -> list[str]:
404432
"""Check the compose document against the supported subset.
405433
@@ -419,6 +447,7 @@ def validate(compose: dict[str, Any]) -> list[str]:
419447
if unknown_top:
420448
msg = f"unsupported top-level keys: {sorted(unknown_top)}"
421449
raise UnsupportedComposeError(msg)
450+
_reject_null_top_level_blocks(compose)
422451
if "networks" in compose:
423452
warnings.append("ignoring top-level 'networks' (all services share the pod namespace)")
424453
if "volumes" in compose:

compose2pod/resources.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ def _check_number(name: str, field: str, value: Any) -> None: # noqa: ANN401 -
1717

1818

1919
def _validate_limits(name: str, svc: dict[str, Any], limits: Any) -> None: # noqa: ANN401 - Compose values are untyped
20-
if limits is None:
21-
return
20+
# No `limits is None` escape: `_reject_null_block` refuses a null `limits:`
21+
# upstream, so a null reaching here is a wrong shape like any other.
2222
if not isinstance(limits, dict):
2323
msg = f"service {name!r}: deploy.resources.limits must be a mapping"
2424
raise UnsupportedComposeError(msg)
@@ -36,8 +36,7 @@ def _validate_limits(name: str, svc: dict[str, Any], limits: Any) -> None: # no
3636

3737

3838
def _validate_reservations(name: str, svc: dict[str, Any], reservations: Any) -> None: # noqa: ANN401 - Compose values are untyped
39-
if reservations is None:
40-
return
39+
# No `reservations is None` escape -- see `_validate_limits`.
4140
if not isinstance(reservations, dict):
4241
msg = f"service {name!r}: deploy.resources.reservations must be a mapping"
4342
raise UnsupportedComposeError(msg)
@@ -57,6 +56,19 @@ def _validate_reservations(name: str, svc: dict[str, Any], reservations: Any) ->
5756
raise UnsupportedComposeError(msg)
5857

5958

59+
def _reject_null_block(name: str, path: str, parent: dict[str, Any], key: str) -> None:
60+
"""Refuse a null where a block of content belongs.
61+
62+
`deploy: {resources: {limits: }}` is the limits' contents deleted, not a
63+
request for no limits: emit would silently drop `--memory`/`--cpus` and say
64+
nothing. A key that is simply *absent* is fine -- that is a document not
65+
asking for limits at all.
66+
"""
67+
if key in parent and parent[key] is None:
68+
msg = f"service {name!r}: '{path}' must not be null"
69+
raise UnsupportedComposeError(msg)
70+
71+
6072
def validate_deploy(name: str, svc: dict[str, Any]) -> None:
6173
"""Validate a service's deploy block: only deploy.resources, only mappable fields, no legacy conflicts."""
6274
deploy = svc.get("deploy")
@@ -70,12 +82,15 @@ def validate_deploy(name: str, svc: dict[str, Any]) -> None:
7082
if unknown:
7183
msg = f"service {name!r}: deploy: only 'resources' is supported (got {sorted(unknown)})"
7284
raise UnsupportedComposeError(msg)
85+
_reject_null_block(name, "deploy.resources", deploy, "resources")
7386
resources = deploy.get("resources")
7487
if resources is None:
7588
return
7689
if not isinstance(resources, dict):
7790
msg = f"service {name!r}: deploy.resources must be a mapping"
7891
raise UnsupportedComposeError(msg)
92+
_reject_null_block(name, "deploy.resources.limits", resources, "limits")
93+
_reject_null_block(name, "deploy.resources.reservations", resources, "reservations")
7994
require_string_keys(f"service {name!r}: deploy.resources", resources)
8095
unknown = set(resources) - {"limits", "reservations"}
8196
if unknown:
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
---
2+
summary: A null is refused in every nested and top-level position Docker refuses one (healthcheck sub-keys, deploy's resource blocks, the top-level networks/volumes/secrets/configs blocks), so a document docker compose would not run cannot pass CI green here.
3+
---
4+
5+
# Design: Refuse a null wherever Docker refuses one
6+
7+
## Summary
8+
9+
`2026-07-14.07` matched Docker's null policy for *service* keys. It stopped
10+
there. Inside `healthcheck` and `deploy`, and in the top-level blocks, a null is
11+
still accepted and silently emits nothing.
12+
13+
Carry the same rule down: **a null is refused wherever `docker compose config`
14+
refuses one.** Measured, position by position.
15+
16+
## Motivation
17+
18+
compose2pod exists to run a Compose file on a rootless CI runner where
19+
`docker compose` and `podman kube play` cannot. It is a **drop-in replacement**:
20+
the file it converts is the same file the developer runs locally with
21+
`docker compose up`.
22+
23+
That makes accepting-what-Docker-refuses actively harmful, not merely lax:
24+
25+
- **Docker refuses, compose2pod accepts** → the document is already broken in the
26+
developer's real workflow. Converting it anyway does not rescue anything; it
27+
means **CI goes green on a file `docker compose` would not run**. A false
28+
green is worse than a hard error.
29+
- **Docker accepts, compose2pod refuses** → a working file breaks. That is the
30+
direction that must never happen.
31+
32+
So the test is not "does this null drop behavior?" but "**would `docker compose`
33+
run this file at all?**" If it would not, there is no point emitting a podman
34+
script for it.
35+
36+
Eight positions failed that test — accepted here, refused by
37+
`docker compose config` (v5.1.2), each emitting nothing:
38+
39+
- `healthcheck.test`, `healthcheck.interval`, `healthcheck.timeout`,
40+
`healthcheck.retries`, `healthcheck.start_period`
41+
- `deploy.resources`, `deploy.resources.limits`, `deploy.resources.reservations`
42+
- top-level `networks:`, `volumes:`, `secrets:`, `configs:`
43+
44+
## Design
45+
46+
Each position rejects an explicit null, with a message naming the path. A key
47+
that is simply *absent* stays fine — that is a document not asking for the thing
48+
at all, which Docker also accepts.
49+
50+
**This reverses `2026-07-13.10`'s null-scalar ruling** (a null
51+
`healthcheck.timeout`/`retries`/`start_period` meant "unset", emitting no flag).
52+
That ruling was taken on the stated premise that it matched
53+
`docker compose config`. It does not: Docker refuses all three. The premise was
54+
wrong, so the conclusion goes with it. A null scalar drops nothing on its own,
55+
but the document carrying it is one Docker would not run — and shipping a green
56+
CI result for such a file is the failure this change exists to prevent.
57+
58+
## Non-goals
59+
60+
- Not touching a null *inside* a value (`environment: {KEY: null}` is Compose's
61+
host-passthrough; `labels: {KEY: null}` is an empty label). Docker accepts
62+
both, so compose2pod must too.
63+
- Not pursuing Docker parity as an end in itself. The project keeps its
64+
documented divergences where they buy something (it never builds; `profiles`
65+
is closure-authoritative). Parity on *refusal* is what the drop-in role
66+
demands, so a file that cannot run under `docker compose` cannot pass here.
67+
68+
## Testing
69+
70+
`just test-ci` at 100%. Each of the twelve positions gets a test — the eight
71+
newly refused, plus the absent-key cases that must stay accepted — so the rule is
72+
pinned position by position rather than implied.
73+
74+
## Risk
75+
76+
- **A document with a bare `timeout:` or `networks:` now raises** where it
77+
previously ran. `docker compose` already refuses it, so the file was never
78+
runnable in the workflow compose2pod serves; it was passing CI on a lie.

tests/test_parsing.py

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -371,20 +371,6 @@ def test_healthcheck_scalars_accept_ints_and_strings(self) -> None:
371371
}
372372
assert validate(compose) == []
373373

374-
def test_healthcheck_scalars_accept_explicit_null(self) -> None:
375-
# A null scalar is treated as unset (see emit._health_flags), same
376-
# ruling `environment`/`volumes`/`command` already get for a null
377-
# value -- it must not raise at the gate.
378-
compose = {
379-
"services": {
380-
"app": {
381-
"image": "x",
382-
"healthcheck": {"test": "true", "retries": None, "timeout": None, "start_period": None},
383-
}
384-
}
385-
}
386-
assert validate(compose) == []
387-
388374
def test_healthcheck_retries_mapping_rejected_at_gate(self) -> None:
389375
# Used to be silently accepted and mis-emitted as the literal
390376
# --health-retries "{'a': 1}".
@@ -960,3 +946,52 @@ def test_null_extension_field_is_accepted(self) -> None:
960946
def test_absent_key_is_not_a_null_key(self) -> None:
961947
# The check is about a key present with no value, not a key that is missing.
962948
assert validate({"services": {"app": {"image": "x"}}}) == []
949+
950+
951+
class TestNullContentBlocks:
952+
"""A null is refused wherever `docker compose config` refuses one.
953+
954+
compose2pod is a drop-in replacement for `docker compose` on rootless
955+
runners: the file it converts is the file the developer runs locally. A
956+
document Docker will not run must not pass CI green here, so accepting a
957+
null it refuses would ship a false green. Verdicts measured against
958+
`docker compose config` v5.1.2, position by position.
959+
"""
960+
961+
HEALTHCHECK_SCALARS = ("interval", "timeout", "retries", "start_period")
962+
TOP_LEVEL_BLOCKS = ("networks", "volumes", "secrets", "configs")
963+
964+
@pytest.mark.parametrize("key", HEALTHCHECK_SCALARS)
965+
def test_null_healthcheck_scalar_is_refused(self, key: str) -> None:
966+
# Reverses the earlier "null scalar means unset" ruling: that was taken on
967+
# the premise it matched Docker, and Docker refuses all four.
968+
healthcheck = {"test": ["CMD", "true"], key: None}
969+
with pytest.raises(UnsupportedComposeError, match=f"healthcheck '{key}' must not be null"):
970+
validate({"services": {"app": {"image": "x", "healthcheck": healthcheck}}})
971+
972+
@pytest.mark.parametrize("key", TOP_LEVEL_BLOCKS)
973+
def test_null_top_level_block_is_refused(self, key: str) -> None:
974+
with pytest.raises(UnsupportedComposeError, match=f"top-level '{key}' must not be null"):
975+
validate({"services": {"app": {"image": "x"}}, key: None})
976+
977+
def test_null_healthcheck_test_is_refused(self) -> None:
978+
# Used to emit no --health-cmd at all: the check silently evaporated.
979+
with pytest.raises(UnsupportedComposeError, match="healthcheck 'test' must not be null"):
980+
validate({"services": {"app": {"image": "x", "healthcheck": {"test": None, "interval": "1s"}}}})
981+
982+
def test_null_deploy_resources_is_refused(self) -> None:
983+
with pytest.raises(UnsupportedComposeError, match=r"'deploy\.resources' must not be null"):
984+
validate({"services": {"app": {"image": "x", "deploy": {"resources": None}}}})
985+
986+
def test_null_deploy_limits_is_refused(self) -> None:
987+
# Used to emit no --memory/--cpus: the limits silently did not exist.
988+
with pytest.raises(UnsupportedComposeError, match=r"'deploy\.resources\.limits' must not be null"):
989+
validate({"services": {"app": {"image": "x", "deploy": {"resources": {"limits": None}}}}})
990+
991+
def test_null_deploy_reservations_is_refused(self) -> None:
992+
with pytest.raises(UnsupportedComposeError, match=r"'deploy\.resources\.reservations' must not be null"):
993+
validate({"services": {"app": {"image": "x", "deploy": {"resources": {"reservations": None}}}}})
994+
995+
def test_absent_blocks_are_not_null_blocks(self) -> None:
996+
assert validate({"services": {"app": {"image": "x", "deploy": {"resources": {}}}}}) == []
997+
assert validate({"services": {"app": {"image": "x", "healthcheck": {"test": ["CMD", "true"]}}}}) == []

tests/test_resources.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,13 @@ def test_unsupported_resources_section_rejected(self) -> None:
3434
validate_deploy("app", _svc({"resources": {"foo": {}}}))
3535

3636
def test_resources_absent_is_noop(self) -> None:
37-
validate_deploy("app", _svc({"resources": None}))
37+
# Genuinely absent -- a document not asking for resources at all.
38+
validate_deploy("app", _svc({}))
39+
40+
def test_null_resources_is_refused(self) -> None:
41+
# `resources:` with its contents deleted. docker compose config refuses it.
42+
with pytest.raises(UnsupportedComposeError, match=r"'deploy.resources' must not be null"):
43+
validate_deploy("app", _svc({"resources": None}))
3844

3945
def test_resources_not_mapping_rejected(self) -> None:
4046
with pytest.raises(UnsupportedComposeError, match=r"deploy.resources must be a mapping"):
@@ -89,11 +95,14 @@ def test_reservation_memory_conflict_rejected(self) -> None:
8995
with pytest.raises(UnsupportedComposeError, match=match):
9096
validate_deploy("app", _svc(deploy, mem_reservation="256m"))
9197

92-
def test_null_limits_is_noop(self) -> None:
93-
validate_deploy("app", _svc({"resources": {"limits": None}}))
98+
def test_null_limits_is_refused(self) -> None:
99+
# Used to emit no --memory/--cpus at all: the limits silently did not exist.
100+
with pytest.raises(UnsupportedComposeError, match=r"'deploy.resources.limits' must not be null"):
101+
validate_deploy("app", _svc({"resources": {"limits": None}}))
94102

95-
def test_null_reservations_is_noop(self) -> None:
96-
validate_deploy("app", _svc({"resources": {"reservations": None}}))
103+
def test_null_reservations_is_refused(self) -> None:
104+
with pytest.raises(UnsupportedComposeError, match=r"'deploy.resources.reservations' must not be null"):
105+
validate_deploy("app", _svc({"resources": {"reservations": None}}))
97106

98107
def test_deploy_mixed_type_unknown_keys_do_not_crash_raw(self) -> None:
99108
# sorted(unknown) used to crash raw (TypeError: '<' not supported

0 commit comments

Comments
 (0)