diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 179fe43..af8965b 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -161,13 +161,40 @@ def _classify_volume(volume: str) -> tuple[str, str | None]: # even when that letter is declared top-level, a declaration Docker ignores. # Two letters (`CC:\data:/var`) is an ordinary named-volume reference instead. # -# podman refuses every mount either reading makes (measured, podman 4.9.3): a -# colon inside a source has nowhere to go in a `-v` spec, which splits into at -# most source:target:options (`invalid option type "/var"`), and a container -# path that is not absolute is refused outright (`invalid container path`). -# So the whole family is a rule-two refusal, and a one-character volume name -# is reachable only through the long form, where Docker honours `source: v`. +# A second colon flips Docker's reading (measured, v5.1.2): `C:\data:/var` and +# `v:/data:ro` are binds whose source keeps the drive colon, while `C:\data`, +# `v:/data` and `v:` are anonymous volumes whose target is the whole string. +# Two letters (`CC:\data:/var`) is an ordinary named-volume reference instead. +# +# The two readings fail for different reasons, so they carry different messages +# (docs/adr/0006-docker-rejection-parity.md). podman 4.9.3 refuses the +# anonymous reading outright -- a container path that is not absolute is an +# `invalid container path`, and no spelling names one. The bind reading it does +# mount, via `--mount type=bind`; only the short `-v` spec cannot spell it, +# since that splits into at most source:target:options. Both escape through the +# long form, which emits `--mount` and where Docker honours `source: v`. _DRIVE_SHAPED_SOURCE = re.compile(r"^[a-zA-Z]:") +_DRIVE_SHAPED_BIND_COLONS = 2 + + +def _reject_drive_shaped_volume(name: str, volume: str) -> None: + """Refuse a short-syntax entry whose leading single letter Docker reads as a drive marker.""" + preamble = ( + f"service {name!r}: volume {volume!r}: a leading single letter is a Windows drive marker " + "to Docker, not a volume name, so this entry is " + ) + if volume.count(":") >= _DRIVE_SHAPED_BIND_COLONS: + msg = ( + f"{preamble}a bind whose source keeps the colon, which the short form cannot emit " + "(use the long form, which mounts it with --mount)" + ) + raise UnsupportedComposeError(msg) + msg = ( + f"{preamble}an anonymous volume whose target is the whole string, and " + "podman refuses a container path that is not absolute " + "(name a one-character volume through the long form instead)" + ) + raise UnsupportedComposeError(msg) _VOLUME_LONG_TYPES = ("bind", "volume", "tmpfs", "image") @@ -207,12 +234,7 @@ def _validate_service_volumes(name: str, svc: dict[str, Any]) -> None: # Refused before classification, so a single leading letter is # never read as a one-character volume name the way Docker never # reads it either. - msg = ( - f"service {name!r}: volume {volume!r}: a leading single letter is a Windows drive path " - "to Docker, not a volume name, and podman cannot express the mount it makes " - "(name a one-character volume through the long form instead)" - ) - raise UnsupportedComposeError(msg) + _reject_drive_shaped_volume(name, volume) kind, _ = _classify_volume(volume) if kind == "anonymous" and not volume.startswith("/"): msg = f"service {name!r}: anonymous volume '{volume}' must be an absolute path" diff --git a/docs/adr/0006-docker-rejection-parity.md b/docs/adr/0006-docker-rejection-parity.md index dbbc24b..64a5e35 100644 --- a/docs/adr/0006-docker-rejection-parity.md +++ b/docs/adr/0006-docker-rejection-parity.md @@ -5,15 +5,28 @@ Two rules, one direction each. A document `docker compose config` rejects, compo rootless runners and accepting a file Docker refuses turns a hard error into a green CI run. A document Docker accepts, compose2pod accepts whenever podman can express it inside a pod. Where podman cannot, that is a legitimate refusal (`network_mode`; `sysctls: ["a"]` with no value; -`volumes: ["a"]`, which podman rejects as a relative mount target; a short-form volume entry whose -source is a single letter, which Docker reads as a Windows drive path (`C:\data:/var`, and `v:/data` -too) and podman cannot mount either way, leaving the long form as the way to name a one-character -volume), and where compose2pod merely -does not parse a form yet, that is a tracked limitation, never a design position. Docker's -verdict binds only on the document, not the host: `env_file` existence, `${VAR:?}`, and a +`volumes: ["a"]`, which podman rejects as a relative mount target; a short-form volume entry Docker +reads as an anonymous volume whose target is not absolute -- a leading single letter is a drive +marker to Docker, never a volume name, so `C:\data`, `v:/data` and `v:` all ask for a container path +podman refuses outright, leaving the long form as the way to name a one-character volume), and where +compose2pod merely does not parse a form yet, that is a tracked limitation, never a design position. +The two are held apart in the refusal messages as well as here: a drive-shaped entry Docker reads as +a *bind* is refused for the short form's own limit, not for podman's, and says so. +Docker's verdict binds only on the document, not the host: `env_file` existence, `${VAR:?}`, and a negative on a top-level numeric key are facts about the machine that runs the script and are deferred to it. `tests/conformance/` runs both oracles for real over a probe matrix generated from `SERVICE_KEYS | STRUCTURAL_KEYS | IGNORED_SERVICE_KEYS`, so a new key is probed the moment -it is added. One residual is open by design: `depends_on` errors among services outside the +it is added, and `tests/integration/refusals.py` measures the other side, pairing each volume-family +refusal above with the `--mount` that expresses what Docker says the document means, so a claim that +podman cannot express something cannot go stale unnoticed either. It carries both verdicts: +`REFUSALS` for the mounts podman will not make, `LIMITATIONS` for the ones it would, which is what +keeps a limitation from quietly reading as rule two. The rest of the refusals get rows, and a gate +that every site has one, under [#109](https://github.com/modern-python/compose2pod/issues/109). +Verdicts are per version: the rulings here are measured against `docker compose config` v5.1.2 and +podman 4.9.3. Two residuals are open by design: `depends_on` errors among services outside the target's closure are accepted here and rejected by Docker -([#87](https://github.com/modern-python/compose2pod/issues/87)). +([#87](https://github.com/modern-python/compose2pod/issues/87)), and the drive-qualified *bind* +(`C:\data:/var`) is a limitation rather than rule two, since podman mounts that source through +`--mount` and only the short `-v` spec cannot spell it -- the long form already emits `--mount`, so +the capability is reachable today and only the short spelling is missing +([#111](https://github.com/modern-python/compose2pod/issues/111)). diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index dbec375..55dad79 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -23,6 +23,26 @@ _SH = shutil.which("sh") _INTEGRATION_DIR = Path(__file__).parent +# Every rule-two refusal probed this run, as `: exit= `. +# Stashed on `Config` (pytest's documented cross-hook slot) rather than a module global +# so it is unambiguously one collector per pytest run, not one per import -- the same +# reason tests/conformance/conftest.py stashes its over-rejection list. +_PODMAN_VERDICTS: pytest.StashKey[list[str]] = pytest.StashKey() + +# A container that starts and exits immediately, so a probe's verdict is its mount's, +# not its workload's. +_PROBE_IMAGE = "busybox:1.36" + + +def _podman_version() -> str: + if _PODMAN is None: + return "podman not installed" + + proc = subprocess.run( # noqa: S603 - _PODMAN is an absolute path from shutil.which + [_PODMAN, "--version"], capture_output=True, text=True, check=False + ) + return proc.stdout.strip() or "podman version unknown" + @pytest.hookimpl(tryfirst=True) def pytest_collection_modifyitems(items: "list[pytest.Item]") -> None: @@ -32,6 +52,28 @@ def pytest_collection_modifyitems(items: "list[pytest.Item]") -> None: item.add_marker(pytest.mark.integration) +def pytest_configure(config: pytest.Config) -> None: + """Create this run's podman-verdict collector before any probe executes.""" + config.stash[_PODMAN_VERDICTS] = [] + + +def pytest_terminal_summary(terminalreporter: pytest.TerminalReporter) -> None: + """Print what podman actually said about every rule-two refusal probed this run. + + The probes assert the exit code only: pinning podman's wording would make the + suite churn on a rewording of someone else's error string. Printing it is how + a drift in *why* podman refuses stays visible without CI going red on + cosmetics. Silent when nothing was collected, which is the normal case for + `just test-ci` (integration is deselected there, so no probe ever runs). + """ + verdicts = terminalreporter.config.stash.get(_PODMAN_VERDICTS, []) + if not verdicts: + return + terminalreporter.section(f"rule two: podman's verdicts ({_podman_version()})") + for label in verdicts: + terminalreporter.write_line(label) + + @dataclass(frozen=True) class PodRun: """Result of running one generated script to completion.""" @@ -94,3 +136,26 @@ def _run( for pod in created: assert _PODMAN is not None # narrows for the type checker; _require_podman already skipped otherwise subprocess.run([_PODMAN, "pod", "rm", "-f", pod], capture_output=True, check=False) # noqa: S603 + + +@pytest.fixture +def probe_podman(request: pytest.FixtureRequest) -> Callable[[str, list[str]], int]: + """Run `podman run --rm busybox true`; return its exit code, record its message. + + The message goes to `pytest_terminal_summary`, never to an assertion. + """ + + def _probe(label: str, argv: list[str]) -> int: + assert _PODMAN is not None # narrows for the type checker; _require_podman already skipped otherwise + proc = subprocess.run( # noqa: S603 - _PODMAN is an absolute path from shutil.which + [_PODMAN, "run", "--rm", *argv, _PROBE_IMAGE, "true"], + capture_output=True, + text=True, + check=False, + timeout=180, + ) + message = " ".join((proc.stderr or proc.stdout).split()) + request.config.stash[_PODMAN_VERDICTS].append(f"{label}: exit={proc.returncode} {message}"[:300]) + return proc.returncode + + return _probe diff --git a/tests/integration/refusals.py b/tests/integration/refusals.py new file mode 100644 index 0000000..cc12960 --- /dev/null +++ b/tests/integration/refusals.py @@ -0,0 +1,117 @@ +r"""Every documented refusal, paired with the podman invocation that would express it. + +ADR-0006 draws a line: compose2pod accepts whatever Docker accepts *whenever podman +can express it*, and refuses where podman cannot. Nothing ran podman to check which +side of that line a refusal sat on, which is how issue #86's unmeasured "podman can +express it" became #104's shipped acceptance of a script that dies at `podman run`. +These tables turn each claim back into a measurement, in both directions: + +- `REFUSALS` -- podman will not make this mount, so refusing is rule two. +- `LIMITATIONS` -- podman *will* make it and we refuse anyway, so the refusal is the + short form's own limit. ADR-0006 calls that "a tracked limitation, never a design + position", and a row here is what keeps it tracked. + +`podman_argv` is the faithful spelling of what `docker compose config` says the +document means -- a `type`/`source`/`target` triple, rendered as the `--mount` that +maps onto it one-to-one. It is deliberately not whatever `emit._volume_flags` would +produce: a short `-v` spec re-splits on colons and so can fail for its own grammar +rather than for podman's inability to mount, which would make a row pass for the +wrong reason and hide an emit bug behind a parity claim. + +Only the volume family is covered so far; the rest of the refusals ADR-0006 names +are issue #109 phase 2, and the gate that every refusal site has a row is phase 3. +""" + +from dataclasses import dataclass +from typing import Any + + +@dataclass(frozen=True) +class Refusal: + """A refusal podman agrees with: it will not make the mount the document asks for.""" + + id: str + compose: dict[str, Any] + refusal_match: str + podman_argv: list[str] + control_argv: list[str] + + +@dataclass(frozen=True) +class Limitation: + """A refusal podman does not agree with: it makes the mount, and compose2pod cannot spell it. + + `host_dir` is created under the test's `tmp_path` first, because a bind whose + source does not exist fails for that reason instead of the one being measured. + `{host}` in `podman_argv` is substituted with its absolute path. + """ + + id: str + compose: dict[str, Any] + refusal_match: str + host_dir: str + podman_argv: list[str] + + +def _one_volume(entry: str) -> dict[str, Any]: + return {"services": {"app": {"image": "busybox:1.36", "volumes": [entry]}}} + + +# An anonymous volume podman *can* mount, run through the same `--mount` grammar +# every row below uses. A row going red because busybox is unpullable or because +# `type=volume` with no source is spelled wrong would take this control with it, +# so "podman refused the mount" is never confused with "nothing ran". +_ANONYMOUS_CONTROL = ["--mount", "type=volume,dst=/data"] + +_NOT_ABSOLUTE = "podman refuses a container path that is not absolute" +_SHORT_FORM_CANNOT_EMIT = "which the short form cannot emit" + + +REFUSALS: list[Refusal] = [ + Refusal( + id="anonymous-volume-relative-target", + compose=_one_volume("a"), + refusal_match="anonymous volume 'a' must be an absolute path", + podman_argv=["--mount", "type=volume,dst=a"], + control_argv=_ANONYMOUS_CONTROL, + ), + Refusal( + id="drive-shaped-source-no-target", + compose=_one_volume("C:\\data"), + refusal_match=_NOT_ABSOLUTE, + podman_argv=["--mount", "type=volume,dst=C:\\data"], + control_argv=_ANONYMOUS_CONTROL, + ), + Refusal( + id="single-letter-source-with-path", + compose=_one_volume("v:/data"), + refusal_match=_NOT_ABSOLUTE, + podman_argv=["--mount", "type=volume,dst=v:/data"], + control_argv=_ANONYMOUS_CONTROL, + ), + Refusal( + id="single-letter-source-empty-target", + compose=_one_volume("v:"), + refusal_match=_NOT_ABSOLUTE, + podman_argv=["--mount", "type=volume,dst=v:"], + control_argv=_ANONYMOUS_CONTROL, + ), +] + + +LIMITATIONS: list[Limitation] = [ + Limitation( + id="drive-qualified-bind", + compose=_one_volume("C:\\data:/var"), + refusal_match=_SHORT_FORM_CANNOT_EMIT, + host_dir="C:\\data", + podman_argv=["--mount", "type=bind,src={host},dst=/var"], + ), + Limitation( + id="drive-relative-bind", + compose=_one_volume("C:data:/var"), + refusal_match=_SHORT_FORM_CANNOT_EMIT, + host_dir="C:data", + podman_argv=["--mount", "type=bind,src={host},dst=/var"], + ), +] diff --git a/tests/integration/test_podman_refusals.py b/tests/integration/test_podman_refusals.py new file mode 100644 index 0000000..d689f78 --- /dev/null +++ b/tests/integration/test_podman_refusals.py @@ -0,0 +1,48 @@ +"""Rule two, measured: which refusals podman agrees with, and which are only ours. + +ADR-0006 lets compose2pod refuse a document `docker compose config` accepts when +podman cannot express it, and calls a refusal podman *would* honour a tracked +limitation instead. That distinction is only as good as the measurement behind it, +and until now there was none -- issue #86 asserted expressibility, #104 shipped it, +and `podman run` exited 125. +""" + +from collections.abc import Callable +from pathlib import Path + +import pytest + +from compose2pod.exceptions import UnsupportedComposeError +from compose2pod.parsing import validate +from tests.integration.refusals import LIMITATIONS, REFUSALS, Limitation, Refusal + + +@pytest.mark.parametrize("refusal", REFUSALS, ids=lambda refusal: refusal.id) +def test_a_documented_refusal_names_a_mount_podman_will_not_make( + refusal: Refusal, probe_podman: Callable[[str, list[str]], int] +) -> None: + with pytest.raises(UnsupportedComposeError, match=refusal.refusal_match): + validate(refusal.compose) + + assert probe_podman(f"{refusal.id} [control]", refusal.control_argv) == 0, ( + "the control mount failed, so this row proves nothing about podman's verdict" + ) + assert probe_podman(refusal.id, refusal.podman_argv) != 0, ( + "podman made the mount this document asks for, so refusing it is a limitation, not rule two" + ) + + +@pytest.mark.parametrize("limitation", LIMITATIONS, ids=lambda limitation: limitation.id) +def test_a_tracked_limitation_names_a_mount_podman_would_have_made( + limitation: Limitation, probe_podman: Callable[[str, list[str]], int], tmp_path: Path +) -> None: + with pytest.raises(UnsupportedComposeError, match=limitation.refusal_match): + validate(limitation.compose) + + source = tmp_path / limitation.host_dir + source.mkdir(parents=True) + argv = [part.format(host=source) for part in limitation.podman_argv] + + assert probe_podman(f"{limitation.id} [limitation]", argv) == 0, ( + "podman refuses this mount too, so it is rule two and belongs in REFUSALS" + ) diff --git a/tests/test_parsing.py b/tests/test_parsing.py index a45e002..4dfc332 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -1739,34 +1739,38 @@ def test_tilde_bind_mount_needs_no_declaration() -> None: validate(_doc(volumes=["~/data:/var"])) +@pytest.mark.parametrize("entry", ["C:\\data", "v:/data", "a:/var", "v:"]) +def test_a_drive_shaped_entry_docker_reads_as_an_anonymous_volume_is_refused_for_its_target(entry: str) -> None: + # Measured against `docker compose config` v5.1.2: a leading single letter + # is a Windows drive marker whatever the letter is, so none of these names + # a volume -- even when the letter is declared top-level, a declaration + # Docker ignores. With one colon it is an anonymous volume whose target is + # the whole string, which podman refuses outright (`invalid container + # path`, measured 4.9.3). + with pytest.raises(UnsupportedComposeError, match="podman refuses a container path that is not absolute"): + validate(_doc(volumes=[entry])) + + @pytest.mark.parametrize( "entry", - [ - "C:\\data:/var", - "C:/data:/var", - "c:\\data:/var", - "C:\\data:/var:ro", - "C:\\data", - "C:data:/var", - "v:/data", - "a:/var", - "v:", - ], + ["C:\\data:/var", "C:/data:/var", "c:\\data:/var", "C:\\data:/var:ro", "C:data:/var", "v:/data:ro"], ) -def test_single_letter_volume_source_is_refused_with_the_podman_reason(entry: str) -> None: - # Measured against `docker compose config` v5.1.2: a leading single letter - # is a Windows drive marker whatever the letter is, so none of these names - # a volume. Docker reads a source keeping the drive colon - # (`C:\data:/var`, `v:/data:ro`), or an anonymous volume whose target is - # the whole string (`C:\data`, `v:/data`, `a:/var`, `v:`) -- even when the - # letter is declared top-level, which Docker ignores. podman 4.9.3 refuses - # every mount either reading makes: a colon inside a source has nowhere to - # go in a `-v` spec (`invalid option type "/var"`), and a container path - # that is not absolute is refused outright (`invalid container path`). - with pytest.raises(UnsupportedComposeError, match="Windows drive path"): +def test_a_drive_shaped_entry_docker_reads_as_a_bind_is_refused_for_the_short_form_spelling(entry: str) -> None: + # A second colon flips Docker's reading to a bind whose source keeps the + # drive colon (measured, v5.1.2). podman does mount that source -- 4.9.3 + # takes `--mount type=bind,src=C:\data,dst=/var` -- so the refusal is the + # short form's own limit, not rule two, and the message must say so. + with pytest.raises(UnsupportedComposeError, match="which the short form cannot emit"): validate(_doc(volumes=[entry])) +def test_a_bind_source_keeping_a_colon_can_still_be_mounted_through_the_long_form() -> None: + # The escape hatch the short-form refusal points at: the long form emits + # `--mount`, whose source is a single field, so the colon has somewhere to + # go. podman 4.9.3 mounts it (exit 0, measured in the integration job). + validate(_doc(volumes=[{"type": "bind", "source": "C:\\data", "target": "/var"}])) + + def test_a_one_character_volume_can_still_be_named_in_the_long_form() -> None: # The refusal is a short-syntax artifact, so the long form keeps the # capability: Docker honours `source: v` there (measured, v5.1.2 -- the