diff --git a/Package.swift b/Package.swift index b08a5c7f..b1417b5c 100644 --- a/Package.swift +++ b/Package.swift @@ -159,6 +159,12 @@ let package = Package( ], path: "Sources/Services/ContainerNetworkService" ), + .testTarget( + name: "ContainerNetworkServiceTests", + dependencies: [ + "ContainerNetworkService" + ] + ), .executableTarget( name: "container-core-images", dependencies: [ diff --git a/Sources/APIServer/APIServer.swift b/Sources/APIServer/APIServer.swift index 67d7d3c3..92df3512 100644 --- a/Sources/APIServer/APIServer.swift +++ b/Sources/APIServer/APIServer.swift @@ -236,7 +236,7 @@ struct APIServer: AsyncParsableCommand { .filter { $0.id == ClientNetwork.defaultNetworkName } .first if defaultNetwork == nil { - let config = NetworkConfiguration(id: ClientNetwork.defaultNetworkName, mode: .nat) + let config = try NetworkConfiguration(id: ClientNetwork.defaultNetworkName, mode: .nat) _ = try await service.create(configuration: config) } diff --git a/Sources/APIServer/Networks/NetworksService.swift b/Sources/APIServer/Networks/NetworksService.swift index 44353a99..4936f694 100644 --- a/Sources/APIServer/Networks/NetworksService.swift +++ b/Sources/APIServer/Networks/NetworksService.swift @@ -97,6 +97,13 @@ actor NetworksService { /// Create a new network from the provided configuration. public func create(configuration: NetworkConfiguration) async throws -> NetworkState { + log.info( + "network service: create", + metadata: [ + "id": "\(configuration.id)" + ]) + + // Ensure nobody is manipulating the network already. guard !busyNetworks.contains(configuration.id) else { throw ContainerizationError(.exists, message: "network \(configuration.id) has a pending operation") } @@ -104,12 +111,6 @@ actor NetworksService { busyNetworks.insert(configuration.id) defer { busyNetworks.remove(configuration.id) } - log.info( - "network service: create", - metadata: [ - "id": "\(configuration.id)" - ]) - // Ensure the network doesn't already exist. guard networkStates[configuration.id] == nil else { throw ContainerizationError(.exists, message: "network \(configuration.id) already exists") @@ -118,7 +119,14 @@ actor NetworksService { // Create and start the network. try await registerService(configuration: configuration) let client = NetworkClient(id: configuration.id) - let networkState = try await client.state() + + // Ensure the network is running, and set up the persistent network state + // using our configuration data, as the one from the helper doesn't include + // metadata. + guard case .running(_, let status) = try await client.state() else { + throw ContainerizationError(.invalidState, message: "network \(configuration.id) failed to start") + } + let networkState: NetworkState = .running(configuration, status) networkStates[configuration.id] = networkState // Persist the configuration data. diff --git a/Sources/CLI/Network/NetworkCreate.swift b/Sources/CLI/Network/NetworkCreate.swift index 535e029e..e07f5fce 100644 --- a/Sources/CLI/Network/NetworkCreate.swift +++ b/Sources/CLI/Network/NetworkCreate.swift @@ -27,14 +27,18 @@ extension Application { commandName: "create", abstract: "Create a new network") - @Argument(help: "Network name") - var name: String - @OptionGroup var global: Flags.Global + @Option(name: .customLong("label"), help: "Set metadata on a network") + var labels: [String] = [] + + @Argument(help: "Network name") + var name: String + func run() async throws { - let config = NetworkConfiguration(id: self.name, mode: .nat) + let parsedLabels = Utility.parseKeyValuePairs(labels) + let config = try NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels) let state = try await ClientNetwork.create(configuration: config) print(state.id) } diff --git a/Sources/CLI/Network/NetworkDelete.swift b/Sources/CLI/Network/NetworkDelete.swift index 836d6c8c..431f53eb 100644 --- a/Sources/CLI/Network/NetworkDelete.swift +++ b/Sources/CLI/Network/NetworkDelete.swift @@ -27,12 +27,12 @@ extension Application { abstract: "Delete one or more networks", aliases: ["rm"]) - @Flag(name: .shortAndLong, help: "Remove all networks") - var all = false - @OptionGroup var global: Flags.Global + @Flag(name: .shortAndLong, help: "Remove all networks") + var all = false + @Argument(help: "Network names") var networkNames: [String] = [] diff --git a/Sources/CLI/Network/NetworkList.swift b/Sources/CLI/Network/NetworkList.swift index 9fb44dcb..2c1dcddc 100644 --- a/Sources/CLI/Network/NetworkList.swift +++ b/Sources/CLI/Network/NetworkList.swift @@ -28,15 +28,15 @@ extension Application { abstract: "List networks", aliases: ["ls"]) + @OptionGroup + var global: Flags.Global + @Flag(name: .shortAndLong, help: "Only output the network name") var quiet = false @Option(name: .long, help: "Format of the output") var format: ListFormat = .table - @OptionGroup - var global: Flags.Global - func run() async throws { let networks = try await ClientNetwork.list() try printNetworks(networks: networks, format: format) diff --git a/Sources/CLI/Volume/VolumeCreate.swift b/Sources/CLI/Volume/VolumeCreate.swift index de7bbe30..a9accb95 100644 --- a/Sources/CLI/Volume/VolumeCreate.swift +++ b/Sources/CLI/Volume/VolumeCreate.swift @@ -25,18 +25,18 @@ extension Application.VolumeCommand { abstract: "Create a volume" ) - @Argument(help: "Volume name") - var name: String - @Option(name: .customShort("s"), help: "Size of the volume (default: 512GB). Examples: 1G, 512MB, 2T") var size: String? - @Option(name: .customLong("opt"), parsing: .upToNextOption, help: "Set driver specific options") + @Option(name: .customLong("opt"), help: "Set driver specific options") var driverOpts: [String] = [] - @Option(name: .customLong("label"), parsing: .upToNextOption, help: "Set metadata on a volume") + @Option(name: .customLong("label"), help: "Set metadata on a volume") var labels: [String] = [] + @Argument(help: "Volume name") + var name: String + func run() async throws { var parsedDriverOpts = Utility.parseKeyValuePairs(driverOpts) let parsedLabels = Utility.parseKeyValuePairs(labels) diff --git a/Sources/Helpers/NetworkVmnet/NetworkVmnetHelper.swift b/Sources/Helpers/NetworkVmnet/NetworkVmnetHelper.swift index 96629b0c..27a3b664 100644 --- a/Sources/Helpers/NetworkVmnet/NetworkVmnetHelper.swift +++ b/Sources/Helpers/NetworkVmnet/NetworkVmnetHelper.swift @@ -65,7 +65,7 @@ extension NetworkVmnetHelper { do { log.info("configuring XPC server") let subnet = try self.subnet.map { try CIDRAddress($0) } - let configuration = NetworkConfiguration(id: id, mode: .nat, subnet: subnet?.description) + let configuration = try NetworkConfiguration(id: id, mode: .nat, subnet: subnet?.description) let network = try Self.createNetwork(configuration: configuration, log: log) try await network.start() let server = try await NetworkService(network: network, log: log) diff --git a/Sources/Services/ContainerNetworkService/NetworkConfiguration.swift b/Sources/Services/ContainerNetworkService/NetworkConfiguration.swift index 0a3f060d..73378569 100644 --- a/Sources/Services/ContainerNetworkService/NetworkConfiguration.swift +++ b/Sources/Services/ContainerNetworkService/NetworkConfiguration.swift @@ -14,6 +14,9 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError +import ContainerizationExtras + /// Configuration parameters for network creation. public struct NetworkConfiguration: Codable, Sendable, Identifiable { /// A unique identifier for the network @@ -25,14 +28,89 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable { /// The preferred CIDR address for the subnet, if specified public let subnet: String? + /// Key-value labels for the network. + public var labels: [String: String] = [:] + /// Creates a network configuration public init( id: String, mode: NetworkMode, - subnet: String? = nil - ) { + subnet: String? = nil, + labels: [String: String] = [:] + ) throws { self.id = id self.mode = mode self.subnet = subnet + self.labels = labels + try validate() + } + + enum CodingKeys: String, CodingKey { + case id + case mode + case subnet + case labels + } + + /// Create a configuration from the supplied Decoder, initializing missing + /// values where possible to reasonable defaults. + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + + id = try container.decode(String.self, forKey: .id) + mode = try container.decode(NetworkMode.self, forKey: .mode) + subnet = try container.decodeIfPresent(String.self, forKey: .subnet) + labels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:] + try validate() + } + + private func validate() throws { + guard id.isValidNetworkID() else { + throw ContainerizationError(.invalidArgument, message: "invalid network ID: \(id)") + } + + if let subnet { + _ = try CIDRAddress(subnet) + } + + for (key, value) in labels { + try validateLabel(key: key, value: value) + } + } + + /// TODO: Extract when we clean up client dependencies. + private func validateLabel(key: String, value: String) throws { + let keyLengthMax = 128 + let labelLengthMax = 4096 + guard key.count <= keyLengthMax else { + throw ContainerizationError(.invalidArgument, message: "invalid label, key length is greater than \(keyLengthMax): \(key)") + } + + guard key.isValidLabelKey() else { + throw ContainerizationError(.invalidArgument, message: "invalid label key: \(key)") + } + + let fullLabel = "\(key)=\(value)" + guard fullLabel.count <= labelLengthMax else { + throw ContainerizationError(.invalidArgument, message: "invalid label, key length is greater than \(labelLengthMax): \(fullLabel)") + } + } +} + +extension String { + /// Ensure that the network ID has the correct syntax. + fileprivate func isValidNetworkID() -> Bool { + let pattern = #"^[a-z0-9](?:[a-z0-9._-]{0,61}[a-z0-9])?$"# + return self.range(of: pattern, options: .regularExpression) != nil + } + + /// Ensure label key conforms to OCI or Docker label guidelines. + /// TODO: Extract when we clean up client dependencies. + fileprivate func isValidLabelKey() -> Bool { + let dockerPattern = #/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)*$/# + let ociPattern = #/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)*(?:/(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)*))*$/# + let dockerMatch = !self.ranges(of: dockerPattern).isEmpty + let ociMatch = !self.ranges(of: ociPattern).isEmpty + return dockerMatch || ociMatch } } diff --git a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift index 3d65bb09..dd9260e9 100644 --- a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift +++ b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift @@ -128,4 +128,60 @@ class TestCLINetwork: CLITest { return } } + + @available(macOS 26, *) + @Test func testNetworkLabels() async throws { + do { + // prep: delete container and network, ignoring if it doesn't exist + let name = Test.current!.name.trimmingCharacters(in: ["(", ")"]) + try? doRemove(name: name) + let networkDeleteArgs = ["network", "delete", name] + _ = try? run(arguments: networkDeleteArgs) + + // create our network + let networkCreateArgs = ["network", "create", "--label", "foo=bar", "--label", "baz=qux", name] + let networkCreateResult = try run(arguments: networkCreateArgs) + guard networkCreateResult.status == 0 else { + throw CLIError.executionFailed("command failed: \(networkCreateResult.error)") + } + + // ensure it's deleted + defer { + _ = try? run(arguments: networkDeleteArgs) + } + + // inspect the network + let networkInspectArgs = ["network", "inspect", name] + let networkInspectResult = try run(arguments: networkInspectArgs) + guard networkInspectResult.status == 0 else { + throw CLIError.executionFailed("command failed: \(networkInspectResult.error)") + } + + // decode the JSON result + let networkInspectOutput = networkInspectResult.output + guard let jsonData = networkInspectOutput.data(using: .utf8) else { + throw CLIError.invalidOutput("network inspect output invalid") + } + + let decoder = JSONDecoder() + let networks = try decoder.decode([NetworkInspectOutput].self, from: jsonData) + guard networks.count == 1 else { + throw CLIError.invalidOutput("expected exactly one network from inspect, got \(networks.count)") + } + + // validate labels + + let expectedLabels = [ + "foo": "bar", + "baz": "qux", + ] + #expect(expectedLabels == networks[0].config.labels) + + // delete should succeed + _ = try run(arguments: networkDeleteArgs) + } catch { + Issue.record("failed to safely delete network \(error)") + return + } + } } diff --git a/Tests/CLITests/Utilities/CLITest.swift b/Tests/CLITests/Utilities/CLITest.swift index 9e6adcde..e7a6f339 100644 --- a/Tests/CLITests/Utilities/CLITest.swift +++ b/Tests/CLITests/Utilities/CLITest.swift @@ -29,6 +29,7 @@ class CLITest { let reference: String } + // These structs need to track their counterpart presentation structs in CLI. struct ImageInspectOutput: Codable { let name: String let variants: [variant] @@ -41,6 +42,13 @@ class CLITest { } } + struct NetworkInspectOutput: Codable { + let id: String + let state: String + let config: NetworkConfiguration + let status: NetworkStatus? + } + init() throws {} let testUUID = UUID().uuidString diff --git a/Tests/ContainerNetworkServiceTests/NetworkConfigurationTest.swift b/Tests/ContainerNetworkServiceTests/NetworkConfigurationTest.swift new file mode 100644 index 00000000..dfcc4ac3 --- /dev/null +++ b/Tests/ContainerNetworkServiceTests/NetworkConfigurationTest.swift @@ -0,0 +1,123 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Apple Inc. and the container project authors. All rights reserved. +// +// 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 ContainerizationError +import ContainerizationExtras +import Testing + +@testable import ContainerNetworkService + +struct NetworkConfigurationTest { + @Test func testValidationOkDefaults() throws { + let id = "foo" + _ = try NetworkConfiguration(id: id, mode: .nat) + } + + @Test func testValidationGoodId() throws { + let ids = [ + String(repeating: "0", count: 63), + "0", + "0-_.1", + ] + for id in ids { + let subnet = "192.168.64.1/24" + let labels = [ + "foo": "bar", + "baz": String(repeating: "0", count: 4096 - "baz".count - "=".count), + ] + _ = try NetworkConfiguration(id: id, mode: .nat, subnet: subnet, labels: labels) + } + } + + @Test func testValidationBadId() throws { + let ids = [ + String(repeating: "0", count: 64), + "-foo", + "foo_", + "Foo", + ] + for id in ids { + let subnet = "192.168.64.1/24" + let labels = [ + "foo": "bar", + "baz": String(repeating: "0", count: 4096 - "baz".count - "=".count), + ] + #expect { + _ = try NetworkConfiguration(id: id, mode: .nat, subnet: subnet, labels: labels) + } throws: { error in + guard let err = error as? ContainerizationError else { return false } + #expect(err.code == .invalidArgument) + #expect(err.message.starts(with: "invalid network ID")) + return true + } + } + } + + @Test func testValidationBadSubnet() throws { + let id = "foo" + let subnet = "192.168.64.1" + let labels = [ + "foo": "bar", + "baz": String(repeating: "0", count: 4096 - "baz".count - "=".count), + ] + #expect { + _ = try NetworkConfiguration(id: id, mode: .nat, subnet: subnet, labels: labels) + } throws: { error in + guard let err = error as? NetworkAddressError else { return false } + #expect(err.description.starts(with: "invalid CIDR block")) + return true + } + } + + @Test func testValidationGoodLabels() throws { + let allLabels = [ + ["com.example.my-label": "bar"], + ["mycompany.com/my-label": "bar"], + ["foo": String(repeating: "0", count: 4096 - "foo".count - "=".count)], + [String(repeating: "0", count: 128): ""], + ] + for labels in allLabels { + let id = "foo" + let subnet = "192.168.64.1/24" + _ = try NetworkConfiguration(id: id, mode: .nat, subnet: subnet, labels: labels) + } + } + + @Test func testValidationBadLabels() throws { + let allLabels = [ + [String(repeating: "0", count: 129): ""], + ["foo": String(repeating: "0", count: 4097 - "foo".count - "=".count)], + ["com..example.my-label": "bar"], + ["mycompany.com//my-label": "bar"], + ["": String(repeating: "0", count: 4096 - "foo".count - "=".count)], + ] + for labels in allLabels { + let id = "foo" + let subnet = "192.168.64.1/24" + #expect { + _ = try NetworkConfiguration(id: id, mode: .nat, subnet: subnet, labels: labels) + } throws: { error in + guard let err = error as? ContainerizationError else { return false } + #expect(err.code == .invalidArgument) + #expect(err.message.starts(with: "invalid label")) + return true + } + } + } + +} diff --git a/docs/command-reference.md b/docs/command-reference.md index f7907665..9012f948 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -465,10 +465,13 @@ Creates a new network with the given name. **Usage** ```bash -container network create NAME +container network create NAME [OPTIONS] ``` -No additional flags; uses global options for debugging, version, and help. +**Options** + +* `--label `: set metadata labels on the network +* **Global**: `--version`, `-h`/`--help` ### `container network delete (rm)` @@ -530,8 +533,8 @@ container volume create [OPTIONS] NAME **Options** * `-s `: size of the volume (default: 512GB). Examples: `1G`, `512MB`, `2T` -* `--opt `: set driver-specific options (repeatable) -* `--label `: set metadata labels on the volume (repeatable) +* `--opt `: set driver-specific options +* `--label `: set metadata labels on the volume * **Global**: `--version`, `-h`/`--help` ### `container volume delete (rm)`