From 3be9af6667d0135c76739df5546f81cbf9c7a819 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 10 Jul 2026 09:59:13 +0300 Subject: [PATCH 1/5] docs: design validate() owning every shape emit() reads --- ...2026-07-10.01-validate-owns-emit-shapes.md | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 planning/changes/2026-07-10.01-validate-owns-emit-shapes.md diff --git a/planning/changes/2026-07-10.01-validate-owns-emit-shapes.md b/planning/changes/2026-07-10.01-validate-owns-emit-shapes.md new file mode 100644 index 0000000..378fd71 --- /dev/null +++ b/planning/changes/2026-07-10.01-validate-owns-emit-shapes.md @@ -0,0 +1,83 @@ +--- +summary: Make validate() own every shape emit() reads — malformed depends_on/healthcheck/interval/hostname/tmpfs/networks are refused with UnsupportedComposeError at the gate instead of crashing raw or mis-emitting silently. +--- + +# Design: validate() owns every shape emit() reads + +## Summary + +Close the validate/emit seam. Today several structural shapes that `emit` +(and `graph`/`healthcheck`) read are never checked by `validate()`: a malformed +`depends_on`, non-mapping `healthcheck`, or unparseable healthcheck `interval` +crashes with a raw `AttributeError`/`ValueError` (the CLI's +`except UnsupportedComposeError` doesn't catch it), and a non-string +`hostname`/`container_name`, a `tmpfs` mapping, or a malformed per-service +`networks` is silently mis-emitted. This change makes the shape-reading +functions own their contract and has `validate()` exercise them, so every +malformed document is refused loudly at the gate. + +## Motivation + +The tool's whole contract (`architecture/supported-subset.md`) is "refuse the +rest loudly rather than silently drop behavior." These holes break that: a typo +like `depends_on: "db"` yields an uncaught traceback, and `tmpfs: {a: b}` emits +a nonsense `--tmpfs a` flag with no error. The declarative-key drift was fixed +by the service-key registry; this closes the remaining structural gaps. + +## Design + +**Robust readers — each raises `UnsupportedComposeError` on malformed input, +owning its shape/grammar in the one function that reads it:** + +- `graph.depends_on` — reject a non-list/non-mapping `depends_on`, and a mapping + whose entry value is not itself a mapping (`{db: "healthy"}`, `{db: null}`). +- `graph.hostnames` — reject non-string `hostname`/`container_name` and a + per-service `networks` that is not a list or mapping. Long-form network + *values* that are not mappings still contribute no aliases (lenient — matches + Compose's valid `{default: null}`). +- `healthcheck.interval_seconds` — raise `UnsupportedComposeError` (not + `ValueError`) on an unparseable `interval`, naming the supported forms + (`30s`, `2m`, `500ms`, bare seconds). No grammar expansion (see Non-goals). + +Because these are robust, a direct `emit_script()` call (both `validate` and +`emit_script` are public) also fails cleanly, not just the CLI path. + +**Gate checks for the two trivial emit-only shapes (`parsing.py`):** + +- Harden `_validate_service_healthcheck` to reject a non-mapping `healthcheck` + before it iterates. +- Add a `tmpfs` form check (string or list). + +**Wiring — `validate()` exercises the readers so errors surface at the gate:** +it already drives `depends_on` (via `_validate_depends_on`); add a +`hostnames(services)` call (covers hostname/container_name/networks) and a +per-service `interval_seconds(interval)` call inside the healthcheck validator. +Both readers are pure, so calling them for their checking effect is a clean +dry-run. + +## Non-goals + +- Expanding the healthcheck duration grammar (`1m30s`, `1h`, `us`/`ns`) — a + separate feature; this change refuses them loudly, it does not accept them. +- A typed validated-document model (review Candidate 3) — out of scope. +- Re-hardening emit's inline `tmpfs` reader for the direct-`emit_script` bypass; + the trivial `tmpfs`/healthcheck-mapping shapes are gate-checked only (no + grammar to own). + +## Testing + +`just test-ci` at 100%. Per hole: a malformed input now raising a specific +`UnsupportedComposeError` at `validate()` (`depends_on: "db"`, +`depends_on: {db: "x"}`, `healthcheck: []`, `interval: "1h30m"`, +`hostname: 5`, `tmpfs: {a: b}`, `networks: "x"`), plus the valid forms still +accepted (list/mapping `depends_on`, `networks: [n1]` short form, +`networks: {default: null}`). `just lint-ci` clean (watch `C901` on the readers +and on `_validate_service` — extract helpers if a check pushes a function over). + +## Risk + +- **False rejection of a valid Compose shape** (med x med): the shape contracts + must admit every valid form — mitigated by the "valid forms still accepted" + tests above (esp. the `networks` short list-form and null-value long-form). +- **`C901` on the readers** (low x low): `hostnames`/`depends_on` gain branches; + extract a small helper if either crosses the limit (as prior bundles did). From b2f796259d3ca08647f00916d9d3c7a32903a36a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 10 Jul 2026 10:33:43 +0300 Subject: [PATCH 2/5] feat: make graph and healthcheck readers reject malformed shapes --- compose2pod/graph.py | 46 +++++++++++++++++++++++++++----------- compose2pod/healthcheck.py | 15 ++++++++----- tests/test_graph.py | 24 ++++++++++++++++++++ tests/test_healthcheck.py | 5 +++++ 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/compose2pod/graph.py b/compose2pod/graph.py index b1945b5..7c7fed1 100644 --- a/compose2pod/graph.py +++ b/compose2pod/graph.py @@ -10,24 +10,44 @@ def depends_on(svc: dict[str, Any]) -> dict[str, str]: deps = svc.get("depends_on") or {} if isinstance(deps, list): return cast(dict[str, str], dict.fromkeys(deps, "service_started")) - return {name: spec.get("condition", "service_started") for name, spec in deps.items()} + if not isinstance(deps, dict): + msg = "'depends_on' must be a list or mapping" + raise UnsupportedComposeError(msg) + result: dict[str, str] = {} + for dep, spec in deps.items(): + if not isinstance(spec, dict): + msg = f"depends_on entry {dep!r} must be a mapping" + raise UnsupportedComposeError(msg) + result[dep] = spec.get("condition", "service_started") + return result + + +def _host_names(name: str, svc: dict[str, Any]) -> list[str]: + """Names one service is reachable by: hostname, container_name, and network aliases.""" + result: list[str] = [] + for key in ("hostname", "container_name"): + value = svc.get(key) + if value is not None and not isinstance(value, str): + msg = f"service {name!r}: {key} must be a string" + raise UnsupportedComposeError(msg) + if value: + result.append(value) + networks = svc.get("networks") + if networks is not None and not isinstance(networks, list | dict): + msg = f"service {name!r}: networks must be a list or mapping" + raise UnsupportedComposeError(msg) + if isinstance(networks, dict): + for network in networks.values(): + if isinstance(network, dict): + result.extend(network.get("aliases") or []) + return result def hostnames(services: dict[str, Any]) -> list[str]: """All names other services may use to reach a service: names, hostnames/container names, then aliases.""" names = list(services) - for svc in services.values(): - hostname = svc.get("hostname") - if hostname: - names.append(hostname) - container_name = svc.get("container_name") - if container_name: - names.append(container_name) - networks = svc.get("networks") - if isinstance(networks, dict): - for network in networks.values(): - if isinstance(network, dict): - names.extend(network.get("aliases") or []) + for name, svc in services.items(): + names.extend(_host_names(name, svc)) return names diff --git a/compose2pod/healthcheck.py b/compose2pod/healthcheck.py index e6b026d..f39cf16 100644 --- a/compose2pod/healthcheck.py +++ b/compose2pod/healthcheck.py @@ -46,9 +46,12 @@ def interval_seconds(duration: object) -> int: if isinstance(duration, (int, float)): return max(int(duration), 1) text = str(duration).strip() - if text.endswith("ms"): - return max(int(float(text[:-2]) / 1000), 1) - if text.endswith("m"): - return max(int(float(text[:-1])) * 60, 1) - text = text.removesuffix("s") - return max(int(float(text)), 1) + try: + if text.endswith("ms"): + return max(int(float(text[:-2]) / 1000), 1) + if text.endswith("m"): + return max(int(float(text[:-1])) * 60, 1) + return max(int(float(text.removesuffix("s"))), 1) + except ValueError: + msg = f"unsupported healthcheck interval {duration!r} (use forms like '30s', '2m', '500ms')" + raise UnsupportedComposeError(msg) from None diff --git a/tests/test_graph.py b/tests/test_graph.py index 339d843..e823241 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -14,6 +14,14 @@ def test_map_form_keeps_conditions(self) -> None: def test_missing_depends_on_is_empty(self) -> None: assert depends_on({"image": "x"}) == {} + def test_non_list_or_mapping_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match="'depends_on' must be a list or mapping"): + depends_on({"depends_on": "db"}) + + def test_mapping_entry_not_a_mapping_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match="depends_on entry 'db' must be a mapping"): + depends_on({"depends_on": {"db": "service_healthy"}}) + class TestHostnames: def test_collects_service_names_and_aliases(self, chats_compose: dict) -> None: @@ -41,6 +49,22 @@ def test_empty_hostname_is_skipped(self) -> None: services = {"a": {"image": "x", "hostname": ""}} assert hostnames(services) == ["a"] + def test_non_string_hostname_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match="'app': hostname must be a string"): + hostnames({"app": {"image": "x", "hostname": 5}}) + + def test_non_string_container_name_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match="'app': container_name must be a string"): + hostnames({"app": {"image": "x", "container_name": ["c"]}}) + + def test_networks_not_list_or_mapping_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match="'app': networks must be a list or mapping"): + hostnames({"app": {"image": "x", "networks": "default"}}) + + def test_networks_list_short_form_and_null_value_accepted(self) -> None: + assert hostnames({"app": {"image": "x", "networks": ["n1"]}}) == ["app"] + assert hostnames({"app": {"image": "x", "networks": {"default": None}}}) == ["app"] + class TestStartupOrder: def test_chats_order(self, chats_compose: dict) -> None: diff --git a/tests/test_healthcheck.py b/tests/test_healthcheck.py index d405d6f..e8635a2 100644 --- a/tests/test_healthcheck.py +++ b/tests/test_healthcheck.py @@ -65,6 +65,11 @@ def test_int_value_passes_through(self) -> None: def test_float_value_below_one_floors_to_one(self) -> None: assert interval_seconds(0.4) == 1 + def test_unparseable_interval_raises(self) -> None: + for bad in ("1h30m", "abc", "5x"): + with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"): + interval_seconds(bad) + class TestHasHealthcheck: def test_true_when_test_present(self) -> None: From 2aa2dc4282ee6b22cb4de39a68f6dfae95c61b06 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 10 Jul 2026 10:38:20 +0300 Subject: [PATCH 3/5] feat: validate healthcheck, tmpfs, hostname and networks shapes at the gate --- compose2pod/parsing.py | 26 ++++++++++++++++++++++---- tests/test_parsing.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index f7a5c46..4f6de19 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -3,8 +3,8 @@ from typing import Any from compose2pod.exceptions import UnsupportedComposeError -from compose2pod.graph import depends_on -from compose2pod.healthcheck import has_healthcheck +from compose2pod.graph import depends_on, hostnames +from compose2pod.healthcheck import has_healthcheck, interval_seconds from compose2pod.keys import SERVICE_KEYS, STRUCTURAL_KEYS @@ -16,13 +16,21 @@ def _validate_service_healthcheck(name: str, svc: dict[str, Any]) -> None: - """Check healthcheck keys against the supported subset, skipping 'x-' extension keys.""" - for key in sorted(svc.get("healthcheck") or {}): + """Check healthcheck is a mapping with supported keys and a parseable interval.""" + healthcheck = svc.get("healthcheck") + if healthcheck is None: + return + if not isinstance(healthcheck, dict): + msg = f"service {name!r}: healthcheck must be a mapping" + raise UnsupportedComposeError(msg) + for key in sorted(healthcheck): if key.startswith("x-"): continue if key not in SUPPORTED_HEALTHCHECK_KEYS: msg = f"service {name!r}: unsupported healthcheck key '{key}'" raise UnsupportedComposeError(msg) + if "interval" in healthcheck: + interval_seconds(healthcheck["interval"]) def _validate_service_volumes(name: str, svc: dict[str, Any]) -> None: @@ -49,6 +57,14 @@ def _validate_entrypoint(name: str, svc: dict[str, Any]) -> None: raise UnsupportedComposeError(msg) +def _validate_tmpfs(name: str, svc: dict[str, Any]) -> None: + """Check tmpfs is a string or list.""" + tmpfs = svc.get("tmpfs") + if tmpfs is not None and not isinstance(tmpfs, str | list): + msg = f"service {name!r}: tmpfs must be a string or list" + raise UnsupportedComposeError(msg) + + def _validate_service(name: str, svc: dict[str, Any]) -> list[str]: """Validate one service; returns warnings, raises UnsupportedComposeError.""" warnings: list[str] = [] @@ -65,6 +81,7 @@ def _validate_service(name: str, svc: dict[str, Any]) -> list[str]: _validate_service_healthcheck(name, svc) _validate_service_volumes(name, svc) _validate_entrypoint(name, svc) + _validate_tmpfs(name, svc) for key, spec in SERVICE_KEYS.items(): if key in svc: spec.validate(name, key, svc[key]) @@ -107,5 +124,6 @@ def validate(compose: dict[str, Any]) -> list[str]: raise UnsupportedComposeError(msg) for name, svc in services.items(): warnings.extend(_validate_service(name, svc)) + hostnames(services) # validate hostname/container_name/networks shapes at the gate _validate_depends_on(services) return warnings diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 8832b15..cc411de 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -255,3 +255,32 @@ def test_pull_policy_null_is_accepted(self) -> 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"]}}}) + + def test_unparseable_healthcheck_interval_raises(self) -> None: + compose = {"services": {"app": {"image": "x", "healthcheck": {"test": "true", "interval": "1h30m"}}}} + with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"): + validate(compose) + + def test_tmpfs_non_string_or_list_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match="tmpfs must be a string or list"): + validate({"services": {"app": {"image": "x", "tmpfs": {"a": "b"}}}}) + + def test_non_string_hostname_raises_at_gate(self) -> None: + with pytest.raises(UnsupportedComposeError, match="hostname must be a string"): + validate({"services": {"app": {"image": "x", "hostname": 5}}}) + + def test_networks_not_list_or_mapping_raises_at_gate(self) -> None: + with pytest.raises(UnsupportedComposeError, match="networks must be a list or mapping"): + validate({"services": {"app": {"image": "x", "networks": "default"}}}) + + def test_malformed_depends_on_raises_at_gate(self) -> None: + with pytest.raises(UnsupportedComposeError, match="'depends_on' must be a list or mapping"): + validate({"services": {"app": {"image": "x", "depends_on": "db"}}}) + + def test_valid_networks_forms_accepted(self) -> None: + assert validate({"services": {"app": {"image": "x", "networks": ["n1"]}}}) == [] + assert validate({"services": {"app": {"image": "x", "networks": {"default": None}}}}) == [] From fa24a0fe06f8a497bc675af7d4b2b877b1bec55e Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 10 Jul 2026 10:43:32 +0300 Subject: [PATCH 4/5] docs: record structural-shape validation in the supported subset --- architecture/supported-subset.md | 35 +++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 03c90b3..d8bf80d 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -88,8 +88,10 @@ warns (ignored, behavior-neutral inside a single pod) or raises `:` (e.g. `/tmp:mode=1777`), passed through verbatim as `podman run --tmpfs ` — Compose's short syntax maps directly onto podman's own `--tmpfs CONTAINER-DIR[:OPTIONS]` flag, so no translation is - needed. No format validation; a malformed option string surfaces as a - podman error at run time. + needed. The key itself must be a string or list — a non-string/non-list + value (e.g. a mapping) raises; no format validation beyond that, so a + malformed option string inside an accepted string/list surfaces as a podman + error at run time. - **`hostname` and `container_name`:** both are made resolvable to `127.0.0.1` like a network alias (added to the shared `--add-host` set), so other services can reach the service by either name. The pod shares the UTS @@ -97,7 +99,17 @@ warns (ignored, behavior-neutral inside a single pod) or raises container is always named `{pod}-{service}` regardless of `container_name` (used internally for `podman cp`, healthcheck polling, and target-container diagnostics) — only name *resolution* is meaningful to other services, and - no per-container `--hostname` or renamed `--name` is emitted. + no per-container `--hostname` or renamed `--name` is emitted. Each must be a + string when present; a non-string raises (`_host_names`, + `compose2pod/graph.py`). +- **Per-service `networks`:** long (mapping) form contributes each entry's + `aliases` to the same resolvable-name set as `hostname`/`container_name`; + short (list) form carries no aliases (a bare network name has none to + contribute). The key must be a list or mapping — anything else raises. A + long-form *value* that isn't itself a mapping (e.g. `networks: {default: + true}`) is lenient, not rejected: it simply contributes no aliases, since + only a mapping value can carry an `aliases` list (`_host_names`, + `compose2pod/graph.py`). - **Ignored (warns):** `ports`, `restart`, `stdin_open`, `tty`, `stop_signal`, `stop_grace_period` — meaningless or irrelevant inside a single shared-namespace pod. `stop_signal`/`stop_grace_period` are inert because the @@ -110,6 +122,16 @@ warns (ignored, behavior-neutral inside a single pod) or raises ## Healthcheck keys - **Supported:** `test`, `interval`, `timeout`, `retries`, `start_period`. +- A `healthcheck` value that isn't a mapping raises + (`_validate_service_healthcheck`, `compose2pod/parsing.py`) — previously a + non-mapping healthcheck reached `.get()` calls downstream and crashed raw + instead of failing at the gate. +- **`interval`:** parsed to whole seconds by `interval_seconds` + (`compose2pod/healthcheck.py`). Supported forms: a bare number of seconds + (`30`, `"30"`, `"30s"`), minutes (`"2m"`), and milliseconds (`"500ms"`). + Compound durations (`"1h30m"`) and hour suffixes (`"1h"`) are not parsed — + each is rejected with an `UnsupportedComposeError` rather than silently + truncated or misinterpreted. - **Extension fields:** any `x-`-prefixed healthcheck key is accepted and ignored silently. - Everything else raises. @@ -199,6 +221,13 @@ All three conditions are honored: `service_started`, `service_healthy`, `service_completed_successfully`. A `service_healthy` dependency on a service with no usable healthcheck raises. +`depends_on` (`compose2pod/graph.py`) itself must be either a list of service +names (short form, each defaulting to `service_started`) or a mapping of +service name to a per-dependency mapping (long form, read for `condition`). +Anything else — a bare string, a number, a mapping whose value isn't itself a +mapping — raises `UnsupportedComposeError` at the gate instead of failing +later with a raw `AttributeError`/`TypeError` when the shape is walked. + ## YAML anchors and merge keys Anchors (`&name` / `*name`) and the merge key (`<<:`) need no handling in From ec68bb11d9bb4446ca848144bc3c4b4981509592 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Fri, 10 Jul 2026 11:03:24 +0300 Subject: [PATCH 5/5] fix: refuse non-finite and overflow healthcheck intervals instead of crashing --- compose2pod/healthcheck.py | 6 ++++-- tests/test_healthcheck.py | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/compose2pod/healthcheck.py b/compose2pod/healthcheck.py index f39cf16..5456e86 100644 --- a/compose2pod/healthcheck.py +++ b/compose2pod/healthcheck.py @@ -1,6 +1,7 @@ """Healthcheck translation: compose healthcheck -> podman --health-* values.""" import json +import math from typing import Any from compose2pod.exceptions import UnsupportedComposeError @@ -43,8 +44,9 @@ def interval_seconds(duration: object) -> int: """Compose duration ('1s', '2m', '500ms', int) to whole seconds, minimum 1.""" if duration is None: return 1 - if isinstance(duration, (int, float)): + if isinstance(duration, (int, float)) and math.isfinite(duration): return max(int(duration), 1) + # Non-finite floats (inf/nan) fall through to the guarded parse below and are refused. text = str(duration).strip() try: if text.endswith("ms"): @@ -52,6 +54,6 @@ def interval_seconds(duration: object) -> int: if text.endswith("m"): return max(int(float(text[:-1])) * 60, 1) return max(int(float(text.removesuffix("s"))), 1) - except ValueError: + except (ValueError, OverflowError): msg = f"unsupported healthcheck interval {duration!r} (use forms like '30s', '2m', '500ms')" raise UnsupportedComposeError(msg) from None diff --git a/tests/test_healthcheck.py b/tests/test_healthcheck.py index e8635a2..e3e9f6a 100644 --- a/tests/test_healthcheck.py +++ b/tests/test_healthcheck.py @@ -70,6 +70,12 @@ def test_unparseable_interval_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"): interval_seconds(bad) + def test_non_finite_interval_raises(self) -> None: + # inf/nan are valid YAML floats; they must be refused, not crash raw. + for bad in (float("inf"), float("nan"), "1e400"): + with pytest.raises(UnsupportedComposeError, match="unsupported healthcheck interval"): + interval_seconds(bad) + class TestHasHealthcheck: def test_true_when_test_present(self) -> None: