From 0106155c87ae8301d7755df9d0aa1415682d9d58 Mon Sep 17 00:00:00 2001 From: Chris George Date: Fri, 8 May 2026 15:07:30 -0700 Subject: [PATCH 1/2] Use SystemPath for SandboxRuntimeConfiguration. Converts RuntimeConfiguration.path, runtimeConfigurationPath, and the readRuntimeConfiguration(from:) parameter from URL to FilePath. Following the migration pattern from #1480 (HostDNSResolver), #1518 (PacketFilter), and discussion #1481. The two server-side callers (ContainersService, SandboxService) get minimal URL <-> FilePath bridge calls. Bundle.create still requires URL (Foundation API) so the bridge stays at that boundary. Bundle+Log.swift in the same module is intentionally NOT converted in this PR: all three of its callers use Foundation FileHandle APIs that require URL, and converting bundle.containerLog without simultaneously updating those callers (in ContainersService and SandboxService) would expand the PR scope significantly. Bundle+Log can be converted later as part of a SandboxService/ContainersService cluster conversion. --- .../Server/Containers/ContainersService.swift | 4 +-- .../RuntimeClient/RuntimeConfiguration.swift | 32 +++++++++++-------- .../RuntimeLinux/Server/RuntimeService.swift | 5 +-- .../RuntimeConfigurationTests.swift | 20 +++++++----- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index b18bf55d5..a5d4f1d93 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -376,7 +376,7 @@ public actor ContainersService { "initfs": "\(initImage ?? self.containerSystemConfig.vminit.image)", ]) let runtimeConfig = RuntimeConfiguration( - path: path, + path: FilePath(path.path(percentEncoded: false)), initialFilesystem: initFilesystem, kernel: kernel, containerConfiguration: configuration, @@ -1146,7 +1146,7 @@ public actor ContainersService { } catch { // Bundle doesn't exist or incomplete, try runtime configuration // This handles containers that were created but not started yet - let runtimeConfig = try RuntimeConfiguration.readRuntimeConfiguration(from: path) + let runtimeConfig = try RuntimeConfiguration.readRuntimeConfiguration(from: FilePath(path.path(percentEncoded: false))) guard let config = runtimeConfig.containerConfiguration else { throw ContainerizationError(.internalError, message: "runtime configuration missing container configuration") } diff --git a/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift b/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift index 29507bb70..37c8ba518 100644 --- a/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift +++ b/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift @@ -18,11 +18,12 @@ import ContainerResource import Containerization import ContainerizationError import Foundation +import SystemPackage public struct RuntimeConfiguration: Codable, Sendable { static let runtimeConfigurationFilename = "runtime-configuration.json" - public let path: URL + public let path: FilePath // TODO: Remove runtime-specific fields (initialFilesystem, kernel, containerRootFilesystem). // These should be encoded into the opaque `runtimeData` field by the CLI. public let initialFilesystem: Filesystem @@ -33,7 +34,7 @@ public struct RuntimeConfiguration: Codable, Sendable { public let runtimeData: Data? public init( - path: URL, + path: FilePath, initialFilesystem: Filesystem, kernel: Kernel, containerConfiguration: ContainerConfiguration? = nil, @@ -50,28 +51,31 @@ public struct RuntimeConfiguration: Codable, Sendable { self.runtimeData = runtimeData } - public var runtimeConfigurationPath: URL { - self.path.appendingPathComponent(Self.runtimeConfigurationFilename) + public var runtimeConfigurationPath: FilePath { + self.path.appending(Self.runtimeConfigurationFilename) } public func writeRuntimeConfiguration() throws { // Ensure the parent directory exists - let directory = self.runtimeConfigurationPath.deletingLastPathComponent() - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: self.path.string, withIntermediateDirectories: true) let data = try JSONEncoder().encode(self) - try data.write(to: self.runtimeConfigurationPath) + try data.write(to: URL(fileURLWithPath: self.runtimeConfigurationPath.string)) } - public static func readRuntimeConfiguration(from runtimeConfigurationPath: URL) throws -> RuntimeConfiguration { - let configurationPath = runtimeConfigurationPath.appendingPathComponent(RuntimeConfiguration.runtimeConfigurationFilename) - let data: Data - do { - data = try Data(contentsOf: configurationPath) - } catch { + public static func readRuntimeConfiguration(from runtimeConfigurationPath: FilePath) throws -> RuntimeConfiguration { + let configurationPath = runtimeConfigurationPath.appending(RuntimeConfiguration.runtimeConfigurationFilename) + guard FileManager.default.fileExists(atPath: configurationPath.string) else { throw ContainerizationError( .notFound, - message: "runtime configuration file not found at path: \(configurationPath.path)" + message: "runtime configuration file not found at path: \(configurationPath.string)" + ) + } + + guard let data = FileManager.default.contents(atPath: configurationPath.string) else { + throw ContainerizationError( + .internalError, + message: "failed to read runtime configuration file at path: \(configurationPath.string)" ) } return try JSONDecoder().decode(RuntimeConfiguration.self, from: data) diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index 187de7af6..0402fc10b 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -1534,9 +1534,10 @@ extension RuntimeService { /// Create bundle from RuntimeConfiguration private func createBundle() throws { do { - let runtimeConfig = try RuntimeConfiguration.readRuntimeConfiguration(from: self.root) + let runtimeConfig = try RuntimeConfiguration.readRuntimeConfiguration( + from: FilePath(self.root.path(percentEncoded: false))) _ = try ContainerResource.Bundle.create( - path: runtimeConfig.path, + path: URL(fileURLWithPath: runtimeConfig.path.string), initialFilesystem: runtimeConfig.initialFilesystem, kernel: runtimeConfig.kernel, containerConfiguration: runtimeConfig.containerConfiguration, diff --git a/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift b/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift index a1529d311..b2de7f1e6 100644 --- a/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift +++ b/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift @@ -19,6 +19,7 @@ import ContainerRuntimeClient import ContainerRuntimeLinuxClient import Containerization import Foundation +import SystemPackage import Testing /// Unit tests for RuntimeConfiguration functionality. @@ -31,8 +32,9 @@ struct RuntimeConfigurationTests { /// appropriate error @Test func testReadNonExistentRuntimeConfiguration() throws { - let tempDir = FileManager.default.temporaryDirectory - let nonExistentPath = tempDir.appendingPathComponent("non-existent-\(UUID()).json") + let tempURL = FileManager.default.temporaryDirectory + let nonExistentPath = FilePath(tempURL.path(percentEncoded: false)) + .appending("non-existent-\(UUID()).json") #expect(throws: Error.self) { _ = try RuntimeConfiguration.readRuntimeConfiguration(from: nonExistentPath) @@ -42,11 +44,12 @@ struct RuntimeConfigurationTests { /// Test that runtime configuration reads and writes as expected @Test func testRuntimeConfigurationReadWrite() throws { - let tempDir = FileManager.default.temporaryDirectory - let bundlePath = tempDir.appendingPathComponent("test-bundle-\(UUID())") + let bundleURL = FileManager.default.temporaryDirectory + .appendingPathComponent("test-bundle-\(UUID())") + let bundlePath = FilePath(bundleURL.path(percentEncoded: false)) defer { - try? FileManager.default.removeItem(at: bundlePath) + try? FileManager.default.removeItem(at: bundleURL) } let initFs = Filesystem.virtiofs( @@ -95,11 +98,12 @@ struct RuntimeConfigurationTests { @Test func testRuntimeConfigurationWithVariant() throws { - let tempDir = FileManager.default.temporaryDirectory - let bundlePath = tempDir.appendingPathComponent("test-bundle-\(UUID())") + let bundleURL = FileManager.default.temporaryDirectory + .appendingPathComponent("test-bundle-\(UUID())") + let bundlePath = FilePath(bundleURL.path(percentEncoded: false)) defer { - try? FileManager.default.removeItem(at: bundlePath) + try? FileManager.default.removeItem(at: bundleURL) } let initFs = Filesystem.virtiofs( From cc4be537c615a775689a202c2117b8be24adf671 Mon Sep 17 00:00:00 2001 From: Chris George Date: Fri, 8 May 2026 15:25:11 -0700 Subject: [PATCH 2/2] RuntimeConfiguration: explicit Codable for path; restore Data(contentsOf:). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the URL → FilePath conversion of RuntimeConfiguration.path: 1. FilePath's default Codable encoding exposes its internal _storage as a keyed container, while URL's default encoding produced a plain absoluteString. Without this fix, runtime-configuration.json files written by daemons before the migration would fail to decode after upgrade. Add explicit CodingKeys + encode(to:) / init(from:) that serialize path as a plain string and accept either form (file:// or bare path) on decode for backward compatibility. Add a regression test for the legacy URL format. 2. Restore Data(contentsOf: URL(fileURLWithPath:)) for reading the config file, matching the codebase precedent (Bundle.swift, ConfigurationLoader.swift, EntityStore.swift). FileManager.contents would have lost typed CocoaError diagnostics on I/O failure. --- .../RuntimeClient/RuntimeConfiguration.swift | 51 ++++++++++++++++--- .../RuntimeConfigurationTests.swift | 24 +++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift b/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift index 37c8ba518..8b76189a8 100644 --- a/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift +++ b/Sources/Services/Runtime/RuntimeClient/RuntimeConfiguration.swift @@ -51,6 +51,50 @@ public struct RuntimeConfiguration: Codable, Sendable { self.runtimeData = runtimeData } + private enum CodingKeys: String, CodingKey { + case path + case initialFilesystem + case kernel + case containerConfiguration + case containerRootFilesystem + case options + case runtimeData + } + + // FilePath's default Codable encoding exposes its internal _storage and + // is not interchangeable with URL's plain-string form. To stay + // wire-compatible with runtime-configuration.json files written before + // the URL → FilePath migration, encode `path` as a plain string and + // accept either the file:// URL form or a bare path on decode. + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let pathString = try container.decode(String.self, forKey: .path) + if pathString.hasPrefix("file://"), + let url = URL(string: pathString), url.isFileURL + { + self.path = FilePath(url.path(percentEncoded: false)) + } else { + self.path = FilePath(pathString) + } + self.initialFilesystem = try container.decode(Filesystem.self, forKey: .initialFilesystem) + self.kernel = try container.decode(Kernel.self, forKey: .kernel) + self.containerConfiguration = try container.decodeIfPresent(ContainerConfiguration.self, forKey: .containerConfiguration) + self.containerRootFilesystem = try container.decodeIfPresent(Filesystem.self, forKey: .containerRootFilesystem) + self.options = try container.decodeIfPresent(ContainerCreateOptions.self, forKey: .options) + self.runtimeData = try container.decodeIfPresent(Data.self, forKey: .runtimeData) + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(self.path.string, forKey: .path) + try container.encode(self.initialFilesystem, forKey: .initialFilesystem) + try container.encode(self.kernel, forKey: .kernel) + try container.encodeIfPresent(self.containerConfiguration, forKey: .containerConfiguration) + try container.encodeIfPresent(self.containerRootFilesystem, forKey: .containerRootFilesystem) + try container.encodeIfPresent(self.options, forKey: .options) + try container.encodeIfPresent(self.runtimeData, forKey: .runtimeData) + } + public var runtimeConfigurationPath: FilePath { self.path.appending(Self.runtimeConfigurationFilename) } @@ -72,12 +116,7 @@ public struct RuntimeConfiguration: Codable, Sendable { ) } - guard let data = FileManager.default.contents(atPath: configurationPath.string) else { - throw ContainerizationError( - .internalError, - message: "failed to read runtime configuration file at path: \(configurationPath.string)" - ) - } + let data = try Data(contentsOf: URL(fileURLWithPath: configurationPath.string)) return try JSONDecoder().decode(RuntimeConfiguration.self, from: data) } } diff --git a/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift b/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift index b2de7f1e6..f2c39eb86 100644 --- a/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift +++ b/Tests/ContainerAPIServiceTests/RuntimeConfigurationTests.swift @@ -136,4 +136,28 @@ struct RuntimeConfigurationTests { let decodedData = try JSONDecoder().decode(LinuxRuntimeData.self, from: readRuntimeConfig.runtimeData!) #expect(decodedData.variant == "test-variant", "Variant should round-trip through RuntimeConfiguration") } + + /// Verify that runtime-configuration.json files written before the + /// URL → FilePath migration (where `path` was a URL absoluteString + /// like "file:///foo/bar") still decode correctly. Otherwise an upgrade + /// would render existing containers unstartable. + @Test + func testRuntimeConfigurationDecodesLegacyURLPathFormat() throws { + let kernel = Kernel(path: URL(fileURLWithPath: "/path/to/kernel"), platform: .linuxArm) + let initFs = Filesystem.virtiofs(source: "/path/to/initfs", destination: "/", options: ["ro"]) + + let kernelJSON = try String(data: JSONEncoder().encode(kernel), encoding: .utf8) ?? "" + let initFsJSON = try String(data: JSONEncoder().encode(initFs), encoding: .utf8) ?? "" + + let legacyJSON = """ + {"path":"file:///tmp/legacy-bundle","initialFilesystem":\(initFsJSON),"kernel":\(kernelJSON)} + """ + let data = Data(legacyJSON.utf8) + + let decoded = try JSONDecoder().decode(RuntimeConfiguration.self, from: data) + + #expect(decoded.path == FilePath("/tmp/legacy-bundle")) + #expect(decoded.kernel.path == kernel.path) + #expect(decoded.initialFilesystem.source == initFs.source) + } }