From 277c96cbea4efbd15ab5e5e798352b8148adc12e Mon Sep 17 00:00:00 2001 From: Dmitry Kovba Date: Tue, 26 May 2026 12:56:44 -0700 Subject: [PATCH 01/15] Bump containerization to `0.33.1` (#1601) --- Package.resolved | 6 +++--- Package.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Package.resolved b/Package.resolved index faba9d8b6..6d19fb55d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "33002bd04671ab82772f00d444616e9876ce990001e53950f58ea42fa332e493", + "originHash" : "d9d032f8b1ad94cf9006bd4b441ce1dad52802b04b1d7f7b30baedd32cee0e8b", "pins" : [ { "identity" : "async-http-client", @@ -15,8 +15,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/containerization.git", "state" : { - "revision" : "db5b5b98405d53543f69105087130ffd623a5b9a", - "version" : "0.32.2" + "revision" : "9205a766302a18e06a0ce43f8b7e483625e3e50f", + "version" : "0.33.1" } }, { diff --git a/Package.swift b/Package.swift index 442aa8b9a..f453ea5d1 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ import PackageDescription let releaseVersion = ProcessInfo.processInfo.environment["RELEASE_VERSION"] ?? "0.0.0" let gitCommit = ProcessInfo.processInfo.environment["GIT_COMMIT"] ?? "unspecified" let builderShimVersion = "0.12.0" -let scVersion = "0.32.2" +let scVersion = "0.33.1" let package = Package( name: "container", From ca3a4d66708bcc517435e9ab46fd8fd529a1d014 Mon Sep 17 00:00:00 2001 From: J Logan Date: Tue, 26 May 2026 16:58:55 -0400 Subject: [PATCH 02/15] Use FilePath for app, install, and log roots. (#1558) - Closes #1557. - Replaces `executableURL` utility function for getting app executable path with `executablePath`. - Adds `FilePath.resolvingSymlinks()` extension. - Also converts for FilePath for types in `ContainerVersion` target. --- Package.swift | 10 ++- Sources/APIServer/APIServer+Start.swift | 50 ++++++----- Sources/ContainerCommands/Application.swift | 25 +++--- .../ContainerCommands/DefaultCommand.swift | 23 ++--- .../System/SystemStart.swift | 45 +++++----- .../FilePath+Symlinks.swift | 39 +++++++++ Sources/ContainerPersistence/PathUtils.swift | 9 +- Sources/ContainerPlugin/ApplicationRoot.swift | 31 +++++-- .../ContainerPlugin/FilePath+Resolve.swift | 48 +++++++++++ Sources/ContainerPlugin/InstallRoot.swift | 31 +++++-- Sources/ContainerPlugin/LogRoot.swift | 20 ++--- .../ContainerVersion/Bundle+AppBundle.swift | 35 +++++--- .../CommandLine+Executable.swift | 9 +- Sources/ContainerVersion/ReleaseVersion.swift | 2 +- Sources/Plugins/CoreImages/ImagesHelper.swift | 19 +++-- .../FilePathSymlinksTests.swift | 51 +++++++++++ .../FilePath+ResolveTests.swift | 74 ++++++++++++++++ .../ContainerPluginTests/RootPathTests.swift | 47 +++++++++++ .../Bundle+AppBundleTests.swift | 84 +++++++++++++++++++ .../CommandLine+ExecutableTests.swift} | 25 ++++-- 20 files changed, 549 insertions(+), 128 deletions(-) create mode 100644 Sources/ContainerPersistence/FilePath+Symlinks.swift create mode 100644 Sources/ContainerPlugin/FilePath+Resolve.swift create mode 100644 Tests/ContainerPersistenceTests/FilePathSymlinksTests.swift create mode 100644 Tests/ContainerPluginTests/FilePath+ResolveTests.swift create mode 100644 Tests/ContainerPluginTests/RootPathTests.swift create mode 100644 Tests/ContainerVersionTests/Bundle+AppBundleTests.swift rename Tests/{ContainerPluginTests/CommandLine+ExecutableTest.swift => ContainerVersionTests/CommandLine+ExecutableTests.swift} (53%) diff --git a/Package.swift b/Package.swift index f453ea5d1..2cc2e7092 100644 --- a/Package.swift +++ b/Package.swift @@ -520,9 +520,17 @@ let package = Package( .target( name: "ContainerVersion", dependencies: [ - "CVersion" + .product(name: "SystemPackage", package: "swift-system"), + "CVersion", ], ), + .testTarget( + name: "ContainerVersionTests", + dependencies: [ + .product(name: "SystemPackage", package: "swift-system"), + "ContainerVersion", + ] + ), .target( name: "CVersion", dependencies: [], diff --git a/Sources/APIServer/APIServer+Start.swift b/Sources/APIServer/APIServer+Start.swift index 527839153..ada055aae 100644 --- a/Sources/APIServer/APIServer+Start.swift +++ b/Sources/APIServer/APIServer+Start.swift @@ -43,16 +43,16 @@ extension APIServer { @Flag(name: .long, help: "Enable debug logging") var debug = false - var appRoot = ApplicationRoot.url + var appRoot = ApplicationRoot.path - var installRoot = InstallRoot.url + var installRoot = InstallRoot.path var logRoot = LogRoot.path func run() async throws { let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() let commandName = APIServer._commandName - let logPath = logRoot.map { $0.appending("\(commandName).log") } + let logPath = logRoot.map { $0.appending(FilePath.Component("\(commandName).log") ?? "unknown") } let log = ServiceLogger.bootstrap(category: "APIServer", debug: debug, logPath: logPath) log.info("starting helper", metadata: ["name": "\(commandName)"]) defer { @@ -179,22 +179,24 @@ extension APIServer { log.info( "initializing plugin loader", metadata: [ - "installRoot": "\(installRoot.path(percentEncoded: false))" + "installRoot": "\(installRoot.string)" ]) - let pluginsURL = PluginLoader.userPluginsDir(installRoot: installRoot) + // TODO: Remove when we convert PluginLoader to FilePath + let installRootURL = URL(fileURLWithPath: installRoot.string) + let pluginsURL = PluginLoader.userPluginsDir(installRoot: installRootURL) log.info("detecting user plugins directory", metadata: ["path": "\(pluginsURL.path(percentEncoded: false))"]) var directoryExists: ObjCBool = false _ = FileManager.default.fileExists(atPath: pluginsURL.path, isDirectory: &directoryExists) let userPluginsURL = directoryExists.boolValue ? pluginsURL : nil // plugins built into the application installed as a Unix-like application - let installRootPluginsURL = + let installRootPluginsPath = installRoot - .appendingPathComponent("libexec") - .appendingPathComponent("container") - .appendingPathComponent("plugins") - .standardized + .appending(FilePath.Component("libexec")) + .appending(FilePath.Component("container")) + .appending(FilePath.Component("plugins")) + let installRootPluginsURL = URL(fileURLWithPath: installRootPluginsPath.string) let pluginDirectories = [ userPluginsURL, @@ -210,9 +212,10 @@ extension APIServer { log.info("discovered plugin directory", metadata: ["path": "\(pluginDirectory.path(percentEncoded: false))"]) } + let appRootURL = URL(fileURLWithPath: appRoot.string) return try PluginLoader( - appRoot: appRoot, - installRoot: installRoot, + appRoot: appRootURL, + installRoot: installRootURL, logRoot: logRoot, pluginDirectories: pluginDirectories, pluginFactories: pluginFactories, @@ -245,9 +248,12 @@ extension APIServer { private func initializeHealthCheckService(log: Logger, routes: inout [XPCRoute: XPCServer.RouteHandler]) { log.info("initializing health check service") + // TODO: Remove when we convert HealthCheckHarness to FilePath + let installRootURL = URL(fileURLWithPath: installRoot.string) + let appRootURL = URL(fileURLWithPath: appRoot.string) let svc = HealthCheckHarness( - appRoot: appRoot, - installRoot: installRoot, + appRoot: appRootURL, + installRoot: installRootURL, logRoot: logRoot, log: log ) @@ -257,7 +263,9 @@ extension APIServer { private func initializeKernelService(log: Logger, routes: inout [XPCRoute: XPCServer.RouteHandler]) throws { log.info("initializing kernel service") - let svc = try KernelService(log: log, appRoot: appRoot) + // TODO: Remove when we convert KernelService to FilePath + let appRootURL = URL(fileURLWithPath: appRoot.string) + let svc = try KernelService(log: log, appRoot: appRootURL) let harness = KernelHarness(service: svc, log: log) routes[XPCRoute.installKernel] = XPCServer.route(harness.install) routes[XPCRoute.getDefaultKernel] = XPCServer.route(harness.getDefaultKernel) @@ -271,8 +279,10 @@ extension APIServer { ) throws -> ContainersService { log.info("initializing containers service") + // TODO: Remove when we convert ContainersService to FilePath + let appRootURL = URL(fileURLWithPath: appRoot.string) let service = try ContainersService( - appRoot: appRoot, + appRoot: appRootURL, pluginLoader: pluginLoader, containerSystemConfig: containerSystemConfig, log: log, @@ -310,9 +320,7 @@ extension APIServer { ) async throws -> NetworksService { log.info("initializing networks service") - // TODO: This goes away when we convert our roots to FilePath - let appPath = FilePath(appRoot.absolutePath()) - let resourceRoot = appPath.appending("networks") + let resourceRoot = appRoot.appending(FilePath.Component("networks")) let service = try await NetworksService( pluginLoader: pluginLoader, resourceRoot: resourceRoot, @@ -355,9 +363,7 @@ extension APIServer { ) throws -> VolumesService { log.info("initializing volume service") - // TODO: This goes away when we convert our roots to FilePath - let appPath = FilePath(appRoot.absolutePath()) - let resourceRoot = appPath.appending("volumes") + let resourceRoot = appRoot.appending(FilePath.Component("volumes")) let service = try VolumesService(resourceRoot: resourceRoot, containersService: containersService, log: log) let harness = VolumesHarness(service: service, log: log) diff --git a/Sources/ContainerCommands/Application.swift b/Sources/ContainerCommands/Application.swift index 6cfd00a83..e0ce6f80f 100644 --- a/Sources/ContainerCommands/Application.swift +++ b/Sources/ContainerCommands/Application.swift @@ -23,6 +23,7 @@ import ContainerizationError import ContainerizationOS import Foundation import Logging +import SystemPackage import TerminalProgress // This logger is only used until `asyncCommand.run()`. @@ -136,11 +137,12 @@ public struct Application: AsyncLoggableCommand { } public static func createPluginLoader() async throws -> PluginLoader { - let installRoot = CommandLine.executablePathUrl - .deletingLastPathComponent() - .appendingPathComponent("..") - .standardized - let pluginsURL = PluginLoader.userPluginsDir(installRoot: installRoot) + let installRootPath = CommandLine.executablePath + .removingLastComponent() + .removingLastComponent() + // TODO: Remove when we convert PluginLoader to FilePath. + let installRootURL = URL(fileURLWithPath: installRootPath.string) + let pluginsURL = PluginLoader.userPluginsDir(installRoot: installRootURL) var directoryExists: ObjCBool = false _ = FileManager.default.fileExists(atPath: pluginsURL.path, isDirectory: &directoryExists) let userPluginsURL = directoryExists.boolValue ? pluginsURL : nil @@ -149,13 +151,12 @@ public struct Application: AsyncLoggableCommand { let appBundlePluginsURL = Bundle.main.resourceURL?.appending(path: "plugins") // plugins built into the application installed as a Unix-like application - let installRootPluginsURL = - installRoot - .appendingPathComponent("libexec") - .appendingPathComponent("container") - .appendingPathComponent("plugins") - .standardized - + let installRootPluginsPath = + installRootPath + .appending(FilePath.Component("libexec")) + .appending(FilePath.Component("container")) + .appending(FilePath.Component("plugins")) + let installRootPluginsURL = URL(fileURLWithPath: installRootPluginsPath.string) let pluginDirectories = [ userPluginsURL, appBundlePluginsURL, diff --git a/Sources/ContainerCommands/DefaultCommand.swift b/Sources/ContainerCommands/DefaultCommand.swift index d623c13de..3df8f570e 100644 --- a/Sources/ContainerCommands/DefaultCommand.swift +++ b/Sources/ContainerCommands/DefaultCommand.swift @@ -19,6 +19,7 @@ import ContainerAPIClient import ContainerPlugin import Darwin import Foundation +import SystemPackage struct DefaultCommand: AsyncLoggableCommand { public static let configuration = CommandConfiguration( @@ -48,17 +49,19 @@ struct DefaultCommand: AsyncLoggableCommand { } // Compute canonical plugin directories to show in helpful errors (avoid hard-coded paths) - let installRoot = CommandLine.executablePathUrl - .deletingLastPathComponent() - .appendingPathComponent("..") - .standardized - let userPluginsURL = PluginLoader.userPluginsDir(installRoot: installRoot) - let installRootPluginsURL = + let installRoot = CommandLine.executablePath + .removingLastComponent() + .removingLastComponent() + + // TODO: Remove when we convert PluginLoader to FilePath + let installRootURL = URL(fileURLWithPath: installRoot.string) + let userPluginsURL = PluginLoader.userPluginsDir(installRoot: installRootURL) + let installRootPluginsPath = installRoot - .appendingPathComponent("libexec") - .appendingPathComponent("container") - .appendingPathComponent("plugins") - .standardized + .appending(FilePath.Component("libexec")) + .appending(FilePath.Component("container")) + .appending(FilePath.Component("plugins")) + let installRootPluginsURL = URL(fileURLWithPath: installRootPluginsPath.string) let hintPaths = [userPluginsURL, installRootPluginsURL] .map { $0.appendingPathComponent(command).path(percentEncoded: false) } .joined(separator: "\n - ") diff --git a/Sources/ContainerCommands/System/SystemStart.swift b/Sources/ContainerCommands/System/SystemStart.swift index 4ddd9f239..1ee000cc3 100644 --- a/Sources/ContainerCommands/System/SystemStart.swift +++ b/Sources/ContainerCommands/System/SystemStart.swift @@ -34,19 +34,19 @@ extension Application { @Option( name: .shortAndLong, help: "Path to the root directory for application data", - transform: { URL(filePath: $0) }) - var appRoot = ApplicationRoot.defaultURL + transform: { FilePath(FileManager.default.currentDirectoryPath).resolve($0, defaultPath: FilePath($0)) }) + var appRoot = ApplicationRoot.defaultPath @Option( name: .long, help: "Path to the root directory for application executables and plugins", - transform: { URL(filePath: $0) }) - var installRoot = InstallRoot.defaultURL + transform: { FilePath(FileManager.default.currentDirectoryPath).resolve($0, defaultPath: FilePath($0)) }) + var installRoot = InstallRoot.defaultPath @Option( name: .long, help: "Path to the root directory for log data, using macOS log facility if not set", - transform: { FilePath($0) }) + transform: { FilePath(FileManager.default.currentDirectoryPath).resolve($0, defaultPath: FilePath($0)) }) var logRoot: FilePath? = nil @Flag( @@ -72,9 +72,7 @@ extension Application { public init() {} public func run() async throws { - let appRootPath = FilePath(appRoot.path(percentEncoded: false)) - let installRootPath = FilePath(installRoot.path(percentEncoded: false)) - try ConfigurationLoader.copyConfigurationToReadOnly(to: appRootPath) + try ConfigurationLoader.copyConfigurationToReadOnly(to: appRoot) // Pass appRoot before installRoot: ConfigurationLoader uses first-match-wins // precedence, so user-provided config in appRoot overrides the defaults // shipped under installRoot. Both layers are passed explicitly because @@ -82,8 +80,8 @@ extension Application { // loader's default search would otherwise ignore those overrides. let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load( configurationFiles: [ - ConfigurationLoader.configurationFile(in: appRootPath, of: .appRoot), - ConfigurationLoader.configurationFile(in: installRootPath, of: .installRoot), + ConfigurationLoader.configurationFile(in: appRoot, of: .appRoot), + ConfigurationLoader.configurationFile(in: installRoot, of: .installRoot), ]) // Without the true path to the binary in the plist, `container-apiserver` won't launch properly. @@ -91,29 +89,27 @@ extension Application { // Gatekeeper / amfid validates code signatures relative to the enclosing .app bundle // hierarchy; launching via a symlink outside the bundle fails that check. // TODO: Can we use the plugin loader to bootstrap the API server? - let executableUrl = CommandLine.executablePathUrl - .deletingLastPathComponent() - .appendingPathComponent("container-apiserver") - .resolvingSymlinksInPath() + let executablePath = try CommandLine.executablePath + .removingLastComponent() + .appending(FilePath.Component("container-apiserver")) + .resolvingSymlinks() - var args = [executableUrl.absolutePath()] + var args = [executablePath.string] args.append("start") if logOptions.debug { args.append("--debug") } - let apiServerDataUrl = appRoot.appending(path: "apiserver") - try! FileManager.default.createDirectory(at: apiServerDataUrl, withIntermediateDirectories: true) + let apiServerDataPath = appRoot.appending(FilePath.Component("apiserver")) + let apiServerDataURL = URL(fileURLWithPath: apiServerDataPath.string) + try! FileManager.default.createDirectory(at: apiServerDataURL, withIntermediateDirectories: true) var env = PluginLoader.filterEnvironment() - env[ApplicationRoot.environmentName] = appRoot.path(percentEncoded: false) - env[InstallRoot.environmentName] = installRoot.path(percentEncoded: false) + env[ApplicationRoot.environmentName] = appRoot.string + env[InstallRoot.environmentName] = installRoot.string if let logRoot { - env[LogRoot.environmentName] = - logRoot.isAbsolute - ? logRoot.string - : FilePath(FileManager.default.currentDirectoryPath).appending(logRoot.components).string + env[LogRoot.environmentName] = logRoot.string } let plist = LaunchPlist( label: "com.apple.container.apiserver", @@ -124,7 +120,8 @@ extension Application { machServices: ["com.apple.container.apiserver"] ) - let plistURL = apiServerDataUrl.appending(path: "apiserver.plist") + let plistPath = apiServerDataPath.appending(FilePath.Component("apiserver.plist")) + let plistURL = URL(fileURLWithPath: plistPath.string) let data = try plist.encode() try data.write(to: plistURL) diff --git a/Sources/ContainerPersistence/FilePath+Symlinks.swift b/Sources/ContainerPersistence/FilePath+Symlinks.swift new file mode 100644 index 000000000..fcf90d332 --- /dev/null +++ b/Sources/ContainerPersistence/FilePath+Symlinks.swift @@ -0,0 +1,39 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Darwin +import SystemPackage + +extension FilePath { + /// Returns a new `FilePath` with all symlinks resolved and `.`/`..` + /// components normalized, by calling `realpath(3)`. + /// + /// Unlike ``lexicallyNormalized()``, this method accesses the file system. + /// It throws ``Errno/noSuchFileOrDirectory`` if any component of the path + /// does not exist. + /// + /// The returned path is always absolute. If the receiver is a relative path, + /// it is resolved against the process's current working directory. + public func resolvingSymlinks() throws -> FilePath { + try withPlatformString { cPath in + guard let resolved = Darwin.realpath(cPath, nil) else { + throw Errno(rawValue: Darwin.errno) + } + defer { free(resolved) } + return FilePath(platformString: resolved) + } + } +} diff --git a/Sources/ContainerPersistence/PathUtils.swift b/Sources/ContainerPersistence/PathUtils.swift index 925c4b08f..d91bb3e4e 100644 --- a/Sources/ContainerPersistence/PathUtils.swift +++ b/Sources/ContainerPersistence/PathUtils.swift @@ -51,11 +51,10 @@ public enum PathUtils { // rather than argv[0]: when the binary is invoked through PATH (e.g. // `container ...`), argv[0] is just the basename and resolves to an // empty FilePath, which FileManager treats as CWD-relative. - let installRootURL = CommandLine.executablePathUrl - .deletingLastPathComponent() - .appendingPathComponent("..") - .standardized - return FilePath(installRootURL.path(percentEncoded: false)) + let installRootPath = CommandLine.executablePath + .removingLastComponent() + .removingLastComponent() + return installRootPath } } } diff --git a/Sources/ContainerPlugin/ApplicationRoot.swift b/Sources/ContainerPlugin/ApplicationRoot.swift index 94db22c38..467732791 100644 --- a/Sources/ContainerPlugin/ApplicationRoot.swift +++ b/Sources/ContainerPlugin/ApplicationRoot.swift @@ -15,19 +15,34 @@ //===----------------------------------------------------------------------===// import Foundation +import SystemPackage /// Provides the application data root path. public struct ApplicationRoot { + /// The environment variable that if set, determines the root directory for the application data store. + /// Otherwise, the system uses the default "~/Library/Application Support/com.apple.container". public static let environmentName = "CONTAINER_APP_ROOT" - public static let defaultURL = FileManager.default.urls( - for: .applicationSupportDirectory, - in: .userDomainMask - ).first!.appendingPathComponent("com.apple.container") + /// The default root directory used when ``environmentName`` is not set: + /// `~/Library/Application Support/com.apple.container`. + public static let defaultPath = FilePath( + FileManager.default.urls( + for: .applicationSupportDirectory, + in: .userDomainMask + ).first!.path(percentEncoded: false) + ) + .appending(FilePath.Component("com.apple.container")) - private static let envPath = ProcessInfo.processInfo.environment[Self.environmentName] + /// The resolved root directory path, always lexically normalized. + /// + /// If the environment variable is set to an absolute path, that path is used directly. + /// If it is set to a relative path, the path is resolved against the working directory. + /// Otherwise, ``defaultPath`` is used. + public static let path = FilePath(FileManager.default.currentDirectoryPath).resolve( + ProcessInfo.processInfo.environment[environmentName], + defaultPath: defaultPath + ) - public static let url = envPath.map { URL(fileURLWithPath: $0) } ?? defaultURL - - public static let path = url.path(percentEncoded: false) + /// The pathname to the root directory + public static let pathname = path.string } diff --git a/Sources/ContainerPlugin/FilePath+Resolve.swift b/Sources/ContainerPlugin/FilePath+Resolve.swift new file mode 100644 index 000000000..54dd22c28 --- /dev/null +++ b/Sources/ContainerPlugin/FilePath+Resolve.swift @@ -0,0 +1,48 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import SystemPackage + +extension FilePath { + /// Resolves a pathname string relative to this path. + /// + /// The result is lexically normalized — `.` components are removed and `..` components + /// collapse the preceding component. Absolute pathnames are returned normalized as-is; + /// relative pathnames are appended to `self` before normalizing. + /// + /// - Parameter pathname: The pathname to resolve. + /// - Returns: The resolved ``FilePath``, or `nil` if `pathname` is `nil` or empty. + package func resolve(_ pathname: String?) -> FilePath? { + guard let pathname, !pathname.isEmpty else { return nil } + let path = FilePath(pathname) + guard !path.isAbsolute else { return path.lexicallyNormalized() } + return self.appending(path.components).lexicallyNormalized() + } + + /// Resolves a pathname string relative to this path, falling back to a default. + /// + /// The result is lexically normalized — `.` components are removed and `..` components + /// collapse the preceding component. Absolute pathnames are returned normalized as-is; + /// relative pathnames are appended to `self` before normalizing. + /// + /// - Parameters: + /// - pathname: The pathname to resolve. + /// - defaultPath: The path returned when `pathname` is `nil` or empty. + /// - Returns: The resolved ``FilePath``, or `defaultPath` lexically normalized if `pathname` is `nil` or empty. + package func resolve(_ pathname: String?, defaultPath: FilePath) -> FilePath { + resolve(pathname) ?? defaultPath.lexicallyNormalized() + } +} diff --git a/Sources/ContainerPlugin/InstallRoot.swift b/Sources/ContainerPlugin/InstallRoot.swift index 8f39c790b..01578b5de 100644 --- a/Sources/ContainerPlugin/InstallRoot.swift +++ b/Sources/ContainerPlugin/InstallRoot.swift @@ -16,19 +16,34 @@ import ContainerVersion import Foundation +import SystemPackage /// Provides the application installation root path. public struct InstallRoot { + /// The environment variable that if set, determines the root directory for installed application. + /// Otherwise, the system computes the install path as the parent of the directory containing the + /// application binary (for example, "/usr/local/bin/container" -> "/usr/local"). public static let environmentName = "CONTAINER_INSTALL_ROOT" - public static let defaultURL = CommandLine.executablePathUrl - .deletingLastPathComponent() - .appendingPathComponent("..") - .standardized + /// The default root directory used when the environment variable is not set. + /// + /// Computed as the grandparent of ``CommandLine/executablePath`` + /// (for example, `/usr/local/bin/container` → `/usr/local`). + /// Lexically normalized but not canonical, as symlinks in the executable path are not resolved. + public static let defaultPath = CommandLine.executablePath + .removingLastComponent() + .removingLastComponent() - private static let envPath = ProcessInfo.processInfo.environment[Self.environmentName] + /// The resolved root directory path, always lexically normalized. + /// + /// If the environment variable is set to an absolute path, that path is used directly. + /// If it is set to a relative path, the path is resolved against the working directory. + /// Otherwise, ``defaultPath`` is used. + public static let path = FilePath(FileManager.default.currentDirectoryPath).resolve( + ProcessInfo.processInfo.environment[environmentName], + defaultPath: defaultPath + ) - public static let url = envPath.map { URL(fileURLWithPath: $0) } ?? defaultURL - - public static let path = url.path(percentEncoded: false) + /// The pathname to the root directory + public static let pathname = path.string } diff --git a/Sources/ContainerPlugin/LogRoot.swift b/Sources/ContainerPlugin/LogRoot.swift index 2550a887b..51630d249 100644 --- a/Sources/ContainerPlugin/LogRoot.swift +++ b/Sources/ContainerPlugin/LogRoot.swift @@ -19,21 +19,19 @@ import SystemPackage /// Provides the application data root path. public struct LogRoot { - - private static let envPath = ProcessInfo.processInfo.environment[Self.environmentName].flatMap { - $0.isEmpty ? nil : FilePath($0) - } - /// The environment variable that if set, determines the root directory for log files. /// Otherwise, the application uses the macOS log facility. public static let environmentName = "CONTAINER_LOG_ROOT" - /// The path object for the log file root directory - public static let path = envPath.map { - guard !$0.isAbsolute else { return $0 } - return FilePath(FileManager.default.currentDirectoryPath).appending($0.components) - } + /// The resolved root directory for log files, or `nil` if the environment variable is not set. + /// + /// When non-nil, the path is always lexically normalized. + /// If the environment variable is set to an absolute path, that path is used directly. + /// If it is set to a relative path, the path is resolved against the working directory. + public static let path = FilePath(FileManager.default.currentDirectoryPath).resolve( + ProcessInfo.processInfo.environment[environmentName] + ) - /// The pathname to the log file root directory + /// The pathname to the root directory public static let pathname = path?.string } diff --git a/Sources/ContainerVersion/Bundle+AppBundle.swift b/Sources/ContainerVersion/Bundle+AppBundle.swift index 5028897e1..9c4941a41 100644 --- a/Sources/ContainerVersion/Bundle+AppBundle.swift +++ b/Sources/ContainerVersion/Bundle+AppBundle.swift @@ -14,18 +14,33 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import Darwin import Foundation +import SystemPackage -/// Retrieve the application bundle for a path that refers to a macOS executable. extension Bundle { - public static func appBundle(executableURL: URL) -> Bundle? { - let resolvedURL = executableURL.resolvingSymlinksInPath() - let macOSURL = resolvedURL.deletingLastPathComponent() - let contentsURL = macOSURL.deletingLastPathComponent() - let bundleURL = contentsURL.deletingLastPathComponent() - if bundleURL.pathExtension == "app" { - return Bundle(url: bundleURL) - } - return nil + /// Retrieves the application bundle for a path that refers to a macOS executable. + /// + /// Resolves symlinks in `executablePath`, then walks up the standard macOS bundle layout + /// (`MacOS/` → `Contents/` → `Foo.app/`) and verifies the `.app` extension. + /// + /// - Parameter executablePath: The path to a macOS executable inside a bundle. + /// - Returns: The ``Bundle`` at the resolved `.app` directory, or `nil` if the executable + /// is not inside a valid macOS application bundle. + public static func appBundle(executablePath: FilePath) -> Bundle? { + let resolvedPath = + executablePath.withPlatformString { cPath in + Darwin.realpath(cPath, nil).map { ptr -> FilePath in + defer { free(ptr) } + return FilePath(platformString: ptr) + } + } ?? executablePath + let bundlePath = + resolvedPath + .removingLastComponent() // MacOS/ + .removingLastComponent() // Contents/ + .removingLastComponent() // Foo.app/ + guard bundlePath.lastComponent?.extension == "app" else { return nil } + return Bundle(url: URL(fileURLWithPath: bundlePath.string)) } } diff --git a/Sources/ContainerVersion/CommandLine+Executable.swift b/Sources/ContainerVersion/CommandLine+Executable.swift index c48cbe573..67d854fe4 100644 --- a/Sources/ContainerVersion/CommandLine+Executable.swift +++ b/Sources/ContainerVersion/CommandLine+Executable.swift @@ -15,9 +15,14 @@ //===----------------------------------------------------------------------===// import Foundation +import SystemPackage extension CommandLine { - public static var executablePathUrl: URL { + /// The path of the running executable. + /// + /// Obtained via `_NSGetExecutablePath`, which returns an absolute, lexically normalized path + /// (no `.` or `..` components). Symlinks are not resolved, so the path is not canonical. + public static var executablePath: FilePath { /// _NSGetExecutablePath with a zero-length buffer returns the needed buffer length var bufferSize: Int32 = 0 var buffer = [CChar](repeating: 0, count: Int(bufferSize)) @@ -31,6 +36,6 @@ extension CommandLine { /// Return the path with the executable file component removed the last component and let executablePath = String(cString: &buffer) - return URL(filePath: executablePath) + return FilePath(executablePath) } } diff --git a/Sources/ContainerVersion/ReleaseVersion.swift b/Sources/ContainerVersion/ReleaseVersion.swift index 4ae98d6a4..99e9ad2b9 100644 --- a/Sources/ContainerVersion/ReleaseVersion.swift +++ b/Sources/ContainerVersion/ReleaseVersion.swift @@ -35,7 +35,7 @@ public struct ReleaseVersion { } public static func version() -> String { - let appBundle = Bundle.appBundle(executableURL: CommandLine.executablePathUrl) + let appBundle = Bundle.appBundle(executablePath: CommandLine.executablePath) let bundleVersion = appBundle?.infoDictionary?["CFBundleShortVersionString"] as? String return bundleVersion ?? get_release_version().map { String(cString: $0) } ?? "0.0.0" } diff --git a/Sources/Plugins/CoreImages/ImagesHelper.swift b/Sources/Plugins/CoreImages/ImagesHelper.swift index 6ba3318b5..3eea9cc0d 100644 --- a/Sources/Plugins/CoreImages/ImagesHelper.swift +++ b/Sources/Plugins/CoreImages/ImagesHelper.swift @@ -25,6 +25,7 @@ import ContainerXPC import Containerization import Foundation import Logging +import SystemPackage @main struct ImagesHelper: AsyncParsableCommand { @@ -51,9 +52,9 @@ extension ImagesHelper { @Option(name: .long, help: "XPC service prefix") var serviceIdentifier: String = "com.apple.container.core.container-core-images" - var appRoot = ApplicationRoot.url + var appRoot = ApplicationRoot.path - var installRoot = InstallRoot.url + var installRoot = InstallRoot.path var logRoot = LogRoot.path @@ -90,11 +91,13 @@ extension ImagesHelper { } } - private func initializeImagesService(root: URL, containerSystemConfig: ContainerSystemConfig, log: Logger, routes: inout [String: XPCServer.RouteHandler]) throws { + private func initializeImagesService(root: FilePath, containerSystemConfig: ContainerSystemConfig, log: Logger, routes: inout [String: XPCServer.RouteHandler]) throws { + // TODO: remove as part of ImageStore URL removal PR + let rootURL = URL(fileURLWithPath: root.string) let contentStore = RemoteContentStoreClient() - let imageStore = try ImageStore(path: root, contentStore: contentStore) + let imageStore = try ImageStore(path: rootURL, contentStore: contentStore) let unpackStrategy = SnapshotStore.defaultUnpackStrategy(initImage: containerSystemConfig.vminit.image) - let snapshotStore = try SnapshotStore(path: root, unpackStrategy: unpackStrategy, log: log) + let snapshotStore = try SnapshotStore(path: rootURL, unpackStrategy: unpackStrategy, log: log) let service = try ImagesService(contentStore: contentStore, imageStore: imageStore, snapshotStore: snapshotStore, log: log) let harness = ImagesServiceHarness(service: service, log: log) @@ -112,8 +115,10 @@ extension ImagesHelper { routes[ImagesServiceXPCRoute.snapshotGet.rawValue] = XPCServer.route(harness.getSnapshot) } - private func initializeContentService(root: URL, log: Logger, routes: inout [String: XPCServer.RouteHandler]) throws { - let service = try ContentStoreService(root: root, log: log) + private func initializeContentService(root: FilePath, log: Logger, routes: inout [String: XPCServer.RouteHandler]) throws { + // TODO: remove as part of ImageStore URL removal PR + let rootURL = URL(fileURLWithPath: root.string) + let service = try ContentStoreService(root: rootURL, log: log) let harness = ContentServiceHarness(service: service, log: log) routes[ImagesServiceXPCRoute.contentClean.rawValue] = XPCServer.route(harness.clean) diff --git a/Tests/ContainerPersistenceTests/FilePathSymlinksTests.swift b/Tests/ContainerPersistenceTests/FilePathSymlinksTests.swift new file mode 100644 index 000000000..26f307de4 --- /dev/null +++ b/Tests/ContainerPersistenceTests/FilePathSymlinksTests.swift @@ -0,0 +1,51 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ContainerTestSupport +import Foundation +import SystemPackage +import Testing + +@testable import ContainerPersistence + +struct FilePathSymlinksTests { + @Test func realPathReturnsAbsolutePath() throws { + let resolved = try FilePath("/tmp").resolvingSymlinks() + #expect(resolved.isAbsolute) + } + + @Test func symlinkResolvesToTarget() async throws { + try await TemporaryStorage.withTempDir { dir in + let target = dir.appending(FilePath.Component("target")) + let link = dir.appending(FilePath.Component("link")) + try "content".write(toFile: target.string, atomically: true, encoding: .utf8) + try FileManager.default.createSymbolicLink(atPath: link.string, withDestinationPath: target.string) + + #expect(try link.resolvingSymlinks() == target.resolvingSymlinks()) + } + } + + @Test func nonExistentPathThrows() { + #expect(throws: Errno.noSuchFileOrDirectory) { + try FilePath("/nonexistent/path/that/does/not/exist").resolvingSymlinks() + } + } + + @Test func relativePathResolvesToAbsolute() throws { + let resolved = try FilePath(".").resolvingSymlinks() + #expect(resolved.isAbsolute) + } +} diff --git a/Tests/ContainerPluginTests/FilePath+ResolveTests.swift b/Tests/ContainerPluginTests/FilePath+ResolveTests.swift new file mode 100644 index 000000000..b9d933491 --- /dev/null +++ b/Tests/ContainerPluginTests/FilePath+ResolveTests.swift @@ -0,0 +1,74 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import SystemPackage +import Testing + +@testable import ContainerPlugin + +private let cwd = FilePath("/current/dir") + +struct FilePathResolveTests { + @Test func nilWhenPathnameIsNil() { + #expect(cwd.resolve(nil) == nil) + } + + @Test func nilWhenPathnameIsEmpty() { + #expect(cwd.resolve("") == nil) + } + + @Test func absolutePathnameReturnedAsIs() { + #expect(cwd.resolve("/custom/root") == FilePath("/custom/root")) + } + + @Test func relativePathnamePrependsCurrentDirectory() { + #expect(cwd.resolve("data") == FilePath("/current/dir/data")) + } + + @Test func relativePathnameWithDotDotIsLexicallyNormalized() { + #expect(cwd.resolve("../sibling") == FilePath("/current/sibling")) + } + + @Test func relativePathnameWithDotIsLexicallyNormalized() { + #expect(cwd.resolve("./data") == FilePath("/current/dir/data")) + } + + @Test func absolutePathnameWithDotDotIsLexicallyNormalized() { + #expect(cwd.resolve("/custom/../root") == FilePath("/root")) + } + + @Test func absolutePathnameWithDotIsLexicallyNormalized() { + #expect(cwd.resolve("/custom/./root") == FilePath("/custom/root")) + } + + @Test func defaultPathUsedWhenPathnameIsNil() { + let fallback = FilePath("/fallback") + #expect(cwd.resolve(nil, defaultPath: fallback) == fallback) + } + + @Test func defaultPathUsedWhenPathnameIsEmpty() { + let fallback = FilePath("/fallback") + #expect(cwd.resolve("", defaultPath: fallback) == fallback) + } + + @Test func defaultPathIsLexicallyNormalized() { + #expect(cwd.resolve(nil, defaultPath: FilePath("/fallback/../normalized")) == FilePath("/normalized")) + } + + @Test func absolutePathnameOverridesDefaultPath() { + #expect(cwd.resolve("/custom", defaultPath: FilePath("/fallback")) == FilePath("/custom")) + } +} diff --git a/Tests/ContainerPluginTests/RootPathTests.swift b/Tests/ContainerPluginTests/RootPathTests.swift new file mode 100644 index 000000000..8efcae926 --- /dev/null +++ b/Tests/ContainerPluginTests/RootPathTests.swift @@ -0,0 +1,47 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import SystemPackage +import Testing + +@testable import ContainerPlugin + +struct ApplicationRootTests { + @Test func defaultPathIsAbsolute() { + #expect(ApplicationRoot.defaultPath.isAbsolute) + } + + @Test func defaultPathEndsWithContainerComponent() { + #expect(ApplicationRoot.defaultPath.lastComponent?.string == "com.apple.container") + } +} + +struct InstallRootTests { + @Test func defaultPathIsAbsolute() { + #expect(InstallRoot.defaultPath.isAbsolute) + } + + @Test func defaultPathIsGrandparentOfExecutable() { + #expect(InstallRoot.defaultPath == CommandLine.executablePath.removingLastComponent().removingLastComponent()) + } +} + +struct LogRootTests { + @Test func pathIsNilWhenEnvUnset() { + // CONTAINER_LOG_ROOT is not set in the unit test environment + #expect(LogRoot.path == nil) + } +} diff --git a/Tests/ContainerVersionTests/Bundle+AppBundleTests.swift b/Tests/ContainerVersionTests/Bundle+AppBundleTests.swift new file mode 100644 index 000000000..263789825 --- /dev/null +++ b/Tests/ContainerVersionTests/Bundle+AppBundleTests.swift @@ -0,0 +1,84 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation +import SystemPackage +import Testing + +@testable import ContainerVersion + +struct BundleAppBundleTests { + @Test func returnsNilForUnixInstallPath() { + // /usr/local/bin/container — no .app bundle in the hierarchy + let path = FilePath("/usr/local/bin/container") + #expect(Bundle.appBundle(executablePath: path) == nil) + } + + @Test func returnsBundleForAppBundleExecutable() throws { + // Build a minimal Foo.app bundle on disk — Bundle(url:) requires the directory to exist. + let tmp = FileManager.default.temporaryDirectory + .appendingPathComponent("BundleTest-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: tmp) } + + let bundleURL = tmp.appendingPathComponent("Foo.app", isDirectory: true) + let contentsURL = bundleURL.appendingPathComponent("Contents", isDirectory: true) + let macOSURL = contentsURL.appendingPathComponent("MacOS", isDirectory: true) + try FileManager.default.createDirectory(at: macOSURL, withIntermediateDirectories: true) + try Data("".utf8) + .write(to: contentsURL.appendingPathComponent("Info.plist")) + + let executablePath = FilePath(macOSURL.path(percentEncoded: false)) + .appending(FilePath.Component("Foo")) + let bundle = Bundle.appBundle(executablePath: executablePath) + #expect(bundle != nil) + #expect(bundle?.bundleURL.lastPathComponent == "Foo.app") + } + + @Test func returnsBundleForSymlinkedExecutable() throws { + let tmp = FileManager.default.temporaryDirectory + .appendingPathComponent("BundleTest-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: tmp) } + + let bundleURL = tmp.appendingPathComponent("Foo.app", isDirectory: true) + let contentsURL = bundleURL.appendingPathComponent("Contents", isDirectory: true) + let macOSURL = contentsURL.appendingPathComponent("MacOS", isDirectory: true) + try FileManager.default.createDirectory(at: macOSURL, withIntermediateDirectories: true) + try Data("".utf8) + .write(to: contentsURL.appendingPathComponent("Info.plist")) + let executableURL = macOSURL.appendingPathComponent("Foo") + try Data().write(to: executableURL) + + // Symlink outside the bundle pointing at the real executable + let symlinkURL = tmp.appendingPathComponent("foo-link") + try FileManager.default.createSymbolicLink(at: symlinkURL, withDestinationURL: executableURL) + + let bundle = Bundle.appBundle(executablePath: FilePath(symlinkURL.path(percentEncoded: false))) + #expect(bundle != nil) + #expect(bundle?.bundleURL.lastPathComponent == "Foo.app") + } + + @Test func returnsNilWhenTooShallow() { + // Only one component above executable — can't be a bundle + let path = FilePath("/Foo.app/binary") + #expect(Bundle.appBundle(executablePath: path) == nil) + } + + @Test func returnsNilWhenThirdAncestorLacksAppExtension() { + // Parent hierarchy exists but doesn't end in .app + let path = FilePath("/opt/tools/bin/helper") + #expect(Bundle.appBundle(executablePath: path) == nil) + } +} diff --git a/Tests/ContainerPluginTests/CommandLine+ExecutableTest.swift b/Tests/ContainerVersionTests/CommandLine+ExecutableTests.swift similarity index 53% rename from Tests/ContainerPluginTests/CommandLine+ExecutableTest.swift rename to Tests/ContainerVersionTests/CommandLine+ExecutableTests.swift index d9e913c8d..cdda4fe89 100644 --- a/Tests/ContainerPluginTests/CommandLine+ExecutableTest.swift +++ b/Tests/ContainerVersionTests/CommandLine+ExecutableTests.swift @@ -1,5 +1,5 @@ //===----------------------------------------------------------------------===// -// Copyright © 2025-2026 Apple Inc. and the container project authors. +// Copyright © 2026 Apple Inc. and the container project authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -14,14 +14,25 @@ // limitations under the License. //===----------------------------------------------------------------------===// -import Foundation +import SystemPackage import Testing -@testable import ContainerPlugin +@testable import ContainerVersion -struct CommandLineExecutableTest { - @Test - func testCLIPluginConfigLoad() async throws { - #expect(CommandLine.executablePathUrl.lastPathComponent == "swiftpm-testing-helper") +struct CommandLineExecutableTests { + @Test func lastComponentIsTestBinary() { + #expect(CommandLine.executablePath.lastComponent?.string == "swiftpm-testing-helper") + } + + @Test func pathIsAbsolute() { + #expect(CommandLine.executablePath.isAbsolute) + } + + @Test func pathIsNonEmpty() { + #expect(!CommandLine.executablePath.string.isEmpty) + } + + @Test func removingLastComponentTwiceIsAbsolute() { + #expect(CommandLine.executablePath.removingLastComponent().removingLastComponent().isAbsolute) } } From 25ab5934e4295162b131dff175748430fe06e57a Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Tue, 26 May 2026 14:33:53 -0700 Subject: [PATCH 03/15] Support use of custom app root with system config in CLI commands (#1600) ## Type of Change - [x] Bug fix ## Motivation and Context Many CLI commands need to reference the system configurations for `container`. Previously, CLI commands would try to load the system configurations from the default application root location, regardless of if `container` had been started with a custom application root location. This PR fixes that issue by having each CLI command ping the APIServer's health check service to get the correct app root path. Closes https://github.com/apple/container/issues/1576 ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf --- Sources/ContainerCommands/Application.swift | 15 +++++++++++++++ Sources/ContainerCommands/BuildCommand.swift | 2 +- .../ContainerCommands/Builder/BuilderStart.swift | 2 +- .../Container/ContainerCreate.swift | 2 +- .../Container/ContainerRun.swift | 2 +- Sources/ContainerCommands/Image/ImageDelete.swift | 2 +- .../ContainerCommands/Image/ImageInspect.swift | 2 +- Sources/ContainerCommands/Image/ImageList.swift | 2 +- Sources/ContainerCommands/Image/ImagePull.swift | 2 +- Sources/ContainerCommands/Image/ImagePush.swift | 2 +- Sources/ContainerCommands/Image/ImageSave.swift | 2 +- Sources/ContainerCommands/Image/ImageTag.swift | 2 +- .../Registry/RegistryLogin.swift | 2 +- .../System/Kernel/KernelSet.swift | 2 +- .../System/Property/PropertyList.swift | 2 +- 15 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Sources/ContainerCommands/Application.swift b/Sources/ContainerCommands/Application.swift index e0ce6f80f..fc702db72 100644 --- a/Sources/ContainerCommands/Application.swift +++ b/Sources/ContainerCommands/Application.swift @@ -17,6 +17,7 @@ import ArgumentParser import ContainerAPIClient import ContainerLog +import ContainerPersistence import ContainerPlugin import ContainerVersion import ContainerizationError @@ -181,6 +182,20 @@ public struct Application: AsyncLoggableCommand { ) } + /// Load the system configuration using `appRoot` / `installRoot` reported by the + /// daemon. `container system start` MUST have previously been run to start the daemon. + public static func loadContainerSystemConfig() async throws -> ContainerSystemConfig { + let health = try await ClientHealthCheck.ping(timeout: .seconds(10)) + let appRoot = FilePath(health.appRoot.path(percentEncoded: false)) + let installRoot = FilePath(health.installRoot.path(percentEncoded: false)) + return try await ConfigurationLoader.load( + configurationFiles: [ + ConfigurationLoader.configurationFile(in: appRoot, of: .appRoot), + ConfigurationLoader.configurationFile(in: installRoot, of: .installRoot), + ] + ) + } + public func validate() throws { // Not really a "validation", but a cheat to run this before // any of the commands do their business. diff --git a/Sources/ContainerCommands/BuildCommand.swift b/Sources/ContainerCommands/BuildCommand.swift index c15503145..f15aa60cc 100644 --- a/Sources/ContainerCommands/BuildCommand.swift +++ b/Sources/ContainerCommands/BuildCommand.swift @@ -149,7 +149,7 @@ extension Application { var pull: Bool = false public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() do { let timeout: Duration = .seconds(300) let progressConfig = try ProgressConfig( diff --git a/Sources/ContainerCommands/Builder/BuilderStart.swift b/Sources/ContainerCommands/Builder/BuilderStart.swift index 8af811bc7..a467ea123 100644 --- a/Sources/ContainerCommands/Builder/BuilderStart.swift +++ b/Sources/ContainerCommands/Builder/BuilderStart.swift @@ -55,7 +55,7 @@ extension Application { public init() {} public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let progressConfig = try ProgressConfig( showTasks: true, showItems: true, diff --git a/Sources/ContainerCommands/Container/ContainerCreate.swift b/Sources/ContainerCommands/Container/ContainerCreate.swift index 2b846e2c4..92f0c02c1 100644 --- a/Sources/ContainerCommands/Container/ContainerCreate.swift +++ b/Sources/ContainerCommands/Container/ContainerCreate.swift @@ -56,7 +56,7 @@ extension Application { var arguments: [String] = [] public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let progressConfig = try ProgressConfig( showTasks: true, showItems: true, diff --git a/Sources/ContainerCommands/Container/ContainerRun.swift b/Sources/ContainerCommands/Container/ContainerRun.swift index 2957fe473..75832bd79 100644 --- a/Sources/ContainerCommands/Container/ContainerRun.swift +++ b/Sources/ContainerCommands/Container/ContainerRun.swift @@ -63,7 +63,7 @@ extension Application { var arguments: [String] = [] public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() var exitCode: Int32 = 127 let id = Utility.createContainerID(name: self.managementFlags.name) diff --git a/Sources/ContainerCommands/Image/ImageDelete.swift b/Sources/ContainerCommands/Image/ImageDelete.swift index daca01166..bea79c815 100644 --- a/Sources/ContainerCommands/Image/ImageDelete.swift +++ b/Sources/ContainerCommands/Image/ImageDelete.swift @@ -109,7 +109,7 @@ extension Application { } public mutating func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() try await DeleteImageImplementation.removeImage(options: options, containerSystemConfig: containerSystemConfig, log: log) } } diff --git a/Sources/ContainerCommands/Image/ImageInspect.swift b/Sources/ContainerCommands/Image/ImageInspect.swift index bcb3055b0..7cf8fc2b9 100644 --- a/Sources/ContainerCommands/Image/ImageInspect.swift +++ b/Sources/ContainerCommands/Image/ImageInspect.swift @@ -36,7 +36,7 @@ extension Application { public init() {} public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let uniqueNames = Set(images) let result = try await ClientImage.get( names: Array(uniqueNames), containerSystemConfig: containerSystemConfig diff --git a/Sources/ContainerCommands/Image/ImageList.swift b/Sources/ContainerCommands/Image/ImageList.swift index c1f2abdbd..b01245bbb 100644 --- a/Sources/ContainerCommands/Image/ImageList.swift +++ b/Sources/ContainerCommands/Image/ImageList.swift @@ -45,7 +45,7 @@ extension Application { public var logOptions: Flags.Logging public mutating func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() try Self.validate(quiet: quiet, verbose: verbose) var images = try await ClientImage.list().filter { img in diff --git a/Sources/ContainerCommands/Image/ImagePull.swift b/Sources/ContainerCommands/Image/ImagePull.swift index e6429a4c5..7506dc315 100644 --- a/Sources/ContainerCommands/Image/ImagePull.swift +++ b/Sources/ContainerCommands/Image/ImagePull.swift @@ -69,7 +69,7 @@ extension Application { } public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let p = try DefaultPlatform.resolve(platform: platform, os: os, arch: arch, log: log) let scheme = try RequestScheme(registry.scheme) diff --git a/Sources/ContainerCommands/Image/ImagePush.swift b/Sources/ContainerCommands/Image/ImagePush.swift index 198a2d6af..151ad5f65 100644 --- a/Sources/ContainerCommands/Image/ImagePush.swift +++ b/Sources/ContainerCommands/Image/ImagePush.swift @@ -57,7 +57,7 @@ extension Application { public init() {} public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let p = try DefaultPlatform.resolve(platform: platform, os: os, arch: arch, log: log) let scheme = try RequestScheme(registry.scheme) diff --git a/Sources/ContainerCommands/Image/ImageSave.swift b/Sources/ContainerCommands/Image/ImageSave.swift index d1cab7965..c514f5c68 100644 --- a/Sources/ContainerCommands/Image/ImageSave.swift +++ b/Sources/ContainerCommands/Image/ImageSave.swift @@ -67,7 +67,7 @@ extension Application { @Argument var references: [String] public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let p = try DefaultPlatform.resolve(platform: platform, os: os, arch: arch, log: log) let progressConfig = try ProgressConfig( diff --git a/Sources/ContainerCommands/Image/ImageTag.swift b/Sources/ContainerCommands/Image/ImageTag.swift index ed183e92a..7ec5a6107 100644 --- a/Sources/ContainerCommands/Image/ImageTag.swift +++ b/Sources/ContainerCommands/Image/ImageTag.swift @@ -36,7 +36,7 @@ extension Application { public var logOptions: Flags.Logging public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let existing = try await ClientImage.get(reference: source, containerSystemConfig: containerSystemConfig) let targetReference = try ClientImage.normalizeReference(target, containerSystemConfig: containerSystemConfig) try await existing.tag(new: targetReference) diff --git a/Sources/ContainerCommands/Registry/RegistryLogin.swift b/Sources/ContainerCommands/Registry/RegistryLogin.swift index d51e76e05..c3ca9d8e9 100644 --- a/Sources/ContainerCommands/Registry/RegistryLogin.swift +++ b/Sources/ContainerCommands/Registry/RegistryLogin.swift @@ -47,7 +47,7 @@ extension Application { var server: String public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() var username = self.username var password = "" if passwordStdin { diff --git a/Sources/ContainerCommands/System/Kernel/KernelSet.swift b/Sources/ContainerCommands/System/Kernel/KernelSet.swift index 33ca768f4..6a5993611 100644 --- a/Sources/ContainerCommands/System/Kernel/KernelSet.swift +++ b/Sources/ContainerCommands/System/Kernel/KernelSet.swift @@ -53,7 +53,7 @@ extension Application { public init() {} public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() if recommended { let url = containerSystemConfig.kernel.url let path: String = containerSystemConfig.kernel.binaryPath diff --git a/Sources/ContainerCommands/System/Property/PropertyList.swift b/Sources/ContainerCommands/System/Property/PropertyList.swift index 1c6b01db2..4ddaf0864 100644 --- a/Sources/ContainerCommands/System/Property/PropertyList.swift +++ b/Sources/ContainerCommands/System/Property/PropertyList.swift @@ -42,7 +42,7 @@ extension Application { public init() {} public func run() async throws { - let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() + let containerSystemConfig: ContainerSystemConfig = try await Application.loadContainerSystemConfig() let output = switch format { case .json: try Output.renderJSON(containerSystemConfig) From 8261a27ed0bed33cc5baa4d6c5115678060d8292 Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Tue, 26 May 2026 17:51:10 -0700 Subject: [PATCH 04/15] Add shmSize to management flag's init (#1603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Type of Change - [x] Bug fix ## Motivation and Context Ensure all fields are initialized in the management flags' init function. Without this change, if someone calls init() on this set of flags, they will get an error like ``` Can't read a value from a parsable argument definition. This error indicates that a property declared with an `@Argument`, `@Option`, `@Flag`, or `@OptionGroup` property wrapper was neither initialized to a value nor decoded from command-line arguments. To get a valid value, either call one of the static parsing methods (`parse`, `parseAsRoot`, or `main`) or define an initializer that initializes _every_ property of your parsable type. ``` ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf --- Sources/Services/ContainerAPIService/Client/Flags.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/Services/ContainerAPIService/Client/Flags.swift b/Sources/Services/ContainerAPIService/Client/Flags.swift index f8361ef68..ec7b83b40 100644 --- a/Sources/Services/ContainerAPIService/Client/Flags.swift +++ b/Sources/Services/ContainerAPIService/Client/Flags.swift @@ -189,6 +189,7 @@ public struct Flags { rosetta: Bool, runtime: String?, ssh: Bool, + shmSize: String?, tmpFs: [String], useInit: Bool, virtualization: Bool, @@ -217,6 +218,7 @@ public struct Flags { self.rosetta = rosetta self.runtime = runtime self.ssh = ssh + self.shmSize = shmSize self.tmpFs = tmpFs self.useInit = useInit self.virtualization = virtualization From 685966d026dcc76b43a18dba532430fc4f82aff0 Mon Sep 17 00:00:00 2001 From: NONE Date: Wed, 27 May 2026 19:54:49 +0200 Subject: [PATCH 05/15] cli: resolve subcommand path in `container help` (#1587) - Fixes #1509. The CLI's own help text tells users to run `container help `, but every form of that results in an error. - Added a captured subcommand path, walked Application`'s `subcommands` + `groupedSubcommands` tree (matching `commandName` and `aliases`), and printed `Application.helpMessage(for:)` for the resolved target. Empty path keeps existing plugin-aware top-level help; unknown path throws `ValidationError`. --- Sources/ContainerCommands/HelpCommand.swift | 39 ++++++++++++- .../HelpCommandTests.swift | 57 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 Tests/ContainerCommandsTests/HelpCommandTests.swift diff --git a/Sources/ContainerCommands/HelpCommand.swift b/Sources/ContainerCommands/HelpCommand.swift index 8d97b1c8e..43e9107c5 100644 --- a/Sources/ContainerCommands/HelpCommand.swift +++ b/Sources/ContainerCommands/HelpCommand.swift @@ -26,8 +26,43 @@ struct HelpCommand: AsyncLoggableCommand { @OptionGroup(visibility: .hidden) public var logOptions: Flags.Logging + @Argument(parsing: .captureForPassthrough) + var subcommandPath: [String] = [] + func run() async throws { - let pluginLoader = try? await Application.createPluginLoader() - await Application.printModifiedHelpText(pluginLoader: pluginLoader) + if subcommandPath.isEmpty { + let pluginLoader = try? await Application.createPluginLoader() + await Application.printModifiedHelpText(pluginLoader: pluginLoader) + return + } + guard let target = Self.resolveSubcommand(path: subcommandPath) else { + throw ValidationError("unknown command '\(subcommandPath.joined(separator: " "))'") + } + print(Application.helpMessage(for: target)) + } + + static func resolveSubcommand(path: [String]) -> ParsableCommand.Type? { + var current: ParsableCommand.Type = Application.self + for name in path { + guard let next = childSubcommands(of: current).first(where: { matches($0, name: name) }) else { + return nil + } + current = next + } + return current + } + + private static func childSubcommands(of command: ParsableCommand.Type) -> [ParsableCommand.Type] { + var all = command.configuration.subcommands + for group in command.configuration.groupedSubcommands { + all.append(contentsOf: group.subcommands) + } + return all + } + + private static func matches(_ command: ParsableCommand.Type, name: String) -> Bool { + let cfg = command.configuration + if cfg.commandName == name { return true } + return cfg.aliases.contains(name) } } diff --git a/Tests/ContainerCommandsTests/HelpCommandTests.swift b/Tests/ContainerCommandsTests/HelpCommandTests.swift new file mode 100644 index 000000000..a728c0fe8 --- /dev/null +++ b/Tests/ContainerCommandsTests/HelpCommandTests.swift @@ -0,0 +1,57 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ArgumentParser +import Foundation +import Testing + +@testable import ContainerCommands + +struct HelpCommandTests { + @Test + func everyStaticSubcommandReachableViaHelp() { + func walk(_ command: ParsableCommand.Type, path: [String]) { + let cfg = command.configuration + var children = cfg.subcommands + for group in cfg.groupedSubcommands { + children.append(contentsOf: group.subcommands) + } + for child in children { + guard let name = child.configuration.commandName else { continue } + let canonical = path + [name] + #expect( + HelpCommand.resolveSubcommand(path: canonical) != nil, + "help should resolve '\(canonical.joined(separator: " "))' but returned nil" + ) + for alias in child.configuration.aliases { + let aliasPath = path + [alias] + #expect( + HelpCommand.resolveSubcommand(path: aliasPath) != nil, + "help should resolve alias path '\(aliasPath.joined(separator: " "))' but returned nil" + ) + } + walk(child, path: canonical) + } + } + walk(Application.self, path: []) + } + + @Test + func unknownSubcommandReturnsNil() { + #expect(HelpCommand.resolveSubcommand(path: ["nonexistent"]) == nil) + #expect(HelpCommand.resolveSubcommand(path: ["image", "nonexistent"]) == nil) + } +} From d2aa01ec05ad9370f573f74afe8b079bb6108430 Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Wed, 27 May 2026 17:36:24 -0700 Subject: [PATCH 06/15] Add ability to configure integration test list at command line (#1608) ## Type of Change - [x] New feature ## Motivation and Context When developing, there are times when I want to run a specific set of CLI tests. This PR allows users to set what integration tests they want to run by setting the makefile variable `INTEGRATION_TEST_SUITES`. Example usage: ``` % INTEGRATION_TEST_SUITES="TestCLIVolumes TestCLIAnonymousVolumes" make all integration ``` ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index e177fef95..b12b1466b 100644 --- a/Makefile +++ b/Makefile @@ -187,7 +187,7 @@ define GENERATE_COV_REPORTS @cat $(COVERAGE_OUTPUT_DIR)/$(2)/coverage-percent.txt endef -INTEGRATION_TEST_SUITES := \ +INTEGRATION_TEST_SUITES ?= \ TestCLIHelp \ TestCLIStatus \ TestCLIVersion \ @@ -316,7 +316,7 @@ check-licenses: .PHONY: pre-commit pre-commit: - cp Scripts/pre-commit.fmt .git/hooks + cp scripts/pre-commit.fmt .git/hooks touch .git/hooks/pre-commit cat .git/hooks/pre-commit | grep -v 'hooks/pre-commit\.fmt' > /tmp/pre-commit.new || true echo 'PRECOMMIT_NOFMT=$${PRECOMMIT_NOFMT} $$(git rev-parse --show-toplevel)/.git/hooks/pre-commit.fmt' >> /tmp/pre-commit.new From d43b58ec737fff3ba8ffaa36c8ba16d070298262 Mon Sep 17 00:00:00 2001 From: J Logan Date: Thu, 28 May 2026 12:07:01 -0400 Subject: [PATCH 07/15] Align JSON output with shape for container resource. (#1611) - Part of #1404. - Use `configuration` for configuration properties, `state` for current state label, `status` for status properties. --- .../Network/NetworkResource.swift | 44 +++++++++++++++---- .../Subcommands/Networks/TestCLINetwork.swift | 2 +- Tests/CLITests/Utilities/CLITest.swift | 10 ++++- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/Sources/ContainerResource/Network/NetworkResource.swift b/Sources/ContainerResource/Network/NetworkResource.swift index 0463605a0..5d400016f 100644 --- a/Sources/ContainerResource/Network/NetworkResource.swift +++ b/Sources/ContainerResource/Network/NetworkResource.swift @@ -24,9 +24,9 @@ import Foundation /// config/status split used by Kubernetes and Docker. `config` is persisted; /// `status` reflects what the network plugin reports at runtime. /// -/// The JSON encoding uses a single `status` object containing a `phase` field -/// alongside any runtime-allocated address properties, replacing the prior flat -/// `state`/`status` pair in the CLI output. +/// JSON encoding produces four top-level keys: `id`, `state` (the lifecycle label: +/// `"created"` or `"running"`), `configuration` (the persistent config), and `status` +/// (runtime address properties, `null` when `state` is `"created"`). public struct NetworkResource: ManagedResource { /// The network's configuration — its persistent, intrinsic properties. public let config: NetworkConfiguration @@ -97,20 +97,48 @@ extension NetworkResource { extension NetworkResource { enum CodingKeys: String, CodingKey { case id - case config + case state + case configuration case status } + private enum StatusCodingKeys: String, CodingKey { + case ipv4Subnet + case ipv4Gateway + case ipv6Subnet + } + public func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(id, forKey: .id) - try container.encode(config, forKey: .config) - try container.encode(status, forKey: .status) + try container.encode(status.phase, forKey: .state) + try container.encode(config, forKey: .configuration) + if status.phase == "running" { + var statusContainer = container.nestedContainer(keyedBy: StatusCodingKeys.self, forKey: .status) + try statusContainer.encodeIfPresent(status.ipv4Subnet, forKey: .ipv4Subnet) + try statusContainer.encodeIfPresent(status.ipv4Gateway, forKey: .ipv4Gateway) + try statusContainer.encodeIfPresent(status.ipv6Subnet, forKey: .ipv6Subnet) + } else { + try container.encodeNil(forKey: .status) + } } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) - self.config = try container.decode(NetworkConfiguration.self, forKey: .config) - self.status = try container.decode(NetworkStatus.self, forKey: .status) + let state = try container.decode(String.self, forKey: .state) + let config = try container.decode(NetworkConfiguration.self, forKey: .configuration) + if try container.decodeNil(forKey: .status) { + self.config = config + self.status = NetworkStatus(phase: state) + } else { + let statusContainer = try container.nestedContainer(keyedBy: StatusCodingKeys.self, forKey: .status) + self.config = config + self.status = NetworkStatus( + phase: state, + ipv4Subnet: try statusContainer.decodeIfPresent(CIDRv4.self, forKey: .ipv4Subnet), + ipv4Gateway: try statusContainer.decodeIfPresent(IPv4Address.self, forKey: .ipv4Gateway), + ipv6Subnet: try statusContainer.decodeIfPresent(CIDRv6.self, forKey: .ipv6Subnet) + ) + } } } diff --git a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift index cdef26d95..6469afe69 100644 --- a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift +++ b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift @@ -192,7 +192,7 @@ class TestCLINetwork: CLITest { "foo": "bar", "baz": "qux", ] - #expect(expectedLabels == networks[0].config.labels.dictionary) + #expect(expectedLabels == networks[0].configuration.labels.dictionary) // delete should succeed _ = try run(arguments: networkDeleteArgs) diff --git a/Tests/CLITests/Utilities/CLITest.swift b/Tests/CLITests/Utilities/CLITest.swift index b3e8a6617..e086c08ed 100644 --- a/Tests/CLITests/Utilities/CLITest.swift +++ b/Tests/CLITests/Utilities/CLITest.swift @@ -47,9 +47,15 @@ class CLITest { } struct NetworkInspectOutput: Codable { + struct Status: Codable { + let ipv4Subnet: String? + let ipv4Gateway: String? + let ipv6Subnet: String? + } let id: String - let config: NetworkConfiguration - let status: NetworkStatus + let state: String + let configuration: NetworkConfiguration + let status: Status? } let testName: String From 046884df376f6bb53ba9c87ae9791c1b95a76e89 Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Thu, 28 May 2026 09:33:21 -0700 Subject: [PATCH 08/15] Use ManagedResource for volumes in CLI commands (#1607) ## Type of Change - [x] New feature ## Motivation and Context Related to https://github.com/apple/container/issues/1404. This PR adds the initial work to have volume resources conform to ManagedResource, in alignment with other resources such as networks (see [here](https://github.com/apple/container/pull/1421)). Further work is necessary to move the use of `VolumeResource` down to the APIServer (in the VolumesService) and in the volume client. Volumes do not currently have any plugin or runtime state, so that information is not included in the `VolumeResource`, but could be added later if needed. ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf --- .../ContainerCommands/OutputRendering.swift | 4 +- .../Volume/VolumeDelete.swift | 4 +- .../Volume/VolumeInspect.swift | 3 +- .../ContainerCommands/Volume/VolumeList.swift | 37 +------- .../VolumeResource+ListDisplayable.swift | 36 ++++++++ ...Volume.swift => VolumeConfiguration.swift} | 4 +- .../Volume/VolumeResource.swift | 90 +++++++++++++++++++ .../Client/ClientVolume.swift | 12 +-- .../ContainerAPIService/Client/Utility.swift | 6 +- .../Server/Volumes/VolumesService.swift | 16 ++-- .../Volumes/TestCLIAnonymousVolumes.swift | 2 +- 11 files changed, 155 insertions(+), 59 deletions(-) create mode 100644 Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift rename Sources/ContainerResource/Volume/{Volume.swift => VolumeConfiguration.swift} (97%) create mode 100644 Sources/ContainerResource/Volume/VolumeResource.swift diff --git a/Sources/ContainerCommands/OutputRendering.swift b/Sources/ContainerCommands/OutputRendering.swift index 20560f6ec..25ef9e33b 100644 --- a/Sources/ContainerCommands/OutputRendering.swift +++ b/Sources/ContainerCommands/OutputRendering.swift @@ -39,7 +39,7 @@ public struct JSONOptions: Sendable { /// /// All list commands route their output through these methods. JSON rendering /// is separate from table/quiet rendering because the JSON model often differs -/// from the display model (e.g., `Volume` for JSON vs `PrintableVolume` for table). +/// from the display model. public enum Output { /// Renders an `Encodable` value as a JSON string. public static func renderJSON(_ value: T, options: JSONOptions = .compact) throws -> String { @@ -83,7 +83,7 @@ public enum Output { /// Renders list output in the requested format. /// /// The JSON and display models may be the same type (e.g., `PrintableContainer`) - /// or different types (e.g., `Volume` for JSON and `PrintableVolume` for table). + /// or different types. public static func render( json: J, display: [D], format: ListFormat, quiet: Bool ) throws { diff --git a/Sources/ContainerCommands/Volume/VolumeDelete.swift b/Sources/ContainerCommands/Volume/VolumeDelete.swift index 327b8bd22..ab150cd82 100644 --- a/Sources/ContainerCommands/Volume/VolumeDelete.swift +++ b/Sources/ContainerCommands/Volume/VolumeDelete.swift @@ -53,7 +53,7 @@ extension Application.VolumeCommand { public func run() async throws { let uniqueVolumeNames = Set(names) - let volumes: [Volume] + let volumes: [VolumeConfiguration] if all { volumes = try await ClientVolume.list() @@ -81,7 +81,7 @@ extension Application.VolumeCommand { var failed = [String]() let _log = log - try await withThrowingTaskGroup(of: Volume?.self) { group in + try await withThrowingTaskGroup(of: VolumeConfiguration?.self) { group in for volume in volumes { group.addTask { do { diff --git a/Sources/ContainerCommands/Volume/VolumeInspect.swift b/Sources/ContainerCommands/Volume/VolumeInspect.swift index 8110a1d3b..7c2367a35 100644 --- a/Sources/ContainerCommands/Volume/VolumeInspect.swift +++ b/Sources/ContainerCommands/Volume/VolumeInspect.swift @@ -38,6 +38,7 @@ extension Application.VolumeCommand { public func run() async throws { let uniqueNames = Set(names) let volumes = try await ClientVolume.list().filter { uniqueNames.contains($0.id) } + let volumeResources = volumes.map { VolumeResource(config: $0) } if volumes.count != uniqueNames.count { let found = Set(volumes.map { $0.id }) @@ -52,7 +53,7 @@ extension Application.VolumeCommand { outputFormatting: [.prettyPrinted, .sortedKeys], dateEncodingStrategy: .iso8601 ) - try Output.emit(Output.renderJSON(volumes, options: options)) + try Output.emit(Output.renderJSON(volumeResources, options: options)) } } } diff --git a/Sources/ContainerCommands/Volume/VolumeList.swift b/Sources/ContainerCommands/Volume/VolumeList.swift index 124205b63..ea833b551 100644 --- a/Sources/ContainerCommands/Volume/VolumeList.swift +++ b/Sources/ContainerCommands/Volume/VolumeList.swift @@ -41,46 +41,15 @@ extension Application.VolumeCommand { public func run() async throws { let volumes = try await ClientVolume.list() + let volumeResources = volumes.map { VolumeResource(config: $0) } if format == .json { let options = JSONOptions(dateEncodingStrategy: .iso8601) - try Output.emit(Output.renderJSON(volumes, options: options)) + try Output.emit(Output.renderJSON(volumeResources, options: options)) return } - // Sort by creation time (newest first) for table display only, - // matching the original behavior where JSON and quiet emit unsorted. - let items = quiet ? volumes : volumes.sorted { $0.createdAt > $1.createdAt } - Output.emit(Output.renderList(items.map { PrintableVolume($0) }, quiet: quiet)) + try Output.render(json: volumeResources, display: volumeResources, format: format, quiet: quiet) } } } - -private struct PrintableVolume: ListDisplayable { - let name: String - let volumeType: String - let driver: String - let optionsString: String - - init(_ volume: Volume) { - self.name = volume.name - self.volumeType = volume.isAnonymous ? "anonymous" : "named" - self.driver = volume.driver - self.optionsString = - volume.options.isEmpty - ? "" - : volume.options.sorted(by: { $0.key < $1.key }).map { "\($0.key)=\($0.value)" }.joined(separator: ",") - } - - static var tableHeader: [String] { - ["NAME", "TYPE", "DRIVER", "OPTIONS"] - } - - var tableRow: [String] { - [name, volumeType, driver, optionsString] - } - - var quietValue: String { - name - } -} diff --git a/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift b/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift new file mode 100644 index 000000000..c16b15b6a --- /dev/null +++ b/Sources/ContainerCommands/Volume/VolumeResource+ListDisplayable.swift @@ -0,0 +1,36 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ContainerResource + +extension VolumeResource: ListDisplayable { + public static var tableHeader: [String] { + ["NAME", "TYPE", "DRIVER", "OPTIONS"] + } + + public var tableRow: [String] { + [ + name, + isAnonymous ? "anonymous" : "named", + config.driver, + config.options.isEmpty ? "" : config.options.sorted(by: { $0.key < $1.key }).map { "\($0.key)=\($0.value)" }.joined(separator: ","), + ] + } + + public var quietValue: String { + name + } +} diff --git a/Sources/ContainerResource/Volume/Volume.swift b/Sources/ContainerResource/Volume/VolumeConfiguration.swift similarity index 97% rename from Sources/ContainerResource/Volume/Volume.swift rename to Sources/ContainerResource/Volume/VolumeConfiguration.swift index fe4de996a..cc35e203d 100644 --- a/Sources/ContainerResource/Volume/Volume.swift +++ b/Sources/ContainerResource/Volume/VolumeConfiguration.swift @@ -17,7 +17,7 @@ import Foundation /// A named or anonymous volume that can be mounted in containers. -public struct Volume: Sendable, Codable, Equatable, Identifiable { +public struct VolumeConfiguration: Sendable, Codable, Equatable, Identifiable { // id of the volume. public var id: String { name } // Name of the volume. @@ -58,7 +58,7 @@ public struct Volume: Sendable, Codable, Equatable, Identifiable { } } -extension Volume { +extension VolumeConfiguration { /// Reserved label key for marking anonymous volumes public static let anonymousLabel = "com.apple.container.resource.anonymous" diff --git a/Sources/ContainerResource/Volume/VolumeResource.swift b/Sources/ContainerResource/Volume/VolumeResource.swift new file mode 100644 index 000000000..5eb7b8f0d --- /dev/null +++ b/Sources/ContainerResource/Volume/VolumeResource.swift @@ -0,0 +1,90 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation + +/// A volume resource, representing a configured volume. +public struct VolumeResource: ManagedResource { + /// The volume's configuration — its persistent, intrinsic properties. + public let config: VolumeConfiguration + + // MARK: ManagedResource + + /// The unique identifier for this volume. Identical to ``VolumeConfiguration/name``. + public var id: String { config.name } + + /// The user-assigned name for this volume. For volumes, name and ID are the same. + public var name: String { config.name } + + /// The time at which this volume was created. + public var creationDate: Date { config.createdAt } + + /// Key-value labels for this volume. If the underlying + /// ``VolumeConfiguration/labels`` dictionary contains values that fail + /// ``ResourceLabels`` validation, this returns an empty label set. + public var labels: ResourceLabels { + (try? ResourceLabels(config.labels)) ?? ResourceLabels() + } + + /// Whether this is an anonymous volume (detected via the configuration's labels). + public var isAnonymous: Bool { config.isAnonymous } + + // MARK: Initialization + + /// Creates a volume resource. + /// + /// - Parameters: + /// - config: The volume's intrinsic configuration. + public init(config: VolumeConfiguration) { + self.config = config + } +} + +extension VolumeResource { + public static let volumeNamePattern = "^[A-Za-z0-9][A-Za-z0-9_.-]*$" + + /// Returns `true` if `name` is a syntactically valid volume identifier. + public static func nameValid(_ name: String) -> Bool { + guard name.count <= 255 else { return false } + + do { + let regex = try Regex(volumeNamePattern) + return (try? regex.wholeMatch(in: name)) != nil + } catch { + return false + } + } +} + +// MARK: - Codable + +extension VolumeResource { + enum CodingKeys: String, CodingKey { + case id + case config + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(id, forKey: .id) + try container.encode(config, forKey: .config) + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.config = try container.decode(VolumeConfiguration.self, forKey: .config) + } +} diff --git a/Sources/Services/ContainerAPIService/Client/ClientVolume.swift b/Sources/Services/ContainerAPIService/Client/ClientVolume.swift index 8100c4120..36cf24439 100644 --- a/Sources/Services/ContainerAPIService/Client/ClientVolume.swift +++ b/Sources/Services/ContainerAPIService/Client/ClientVolume.swift @@ -27,7 +27,7 @@ public struct ClientVolume { driver: String = "local", driverOpts: [String: String] = [:], labels: [String: String] = [:] - ) async throws -> Volume { + ) async throws -> VolumeConfiguration { let client = XPCClient(service: serviceIdentifier) let message = XPCMessage(route: .volumeCreate) message.set(key: .volumeName, value: name) @@ -45,7 +45,7 @@ public struct ClientVolume { throw VolumeError.storageError("invalid response from server") } - return try JSONDecoder().decode(Volume.self, from: responseData) + return try JSONDecoder().decode(VolumeConfiguration.self, from: responseData) } public static func delete(name: String) async throws { @@ -56,7 +56,7 @@ public struct ClientVolume { _ = try await client.send(message) } - public static func list() async throws -> [Volume] { + public static func list() async throws -> [VolumeConfiguration] { let client = XPCClient(service: serviceIdentifier) let message = XPCMessage(route: .volumeList) let reply = try await client.send(message) @@ -65,10 +65,10 @@ public struct ClientVolume { return [] } - return try JSONDecoder().decode([Volume].self, from: responseData) + return try JSONDecoder().decode([VolumeConfiguration].self, from: responseData) } - public static func inspect(_ name: String) async throws -> Volume { + public static func inspect(_ name: String) async throws -> VolumeConfiguration { let client = XPCClient(service: serviceIdentifier) let message = XPCMessage(route: .volumeInspect) message.set(key: .volumeName, value: name) @@ -79,7 +79,7 @@ public struct ClientVolume { throw VolumeError.volumeNotFound(name) } - return try JSONDecoder().decode(Volume.self, from: responseData) + return try JSONDecoder().decode(VolumeConfiguration.self, from: responseData) } public static func volumeDiskUsage(name: String) async throws -> UInt64 { diff --git a/Sources/Services/ContainerAPIService/Client/Utility.swift b/Sources/Services/ContainerAPIService/Client/Utility.swift index cf5b8d6df..f649b68aa 100644 --- a/Sources/Services/ContainerAPIService/Client/Utility.swift +++ b/Sources/Services/ContainerAPIService/Client/Utility.swift @@ -367,10 +367,10 @@ public struct Utility { /// Gets an existing volume or creates it if it doesn't exist. /// Shows a warning for named volumes when auto-creating. - private static func getOrCreateVolume(parsed: ParsedVolume, log: Logger) async throws -> Volume { - let labels = parsed.isAnonymous ? [Volume.anonymousLabel: ""] : [:] + private static func getOrCreateVolume(parsed: ParsedVolume, log: Logger) async throws -> VolumeConfiguration { + let labels = parsed.isAnonymous ? [VolumeConfiguration.anonymousLabel: ""] : [:] - let volume: Volume + let volume: VolumeConfiguration var wasCreated = false do { volume = try await ClientVolume.create( diff --git a/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift b/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift index a90343bc9..1d589f834 100644 --- a/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift +++ b/Sources/Services/ContainerAPIService/Server/Volumes/VolumesService.swift @@ -28,7 +28,7 @@ import SystemPackage public actor VolumesService { private let resourceRoot: FilePath - private let store: ContainerPersistence.FilesystemEntityStore + private let store: ContainerPersistence.FilesystemEntityStore private let log: Logger private let lock = AsyncLock() private let containersService: ContainersService @@ -40,7 +40,7 @@ public actor VolumesService { public init(resourceRoot: FilePath, containersService: ContainersService, log: Logger) throws { try FileManager.default.createDirectory(atPath: resourceRoot.string, withIntermediateDirectories: true) self.resourceRoot = resourceRoot - self.store = try FilesystemEntityStore(path: resourceRoot, type: "volumes", log: log) + self.store = try FilesystemEntityStore(path: resourceRoot, type: "volumes", log: log) self.containersService = containersService self.log = log } @@ -50,7 +50,7 @@ public actor VolumesService { driver: String = "local", driverOpts: [String: String] = [:], labels: [String: String] = [:] - ) async throws -> Volume { + ) async throws -> VolumeConfiguration { log.debug( "VolumesService: enter", metadata: [ @@ -96,7 +96,7 @@ public actor VolumesService { } } - public func list() async throws -> [Volume] { + public func list() async throws -> [VolumeConfiguration] { log.debug( "VolumesService: enter", metadata: [ @@ -115,7 +115,7 @@ public actor VolumesService { return try await store.list() } - public func inspect(_ name: String) async throws -> Volume { + public func inspect(_ name: String) async throws -> VolumeConfiguration { log.debug( "VolumesService: enter", metadata: [ @@ -325,7 +325,7 @@ public actor VolumesService { driver: String, driverOpts: [String: String], labels: [String: String] - ) async throws -> Volume { + ) async throws -> VolumeConfiguration { guard VolumeStorage.isValidVolumeName(name) else { throw VolumeError.invalidVolumeName("invalid volume name '\(name)': must match \(VolumeStorage.volumeNamePattern)") } @@ -350,7 +350,7 @@ public actor VolumesService { try createVolumeImage(for: name, sizeInBytes: sizeInBytes, journal: journalConfig) - let volume = Volume( + let volume = VolumeConfiguration( name: name, driver: driver, format: "ext4", @@ -400,7 +400,7 @@ public actor VolumesService { log.info("deleted volume", metadata: ["name": "\(name)"]) } - private func _inspect(_ name: String) async throws -> Volume { + private func _inspect(_ name: String) async throws -> VolumeConfiguration { guard VolumeStorage.isValidVolumeName(name) else { throw VolumeError.invalidVolumeName("invalid volume name '\(name)': must match \(VolumeStorage.volumeNamePattern)") } diff --git a/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift b/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift index 1cd3295b1..81b920906 100644 --- a/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift +++ b/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift @@ -321,7 +321,7 @@ class TestCLIAnonymousVolumes: CLITest { let data = output.data(using: .utf8)! let decoder = JSONDecoder() decoder.dateDecodingStrategy = .iso8601 - let volumes = try decoder.decode([Volume].self, from: data) + let volumes = try decoder.decode([VolumeResource].self, from: data) let anonVolume = volumes.first { $0.name == volumeName } #expect(anonVolume != nil, "should find anonymous volume in list") From 445c90f927c0e09b53b5f48ca7d337a0350081b3 Mon Sep 17 00:00:00 2001 From: J Logan Date: Thu, 28 May 2026 14:12:04 -0400 Subject: [PATCH 09/15] Fix potential integer math crash on `PublishPort`. (#1612) - Closes #1610. - Discovered, and originally filed as a security advisory, by: PresidentL <131139636+liyander@users.noreply.github.com>. - `PublishPort` currently can store invalid combinations of starting port and range that can overflow UInt16 values when summed, crashing the process. - Updates `PublishPort` to validate inputs on initialization. --- .../Container/PublishPort.swift | 23 +++++- .../ContainerAPIService/Client/Parser.swift | 2 +- .../PublishPortTests.swift | 73 +++++++++++++++++-- 3 files changed, 86 insertions(+), 12 deletions(-) diff --git a/Sources/ContainerResource/Container/PublishPort.swift b/Sources/ContainerResource/Container/PublishPort.swift index 6895a924c..fe6ad00f4 100644 --- a/Sources/ContainerResource/Container/PublishPort.swift +++ b/Sources/ContainerResource/Container/PublishPort.swift @@ -14,6 +14,7 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError import ContainerizationExtras /// The network protocols available for port forwarding. @@ -54,12 +55,20 @@ public struct PublishPort: Sendable, Codable { public let count: UInt16 /// Creates a new port forwarding specification. - public init(hostAddress: IPAddress, hostPort: UInt16, containerPort: UInt16, proto: PublishProtocol, count: UInt16) { + public init( + hostAddress: IPAddress, + hostPort: UInt16, + containerPort: UInt16, + proto: PublishProtocol, + count: UInt16 + ) throws { self.hostAddress = hostAddress self.hostPort = hostPort self.containerPort = containerPort self.proto = proto self.count = count + try validatePortRange(port: hostPort, count: count) + try validatePortRange(port: containerPort, count: count) } /// Create a configuration from the supplied Decoder, initializing missing @@ -72,6 +81,14 @@ public struct PublishPort: Sendable, Codable { containerPort = try container.decode(UInt16.self, forKey: .containerPort) proto = try container.decode(PublishProtocol.self, forKey: .proto) count = try container.decodeIfPresent(UInt16.self, forKey: .count) ?? 1 + try validatePortRange(port: hostPort, count: count) + try validatePortRange(port: containerPort, count: count) + } + + private func validatePortRange(port: UInt16, count: UInt16) throws { + guard count > 0, UInt16.max - port >= count - 1 else { + throw ContainerizationError(.invalidArgument, message: "invalid port and count: \(port), \(count)") + } } } @@ -79,8 +96,8 @@ extension [PublishPort] { public func hasOverlaps() -> Bool { var hostPorts = Set() for publishPort in self { - for index in publishPort.hostPort..<(publishPort.hostPort + publishPort.count) { - let hostPortKey = "\(index)/\(publishPort.proto.rawValue)" + for offset in 0.. Date: Sat, 9 May 2026 09:20:33 -0700 Subject: [PATCH 10/15] Use FilePath for PublishSocket. Closes CHAOS-1459. Unblocks CHAOS-1463 (Parser). --- .../Container/PublishSocket.swift | 87 ++++++- .../ContainerAPIService/Client/Parser.swift | 5 +- .../RuntimeLinux/Server/RuntimeService.swift | 5 +- .../PublishSocketTests.swift | 224 ++++++++++++++++++ 4 files changed, 313 insertions(+), 8 deletions(-) create mode 100644 Tests/ContainerResourceTests/PublishSocketTests.swift diff --git a/Sources/ContainerResource/Container/PublishSocket.swift b/Sources/ContainerResource/Container/PublishSocket.swift index 08ea752bf..1763b01ca 100644 --- a/Sources/ContainerResource/Container/PublishSocket.swift +++ b/Sources/ContainerResource/Container/PublishSocket.swift @@ -20,21 +20,100 @@ import SystemPackage /// Represents a socket that should be published from container to host. public struct PublishSocket: Sendable, Codable { /// The path to the socket in the container. - public var containerPath: URL + public var containerPath: FilePath /// The path where the socket should appear on the host. - public var hostPath: URL + public var hostPath: FilePath /// File permissions for the socket on the host. public var permissions: FilePermissions? public init( - containerPath: URL, - hostPath: URL, + containerPath: FilePath, + hostPath: FilePath, permissions: FilePermissions? = nil ) { self.containerPath = containerPath self.hostPath = hostPath self.permissions = permissions } + + private enum CodingKeys: String, CodingKey { + case containerPath + case hostPath + case permissions + } + + /// Encode paths as file-URL absolute strings (e.g. `"file:///var/run/docker.sock"`). + /// + /// These fields were previously typed `URL`; `JSONEncoder` special-cases + /// `URL` to emit `absoluteString`. `FilePath`'s synthesized `Codable` + /// would instead emit a keyed container (`{"_storage": "..."}`), changing + /// the on-disk and XPC wire format. We therefore encode each path as the + /// equivalent `URL.absoluteString` so the byte form remains compatible + /// with persisted bundles and any readers (e.g. an older service binary) + /// that still decode these fields as `URL`. + public func encode(to encoder: any Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(Self.encodePath(containerPath), forKey: .containerPath) + try container.encode(Self.encodePath(hostPath), forKey: .hostPath) + try container.encodeIfPresent(permissions, forKey: .permissions) + } + + public init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.containerPath = try Self.decodePath(from: container, forKey: .containerPath) + self.hostPath = try Self.decodePath(from: container, forKey: .hostPath) + self.permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) + } + + /// Encode a `FilePath` as a file-URL `absoluteString` to match the prior + /// `URL`-typed wire format byte-for-byte. + private static func encodePath(_ path: FilePath) -> String { + URL(filePath: path.string).absoluteString + } + + /// Decode a `FilePath` from either the canonical file-URL form + /// (e.g. `"file:///foo"`) emitted by `encodePath(_:)` and the legacy + /// `URL`-typed wire format, or a plain absolute path string. Throws + /// `DecodingError.dataCorrupted` on malformed, empty, or non-absolute + /// inputs so corrupt persisted state fails loudly rather than silently + /// producing an invalid socket path. + private static func decodePath( + from container: KeyedDecodingContainer, + forKey key: CodingKeys + ) throws -> FilePath { + let raw = try container.decode(String.self, forKey: key) + + let path: String + if raw.hasPrefix("file:") { + guard let url = URL(string: raw), url.isFileURL else { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "malformed file URL: \(raw)" + ) + } + if let host = url.host(), !host.isEmpty, host != "localhost" { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "file URL host must be empty or 'localhost': \(raw)" + ) + } + path = url.path(percentEncoded: false) + } else { + path = raw + } + + guard path.hasPrefix("/") else { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "socket path must be absolute: \(raw)" + ) + } + + return FilePath(path) + } } diff --git a/Sources/Services/ContainerAPIService/Client/Parser.swift b/Sources/Services/ContainerAPIService/Client/Parser.swift index 7cbe7e0b0..16cb94e72 100644 --- a/Sources/Services/ContainerAPIService/Client/Parser.swift +++ b/Sources/Services/ContainerAPIService/Client/Parser.swift @@ -22,6 +22,7 @@ import ContainerizationExtras import ContainerizationOCI import ContainerizationOS import Foundation +import SystemPackage /// A parsed volume specification from user input public struct ParsedVolume { @@ -787,8 +788,8 @@ public struct Parser { // Create and return PublishSocket object with validated paths return PublishSocket( - containerPath: URL(fileURLWithPath: containerPath), - hostPath: URL(fileURLWithPath: absoluteHostPath), + containerPath: FilePath(containerPath), + hostPath: FilePath(absoluteHostPath), permissions: nil ) diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index 187de7af6..d4d468108 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -999,9 +999,10 @@ public actor RuntimeService { } for publishedSocket in config.publishedSockets { + // UnixSocketConfiguration (Containerization) takes URL; convert from FilePath at the boundary. let socketConfig = UnixSocketConfiguration( - source: publishedSocket.containerPath, - destination: publishedSocket.hostPath, + source: URL(filePath: publishedSocket.containerPath.string), + destination: URL(filePath: publishedSocket.hostPath.string), permissions: publishedSocket.permissions, direction: .outOf ) diff --git a/Tests/ContainerResourceTests/PublishSocketTests.swift b/Tests/ContainerResourceTests/PublishSocketTests.swift new file mode 100644 index 000000000..4e7ab32ac --- /dev/null +++ b/Tests/ContainerResourceTests/PublishSocketTests.swift @@ -0,0 +1,224 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation +import SystemPackage +import Testing + +@testable import ContainerResource + +/// Tests covering the custom `Codable` implementation on ``PublishSocket``. +/// +/// `containerPath` and `hostPath` were migrated from `URL` to `FilePath`. +/// Because `URL` is encoded by `JSONEncoder` as its `absoluteString` (e.g. +/// `"file:///var/run/docker.sock"`), persisted bundles and any service +/// binaries still typed against `URL` expect that exact byte format on the +/// wire. The tests below pin the encoder to that legacy shape and verify the +/// decoder accepts both the legacy file-URL form and a plain absolute path. +struct PublishSocketTests { + // MARK: - Encoding + + @Test + func testEncodeProducesFileURLAbsoluteString() throws { + let socket = PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("/Users/me/docker.sock") + ) + let json = try JSONEncoder().encode(socket) + let decoded = try JSONSerialization.jsonObject(with: json) as? [String: Any] + let container = try #require(decoded) + #expect(container["containerPath"] as? String == "file:///var/run/docker.sock") + #expect(container["hostPath"] as? String == "file:///Users/me/docker.sock") + #expect(container["permissions"] == nil) + } + + @Test + func testEncodeIsByteCompatibleWithLegacyURLEncoding() throws { + // Pre-migration, these fields were `URL` and JSON-encoded via + // `URL.absoluteString`. Encoding must produce the exact same bytes + // for any reader still decoding them as `URL`. + let containerPath = "/var/run/docker.sock" + let hostPath = "/Users/me/docker.sock" + let legacy = LegacyPublishSocket( + containerPath: URL(fileURLWithPath: containerPath), + hostPath: URL(fileURLWithPath: hostPath) + ) + let current = PublishSocket( + containerPath: FilePath(containerPath), + hostPath: FilePath(hostPath) + ) + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + #expect(try encoder.encode(current) == encoder.encode(legacy)) + } + + @Test + func testEncodePercentEncodesSpacesAndSpecialCharacters() throws { + let socket = PublishSocket( + containerPath: FilePath("/tmp/a b.sock"), + hostPath: FilePath("/tmp/dir with spaces/sock") + ) + let json = try JSONEncoder().encode(socket) + let decoded = try #require(try JSONSerialization.jsonObject(with: json) as? [String: Any]) + #expect(decoded["containerPath"] as? String == "file:///tmp/a%20b.sock") + #expect(decoded["hostPath"] as? String == "file:///tmp/dir%20with%20spaces/sock") + } + + // MARK: - Decoding (canonical / legacy file-URL form) + + @Test + func testDecodeFileURLForm() throws { + let json = """ + {"containerPath":"file:///var/run/docker.sock","hostPath":"file:///Users/me/docker.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + #expect(socket.permissions == nil) + } + + @Test + func testDecodeFileURLPercentEncodingIsResolved() throws { + // Persisted bundles created via `URL(fileURLWithPath:)` percent-encode + // spaces; decoding must yield the original literal path. + let json = """ + {"containerPath":"file:///tmp/a%20b.sock","hostPath":"file:///tmp/x%2Fy.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/tmp/a b.sock")) + // `%2F` decodes to a literal `/` inside the path component. + #expect(socket.hostPath == FilePath("/tmp/x/y.sock")) + } + + @Test + func testDecodeFileURLWithLocalhostHost() throws { + let json = """ + {"containerPath":"file://localhost/var/run/docker.sock","hostPath":"file:///host.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/host.sock")) + } + + // MARK: - Decoding (forward-compat plain path form) + + @Test + func testDecodePlainAbsolutePath() throws { + // Forward-compat: if some other encoder emits a clean path string, + // accept it rather than rejecting at the boundary. + let json = """ + {"containerPath":"/var/run/docker.sock","hostPath":"/Users/me/docker.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + } + + // MARK: - Round-trip + + @Test + func testRoundTrip() throws { + let original = PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("/tmp/socket with spaces.sock"), + permissions: FilePermissions(rawValue: 0o660) + ) + let encoder = JSONEncoder() + let decoder = JSONDecoder() + let data = try encoder.encode(original) + let decoded = try decoder.decode(PublishSocket.self, from: data) + #expect(decoded.containerPath == original.containerPath) + #expect(decoded.hostPath == original.hostPath) + #expect(decoded.permissions == original.permissions) + } + + // MARK: - Decoding errors + + @Test + func testDecodeEmptyStringThrows() { + let json = """ + {"containerPath":"","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeFileColonOnlyThrows() { + // `"file:"` parses as a URL but yields an empty path; reject loudly + // rather than silently producing `FilePath("")`. + let json = """ + {"containerPath":"file:","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeFileSchemeNoPathThrows() { + let json = """ + {"containerPath":"file://","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeRelativePathThrows() { + let json = """ + {"containerPath":"relative/path.sock","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeNonLocalHostFileURLThrows() { + // file URLs with a non-empty / non-localhost host are unsafe to + // interpret as a local path. + let json = """ + {"containerPath":"file://example.com/etc/passwd","hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + + @Test + func testDecodeMissingRequiredKeyThrows() { + let json = """ + {"hostPath":"/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } +} + +// MARK: - Helpers + +/// Mirrors the pre-migration `URL`-typed `PublishSocket` shape, used only to +/// confirm the new encoder produces byte-identical output for any reader +/// still decoding these fields as `URL`. +private struct LegacyPublishSocket: Encodable { + var containerPath: URL + var hostPath: URL + var permissions: FilePermissions? +} From c572ff18adfb520b3d941a0b96d83e198bb65f00 Mon Sep 17 00:00:00 2001 From: Chris George Date: Wed, 27 May 2026 12:09:29 -0700 Subject: [PATCH 11/15] Address PR #1594 review feedback - PublishSocket.init now throws and validates absolute paths so objects are correct by construction; adds DocC for parameters and constraints (addresses comment on PublishSocket.swift:33 and :72). - Adds deprecation/migration note on the struct documenting that the decoder accepts both new `FilePath` and legacy `URL` forms for one release (addresses comment on PublishSocket.swift:21). - Wire format pre-1.0 breaking change: encoder now emits the plain absolute path (e.g. "/var/run/docker.sock") instead of the legacy file-URL form ("file:///var/run/docker.sock"). The decoder still accepts both forms so persisted bundles from earlier releases continue to load; that compatibility will be removed in a later release (addresses comment on PublishSocket.swift:58). - Bumps containerization to 0.33.2 and replaces URL(fileURLWithPath:).absoluteURL.path with ContainerizationOS.FilePathOps.absolutePath in Parser.publishSocket, removing the duplicate "must be absolute" check now that init() enforces it (addresses comments on Parser.swift:754 and :762). --- Package.resolved | 6 +- Package.swift | 2 +- .../Container/PublishSocket.swift | 91 +++++++---- .../ContainerAPIService/Client/Parser.swift | 31 ++-- .../PublishSocketTests.swift | 143 +++++++++--------- 5 files changed, 148 insertions(+), 125 deletions(-) diff --git a/Package.resolved b/Package.resolved index 6d19fb55d..3fdbb381c 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "d9d032f8b1ad94cf9006bd4b441ce1dad52802b04b1d7f7b30baedd32cee0e8b", + "originHash" : "92e9d30c1bb3bc52ba8c855f83a17db4752940b9438a9f5d20fe5b7c1328aae0", "pins" : [ { "identity" : "async-http-client", @@ -15,8 +15,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/containerization.git", "state" : { - "revision" : "9205a766302a18e06a0ce43f8b7e483625e3e50f", - "version" : "0.33.1" + "revision" : "2550dd49f1890702f6fe0171212050bbce9d3825", + "version" : "0.33.2" } }, { diff --git a/Package.swift b/Package.swift index 2cc2e7092..f6dda67b9 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ import PackageDescription let releaseVersion = ProcessInfo.processInfo.environment["RELEASE_VERSION"] ?? "0.0.0" let gitCommit = ProcessInfo.processInfo.environment["GIT_COMMIT"] ?? "unspecified" let builderShimVersion = "0.12.0" -let scVersion = "0.33.1" +let scVersion = "0.33.2" let package = Package( name: "container", diff --git a/Sources/ContainerResource/Container/PublishSocket.swift b/Sources/ContainerResource/Container/PublishSocket.swift index 1763b01ca..4b66c9076 100644 --- a/Sources/ContainerResource/Container/PublishSocket.swift +++ b/Sources/ContainerResource/Container/PublishSocket.swift @@ -14,25 +14,52 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError import Foundation import SystemPackage /// Represents a socket that should be published from container to host. +/// +/// - Deprecated: New for 1.0.0, path types changed from `URL` to `FilePath`. +/// - Note: Decoder handles `FilePath` and `URL` for persistent data compatibility; +/// this compatibility will be removed in a later release. public struct PublishSocket: Sendable, Codable { - /// The path to the socket in the container. + /// Absolute path to the socket inside the container. public var containerPath: FilePath - /// The path where the socket should appear on the host. + /// Absolute path where the socket appears on the host. public var hostPath: FilePath /// File permissions for the socket on the host. public var permissions: FilePermissions? + /// Creates a `PublishSocket` with validated absolute paths. + /// + /// - Parameters: + /// - containerPath: Absolute path to the socket inside the container. + /// Must begin with `/`. + /// - hostPath: Absolute path where the socket appears on the host. + /// Must begin with `/`. + /// - permissions: File permissions applied to the socket on the host. + /// - Throws: `ContainerizationError` with code `.invalidArgument` if + /// either path is not absolute. public init( containerPath: FilePath, hostPath: FilePath, permissions: FilePermissions? = nil - ) { + ) throws { + guard containerPath.isAbsolute else { + throw ContainerizationError( + .invalidArgument, + message: "containerPath must be absolute: \(containerPath)" + ) + } + guard hostPath.isAbsolute else { + throw ContainerizationError( + .invalidArgument, + message: "hostPath must be absolute: \(hostPath)" + ) + } self.containerPath = containerPath self.hostPath = hostPath self.permissions = permissions @@ -44,41 +71,45 @@ public struct PublishSocket: Sendable, Codable { case permissions } - /// Encode paths as file-URL absolute strings (e.g. `"file:///var/run/docker.sock"`). + /// Encodes each path as its plain absolute string (e.g. `"/var/run/docker.sock"`). /// - /// These fields were previously typed `URL`; `JSONEncoder` special-cases - /// `URL` to emit `absoluteString`. `FilePath`'s synthesized `Codable` - /// would instead emit a keyed container (`{"_storage": "..."}`), changing - /// the on-disk and XPC wire format. We therefore encode each path as the - /// equivalent `URL.absoluteString` so the byte form remains compatible - /// with persisted bundles and any readers (e.g. an older service binary) - /// that still decode these fields as `URL`. + /// Pre-1.0 wire-format change from the prior `URL`-typed encoding which + /// emitted `URL.absoluteString` (`"file:///var/run/docker.sock"`). The + /// decoder accepts both forms for compatibility with persisted bundles + /// from earlier releases; that compatibility will be removed in a later + /// release. public func encode(to encoder: any Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) - try container.encode(Self.encodePath(containerPath), forKey: .containerPath) - try container.encode(Self.encodePath(hostPath), forKey: .hostPath) + try container.encode(containerPath.string, forKey: .containerPath) + try container.encode(hostPath.string, forKey: .hostPath) try container.encodeIfPresent(permissions, forKey: .permissions) } public init(from decoder: any Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) - self.containerPath = try Self.decodePath(from: container, forKey: .containerPath) - self.hostPath = try Self.decodePath(from: container, forKey: .hostPath) - self.permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) - } - - /// Encode a `FilePath` as a file-URL `absoluteString` to match the prior - /// `URL`-typed wire format byte-for-byte. - private static func encodePath(_ path: FilePath) -> String { - URL(filePath: path.string).absoluteString + let containerPath = try Self.decodePath(from: container, forKey: .containerPath) + let hostPath = try Self.decodePath(from: container, forKey: .hostPath) + let permissions = try container.decodeIfPresent(FilePermissions.self, forKey: .permissions) + do { + try self.init( + containerPath: containerPath, + hostPath: hostPath, + permissions: permissions + ) + } catch let error as ContainerizationError { + throw DecodingError.dataCorruptedError( + forKey: .containerPath, + in: container, + debugDescription: String(describing: error) + ) + } } - /// Decode a `FilePath` from either the canonical file-URL form - /// (e.g. `"file:///foo"`) emitted by `encodePath(_:)` and the legacy - /// `URL`-typed wire format, or a plain absolute path string. Throws - /// `DecodingError.dataCorrupted` on malformed, empty, or non-absolute - /// inputs so corrupt persisted state fails loudly rather than silently - /// producing an invalid socket path. + /// Decodes a `FilePath` accepting either the new plain-path form + /// (`"/var/run/docker.sock"`) or the legacy file-URL form emitted by + /// older releases (`"file:///var/run/docker.sock"`). Throws + /// `DecodingError.dataCorrupted` on a malformed file URL or empty input. + /// Absoluteness is enforced in `init(containerPath:hostPath:permissions:)`. private static func decodePath( from container: KeyedDecodingContainer, forKey key: CodingKeys @@ -106,11 +137,11 @@ public struct PublishSocket: Sendable, Codable { path = raw } - guard path.hasPrefix("/") else { + guard !path.isEmpty else { throw DecodingError.dataCorruptedError( forKey: key, in: container, - debugDescription: "socket path must be absolute: \(raw)" + debugDescription: "decoded socket path is empty: \(raw)" ) } diff --git a/Sources/Services/ContainerAPIService/Client/Parser.swift b/Sources/Services/ContainerAPIService/Client/Parser.swift index 16cb94e72..61779498e 100644 --- a/Sources/Services/ContainerAPIService/Client/Parser.swift +++ b/Sources/Services/ContainerAPIService/Client/Parser.swift @@ -740,7 +740,6 @@ public struct Parser { let hostPath = String(parts[0]) let containerPath = String(parts[1]) - // Validate paths are not empty if hostPath.isEmpty { throw ContainerizationError( .invalidArgument, message: "host socket path cannot be empty") @@ -750,28 +749,18 @@ public struct Parser { .invalidArgument, message: "container socket path cannot be empty") } - // Ensure container path must start with / - if !containerPath.hasPrefix("/") { - throw ContainerizationError( - .invalidArgument, - message: "container socket path must be absolute: \(containerPath)") - } - - // Convert host path to absolute path for consistency - let hostURL = URL(fileURLWithPath: hostPath) - let absoluteHostPath = hostURL.absoluteURL.path + let absoluteHostPath = FilePathOps.absolutePath(FilePath(hostPath)) - // Check if host socket already exists and might be in use - if FileManager.default.fileExists(atPath: absoluteHostPath) { + if FileManager.default.fileExists(atPath: absoluteHostPath.string) { do { - let attrs = try FileManager.default.attributesOfItem(atPath: absoluteHostPath) + let attrs = try FileManager.default.attributesOfItem(atPath: absoluteHostPath.string) if let fileType = attrs[.type] as? FileAttributeType, fileType == .typeSocket { throw ContainerizationError( .invalidArgument, message: "host socket \(absoluteHostPath) already exists and may be in use") } // If it exists but is not a socket, we can remove it and create socket - try FileManager.default.removeItem(atPath: absoluteHostPath) + try FileManager.default.removeItem(atPath: absoluteHostPath.string) } catch let error as ContainerizationError { throw error } catch { @@ -779,17 +768,15 @@ public struct Parser { } } - // Create host directory if it doesn't exist - let hostDir = hostURL.deletingLastPathComponent() - if !FileManager.default.fileExists(atPath: hostDir.path) { + let hostDir = absoluteHostPath.removingLastComponent() + if !FileManager.default.fileExists(atPath: hostDir.string) { try FileManager.default.createDirectory( - at: hostDir, withIntermediateDirectories: true) + atPath: hostDir.string, withIntermediateDirectories: true) } - // Create and return PublishSocket object with validated paths - return PublishSocket( + return try PublishSocket( containerPath: FilePath(containerPath), - hostPath: FilePath(absoluteHostPath), + hostPath: absoluteHostPath, permissions: nil ) diff --git a/Tests/ContainerResourceTests/PublishSocketTests.swift b/Tests/ContainerResourceTests/PublishSocketTests.swift index 4e7ab32ac..2bfb0639c 100644 --- a/Tests/ContainerResourceTests/PublishSocketTests.swift +++ b/Tests/ContainerResourceTests/PublishSocketTests.swift @@ -14,73 +14,101 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError import Foundation import SystemPackage import Testing @testable import ContainerResource -/// Tests covering the custom `Codable` implementation on ``PublishSocket``. +/// Tests covering the custom `Codable` implementation and validating +/// initializer on ``PublishSocket``. /// -/// `containerPath` and `hostPath` were migrated from `URL` to `FilePath`. -/// Because `URL` is encoded by `JSONEncoder` as its `absoluteString` (e.g. -/// `"file:///var/run/docker.sock"`), persisted bundles and any service -/// binaries still typed against `URL` expect that exact byte format on the -/// wire. The tests below pin the encoder to that legacy shape and verify the -/// decoder accepts both the legacy file-URL form and a plain absolute path. +/// `containerPath` and `hostPath` were migrated from `URL` to `FilePath`. The +/// wire format was simultaneously changed from `URL.absoluteString` +/// (e.g. `"file:///var/run/docker.sock"`) to the plain absolute path string +/// (e.g. `"/var/run/docker.sock"`). The decoder retains compatibility with +/// the legacy file-URL form so persisted bundles from earlier releases +/// continue to load. struct PublishSocketTests { - // MARK: - Encoding + // MARK: - init validation @Test - func testEncodeProducesFileURLAbsoluteString() throws { - let socket = PublishSocket( + func testInitAcceptsAbsolutePaths() throws { + let socket = try PublishSocket( containerPath: FilePath("/var/run/docker.sock"), hostPath: FilePath("/Users/me/docker.sock") ) - let json = try JSONEncoder().encode(socket) - let decoded = try JSONSerialization.jsonObject(with: json) as? [String: Any] - let container = try #require(decoded) - #expect(container["containerPath"] as? String == "file:///var/run/docker.sock") - #expect(container["hostPath"] as? String == "file:///Users/me/docker.sock") - #expect(container["permissions"] == nil) - } - - @Test - func testEncodeIsByteCompatibleWithLegacyURLEncoding() throws { - // Pre-migration, these fields were `URL` and JSON-encoded via - // `URL.absoluteString`. Encoding must produce the exact same bytes - // for any reader still decoding them as `URL`. - let containerPath = "/var/run/docker.sock" - let hostPath = "/Users/me/docker.sock" - let legacy = LegacyPublishSocket( - containerPath: URL(fileURLWithPath: containerPath), - hostPath: URL(fileURLWithPath: hostPath) - ) - let current = PublishSocket( - containerPath: FilePath(containerPath), - hostPath: FilePath(hostPath) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + #expect(socket.permissions == nil) + } + + @Test + func testInitRejectsRelativeContainerPath() { + #expect(throws: ContainerizationError.self) { + try PublishSocket( + containerPath: FilePath("relative/path.sock"), + hostPath: FilePath("/host.sock") + ) + } + } + + @Test + func testInitRejectsRelativeHostPath() { + #expect(throws: ContainerizationError.self) { + try PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("relative/host.sock") + ) + } + } + + // MARK: - Encoding (plain absolute path) + + @Test + func testEncodeProducesPlainAbsolutePath() throws { + let socket = try PublishSocket( + containerPath: FilePath("/var/run/docker.sock"), + hostPath: FilePath("/Users/me/docker.sock") ) - let encoder = JSONEncoder() - encoder.outputFormatting = [.sortedKeys] - #expect(try encoder.encode(current) == encoder.encode(legacy)) + let json = try JSONEncoder().encode(socket) + let decoded = try #require(try JSONSerialization.jsonObject(with: json) as? [String: Any]) + #expect(decoded["containerPath"] as? String == "/var/run/docker.sock") + #expect(decoded["hostPath"] as? String == "/Users/me/docker.sock") + #expect(decoded["permissions"] == nil) } @Test - func testEncodePercentEncodesSpacesAndSpecialCharacters() throws { - let socket = PublishSocket( + func testEncodeDoesNotPercentEncode() throws { + // Plain-path encoding preserves spaces and special characters verbatim + // (no URL percent-encoding layer). + let socket = try PublishSocket( containerPath: FilePath("/tmp/a b.sock"), hostPath: FilePath("/tmp/dir with spaces/sock") ) let json = try JSONEncoder().encode(socket) let decoded = try #require(try JSONSerialization.jsonObject(with: json) as? [String: Any]) - #expect(decoded["containerPath"] as? String == "file:///tmp/a%20b.sock") - #expect(decoded["hostPath"] as? String == "file:///tmp/dir%20with%20spaces/sock") + #expect(decoded["containerPath"] as? String == "/tmp/a b.sock") + #expect(decoded["hostPath"] as? String == "/tmp/dir with spaces/sock") } - // MARK: - Decoding (canonical / legacy file-URL form) + // MARK: - Decoding (canonical plain-path form) @Test - func testDecodeFileURLForm() throws { + func testDecodePlainAbsolutePath() throws { + let json = """ + {"containerPath":"/var/run/docker.sock","hostPath":"/Users/me/docker.sock"} + """.data(using: .utf8)! + let socket = try JSONDecoder().decode(PublishSocket.self, from: json) + #expect(socket.containerPath == FilePath("/var/run/docker.sock")) + #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) + } + + // MARK: - Decoding (legacy file-URL form, compat) + + @Test + func testDecodeLegacyFileURLForm() throws { let json = """ {"containerPath":"file:///var/run/docker.sock","hostPath":"file:///Users/me/docker.sock"} """.data(using: .utf8)! @@ -91,7 +119,7 @@ struct PublishSocketTests { } @Test - func testDecodeFileURLPercentEncodingIsResolved() throws { + func testDecodeLegacyFileURLResolvesPercentEncoding() throws { // Persisted bundles created via `URL(fileURLWithPath:)` percent-encode // spaces; decoding must yield the original literal path. let json = """ @@ -104,7 +132,7 @@ struct PublishSocketTests { } @Test - func testDecodeFileURLWithLocalhostHost() throws { + func testDecodeLegacyFileURLWithLocalhostHost() throws { let json = """ {"containerPath":"file://localhost/var/run/docker.sock","hostPath":"file:///host.sock"} """.data(using: .utf8)! @@ -113,25 +141,11 @@ struct PublishSocketTests { #expect(socket.hostPath == FilePath("/host.sock")) } - // MARK: - Decoding (forward-compat plain path form) - - @Test - func testDecodePlainAbsolutePath() throws { - // Forward-compat: if some other encoder emits a clean path string, - // accept it rather than rejecting at the boundary. - let json = """ - {"containerPath":"/var/run/docker.sock","hostPath":"/Users/me/docker.sock"} - """.data(using: .utf8)! - let socket = try JSONDecoder().decode(PublishSocket.self, from: json) - #expect(socket.containerPath == FilePath("/var/run/docker.sock")) - #expect(socket.hostPath == FilePath("/Users/me/docker.sock")) - } - // MARK: - Round-trip @Test func testRoundTrip() throws { - let original = PublishSocket( + let original = try PublishSocket( containerPath: FilePath("/var/run/docker.sock"), hostPath: FilePath("/tmp/socket with spaces.sock"), permissions: FilePermissions(rawValue: 0o660) @@ -181,6 +195,8 @@ struct PublishSocketTests { @Test func testDecodeRelativePathThrows() { + // Reject non-absolute paths — `init` enforces absoluteness and the + // decoder maps the validation error to `DecodingError`. let json = """ {"containerPath":"relative/path.sock","hostPath":"/host.sock"} """.data(using: .utf8)! @@ -211,14 +227,3 @@ struct PublishSocketTests { } } } - -// MARK: - Helpers - -/// Mirrors the pre-migration `URL`-typed `PublishSocket` shape, used only to -/// confirm the new encoder produces byte-identical output for any reader -/// still decoding these fields as `URL`. -private struct LegacyPublishSocket: Encodable { - var containerPath: URL - var hostPath: URL - var permissions: FilePermissions? -} From 605408b41e553e0f1898604e8d14c665b74859c0 Mon Sep 17 00:00:00 2001 From: Chris George Date: Thu, 28 May 2026 13:00:34 -0700 Subject: [PATCH 12/15] Validate absolute paths in PublishSocket.decodePath Enforce absoluteness directly in the decode helper (in addition to the by-construction check in init) so decoded PublishSocket paths are lexically correct even when read from manually edited or corrupt persisted configs. Non-absolute decoded paths now throw DecodingError.dataCorrupted at the decode layer. Adds testDecodeRelativeHostPathThrows covering a relative hostPath on decode and clarifies the existing relative-containerPath test. Addresses PR #1594 review comments on PublishSocket.swift:90 and PublishSocketTests.swift:99. --- .../Container/PublishSocket.swift | 17 ++++++++++++++--- .../PublishSocketTests.swift | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Sources/ContainerResource/Container/PublishSocket.swift b/Sources/ContainerResource/Container/PublishSocket.swift index 4b66c9076..3d53cd475 100644 --- a/Sources/ContainerResource/Container/PublishSocket.swift +++ b/Sources/ContainerResource/Container/PublishSocket.swift @@ -108,8 +108,10 @@ public struct PublishSocket: Sendable, Codable { /// Decodes a `FilePath` accepting either the new plain-path form /// (`"/var/run/docker.sock"`) or the legacy file-URL form emitted by /// older releases (`"file:///var/run/docker.sock"`). Throws - /// `DecodingError.dataCorrupted` on a malformed file URL or empty input. - /// Absoluteness is enforced in `init(containerPath:hostPath:permissions:)`. + /// `DecodingError.dataCorrupted` on a malformed file URL, empty input, or a + /// non-absolute path — validating decoded paths here guards against + /// manually edited or corrupt persisted configs, complementing the + /// by-construction check in `init(containerPath:hostPath:permissions:)`. private static func decodePath( from container: KeyedDecodingContainer, forKey key: CodingKeys @@ -145,6 +147,15 @@ public struct PublishSocket: Sendable, Codable { ) } - return FilePath(path) + let filePath = FilePath(path) + guard filePath.isAbsolute else { + throw DecodingError.dataCorruptedError( + forKey: key, + in: container, + debugDescription: "decoded socket path must be absolute: \(raw)" + ) + } + + return filePath } } diff --git a/Tests/ContainerResourceTests/PublishSocketTests.swift b/Tests/ContainerResourceTests/PublishSocketTests.swift index 2bfb0639c..6d40d2fdb 100644 --- a/Tests/ContainerResourceTests/PublishSocketTests.swift +++ b/Tests/ContainerResourceTests/PublishSocketTests.swift @@ -195,8 +195,9 @@ struct PublishSocketTests { @Test func testDecodeRelativePathThrows() { - // Reject non-absolute paths — `init` enforces absoluteness and the - // decoder maps the validation error to `DecodingError`. + // Reject non-absolute paths. `decodePath` validates absoluteness at the + // decode layer (and `init` enforces it by construction), surfacing the + // failure as a `DecodingError`. let json = """ {"containerPath":"relative/path.sock","hostPath":"/host.sock"} """.data(using: .utf8)! @@ -205,6 +206,17 @@ struct PublishSocketTests { } } + @Test + func testDecodeRelativeHostPathThrows() { + // A relative `hostPath` is likewise rejected at the decode layer. + let json = """ + {"containerPath":"/var/run/docker.sock","hostPath":"relative/host.sock"} + """.data(using: .utf8)! + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(PublishSocket.self, from: json) + } + } + @Test func testDecodeNonLocalHostFileURLThrows() { // file URLs with a non-empty / non-localhost host are unsafe to From c5a8d7a8027598ed60f9b27ad83d41a6c5d7ca0c Mon Sep 17 00:00:00 2001 From: J Logan Date: Thu, 28 May 2026 18:26:31 -0400 Subject: [PATCH 13/15] Reorganize Swift package targets for network plugin. (#1615) - Part of #1404. - Updates containerization to 0.33.2. - Reorganizes network plugin targets into: - `ContainerNetworkClient` - network plugin client and default types - `ContainerNetworkServer` - separate protocols for `Network` which manages the underlying virtual network, `NetworkService`, which takes a network and implements the API, and an actor `NetworkHarness` that marshals between the API and the XPC protocol. The service-harness separation will help us ensure XPC protocol compatibility in both directions as we evolve the plugin APIs. - Removes `disableAllocator()` which is no longer used since #1545 switched over to using XPC connections between runtime and network plugin instances to track whether a network has attached containers. --- Package.resolved | 6 +- Package.swift | 52 ++++++----- Sources/APIServer/APIServer+Start.swift | 1 - .../NetworkVmnetHelper+Start.swift | 15 ++-- .../Server/Networks/NetworksService.swift | 6 +- .../Client/NetworkClient.swift | 13 --- .../Client/NetworkKeys.swift | 1 - .../Client/NetworkRoutes.swift | 2 - .../Server/AttachmentAllocator.swift | 5 -- .../Server/DefaultNetworkService.swift} | 78 +++++------------ .../Server/Network.swift | 0 .../Network/Server/NetworkHarness.swift | 87 +++++++++++++++++++ .../Network/Server/NetworkService.swift | 35 ++++++++ .../Server/AllocationOnlyVmnetNetwork.swift | 2 +- .../Server/ReservedVmnetNetwork.swift | 6 +- .../RuntimeLinux/Server/RuntimeService.swift | 4 +- .../AttachmentAllocatorTest.swift | 51 +---------- scripts/make-docs.sh | 5 +- 18 files changed, 194 insertions(+), 175 deletions(-) rename Sources/Services/{ContainerNetworkService => Network}/Client/NetworkClient.swift (93%) rename Sources/Services/{ContainerNetworkService => Network}/Client/NetworkKeys.swift (97%) rename Sources/Services/{ContainerNetworkService => Network}/Client/NetworkRoutes.swift (89%) rename Sources/Services/{ContainerNetworkService => Network}/Server/AttachmentAllocator.swift (92%) rename Sources/Services/{ContainerNetworkService/Server/NetworkService.swift => Network/Server/DefaultNetworkService.swift} (69%) rename Sources/Services/{ContainerNetworkService => Network}/Server/Network.swift (100%) create mode 100644 Sources/Services/Network/Server/NetworkHarness.swift create mode 100644 Sources/Services/Network/Server/NetworkService.swift rename Sources/Services/{ContainerNetworkService => NetworkVmnet}/Server/AllocationOnlyVmnetNetwork.swift (99%) rename Sources/Services/{ContainerNetworkService => NetworkVmnet}/Server/ReservedVmnetNetwork.swift (98%) rename Tests/{ContainerNetworkServiceTests => ContainerNetworkServerTests}/AttachmentAllocatorTest.swift (78%) diff --git a/Package.resolved b/Package.resolved index 6d19fb55d..f0a33d439 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "d9d032f8b1ad94cf9006bd4b441ce1dad52802b04b1d7f7b30baedd32cee0e8b", + "originHash" : "f3e6e0a1ee627e1f396b1565e72a50b179394e7011667ec4569dc53455ee06ed", "pins" : [ { "identity" : "async-http-client", @@ -15,8 +15,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/containerization.git", "state" : { - "revision" : "9205a766302a18e06a0ce43f8b7e483625e3e50f", - "version" : "0.33.1" + "revision" : "2550dd49f1890702f6fe0171212050bbce9d3825", + "version" : "0.33.2" } }, { diff --git a/Package.swift b/Package.swift index 2cc2e7092..adad7a5e7 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ import PackageDescription let releaseVersion = ProcessInfo.processInfo.environment["RELEASE_VERSION"] ?? "0.0.0" let gitCommit = ProcessInfo.processInfo.environment["GIT_COMMIT"] ?? "unspecified" let builderShimVersion = "0.12.0" -let scVersion = "0.33.1" +let scVersion = "0.33.2" let package = Package( name: "container", @@ -34,7 +34,9 @@ let package = Package( .library(name: "ContainerAPIService", targets: ["ContainerAPIService"]), .library(name: "ContainerAPIClient", targets: ["ContainerAPIClient"]), .library(name: "ContainerImagesService", targets: ["ContainerImagesService", "ContainerImagesServiceClient"]), - .library(name: "ContainerNetworkService", targets: ["ContainerNetworkService", "ContainerNetworkServiceClient"]), + .library(name: "ContainerNetworkClient", targets: ["ContainerNetworkClient"]), + .library(name: "ContainerNetworkServer", targets: ["ContainerNetworkServer"]), + .library(name: "ContainerNetworkVmnetServer", targets: ["ContainerNetworkVmnetServer"]), .library(name: "ContainerResource", targets: ["ContainerResource"]), .library(name: "ContainerLog", targets: ["ContainerLog"]), .library(name: "ContainerPersistence", targets: ["ContainerPersistence"]), @@ -106,7 +108,6 @@ let package = Package( "ContainerBuild", "ContainerAPIClient", "ContainerLog", - "ContainerNetworkService", "ContainerPersistence", "ContainerPlugin", "ContainerResource", @@ -166,7 +167,7 @@ let package = Package( "ContainerAPIService", "ContainerAPIClient", "ContainerLog", - "ContainerNetworkService", + "ContainerNetworkClient", "ContainerPersistence", "ContainerPlugin", "ContainerResource", @@ -188,7 +189,7 @@ let package = Package( .product(name: "SystemPackage", package: "swift-system"), "CVersion", "ContainerAPIClient", - "ContainerNetworkServiceClient", + "ContainerNetworkClient", "ContainerPersistence", "ContainerPlugin", "ContainerResource", @@ -291,13 +292,12 @@ let package = Package( dependencies: [ .product(name: "ArgumentParser", package: "swift-argument-parser"), .product(name: "Logging", package: "swift-log"), - .product(name: "Containerization", package: "containerization"), .product(name: "ContainerizationExtras", package: "containerization"), - .product(name: "ContainerizationIO", package: "containerization"), .product(name: "ContainerizationOS", package: "containerization"), "ContainerLog", - "ContainerNetworkService", - "ContainerNetworkServiceClient", + "ContainerNetworkClient", + "ContainerNetworkServer", + "ContainerNetworkVmnetServer", "ContainerPersistence", "ContainerPlugin", "ContainerResource", @@ -308,36 +308,42 @@ let package = Package( exclude: ["config.toml"] ), .target( - name: "ContainerNetworkService", + name: "ContainerNetworkClient", + dependencies: [ + .product(name: "ContainerizationExtras", package: "containerization"), + "ContainerResource", + "ContainerXPC", + ], + path: "Sources/Services/Network/Client" + ), + .target( + name: "ContainerNetworkServer", dependencies: [ .product(name: "Logging", package: "swift-log"), - .product(name: "Containerization", package: "containerization"), - .product(name: "ContainerizationOS", package: "containerization"), - "ContainerNetworkServiceClient", - "ContainerPersistence", + .product(name: "ContainerizationExtras", package: "containerization"), + "ContainerNetworkClient", "ContainerResource", "ContainerXPC", ], - path: "Sources/Services/ContainerNetworkService/Server" + path: "Sources/Services/Network/Server" ), .testTarget( - name: "ContainerNetworkServiceTests", + name: "ContainerNetworkServerTests", dependencies: [ - .product(name: "Containerization", package: "containerization"), .product(name: "ContainerizationExtras", package: "containerization"), - "ContainerNetworkService", + "ContainerNetworkServer", ] ), .target( - name: "ContainerNetworkServiceClient", + name: "ContainerNetworkVmnetServer", dependencies: [ .product(name: "Logging", package: "swift-log"), - .product(name: "Containerization", package: "containerization"), - "ContainerLog", + .product(name: "ContainerizationExtras", package: "containerization"), + "ContainerNetworkServer", "ContainerResource", "ContainerXPC", ], - path: "Sources/Services/ContainerNetworkService/Client" + path: "Sources/Services/NetworkVmnet/Server" ), .target( name: "ContainerRuntimeLinuxClient", @@ -371,7 +377,7 @@ let package = Package( .product(name: "ContainerizationOS", package: "containerization"), .product(name: "ArgumentParser", package: "swift-argument-parser"), "ContainerAPIClient", - "ContainerNetworkServiceClient", + "ContainerNetworkClient", "ContainerOS", "ContainerPersistence", "ContainerResource", diff --git a/Sources/APIServer/APIServer+Start.swift b/Sources/APIServer/APIServer+Start.swift index ada055aae..e6180e575 100644 --- a/Sources/APIServer/APIServer+Start.swift +++ b/Sources/APIServer/APIServer+Start.swift @@ -18,7 +18,6 @@ import ArgumentParser import ContainerAPIClient import ContainerAPIService import ContainerLog -import ContainerNetworkService import ContainerPersistence import ContainerPlugin import ContainerResource diff --git a/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift b/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift index 2cc52c555..2dadd428b 100644 --- a/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift +++ b/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift @@ -16,8 +16,9 @@ import ArgumentParser import ContainerLog -import ContainerNetworkService -import ContainerNetworkServiceClient +import ContainerNetworkClient +import ContainerNetworkServer +import ContainerNetworkVmnetServer import ContainerPlugin import ContainerResource import ContainerXPC @@ -99,14 +100,14 @@ extension NetworkVmnetHelper { log: log ) try await network.start() - let server = try await NetworkService(network: network, log: log) + let service = try await DefaultNetworkService(network: network, log: log) + let harness = NetworkHarness(service: service) let xpc = XPCServer( identifier: serviceIdentifier, routes: [ - NetworkRoutes.state.rawValue: XPCServer.route(server.state), - NetworkRoutes.allocate.rawValue: server.allocate, - NetworkRoutes.lookup.rawValue: XPCServer.route(server.lookup), - NetworkRoutes.disableAllocator.rawValue: XPCServer.route(server.disableAllocator), + NetworkRoutes.state.rawValue: XPCServer.route(harness.state), + NetworkRoutes.allocate.rawValue: harness.allocate, + NetworkRoutes.lookup.rawValue: XPCServer.route(harness.lookup), ], log: log ) diff --git a/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift b/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift index 271f07194..512194f2d 100644 --- a/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift +++ b/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift @@ -15,7 +15,7 @@ //===----------------------------------------------------------------------===// import ContainerAPIClient -import ContainerNetworkServiceClient +import ContainerNetworkClient import ContainerPersistence import ContainerPlugin import ContainerResource @@ -30,7 +30,7 @@ import SystemPackage public actor NetworksService { struct NetworkServiceState { var networkState: NetworkState - var client: ContainerNetworkServiceClient.NetworkClient + var client: ContainerNetworkClient.NetworkClient } private let pluginLoader: PluginLoader @@ -389,7 +389,7 @@ public actor NetworksService { return pluginInfo } - private static func getClient(configuration: NetworkConfiguration) throws -> ContainerNetworkServiceClient.NetworkClient { + private static func getClient(configuration: NetworkConfiguration) throws -> ContainerNetworkClient.NetworkClient { guard let pluginInfo = configuration.pluginInfo else { throw ContainerizationError(.internalError, message: "network \(configuration.id) missing plugin information") } diff --git a/Sources/Services/ContainerNetworkService/Client/NetworkClient.swift b/Sources/Services/Network/Client/NetworkClient.swift similarity index 93% rename from Sources/Services/ContainerNetworkService/Client/NetworkClient.swift rename to Sources/Services/Network/Client/NetworkClient.swift index b7a0e8bb1..18b6591ca 100644 --- a/Sources/Services/ContainerNetworkService/Client/NetworkClient.swift +++ b/Sources/Services/Network/Client/NetworkClient.swift @@ -113,15 +113,6 @@ extension NetworkClient { } } - public func disableAllocator() async throws -> Bool { - let request = XPCMessage(route: NetworkRoutes.disableAllocator.rawValue) - - let client = createClient() - - let response = try await client.send(request) - return try response.allocatorDisabled() - } - private func createClient() -> XPCClient { XPCClient(service: machServiceLabel) } @@ -135,10 +126,6 @@ extension XPCMessage { return XPCMessage(object: additionalData) } - public func allocatorDisabled() throws -> Bool { - self.bool(key: NetworkKeys.allocatorDisabled.rawValue) - } - public func attachment() throws -> Attachment { let data = self.dataNoCopy(key: NetworkKeys.attachment.rawValue) guard let data else { diff --git a/Sources/Services/ContainerNetworkService/Client/NetworkKeys.swift b/Sources/Services/Network/Client/NetworkKeys.swift similarity index 97% rename from Sources/Services/ContainerNetworkService/Client/NetworkKeys.swift rename to Sources/Services/Network/Client/NetworkKeys.swift index cc7715a30..af10bdae7 100644 --- a/Sources/Services/ContainerNetworkService/Client/NetworkKeys.swift +++ b/Sources/Services/Network/Client/NetworkKeys.swift @@ -16,7 +16,6 @@ public enum NetworkKeys: String { case additionalData - case allocatorDisabled case attachment case hostname case macAddress diff --git a/Sources/Services/ContainerNetworkService/Client/NetworkRoutes.swift b/Sources/Services/Network/Client/NetworkRoutes.swift similarity index 89% rename from Sources/Services/ContainerNetworkService/Client/NetworkRoutes.swift rename to Sources/Services/Network/Client/NetworkRoutes.swift index d3f005498..6e3b91fe4 100644 --- a/Sources/Services/ContainerNetworkService/Client/NetworkRoutes.swift +++ b/Sources/Services/Network/Client/NetworkRoutes.swift @@ -19,8 +19,6 @@ public enum NetworkRoutes: String { case state = "com.apple.container.network/state" /// Allocates parameters for attaching a sandbox to the network. case allocate = "com.apple.container.network/allocate" - /// Disables the allocator if no sandboxes are attached. - case disableAllocator = "com.apple.container.network/disableAllocator" /// Retrieves the allocation for a hostname. case lookup = "com.apple.container.network/lookup" } diff --git a/Sources/Services/ContainerNetworkService/Server/AttachmentAllocator.swift b/Sources/Services/Network/Server/AttachmentAllocator.swift similarity index 92% rename from Sources/Services/ContainerNetworkService/Server/AttachmentAllocator.swift rename to Sources/Services/Network/Server/AttachmentAllocator.swift index fb1f537c3..b7d3aeebb 100644 --- a/Sources/Services/ContainerNetworkService/Server/AttachmentAllocator.swift +++ b/Sources/Services/Network/Server/AttachmentAllocator.swift @@ -52,11 +52,6 @@ actor AttachmentAllocator { return index } - /// If no addresses are allocated, prevent future allocations and return true. - func disableAllocator() async -> Bool { - allocator.disableAllocator() - } - /// Retrieve the allocator index for a hostname. func lookup(hostname: String) async throws -> UInt32? { hostnames[hostname] diff --git a/Sources/Services/ContainerNetworkService/Server/NetworkService.swift b/Sources/Services/Network/Server/DefaultNetworkService.swift similarity index 69% rename from Sources/Services/ContainerNetworkService/Server/NetworkService.swift rename to Sources/Services/Network/Server/DefaultNetworkService.swift index 8d12ae76c..43990a662 100644 --- a/Sources/Services/ContainerNetworkService/Server/NetworkService.swift +++ b/Sources/Services/Network/Server/DefaultNetworkService.swift @@ -14,15 +14,13 @@ // limitations under the License. //===----------------------------------------------------------------------===// -import ContainerNetworkServiceClient import ContainerResource import ContainerXPC import ContainerizationError import ContainerizationExtras -import Foundation import Logging -public actor NetworkService: Sendable { +public actor DefaultNetworkService: NetworkService { private let network: any Network private let log: Logger private var allocator: AttachmentAllocator @@ -50,15 +48,16 @@ public actor NetworkService: Sendable { } @Sendable - public func state(_ message: XPCMessage) async throws -> XPCMessage { - let reply = message.reply() - let state = await network.state - try reply.setState(state) - return reply + public func state() async throws -> NetworkState { + await network.state } @Sendable - public func allocate(_ message: XPCMessage, _ session: XPCServerSession) async throws -> XPCMessage { + public func allocate( + hostname: String, + macAddress: MACAddress?, + session: XPCServerSession + ) async throws -> (attachment: Attachment, additionalData: XPCMessage?) { log.debug("enter", metadata: ["func": "\(#function)"]) defer { log.debug("exit", metadata: ["func": "\(#function)"]) } @@ -67,11 +66,7 @@ public actor NetworkService: Sendable { throw ContainerizationError(.invalidState, message: "invalid network state - network \(state.id) must be running") } - let hostname = try message.hostname() - let macAddress = - try message.string(key: NetworkKeys.macAddress.rawValue) - .map { try MACAddress($0) } - ?? MACAddress((UInt64.random(in: 0...UInt64.max) & 0x0cff_ffff_ffff) | 0xf200_0000_0000) + let macAddress = macAddress ?? MACAddress((UInt64.random(in: 0...UInt64.max) & 0x0cff_ffff_ffff) | 0xf200_0000_0000) let index = try await allocator.allocate(hostname: hostname) let ipv6Address = try status.ipv6Subnet .map { try CIDRv6(macAddress.ipv6Address(network: $0.lower), prefix: $0.prefix) } @@ -93,12 +88,10 @@ public actor NetworkService: Sendable { "ipv6Address": "\(attachment.ipv6Address?.description ?? "unavailable")", "macAddress": "\(attachment.macAddress?.description ?? "unspecified")", ]) - let reply = message.reply() - try reply.setAttachment(attachment) + + var additionalData: XPCMessage? try network.withAdditionalData { - if let additionalData = $0 { - try reply.setAdditionalData(additionalData.underlying) - } + additionalData = $0 } macAddresses[index] = macAddress @@ -110,7 +103,7 @@ public actor NetworkService: Sendable { } allocationsBySession[session]!.append((hostname: hostname, index: index)) - return reply + return (attachment: attachment, additionalData: additionalData) } private func releaseSession(_ session: XPCServerSession) async { @@ -125,7 +118,7 @@ public actor NetworkService: Sendable { } @Sendable - public func lookup(_ message: XPCMessage) async throws -> XPCMessage { + public func lookup(hostname: String) async throws -> Attachment? { log.debug("enter", metadata: ["func": "\(#function)"]) defer { log.debug("exit", metadata: ["func": "\(#function)"]) } @@ -134,15 +127,16 @@ public actor NetworkService: Sendable { throw ContainerizationError(.invalidState, message: "invalid network state - network \(state.id) must be running") } - let hostname = try message.hostname() + // Invariant: hostname -> index if and only if index -> MAC address let index = try await allocator.lookup(hostname: hostname) - let reply = message.reply() guard let index else { - return reply + return nil } guard let macAddress = macAddresses[index] else { - return reply + return nil } + + // populate attachment let address = IPv4Address(index) let subnet = status.ipv4Subnet let ipv4Address = try CIDRv4(address, prefix: subnet.prefix) @@ -162,39 +156,7 @@ public actor NetworkService: Sendable { "hostname": "\(hostname)", "address": "\(address)", ]) - try reply.setAttachment(attachment) - return reply - } - - @Sendable - public func disableAllocator(_ message: XPCMessage) async throws -> XPCMessage { - log.debug("enter", metadata: ["func": "\(#function)"]) - defer { log.debug("exit", metadata: ["func": "\(#function)"]) } - - let success = await allocator.disableAllocator() - log.info("attempted allocator disable", metadata: ["success": "\(success)"]) - let reply = message.reply() - reply.setAllocatorDisabled(success) - return reply - } -} - -extension XPCMessage { - fileprivate func setAdditionalData(_ additionalData: xpc_object_t) throws { - xpc_dictionary_set_value(self.underlying, NetworkKeys.additionalData.rawValue, additionalData) - } - - fileprivate func setAllocatorDisabled(_ allocatorDisabled: Bool) { - self.set(key: NetworkKeys.allocatorDisabled.rawValue, value: allocatorDisabled) - } - - fileprivate func setAttachment(_ attachment: Attachment) throws { - let data = try JSONEncoder().encode(attachment) - self.set(key: NetworkKeys.attachment.rawValue, value: data) - } - fileprivate func setState(_ state: NetworkState) throws { - let data = try JSONEncoder().encode(state) - self.set(key: NetworkKeys.state.rawValue, value: data) + return attachment } } diff --git a/Sources/Services/ContainerNetworkService/Server/Network.swift b/Sources/Services/Network/Server/Network.swift similarity index 100% rename from Sources/Services/ContainerNetworkService/Server/Network.swift rename to Sources/Services/Network/Server/Network.swift diff --git a/Sources/Services/Network/Server/NetworkHarness.swift b/Sources/Services/Network/Server/NetworkHarness.swift new file mode 100644 index 000000000..28023de6d --- /dev/null +++ b/Sources/Services/Network/Server/NetworkHarness.swift @@ -0,0 +1,87 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ContainerNetworkClient +import ContainerResource +import ContainerXPC +import ContainerizationExtras +import Foundation + +public actor NetworkHarness: Sendable { + private let service: NetworkService + + public init(service: NetworkService) { + self.service = service + } + + @Sendable + public func state(_ message: XPCMessage) async throws -> XPCMessage { + let reply = message.reply() + let state = try await service.state() + try reply.setState(state) + return reply + } + + @Sendable + public func allocate(_ message: XPCMessage, _ session: XPCServerSession) async throws -> XPCMessage { + let hostname = try message.hostname() + let macAddress = + try message.string(key: NetworkKeys.macAddress.rawValue) + .map { try MACAddress($0) } + + let (attachment:attachment, additionalData:additionalData) = try await service.allocate( + hostname: hostname, + macAddress: macAddress, + session: session + ) + + let reply = message.reply() + try reply.setAttachment(attachment) + if let additionalData { + try reply.setAdditionalData(additionalData.underlying) + } + + return reply + } + + @Sendable + public func lookup(_ message: XPCMessage) async throws -> XPCMessage { + let hostname = try message.hostname() + let reply = message.reply() + guard let attachment = try await service.lookup(hostname: hostname) else { + return reply + } + + try reply.setAttachment(attachment) + return reply + } +} + +extension XPCMessage { + fileprivate func setAdditionalData(_ additionalData: xpc_object_t) throws { + xpc_dictionary_set_value(self.underlying, NetworkKeys.additionalData.rawValue, additionalData) + } + + fileprivate func setAttachment(_ attachment: Attachment) throws { + let data = try JSONEncoder().encode(attachment) + self.set(key: NetworkKeys.attachment.rawValue, value: data) + } + + fileprivate func setState(_ state: NetworkState) throws { + let data = try JSONEncoder().encode(state) + self.set(key: NetworkKeys.state.rawValue, value: data) + } +} diff --git a/Sources/Services/Network/Server/NetworkService.swift b/Sources/Services/Network/Server/NetworkService.swift new file mode 100644 index 000000000..3644fc838 --- /dev/null +++ b/Sources/Services/Network/Server/NetworkService.swift @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import ContainerResource +import ContainerXPC +import ContainerizationExtras + +/// A network service +public protocol NetworkService: Sendable { + /// Gets the properties of the realized network. + func state() async throws -> NetworkState + + /// Register a hostname and allocate associated addresses. + func allocate( + hostname: String, + macAddress: MACAddress?, + session: XPCServerSession + ) async throws -> (attachment: Attachment, additionalData: XPCMessage?) + + /// Return the attachment for a hostname if it is registered with the network. + func lookup(hostname: String) async throws -> Attachment? +} diff --git a/Sources/Services/ContainerNetworkService/Server/AllocationOnlyVmnetNetwork.swift b/Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift similarity index 99% rename from Sources/Services/ContainerNetworkService/Server/AllocationOnlyVmnetNetwork.swift rename to Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift index d63e48511..c17e60242 100644 --- a/Sources/Services/ContainerNetworkService/Server/AllocationOnlyVmnetNetwork.swift +++ b/Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift @@ -14,11 +14,11 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerNetworkServer import ContainerResource import ContainerXPC import ContainerizationError import ContainerizationExtras -import Foundation import Logging public actor AllocationOnlyVmnetNetwork: Network { diff --git a/Sources/Services/ContainerNetworkService/Server/ReservedVmnetNetwork.swift b/Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift similarity index 98% rename from Sources/Services/ContainerNetworkService/Server/ReservedVmnetNetwork.swift rename to Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift index 6b7977cc5..43a325823 100644 --- a/Sources/Services/ContainerNetworkService/Server/ReservedVmnetNetwork.swift +++ b/Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift @@ -14,22 +14,20 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerNetworkServer import ContainerResource import ContainerXPC -import Containerization import ContainerizationError import ContainerizationExtras -import Dispatch import Foundation import Logging import Synchronization -import SystemConfiguration import XPC import vmnet /// Creates a vmnet network with reservation APIs. @available(macOS 26, *) -public final class ReservedVmnetNetwork: Network { +public final class ReservedVmnetNetwork: ContainerNetworkServer.Network { private struct State { var networkState: NetworkState var network: vmnet_network_ref? diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index 187de7af6..27ae89cb4 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -14,7 +14,7 @@ // limitations under the License. //===----------------------------------------------------------------------===// -import ContainerNetworkServiceClient +import ContainerNetworkClient import ContainerOS import ContainerPersistence import ContainerResource @@ -177,7 +177,7 @@ public actor RuntimeService { do { for (index, info) in networkBootstrapInfos.enumerated() { let attachmentConfig = config.networks[index] - let client = ContainerNetworkServiceClient.NetworkClient(id: attachmentConfig.network, plugin: info.pluginInfo.plugin) + let client = ContainerNetworkClient.NetworkClient(id: attachmentConfig.network, plugin: info.pluginInfo.plugin) let session = client.connect() sessions.append(session) var (attachment, additionalData) = try await client.allocate( diff --git a/Tests/ContainerNetworkServiceTests/AttachmentAllocatorTest.swift b/Tests/ContainerNetworkServerTests/AttachmentAllocatorTest.swift similarity index 78% rename from Tests/ContainerNetworkServiceTests/AttachmentAllocatorTest.swift rename to Tests/ContainerNetworkServerTests/AttachmentAllocatorTest.swift index 9db889763..86ea3eff0 100644 --- a/Tests/ContainerNetworkServiceTests/AttachmentAllocatorTest.swift +++ b/Tests/ContainerNetworkServerTests/AttachmentAllocatorTest.swift @@ -14,11 +14,9 @@ // limitations under the License. //===----------------------------------------------------------------------===// -import ContainerizationError -import ContainerizationExtras import Testing -@testable import ContainerNetworkService +@testable import ContainerNetworkServer struct AttachmentAllocatorTest { @Test func testAllocateSingleHostname() async throws { @@ -145,53 +143,6 @@ struct AttachmentAllocatorTest { #expect(finalAddress4 == newAddress) } - @Test func testDisableAllocatorWhenEmpty() async throws { - let allocator = try AttachmentAllocator(lower: 100, size: 10) - - let disabled = await allocator.disableAllocator() - - #expect(disabled == true) - - // After disabling, allocation should fail - await #expect(throws: Error.self) { - try await allocator.allocate(hostname: "test-host") - } - } - - @Test func testDisableAllocatorWhenNotEmpty() async throws { - let allocator = try AttachmentAllocator(lower: 100, size: 10) - - _ = try await allocator.allocate(hostname: "test-host") - - let disabled = await allocator.disableAllocator() - - #expect(disabled == false) - - // Since disable failed, should still be able to allocate - let address = try await allocator.allocate(hostname: "another-host") - #expect(address >= 100) - #expect(address < 110) - } - - @Test func testDisableAfterDeallocatingAll() async throws { - let allocator = try AttachmentAllocator(lower: 100, size: 10) - - _ = try await allocator.allocate(hostname: "host1") - _ = try await allocator.allocate(hostname: "host2") - - try await allocator.deallocate(hostname: "host1") - try await allocator.deallocate(hostname: "host2") - - let disabled = await allocator.disableAllocator() - - #expect(disabled == true) - - // After disabling, allocation should fail - await #expect(throws: Error.self) { - try await allocator.allocate(hostname: "test-host") - } - } - @Test func testMultipleDeallocationsOfSameHostname() async throws { let allocator = try AttachmentAllocator(lower: 100, size: 10) diff --git a/scripts/make-docs.sh b/scripts/make-docs.sh index d904eb0d4..9f1e594c4 100755 --- a/scripts/make-docs.sh +++ b/scripts/make-docs.sh @@ -21,8 +21,9 @@ opts+=("--target" "ContainerAPIClient") opts+=("--target" "ContainerRuntimeClient") opts+=("--target" "ContainerRuntimeLinuxClient") opts+=("--target" "ContainerRuntimeLinuxServer") -opts+=("--target" "ContainerNetworkService") -opts+=("--target" "ContainerNetworkServiceClient") +opts+=("--target" "ContainerNetworkClient") +opts+=("--target" "ContainerNetworkServer") +opts+=("--target" "ContainerNetworkVmnetServer") opts+=("--target" "ContainerImagesService") opts+=("--target" "ContainerImagesServiceClient") opts+=("--target" "ContainerResource") From f4f5925c08568b4d0d30b4f5ccd0d676b43ea4f9 Mon Sep 17 00:00:00 2001 From: Saehej Kang <20051028+saehejkang@users.noreply.github.com> Date: Thu, 28 May 2026 15:45:41 -0700 Subject: [PATCH 14/15] [how-to]: fix documentation example (#1596) - Related to #1534 --- docs/how-to.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/docs/how-to.md b/docs/how-to.md index 1b3c3d62f..b4ca8b8df 100644 --- a/docs/how-to.md +++ b/docs/how-to.md @@ -244,14 +244,11 @@ The MAC address must be in the format `XX:XX:XX:XX:XX:XX` (with colons or hyphen container run --network default,mac=02:42:ac:11:00:02 ubuntu:latest ``` -To verify the MAC address is set correctly, run `ip addr show` inside the container: +To verify the MAC address is set correctly, read the interface MAC directly from sysfs inside the container: ```console -% container run --rm --network default,mac=02:42:ac:11:00:02 ubuntu:latest ip addr show eth0 -2: eth0: mtu 1500 qdisc fq_codel state UP group default qlen 1000 - link/ether 02:42:ac:11:00:02 brd ff:ff:ff:ff:ff:ff - inet 192.168.64.2/24 brd 192.168.64.255 scope global eth0 - valid_lft forever preferred_lft forever +% container run --rm --network default,mac=02:42:ac:11:00:02 ubuntu:latest cat /sys/class/net/eth0/address +02:42:ac:11:00:02 ``` If you don't specify a MAC address, `container` will generate one for you. The generated address has a first nibble set to hexadecimal `f` (`fX:XX:XX:XX:XX:XX`) in case you want to minimize the very small chance of conflict between your MAC address and generated addresses. From 37595a734c4ec109bdfa072469c2a725b8de4f59 Mon Sep 17 00:00:00 2001 From: J Logan Date: Fri, 29 May 2026 12:33:45 -0700 Subject: [PATCH 15/15] Remove XPC compatibility code, simplify network model. (#1616) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refactor network model types: replace `NetworkState` enum and phase-based NetworkStatus with a flat `NetworkStatus` struct. - Simplify API server ↔ plugin protocol: plugin `status()` returns runtime status only, API server owns configuration. - `NetworksService` `list()`/`create()` now return `NetworkResource` directly. - Remove lifecycle phase checks and state machine guards throughout CLI and API server. - `variant` is plugin-specific, it's not a required property. This PR replaces `NetworkPluginInfo` with a `plugin` name property on `NetworkConfiguration` and an `options` list similar to that for volumes. - Moved `variant` to the option list. --- Sources/APIServer/APIServer+Start.swift | 2 +- .../Builder/BuilderStart.swift | 3 - .../Network/NetworkCreate.swift | 20 +- .../NetworkResource+ListDisplayable.swift | 4 +- .../Network/NetworkConfiguration.swift | 55 ++++-- .../Network/NetworkResource.swift | 90 +++------ .../Network/NetworkState.swift | 118 ------------ .../Network/NetworkStatus.swift | 49 +---- .../NetworkVmnetHelper+Start.swift | 9 +- .../RuntimeLinuxHelper+Start.swift | 6 +- .../Client/NetworkClient.swift | 27 +-- .../ContainerAPIService/Client/Utility.swift | 5 +- .../ContainerAPIService/Client/XPC+.swift | 5 - .../Server/Containers/ContainersService.swift | 6 +- .../Server/Networks/NetworksHarness.swift | 19 +- .../Server/Networks/NetworksService.swift | 173 +++++++----------- .../Network/Client/NetworkClient.swift | 42 ++--- .../Services/Network/Client/NetworkKeys.swift | 2 +- .../Network/Client/NetworkRoutes.swift | 4 +- .../Server/DefaultNetworkService.swift | 28 ++- Sources/Services/Network/Server/Network.swift | 11 +- .../Network/Server/NetworkHarness.swift | 13 +- .../Network/Server/NetworkService.swift | 2 +- .../Server/AllocationOnlyVmnetNetwork.swift | 22 +-- .../Server/ReservedVmnetNetwork.swift | 22 ++- .../RuntimeClient/InterfaceStrategy.swift | 11 ++ .../RuntimeClient/NetworkBootstrapInfo.swift | 13 +- .../RuntimeLinux/Server/RuntimeService.swift | 11 +- .../Subcommands/Networks/TestCLINetwork.swift | 2 +- Tests/CLITests/Utilities/CLITest.swift | 3 +- .../ListFormattingTests.swift | 6 +- .../NetworkConfigurationTest.swift | 8 +- docs/command-reference.md | 4 +- 33 files changed, 265 insertions(+), 530 deletions(-) delete mode 100644 Sources/ContainerResource/Network/NetworkState.swift diff --git a/Sources/APIServer/APIServer+Start.swift b/Sources/APIServer/APIServer+Start.swift index e6180e575..510edbad9 100644 --- a/Sources/APIServer/APIServer+Start.swift +++ b/Sources/APIServer/APIServer+Start.swift @@ -339,7 +339,7 @@ extension APIServer { ipv4Subnet: containerSystemConfig.network.subnet, ipv6Subnet: containerSystemConfig.network.subnetv6, labels: try .init([ResourceLabelKeys.role: ResourceRoleValues.builtin]), - pluginInfo: NetworkPluginInfo(plugin: "container-network-vmnet") + plugin: "container-network-vmnet" ) _ = try await service.create(configuration: config) } diff --git a/Sources/ContainerCommands/Builder/BuilderStart.swift b/Sources/ContainerCommands/Builder/BuilderStart.swift index a467ea123..cf1d4e1c0 100644 --- a/Sources/ContainerCommands/Builder/BuilderStart.swift +++ b/Sources/ContainerCommands/Builder/BuilderStart.swift @@ -266,9 +266,6 @@ extension Application { guard let defaultNetwork = try await networkClient.builtin else { throw ContainerizationError(.invalidState, message: "default network is not present") } - guard defaultNetwork.status.phase == "running" else { - throw ContainerizationError(.invalidState, message: "default network is not running") - } config.networks = [ AttachmentConfiguration(network: defaultNetwork.id, options: AttachmentOptions(hostname: Builder.builderContainerId)) ] diff --git a/Sources/ContainerCommands/Network/NetworkCreate.swift b/Sources/ContainerCommands/Network/NetworkCreate.swift index 7b770240c..170cbaf35 100644 --- a/Sources/ContainerCommands/Network/NetworkCreate.swift +++ b/Sources/ContainerCommands/Network/NetworkCreate.swift @@ -28,11 +28,17 @@ extension Application { commandName: "create", abstract: "Create a new network") + @Flag(name: .customLong("internal"), help: "Restrict to host-only network") + var hostOnly: Bool = false + @Option(name: .customLong("label"), help: "Set metadata for a network") var labels: [String] = [] - @Flag(name: .customLong("internal"), help: "Restrict to host-only network") - var hostOnly: Bool = false + @Option(name: .customLong("option"), help: "Set a plugin-specific option (key=value)") + var options: [String] = [] + + @Option(name: .long, help: "Set the plugin to use to create this network.") + var plugin: String = "container-network-vmnet" @Option( name: .customLong("subnet"), help: "Set subnet for a network", @@ -48,12 +54,6 @@ extension Application { }) var ipv6Subnet: CIDRv6? = nil - @Option(name: .long, help: "Set the plugin to use to create this network.") - var plugin: String = "container-network-vmnet" - - @Option(name: .long, help: "Set the variant of the network plugin to use.") - var pluginVariant: String? - @OptionGroup public var logOptions: Flags.Logging @@ -64,6 +64,7 @@ extension Application { public func run() async throws { let parsedLabels = try ResourceLabels(Utility.parseKeyValuePairs(labels)) + let parsedOptions = Utility.parseKeyValuePairs(options) let mode: NetworkMode = hostOnly ? .hostOnly : .nat let config = try NetworkConfiguration( id: self.name, @@ -71,7 +72,8 @@ extension Application { ipv4Subnet: ipv4Subnet, ipv6Subnet: ipv6Subnet, labels: parsedLabels, - pluginInfo: NetworkPluginInfo(plugin: self.plugin, variant: self.pluginVariant) + plugin: self.plugin, + options: parsedOptions ) let networkClient = NetworkClient() let network = try await networkClient.create(configuration: config) diff --git a/Sources/ContainerCommands/Network/NetworkResource+ListDisplayable.swift b/Sources/ContainerCommands/Network/NetworkResource+ListDisplayable.swift index 3f3395d8e..26be85ca9 100644 --- a/Sources/ContainerCommands/Network/NetworkResource+ListDisplayable.swift +++ b/Sources/ContainerCommands/Network/NetworkResource+ListDisplayable.swift @@ -18,11 +18,11 @@ import ContainerResource extension NetworkResource: ListDisplayable { public static var tableHeader: [String] { - ["NETWORK", "STATE", "SUBNET"] + ["NETWORK", "SUBNET"] } public var tableRow: [String] { - [id, status.phase, status.ipv4Subnet?.description ?? "none"] + [id, status.ipv4Subnet.description] } public var quietValue: String { diff --git a/Sources/ContainerResource/Network/NetworkConfiguration.swift b/Sources/ContainerResource/Network/NetworkConfiguration.swift index fbd0c8890..c9da14ddd 100644 --- a/Sources/ContainerResource/Network/NetworkConfiguration.swift +++ b/Sources/ContainerResource/Network/NetworkConfiguration.swift @@ -18,16 +18,6 @@ import ContainerizationError import ContainerizationExtras import Foundation -public struct NetworkPluginInfo: Codable, Sendable, Hashable { - public let plugin: String - public let variant: String? - - public init(plugin: String, variant: String? = nil) { - self.plugin = plugin - self.variant = variant - } -} - /// Configuration parameters for network creation. public struct NetworkConfiguration: Codable, Sendable, Identifiable { /// A unique identifier for the network @@ -49,10 +39,11 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { /// Resource labels should not be mutated, except while building a network configurations. public let labels: ResourceLabels - /// Details about the network plugin that manages this network. - /// FIXME: This field only needs to be optional while we wait for the field - /// to be proliferated to most users when they update container. - public let pluginInfo: NetworkPluginInfo? + /// The network plugin that manages this network. + public let plugin: String + + /// Plugin-specific options for this network. + public let options: [String: String] /// Creates a network configuration public init( @@ -61,7 +52,8 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { ipv4Subnet: CIDRv4? = nil, ipv6Subnet: CIDRv6? = nil, labels: ResourceLabels = .init(), - pluginInfo: NetworkPluginInfo? + plugin: String, + options: [String: String] = [:] ) throws { self.id = id self.creationDate = Date() @@ -69,7 +61,8 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { self.ipv4Subnet = ipv4Subnet self.ipv6Subnet = ipv6Subnet self.labels = labels - self.pluginInfo = pluginInfo + self.plugin = plugin + self.options = options try validate() } @@ -80,8 +73,10 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { case ipv4Subnet case ipv6Subnet case labels + case plugin + case options + // TODO: retain for deserialization compatibility, remove in next major version case pluginInfo - // TODO: retain for deserialization compatibility for now, remove later case subnet } @@ -101,7 +96,22 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { .map { try CIDRv6($0) } let decodedLabels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:] labels = try .init(decodedLabels) - pluginInfo = try container.decodeIfPresent(NetworkPluginInfo.self, forKey: .pluginInfo) + + if let plugin = try container.decodeIfPresent(String.self, forKey: .plugin) { + self.plugin = plugin + self.options = try container.decodeIfPresent([String: String].self, forKey: .options) ?? [:] + } else if let legacy = try container.decodeIfPresent(_LegacyPluginInfo.self, forKey: .pluginInfo) { + // - Deprecated: As of 1.0.0. Use ``plugin`` and ``options`` instead. + // - Note: Will be removed in a later release. + self.plugin = legacy.plugin + var opts: [String: String] = [:] + if let variant = legacy.variant { opts["variant"] = variant } + self.options = opts + } else { + self.plugin = "container-network-vmnet" + self.options = [:] + } + try validate() } @@ -115,7 +125,8 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { try container.encodeIfPresent(ipv4Subnet, forKey: .ipv4Subnet) try container.encodeIfPresent(ipv6Subnet, forKey: .ipv6Subnet) try container.encode(labels, forKey: .labels) - try container.encodeIfPresent(pluginInfo, forKey: .pluginInfo) + try container.encode(plugin, forKey: .plugin) + try container.encode(options, forKey: .options) } private func validate() throws { @@ -124,3 +135,9 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { } } } + +/// Decode helper for stored configurations that used the old `pluginInfo` key. +private struct _LegacyPluginInfo: Codable { + let plugin: String + let variant: String? +} diff --git a/Sources/ContainerResource/Network/NetworkResource.swift b/Sources/ContainerResource/Network/NetworkResource.swift index 5d400016f..9699a3901 100644 --- a/Sources/ContainerResource/Network/NetworkResource.swift +++ b/Sources/ContainerResource/Network/NetworkResource.swift @@ -20,34 +20,32 @@ import Foundation /// A network resource, representing a configured virtual network and its runtime status. /// /// `NetworkResource` conforms to `ManagedResource` and separates the network's -/// intrinsic configuration from its ephemeral runtime status — following the same -/// config/status split used by Kubernetes and Docker. `config` is persisted; -/// `status` reflects what the network plugin reports at runtime. +/// intrinsic configuration from its runtime status — following the same config/status +/// split used by Kubernetes and Docker. `configuration` is persisted; `status` reflects +/// what the network plugin reports at runtime. /// -/// JSON encoding produces four top-level keys: `id`, `state` (the lifecycle label: -/// `"created"` or `"running"`), `configuration` (the persistent config), and `status` -/// (runtime address properties, `null` when `state` is `"created"`). +/// JSON encoding produces three top-level keys: `id`, `configuration` (the persistent +/// config), and `status` (runtime address properties assigned by the network plugin). public struct NetworkResource: ManagedResource { /// The network's configuration — its persistent, intrinsic properties. - public let config: NetworkConfiguration + public let configuration: NetworkConfiguration - /// The network's current status, including lifecycle phase and any - /// runtime-allocated address properties. + /// The network's runtime status — the addresses assigned by the network plugin. public let status: NetworkStatus // MARK: ManagedResource - /// The unique identifier for this network. Identical to ``config/id``. - public var id: String { config.id } + /// The unique identifier for this network. Identical to ``configuration/id``. + public var id: String { configuration.id } /// The user-assigned name for this network. For networks, name and ID are the same. - public var name: String { config.id } + public var name: String { configuration.id } /// The time at which this network was created. - public var creationDate: Date { config.creationDate } + public var creationDate: Date { configuration.creationDate } /// Key-value labels for this network. - public var labels: ResourceLabels { config.labels } + public var labels: ResourceLabels { configuration.labels } /// Returns `true` for a system-managed network that cannot be deleted by the user. public var isBuiltin: Bool { labels.isBuiltin } @@ -66,29 +64,11 @@ public struct NetworkResource: ManagedResource { /// Creates a network resource. /// /// - Parameters: - /// - config: The network's intrinsic configuration. - /// - networkStatus: The plugin-reported runtime status, or `nil` if the - /// network is not yet running. - public init(config: NetworkConfiguration, networkStatus: NetworkPluginStatus? = nil) { - self.config = config - self.status = networkStatus.map { NetworkStatus(running: $0) } ?? .created - } -} - -// MARK: - Conversion from NetworkState - -extension NetworkResource { - /// Creates a network resource from a ``NetworkState``. - /// - /// Used when translating from the internal plugin-protocol type to the - /// public API surface type. - public init(_ networkState: NetworkState) { - switch networkState { - case .created(let config): - self.init(config: config) - case .running(let config, let status): - self.init(config: config, networkStatus: status) - } + /// - configuration: The network's intrinsic configuration. + /// - status: The runtime status reported by the network plugin. + public init(configuration: NetworkConfiguration, status: NetworkStatus) { + self.configuration = configuration + self.status = status } } @@ -97,48 +77,20 @@ extension NetworkResource { extension NetworkResource { enum CodingKeys: String, CodingKey { case id - case state case configuration case status } - private enum StatusCodingKeys: String, CodingKey { - case ipv4Subnet - case ipv4Gateway - case ipv6Subnet - } - public func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(id, forKey: .id) - try container.encode(status.phase, forKey: .state) - try container.encode(config, forKey: .configuration) - if status.phase == "running" { - var statusContainer = container.nestedContainer(keyedBy: StatusCodingKeys.self, forKey: .status) - try statusContainer.encodeIfPresent(status.ipv4Subnet, forKey: .ipv4Subnet) - try statusContainer.encodeIfPresent(status.ipv4Gateway, forKey: .ipv4Gateway) - try statusContainer.encodeIfPresent(status.ipv6Subnet, forKey: .ipv6Subnet) - } else { - try container.encodeNil(forKey: .status) - } + try container.encode(configuration, forKey: .configuration) + try container.encode(status, forKey: .status) } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) - let state = try container.decode(String.self, forKey: .state) - let config = try container.decode(NetworkConfiguration.self, forKey: .configuration) - if try container.decodeNil(forKey: .status) { - self.config = config - self.status = NetworkStatus(phase: state) - } else { - let statusContainer = try container.nestedContainer(keyedBy: StatusCodingKeys.self, forKey: .status) - self.config = config - self.status = NetworkStatus( - phase: state, - ipv4Subnet: try statusContainer.decodeIfPresent(CIDRv4.self, forKey: .ipv4Subnet), - ipv4Gateway: try statusContainer.decodeIfPresent(IPv4Address.self, forKey: .ipv4Gateway), - ipv6Subnet: try statusContainer.decodeIfPresent(CIDRv6.self, forKey: .ipv6Subnet) - ) - } + configuration = try container.decode(NetworkConfiguration.self, forKey: .configuration) + status = try container.decode(NetworkStatus.self, forKey: .status) } } diff --git a/Sources/ContainerResource/Network/NetworkState.swift b/Sources/ContainerResource/Network/NetworkState.swift deleted file mode 100644 index 6965ea7bc..000000000 --- a/Sources/ContainerResource/Network/NetworkState.swift +++ /dev/null @@ -1,118 +0,0 @@ -//===----------------------------------------------------------------------===// -// Copyright © 2026 Apple Inc. and the container project authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -//===----------------------------------------------------------------------===// - -import ContainerizationExtras -import Foundation - -public struct NetworkPluginStatus: Codable, Sendable { - /// The address allocated for the network if no subnet was specified at - /// creation time; otherwise, the subnet from the configuration. - public let ipv4Subnet: CIDRv4 - - /// The gateway IPv4 address. - public let ipv4Gateway: IPv4Address - - /// The address allocated for the IPv6 network if no subnet was specified at - /// creation time; otherwise, the IPv6 subnet from the configuration. - /// The value is nil if the IPv6 subnet cannot be determined at creation time. - public let ipv6Subnet: CIDRv6? - - public init( - ipv4Subnet: CIDRv4, - ipv4Gateway: IPv4Address, - ipv6Subnet: CIDRv6?, - ) { - self.ipv4Subnet = ipv4Subnet - self.ipv4Gateway = ipv4Gateway - self.ipv6Subnet = ipv6Subnet - } - - enum CodingKeys: String, CodingKey { - case ipv4Subnet - case ipv4Gateway - case ipv6Subnet - // TODO: retain for deserialization compatibility for now, remove later - case address - case gateway - } - - /// Create a configuration from the supplied Decoder, initializing missing - /// values where possible to reasonable defaults. - public init(from decoder: Decoder) throws { - let container = try decoder.container(keyedBy: CodingKeys.self) - - if let address = try? container.decode(CIDRv4.self, forKey: .ipv4Subnet) { - ipv4Subnet = address - } else { - ipv4Subnet = try container.decode(CIDRv4.self, forKey: .address) - } - if let gateway = try? container.decode(IPv4Address.self, forKey: .ipv4Gateway) { - ipv4Gateway = gateway - } else { - ipv4Gateway = try container.decode(IPv4Address.self, forKey: .gateway) - } - ipv6Subnet = try container.decodeIfPresent(String.self, forKey: .ipv6Subnet) - .map { try CIDRv6($0) } - } - - /// Encode the configuration to the supplied Encoder. - public func encode(to encoder: Encoder) throws { - var container = encoder.container(keyedBy: CodingKeys.self) - - try container.encode(ipv4Subnet, forKey: .ipv4Subnet) - try container.encode(ipv4Gateway, forKey: .ipv4Gateway) - try container.encodeIfPresent(ipv6Subnet, forKey: .ipv6Subnet) - } -} - -/// The configuration and runtime attributes for a network. -public enum NetworkState: Codable, Sendable { - // The network has been configured. - case created(NetworkConfiguration) - // The network is running. - case running(NetworkConfiguration, NetworkPluginStatus) - - public var state: String { - switch self { - case .created: "created" - case .running: "running" - } - } - - public var id: String { - switch self { - case .created(let config), .running(let config, _): config.id - } - } - - public var creationDate: Date { - switch self { - case .created(let config), .running(let config, _): config.creationDate - } - } - - public var isBuiltin: Bool { - switch self { - case .created(let config), .running(let config, _): config.labels.isBuiltin - } - } - - public var pluginInfo: NetworkPluginInfo? { - switch self { - case .created(let configuration), .running(let configuration, _): configuration.pluginInfo - } - } -} diff --git a/Sources/ContainerResource/Network/NetworkStatus.swift b/Sources/ContainerResource/Network/NetworkStatus.swift index 99baa6b3f..89164b677 100644 --- a/Sources/ContainerResource/Network/NetworkStatus.swift +++ b/Sources/ContainerResource/Network/NetworkStatus.swift @@ -15,55 +15,26 @@ //===----------------------------------------------------------------------===// import ContainerizationExtras -import Foundation -/// The runtime status of a network resource. -/// -/// `phase` names the current lifecycle stage; the address fields are present -/// only when `phase` is `"running"` and are `nil` otherwise. Clients should -/// treat unrecognised `phase` values as unknown forward-compatible stages rather -/// than treating them as errors. +/// The runtime status of a network — the addresses assigned once the network +/// plugin is active. Only present after the network has started. public struct NetworkStatus: Codable, Sendable { - /// The current lifecycle phase of the network. - /// - /// Defined values: `"created"` (configured, plugin not yet active) and - /// `"running"` (plugin active, subnet and gateway assigned). - public let phase: String + /// The IPv4 subnet assigned to the network. + public let ipv4Subnet: CIDRv4 - /// The allocated IPv4 subnet. Present only when `phase` is `"running"`. - public let ipv4Subnet: CIDRv4? + /// The IPv4 gateway address. + public let ipv4Gateway: IPv4Address - /// The IPv4 gateway address. Present only when `phase` is `"running"`. - public let ipv4Gateway: IPv4Address? - - /// The allocated IPv6 subnet. Present only when `phase` is `"running"` and - /// the network has IPv6 enabled. + /// The IPv6 subnet assigned to the network, if IPv6 is enabled. public let ipv6Subnet: CIDRv6? public init( - phase: String, - ipv4Subnet: CIDRv4? = nil, - ipv4Gateway: IPv4Address? = nil, - ipv6Subnet: CIDRv6? = nil + ipv4Subnet: CIDRv4, + ipv4Gateway: IPv4Address, + ipv6Subnet: CIDRv6? ) { - self.phase = phase self.ipv4Subnet = ipv4Subnet self.ipv4Gateway = ipv4Gateway self.ipv6Subnet = ipv6Subnet } } - -extension NetworkStatus { - /// The status value for a network that is configured but not yet running. - public static let created = NetworkStatus(phase: "created") - - /// Creates a running-phase status from a ``NetworkPluginStatus``. - init(running networkStatus: NetworkPluginStatus) { - self.init( - phase: "running", - ipv4Subnet: networkStatus.ipv4Subnet, - ipv4Gateway: networkStatus.ipv4Gateway, - ipv6Subnet: networkStatus.ipv6Subnet - ) - } -} diff --git a/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift b/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift index 2dadd428b..994933489 100644 --- a/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift +++ b/Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift @@ -82,17 +82,14 @@ extension NetworkVmnetHelper { log.info("configuring XPC server") let ipv4Subnet = try self.ipv4Subnet.map { try CIDRv4($0) } let ipv6Subnet = try self.ipv6Subnet.map { try CIDRv6($0) } - let pluginInfo = NetworkPluginInfo( - plugin: NetworkVmnetHelper._commandName, - variant: self.variant.rawValue - ) let configuration = try NetworkConfiguration( id: id, mode: mode, ipv4Subnet: ipv4Subnet, ipv6Subnet: ipv6Subnet, - pluginInfo: pluginInfo + plugin: NetworkVmnetHelper._commandName, + options: ["variant": self.variant.rawValue] ) let network = try Self.createNetwork( configuration: configuration, @@ -105,7 +102,7 @@ extension NetworkVmnetHelper { let xpc = XPCServer( identifier: serviceIdentifier, routes: [ - NetworkRoutes.state.rawValue: XPCServer.route(harness.state), + NetworkRoutes.status.rawValue: XPCServer.route(harness.status), NetworkRoutes.allocate.rawValue: harness.allocate, NetworkRoutes.lookup.rawValue: XPCServer.route(harness.lookup), ], diff --git a/Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift b/Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift index a2fa0b7db..3c7938b8e 100644 --- a/Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift +++ b/Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift @@ -64,11 +64,11 @@ extension RuntimeLinuxHelper { signal(SIGPIPE, SIG_IGN) // FIXME: The network plugins that the runtime supports should be configurable elsewhere - var interfaceStrategies: [NetworkPluginInfo: InterfaceStrategy] = [ - NetworkPluginInfo(plugin: "container-network-vmnet", variant: "allocationOnly"): IsolatedInterfaceStrategy() + var interfaceStrategies: [NetworkInterfaceKey: InterfaceStrategy] = [ + NetworkInterfaceKey(plugin: "container-network-vmnet", variant: "allocationOnly"): IsolatedInterfaceStrategy() ] if #available(macOS 26, *) { - interfaceStrategies[NetworkPluginInfo(plugin: "container-network-vmnet", variant: "reserved")] = NonisolatedInterfaceStrategy(log: log) + interfaceStrategies[NetworkInterfaceKey(plugin: "container-network-vmnet", variant: "reserved")] = NonisolatedInterfaceStrategy(log: log) } log.info("configuring XPC server") diff --git a/Sources/Services/ContainerAPIService/Client/NetworkClient.swift b/Sources/Services/ContainerAPIService/Client/NetworkClient.swift index 24683036e..b7ea51f31 100644 --- a/Sources/Services/ContainerAPIService/Client/NetworkClient.swift +++ b/Sources/Services/ContainerAPIService/Client/NetworkClient.swift @@ -82,18 +82,10 @@ public struct NetworkClient: Sendable { let response = try await xpcSend(message: request) - // Prefer current encoding (≥ 0.12.0 server). - if let resourceData = response.dataNoCopy(key: .networkResource) { - return try JSONDecoder().decode(NetworkResource.self, from: resourceData) + guard let resourceData = response.dataNoCopy(key: .networkResource) else { + throw ContainerizationError(.invalidArgument, message: "network configuration not received") } - - // Fall back to pre-0.12.0 server: decode NetworkState and convert. - if let stateData = response.dataNoCopy(key: .networkState) { - let state = try JSONDecoder().decode(NetworkState.self, from: stateData) - return NetworkResource(state) - } - - throw ContainerizationError(.invalidArgument, message: "network configuration not received") + return try JSONDecoder().decode(NetworkResource.self, from: resourceData) } /// Returns the current state of all networks known to the API server. @@ -106,17 +98,10 @@ public struct NetworkClient: Sendable { let response = try await xpcSend(message: request, timeout: .seconds(1)) - // Prefer current encoding (≥ 0.12.0 server). - if let resourceData = response.dataNoCopy(key: .networkResources) { - return try JSONDecoder().decode([NetworkResource].self, from: resourceData) + guard let resourceData = response.dataNoCopy(key: .networkResources) else { + return [] } - - // Fall back to pre-0.12.0 server: decode NetworkState and convert. - if let stateData = response.dataNoCopy(key: .networkStates) { - return try JSONDecoder().decode([NetworkState].self, from: stateData).map(NetworkResource.init) - } - - return [] + return try JSONDecoder().decode([NetworkResource].self, from: resourceData) } /// Returns the network with the given identifier. diff --git a/Sources/Services/ContainerAPIService/Client/Utility.swift b/Sources/Services/ContainerAPIService/Client/Utility.swift index f649b68aa..f405cf6d4 100644 --- a/Sources/Services/ContainerAPIService/Client/Utility.swift +++ b/Sources/Services/ContainerAPIService/Client/Utility.swift @@ -216,10 +216,7 @@ public struct Utility { dnsDomain: containerSystemConfig.dns.domain, ) for attachmentConfiguration in config.networks { - let network = try await networkClient.get(id: attachmentConfiguration.network) - guard network.status.phase == "running" else { - throw ContainerizationError(.invalidState, message: "network \(attachmentConfiguration.network) is not running") - } + _ = try await networkClient.get(id: attachmentConfiguration.network) } } diff --git a/Sources/Services/ContainerAPIService/Client/XPC+.swift b/Sources/Services/ContainerAPIService/Client/XPC+.swift index 4a6f6f42c..499b82b84 100644 --- a/Sources/Services/ContainerAPIService/Client/XPC+.swift +++ b/Sources/Services/ContainerAPIService/Client/XPC+.swift @@ -103,11 +103,6 @@ public enum XPCKeys: String { /// Network case networkId case networkConfig - case networkState - case networkStates - // Added in 0.12.0: NetworkResource encoding (status.phase shape). - // DEPRECATED 0.12.0: networkState/networkStates retained for down-revision - // client compatibility; remove at next major version boundary. case networkResource case networkResources diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index b18bf55d5..fdbc9759f 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -435,10 +435,10 @@ public actor ContainersService { var networkBootstrapInfos = [NetworkBootstrapInfo]() for n in config.networks { - guard let pluginInfo = try await self.networksService?.pluginInfo(id: n.network) else { - throw ContainerizationError(.internalError, message: "failed to get plugin info for network \(n.network)") + guard let (plugin, options) = try await self.networksService?.pluginConfiguration(id: n.network) else { + throw ContainerizationError(.internalError, message: "failed to get plugin configuration for network \(n.network)") } - networkBootstrapInfos.append(NetworkBootstrapInfo(pluginInfo: pluginInfo)) + networkBootstrapInfos.append(NetworkBootstrapInfo(plugin: plugin, options: options)) } do { diff --git a/Sources/Services/ContainerAPIService/Server/Networks/NetworksHarness.swift b/Sources/Services/ContainerAPIService/Server/Networks/NetworksHarness.swift index 6c5efa554..ec525be83 100644 --- a/Sources/Services/ContainerAPIService/Server/Networks/NetworksHarness.swift +++ b/Sources/Services/ContainerAPIService/Server/Networks/NetworksHarness.swift @@ -32,18 +32,11 @@ public struct NetworksHarness: Sendable { @Sendable public func list(_ message: XPCMessage) async throws -> XPCMessage { - let states = try await service.list() + let resources = try await service.list() let reply = message.reply() - - // Current encoding: NetworkResource with status.phase shape (≥ 0.12.0). - let resources = states.map(NetworkResource.init) reply.set(key: .networkResources, value: try JSONEncoder().encode(resources)) - // DEPRECATED 0.12.0 — retained for down-revision client compatibility. - // Remove at next major version boundary. - reply.set(key: .networkStates, value: try JSONEncoder().encode(states)) - return reply } @@ -55,16 +48,10 @@ public struct NetworksHarness: Sendable { } let config = try JSONDecoder().decode(NetworkConfiguration.self, from: data) - let networkState = try await service.create(configuration: config) + let resource = try await service.create(configuration: config) let reply = message.reply() - - // Current encoding: NetworkResource with status.phase shape (≥ 0.12.0). - reply.set(key: .networkResource, value: try JSONEncoder().encode(NetworkResource(networkState))) - - // DEPRECATED 0.12.0 — retained for down-revision client compatibility. - // Remove at next major version boundary. - reply.set(key: .networkState, value: try JSONEncoder().encode(networkState)) + reply.set(key: .networkResource, value: try JSONEncoder().encode(resource)) return reply } diff --git a/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift b/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift index 512194f2d..92655d229 100644 --- a/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift +++ b/Sources/Services/ContainerAPIService/Server/Networks/NetworksService.swift @@ -28,8 +28,9 @@ import Logging import SystemPackage public actor NetworksService { - struct NetworkServiceState { - var networkState: NetworkState + struct NetworkEntry { + var configuration: NetworkConfiguration + var status: NetworkStatus var client: ContainerNetworkClient.NetworkClient } @@ -44,7 +45,7 @@ public actor NetworksService { private var busyNetworks = Set() private let stateLock = AsyncLock() - private var serviceStates = [String: NetworkServiceState]() + private var serviceStates = [String: NetworkEntry]() public init( pluginLoader: PluginLoader, @@ -88,26 +89,45 @@ public actor NetworksService { } } - // Ensure that the network always has plugin information. - // Before this field was added, the code always assumed we were using the - // container-network-vmnet network plugin, so it should be safe to fallback to that - // if no info was found in an on disk configuration. - if updatedLabels != nil || configuration.pluginInfo == nil { + if let updatedLabels { let updatedConfiguration = try NetworkConfiguration( id: configuration.id, mode: configuration.mode, ipv4Subnet: configuration.ipv4Subnet, ipv6Subnet: configuration.ipv6Subnet, - labels: updatedLabels.map { try .init($0) } ?? configuration.labels, - pluginInfo: configuration.pluginInfo ?? NetworkPluginInfo(plugin: "container-network-vmnet") + labels: try .init(updatedLabels), + plugin: configuration.plugin, + options: configuration.options ) try await store.update(updatedConfiguration) - } // Start up the network. + // This call will normally take ~20-100ms to complete after service + // registration, but on a fresh system (e.g. CI runner), it may take + // 5 seconds or considerably more from the registration of this first + // network service to its execution. do { try await registerService(configuration: configuration) + let client = try Self.getClient(configuration: configuration) + let networkStatus = try await client.status() + let finalConfiguration = + updatedLabels.flatMap { labels in + try? NetworkConfiguration( + id: configuration.id, + mode: configuration.mode, + ipv4Subnet: configuration.ipv4Subnet, + ipv6Subnet: configuration.ipv6Subnet, + labels: (try? ResourceLabels(labels)) ?? configuration.labels, + plugin: configuration.plugin, + options: configuration.options + ) + } ?? configuration + serviceStates[finalConfiguration.id] = NetworkEntry( + configuration: finalConfiguration, + status: networkStatus, + client: client + ) } catch { log.error( "failed to start network", @@ -116,74 +136,21 @@ public actor NetworksService { "error": "\(error)", ]) } - - // This call will normally take ~20-100ms to complete after service - // registration, but on a fresh system (e.g. CI runner), it may take - // 5 seconds or considerably more from the registration of this first - // network service to its execution. - let client = try Self.getClient(configuration: configuration) - var networkState = try await client.state() - - // FIXME: Temporary workaround for persisted configuration being overwritten - // by what comes back from the network helper, which messes up creationDate. - // FIXME: Temporarily need to override the plugin information with the info from - // the helper, so we can ensure that older networks get a variant value. - let finalConfiguration: NetworkConfiguration - switch networkState { - case .created(let helperConfig): - finalConfiguration = try NetworkConfiguration( - id: configuration.id, - mode: configuration.mode, - ipv4Subnet: configuration.ipv4Subnet, - ipv6Subnet: configuration.ipv6Subnet, - labels: updatedLabels.map { try .init($0) } ?? configuration.labels, - pluginInfo: helperConfig.pluginInfo - ) - networkState = NetworkState.created(finalConfiguration) - case .running(let helperConfig, let status): - finalConfiguration = try NetworkConfiguration( - id: configuration.id, - mode: configuration.mode, - ipv4Subnet: configuration.ipv4Subnet, - ipv6Subnet: configuration.ipv6Subnet, - labels: updatedLabels.map { try .init($0) } ?? configuration.labels, - pluginInfo: helperConfig.pluginInfo - ) - networkState = NetworkState.running(finalConfiguration, status) - } - - let state = NetworkServiceState( - networkState: networkState, - client: client - ) - - serviceStates[finalConfiguration.id] = state - - guard case .running = networkState else { - log.error( - "network failed to start", - metadata: [ - "id": "\(finalConfiguration.id)", - "state": "\(networkState.state)", - ]) - return - } } } /// List all networks registered with the service. - public func list() async throws -> [NetworkState] { + public func list() async throws -> [NetworkResource] { log.debug("NetworksService: enter", metadata: ["func": "\(#function)"]) defer { log.debug("NetworksService: exit", metadata: ["func": "\(#function)"]) } - return serviceStates.reduce(into: [NetworkState]()) { - $0.append($1.value.networkState) - } - .sorted { $0.id < $1.id } + return serviceStates.values + .map { NetworkResource(configuration: $0.configuration, status: $0.status) } + .sorted { $0.id < $1.id } } /// Create a new network from the provided configuration. - public func create(configuration: NetworkConfiguration) async throws -> NetworkState { + public func create(configuration: NetworkConfiguration) async throws -> NetworkResource { log.debug( "NetworksService: enter", metadata: [ @@ -224,11 +191,8 @@ public actor NetworksService { try await self.registerService(configuration: configuration) let client = try Self.getClient(configuration: configuration) - // Ensure the network is running, and set up the persistent network state - // using our configuration data - guard case .running(let helperConfig, let status) = try await client.state() else { - throw ContainerizationError(.invalidState, message: "network \(configuration.id) failed to start") - } + // Ensure the network is running + let networkStatus = try await client.status() let finalConfiguration = try NetworkConfiguration( id: configuration.id, @@ -236,17 +200,17 @@ public actor NetworksService { ipv4Subnet: configuration.ipv4Subnet, ipv6Subnet: configuration.ipv6Subnet, labels: configuration.labels, - pluginInfo: helperConfig.pluginInfo + plugin: configuration.plugin, + options: configuration.options ) - let networkState: NetworkState = .running(finalConfiguration, status) - let serviceState = NetworkServiceState(networkState: networkState, client: client) - await self.setServiceState(key: finalConfiguration.id, value: serviceState) + let entry = NetworkEntry(configuration: finalConfiguration, status: networkStatus, client: client) + await self.setServiceState(key: finalConfiguration.id, value: entry) // Persist the configuration data. do { try await self.store.create(finalConfiguration) - return networkState + return NetworkResource(configuration: finalConfiguration, status: networkStatus) } catch { await self.removeServiceState(key: finalConfiguration.id) do { @@ -304,12 +268,8 @@ public actor NetworksService { throw ContainerizationError(.notFound, message: "no network for id \(id)") } - guard case .running(let netConfig, _) = serviceState.networkState else { - throw ContainerizationError(.invalidState, message: "cannot delete network \(id) in state \(serviceState.networkState.state)") - } - // basic sanity checks on network itself - if serviceState.networkState.isBuiltin { + if serviceState.configuration.labels.isBuiltin { throw ContainerizationError(.invalidArgument, message: "cannot delete builtin network: \(id)") } @@ -336,7 +296,7 @@ public actor NetworksService { // start network deletion, this is the last place we'll want to throw do { - try await self.deregisterService(configuration: netConfig) + try await self.deregisterService(configuration: serviceState.configuration) } catch { self.log.error( "failed to deregister network service", @@ -379,21 +339,23 @@ public actor NetworksService { } } - public func pluginInfo(id: String) throws -> NetworkPluginInfo { + public func pluginConfiguration(id: String) throws -> (plugin: String, options: [String: String]) { guard let serviceState = serviceStates[id] else { throw ContainerizationError(.notFound, message: "no network for id \(id)") } - guard let pluginInfo = serviceState.networkState.pluginInfo else { - throw ContainerizationError(.internalError, message: "network \(id) missing plugin information") + var options = serviceState.configuration.options + if options["variant"] == nil { + if #available(macOS 26, *) { + options["variant"] = "reserved" + } else { + options["variant"] = "allocationOnly" + } } - return pluginInfo + return (plugin: serviceState.configuration.plugin, options: options) } private static func getClient(configuration: NetworkConfiguration) throws -> ContainerNetworkClient.NetworkClient { - guard let pluginInfo = configuration.pluginInfo else { - throw ContainerizationError(.internalError, message: "network \(configuration.id) missing plugin information") - } - return NetworkClient(id: configuration.id, plugin: pluginInfo.plugin) + NetworkClient(id: configuration.id, plugin: configuration.plugin) } private func registerService(configuration: NetworkConfiguration) async throws { @@ -401,14 +363,10 @@ public actor NetworksService { throw ContainerizationError(.invalidArgument, message: "unsupported network mode \(configuration.mode.rawValue)") } - guard let pluginInfo = configuration.pluginInfo else { - throw ContainerizationError(.internalError, message: "network \(configuration.id) missing plugin information") - } - - guard let networkPlugin = self.networkPlugins.first(where: { $0.name == pluginInfo.plugin }) else { + guard let networkPlugin = self.networkPlugins.first(where: { $0.name == configuration.plugin }) else { throw ContainerizationError( .notFound, - message: "unable to locate network plugin \(pluginInfo.plugin)" + message: "unable to locate network plugin \(configuration.plugin)" ) } @@ -431,9 +389,7 @@ public actor NetworksService { if let ipv4Subnet = configuration.ipv4Subnet { var existingCidrs: [CIDRv4] = [] for serviceState in serviceStates.values { - if case .running(_, let status) = serviceState.networkState { - existingCidrs.append(status.ipv4Subnet) - } + existingCidrs.append(serviceState.status.ipv4Subnet) } let overlap = existingCidrs.first { $0.contains(ipv4Subnet.lower) @@ -451,7 +407,7 @@ public actor NetworksService { if let ipv6Subnet = configuration.ipv6Subnet { var existingCidrs: [CIDRv6] = [] for serviceState in serviceStates.values { - if case .running(_, let status) = serviceState.networkState, let otherIPv6Subnet = status.ipv6Subnet { + if let otherIPv6Subnet = serviceState.status.ipv6Subnet { existingCidrs.append(otherIPv6Subnet) } } @@ -468,7 +424,7 @@ public actor NetworksService { args += ["--subnet-v6", ipv6Subnet.description] } - if let variant = configuration.pluginInfo?.variant { + if let variant = configuration.options["variant"] { args += ["--variant", variant] } @@ -482,13 +438,10 @@ public actor NetworksService { } private func deregisterService(configuration: NetworkConfiguration) async throws { - guard let pluginInfo = configuration.pluginInfo else { - throw ContainerizationError(.internalError, message: "network \(configuration.id) missing plugin information") - } - guard let networkPlugin = self.networkPlugins.first(where: { $0.name == pluginInfo.plugin }) else { + guard let networkPlugin = self.networkPlugins.first(where: { $0.name == configuration.plugin }) else { throw ContainerizationError( .notFound, - message: "unable to locate network plugin \(pluginInfo.plugin)" + message: "unable to locate network plugin \(configuration.plugin)" ) } try self.pluginLoader.deregisterWithLaunchd(plugin: networkPlugin, instanceId: configuration.id) @@ -500,7 +453,7 @@ extension NetworksService { self.serviceStates.removeValue(forKey: key) } - private func setServiceState(key: String, value: NetworkServiceState) { + private func setServiceState(key: String, value: NetworkEntry) { self.serviceStates[key] = value } } diff --git a/Sources/Services/Network/Client/NetworkClient.swift b/Sources/Services/Network/Client/NetworkClient.swift index 18b6591ca..97598a786 100644 --- a/Sources/Services/Network/Client/NetworkClient.swift +++ b/Sources/Services/Network/Client/NetworkClient.swift @@ -44,33 +44,6 @@ public struct NetworkClient: Sendable { // Runtime Methods extension NetworkClient { - public func state() async throws -> NetworkState { - let request = XPCMessage(route: NetworkRoutes.state.rawValue) - let client = createClient() - - let response = try await client.send(request) - let state = try response.state() - return state - } - - public func allocate( - hostname: String, - macAddress: MACAddress? = nil - ) async throws -> (attachment: Attachment, additionalData: XPCMessage?) { - let request = XPCMessage(route: NetworkRoutes.allocate.rawValue) - request.set(key: NetworkKeys.hostname.rawValue, value: hostname) - if let macAddress = macAddress { - request.set(key: NetworkKeys.macAddress.rawValue, value: macAddress.description) - } - - let client = createClient() - - let response = try await client.send(request) - let attachment = try response.attachment() - let additionalData = response.additionalData() - return (attachment, additionalData) - } - /// Open a persistent connection to the network helper. /// /// The returned session should be reused for `allocate(on:)` calls. The @@ -80,6 +53,15 @@ extension NetworkClient { createClient().openSession() } + public func status() async throws -> NetworkStatus { + let request = XPCMessage(route: NetworkRoutes.status.rawValue) + let client = createClient() + + let response = try await client.send(request) + let status = try response.status() + return status + } + /// Allocate a network attachment over an existing session. /// /// Use `connect()` to obtain a session, then pass it here. The session @@ -142,11 +124,11 @@ extension XPCMessage { return hostname } - public func state() throws -> NetworkState { - let data = self.dataNoCopy(key: NetworkKeys.state.rawValue) + public func status() throws -> NetworkStatus { + let data = self.dataNoCopy(key: NetworkKeys.status.rawValue) guard let data else { throw ContainerizationError(.invalidArgument, message: "no network snapshot data in message") } - return try JSONDecoder().decode(NetworkState.self, from: data) + return try JSONDecoder().decode(NetworkStatus.self, from: data) } } diff --git a/Sources/Services/Network/Client/NetworkKeys.swift b/Sources/Services/Network/Client/NetworkKeys.swift index af10bdae7..b2ec74180 100644 --- a/Sources/Services/Network/Client/NetworkKeys.swift +++ b/Sources/Services/Network/Client/NetworkKeys.swift @@ -20,5 +20,5 @@ public enum NetworkKeys: String { case hostname case macAddress case network - case state + case status } diff --git a/Sources/Services/Network/Client/NetworkRoutes.swift b/Sources/Services/Network/Client/NetworkRoutes.swift index 6e3b91fe4..3fca4e986 100644 --- a/Sources/Services/Network/Client/NetworkRoutes.swift +++ b/Sources/Services/Network/Client/NetworkRoutes.swift @@ -15,8 +15,8 @@ //===----------------------------------------------------------------------===// public enum NetworkRoutes: String { - /// Return the current state of the network. - case state = "com.apple.container.network/state" + /// Return the current status of the network. + case status = "com.apple.container.network/status" /// Allocates parameters for attaching a sandbox to the network. case allocate = "com.apple.container.network/allocate" /// Retrieves the allocation for a hostname. diff --git a/Sources/Services/Network/Server/DefaultNetworkService.swift b/Sources/Services/Network/Server/DefaultNetworkService.swift index 43990a662..29f3fbe82 100644 --- a/Sources/Services/Network/Server/DefaultNetworkService.swift +++ b/Sources/Services/Network/Server/DefaultNetworkService.swift @@ -32,13 +32,11 @@ public actor DefaultNetworkService: NetworkService { network: any Network, log: Logger ) async throws { - let state = await network.state - guard case .running(_, let status) = state else { - throw ContainerizationError(.invalidState, message: "invalid network state - network \(state.id) must be running") + guard let status = await network.status else { + throw ContainerizationError(.invalidState, message: "network \(network.id) must be running") } let subnet = status.ipv4Subnet - let size = Int(subnet.upper.value - subnet.lower.value - 3) self.network = network self.log = log @@ -48,8 +46,11 @@ public actor DefaultNetworkService: NetworkService { } @Sendable - public func state() async throws -> NetworkState { - await network.state + public func status() async throws -> NetworkStatus { + guard let status = await network.status else { + throw ContainerizationError(.invalidState, message: "network \(network.id) is not running") + } + return status } @Sendable @@ -61,9 +62,8 @@ public actor DefaultNetworkService: NetworkService { log.debug("enter", metadata: ["func": "\(#function)"]) defer { log.debug("exit", metadata: ["func": "\(#function)"]) } - let state = await network.state - guard case .running(_, let status) = state else { - throw ContainerizationError(.invalidState, message: "invalid network state - network \(state.id) must be running") + guard let status = await network.status else { + throw ContainerizationError(.invalidState, message: "network \(network.id) must be running") } let macAddress = macAddress ?? MACAddress((UInt64.random(in: 0...UInt64.max) & 0x0cff_ffff_ffff) | 0xf200_0000_0000) @@ -72,7 +72,7 @@ public actor DefaultNetworkService: NetworkService { .map { try CIDRv6(macAddress.ipv6Address(network: $0.lower), prefix: $0.prefix) } let ip = IPv4Address(index) let attachment = Attachment( - network: state.id, + network: network.id, hostname: hostname, ipv4Address: try CIDRv4(ip, prefix: status.ipv4Subnet.prefix), ipv4Gateway: status.ipv4Gateway, @@ -122,9 +122,8 @@ public actor DefaultNetworkService: NetworkService { log.debug("enter", metadata: ["func": "\(#function)"]) defer { log.debug("exit", metadata: ["func": "\(#function)"]) } - let state = await network.state - guard case .running(_, let status) = state else { - throw ContainerizationError(.invalidState, message: "invalid network state - network \(state.id) must be running") + guard let status = await network.status else { + throw ContainerizationError(.invalidState, message: "network \(network.id) must be running") } // Invariant: hostname -> index if and only if index -> MAC address @@ -136,14 +135,13 @@ public actor DefaultNetworkService: NetworkService { return nil } - // populate attachment let address = IPv4Address(index) let subnet = status.ipv4Subnet let ipv4Address = try CIDRv4(address, prefix: subnet.prefix) let ipv6Address = try status.ipv6Subnet .map { try CIDRv6(macAddress.ipv6Address(network: $0.lower), prefix: $0.prefix) } let attachment = Attachment( - network: state.id, + network: network.id, hostname: hostname, ipv4Address: ipv4Address, ipv4Gateway: status.ipv4Gateway, diff --git a/Sources/Services/Network/Server/Network.swift b/Sources/Services/Network/Server/Network.swift index 9be7e0985..aca64d04e 100644 --- a/Sources/Services/Network/Server/Network.swift +++ b/Sources/Services/Network/Server/Network.swift @@ -19,12 +19,15 @@ import ContainerXPC /// Defines common characteristics and operations for a network. public protocol Network: Sendable { - // Contains network attributes while the network is running - var state: NetworkState { get async } + /// The network's identifier. + var id: String { get } - // Use implementation-dependent network attributes + /// The network's runtime status. `nil` before ``start()`` completes. + var status: NetworkStatus? { get async } + + /// Use implementation-dependent network attributes. nonisolated func withAdditionalData(_ handler: (XPCMessage?) throws -> Void) throws - // Start the network + /// Start the network. func start() async throws } diff --git a/Sources/Services/Network/Server/NetworkHarness.swift b/Sources/Services/Network/Server/NetworkHarness.swift index 28023de6d..6c9a96a16 100644 --- a/Sources/Services/Network/Server/NetworkHarness.swift +++ b/Sources/Services/Network/Server/NetworkHarness.swift @@ -28,10 +28,10 @@ public actor NetworkHarness: Sendable { } @Sendable - public func state(_ message: XPCMessage) async throws -> XPCMessage { + public func status(_ message: XPCMessage) async throws -> XPCMessage { let reply = message.reply() - let state = try await service.state() - try reply.setState(state) + let status = try await service.status() + try reply.setStatus(status) return reply } @@ -80,8 +80,9 @@ extension XPCMessage { self.set(key: NetworkKeys.attachment.rawValue, value: data) } - fileprivate func setState(_ state: NetworkState) throws { - let data = try JSONEncoder().encode(state) - self.set(key: NetworkKeys.state.rawValue, value: data) + fileprivate func setStatus(_ status: NetworkStatus) throws { + let data = try JSONEncoder().encode(status) + self.set(key: NetworkKeys.status.rawValue, value: data) } + } diff --git a/Sources/Services/Network/Server/NetworkService.swift b/Sources/Services/Network/Server/NetworkService.swift index 3644fc838..6e9f2b784 100644 --- a/Sources/Services/Network/Server/NetworkService.swift +++ b/Sources/Services/Network/Server/NetworkService.swift @@ -21,7 +21,7 @@ import ContainerizationExtras /// A network service public protocol NetworkService: Sendable { /// Gets the properties of the realized network. - func state() async throws -> NetworkState + func status() async throws -> NetworkStatus /// Register a hostname and allocate associated addresses. func allocate( diff --git a/Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift b/Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift index c17e60242..321baf1e2 100644 --- a/Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift +++ b/Sources/Services/NetworkVmnet/Server/AllocationOnlyVmnetNetwork.swift @@ -25,8 +25,9 @@ public actor AllocationOnlyVmnetNetwork: Network { // The IPv4 subnet to be used if none explicitly passed in the `NetworkConfiguration` private static let defaultIPv4Subnet = try! CIDRv4("192.168.64.1/24") + private let configuration: NetworkConfiguration private let log: Logger - private var _state: NetworkState + private var _status: NetworkStatus? /// Configure a bridge network that allows external system access using /// network address translation. @@ -42,21 +43,22 @@ public actor AllocationOnlyVmnetNetwork: Network { throw ContainerizationError(.unsupported, message: "IPv6 subnet assignment is not yet implemented") } + self.configuration = configuration self.log = log - self._state = .created(configuration) + self._status = nil } - public var state: NetworkState { - self._state - } + public nonisolated var id: String { configuration.id } + + public var status: NetworkStatus? { _status } public nonisolated func withAdditionalData(_ handler: (XPCMessage?) throws -> Void) throws { try handler(nil) } public func start() async throws { - guard case .created(let configuration) = _state else { - throw ContainerizationError(.invalidState, message: "cannot start network \(_state.id) in \(_state.state) state") + guard _status == nil else { + throw ContainerizationError(.invalidState, message: "cannot start network \(configuration.id): already started") } log.info( @@ -68,14 +70,12 @@ public actor AllocationOnlyVmnetNetwork: Network { ) let ipv4Subnet = configuration.ipv4Subnet ?? Self.defaultIPv4Subnet - let gateway = IPv4Address(ipv4Subnet.lower.value + 1) - let status = NetworkPluginStatus( + self._status = NetworkStatus( ipv4Subnet: ipv4Subnet, ipv4Gateway: gateway, - ipv6Subnet: nil, + ipv6Subnet: nil ) - self._state = .running(configuration, status) log.info( "started allocation-only network", metadata: [ diff --git a/Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift b/Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift index 43a325823..c11942427 100644 --- a/Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift +++ b/Sources/Services/NetworkVmnet/Server/ReservedVmnetNetwork.swift @@ -29,7 +29,7 @@ import vmnet @available(macOS 26, *) public final class ReservedVmnetNetwork: ContainerNetworkServer.Network { private struct State { - var networkState: NetworkState + var status: NetworkStatus? var network: vmnet_network_ref? } @@ -40,6 +40,7 @@ public final class ReservedVmnetNetwork: ContainerNetworkServer.Network { let ipv6Subnet: CIDRv6 } + private let configuration: NetworkConfiguration private let stateMutex: Mutex private let log: Logger @@ -54,14 +55,16 @@ public final class ReservedVmnetNetwork: ContainerNetworkServer.Network { } log.info("creating vmnet network") + self.configuration = configuration self.log = log - let initialState = State(networkState: .created(configuration)) - stateMutex = Mutex(initialState) + stateMutex = Mutex(State()) log.info("created vmnet network") } - public var state: NetworkState { - stateMutex.withLock { $0.networkState } + public nonisolated var id: String { configuration.id } + + public var status: NetworkStatus? { + stateMutex.withLock { $0.status } } public nonisolated func withAdditionalData(_ handler: (XPCMessage?) throws -> Void) throws { @@ -72,18 +75,17 @@ public final class ReservedVmnetNetwork: ContainerNetworkServer.Network { public func start() async throws { try stateMutex.withLock { state in - guard case .created(let configuration) = state.networkState else { - throw ContainerizationError(.invalidArgument, message: "cannot start network that is in \(state.networkState.state) state") + guard state.status == nil else { + throw ContainerizationError(.invalidArgument, message: "cannot start network \(configuration.id): already started") } let networkInfo = try startNetwork(configuration: configuration, log: log) - let networkStatus = NetworkPluginStatus( + state.status = NetworkStatus( ipv4Subnet: networkInfo.ipv4Subnet, ipv4Gateway: networkInfo.ipv4Gateway, - ipv6Subnet: networkInfo.ipv6Subnet, + ipv6Subnet: networkInfo.ipv6Subnet ) - state.networkState = NetworkState.running(configuration, networkStatus) state.network = networkInfo.network } } diff --git a/Sources/Services/Runtime/RuntimeClient/InterfaceStrategy.swift b/Sources/Services/Runtime/RuntimeClient/InterfaceStrategy.swift index 1fae0a783..24d842df3 100644 --- a/Sources/Services/Runtime/RuntimeClient/InterfaceStrategy.swift +++ b/Sources/Services/Runtime/RuntimeClient/InterfaceStrategy.swift @@ -18,6 +18,17 @@ import ContainerResource import ContainerXPC import Containerization +/// Key identifying which interface strategy to use for a network attachment. +public struct NetworkInterfaceKey: Hashable, Sendable { + public let plugin: String + public let variant: String? + + public init(plugin: String, variant: String?) { + self.plugin = plugin + self.variant = variant + } +} + /// A strategy for mapping network attachment information to a network interface. public protocol InterfaceStrategy: Sendable { /// Map a client network attachment request to a network interface specification. diff --git a/Sources/Services/Runtime/RuntimeClient/NetworkBootstrapInfo.swift b/Sources/Services/Runtime/RuntimeClient/NetworkBootstrapInfo.swift index 613e9b6a2..f4461b82c 100644 --- a/Sources/Services/Runtime/RuntimeClient/NetworkBootstrapInfo.swift +++ b/Sources/Services/Runtime/RuntimeClient/NetworkBootstrapInfo.swift @@ -19,11 +19,14 @@ import ContainerResource /// Plugin info passed from the API server in the sandbox bootstrap message so the /// runtime can connect to the correct network helper and configure the interface. public struct NetworkBootstrapInfo: Codable, Sendable { - /// Plugin info identifying which network helper to contact and which interface - /// strategy the runtime should use. - public let pluginInfo: NetworkPluginInfo + /// The network plugin name identifying which network helper to contact. + public let plugin: String - public init(pluginInfo: NetworkPluginInfo) { - self.pluginInfo = pluginInfo + /// Plugin-specific options, including `variant` which selects the interface strategy. + public let options: [String: String] + + public init(plugin: String, options: [String: String] = [:]) { + self.plugin = plugin + self.options = options } } diff --git a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift index 27ae89cb4..1f23e0966 100644 --- a/Sources/Services/RuntimeLinux/Server/RuntimeService.swift +++ b/Sources/Services/RuntimeLinux/Server/RuntimeService.swift @@ -40,7 +40,7 @@ import struct ContainerizationOCI.Process public actor RuntimeService { private let connection: xpc_connection_t private let root: URL - private let interfaceStrategies: [NetworkPluginInfo: InterfaceStrategy] + private let interfaceStrategies: [NetworkInterfaceKey: InterfaceStrategy] private var container: ContainerInfo? private let monitor: ExitMonitor private let eventLoopGroup: any EventLoopGroup @@ -96,7 +96,7 @@ public actor RuntimeService { public init( root: URL, - interfaceStrategies: [NetworkPluginInfo: InterfaceStrategy], + interfaceStrategies: [NetworkInterfaceKey: InterfaceStrategy], eventLoopGroup: any EventLoopGroup, connection: xpc_connection_t, log: Logger @@ -177,7 +177,7 @@ public actor RuntimeService { do { for (index, info) in networkBootstrapInfos.enumerated() { let attachmentConfig = config.networks[index] - let client = ContainerNetworkClient.NetworkClient(id: attachmentConfig.network, plugin: info.pluginInfo.plugin) + let client = ContainerNetworkClient.NetworkClient(id: attachmentConfig.network, plugin: info.plugin) let session = client.connect() sessions.append(session) var (attachment, additionalData) = try await client.allocate( @@ -196,9 +196,10 @@ public actor RuntimeService { mtu: mtu ) } - guard let iStrategy = self.interfaceStrategies[info.pluginInfo] else { + guard let iStrategy = self.interfaceStrategies[NetworkInterfaceKey(plugin: info.plugin, variant: info.options["variant"])] else { throw ContainerizationError( - .internalError, message: "no available interface strategy for network \(attachment.network), \(info.pluginInfo)") + .internalError, + message: "no available interface strategy for network \(attachment.network), plugin=\(info.plugin) variant=\(info.options["variant"] ?? "nil")") } let interface = try iStrategy.toInterface( attachment: attachment, diff --git a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift index 6469afe69..2e30c74a9 100644 --- a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift +++ b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift @@ -290,7 +290,7 @@ class TestCLINetwork: CLITest { let (_, output, error, status) = try run(arguments: ["network", "list"]) #expect(status == 0, "network list should succeed, stderr: \(error)") - let headers = ["NETWORK", "STATE", "SUBNET"] + let headers = ["NETWORK", "SUBNET"] #expect(headers.allSatisfy { output.contains($0) }, "table should contain all headers") #expect(output.contains(name), "table should contain the created network") } diff --git a/Tests/CLITests/Utilities/CLITest.swift b/Tests/CLITests/Utilities/CLITest.swift index e086c08ed..abc6fb8e9 100644 --- a/Tests/CLITests/Utilities/CLITest.swift +++ b/Tests/CLITests/Utilities/CLITest.swift @@ -53,9 +53,8 @@ class CLITest { let ipv6Subnet: String? } let id: String - let state: String let configuration: NetworkConfiguration - let status: Status? + let status: Status } let testName: String diff --git a/Tests/ContainerCommandsTests/ListFormattingTests.swift b/Tests/ContainerCommandsTests/ListFormattingTests.swift index 501f5af20..0eac5a5c0 100644 --- a/Tests/ContainerCommandsTests/ListFormattingTests.swift +++ b/Tests/ContainerCommandsTests/ListFormattingTests.swift @@ -240,9 +240,9 @@ struct PrintableContainerDisplayTests { struct NetworkResourceDisplayTests { @Test - func tableHeaderHasThreeColumns() { - #expect(NetworkResource.tableHeader.count == 3) - #expect(NetworkResource.tableHeader == ["NETWORK", "STATE", "SUBNET"]) + func tableHeaderHasTwoColumns() { + #expect(NetworkResource.tableHeader.count == 2) + #expect(NetworkResource.tableHeader == ["NETWORK", "SUBNET"]) } } diff --git a/Tests/ContainerResourceTests/NetworkConfigurationTest.swift b/Tests/ContainerResourceTests/NetworkConfigurationTest.swift index 6fff998ee..dfbaf8644 100644 --- a/Tests/ContainerResourceTests/NetworkConfigurationTest.swift +++ b/Tests/ContainerResourceTests/NetworkConfigurationTest.swift @@ -21,14 +21,12 @@ import Testing @testable import ContainerResource struct NetworkConfigurationTest { - let defaultNetworkPluginInfo = NetworkPluginInfo(plugin: "container-network-vmnet") - @Test func testValidationOkDefaults() throws { let id = "foo" _ = try NetworkConfiguration( id: id, mode: .nat, - pluginInfo: defaultNetworkPluginInfo + plugin: "container-network-vmnet" ) } @@ -49,7 +47,7 @@ struct NetworkConfigurationTest { mode: .nat, ipv4Subnet: ipv4Subnet, labels: labels, - pluginInfo: defaultNetworkPluginInfo + plugin: "container-network-vmnet" ) } } @@ -73,7 +71,7 @@ struct NetworkConfigurationTest { mode: .nat, ipv4Subnet: ipv4Subnet, labels: labels, - pluginInfo: defaultNetworkPluginInfo + plugin: "container-network-vmnet" ) } throws: { error in guard let err = error as? ContainerizationError else { return false } diff --git a/docs/command-reference.md b/docs/command-reference.md index a15fb8d26..373dbd149 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -753,7 +753,7 @@ Creates a new network with the given name. **Usage** ```bash -container network create [--label