diff --git a/compose2pod/emit.py b/compose2pod/emit.py index 2c4c8cb..b04b6ad 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -11,7 +11,7 @@ from compose2pod.graph import depends_on, hostnames, startup_order from compose2pod.healthcheck import health_cmd, interval_seconds from compose2pod.keys import SERVICE_KEYS, Expand, GuardedEnvFile, Token -from compose2pod.parsing import split_volume, validate +from compose2pod.parsing import validate from compose2pod.pod import hosts_file_tokens, pod_create_flags from compose2pod.resources import deploy_resource_flags from compose2pod.shell import to_shell, variable_names @@ -137,7 +137,7 @@ def _volume_flags(svc: dict[str, Any], project_dir: str) -> list[Token]: # Anonymous volume: a bare container path, no host source to translate. flags += ["-v", Expand(value=volume)] continue - source, destination = split_volume(volume) + source, destination = volume.split(":", 1) if source.startswith("."): # Relative bind mount: resolve against project_dir. source = str(Path(project_dir, source)) diff --git a/compose2pod/parsing.py b/compose2pod/parsing.py index 6c9c09c..d453d8c 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -139,46 +139,35 @@ 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 Windows drive-letter source (`C:\data:/var`) reaches that name grammar - as the full `"C:\data"`, never the bare `"C"` a first-colon split yields, - because `split_volume` keeps the drive marker attached -- the one reason - the split is a shared function rather than a `partition(":")` at each site. + 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. """ if ":" not in volume: return "anonymous", None - source, _ = split_volume(volume) + source, _, _ = volume.partition(":") if stores.NAME_PATTERN.fullmatch(source): return "named", source return "bind", None -# Docker reads a leading `:` as a Windows drive marker and keeps it -# attached to the source. Measured against `docker compose config` v5.1.2: -# `C:\data:/var` and `C:/data:/var` both resolve to `{source: C:\data, target: -# /var}`, either case; `CC:\data:/var` does not -- the marker is one letter -# wide, and a second letter makes it a named-volume reference Docker rejects -# as undeclared. -_WINDOWS_DRIVE = re.compile(r"^[a-zA-Z]:[\\/]") - - -def split_volume(volume: str) -> tuple[str, str]: - r"""Split a short-syntax volume entry into its source and everything after it. - - The one definition of the colon-form split, shared by `_classify_volume` - here and `emit._volume_flags` so the drive-marker rule cannot hold at one - site and not the other. - - The marker only survives when a target follows it: `C:\data` on its own is - a single colon-form part, which Docker reads as an anonymous volume whose - target is the whole string, so that entry keeps the split -- and the - verdict -- it has always had. - """ - if _WINDOWS_DRIVE.match(volume): - source, separator, rest = volume[2:].partition(":") - if separator: - return volume[:2] + source, rest - source, _, rest = volume.partition(":") - return source, rest +# 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. +# +# 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]:[\\/][^:]*:") _VOLUME_LONG_TYPES = ("bind", "volume", "tmpfs", "image") @@ -214,6 +203,15 @@ 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"`). + msg = ( + f"service {name!r}: volume {volume!r}: a Windows drive-letter path " + "is not supported (podman cannot express it)" + ) + raise UnsupportedComposeError(msg) 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 2699373..9ffa1be 100644 --- a/docs/adr/0006-docker-rejection-parity.md +++ b/docs/adr/0006-docker-rejection-parity.md @@ -5,7 +5,8 @@ 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), and where compose2pod merely +`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 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/test_corpus.py b/tests/conformance/test_corpus.py index ace2c53..6c4b12e 100644 --- a/tests/conformance/test_corpus.py +++ b/tests/conformance/test_corpus.py @@ -130,16 +130,16 @@ def test_volumes_long_form_image_type_is_no_longer_an_over_rejection( assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept" -def test_volume_windows_drive_letter_bind_is_no_longer_an_over_rejection( +def test_volume_windows_drive_letter_bind_is_a_catalogued_over_rejection( assert_rule: Callable[[dict[str, Any]], str], ) -> None: - r"""`split_volume` keeps the drive marker attached instead of splitting on the first colon. + r"""Docker accepts `volumes: ['C:\data:/var']`; podman cannot express it, so we refuse it. - Same reasoning as the over-rejection tests above: the generic corpus run alone - would stay green even pre-fix, filing `volume_windows_drive_letter_bind` under - the allowed 'over-reject' verdict instead of catching a regression. The - stronger claim -- both oracles ACCEPT `volumes: ['C:\data:/var']` with no - top-level declaration -- needs this dedicated assertion on the verdict itself. + The inverse of the tests above, and the reason it is asserted rather than + left to the generic corpus run: `over-reject` is an allowed verdict, so the + run stays green whichever way this file falls. Pinning it here makes the + catalogued limitation (issue 105, measured against podman 4.9.3) fail loudly + if it ever turns into an accept without the ruling being revisited. """ path = Path(__file__).parent / "corpus" / "volume_windows_drive_letter_bind.yaml" - assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept" + assert assert_rule(yaml.safe_load(path.read_text())) == "over-reject" diff --git a/tests/test_emit.py b/tests/test_emit.py index d429d44..b158e76 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -127,10 +127,6 @@ def test_anonymous_volume_emitted_as_single_path(self) -> None: flags = run_flags("app", {"image": "x", "volumes": ["/var/cache/models"]}, "p", "/builds/x") assert flags[4:6] == ["-v", Expand(value="/var/cache/models")] - def test_windows_drive_letter_source_is_kept_as_is(self) -> None: - flags = run_flags("app", {"image": "x", "volumes": ["C:\\data:/var"]}, "p", "/builds/x") - assert flags[4:6] == ["-v", Expand(value="C:\\data:/var")] - def test_named_volume_emitted_without_project_dir_translation(self) -> None: svc = {"image": "x", "volumes": ["pgdata:/var/lib/postgresql/data"]} flags = run_flags("db", svc, "p", "/builds/x") diff --git a/tests/test_parsing.py b/tests/test_parsing.py index df25c3e..00d40d2 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -1739,25 +1739,27 @@ def test_tilde_bind_mount_needs_no_declaration() -> None: validate(_doc(volumes=["~/data:/var"])) -def test_windows_drive_letter_bind_needs_no_declaration() -> None: - # Measured against `docker compose config` v5.1.2, no top-level 'volumes:' - # block: every spelling below resolves to `{type: bind, source: C:\data, - # target: /var}`. A leading `:` is a drive marker, so the source - # keeps it instead of being read as the one-character volume name a - # first-colon split yields, and either separator spells the drive. - validate(_doc(volumes=["C:\\data:/var"])) - validate(_doc(volumes=["C:/data:/var"])) - validate(_doc(volumes=["c:\\data:/var"])) - validate(_doc(volumes=["C:\\data:/var:ro"])) - - -def test_drive_marker_without_a_target_keeps_its_old_split() -> None: - # `C:\data` carries no second colon, so it is one colon-form part and the - # drive marker does not apply: the split stays where it was, the source is - # still `C`, and the entry is still refused as an undeclared named volume. - # Docker reads it as an anonymous volume whose target is the whole string - # (measured, v5.1.2) -- an over-rejection this change neither introduces - # nor closes, kept visible here rather than silently widened into. +@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"): + 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"]))