From 046884df376f6bb53ba9c87ae9791c1b95a76e89 Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Thu, 28 May 2026 09:33:21 -0700 Subject: [PATCH] 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 20560f6e..25ef9e33 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 327b8bd2..ab150cd8 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 8110a1d3..7c2367a3 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 124205b6..ea833b55 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 00000000..c16b15b6 --- /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 fe4de996..cc35e203 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 00000000..5eb7b8f0 --- /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 8100c412..36cf2443 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 cf5b8d6d..f649b68a 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 a90343bc..1d589f83 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 1cd3295b..81b92090 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")