Skip to content

Use the Mount API for mounts whose target contains a colon - #14423

Open
Huang-404-Q wants to merge 1 commit into
docker:mainfrom
Huang-404-Q:fix/volume-colon-target
Open

Huang-404-Q wants to merge 1 commit into
docker:mainfrom
Huang-404-Q:fix/volume-colon-target

Conversation

@Huang-404-Q

Copy link
Copy Markdown

Fixes #12993.

A long-syntax volume whose target contains a colon fails to create, because Compose sends it as a legacy bind string and the engine splits that on colons:

target: /foo/bar:baz   →   Binds: ["my_volume:/foo/bar:baz:rw"]   →   "invalid volume specification"

A colon is a legal filename character on Linux. This makes such mounts fall back to the Mounts API, which carries source and target as separate fields. Windows drive letters (C:\data) are not treated as separators and keep using the bind string.

Details

Why

Compose prefers HostConfig.Binds when a mount is simple enough and uses Mounts otherwise. The "simple enough" checks only looked at mount options (bindRequiresMountAPI / volumeRequiresMountAPI), but the engine splits a bind string on colons (source:target:options), so a colon in the source or the target is read as a field separator no matter what the options are.

Change

if bindRequiresMountAPI(v.Bind) || !bindStringCanRepresent(m.Source, m.Target) {
    return "", false
}

with

func bindStringCanRepresent(source, target string) bool {
	return !hasBindStringSeparator(source) && !hasBindStringSeparator(target)
}

func hasBindStringSeparator(p string) bool {
	return strings.Contains(p, ":") && !isWindowsAbs(p)
}

Tests

Three cases added to Test_buildContainerVolumes: a named volume and a bind mount with a colon in the target (both must produce Mounts and no bind string), plus a Windows target (C:\data) that must still produce my_volume:C:\data:rw.

The pkg/compose test package does not build on Windows (start_test.go is //go:build !windows while other test files use its helpers), so the binary was cross-compiled and run under Linux:

GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go test -c -o compose.test ./pkg/compose/
./compose.test -test.run Test_buildContainerVolumes -test.v

Without the change:

    --- FAIL: Test_buildContainerVolumes/mount_volume,_target_with_a_colon (0.01s)
    --- FAIL: Test_buildContainerVolumes/bind_mount,_target_with_a_colon (0.00s)

With it:

--- PASS: Test_buildContainerVolumes (0.02s)
    --- PASS: .../mount_volume,_target_with_a_colon
    --- PASS: .../bind_mount,_target_with_a_colon
    --- PASS: .../mount_volume,_windows_target_keeps_the_bind_string

AI usage: the change and this description were drafted with Claude Code and then reviewed, built and tested by me; the analysis above is checked against the engine's mount parser and the tests were run as shown.

…ress

The legacy Bind-API form "source:target[:options]" is split on colons by
the engine, so a bind source or a mount target containing a colon - a
named volume mounted at /foo/bar:baz, for instance - was sent verbatim
and rejected as an invalid volume specification. Detect that case and
fall back to the Mount API, which carries the paths as fields. Windows
drive-letter paths keep the string form: their colon is not a separator,
and switching them would change bind mountpoint creation for no reason.

Signed-off-by: huang <13165998033@163.com>
@Huang-404-Q
Huang-404-Q requested review from a team as code owners October 29, 2026 09:45
@Huang-404-Q
Huang-404-Q requested review from glours and ndeloof October 29, 2026 09:45
@ndeloof

ndeloof commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

I'd prefer we introduce an explicit option to opt-in for the Mount API vs introduce yet another corner-case

@ndeloof

ndeloof commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

see compose-spec/compose-go#979

@Huang-404-Q

Copy link
Copy Markdown
Author

Fair — and thanks for pointing at compose-go#979.

Worth noting what the current state is: the Mount API is already reachable today, just not explicitly — any advanced option on the entry forces it (bind.create_host_path: false, bind.propagation, bind.recursive, volumes[].labels, subpath, no_copy). So a user hitting the colon problem has a workaround, it's just unobvious and has side effects of its own (e.g. create_host_path: false changes bind behaviour).

Given that, I see three ways forward and I'm happy to do whichever you prefer:

  1. Documentation only. Describe the existing advanced options as the supported way to force the Mount API, and drop this heuristic. Smallest change; leaves the colon case as "set one of these and it works".
  2. Explicit opt-in in Compose. Read something like x-mount-api: true off the volume entry's extensions and force the Mount API for that mount. Contained to Compose, but it would be a non-standard knob and probably wants to live in compose-go's model rather than in an extension.
  3. Wait for compose-go#979 and let the canonical short syntax plus mounts settle this properly. I'll close this PR if that's the direction — the test here (the engine splits a bind string on colons, so a colon in source/target makes the string form unusable) stays valid either way.

My preference is 3 if #979 is likely to land in this cycle, otherwise 1 as a cheap interim, since it doesn't commit the project to a new field. Let me know and I'll rework or close.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mounting a volume to a target containing a colon (:)

2 participants