Create ResourceLabels and use for ManagedResource, NetworkConfiguration. (#1360)

- Closes #1359.
- Create a ResourceLabels type and extract the label validation from
NetworkConfiguration into the new type.
- Create a base AppError type that is compatible with structured logging
and delegates message presentation to the error receiver.
- Define LabelError over AppError for label validation.
- Slightly reworks NetworkConfiguration entity migration code in
NetworksService.
This commit is contained in:
J Logan
2026-04-07 16:31:18 -07:00
committed by GitHub
parent af43a8b256
commit e37dcc19a5
14 changed files with 251 additions and 126 deletions
+1 -1
View File
@@ -316,7 +316,7 @@ extension APIServer {
let config = try NetworkConfiguration(
id: ClientNetwork.defaultNetworkName,
mode: .nat,
labels: [ResourceLabelKeys.role: ResourceRoleValues.builtin],
labels: try .init([ResourceLabelKeys.role: ResourceRoleValues.builtin]),
pluginInfo: NetworkPluginInfo(plugin: "container-network-vmnet")
)
_ = try await service.create(configuration: config)
@@ -73,7 +73,7 @@ extension Application {
for mount in container.configuration.mounts where mount.isVirtiofs {
if !FileManager.default.fileExists(atPath: mount.source) {
throw ContainerizationError(.invalidState, message: "path '\(mount.source)' is not a directory")
throw ContainerizationError(.invalidState, message: "mount source path '\(mount.source)' does not exist")
}
}
@@ -63,7 +63,7 @@ extension Application {
public init() {}
public func run() async throws {
let parsedLabels = Utility.parseKeyValuePairs(labels)
let parsedLabels = try ResourceLabels(Utility.parseKeyValuePairs(labels))
let mode: NetworkMode = hostOnly ? .hostOnly : .nat
let config = try NetworkConfiguration(
id: self.name,
@@ -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.
//===----------------------------------------------------------------------===//
/// Protocol for errors with a stable code and structured metadata.
/// This allows the client to present the error as it chooses.
import Collections
public protocol AppError: Error {
var code: AppErrorCode { get }
var metadata: OrderedDictionary<String, String> { get }
var underlyingError: Error? { get }
}
public struct AppErrorCode: RawRepresentable, Hashable, Sendable {
public let rawValue: String
public init(rawValue: String) {
self.rawValue = rawValue
}
public static let invalidArgument = AppErrorCode(rawValue: "invalid_argument")
}
@@ -32,8 +32,10 @@ public protocol ManagedResource: Identifiable, Sendable, Codable {
/// Key-value properties for the resource. The user and system may both
/// make use of labels to read and write annotations or other metadata.
/// A good practice is to use
var labels: [String: String] { get }
/// A good practice when using labels for automation is to use reverse
/// domain name notation (example: `com.example.mytool.role`) for
/// label names.
var labels: ResourceLabels { get }
/// Generates a unique resource ID value.
static func generateId() -> String
@@ -52,7 +54,7 @@ extension ManagedResource {
}
}
// FIXME: This moves to ManagedResource and/or a ResourceLabels typealias eventually.
extension [String: String] {
extension ResourceLabels {
/// Returns true if for a resource that the system automatically manages.
public var isBuiltin: Bool { self.contains { $0 == ResourceLabelKeys.role && $1 == ResourceRoleValues.builtin } }
}
@@ -14,6 +14,90 @@
// limitations under the License.
//===----------------------------------------------------------------------===//
import Collections
/// Metadata for a managed resource.
public struct ResourceLabels: Sendable, Equatable {
public static let keyLengthMax = 128
public static let labelLengthMax = 4096
public let dictionary: [String: String]
public struct LabelError: AppError {
public var code: AppErrorCode
public var metadata: OrderedDictionary<String, String>
public var underlyingError: (any Error)? { nil }
}
public init() {
dictionary = [:]
}
public init(_ labels: [String: String]) throws {
for (key, value) in labels {
try Self.validateLabel(key: key, value: value)
}
self.dictionary = labels
}
public static func validateLabelKey(_ key: String) throws {
guard key.count <= Self.keyLengthMax else {
throw LabelError(code: .invalidLabelKeyLength, metadata: ["key": key, "maxLength": "\(Self.keyLengthMax)"])
}
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 = !key.ranges(of: dockerPattern).isEmpty
let ociMatch = !key.ranges(of: ociPattern).isEmpty
guard dockerMatch || ociMatch else {
throw LabelError(code: .invalidLabelKeyContent, metadata: ["key": key])
}
}
public static func validateLabel(key: String, value: String) throws {
try validateLabelKey(key)
let fullLabel = "\(key)=\(value)"
guard fullLabel.count <= labelLengthMax else {
throw LabelError(code: .invalidLabelLength, metadata: ["label": fullLabel, "maxLength": "\(Self.labelLengthMax)"])
}
}
}
extension ResourceLabels: Codable {
public func encode(to encoder: Encoder) throws {
try dictionary.encode(to: encoder)
}
public init(from decoder: Decoder) throws {
let dict = try [String: String](from: decoder)
try self.init(dict)
}
}
extension ResourceLabels: Collection {
public typealias Index = Dictionary<String, String>.Index
public typealias Element = Dictionary<String, String>.Element
public var startIndex: Index { dictionary.startIndex }
public var endIndex: Index { dictionary.endIndex }
public subscript(position: Index) -> Element { dictionary[position] }
public func index(after i: Index) -> Index { dictionary.index(after: i) }
// Direct key access
public subscript(key: String) -> String? {
get { dictionary[key] }
}
}
extension AppErrorCode {
public static let invalidLabelKeyContent = AppErrorCode(rawValue: "invalid_label_key_content")
public static let invalidLabelKeyLength = AppErrorCode(rawValue: "invalid_label_key_length")
public static let invalidLabelLength = AppErrorCode(rawValue: "invalid_label_length")
}
/// System-defined keys for resource labels.
public struct ResourceLabelKeys {
/// Indicates a owner of a resource managed by a plugin.
@@ -46,12 +46,13 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable {
public let ipv6Subnet: CIDRv6?
/// Key-value labels for the network.
public var labels: [String: String] = [:]
/// Resource labels should not be mutated, except while building a network configurations.
public let labels: ResourceLabels
/// Details about the network plugin that manages this network.
/// FIXME: This field only needs to be optional while we wait for the field
/// to be proliferated to most users when they update container.
public var pluginInfo: NetworkPluginInfo?
public let pluginInfo: NetworkPluginInfo?
/// Creates a network configuration
public init(
@@ -59,8 +60,8 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable {
mode: NetworkMode,
ipv4Subnet: CIDRv4? = nil,
ipv6Subnet: CIDRv6? = nil,
labels: [String: String] = [:],
pluginInfo: NetworkPluginInfo
labels: ResourceLabels = .init(),
pluginInfo: NetworkPluginInfo?
) throws {
self.id = id
self.creationDate = Date()
@@ -98,7 +99,8 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable {
ipv4Subnet = try subnetText.map { try CIDRv4($0) }
ipv6Subnet = try container.decodeIfPresent(String.self, forKey: .ipv6Subnet)
.map { try CIDRv6($0) }
labels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:]
let decodedLabels = try container.decodeIfPresent([String: String].self, forKey: .labels) ?? [:]
labels = try .init(decodedLabels)
pluginInfo = try container.decodeIfPresent(NetworkPluginInfo.self, forKey: .pluginInfo)
try validate()
}
@@ -120,28 +122,6 @@ public struct NetworkConfiguration: Codable, Sendable, Identifiable {
guard id.isValidNetworkID() else {
throw ContainerizationError(.invalidArgument, message: "invalid network ID: \(id)")
}
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)")
}
}
}
@@ -151,14 +131,4 @@ extension String {
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
}
}
@@ -32,22 +32,22 @@ public struct RegistryResource: ManagedResource {
///
/// This value must be a valid DNS hostname or IPv6 address, optionally
/// followed by a port number (e.g., "docker.io", "localhost:5000", "[::1]:5000").
public var name: String
public let name: String
/// The username used for authentication with this registry.
public let username: String
/// The time at which the system created this registry resource.
public var creationDate: Date
public let creationDate: Date
/// The time at which the registry resource was last modified.
public var modificationDate: Date
public let modificationDate: Date
/// Key-value properties for the resource.
///
/// The user and system may both make use of labels to read and write
/// annotations or other metadata.
public var labels: [String: String]
public let labels: ResourceLabels
/// Validates a registry hostname according to OCI distribution specification.
///
@@ -94,7 +94,7 @@ public struct RegistryResource: ManagedResource {
username: String,
creationDate: Date,
modificationDate: Date,
labels: [String: String] = [:]
labels: ResourceLabels = .init()
) {
self.id = hostname
self.name = hostname
@@ -76,13 +76,15 @@ public actor NetworksService {
self.networkPlugins = networkPlugins
let configurations = try await store.list()
for var configuration in configurations {
for configuration in configurations {
// Ensure the network with id "default" is marked as builtin.
var updatedLabels: [String: String]?
if configuration.id == ClientNetwork.defaultNetworkName {
let role = configuration.labels[ResourceLabelKeys.role]
if role == nil || role != ResourceRoleValues.builtin {
configuration.labels[ResourceLabelKeys.role] = ResourceRoleValues.builtin
try await store.update(configuration)
var labels = configuration.labels.dictionary
labels[ResourceLabelKeys.role] = ResourceRoleValues.builtin
updatedLabels = labels
}
}
@@ -90,9 +92,17 @@ public actor NetworksService {
// Before this field was added, the code always assumed we were using the
// container-network-vmnet network plugin, so it should be safe to fallback to that
// if no info was found in an on disk configuration.
if configuration.pluginInfo == nil {
configuration.pluginInfo = NetworkPluginInfo(plugin: "container-network-vmnet")
try await store.update(configuration)
if updatedLabels != nil || configuration.pluginInfo == nil {
let updatedConfiguration = try NetworkConfiguration(
id: configuration.id,
mode: configuration.mode,
ipv4Subnet: configuration.ipv4Subnet,
ipv6Subnet: configuration.ipv6Subnet,
labels: updatedLabels.map { try .init($0) } ?? configuration.labels,
pluginInfo: configuration.pluginInfo ?? NetworkPluginInfo(plugin: "container-network-vmnet")
)
try await store.update(updatedConfiguration)
}
// Start up the network.
@@ -118,14 +128,28 @@ public actor NetworksService {
// by what comes back from the network helper, which messes up creationDate.
// FIXME: Temporarily need to override the plugin information with the info from
// the helper, so we can ensure that older networks get a variant value.
var finalConfig = configuration
let finalConfiguration: NetworkConfiguration
switch networkState {
case .created(let helperConfig):
finalConfig.pluginInfo = helperConfig.pluginInfo
networkState = NetworkState.created(finalConfig)
finalConfiguration = try NetworkConfiguration(
id: configuration.id,
mode: configuration.mode,
ipv4Subnet: configuration.ipv4Subnet,
ipv6Subnet: configuration.ipv6Subnet,
labels: updatedLabels.map { try .init($0) } ?? configuration.labels,
pluginInfo: helperConfig.pluginInfo
)
networkState = NetworkState.created(finalConfiguration)
case .running(let helperConfig, let status):
finalConfig.pluginInfo = helperConfig.pluginInfo
networkState = NetworkState.running(finalConfig, status)
finalConfiguration = try NetworkConfiguration(
id: configuration.id,
mode: configuration.mode,
ipv4Subnet: configuration.ipv4Subnet,
ipv6Subnet: configuration.ipv6Subnet,
labels: updatedLabels.map { try .init($0) } ?? configuration.labels,
pluginInfo: helperConfig.pluginInfo
)
networkState = NetworkState.running(finalConfiguration, status)
}
let state = NetworkServiceState(
@@ -133,13 +157,13 @@ public actor NetworksService {
client: client
)
serviceStates[finalConfig.id] = state
serviceStates[finalConfiguration.id] = state
guard case .running = networkState else {
log.error(
"network failed to start",
metadata: [
"id": "\(finalConfig.id)",
"id": "\(finalConfiguration.id)",
"state": "\(networkState.state)",
])
return
@@ -204,26 +228,33 @@ public actor NetworksService {
guard case .running(let helperConfig, let status) = try await client.state() else {
throw ContainerizationError(.invalidState, message: "network \(configuration.id) failed to start")
}
var finalConfig = configuration
finalConfig.pluginInfo = helperConfig.pluginInfo
let networkState: NetworkState = .running(finalConfig, status)
let finalConfiguration = try NetworkConfiguration(
id: configuration.id,
mode: configuration.mode,
ipv4Subnet: configuration.ipv4Subnet,
ipv6Subnet: configuration.ipv6Subnet,
labels: configuration.labels,
pluginInfo: helperConfig.pluginInfo
)
let networkState: NetworkState = .running(finalConfiguration, status)
let serviceState = NetworkServiceState(networkState: networkState, client: client)
await self.setServiceState(key: finalConfig.id, value: serviceState)
await self.setServiceState(key: finalConfiguration.id, value: serviceState)
// Persist the configuration data.
do {
try await self.store.create(finalConfig)
try await self.store.create(finalConfiguration)
return networkState
} catch {
await self.removeServiceState(key: finalConfig.id)
await self.removeServiceState(key: finalConfiguration.id)
do {
try await self.deregisterService(configuration: finalConfig)
try await self.deregisterService(configuration: finalConfiguration)
} catch {
self.log.error(
"failed to deregister network service after failed creation",
metadata: [
"id": "\(finalConfig.id)",
"id": "\(finalConfiguration.id)",
"error": "\(error.localizedDescription)",
])
}
@@ -182,7 +182,7 @@ class TestCLINetwork: CLITest {
"foo": "bar",
"baz": "qux",
]
#expect(expectedLabels == networks[0].config.labels)
#expect(expectedLabels == networks[0].config.labels.dictionary)
// delete should succeed
_ = try run(arguments: networkDeleteArgs)
@@ -26,7 +26,7 @@ struct ManagedResourceTests {
var id: String
var name: String
var creationDate: Date
var labels: [String: String]
var labels: ResourceLabels
static func nameValid(_ name: String) -> Bool {
true
@@ -40,10 +40,10 @@ struct NetworkConfigurationTest {
]
for id in ids {
let ipv4Subnet = try CIDRv4("192.168.64.1/24")
let labels = [
let labels = try ResourceLabels([
"foo": "bar",
"baz": String(repeating: "0", count: 4096 - "baz".count - "=".count),
]
])
_ = try NetworkConfiguration(
id: id,
mode: .nat,
@@ -63,10 +63,10 @@ struct NetworkConfigurationTest {
]
for id in ids {
let ipv4Subnet = try CIDRv4("192.168.64.1/24")
let labels = [
let labels = try ResourceLabels([
"foo": "bar",
"baz": String(repeating: "0", count: 4096 - "baz".count - "=".count),
]
])
#expect {
_ = try NetworkConfiguration(
id: id,
@@ -84,52 +84,4 @@ struct NetworkConfigurationTest {
}
}
@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 ipv4Subnet = try CIDRv4("192.168.64.1/24")
_ = try NetworkConfiguration(
id: id,
mode: .nat,
ipv4Subnet: ipv4Subnet,
labels: labels,
pluginInfo: defaultNetworkPluginInfo
)
}
}
@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 ipv4Subnet = try CIDRv4("192.168.64.1/24")
#expect {
_ = try NetworkConfiguration(
id: id,
mode: .nat,
ipv4Subnet: ipv4Subnet,
labels: labels,
pluginInfo: defaultNetworkPluginInfo
)
} 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
}
}
}
}
@@ -137,7 +137,7 @@ struct RegistryResourceTests {
}
@Test("RegistryResource can have labels")
func testRegistryResourceWithLabels() {
func testRegistryResourceWithLabels() throws {
let hostname = "docker.io"
let username = "testuser"
let labels = [
@@ -150,7 +150,7 @@ struct RegistryResourceTests {
username: username,
creationDate: Date(),
modificationDate: Date(),
labels: labels
labels: try .init(labels)
)
#expect(resource.labels.count == 2)
@@ -0,0 +1,50 @@
//===----------------------------------------------------------------------===//
// 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 Testing
@testable import ContainerResource
struct ResourceLabelsTest {
@Test func testValidationGoodLabels() throws {
let allLabels: [[String: String]] = [
["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 {
_ = try ResourceLabels(labels)
}
}
@Test func testValidationBadLabels() throws {
let allLabels: [[String: String]] = [
[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 {
#expect {
_ = try ResourceLabels(labels)
} throws: { error in
error is ResourceLabels.LabelError
}
}
}
}