From d2d9501ef7ea33ab8cdce79a607295355aeb10d8 Mon Sep 17 00:00:00 2001 From: Jarek Krochmalski Date: Sun, 6 Sep 2026 17:03:47 +0530 Subject: [PATCH] normalize mount targets so trailing slashes don't cause duplicate mounts --- src/lib/server/mount-dedupe.ts | 34 +++++++++----- tests/mount-dedupe.test.ts | 81 ++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 12 deletions(-) diff --git a/src/lib/server/mount-dedupe.ts b/src/lib/server/mount-dedupe.ts index ca56790..c05dfcc 100644 --- a/src/lib/server/mount-dedupe.ts +++ b/src/lib/server/mount-dedupe.ts @@ -10,10 +10,23 @@ type InspectMountLike = { Destination?: string | null; }; -/** Destination path of a bind string ("src:/dst:ro" -> "/dst", "/dst" -> "/dst"). */ +/** + * Canonical form of a mount target for comparison. Docker treats "/var/log/" and + * "/var/log" as the SAME mount point and rejects a create carrying both ("Duplicate + * mount point"), but it normalizes a mount's Destination in inspect (strips the trailing + * slash) while leaving the user's Binds string as typed. So a user bind "vol:/var/log/" + * and the same volume's inspect Destination "/var/log" must compare equal here, or dedup + * misses the collision and recreate sends both (#583). Strip trailing slashes; keep root "/". + */ +export function normalizeMountTarget(target: string): string { + const stripped = target.replace(/\/+$/, ''); + return stripped === '' ? '/' : stripped; +} + +/** Destination path of a bind string ("src:/dst:ro" -> "/dst", "/dst" -> "/dst"), normalized. */ function bindTarget(bind: string): string { const parts = bind.split(':'); - return parts.length >= 2 ? parts[1] : parts[0]; + return normalizeMountTarget(parts.length >= 2 ? parts[1] : parts[0]); } /** @@ -32,15 +45,15 @@ export function dedupeVolumesForRecreate( ): Record | undefined { if (!volumes) return undefined; const mounted = new Set(); - for (const p of Object.keys(hostConfig.Tmpfs || {})) mounted.add(p); + for (const p of Object.keys(hostConfig.Tmpfs || {})) mounted.add(normalizeMountTarget(p)); for (const b of hostConfig.Binds || []) mounted.add(bindTarget(b)); for (const b of additionalBinds) mounted.add(bindTarget(b)); - for (const m of hostConfig.Mounts || []) { if (m?.Target) mounted.add(m.Target); } - for (const m of mounts || []) { if (m.Destination) mounted.add(m.Destination); } + for (const m of hostConfig.Mounts || []) { if (m?.Target) mounted.add(normalizeMountTarget(m.Target)); } + for (const m of mounts || []) { if (m.Destination) mounted.add(normalizeMountTarget(m.Destination)); } const kept: Record = {}; for (const [path, val] of Object.entries(volumes)) { - if (!mounted.has(path)) kept[path] = val; + if (!mounted.has(normalizeMountTarget(path))) kept[path] = val; } return Object.keys(kept).length > 0 ? kept : undefined; } @@ -50,19 +63,16 @@ export function getAdditionalVolumeBinds( hostConfig: HostConfigLike, mounts: InspectMountLike[] ): string[] { - const existingMountTargets = new Set((hostConfig.Binds || []).map((bind: string) => { - const parts = bind.split(':'); - return parts.length >= 2 ? parts[1] : parts[0]; - })); + const existingMountTargets = new Set((hostConfig.Binds || []).map(bindTarget)); for (const mount of hostConfig.Mounts || []) { - if (mount?.Target) existingMountTargets.add(mount.Target); + if (mount?.Target) existingMountTargets.add(normalizeMountTarget(mount.Target)); } const additionalBinds: string[] = []; for (const mount of mounts || []) { if (mount.Type === 'volume' && mount.Name && mount.Destination) { - if (!existingMountTargets.has(mount.Destination)) { + if (!existingMountTargets.has(normalizeMountTarget(mount.Destination))) { additionalBinds.push(`${mount.Name}:${mount.Destination}`); } } diff --git a/tests/mount-dedupe.test.ts b/tests/mount-dedupe.test.ts index e618ab7..91a4f31 100644 --- a/tests/mount-dedupe.test.ts +++ b/tests/mount-dedupe.test.ts @@ -195,3 +195,84 @@ describe('dedupeVolumesForRecreate', () => { assert.equal(dedupeVolumesForRecreate({}, { Binds: [] }, [], []), undefined); }); }); + +describe('#583 trailing-slash mount targets (Duplicate mount point)', () => { + // The user bind carries a trailing slash ("caddy-logs:/var/log/") but Docker + // normalizes the same volume's inspect Destination to "/var/log". Recreate must + // treat them as the same mount point, or it re-adds the volume and the daemon + // rejects the create with "Duplicate mount point: /var/log". + it('getAdditionalVolumeBinds does NOT re-add a volume already bound with a trailing slash', () => { + const additionalBinds = getAdditionalVolumeBinds( + { Binds: ['caddy-logs:/var/log/'] }, + [{ Type: 'volume', Name: 'caddy-logs', Destination: '/var/log' }] + ); + assert.deepEqual(additionalBinds, []); + }); + + it('getAdditionalVolumeBinds handles the inverse (bind no slash, inspect Destination with slash)', () => { + const additionalBinds = getAdditionalVolumeBinds( + { Binds: ['caddy-logs:/var/log'] }, + [{ Type: 'volume', Name: 'caddy-logs', Destination: '/var/log/' }] + ); + assert.deepEqual(additionalBinds, []); + }); + + it('getAdditionalVolumeBinds still re-adds a genuinely different, unmounted volume', () => { + const additionalBinds = getAdditionalVolumeBinds( + { Binds: ['caddy-logs:/var/log/'] }, + [ + { Type: 'volume', Name: 'caddy-logs', Destination: '/var/log' }, + { Type: 'volume', Name: 'caddy-data', Destination: '/data' } + ] + ); + assert.deepEqual(additionalBinds, ['caddy-data:/data']); + }); + + it('dedupeVolumesForRecreate drops a Config.Volumes entry already bound with a trailing-slash difference', () => { + const kept = dedupeVolumesForRecreate( + { '/var/log': {} }, + { Binds: ['caddy-logs:/var/log/'] }, + [{ Type: 'volume', Name: 'caddy-logs', Destination: '/var/log' }], + [] + ); + assert.equal(kept, undefined); + }); + + it('dedupeVolumesForRecreate drops a trailing-slash Config.Volumes key against a no-slash mount', () => { + const kept = dedupeVolumesForRecreate( + { '/var/log/': {} }, + { Binds: [] }, + [{ Type: 'volume', Name: 'caddy-logs', Destination: '/var/log' }], + [] + ); + assert.equal(kept, undefined); + }); + + it('collapses MULTIPLE trailing slashes (pins the /+$ quantifier)', () => { + // A regression narrowing normalizeMountTarget to a single-slash strip would keep + // every other test green but re-add this volume -> duplicate mount. + const additionalBinds = getAdditionalVolumeBinds( + { Binds: ['caddy-logs:/var/log//'] }, + [{ Type: 'volume', Name: 'caddy-logs', Destination: '/var/log' }] + ); + assert.deepEqual(additionalBinds, []); + + const kept = dedupeVolumesForRecreate( + { '/var/log//': {} }, + { Binds: [] }, + [{ Type: 'volume', Name: 'caddy-logs', Destination: '/var/log' }], + [] + ); + assert.equal(kept, undefined); + }); + + it('root "/" is preserved (not stripped to empty)', () => { + const kept = dedupeVolumesForRecreate( + { '/': {} }, + { Binds: ['somevol:/'] }, + [], + [] + ); + assert.equal(kept, undefined); + }); +});