From d8f918c87515202597a2b501792ed9d49246c662 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 20 Sep 2026 11:19:37 +0300 Subject: [PATCH] fix: keep a Windows drive letter attached to a volume source --- compose2pod/emit.py | 4 +-- compose2pod/parsing.py | 48 +++++++++++++++++++++++--------- tests/conformance/test_corpus.py | 15 ++++++++++ tests/test_emit.py | 4 +++ tests/test_parsing.py | 31 +++++++++++++++++++++ 5 files changed, 87 insertions(+), 15 deletions(-) diff --git a/compose2pod/emit.py b/compose2pod/emit.py index b04b6ad..2c4c8cb 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 validate +from compose2pod.parsing import split_volume, 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 = volume.split(":", 1) + source, destination = split_volume(volume) 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 3688cab..6c9c09c 100644 --- a/compose2pod/parsing.py +++ b/compose2pod/parsing.py @@ -1,5 +1,6 @@ """Validate a compose document against the supported subset.""" +import re from collections.abc import Callable from typing import Any @@ -138,27 +139,48 @@ 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 genuine Windows drive-letter source (`C:\\data:/var`) is NOT fixed by - this pattern swap: `source, _, _ = volume.partition(":")` splits on the - FIRST colon regardless, so for that entry `source` is just `"C"` -- a - single letter, which is itself a syntactically valid NAME_PATTERN match -- - not the full `"C:\\data"` a naive reading of "doesn't match the pattern" - might suggest. Docker's own parser special-cases a leading `:\\` - to keep the drive letter attached to the source before ever comparing it - to a name grammar; this module (and `emit.py`'s `_volume_flags`, which - shares the same first-colon split for the same reason) does not. Measured, - still REJECTs post-fix -- a pre-existing, uncatalogued residual from when - this check was introduced (`_named_volume_source`'s Task 14 predecessor), - not something this change introduces or was scoped to close. + 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. """ if ":" not in volume: return "anonymous", None - source, _, _ = volume.partition(":") + source, _ = split_volume(volume) 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 + + _VOLUME_LONG_TYPES = ("bind", "volume", "tmpfs", "image") _VOLUME_LONG_KEYS = {"type", "source", "target", "read_only", "consistency"} # Docker's own per-type nested option map keys (measured, docker compose config diff --git a/tests/conformance/test_corpus.py b/tests/conformance/test_corpus.py index 4c44d85..ace2c53 100644 --- a/tests/conformance/test_corpus.py +++ b/tests/conformance/test_corpus.py @@ -128,3 +128,18 @@ def test_volumes_long_form_image_type_is_no_longer_an_over_rejection( """ path = Path(__file__).parent / "corpus" / "volumes_long_form_image_type.yaml" assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept" + + +def test_volume_windows_drive_letter_bind_is_no_longer_an_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. + + 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. + """ + path = Path(__file__).parent / "corpus" / "volume_windows_drive_letter_bind.yaml" + assert assert_rule(yaml.safe_load(path.read_text())) == "both-accept" diff --git a/tests/test_emit.py b/tests/test_emit.py index b158e76..d429d44 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -127,6 +127,10 @@ 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 ca10a73..df25c3e 100644 --- a/tests/test_parsing.py +++ b/tests/test_parsing.py @@ -1739,6 +1739,37 @@ 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. + with pytest.raises(UnsupportedComposeError, match="undefined volume 'C'"): + validate(_doc(volumes=["C:\\data"])) + + +def test_two_letter_drive_prefix_is_still_a_named_volume() -> None: + # The drive marker is exactly one letter wide: `CC:\data:/var` REJECTS as + # "refers to undefined volume CC" (measured, v5.1.2), so widening the + # prefix would start accepting a document Docker refuses -- rule one. + with pytest.raises(UnsupportedComposeError, match="undefined volume 'CC'"): + validate(_doc(volumes=["CC:\\data:/var"])) + + def test_hyphenated_and_underscored_named_volume_must_be_declared() -> None: # A bare identifier still needs a top-level declaration regardless of # which NAME_PATTERN characters it uses -- this is not weakened by the