Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions compose2pod/parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -260,15 +264,19 @@ 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 --
`bind`/`volume`/`tmpfs` -- the entry is allowed to carry alongside it.
"""
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)
Expand All @@ -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"]):
Expand Down
11 changes: 8 additions & 3 deletions docs/adr/0006-docker-rejection-parity.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions tests/conformance/corpus/volume_long_form_cluster_type.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
app:
image: nginx
volumes:
- type: cluster
source: x
target: /d
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
app:
image: nginx
volumes:
- type: bnid
source: x
target: /d
27 changes: 27 additions & 0 deletions tests/conformance/test_corpus.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
21 changes: 15 additions & 6 deletions tests/integration/refusals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -167,22 +170,28 @@ 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"],
control_argv=_ANONYMOUS_CONTROL,
),
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,
),
Expand Down
27 changes: 26 additions & 1 deletion tests/test_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'"),
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 "/".
Expand Down
19 changes: 18 additions & 1 deletion tests/test_podman_claim_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand All @@ -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.

Expand Down
Loading