diff --git a/Sources/MockerKit/Compose/ComposeOrchestrator.swift b/Sources/MockerKit/Compose/ComposeOrchestrator.swift index 5929bd3..7af4f23 100644 --- a/Sources/MockerKit/Compose/ComposeOrchestrator.swift +++ b/Sources/MockerKit/Compose/ComposeOrchestrator.swift @@ -81,6 +81,21 @@ public actor ComposeOrchestrator { } } + // Named volumes are bind-mounted from their backing directory, so both an + // unusable name and a missing external volume have to fail here — before any + // service starts — rather than as a raw runtime path error half-way through. + if !composeFile.volumes.isEmpty { + let existing = Set(await volumeManager.list().map(\.name)) + for volume in composeFile.volumes.values { + let name = volume.runtimeName(projectName: projectName) + _ = try volumeManager.mountpoint(name) + guard !volume.external || existing.contains(name) else { + throw MockerError.operationFailed( + "volume \(name) declared as external, but could not be found") + } + } + } + // Create networks for (fullName, driver) in Self.networksToCreate(composeFile: composeFile, projectName: projectName) { do { @@ -523,7 +538,15 @@ public actor ComposeOrchestrator { // Parse port mappings let ports = try service.ports.map { try PortMapping.parse($0) } - let volumes = try Self.resolveVolumeMounts(service.volumes, projectDir: projectDir) + // Named volumes bind-mount their backing directory, exactly as Docker does + // internally, so the data survives container recreation. + let volumes = try Self.resolveVolumeMounts( + service.volumes, + projectDir: projectDir, + namedVolumeSources: composeFile.volumes.mapValues { + try volumeManager.mountpoint($0.runtimeName(projectName: projectName)) + } + ) let config = ContainerConfig( name: containerName, @@ -563,32 +586,38 @@ public actor ComposeOrchestrator { /// Resolve volume spec strings from a compose service into `VolumeMount` values. /// - /// Bind-mount host paths (absolute or relative) are included; anonymous volumes - /// (container paths only) are included; named volumes (bare names without path - /// separators) are skipped — Apple's virtiofs doesn't support chown from within - /// containers, which breaks images like postgres that chown their data directory - /// on init. + /// Bind-mount host paths and anonymous volumes (container paths only) are + /// included as-is. A name declared in the file's top-level `volumes:` section is + /// bind-mounted from its backing directory (`namedVolumeSources`), which is what + /// keeps the data alive across `compose up --force-recreate`; an undeclared bare + /// name has no backing directory and is dropped. /// /// Relative paths (`./foo`, `../bar`, `data/dir`) are resolved to absolute paths /// against `projectDir` (the Compose `--project-directory`, i.e. the directory /// containing the compose file unless overridden). This matches Docker Compose /// behaviour, where bind-mount sources are anchored to the project directory /// rather than the process's current working directory. - static func resolveVolumeMounts(_ volSpecs: [String], projectDir: URL) throws -> [VolumeMount] { + static func resolveVolumeMounts( + _ volSpecs: [String], + projectDir: URL, + namedVolumeSources: [String: String] + ) throws -> [VolumeMount] { var volumes: [VolumeMount] = [] for volSpec in volSpecs { var mount = try VolumeMount.parse(volSpec) - if mount.source.isEmpty { - volumes.append(mount) - } else if mount.source.hasPrefix("/") { + if mount.source.isEmpty || mount.source.hasPrefix("/") { volumes.append(mount) } else if mount.source.hasPrefix("~") { mount.source = (mount.source as NSString).expandingTildeInPath volumes.append(mount) } else if mount.source.hasPrefix(".") || mount.source.contains("/") { + // Relative bind mount, anchored to the project directory. mount.source = projectDir.appendingPathComponent(mount.source).standardized.path volumes.append(mount) + } else if let source = namedVolumeSources[mount.source] { + mount.source = source + volumes.append(mount) } } return volumes diff --git a/Sources/MockerKit/Volume/VolumeManager.swift b/Sources/MockerKit/Volume/VolumeManager.swift index 6728454..8c8de7f 100644 --- a/Sources/MockerKit/Volume/VolumeManager.swift +++ b/Sources/MockerKit/Volume/VolumeManager.swift @@ -28,14 +28,23 @@ public actor VolumeManager { } } + /// Backing directory holding a volume's data. Compose bind-mounts it so named + /// volumes survive container recreation. + public nonisolated func mountpoint(_ name: String) throws -> String { + try Self.validateName(name) + return "\(storagePath)/\(name)/_data" + } + /// Reject names that would escape the volumes directory. Every volume path is /// built by interpolating the name, and a compose file can supply it verbatim /// (`volumes: {data: {name: ...}}`), so `../` must never get through. static func validateName(_ name: String) throws { - // Only path escape is rejected — anything else stays removable, including - // volumes created before this check existed. + // Path escape and `:` are rejected — the latter because the backing directory + // goes into a `-v source:destination` argument, where it would misparse. + // Anything else stays removable, including volumes created before this check. let valid = !name.isEmpty && !name.contains("/") + && !name.contains(":") && name != "." && name != ".." guard valid else { @@ -45,12 +54,11 @@ public actor VolumeManager { /// Create a new volume. public func create(name: String, driver: String = "local", labels: [String: String] = [:]) throws -> VolumeInfo { - try Self.validateName(name) guard volumes[name] == nil else { throw MockerError.operationFailed("Volume \(name) already exists") } - let mountpoint = "\(config.volumesPath)/\(name)/_data" + let mountpoint = try mountpoint(name) let fm = FileManager.default if !fm.fileExists(atPath: mountpoint) { try fm.createDirectory(atPath: mountpoint, withIntermediateDirectories: true) diff --git a/Tests/MockerKitTests/ComposeOrchestratorTests.swift b/Tests/MockerKitTests/ComposeOrchestratorTests.swift index d5ff77f..f393ddb 100644 --- a/Tests/MockerKitTests/ComposeOrchestratorTests.swift +++ b/Tests/MockerKitTests/ComposeOrchestratorTests.swift @@ -161,7 +161,7 @@ struct ComposeOrchestratorTests { @Test("Absolute bind mount included as-is") func resolveAbsoluteBindMount() throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts(["/host/data:/container/data"], projectDir: Self.cwd) + let mounts = try Self.resolve(["/host/data:/container/data"]) #expect(mounts.count == 1) #expect(mounts[0].source == "/host/data") #expect(mounts[0].destination == "/container/data") @@ -176,22 +176,30 @@ struct ComposeOrchestratorTests { ] ) func resolveRelativeStyleMounts(spec: String, suffix: String, destination: String) throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts([spec], projectDir: Self.cwd) + let mounts = try Self.resolve([spec]) #expect(mounts.count == 1) #expect(mounts[0].source.hasPrefix("/")) #expect(mounts[0].source.hasSuffix(suffix)) #expect(mounts[0].destination == destination) } - @Test("Named volume skipped") - func skipNamedVolume() throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts(["mydata:/container/data"], projectDir: Self.cwd) + @Test("Declared named volume resolved to its backing directory") + func resolveDeclaredNamedVolume() throws { + let mounts = try Self.resolve(["mydata:/container/data"], named: ["mydata": "/volumes/proj-mydata/_data"]) + #expect(mounts.count == 1) + #expect(mounts[0].source == "/volumes/proj-mydata/_data") + #expect(mounts[0].destination == "/container/data") + } + + @Test("Undeclared bare name is dropped") + func dropUndeclaredName() throws { + let mounts = try Self.resolve(["mydata:/container/data"]) #expect(mounts.isEmpty) } @Test("Anonymous volume included") func includeAnonymousVolume() throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts(["/container/data"], projectDir: Self.cwd) + let mounts = try Self.resolve(["/container/data"]) #expect(mounts.count == 1) #expect(mounts[0].source == "") #expect(mounts[0].destination == "/container/data") @@ -199,16 +207,17 @@ struct ComposeOrchestratorTests { @Test("Mix of bind mounts, named volumes, and anonymous volumes") func resolveMixedVolumes() throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts([ + let mounts = try Self.resolve([ "/abs/path:/app/data", "./relative:/app/rel", "namedvol:/app/named", "/app/anon", "sub/dir:/app/sub", - ], projectDir: Self.cwd) - #expect(mounts.count == 4) // namedvol skipped + ], named: ["namedvol": "/volumes/proj-namedvol/_data"]) + #expect(mounts.count == 5) let sources = mounts.map(\.source) #expect(sources.contains("/abs/path")) + #expect(sources.contains("/volumes/proj-namedvol/_data")) #expect(sources.contains("")) // anonymous #expect(sources.contains(where: { $0.hasSuffix("/relative") })) #expect(sources.contains(where: { $0.hasSuffix("sub/dir") })) @@ -216,7 +225,7 @@ struct ComposeOrchestratorTests { @Test("Read-only relative bind mount preserves ro flag") func resolveRelativeReadOnly() throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts(["./data:/container/data:ro"], projectDir: Self.cwd) + let mounts = try Self.resolve(["./data:/container/data:ro"]) #expect(mounts.count == 1) #expect(mounts[0].readOnly == true) #expect(mounts[0].destination == "/container/data") @@ -225,13 +234,26 @@ struct ComposeOrchestratorTests { @Test("Home-relative path resolved") func resolveHomeRelativePath() throws { - let mounts = try ComposeOrchestrator.resolveVolumeMounts(["~/data:/container/data"], projectDir: Self.cwd) + let mounts = try Self.resolve(["~/data:/container/data"]) #expect(mounts.count == 1) #expect(mounts[0].source.hasPrefix("/")) #expect(mounts[0].source.contains("data")) #expect(mounts[0].destination == "/container/data") } + /// Resolve specs against the test project directory, with named volumes already + /// mapped to their backing directories. + private static func resolve( + _ specs: [String], + named: [String: String] = [:] + ) throws -> [VolumeMount] { + try ComposeOrchestrator.resolveVolumeMounts( + specs, + projectDir: Self.cwd, + namedVolumeSources: named + ) + } + // MARK: - reconcileDecision (issue #59) private func singleServiceFile() throws -> ComposeFile { @@ -488,7 +510,8 @@ struct ComposeOrchestratorTests { let projectDir = URL(fileURLWithPath: "/tmp/mocker-issue-60-project") let mounts = try ComposeOrchestrator.resolveVolumeMounts( ["./data:/container/data"], - projectDir: projectDir + projectDir: projectDir, + namedVolumeSources: [:] ) #expect(mounts.count == 1) #expect(mounts[0].source == "/tmp/mocker-issue-60-project/data") @@ -500,7 +523,8 @@ struct ComposeOrchestratorTests { let projectDir = URL(fileURLWithPath: "/tmp/mocker-issue-60-project/nested") let mounts = try ComposeOrchestrator.resolveVolumeMounts( ["../shared:/container/shared"], - projectDir: projectDir + projectDir: projectDir, + namedVolumeSources: [:] ) #expect(mounts.count == 1) #expect(mounts[0].source == "/tmp/mocker-issue-60-project/shared") diff --git a/Tests/MockerKitTests/ComposeVolumeLifecycleTests.swift b/Tests/MockerKitTests/ComposeVolumeLifecycleTests.swift index c4b6da3..a0964ab 100644 --- a/Tests/MockerKitTests/ComposeVolumeLifecycleTests.swift +++ b/Tests/MockerKitTests/ComposeVolumeLifecycleTests.swift @@ -94,7 +94,7 @@ struct ComposeVolumeLifecycleTests { } @Test("Volume names that would escape the volumes directory are rejected", arguments: [ - "../etc", "a/b", "..", ".", "", + "../etc", "a/b", "..", ".", "", "host:path", ]) func rejectsEscapingVolumeNames(name: String) { #expect(throws: MockerError.self) {