diff --git a/Package.swift b/Package.swift index b2f5ba69c..641100af8 100644 --- a/Package.swift +++ b/Package.swift @@ -292,6 +292,7 @@ let package = Package( .product(name: "SystemPackage", package: "swift-system"), "ContainerAPIClient", "ContainerPersistence", + "ContainerResource", "ContainerTestSupport", ] ), diff --git a/Sources/APIServer/APIServer+Start.swift b/Sources/APIServer/APIServer+Start.swift index 0c1b82a91..5bdd40b84 100644 --- a/Sources/APIServer/APIServer+Start.swift +++ b/Sources/APIServer/APIServer+Start.swift @@ -292,7 +292,9 @@ extension APIServer { routes[XPCRoute.containerList] = XPCServer.route(harness.list) routes[XPCRoute.containerCreate] = XPCServer.route(harness.create) + routes[XPCRoute.containerCreateWithResult] = XPCServer.route(harness.create) routes[XPCRoute.containerDelete] = XPCServer.route(harness.delete) + routes[XPCRoute.containerDeleteIfInstance] = XPCServer.route(harness.deleteIfInstance) routes[XPCRoute.containerLogs] = XPCServer.route(harness.logs) routes[XPCRoute.containerBootstrap] = XPCServer.route(harness.bootstrap) routes[XPCRoute.containerDial] = XPCServer.route(harness.dial) diff --git a/Sources/ContainerCommands/Container/ContainerCreate.swift b/Sources/ContainerCommands/Container/ContainerCreate.swift index 97febb060..661dff9b6 100644 --- a/Sources/ContainerCommands/Container/ContainerCreate.swift +++ b/Sources/ContainerCommands/Container/ContainerCreate.swift @@ -25,6 +25,10 @@ import TerminalProgress extension Application { public struct ContainerCreate: AsyncLoggableCommand { + enum OutputFormat: String, CaseIterable, ExpressibleByArgument, Sendable { + case json + } + public init() {} public static let configuration = CommandConfiguration( @@ -49,6 +53,9 @@ extension Application { @OptionGroup public var logOptions: Flags.Logging + @Option(help: "Output the server-generated container identity (values: json)") + var format: OutputFormat? + @Argument(help: "Image name") var image: String @@ -91,11 +98,29 @@ extension Application { let options = ContainerCreateOptions(autoRemove: managementFlags.remove) let client = ContainerClient() - try await client.create(configuration: ck.0, options: options, kernel: ck.1, initImage: ck.2) + let createResult: ContainerCreateResult? + if format == .json { + createResult = try await client.createWithResult( + configuration: ck.0, + options: options, + kernel: ck.1, + initImage: ck.2 + ) + } else { + _ = try await client.create( + configuration: ck.0, + options: options, + kernel: ck.1, + initImage: ck.2 + ) + createResult = nil + } + + let createdID = createResult?.id ?? id if !self.managementFlags.cidfile.isEmpty { let path = self.managementFlags.cidfile - let data = id.data(using: .utf8) + let data = createdID.data(using: .utf8) var attributes = [FileAttributeKey: Any]() attributes[.posixPermissions] = 0o644 let success = FileManager.default.createFile( @@ -110,7 +135,11 @@ extension Application { } progress.finish() - print(id) + if let createResult { + Output.emit(try Output.renderJSON(createResult)) + } else { + print(createdID) + } } } } diff --git a/Sources/ContainerCommands/Container/ContainerDelete.swift b/Sources/ContainerCommands/Container/ContainerDelete.swift index 1eddc6b85..890b809d0 100644 --- a/Sources/ContainerCommands/Container/ContainerDelete.swift +++ b/Sources/ContainerCommands/Container/ContainerDelete.swift @@ -35,6 +35,9 @@ extension Application { @Flag(name: .shortAndLong, help: "Delete containers even if they are running") var force = false + @Option(help: "Delete only if the container has this instance token") + var ifInstanceToken: String? + @OptionGroup public var logOptions: Flags.Logging @@ -51,11 +54,18 @@ extension Application { message: "explicitly supplied container ID(s) conflict with the --all flag" ) } + if ifInstanceToken != nil && (all || containerIds.count != 1) { + throw ContainerizationError( + .invalidArgument, + message: "--if-instance-token requires exactly one explicit container ID" + ) + } } public mutating func run() async throws { let client = ContainerClient() let force = self.force + let expectedInstanceToken = self.ifInstanceToken let containers: [String] if all { @@ -76,7 +86,15 @@ extension Application { for container in containers { group.addTask { do { - try await client.delete(id: container, force: force) + if let expectedInstanceToken { + try await client.deleteIfInstance( + id: container, + force: force, + expectedInstanceToken: expectedInstanceToken + ) + } else { + try await client.delete(id: container, force: force) + } print(container) return nil } catch { diff --git a/Sources/ContainerResource/Container/ContainerConfiguration.swift b/Sources/ContainerResource/Container/ContainerConfiguration.swift index 87e0f9049..b25bd1730 100644 --- a/Sources/ContainerResource/Container/ContainerConfiguration.swift +++ b/Sources/ContainerResource/Container/ContainerConfiguration.swift @@ -20,6 +20,11 @@ import Foundation public struct ContainerConfiguration: Sendable, Codable { /// Identifier for the container. public var id: String + /// Opaque server-generated identity for this incarnation of the container. + /// + /// Older persisted containers may not have a token. The API server assigns + /// a new token on every create and ignores any value supplied by a client. + public var instanceToken: String? /// Image used to create the container. public var image: ImageDescription /// External mounts to add to the container. @@ -75,6 +80,7 @@ public struct ContainerConfiguration: Sendable, Codable { enum CodingKeys: String, CodingKey { case id + case instanceToken case image case mounts case publishedPorts @@ -107,6 +113,7 @@ public struct ContainerConfiguration: Sendable, Codable { let container = try decoder.container(keyedBy: CodingKeys.self) id = try container.decode(String.self, forKey: .id) + instanceToken = try container.decodeIfPresent(String.self, forKey: .instanceToken) image = try container.decode(ImageDescription.self, forKey: .image) mounts = try container.decodeIfPresent([Filesystem].self, forKey: .mounts) ?? [] publishedPorts = try container.decodeIfPresent([PublishPort].self, forKey: .publishedPorts) ?? [] diff --git a/Sources/ContainerResource/Container/ContainerCreateResult.swift b/Sources/ContainerResource/Container/ContainerCreateResult.swift new file mode 100644 index 000000000..1252afb0c --- /dev/null +++ b/Sources/ContainerResource/Container/ContainerCreateResult.swift @@ -0,0 +1,29 @@ +//===----------------------------------------------------------------------===// +// 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. +//===----------------------------------------------------------------------===// + +/// The server-owned identity returned by an atomic container create operation. +public struct ContainerCreateResult: Codable, Equatable, Sendable { + /// The reusable container identifier selected by the caller. + public let id: String + + /// The opaque identity generated by the API server for this incarnation. + public let instanceToken: String + + public init(id: String, instanceToken: String) { + self.id = id + self.instanceToken = instanceToken + } +} diff --git a/Sources/ContainerResource/Container/ManagedContainer.swift b/Sources/ContainerResource/Container/ManagedContainer.swift index dbaae064a..f194997d2 100644 --- a/Sources/ContainerResource/Container/ManagedContainer.swift +++ b/Sources/ContainerResource/Container/ManagedContainer.swift @@ -41,6 +41,9 @@ public struct ManagedContainer: ManagedResource { /// not the protocol's 64-hex default. public static func generateId() -> String { UUID().uuidString.lowercased() } + /// Mint an opaque, high-entropy identity for one container incarnation. + public static func generateInstanceToken() -> String { UUID().uuidString.lowercased() } + /// Container name rule public static func nameValid(_ name: String) -> Bool { // Maximum Linux hostname length is 64, but limit to maximum DNS label length diff --git a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift index b52538d94..04a3e2692 100644 --- a/Sources/Services/ContainerAPIService/Client/ContainerClient.swift +++ b/Sources/Services/ContainerAPIService/Client/ContainerClient.swift @@ -45,6 +45,9 @@ public struct ContainerClient: Sendable { } /// Create a new container with the given configuration. + /// + /// This legacy API intentionally returns `Void`. Call ``createWithResult(configuration:options:kernel:initImage:runtimeData:)`` + /// when the authoritative server-generated instance identity is required. public func create( configuration: ContainerConfiguration, options: ContainerCreateOptions = .default, @@ -52,8 +55,74 @@ public struct ContainerClient: Sendable { initImage: String? = nil, runtimeData: Data? = nil ) async throws { + _ = try await create( + configuration: configuration, + options: options, + kernel: kernel, + initImage: initImage, + runtimeData: runtimeData, + route: .containerCreate + ) + } + + /// Create a new container and require its server-generated instance identity. + /// + /// The capability check and distinct route ensure that an older server + /// rejects the request before creating a container without a result. + public func createWithResult( + configuration: ContainerConfiguration, + options: ContainerCreateOptions = .default, + kernel: Kernel, + initImage: String? = nil, + runtimeData: Data? = nil + ) async throws -> ContainerCreateResult { + do { + let capabilityRequest = XPCMessage(route: .ping) + let capabilityReply = try await xpcSend(message: capabilityRequest, timeout: .seconds(10)) + guard capabilityReply.bool(key: .containerCreateResultSupported) else { + throw ContainerizationError( + .unsupported, + message: "API server does not support atomic container create results" + ) + } + + guard + let result = try await create( + configuration: configuration, + options: options, + kernel: kernel, + initImage: initImage, + runtimeData: runtimeData, + route: .containerCreateWithResult + ) + else { + throw ContainerizationError( + .internalError, + message: "API server omitted the atomic container create result" + ) + } + return result + } catch let error as ContainerizationError { + throw error + } catch { + throw ContainerizationError( + .internalError, + message: "failed to create container with an atomic result", + cause: error + ) + } + } + + private func create( + configuration: ContainerConfiguration, + options: ContainerCreateOptions, + kernel: Kernel, + initImage: String?, + runtimeData: Data?, + route: XPCRoute + ) async throws -> ContainerCreateResult? { do { - let request = XPCMessage(route: .containerCreate) + let request = XPCMessage(route: route) let data = try JSONEncoder().encode(configuration) let kdata = try JSONEncoder().encode(kernel) @@ -70,7 +139,8 @@ public struct ContainerClient: Sendable { request.set(key: .runtimeData, value: runtimeData) } - try await xpcSend(message: request) + let response = try await xpcSend(message: request) + return try Self.decodeCreateResult(response.dataNoCopy(key: .containerCreateResult)) } catch let error as ContainerizationError { throw error } catch { @@ -82,6 +152,13 @@ public struct ContainerClient: Sendable { } } + static func decodeCreateResult(_ data: Data?) throws -> ContainerCreateResult? { + guard let data else { + return nil + } + return try JSONDecoder().decode(ContainerCreateResult.self, from: data) + } + /// List containers matching the given filters. public func list(filters: ContainerListFilters = .all) async throws -> [ContainerSnapshot] { do { @@ -211,6 +288,44 @@ public struct ContainerClient: Sendable { } } + /// Delete the container only when its current instance identity matches. + public func deleteIfInstance(id: String, force: Bool, expectedInstanceToken: String) async throws { + try await Self.withConditionalDeleteErrorPreservation { + do { + let capabilityRequest = XPCMessage(route: .ping) + let capabilityReply = try await xpcSend(message: capabilityRequest, timeout: .seconds(10)) + guard capabilityReply.bool(key: .conditionalContainerDeleteSupported) else { + throw ContainerizationError( + .unsupported, + message: "API server does not support conditional container deletion" + ) + } + + let request = XPCMessage(route: .containerDeleteIfInstance) + request.set(key: .id, value: id) + request.set(key: .forceDelete, value: force) + request.set(key: .expectedInstanceToken, value: expectedInstanceToken) + try await xpcClient.send(request) + } + } + } + + static func withConditionalDeleteErrorPreservation( + _ operation: @Sendable () async throws -> Void + ) async throws { + do { + try await operation() + } catch let error as ContainerizationError { + throw error + } catch { + throw ContainerizationError( + .internalError, + message: "failed to conditionally delete container", + cause: error + ) + } + } + /// Get the disk usage for a container. public func diskUsage(id: String) async throws -> UInt64 { let request = XPCMessage(route: .containerDiskUsage) diff --git a/Sources/Services/ContainerAPIService/Client/XPC+.swift b/Sources/Services/ContainerAPIService/Client/XPC+.swift index 7fdd83537..e4f02824d 100644 --- a/Sources/Services/ContainerAPIService/Client/XPC+.swift +++ b/Sources/Services/ContainerAPIService/Client/XPC+.swift @@ -30,6 +30,8 @@ public enum XPCKeys: String { case processIdentifier /// Container configuration key. case containerConfig + /// Atomic container create result key. + case containerCreateResult /// Container options key. case containerOptions /// Opaque runtime-specific data. @@ -52,6 +54,12 @@ public enum XPCKeys: String { case stopOptions /// Whether to force stop a container when deleting. case forceDelete + /// Expected server-generated identity for conditional container deletion. + case expectedInstanceToken + /// Whether the server implements atomic conditional container deletion. + case conditionalContainerDeleteSupported + /// Whether the server implements create with an atomic instance identity result. + case containerCreateResultSupported /// Plugins case pluginName case plugins @@ -148,11 +156,13 @@ public enum XPCKeys: String { public enum XPCRoute: String { case containerList case containerCreate + case containerCreateWithResult case containerBootstrap case containerCreateProcess case containerStartProcess case containerWait case containerDelete + case containerDeleteIfInstance case containerStop case containerDial case containerResize diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift index 72f78b337..5a26c7d18 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersHarness.swift @@ -204,8 +204,16 @@ public struct ContainersHarness: Sendable { let initImage = message.string(key: .initImage) let runtimeData = message.dataNoCopy(key: .runtimeData) - try await service.create(configuration: config, kernel: kernel, options: options, initImage: initImage, runtimeData: runtimeData) - return message.reply() + let result = try await service.create( + configuration: config, + kernel: kernel, + options: options, + initImage: initImage, + runtimeData: runtimeData + ) + let reply = message.reply() + reply.set(key: .containerCreateResult, value: try JSONEncoder().encode(result)) + return reply } @Sendable @@ -264,6 +272,21 @@ public struct ContainersHarness: Sendable { @Sendable public func delete(_ message: XPCMessage) async throws -> XPCMessage { + try await delete(message, expectedInstanceToken: nil) + } + + @Sendable + public func deleteIfInstance(_ message: XPCMessage) async throws -> XPCMessage { + guard let expectedInstanceToken = message.string(key: .expectedInstanceToken) else { + throw ContainerizationError( + .invalidArgument, + message: "expected container instance token cannot be empty" + ) + } + return try await delete(message, expectedInstanceToken: expectedInstanceToken) + } + + private func delete(_ message: XPCMessage, expectedInstanceToken: String?) async throws -> XPCMessage { let id = message.string(key: .id) guard let id else { throw ContainerizationError(.invalidArgument, message: "id cannot be empty") @@ -272,7 +295,15 @@ public struct ContainersHarness: Sendable { throw ContainerizationError(.invalidArgument, message: "container ID \(id) is not a valid container ID") } let forceDelete = message.bool(key: .forceDelete) - try await service.delete(id: id, force: forceDelete) + if let expectedInstanceToken { + try await service.deleteIfInstance( + id: id, + force: forceDelete, + expectedInstanceToken: expectedInstanceToken + ) + } else { + try await service.delete(id: id, force: forceDelete) + } return message.reply() } diff --git a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift index 06a798cfc..030f05b55 100644 --- a/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift +++ b/Sources/Services/ContainerAPIService/Server/Containers/ContainersService.swift @@ -61,6 +61,7 @@ public actor ContainersService { private let lock: AsyncLock private var containers: [String: ContainerState] + private var deletions: Set // FIXME: Find a better mechanism for services running on the APIServer to work with each other private weak var networksService: NetworksService? @@ -82,14 +83,20 @@ public actor ContainersService { self.log = log self.debugHelpers = debugHelpers self.runtimePlugins = pluginLoader.findPlugins().filter { $0.hasType(.runtime) } - self.containers = try Self.loadAtBoot(root: containerRoot, loader: pluginLoader, log: log) + let recovered = try Self.loadAtBoot(root: containerRoot, loader: pluginLoader, log: log) + self.containers = recovered.containers + self.deletions = recovered.reservations } public func setNetworksService(_ service: NetworksService) async { self.networksService = service } - static func loadAtBoot(root: URL, loader: PluginLoader, log: Logger) throws -> [String: ContainerState] { + static func loadAtBoot( + root: URL, + loader: PluginLoader, + log: Logger + ) throws -> (containers: [String: ContainerState], reservations: Set) { var directories = try FileManager.default.contentsOfDirectory( at: root, includingPropertiesForKeys: [.isDirectoryKey] @@ -100,9 +107,12 @@ public actor ContainersService { let runtimePlugins = loader.findPlugins().filter { $0.hasType(.runtime) } var results = [String: ContainerState]() + var reservations = Set() for dir in directories { + var candidateIDs: Set = [dir.lastPathComponent] do { let (config, options) = try Self.getContainerConfiguration(at: dir) + candidateIDs.insert(config.id) if options?.autoRemove ?? false { log.info( "reap auto-remove container", @@ -128,10 +138,27 @@ public actor ContainersService { } let bundle = ContainerResource.Bundle(path: dir) - try? bundle.delete() + do { + try bundle.delete() + } catch { + reservations.formUnion(candidateIDs) + log.error( + "failed to delete auto-remove container bundle; reserving container ID", + metadata: [ + "id": "\(config.id)", + "path": "\(dir.path)", + "error": "\(error)", + ]) + } continue } + guard runtimePlugins.first(where: { $0.name == config.runtimeHandler }) != nil else { + throw ContainerizationError( + .internalError, + message: "failed to find runtime plugin \(config.runtimeHandler)" + ) + } let state = ContainerState( snapshot: .init( configuration: config, @@ -141,23 +168,29 @@ public actor ContainersService { ), ) results[config.id] = state - guard runtimePlugins.first(where: { $0.name == config.runtimeHandler }) != nil else { - throw ContainerizationError( - .internalError, - message: "failed to find runtime plugin \(config.runtimeHandler)" - ) - } } catch { - try? FileManager.default.removeItem(at: dir) + let loadError = error + do { + try FileManager.default.removeItem(at: dir) + } catch { + reservations.formUnion(candidateIDs) + log.error( + "failed to remove invalid container bundle; reserving container ID", + metadata: [ + "path": "\(dir.path)", + "loadError": "\(loadError)", + "cleanupError": "\(error)", + ]) + } log.warning( "failed to load container", metadata: [ "path": "\(dir.path)", - "error": "\(error)", + "error": "\(loadError)", ]) } } - return results + return (results, reservations) } /// List containers matching the given filters. @@ -264,7 +297,13 @@ public actor ContainersService { } /// Create a new container from the provided id and configuration. - public func create(configuration: ContainerConfiguration, kernel: Kernel, options: ContainerCreateOptions, initImage: String? = nil, runtimeData: Data? = nil) async throws { + public func create( + configuration: ContainerConfiguration, + kernel: Kernel, + options: ContainerCreateOptions, + initImage: String? = nil, + runtimeData: Data? = nil + ) async throws -> ContainerCreateResult { log.debug( "ContainersService: enter", metadata: [ @@ -282,14 +321,19 @@ public actor ContainersService { ) } - try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(configuration.id)"]) { context in - guard await self.containers[configuration.id] == nil else { + return try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(configuration.id)"]) { context in + guard await self.canCreate(id: configuration.id, context: context) else { throw ContainerizationError( .exists, message: "container already exists: \(configuration.id)" ) } + // Assign the identity while holding the same lifecycle lock that + // authorizes and persists the create. The returned result is built + // from this exact authoritative configuration, never caller input. + let (configuration, result) = Self.assignInstanceIdentity(to: configuration) + var allHostnames = Set() for container in await self.containers.values { for attachmentConfiguration in container.snapshot.configuration.networks { @@ -384,6 +428,7 @@ public actor ContainersService { } catch { throw error } + return result } } @@ -408,6 +453,7 @@ public actor ContainersService { } try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { context in + try await self.ensureNotDeleting(id: id, context: context) var state = try await self.getContainerState(id: id, context: context) // We've already bootstrapped this container. Ideally we should be able to @@ -444,10 +490,14 @@ public actor ContainersService { ) try await runtimeClient.bootstrap(stdio: stdio, networkBootstrapInfos: networkBootstrapInfos, dynamicEnv: dynamicEnv) - try await self.exitMonitor.registerProcess( - id: id, - onExit: self.handleContainerExit - ) + let instanceToken = state.snapshot.configuration.instanceToken + try await self.exitMonitor.registerProcess(id: id) { id, exitStatus in + try await self.handleContainerExit( + id: id, + expectedInstanceToken: instanceToken, + code: exitStatus + ) + } state.client = runtimeClient await self.setContainerState(id, state, context: context) @@ -523,6 +573,7 @@ public actor ContainersService { } try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)", "processId": "\(processID)"]) { context in + try await self.ensureNotDeleting(id: id, context: context) var state = try await self.getContainerState(id: id, context: context) let isInit = Self.isInitProcess(id: id, processID: processID) @@ -596,7 +647,10 @@ public actor ContainersService { // container's init process, follow up with the same API-server cleanup // that `stop` performs. if processID == id, (try? Signal(signal)) == .kill { - try await handleContainerExit(id: id) + try await handleContainerExit( + id: id, + expectedInstanceToken: state.snapshot.configuration.instanceToken + ) } } @@ -642,7 +696,10 @@ public actor ContainersService { throw err } } - try await handleContainerExit(id: id) + try await handleContainerExit( + id: id, + expectedInstanceToken: state.snapshot.configuration.instanceToken + ) } public func dial(id: String, port: UInt32) async throws -> FileHandle { @@ -812,6 +869,15 @@ public actor ContainersService { /// Delete a container and its resources. public func delete(id: String, force: Bool) async throws { + try await delete(id: id, force: force, expectedInstanceToken: nil) + } + + /// Delete a container only when its current instance identity matches. + public func deleteIfInstance(id: String, force: Bool, expectedInstanceToken: String) async throws { + try await delete(id: id, force: force, expectedInstanceToken: expectedInstanceToken) + } + + private func delete(id: String, force: Bool, expectedInstanceToken: String?) async throws { log.info( "ContainersService: enter", metadata: [ @@ -830,15 +896,20 @@ public actor ContainersService { ) } - let state = try self._getContainerState(id: id) - switch state.snapshot.status { - case .running: - if !force { - throw ContainerizationError( - .invalidState, - message: "container \(id) is \(state.snapshot.status) and can not be deleted" - ) - } + let state = try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { context in + try await self.selectForDelete( + id: id, + force: force, + expectedInstanceToken: expectedInstanceToken, + context: context + ) + } + guard let state else { + return + } + let selectedInstanceToken = state.snapshot.configuration.instanceToken + + do { let opts = ContainerStopOptions( timeoutInSeconds: 5, signal: "SIGKILL" @@ -846,31 +917,17 @@ public actor ContainersService { let client = try state.getClient() try await client.stop(options: opts) try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { context in - self.log.info( - "ContainersService: attempt cleanup", - metadata: [ - "func": "\(#function)", - "id": "\(id)", - ] - ) - try await self.cleanUp(id: id, context: context) - self.log.info( - "ContainersService: successful cleanup", - metadata: [ - "func": "\(#function)", - "id": "\(id)", - ] + try await self.finishDelete( + id: id, + selected: selectedInstanceToken, + context: context ) } - case .stopping: - throw ContainerizationError( - .invalidState, - message: "container \(id) is \(state.snapshot.status) and can not be deleted" - ) - default: - try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { context in - try await self.cleanUp(id: id, context: context) + } catch { + await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { context in + await self.cancelDelete(id: id, context: context) } + throw error } } @@ -931,13 +988,34 @@ public actor ContainersService { try await client.clean(id: id) } - private func handleContainerExit(id: String, code: ExitStatus? = nil) async throws { + func handleContainerExit( + id: String, + expectedInstanceToken: String?, + code: ExitStatus? = nil + ) async throws { try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { [self] context in - try await handleContainerExit(id: id, code: code, context: context) + try await handleContainerExit( + id: id, + expectedInstanceToken: expectedInstanceToken, + code: code, + context: context + ) } } - private func handleContainerExit(id: String, code: ExitStatus?, context: AsyncLock.Context) async throws { + private func handleContainerExit( + id: String, + expectedInstanceToken: String?, + code: ExitStatus?, + context: AsyncLock.Context + ) async throws { + guard var state = self.containers[id] else { + return + } + guard state.snapshot.configuration.instanceToken == expectedInstanceToken else { + return + } + if let code { self.log.info( "handling container exit", @@ -947,14 +1025,7 @@ public actor ContainersService { ]) } - var state: ContainerState - do { - state = try self.getContainerState(id: id, context: context) - if state.snapshot.status == .stopped { - return - } - } catch { - // Was auto removed by the background thread, nothing for us to do. + if state.snapshot.status == .stopped { return } @@ -1072,18 +1143,10 @@ public actor ContainersService { try? ServiceManager.deregister(fullServiceLabel: label) } - // Always try to delete the bundle directory, even if it's incomplete - do { - try bundle.delete() - } catch { - self.log.warning( - "failed to delete bundle for container", - metadata: [ - "id": "\(id)", - "error": "\(error)", - ]) - } - + // Bundle removal is the cleanup commit point. Retain the in-memory + // identity if it fails so that the same ID cannot be reused while + // resources from this instance remain on disk. + try bundle.delete() self.containers.removeValue(forKey: id) } @@ -1131,6 +1194,129 @@ public actor ContainersService { self.containers[id] = state } + #if DEBUG + func setContainerStatusForTesting(id: String, status: RuntimeStatus) throws { + guard var state = self.containers[id] else { + throw ContainerizationError(.notFound, message: "container \(id) not found") + } + state.snapshot.status = status + self.containers[id] = state + } + + func reserveContainerForForceDeletionForTesting(id: String) async throws { + try await self.lock.withLock(logMetadata: ["acquirer": "\(#function)", "id": "\(id)"]) { context in + try await self.reserveContainerForForceDeletionForTesting(id: id, context: context) + } + } + + private func reserveContainerForForceDeletionForTesting( + id: String, + context: AsyncLock.Context + ) throws { + _ = try self.getContainerState(id: id, context: context) + self.deletions.insert(id) + } + #endif + + static func assignInstanceIdentity( + to callerConfiguration: ContainerConfiguration + ) -> (configuration: ContainerConfiguration, result: ContainerCreateResult) { + var configuration = callerConfiguration + let instanceToken = ManagedContainer.generateInstanceToken() + configuration.instanceToken = instanceToken + return ( + configuration, + ContainerCreateResult(id: configuration.id, instanceToken: instanceToken) + ) + } + + private func canCreate(id: String, context: AsyncLock.Context) -> Bool { + self.containers[id] == nil && !self.deletions.contains(id) + } + + private func ensureNotDeleting(id: String, context: AsyncLock.Context) throws { + guard !self.deletions.contains(id) else { + throw ContainerizationError( + .invalidState, + message: "container \(id) is being deleted" + ) + } + } + + private func selectForDelete( + id: String, + force: Bool, + expectedInstanceToken: String?, + context: AsyncLock.Context + ) async throws -> ContainerState? { + guard !self.deletions.contains(id) else { + throw ContainerizationError( + .invalidState, + message: "container \(id) is already being deleted" + ) + } + + let state = try self.getContainerState(id: id, context: context) + try Self.validateInstanceToken( + expected: expectedInstanceToken, + current: state.snapshot.configuration.instanceToken, + id: id + ) + + switch state.snapshot.status { + case .running: + guard force else { + throw ContainerizationError( + .invalidState, + message: "container \(id) is \(state.snapshot.status) and can not be deleted" + ) + } + self.deletions.insert(id) + return state + case .stopping: + throw ContainerizationError( + .invalidState, + message: "container \(id) is \(state.snapshot.status) and can not be deleted" + ) + default: + try await self.cleanUp(id: id, context: context) + return nil + } + } + + private func finishDelete(id: String, selected: String?, context: AsyncLock.Context) async throws { + defer { + self.deletions.remove(id) + } + guard let currentState = self.containers[id] else { + return + } + try Self.validateSelectedInstance( + selected: selected, + current: currentState.snapshot.configuration.instanceToken, + id: id + ) + self.log.info( + "ContainersService: attempt cleanup", + metadata: [ + "func": "\(#function)", + "id": "\(id)", + ] + ) + try await self.cleanUp(id: id, context: context) + self.log.info( + "ContainersService: successful cleanup", + metadata: [ + "func": "\(#function)", + "id": "\(id)", + ] + ) + } + + private func cancelDelete(id: String, context: AsyncLock.Context) { + self.deletions.remove(id) + } + private func getContainerState(id: String, context: AsyncLock.Context) throws -> ContainerState { try self._getContainerState(id: id) } @@ -1150,6 +1336,27 @@ public actor ContainersService { id == processID } + private static func validateInstanceToken(expected: String?, current: String?, id: String) throws { + guard let expected else { + return + } + guard let current, current == expected else { + throw ContainerizationError( + .invalidState, + message: "container instance precondition failed for \(id)" + ) + } + } + + private static func validateSelectedInstance(selected: String?, current: String?, id: String) throws { + guard selected == current else { + throw ContainerizationError( + .invalidState, + message: "container instance changed while deleting \(id)" + ) + } + } + /// Get container configuration, either from existing bundle or from RuntimeConfiguration private static func getContainerConfiguration(at path: URL) throws -> (ContainerConfiguration, ContainerCreateOptions?) { let bundle = ContainerResource.Bundle(path: path) diff --git a/Sources/Services/ContainerAPIService/Server/HealthCheck/HealthCheckHarness.swift b/Sources/Services/ContainerAPIService/Server/HealthCheck/HealthCheckHarness.swift index 82907c6b9..668727f40 100644 --- a/Sources/Services/ContainerAPIService/Server/HealthCheck/HealthCheckHarness.swift +++ b/Sources/Services/ContainerAPIService/Server/HealthCheck/HealthCheckHarness.swift @@ -49,6 +49,8 @@ public actor HealthCheckHarness { // Extra optional fields for richer client display reply.set(key: .apiServerBuild, value: ReleaseVersion.buildType()) reply.set(key: .apiServerAppName, value: "container-apiserver") + reply.set(key: .conditionalContainerDeleteSupported, value: true) + reply.set(key: .containerCreateResultSupported, value: true) return reply } } diff --git a/Sources/Services/Runtime/RuntimeClient/ExitMonitor.swift b/Sources/Services/Runtime/RuntimeClient/ExitMonitor.swift index 6ac2fa9e6..cc9d72a79 100644 --- a/Sources/Services/Runtime/RuntimeClient/ExitMonitor.swift +++ b/Sources/Services/Runtime/RuntimeClient/ExitMonitor.swift @@ -36,8 +36,18 @@ public actor ExitMonitor { self.log = log } - private var exitCallbacks: [String: ExitCallback] = [:] - private var runningTasks: [String: Task] = [:] + private struct CallbackRegistration { + let generation: UUID + let callback: ExitCallback + } + + private struct RunningTask { + let generation: UUID + let task: Task + } + + private var exitCallbacks: [String: CallbackRegistration] = [:] + private var runningTasks: [String: RunningTask] = [:] private let log: Logger? /// Remove tracked work from the monitor. @@ -45,11 +55,10 @@ public actor ExitMonitor { /// - Parameters: /// - id: The client identifier for the tracked work. public func stopTracking(id: String) async { - if let task = self.runningTasks[id] { - task.cancel() + if let runningTask = self.runningTasks.removeValue(forKey: id) { + runningTask.task.cancel() } exitCallbacks.removeValue(forKey: id) - runningTasks.removeValue(forKey: id) } /// Register long running work so that the monitor invokes @@ -62,7 +71,10 @@ public actor ExitMonitor { guard self.exitCallbacks[id] == nil else { throw ContainerizationError(.invalidState, message: "ExitMonitor already setup for process \(id)") } - self.exitCallbacks[id] = onExit + self.exitCallbacks[id] = CallbackRegistration( + generation: UUID(), + callback: onExit + ) } /// Await the completion of previously registered item of work. @@ -72,20 +84,61 @@ public actor ExitMonitor { /// - waitingOn: A function that waits for the work to complete, /// and then returns an exit code. public func track(id: String, waitingOn: @escaping WaitHandler) async throws { - guard let onExit = self.exitCallbacks[id] else { + guard let registration = self.exitCallbacks[id] else { throw ContainerizationError(.invalidState, message: "ExitMonitor not setup for process \(id)") } guard self.runningTasks[id] == nil else { throw ContainerizationError(.invalidState, message: "already have a running task tracking process \(id)") } - self.runningTasks[id] = Task { + + let generation = registration.generation + let task = Task { + let exitStatus: ExitStatus do { - let exitStatus = try await waitingOn() - try await onExit(id, exitStatus) + exitStatus = try await waitingOn() } catch { + guard !Task.isCancelled else { + self.discardRegistration(id: id, generation: generation) + return + } self.log?.error("WaitHandler for \(id) threw error \(String(describing: error))") - try? await onExit(id, ExitStatus(exitCode: -1)) + exitStatus = ExitStatus(exitCode: -1) + } + + guard !Task.isCancelled else { + self.discardRegistration(id: id, generation: generation) + return + } + guard let onExit = self.callback(id: id, generation: generation) else { + return + } + defer { + self.discardRegistration(id: id, generation: generation) + } + + do { + try await onExit(id, exitStatus) + } catch { + self.log?.error("Exit callback for \(id) threw error \(String(describing: error))") } } + self.runningTasks[id] = RunningTask(generation: generation, task: task) + } + + private func callback(id: String, generation: UUID) -> ExitCallback? { + guard let registration = self.exitCallbacks[id], registration.generation == generation else { + return nil + } + return registration.callback + } + + private func discardRegistration(id: String, generation: UUID) { + guard self.exitCallbacks[id]?.generation == generation else { + return + } + self.exitCallbacks.removeValue(forKey: id) + if self.runningTasks[id]?.generation == generation { + self.runningTasks.removeValue(forKey: id) + } } } diff --git a/Tests/ContainerAPIClientTests/ContainerCreateResultTests.swift b/Tests/ContainerAPIClientTests/ContainerCreateResultTests.swift new file mode 100644 index 000000000..723d36dea --- /dev/null +++ b/Tests/ContainerAPIClientTests/ContainerCreateResultTests.swift @@ -0,0 +1,79 @@ +//===----------------------------------------------------------------------===// +// 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 Containerization +import ContainerizationError +import Foundation +import Testing + +@testable import ContainerAPIClient + +struct ContainerCreateResultTests { + @Test func legacyAndResultCreateSignaturesRemainDistinct() { + let client = ContainerClient() + let legacyCreate: (ContainerConfiguration, ContainerCreateOptions, Kernel, String?, Data?) async throws -> Void = client.create + let resultCreate: (ContainerConfiguration, ContainerCreateOptions, Kernel, String?, Data?) async throws -> ContainerCreateResult = + client.createWithResult + + _ = legacyCreate + _ = resultCreate + } + + @Test func legacyAndConditionalDeleteSignaturesRemainDistinct() { + let client = ContainerClient() + let legacyDelete: (String, Bool) async throws -> Void = client.delete + let conditionalDelete: (String, Bool, String) async throws -> Void = client.deleteIfInstance + + _ = legacyDelete + _ = conditionalDelete + } + + @Test func conditionalDeletePreservesTypedErrors() async { + let unsupported = await #expect(throws: ContainerizationError.self) { + try await ContainerClient.withConditionalDeleteErrorPreservation { + throw ContainerizationError(.unsupported, message: "unsupported conditional delete") + } + } + #expect(unsupported?.code == .unsupported) + + let unknown = await #expect(throws: ContainerizationError.self) { + try await ContainerClient.withConditionalDeleteErrorPreservation { + throw NSError(domain: "ContainerCreateResultTests", code: 1) + } + } + #expect(unknown?.code == .internalError) + } + + @Test func decodesAtomicCreateResult() throws { + let expected = ContainerCreateResult(id: "created", instanceToken: "server-token") + let data = try JSONEncoder().encode(expected) + + #expect(try ContainerClient.decodeCreateResult(data) == expected) + } + + @Test func missingResultFromLegacyServerRemainsCompatible() throws { + #expect(try ContainerClient.decodeCreateResult(nil) == nil) + } + + @Test func ignoresFutureResultFields() throws { + let data = Data(#"{"id":"created","instanceToken":"server-token","future":true}"#.utf8) + let result = try #require(try ContainerClient.decodeCreateResult(data)) + + #expect(result.id == "created") + #expect(result.instanceToken == "server-token") + } +} diff --git a/Tests/ContainerAPIServiceTests/ContainerInstanceTokenTests.swift b/Tests/ContainerAPIServiceTests/ContainerInstanceTokenTests.swift new file mode 100644 index 000000000..856be526a --- /dev/null +++ b/Tests/ContainerAPIServiceTests/ContainerInstanceTokenTests.swift @@ -0,0 +1,372 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerPersistence +import ContainerResource +import ContainerTestSupport +import ContainerXPC +import Containerization +import ContainerizationError +import Foundation +import Logging +import SystemPackage +import Testing + +@testable import ContainerAPIService +@testable import ContainerPlugin + +private struct InstanceTokenTestPluginFactory: PluginFactory { + let plugin: Plugin + + func create(installURL: URL) throws -> Plugin? { + plugin + } + + func create(parentURL: URL, name: String) throws -> Plugin? { + try create(installURL: parentURL.appendingPathComponent(name)) + } +} + +struct ContainerInstanceTokenTests { + private let log = Logger(label: "container-instance-token-tests") + + @Test func createResultUsesServerGeneratedIdentity() { + var callerConfiguration = makeConfiguration(id: "created", token: "caller-token") + let first = ContainersService.assignInstanceIdentity(to: callerConfiguration) + let second = ContainersService.assignInstanceIdentity(to: callerConfiguration) + + #expect(first.configuration.instanceToken == first.result.instanceToken) + #expect(first.result.id == callerConfiguration.id) + #expect(first.result.instanceToken != "caller-token") + #expect(second.result.instanceToken != first.result.instanceToken) + + callerConfiguration.instanceToken = first.result.instanceToken + let replacement = ContainersService.assignInstanceIdentity(to: callerConfiguration) + #expect(replacement.result.instanceToken != first.result.instanceToken) + } + + @Test func survivesServiceReconstruction() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let token = "persisted-instance-token" + try writeContainer(id: "restart-test", token: token, appRoot: appRoot) + + let firstService = try makeService(appRoot: appRoot) + let firstSnapshots = try await firstService.list() + let first = try #require(firstSnapshots.first) + #expect(first.configuration.instanceToken == token) + + let restartedService = try makeService(appRoot: appRoot) + let restartedSnapshots = try await restartedService.list() + let restarted = try #require(restartedSnapshots.first) + #expect(restarted.configuration.instanceToken == token) + } + } + + @Test func conditionalDeleteRejectsMismatchWithoutDeleting() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "mismatch-test" + try writeContainer(id: id, token: "current-token", appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + + let error = await #expect(throws: ContainerizationError.self) { + try await service.deleteIfInstance(id: id, force: false, expectedInstanceToken: "stale-token") + } + + #expect(error?.code == .invalidState) + let snapshots = try await service.list() + #expect(snapshots.map(\.id) == [id]) + } + } + + @Test func staleTokenCannotDeleteReplacementWithReusedID() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "reused-id" + let firstToken = "first-incarnation-token" + let firstBundlePath = try writeContainer(id: id, token: firstToken, appRoot: appRoot) + let firstService = try makeService(appRoot: appRoot) + + try FileManager.default.removeItem(at: firstBundlePath.appendingPathComponent("config.json")) + try await firstService.deleteIfInstance(id: id, force: false, expectedInstanceToken: firstToken) + + let replacementToken = "replacement-incarnation-token" + try writeContainer(id: id, token: replacementToken, appRoot: appRoot) + let replacementService = try makeService(appRoot: appRoot) + + let error = await #expect(throws: ContainerizationError.self) { + try await replacementService.deleteIfInstance(id: id, force: false, expectedInstanceToken: firstToken) + } + + #expect(error?.code == .invalidState) + let snapshots = try await replacementService.list() + #expect(snapshots.first?.configuration.instanceToken == replacementToken) + } + } + + @Test func conditionalDeleteRejectsTokenFromAnotherContainer() async throws { + try await TemporaryStorage.withTempDir { appRoot in + try writeContainer(id: "first", token: "first-token", appRoot: appRoot) + try writeContainer(id: "second", token: "second-token", appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + + let error = await #expect(throws: ContainerizationError.self) { + try await service.deleteIfInstance(id: "first", force: false, expectedInstanceToken: "second-token") + } + + #expect(error?.code == .invalidState) + let snapshots = try await service.list() + #expect(Set(snapshots.map(\.id)) == Set(["first", "second"])) + } + } + + @Test func conditionalDeleteWithMatchingTokenSucceeds() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "matching-test" + let token = "matching-token" + let bundlePath = try writeContainer(id: id, token: token, appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + + // Avoid touching launchd in this focused unit test. The in-memory + // snapshot remains authoritative for the instance check. + try FileManager.default.removeItem(at: bundlePath.appendingPathComponent("config.json")) + try await service.deleteIfInstance(id: id, force: false, expectedInstanceToken: token) + + let snapshots = try await service.list() + #expect(snapshots.isEmpty) + #expect(!FileManager.default.fileExists(atPath: bundlePath.path)) + } + } + + @Test func legacyDeleteRemainsCompatibleAndConditionalDeleteFailsClosed() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "legacy-test" + let bundlePath = try writeContainer(id: id, token: nil, appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + + let error = await #expect(throws: ContainerizationError.self) { + try await service.deleteIfInstance(id: id, force: false, expectedInstanceToken: "caller-token") + } + #expect(error?.code == .invalidState) + let preservedSnapshots = try await service.list() + #expect(preservedSnapshots.count == 1) + + try FileManager.default.removeItem(at: bundlePath.appendingPathComponent("config.json")) + try await service.delete(id: id, force: false) + let deletedSnapshots = try await service.list() + #expect(deletedSnapshots.isEmpty) + } + } + + @Test func conditionalDeleteRouteRejectsMissingToken() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "missing-token-test" + try writeContainer(id: id, token: "current-token", appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + let harness = ContainersHarness(service: service, log: log) + let request = XPCMessage(route: .containerDeleteIfInstance) + request.set(key: .id, value: id) + + let error = await #expect(throws: ContainerizationError.self) { + _ = try await harness.deleteIfInstance(request) + } + + #expect(error?.code == .invalidArgument) + let snapshots = try await service.list() + #expect(snapshots.map(\.id) == [id]) + } + } + + @Test func delayedExitFromForceDeletedInstanceDoesNotMutateReplacementAcrossRestart() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "delayed-exit-reuse" + let oldToken = "old-instance-token" + let oldBundlePath = try writeContainer(id: id, token: oldToken, appRoot: appRoot) + let oldService = try makeService(appRoot: appRoot) + + // Exercise force-request cleanup without touching launchd, then + // reconstruct the service around an immediate same-ID replacement. + try FileManager.default.removeItem(at: oldBundlePath.appendingPathComponent("config.json")) + try await oldService.deleteIfInstance(id: id, force: true, expectedInstanceToken: oldToken) + + let replacementToken = "replacement-instance-token" + let replacementBundlePath = try writeContainer(id: id, token: replacementToken, appRoot: appRoot) + let restartedService = try makeService(appRoot: appRoot) + try await restartedService.setContainerStatusForTesting(id: id, status: .running) + + try await restartedService.handleContainerExit( + id: id, + expectedInstanceToken: oldToken, + code: ExitStatus(exitCode: 0) + ) + + let snapshots = try await restartedService.list() + let replacement = try #require(snapshots.first) + #expect(replacement.configuration.instanceToken == replacementToken) + #expect(replacement.status == .running) + #expect(FileManager.default.fileExists(atPath: replacementBundlePath.path)) + } + } + + @Test func lifecycleStartAndBootstrapRejectForceDeletionReservation() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "force-delete-reservation" + try writeContainer(id: id, token: "reserved-token", appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + try await service.setContainerStatusForTesting(id: id, status: .running) + try await service.reserveContainerForForceDeletionForTesting(id: id) + + let bootstrapError = await #expect(throws: ContainerizationError.self) { + try await service.bootstrap(id: id, stdio: [nil, nil, nil], dynamicEnv: [:]) + } + #expect(bootstrapError?.code == .invalidState) + + let startError = await #expect(throws: ContainerizationError.self) { + try await service.startProcess(id: id, processID: id) + } + #expect(startError?.code == .invalidState) + } + } + + @Test func failedBundleRemovalRetainsIdentityAndRestartReservation() async throws { + try await TemporaryStorage.withTempDir { appRoot in + let id = "cleanup-failure-reservation" + let token = "preserved-instance-token" + let bundlePath = try writeContainer(id: id, token: token, appRoot: appRoot) + let service = try makeService(appRoot: appRoot) + let containerRoot = bundlePath.deletingLastPathComponent() + + // Make the residual invalid for restart so recovery cannot treat it + // as a healthy container, then prevent either cleanup path from + // unlinking the bundle directory. + try FileManager.default.removeItem(at: bundlePath.appendingPathComponent("config.json")) + try FileManager.default.setAttributes( + [.posixPermissions: 0o500], + ofItemAtPath: containerRoot.path + ) + defer { + try? FileManager.default.setAttributes( + [.posixPermissions: 0o700], + ofItemAtPath: containerRoot.path + ) + } + + do { + try await service.deleteIfInstance(id: id, force: false, expectedInstanceToken: token) + Issue.record("expected bundle removal to fail") + } catch {} + + let preserved = try await service.list() + #expect(preserved.first?.configuration.instanceToken == token) + + let createError = await #expect(throws: ContainerizationError.self) { + _ = try await service.create( + configuration: makeConfiguration(id: id, token: nil), + kernel: Kernel(path: URL(fileURLWithPath: "/nonexistent"), platform: .linuxArm), + options: .default + ) + } + #expect(createError?.code == .exists) + + let restartedService = try makeService(appRoot: appRoot) + #expect(try await restartedService.list().isEmpty) + + let restartedCreateError = await #expect(throws: ContainerizationError.self) { + _ = try await restartedService.create( + configuration: makeConfiguration(id: id, token: nil), + kernel: Kernel(path: URL(fileURLWithPath: "/nonexistent"), platform: .linuxArm), + options: .default + ) + } + #expect(restartedCreateError?.code == .exists) + + let staleDeleteError = await #expect(throws: ContainerizationError.self) { + try await restartedService.deleteIfInstance( + id: id, + force: false, + expectedInstanceToken: "stale-token" + ) + } + #expect(staleDeleteError?.code == .invalidState) + #expect(FileManager.default.fileExists(atPath: bundlePath.path)) + } + } + + private func makeService(appRoot: FilePath) throws -> ContainersService { + let appRootURL = URL(fileURLWithPath: appRoot.string) + let pluginDirectory = appRootURL.appendingPathComponent("plugins") + let installURL = pluginDirectory.appendingPathComponent("container-runtime-linux") + try FileManager.default.createDirectory(at: installURL, withIntermediateDirectories: true) + + let servicesConfig = PluginConfig.ServicesConfig( + loadAtBoot: false, + runAtLoad: false, + services: [.init(type: .runtime, description: nil)], + defaultArguments: [] + ) + let plugin = Plugin( + binaryURL: URL(fileURLWithPath: "/bin/container-runtime-linux"), + config: PluginConfig(abstract: "test runtime", author: nil, servicesConfig: servicesConfig) + ) + let loader = try PluginLoader( + appRoot: appRootURL, + installRoot: appRootURL, + logRoot: nil, + pluginDirectories: [pluginDirectory], + pluginFactories: [InstanceTokenTestPluginFactory(plugin: plugin)] + ) + return try ContainersService( + appRoot: appRootURL, + pluginLoader: loader, + containerSystemConfig: ContainerSystemConfig(), + log: log + ) + } + + @discardableResult + private func writeContainer(id: String, token: String?, appRoot: FilePath) throws -> URL { + let configuration = makeConfiguration(id: id, token: token) + + let bundlePath = URL(fileURLWithPath: appRoot.string) + .appendingPathComponent("containers") + .appendingPathComponent(id) + try FileManager.default.createDirectory(at: bundlePath, withIntermediateDirectories: true) + try Bundle(path: bundlePath).set(configuration: configuration) + return bundlePath + } + + private func makeConfiguration(id: String, token: String?) -> ContainerConfiguration { + let image = ImageDescription( + reference: "docker.io/library/alpine:latest", + descriptor: .init( + mediaType: "application/vnd.oci.image.manifest.v1+json", + digest: "sha256:" + String(repeating: "0", count: 64), + size: 0 + ) + ) + let process = ProcessConfiguration( + executable: "/bin/sh", + arguments: [], + environment: [], + workingDirectory: "/", + terminal: false, + user: .id(uid: 0, gid: 0), + supplementalGroups: [], + rlimits: [] + ) + var configuration = ContainerConfiguration(id: id, image: image, process: process) + configuration.instanceToken = token + return configuration + } +} diff --git a/Tests/ContainerAPIServiceTests/ExitMonitorTests.swift b/Tests/ContainerAPIServiceTests/ExitMonitorTests.swift new file mode 100644 index 000000000..582259687 --- /dev/null +++ b/Tests/ContainerAPIServiceTests/ExitMonitorTests.swift @@ -0,0 +1,165 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerRuntimeClient +import Containerization +import ContainerizationError +import Foundation +import Testing + +private actor CallbackRecorder { + private var count = 0 + private var waiters: [CheckedContinuation] = [] + + func record() { + count += 1 + let waiters = self.waiters + self.waiters.removeAll() + waiters.forEach { $0.resume() } + } + + func invocationCount() -> Int { + count + } + + func waitForInvocation() async { + guard count == 0 else { + return + } + await withCheckedContinuation { continuation in + waiters.append(continuation) + } + } +} + +private actor WaitGate { + private var continuation: CheckedContinuation? + + var isWaiting: Bool { + continuation != nil + } + + func wait() async -> Containerization.ExitStatus { + await withCheckedContinuation { continuation in + self.continuation = continuation + } + } + + func release(_ status: Containerization.ExitStatus) { + continuation?.resume(returning: status) + continuation = nil + } +} + +struct ExitMonitorTests { + @Test func intentionalCancellationDoesNotInvokeExitCallback() async throws { + let monitor = ExitMonitor() + let recorder = CallbackRecorder() + + try await monitor.registerProcess(id: "cancelled") { _, _ in + await recorder.record() + } + try await monitor.track(id: "cancelled") { + try await Task.sleep(for: .seconds(60)) + return Containerization.ExitStatus(exitCode: 0) + } + + await monitor.stopTracking(id: "cancelled") + try await Task.sleep(for: .milliseconds(50)) + + #expect(await recorder.invocationCount() == 0) + } + + @Test func cancelledOldGenerationCannotInvokeAfterSameIDRegistration() async throws { + let monitor = ExitMonitor() + let oldRecorder = CallbackRecorder() + let replacementRecorder = CallbackRecorder() + let gate = WaitGate() + + try await monitor.registerProcess(id: "reused") { _, _ in + await oldRecorder.record() + } + try await monitor.track(id: "reused") { + await gate.wait() + } + while !(await gate.isWaiting) { + await Task.yield() + } + + await monitor.stopTracking(id: "reused") + try await monitor.registerProcess(id: "reused") { _, _ in + await replacementRecorder.record() + } + await gate.release(Containerization.ExitStatus(exitCode: 0)) + try await Task.sleep(for: .milliseconds(50)) + + #expect(await oldRecorder.invocationCount() == 0) + #expect(await replacementRecorder.invocationCount() == 0) + } + + @Test func callbackRetainsRegistrationUntilItCompletes() async throws { + let monitor = ExitMonitor() + let callbackStarted = CallbackRecorder() + let callbackGate = WaitGate() + + try await monitor.registerProcess(id: "callback-in-progress") { _, _ in + await callbackStarted.record() + _ = await callbackGate.wait() + } + try await monitor.track(id: "callback-in-progress") { + Containerization.ExitStatus(exitCode: 0) + } + await callbackStarted.waitForInvocation() + + await #expect(throws: ContainerizationError.self) { + try await monitor.registerProcess(id: "callback-in-progress") { _, _ in } + } + + await callbackGate.release(Containerization.ExitStatus(exitCode: 0)) + } + + @Test func waitFailureStillInvokesExitCallbackOnce() async throws { + struct WaitFailure: Error {} + + let monitor = ExitMonitor() + let recorder = CallbackRecorder() + + try await monitor.registerProcess(id: "failed-wait") { _, _ in + await recorder.record() + } + try await monitor.track(id: "failed-wait") { + throw WaitFailure() + } + await recorder.waitForInvocation() + + #expect(await recorder.invocationCount() == 1) + } + + @Test func uncancelledCancellationErrorStillInvokesExitCallbackOnce() async throws { + let monitor = ExitMonitor() + let recorder = CallbackRecorder() + + try await monitor.registerProcess(id: "failed-wait-cancellation-error") { _, _ in + await recorder.record() + } + try await monitor.track(id: "failed-wait-cancellation-error") { + throw CancellationError() + } + await recorder.waitForInvocation() + + #expect(await recorder.invocationCount() == 1) + } +} diff --git a/Tests/ContainerCommandsTests/ContainerCreateTests.swift b/Tests/ContainerCommandsTests/ContainerCreateTests.swift new file mode 100644 index 000000000..52ba45942 --- /dev/null +++ b/Tests/ContainerCommandsTests/ContainerCreateTests.swift @@ -0,0 +1,40 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerResource +import Foundation +import Testing + +@testable import ContainerCommands + +struct ContainerCreateTests { + @Test func acceptsJSONOutputFormat() throws { + let command = try Application.ContainerCreate.parse([ + "--format", "json", "docker.io/library/alpine:latest", + ]) + + #expect(command.format == .json) + } + + @Test func createResultRendersAsMachineReadableJSON() throws { + let expected = ContainerCreateResult(id: "created", instanceToken: "server-token") + let rendered = try Output.renderJSON(expected) + let decoded = try JSONDecoder().decode(ContainerCreateResult.self, from: Data(rendered.utf8)) + + #expect(decoded == expected) + } +} diff --git a/Tests/ContainerCommandsTests/ContainerDeleteTests.swift b/Tests/ContainerCommandsTests/ContainerDeleteTests.swift new file mode 100644 index 000000000..c9e3fafa7 --- /dev/null +++ b/Tests/ContainerCommandsTests/ContainerDeleteTests.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 ArgumentParser +import Testing + +@testable import ContainerCommands + +struct ContainerDeleteTests { + @Test func instanceTokenRequiresOneExplicitContainer() throws { + #expect(throws: (any Error).self) { + _ = try Application.ContainerDelete.parse([ + "--if-instance-token", "opaque-token", "--all", + ]) + } + + #expect(throws: (any Error).self) { + _ = try Application.ContainerDelete.parse([ + "--if-instance-token", "opaque-token", "first", "second", + ]) + } + } + + @Test func instanceTokenAcceptsOneExplicitContainer() throws { + let command = try Application.ContainerDelete.parse([ + "--force", "--if-instance-token", "opaque-token", "target", + ]) + + try command.validate() + #expect(command.force) + #expect(command.ifInstanceToken == "opaque-token") + #expect(command.containerIds == ["target"]) + } +} diff --git a/Tests/ContainerResourceTests/ContainerConfigurationTests.swift b/Tests/ContainerResourceTests/ContainerConfigurationTests.swift index b1aafa0b0..5293a72b8 100644 --- a/Tests/ContainerResourceTests/ContainerConfigurationTests.swift +++ b/Tests/ContainerResourceTests/ContainerConfigurationTests.swift @@ -92,3 +92,28 @@ struct ContainerConfigurationCreationDateTests { #expect(decoded.creationDate == Date(timeIntervalSince1970: 0)) } } + +struct ContainerConfigurationInstanceTokenTests { + @Test func roundTripsInstanceToken() throws { + var config = makeTestConfiguration() + config.instanceToken = "opaque-instance-token" + + let data = try JSONEncoder().encode(config) + let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: data) + + #expect(decoded.instanceToken == "opaque-instance-token") + } + + @Test func decodesMissingInstanceTokenAsNil() throws { + var config = makeTestConfiguration() + config.instanceToken = "opaque-instance-token" + let data = try JSONEncoder().encode(config) + var obj = try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any]) + obj.removeValue(forKey: "instanceToken") + + let stripped = try JSONSerialization.data(withJSONObject: obj) + let decoded = try JSONDecoder().decode(ContainerConfiguration.self, from: stripped) + + #expect(decoded.instanceToken == nil) + } +} diff --git a/Tests/ContainerResourceTests/ManagedContainerTests.swift b/Tests/ContainerResourceTests/ManagedContainerTests.swift index 496880fb3..a5bdc24de 100644 --- a/Tests/ContainerResourceTests/ManagedContainerTests.swift +++ b/Tests/ContainerResourceTests/ManagedContainerTests.swift @@ -74,6 +74,15 @@ struct ManagedContainerTests { #expect(UUID(uuidString: id) != nil) } + @Test func generateInstanceTokenIsOpaqueAndUnique() { + let first = ManagedContainer.generateInstanceToken() + let second = ManagedContainer.generateInstanceToken() + + #expect(first != second) + #expect(first == first.lowercased()) + #expect(UUID(uuidString: first) != nil) + } + @Test func labelsDeriveFromConfiguration() { let mc = ManagedContainer( configuration: makeTestConfiguration(labels: ["com.example.role": "x"]), diff --git a/Tests/IntegrationTests/Containers/TestCLICreateCommand.swift b/Tests/IntegrationTests/Containers/TestCLICreateCommand.swift index 052dd50a2..0d61accef 100644 --- a/Tests/IntegrationTests/Containers/TestCLICreateCommand.swift +++ b/Tests/IntegrationTests/Containers/TestCLICreateCommand.swift @@ -14,6 +14,7 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerResource import ContainerTestSupport import ContainerizationExtras import Foundation @@ -21,6 +22,24 @@ import Testing @Suite struct TestCLICreateCommand { + @Test func testJSONCreateReturnsPersistedServerIdentity() async throws { + try await ContainerFixture.with { f in + let name = "\(f.testID)-json" + f.addCleanup { try f.doRemoveIfExists(name, ignoreFailure: true) } + + var args = ["create", "--format", "json", "--name", name] + args += f.proxyEnvironmentArgs + args += [WarmupImage.alpine320.rawValue, "sleep", "infinity"] + let command = try f.run(args).check() + let result = try JSONDecoder().decode(ContainerCreateResult.self, from: command.outputData) + let inspected = try f.inspectContainer(name) + + #expect(result.id == name) + #expect(!result.instanceToken.isEmpty) + #expect(inspected.configuration.instanceToken == result.instanceToken) + } + } + @Test func testCreateArgsPassthrough() async throws { try await ContainerFixture.with { f in let image = WarmupImage.alpine320.rawValue diff --git a/Tests/IntegrationTests/Containers/TestCLIRmRaceCondition.swift b/Tests/IntegrationTests/Containers/TestCLIRmRaceCondition.swift index 529fcbb63..d71f16642 100644 --- a/Tests/IntegrationTests/Containers/TestCLIRmRaceCondition.swift +++ b/Tests/IntegrationTests/Containers/TestCLIRmRaceCondition.swift @@ -14,11 +14,61 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerResource import ContainerTestSupport +import Foundation import Testing @Suite struct TestCLIRmRaceCondition { + @Test func testStaleInstanceTokenCannotDeleteReplacement() async throws { + try await ContainerFixture.with { f in + let name = "\(f.testID)-reuse" + let otherName = "\(f.testID)-other" + f.addCleanup { try f.doRemoveIfExists(name, force: true, ignoreFailure: true) } + f.addCleanup { try f.doRemoveIfExists(otherName, force: true, ignoreFailure: true) } + + let firstCreate = try createWithResult(f, name: name) + let firstToken = firstCreate.instanceToken + try f.run(["delete", "--if-instance-token", firstToken, name]).check() + + let replacementCreate = try createWithResult(f, name: name) + let replacementToken = replacementCreate.instanceToken + #expect(replacementToken != firstToken) + + let staleDelete = try f.run(["delete", "--if-instance-token", firstToken, name]) + #expect(staleDelete.status != 0) + #expect(staleDelete.error.contains("container instance precondition failed")) + #expect(try f.inspectContainer(name).configuration.instanceToken == replacementToken) + + let otherCreate = try createWithResult(f, name: otherName) + let otherToken = otherCreate.instanceToken + let crossContainerDelete = try f.run(["delete", "--if-instance-token", otherToken, name]) + #expect(crossContainerDelete.status != 0) + #expect(try f.inspectContainer(name).configuration.instanceToken == replacementToken) + + try f.run(["delete", "--if-instance-token", replacementToken, name]).check() + #expect((try f.run(["inspect", name])).status != 0) + } + } + + @Test func testConditionalForceDeleteChecksTokenBeforeStopping() async throws { + try await ContainerFixture.with { f in + let name = "\(f.testID)-force" + f.addCleanup { try f.doRemoveIfExists(name, force: true, ignoreFailure: true) } + + try await f.doLongRun(name: name, autoRemove: false, waitUntilRunning: true) + let token = try #require(try f.inspectContainer(name).configuration.instanceToken) + + let mismatch = try f.run(["delete", "--force", "--if-instance-token", "stale-token", name]) + #expect(mismatch.status != 0) + #expect(try f.getContainerStatus(name) == "running") + + try f.run(["delete", "--force", "--if-instance-token", token, name]).check() + #expect((try f.run(["inspect", name])).status != 0) + } + } + @Test func testStopRmRace() async throws { try await ContainerFixture.with { f in let name = "\(f.testID)-c" @@ -77,4 +127,12 @@ struct TestCLIRmRaceCondition { } } } + + private func createWithResult(_ fixture: ContainerFixture, name: String) throws -> ContainerCreateResult { + var args = ["create", "--format", "json", "--rm", "--name", name] + args += fixture.proxyEnvironmentArgs + args += [WarmupImage.alpine320.rawValue, "sleep", "infinity"] + let command = try fixture.run(args).check() + return try JSONDecoder().decode(ContainerCreateResult.self, from: command.outputData) + } } diff --git a/docs/command-reference.md b/docs/command-reference.md index a99ac8528..aa45c47a7 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -192,7 +192,7 @@ Creates a container from an image without starting it. This command accepts most **Usage** ```bash -container create [] [ ...] +container create [--format json] [] [ ...] ``` **Arguments** @@ -223,6 +223,7 @@ container create [] [ ...] * `--cap-add `: Add a Linux capability (e.g. `CAP_NET_RAW`, `NET_RAW`, or `ALL`) * `--cap-drop `: Drop a Linux capability (e.g. `CAP_NET_RAW`, `NET_RAW`, or `ALL`) * `--cidfile `: Write the container ID to the path provided +* `--format json`: Output the created container ID and server-generated instance token as JSON. This mode fails before creation if the API server does not support an atomic create result. * `-d, --detach`: Run the container and detach from the process * `--dns `: DNS nameserver IP address * `--dns-domain `: Default DNS domain @@ -253,6 +254,19 @@ container create [] [ ...] * `-v, --volume `: Bind mount a volume into the container * `--virtualization`: Expose virtualization capabilities to the container (requires host and guest support) +**Machine-readable ownership** + +Use JSON output when another process must later delete the exact container instance it created: + +```bash +container create --format json --name worker alpine:latest +# {"id":"worker","instanceToken":"..."} + +container rm --if-instance-token "" worker +``` + +The instance token is generated by the API server, returned by the same create operation, and changes whenever an ID is reused. Do not replace it with a token obtained from a later inspect operation. + **Registry Options** * `--scheme `: Scheme to use when connecting to the container registry. One of (http, https, auto) (default: auto) @@ -326,7 +340,7 @@ Deletes one or more containers. If the container is running, you may force delet **Usage** ```bash -container delete [--all] [--force] [--debug] [ ...] +container delete [--all] [--force] [--if-instance-token ] [--debug] [ ...] ``` **Arguments** @@ -337,6 +351,7 @@ container delete [--all] [--force] [--debug] [ ...] * `-a, --all`: Delete all containers * `-f, --force`: Delete containers even if they are running +* `--if-instance-token `: Delete only if the single explicit container ID still identifies the instance that supplied this token. This option cannot be combined with `--all` or multiple IDs. ### `container list (ls)`