Add `--labels` for networks. by jglogan · Pull Request #600 · apple/container

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

jglogan · 2025-09-10T20:33:49Z

Closes[Request]: Add container network create --label option. #557.
Breaking change: removes .upToNextOption for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

Type of Change

Bug fix
New feature
Breaking change
Documentation update

Motivation and Context

Consistent features and UX across managed resources.

Testing

Tested locally
Added/updated tests
Added/updated docs

- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

realrajaryan · 2025-09-11T23:26:12Z

+ 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) ?? [:]


Should we use the CodingKey enum to stay consistent with how ContainerConfiguration.swift handles labels?

realrajaryan · 2025-09-11T23:26:50Z

+ // 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(.exists, message: "network \(configuration.id) failed to start")


would .invalidState be better?

yes, .exists is just wrong, I think it must've just autofilled and I didn't notice it

realrajaryan · 2025-09-11T23:30:56Z

 func run() async throws {
- let config = NetworkConfiguration(id: self.name, mode: .nat)
+ let parsedLabels = Utility.parseKeyValuePairs(labels)
+ let config = NetworkConfiguration(id: self.name, mode: .nat, labels: parsedLabels)


should we have some sort of validation on the CLI level like containers or server level like volumes, for network name or labels?

yes, good call, I decided to make init() for NetworkConfiguration validate and throw

commit 449f1d2 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 13:37:55 2025 -0700 Replace scattered defaults subcommands with `system property`. (apple#604) Common subcommands for all defaults. - Closesapple#384. - Replaces `registry default` and `system dns default` subcommands with `system property`. - Users can use `system property ls` to see details about each supported default value. - `system property set` implements reasonable validation for all properties. - NOTE: Probing of the registry for `registry default set` was removed, which means users will find out about a botched setting when pulling or pushing. - Updates docs. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context See apple#384. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs commit 386fd87 Author: Kathryn Baldauf <k_baldauf@apple.com> Date: Tue Sep 16 10:52:08 2025 -0700 Enumerate using relative paths to avoid mismatch with symlink resolution of special paths like /tmp (apple#613) ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Fixesapple#588. This PR changes the archiver compression file enumeration to use the [enumerator(atPath:)](https://developer.apple.com/documentation/foundation/filemanager/enumerator(atpath:)) version. This version returns relative paths instead of full file paths from the filesystem. /tmp is symlinked to /private/tmp and some swift packages will handle that path differently. While a call to Foundation's `URL.resolvingSymlinksInPath()` will return "/tmp", a call to `FileManager.enumerator(at:)` will return "/private/tmp". This difference causes a container image build to fail when the user is using a path under /tmp or other special case paths as the context directory. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com> commit 79cc363 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Tue Sep 16 10:14:14 2025 -0700 Relocates API server to Helpers, service to Services. (apple#616) - Closesapple#615. Improves project organization. Separates service so it can be tested and used separately from the executable target. No functional changes. commit a54be36 Author: J Logan <sgtbakerrahulnet@yahoo.com> Date: Mon Sep 15 11:27:51 2025 -0700 Add `--labels` for networks. (apple#600) - Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive. ## Type of Change - [ ] Bug fix - [x] New feature - [x] Breaking change - [x] Documentation update ## Motivation and Context Consistent features and UX across managed resources. ## Testing - [x] Tested locally - [x] Added/updated tests - [x] Added/updated docs

Add --labels for networks.
c57ce86
- Closesapple#557. - Breaking change: removes `.upToNextOption` for labels on volumes as this is not what is done for containers, and it forces the argument to precede the options if a label is supplied, which is non-intuitive.

jglogan requested a review from realrajaryan September 10, 2025 21:18

realrajaryan reviewed Sep 11, 2025
View reviewed changes

jglogan added 4 commits September 12, 2025 16:34

Fix incorrect error code.
00be063

Validate NetworkConfiguration on init().
9ac3dc4

Explicit CodingKeys for NetworkConfiguration.
1bdd771

NetworkConfiguration unit tests.
3050116

realrajaryan approved these changes Sep 15, 2025
View reviewed changes

jglogan merged commit a54be36 into apple:mainSep 15, 2025
2 checks passed

jglogan mentioned this pull request Sep 18, 2025
[Request]: Add --label option for network create. #511
Closed
1 task

jglogan deleted the users/jglogan/network-labels branch September 23, 2025 21:51

benoitf mentioned this pull request Oct 4, 2025
add support for labels in networks socktainer/socktainer#111
Closed