diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 5127272..71387f3 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -207,6 +207,7 @@ def _reject_drive_shaped_volume(name: str, volume: str) -> None: _VOLUME_LONG_TYPES = ("bind", "volume", "tmpfs", "image") +_DOCKER_ONLY_VOLUME_TYPES = ("cluster", "npipe") _VOLUME_LONG_KEYS = {"type", "source", "target", "read_only", "consistency"} # Docker's own per-type nested option map keys (measured, docker compose config # v5.1.2). `create_host_path`/`nocopy` are real docker keys, so they land here @@ -246,7 +247,10 @@ def _validate_service_volumes(name: str, svc: dict[str, Any]) -> None: _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" + msg = ( + f"service {name!r}: anonymous volume '{volume}' must be an absolute path " + f"({podman.REFUSES_RELATIVE_CONTAINER_PATH})" + ) raise UnsupportedComposeError(msg) # A named or bind entry needs no further shape check here -- both are # accepted; podman creates a named volume implicitly on first @@ -260,8 +264,9 @@ def _validate_volume_long_form(name: str, entry: dict[str, Any]) -> None: type (bind/volume/tmpfs/image), source, target, read_only, consistency, plus the one nested option map matching `type` (a mismatched sub-map is refused -- a deliberate stricter-than-docker check; docker - accepts-and-ignores it). cluster/npipe types are refused (podman cannot - express them). + accepts-and-ignores it). cluster and npipe are split off first because + docker takes them and podman cannot express them, which is a different + refusal from a misspelling docker rejects too. `type` is validated before the unknown-key check (unlike every other field here) because the check itself needs `vtype` to know which sub-map key -- @@ -269,6 +274,9 @@ def _validate_volume_long_form(name: str, entry: dict[str, Any]) -> None: """ keys = require_string_keys(f"service {name!r}: volume", entry) vtype = entry.get("type") + if vtype in _DOCKER_ONLY_VOLUME_TYPES: + msg = f"service {name!r}: volume 'type: {vtype}' is not supported ({podman.CANNOT_EXPRESS})" + raise UnsupportedComposeError(msg) if vtype not in _VOLUME_LONG_TYPES: msg = f"service {name!r}: volume 'type' must be one of {list(_VOLUME_LONG_TYPES)}" raise UnsupportedComposeError(msg) @@ -281,11 +289,9 @@ def _validate_volume_long_form(name: str, entry: dict[str, Any]) -> None: msg = f"service {name!r}: volume 'target' must be a string" raise UnsupportedComposeError(msg) if not target.startswith("/") and not values.has_variable(target): - # podman rejects a relative --mount target for every type ("must be - # an absolute path"); docker accepts it. A ${VAR} target is - # host-dependent, so it is carved out like every other - # values.has_variable case in this file. - msg = f"service {name!r}: volume 'target' must be an absolute path" + # A ${VAR} target is host-dependent, so it is carved out like every + # other values.has_variable case in this file. + msg = f"service {name!r}: volume 'target' must be an absolute path ({podman.REFUSES_RELATIVE_CONTAINER_PATH})" raise UnsupportedComposeError(msg) _validate_volume_long_form_source(name, vtype, entry.get("source")) if "read_only" in entry and not values.is_bool_like(entry["read_only"]): diff --git a/docs/adr/0006-docker-rejection-parity.md b/docs/adr/0006-docker-rejection-parity.md index 4f56cee..d7d59ba 100644 --- a/docs/adr/0006-docker-rejection-parity.md +++ b/docs/adr/0006-docker-rejection-parity.md @@ -38,9 +38,14 @@ its clause from there, and `tests/test_podman_claim_coverage.py` pairs each site with the row that measures it, in both directions. The handle is the attribute name, not the wording, because prose is not a registry -- a message is assembled from an f-string, a shared preamble or a lookup table depending on the site. A refusal that makes no claim is out of scope by -construction, which is how `network_mode` needs no row; a refusal whose reason is podman's but whose -message keeps that to itself is invisible to the gate, and that list is -[#121](https://github.com/modern-python/compose2pod/issues/121). +construction, which is how `network_mode` needs no row. A refusal whose reason is podman's but +whose message keeps that to itself would be invisible to that gate, so the table closes it from +the other side: every `REFUSALS` row names a claim, because the table's premise is that podman +will not make the mount. That is what emptied +[#121](https://github.com/modern-python/compose2pod/issues/121)'s list, which is also why a +long-form `type: cluster` no longer refuses with the same message as a misspelled one -- docker +takes cluster and npipe, so refusing them is rule two and says so, while `bnid` is rule one and +podman has nothing to do with it. Verdicts are per version, and the supported range is stated rather than implied: the rulings here are measured against `docker compose config` v5.1.2 and podman 4.9.3, and compose2pod supports podman 4.9 and up. Rule two reads across that whole range. A form is accepted only where podman diff --git a/tests/conformance/corpus/volume_long_form_cluster_type.yaml b/tests/conformance/corpus/volume_long_form_cluster_type.yaml new file mode 100644 index 0000000..e2105af --- /dev/null +++ b/tests/conformance/corpus/volume_long_form_cluster_type.yaml @@ -0,0 +1,7 @@ +services: + app: + image: nginx + volumes: + - type: cluster + source: x + target: /d diff --git a/tests/conformance/corpus/volume_long_form_misspelled_type.yaml b/tests/conformance/corpus/volume_long_form_misspelled_type.yaml new file mode 100644 index 0000000..1e56a72 --- /dev/null +++ b/tests/conformance/corpus/volume_long_form_misspelled_type.yaml @@ -0,0 +1,7 @@ +services: + app: + image: nginx + volumes: + - type: bnid + source: x + target: /d diff --git a/tests/conformance/test_corpus.py b/tests/conformance/test_corpus.py index 17355bf..c06d43a 100644 --- a/tests/conformance/test_corpus.py +++ b/tests/conformance/test_corpus.py @@ -145,6 +145,33 @@ def test_volume_windows_drive_letter_bind_is_a_catalogued_over_rejection( assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject" +def test_volume_long_form_cluster_type_is_a_catalogued_over_rejection( + assert_rule: Callable[[dict[str, Any]], str], +) -> None: + """Docker accepts `type: cluster`; podman has no such mount, so we refuse it -- rule two. + + Asserted rather than left to the generic corpus run for the usual reason: `over-reject` + is an allowed verdict, so the run stays green whichever way this file falls. What it + pins is the half of issue #121's split that only docker can answer -- the refusal cites + podman, and citing podman is only legitimate while docker itself takes the document. + """ + path = Path(__file__).parent / "corpus" / "volume_long_form_cluster_type.yaml" + assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject" + + +def test_volume_long_form_misspelled_type_is_rejected_by_both( + assert_rule: Callable[[dict[str, Any]], str], +) -> None: + """The other half of the split: docker rejects a typo too, so podman is not the reason. + + `volume 'type' must be one of [...]` fires for both this and `cluster`, and the two are + refused for opposite reasons. Pinning the verdict keeps the message that claims nothing + about podman attached to the case where podman has nothing to do with it. + """ + path = Path(__file__).parent / "corpus" / "volume_long_form_misspelled_type.yaml" + assert assert_rule(yaml.safe_load(path.read_text())) == "both-reject" + + def test_volume_single_letter_source_is_a_catalogued_over_rejection( assert_rule: Callable[[dict[str, Any]], str], ) -> None: diff --git a/tests/integration/refusals.py b/tests/integration/refusals.py index 95c7c7d..8a6192d 100644 --- a/tests/integration/refusals.py +++ b/tests/integration/refusals.py @@ -30,9 +30,11 @@ A row measuring a refusal whose message draws a clause from `compose2pod/podman.py` names the `site` that makes the claim and the `claim` it makes, which is what -`tests/test_podman_claim_coverage.py` gates on. A row whose refusal makes no such claim -- -a relative target, an unsupported long-form `type` -- leaves both empty: the reason is -podman's, but the message keeps it to itself, which is issue #121's subject. +`tests/test_podman_claim_coverage.py` gates on. Every `REFUSALS` row names one, because the +table's own premise is that podman will not make the mount: a row that named none would mark +a message stating a rule while keeping podman's reason to itself, which is what #121 found +in four of them. Only a `LIMITATIONS` row leaves both empty, and only where the limit really +is the short form's rather than podman's. Four claims, four experiments. `network_mode` alone has no row: it is refused under ADR-0003, not rule two, and podman honours it (#115). The gate that every rule-two site @@ -127,12 +129,13 @@ def _one_volume(entry: "str | dict[str, Any]") -> dict[str, Any]: _NOT_ABSOLUTE = "podman refuses a container path that is not absolute" _SHORT_FORM_CANNOT_EMIT = "which the short form cannot emit" -_UNSUPPORTED_LONG_TYPE = "volume 'type' must be one of" REFUSALS: list[Refusal] = [ Refusal( id="anonymous-volume-relative-target", + site="parsing._validate_service_volumes", + claim="REFUSES_RELATIVE_CONTAINER_PATH", compose=_one_volume("a"), refusal_match="anonymous volume 'a' must be an absolute path", podman_argv=["--mount", "type=volume,dst=a"], @@ -167,6 +170,8 @@ def _one_volume(entry: "str | dict[str, Any]") -> dict[str, Any]: ), Refusal( id="long-form-relative-target", + site="parsing._validate_volume_long_form", + claim="REFUSES_RELATIVE_CONTAINER_PATH", compose=_one_volume({"type": "volume", "target": "rel"}), refusal_match="volume 'target' must be an absolute path", podman_argv=["--mount", "type=volume,dst=rel"], @@ -174,15 +179,19 @@ def _one_volume(entry: "str | dict[str, Any]") -> dict[str, Any]: ), Refusal( id="long-form-type-cluster", + site="parsing._validate_volume_long_form", + claim="CANNOT_EXPRESS", compose=_one_volume({"type": "cluster", "target": "/data"}), - refusal_match=_UNSUPPORTED_LONG_TYPE, + refusal_match="volume 'type: cluster' is not supported", podman_argv=["--mount", "type=cluster,dst=/data"], control_argv=_ANONYMOUS_CONTROL, ), Refusal( id="long-form-type-npipe", + site="parsing._validate_volume_long_form", + claim="CANNOT_EXPRESS", compose=_one_volume({"type": "npipe", "target": "/data"}), - refusal_match=_UNSUPPORTED_LONG_TYPE, + refusal_match="volume 'type: npipe' is not supported", podman_argv=["--mount", "type=npipe,dst=/data"], control_argv=_ANONYMOUS_CONTROL, ), diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 4d63047..b6d368a 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -121,7 +121,7 @@ def test_long_volume_entry_rejects(self) -> None: # Each case drives one distinct reject branch (all six needed for 100% coverage). cases = [ ([5], r"volume entry must be a string or mapping"), - ([{"type": "cluster", "source": "x", "target": "/d"}], r"volume 'type' must be one of"), + ([{"type": "bnid", "source": "x", "target": "/d"}], r"volume 'type' must be one of"), ([{"type": "volume", "target": 5}], r"volume 'target' must be a string"), ([{"type": "bind", "target": "/d"}], r"bind volume 'source' must be a string"), ([{"type": "tmpfs", "source": "x", "target": "/t"}], r"tmpfs volume takes no 'source'"), @@ -151,6 +151,27 @@ def test_long_volume_relative_target_rejected(self) -> None: with pytest.raises(UnsupportedComposeError, match=r"volume 'target' must be an absolute path"): validate({"services": {"app": {"image": "x", "volumes": [entry]}}}) + def test_a_volume_type_docker_takes_and_podman_cannot_is_refused_with_podmans_reason(self) -> None: + # docker compose config v5.1.2 accepts cluster and npipe, so refusing them is rule two. + for vtype in ("cluster", "npipe"): + entry = {"type": vtype, "source": "x", "target": "/d"} + with pytest.raises( + UnsupportedComposeError, + match=rf"volume 'type: {vtype}' is not supported \(podman cannot express it\)", + ): + validate({"services": {"app": {"image": "x", "volumes": [entry]}}}) + + def test_a_volume_type_docker_rejects_too_is_refused_without_a_podman_reason(self) -> None: + # docker compose config v5.1.2 rejects a typo as well, so podman has nothing to do with it. + with pytest.raises(UnsupportedComposeError, match=r"volume 'type' must be one of") as refusal: + validate({"services": {"app": {"image": "x", "volumes": [{"type": "bnid", "target": "/d"}]}}}) + + assert "podman" not in str(refusal.value) + + def test_a_relative_long_form_target_is_refused_with_podmans_reason(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"podman refuses a container path that is not absolute"): + validate({"services": {"app": {"image": "x", "volumes": [{"type": "volume", "target": "rel"}]}}}) + def test_long_volume_variable_target_accepted(self) -> None: # A ${VAR}-carrying target is host-dependent -- accepted, matching # every other values.has_variable carve-out in parsing.py. @@ -268,6 +289,10 @@ def test_relative_anonymous_volume_raises(self) -> None: with pytest.raises(UnsupportedComposeError, match="absolute"): validate({"services": {"app": {"image": "x", "volumes": ["./cache"]}}}) + def test_a_relative_anonymous_volume_is_refused_with_podmans_reason(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"podman refuses a container path that is not absolute"): + validate({"services": {"app": {"image": "x", "volumes": ["./cache"]}}}) + def test_string_volumes_rejected_at_gate(self) -> None: # Used to be iterated character-wise: "/data:/data" reported the nonsense # "anonymous volume 'd'", and "/" was silently accepted as -v "/". diff --git a/tests/test_podman_claim_coverage.py b/tests/test_podman_claim_coverage.py index e377085..b3fa443 100644 --- a/tests/test_podman_claim_coverage.py +++ b/tests/test_podman_claim_coverage.py @@ -64,7 +64,8 @@ def test_every_claim_compose2pod_makes_about_podman_is_measured_by_a_row() -> No Out of scope by construction: a refusal that makes no claim here. `network_mode` is refused under docs/adr/0003-the-shared-namespace-decides-key-classification.md and podman honours it, so it has no claim and needs no row. A refusal whose reason *is* podman's but - whose message keeps that to itself is invisible to this gate, which is issue #121. + whose message keeps that to itself would be invisible to this gate, which is what the + invariant below covers from the other side. """ unmeasured = sorted(claim_sites(_PACKAGE) - _measured_pairs()) @@ -73,6 +74,22 @@ def test_every_claim_compose2pod_makes_about_podman_is_measured_by_a_row() -> No ) +def test_every_refusal_podman_agrees_with_tells_the_user_podmans_reason() -> None: + """INVARIANT: a rule-two refusal states the reason that makes it legitimate, not just the rule. + + A `REFUSALS` row exists because podman will not make the mount, so the refusal's reason is + podman's. A row with no claim therefore marks a message that states a rule while keeping + its reason in the source, which is what issue #121 found in four of them, and which the + gate above cannot see because there is no claim for it to scan. + + `LIMITATIONS` is where a refusal with no podman reason belongs: the drive-qualified bind + is the short form's own limit, and its message says so. + """ + silent = sorted(row.id for row in REFUSALS if not row.claim) + + assert silent == [], "\n".join(f"{row} refuses for podman's reason without telling the user" for row in silent) + + def test_every_row_naming_a_claim_names_one_the_source_still_makes() -> None: """INVARIANT: a row measures a claim that exists, at the site that makes it.