From 132879c985a6078de430fde60497dc226ddbafee Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 20 Sep 2026 12:17:01 +0300 Subject: [PATCH] fix: refuse a single-letter volume source in the short syntax --- compose2pod/parsing.py | 51 ++++++++-------- docs/adr/0006-docker-rejection-parity.md | 6 +- .../corpus/volume_single_letter_source.yaml | 7 +++ tests/conformance/test_corpus.py | 16 +++++ tests/test_parsing.py | 59 ++++++++++++------- 5 files changed, 90 insertions(+), 49 deletions(-) create mode 100644 tests/conformance/corpus/volume_single_letter_source.yaml diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index d453d8c..179fe43 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -139,11 +139,10 @@ def _classify_volume(volume: str) -> tuple[str, str | None]: (tilde in particular) into "named" -- an over-rejection once paired with the reference check below, since neither needs a top-level declaration. - A drive-qualified source with a target (`C:\data:/var`) never reaches this - split: `_validate_service_volumes` refuses it first, so the leading `C` - this would otherwise read as a one-character volume name is not a verdict - anyone sees. A drive-shaped entry with no target (`C:\data`) does reach it, - and is still classified by the name grammar -- see issue 105. + No drive-shaped entry (`C:\data:/var`, `v:/data`) reaches this split: + `_validate_service_volumes` refuses the family first. So the name grammar + below never sees a one-character source, and the `named` verdict it can + return always names a volume Docker would name too. """ if ":" not in volume: return "anonymous", None @@ -153,21 +152,22 @@ def _classify_volume(volume: str) -> tuple[str, str | None]: return "bind", None -# A source Docker reads as a Windows drive path, with a target after it: any -# single letter, either separator, then a further colon. Measured against -# `docker compose config` v5.1.2, the drive marker is what the letter means -- -# `C:\data:/var` and `C:/data:/var` are binds on `{source: C:\data, target: -# /var}`, and so is `v:/data:ro`, read as `{source: v:/data, target: ro}` -# rather than the named volume `v` its spelling suggests. Two letters -# (`CC:\data:/var`) is an ordinary named-volume reference instead. +# A short-syntax entry Docker reads as a Windows drive path. The marker is one +# leading letter and a colon, whatever the letter is: measured against `docker +# compose config` v5.1.2, `C:\data:/var` and `v:/data:ro` are both binds whose +# source keeps the colon (`{source: v:/data, target: ro}`, not the named +# volume `v` the spelling suggests), and `C:\data`, `v:/data`, `a:/var`, `v:` +# are all anonymous volumes whose target is the whole string -- the last three +# even when that letter is declared top-level, a declaration Docker ignores. +# Two letters (`CC:\data:/var`) is an ordinary named-volume reference instead. # -# The trailing colon is load-bearing: it is what makes the source carry a -# colon, which is the thing podman's `-v` cannot take (it splits a spec into -# at most source:target:options, measured against podman 4.9.3). Without it -- -# `C:\data`, `v:/data` -- Docker reads an anonymous volume whose target is the -# whole string, a different divergence with its own verdict, tracked in issue -# 105 rather than refused here. -_WINDOWS_DRIVE_SOURCE = re.compile(r"^[a-zA-Z]:[\\/][^:]*:") +# 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`. +_DRIVE_SHAPED_SOURCE = re.compile(r"^[a-zA-Z]:") _VOLUME_LONG_TYPES = ("bind", "volume", "tmpfs", "image") @@ -203,13 +203,14 @@ def _validate_service_volumes(name: str, svc: dict[str, Any]) -> None: if not isinstance(volume, str): msg = f"service {name!r}: volume entry must be a string or mapping" raise UnsupportedComposeError(msg) - if _WINDOWS_DRIVE_SOURCE.match(volume): - # Refused before classification, so the drive colon is never read - # as the end of a one-character volume name (measured, podman - # 4.9.3: `invalid option type "/var"`). + if _DRIVE_SHAPED_SOURCE.match(volume): + # 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 Windows drive-letter path " - "is not supported (podman cannot express it)" + 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) kind, _ = _classify_volume(volume) diff --git a/docs/adr/0006-docker-rejection-parity.md b/docs/adr/0006-docker-rejection-parity.md index 9ffa1be..dbbc24b 100644 --- a/docs/adr/0006-docker-rejection-parity.md +++ b/docs/adr/0006-docker-rejection-parity.md @@ -5,8 +5,10 @@ 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 drive-qualified volume source -such as `C:\data:/var`, whose colon podman's `-v` cannot carry), and where compose2pod merely +`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 negative on a top-level numeric key are facts about the machine that runs the script and are diff --git a/tests/conformance/corpus/volume_single_letter_source.yaml b/tests/conformance/corpus/volume_single_letter_source.yaml new file mode 100644 index 0000000..8a90562 --- /dev/null +++ b/tests/conformance/corpus/volume_single_letter_source.yaml @@ -0,0 +1,7 @@ +services: + app: + image: nginx + volumes: + - 'v:/data' +volumes: + v: diff --git a/tests/conformance/test_corpus.py b/tests/conformance/test_corpus.py index 6c4b12e..17355bf 100644 --- a/tests/conformance/test_corpus.py +++ b/tests/conformance/test_corpus.py @@ -143,3 +143,19 @@ def test_volume_windows_drive_letter_bind_is_a_catalogued_over_rejection( """ path = Path(__file__).parent / "corpus" / "volume_windows_drive_letter_bind.yaml" assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject" + + +def test_volume_single_letter_source_is_a_catalogued_over_rejection( + assert_rule: Callable[[dict[str, Any]], str], +) -> None: + """Docker accepts `volumes: ['v:/data']` -- as an anonymous volume, not as the declared `v`. + + The declaration in the file is deliberate: Docker ignores it, because a + leading single letter is a drive marker and never a volume name. Both + oracles once accepted this document while meaning different mounts, which + is a divergence the harness cannot see -- it compares verdicts, not + meanings. Refusing it makes the disagreement visible as an over-rejection, + and asserting the verdict here keeps it that way. + """ + path = Path(__file__).parent / "corpus" / "volume_single_letter_source.yaml" + assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject" diff --git a/tests/test_parsing.py b/tests/test_parsing.py index 00d40d2..a45e002 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -1739,29 +1739,44 @@ def test_tilde_bind_mount_needs_no_declaration() -> None: validate(_doc(volumes=["~/data:/var"])) -@pytest.mark.parametrize("entry", ["C:\\data:/var", "C:/data:/var", "c:\\data:/var", "C:\\data:/var:ro", "v:/data:ro"]) -def test_drive_qualified_volume_source_is_refused_with_the_podman_reason(entry: str) -> None: - # Measured on both sides. `docker compose config` v5.1.2 ACCEPTS each of - # these as a bind whose source keeps the drive colon -- including - # `v:/data:ro`, read as `{source: v:/data, target: ro}` rather than the - # named volume its spelling suggests. podman 4.9.3 REJECTS the `-v` spec - # they render to (`invalid option type "/var"`): a spec splits into at - # most source:target:options, so a colon inside the source pushes the - # target into the option slot. Rule two -- a refusal that cites podman, - # where the old one named a phantom volume 'C' the document never wrote. - with pytest.raises(UnsupportedComposeError, match="Windows drive-letter path"): +@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:", + ], +) +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"): validate(_doc(volumes=[entry])) -def test_drive_shaped_entry_without_a_target_keeps_its_old_verdict() -> None: - # `C:\data` carries one colon, so Docker reads an anonymous volume whose - # target is the whole string, and podman refuses it as a non-absolute - # container path. Neither the source-with-a-colon shape the refusal above - # is about, nor a form this change rules on: it keeps the verdict it has - # always had, tracked in issue 105 with the other drive-adjacent - # spellings. - with pytest.raises(UnsupportedComposeError, match="undefined volume 'C'"): - validate(_doc(volumes=["C:\\data"])) +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 + # document resolves to the declared volume `v`, not to a drive path), and + # podman expresses it as an ordinary named-volume mount. + compose = { + "services": {"app": {"image": "nginx", "volumes": [{"type": "volume", "source": "v", "target": "/data"}]}}, + "volumes": {"v": None}, + } + assert validate(compose) == ["ignoring top-level 'volumes' (podman creates named volumes on first reference)"] def test_two_letter_drive_prefix_is_still_a_named_volume() -> None: @@ -1923,8 +1938,8 @@ def _net_def_doc(definition: object) -> dict: def _vol_def_doc(definition: object) -> dict: - """One declared top-level volume 'v', referenced by a service, with 'definition' as its own body.""" - return {"services": {"app": {"image": "nginx", "volumes": ["v:/data"]}}, "volumes": {"v": definition}} + """One declared top-level volume 'vol', referenced by a service, with 'definition' as its own body.""" + return {"services": {"app": {"image": "nginx", "volumes": ["vol:/data"]}}, "volumes": {"vol": definition}} # Task 12: the top-level `networks:`/`volumes:` blocks' own DEFINITION contents