Skip to content

Commit 20de850

Browse files
authored
Fix relative path bind mounts regression (#572)
## Type of Change - [x] Bug fix ## Description Resolves #565. Relative paths in type=bind mounts now resolve to absolute paths instead of being validated as volume names. Added some tests generated using gen AI for this scenario.
1 parent 2525d55 commit 20de850

File tree

2 files changed

+127
-22
lines changed

2 files changed

+127
-22
lines changed

Sources/ContainerClient/Parser.swift

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,9 @@ public struct Parser {
316316
fs.type = Filesystem.FSType.virtiofs
317317
case "tmpfs":
318318
fs.type = Filesystem.FSType.tmpfs
319+
case "volume":
320+
// Volume type will be set later in source parsing when we create the actual volume filesystem
321+
break
319322
default:
320323
throw ContainerizationError(.invalidArgument, message: "unsupported mount type \(val)")
321324
}
@@ -343,29 +346,28 @@ public struct Parser {
343346
case "source":
344347
switch type {
345348
case "virtiofs", "bind":
346-
// Check if it's an absolute directory path first
347-
if val.hasPrefix("/") {
348-
let url = URL(filePath: val)
349-
let absolutePath = url.absoluteURL.path
350-
351-
var isDirectory: ObjCBool = false
352-
guard FileManager.default.fileExists(atPath: absolutePath, isDirectory: &isDirectory) else {
353-
throw ContainerizationError(.invalidArgument, message: "path '\(val)' does not exist")
354-
}
355-
guard isDirectory.boolValue else {
356-
throw ContainerizationError(.invalidArgument, message: "path '\(val)' is not a directory")
357-
}
358-
fs.source = absolutePath
359-
} else {
360-
guard VolumeStorage.isValidVolumeName(val) else {
361-
throw ContainerizationError(.invalidArgument, message: "Invalid volume name '\(val)': must match \(VolumeStorage.volumeNamePattern)")
362-
}
363-
364-
// This is a named volume
365-
isVolume = true
366-
volumeName = val
367-
fs.source = val
349+
// For bind mounts, resolve both absolute and relative paths
350+
let url = URL(filePath: val)
351+
let absolutePath = url.absoluteURL.path
352+
353+
var isDirectory: ObjCBool = false
354+
guard FileManager.default.fileExists(atPath: absolutePath, isDirectory: &isDirectory) else {
355+
throw ContainerizationError(.invalidArgument, message: "path '\(val)' does not exist")
356+
}
357+
guard isDirectory.boolValue else {
358+
throw ContainerizationError(.invalidArgument, message: "path '\(val)' is not a directory")
368359
}
360+
fs.source = absolutePath
361+
case "volume":
362+
// For volume mounts, validate as volume name
363+
guard VolumeStorage.isValidVolumeName(val) else {
364+
throw ContainerizationError(.invalidArgument, message: "Invalid volume name '\(val)': must match \(VolumeStorage.volumeNamePattern)")
365+
}
366+
367+
// This is a named volume
368+
isVolume = true
369+
volumeName = val
370+
fs.source = val
369371
case "tmpfs":
370372
throw ContainerizationError(.invalidArgument, message: "cannot specify source for tmpfs mount")
371373
default:

Tests/ContainerClientTests/ParserTest.swift

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,107 @@ struct ParserTest {
122122
return error.description.contains("invalid publish container port")
123123
}
124124
}
125+
126+
@Test
127+
func testMountBindRelativePath() throws {
128+
129+
let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent("test-bind-\(UUID().uuidString)")
130+
try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true)
131+
defer {
132+
try? FileManager.default.removeItem(at: tempDir)
133+
}
134+
135+
let originalDir = FileManager.default.currentDirectoryPath
136+
FileManager.default.changeCurrentDirectoryPath(tempDir.path)
137+
defer {
138+
FileManager.default.changeCurrentDirectoryPath(originalDir)
139+
}
140+
141+
let result = try Parser.mount("type=bind,src=.,dst=/foo")
142+
143+
switch result {
144+
case .filesystem(let fs):
145+
let expectedPath = URL(filePath: ".").absoluteURL.path
146+
#expect(fs.source == expectedPath)
147+
#expect(fs.destination == "/foo")
148+
#expect(!fs.isVolume)
149+
case .volume:
150+
#expect(Bool(false), "Expected filesystem mount, got volume")
151+
}
152+
}
153+
154+
@Test
155+
func testMountBindAbsolutePath() throws {
156+
let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent("test-bind-abs-\(UUID().uuidString)")
157+
try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true)
158+
defer {
159+
try? FileManager.default.removeItem(at: tempDir)
160+
}
161+
162+
let result = try Parser.mount("type=bind,src=\(tempDir.path),dst=/foo")
163+
164+
switch result {
165+
case .filesystem(let fs):
166+
#expect(fs.source == tempDir.path)
167+
#expect(fs.destination == "/foo")
168+
#expect(!fs.isVolume)
169+
case .volume:
170+
#expect(Bool(false), "Expected filesystem mount, got volume")
171+
}
172+
}
173+
174+
@Test
175+
func testMountVolumeValidName() throws {
176+
let result = try Parser.mount("type=volume,src=myvolume,dst=/data")
177+
178+
switch result {
179+
case .filesystem:
180+
#expect(Bool(false), "Expected volume mount, got filesystem")
181+
case .volume(let vol):
182+
#expect(vol.name == "myvolume")
183+
#expect(vol.destination == "/data")
184+
}
185+
}
186+
187+
@Test
188+
func testMountVolumeInvalidName() throws {
189+
#expect {
190+
_ = try Parser.mount("type=volume,src=.,dst=/data")
191+
} throws: { error in
192+
guard let error = error as? ContainerizationError else {
193+
return false
194+
}
195+
return error.description.contains("Invalid volume name")
196+
}
197+
}
198+
199+
@Test
200+
func testMountBindNonExistentPath() throws {
201+
#expect {
202+
_ = try Parser.mount("type=bind,src=/nonexistent/path,dst=/foo")
203+
} throws: { error in
204+
guard let error = error as? ContainerizationError else {
205+
return false
206+
}
207+
return error.description.contains("path") && error.description.contains("does not exist")
208+
}
209+
}
210+
211+
@Test
212+
func testMountBindFileInsteadOfDirectory() throws {
213+
let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent("test-file-\(UUID().uuidString)")
214+
try "test content".write(to: tempFile, atomically: true, encoding: .utf8)
215+
defer {
216+
try? FileManager.default.removeItem(at: tempFile)
217+
}
218+
219+
#expect {
220+
_ = try Parser.mount("type=bind,src=\(tempFile.path),dst=/foo")
221+
} throws: { error in
222+
guard let error = error as? ContainerizationError else {
223+
return false
224+
}
225+
return error.description.contains("path") && error.description.contains("is not a directory")
226+
}
227+
}
125228
}

0 commit comments

Comments
 (0)