From 404966d49aab579814508a9b11710a3dde41947f Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Wed, 2 Sep 2026 06:24:00 +0000 Subject: [PATCH 1/5] fix(up): interpolate the top-level network name A compose file that selects its ingress network per environment writes networks: reverse-proxy: name: ${INGRESS_NET:-local-ingress} The service-level network reference is resolved at the run-args site, but the top-level name was passed through raw, so container rejected it: Creating network: reverse-proxy (Actual name: ${INGRESS_NET:-local-ingress}) Error: invalid network name: ${INGRESS_NET:-local-ingress} environmentVariables is already an instance property here, so this was an omission rather than a structural limit. The resolution is extracted as a pure function because setupNetwork has no test seam - the same reason healthcheck.timeout could be decoded, asserted on by two parsing tests, and still never reach the runtime. --- .../Commands/ComposeUp.swift | 22 ++++- .../NetworkNameInterpolationTests.swift | 85 +++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index d42efd7..b0b28c3 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -486,6 +486,22 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { return (value, false) } + /// The name a top-level network is actually created under. + /// + /// Interpolated for the same reason the service-level network reference is: + /// a compose file that selects its ingress network per environment writes + /// `name: ${INGRESS_NET:-local}`, and passing that through raw makes + /// `container network create` reject it with "invalid network name". + /// + /// Pure so the mapping is testable; `setupNetwork` itself has no test seam. + static func resolvedNetworkName( + key networkKey: String, + config: Network?, + environment: [String: String] + ) -> String { + resolveVariable(config?.name ?? networkKey, with: environment) + } + static func validateStoppedServiceExitCode(_ exitCode: Int32, serviceName: String) throws { guard exitCode == 0 else { throw ComposeError.containerRunFailed(serviceName, exitCode) @@ -749,7 +765,11 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { } private func setupNetwork(name networkName: String, config networkConfig: Network?) async throws { - let actualNetworkName = networkConfig?.name ?? networkName // Use explicit name or key as name + let actualNetworkName = Self.resolvedNetworkName( + key: networkName, + config: networkConfig, + environment: environmentVariables + ) if let externalNetwork = networkConfig?.external, externalNetwork.isExternal { print("Info: Network '\(networkName)' is declared as external.") diff --git a/Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift b/Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift new file mode 100644 index 0000000..09ea433 --- /dev/null +++ b/Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift @@ -0,0 +1,85 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Morris Richman and the Container-Compose 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 Testing +import Foundation +@testable import Yams +@testable import ContainerComposeCore + +@Suite("Top-level Network Name Interpolation") +struct NetworkNameInterpolationTests { + + private func network(_ yaml: String) throws -> Network { + try YAMLDecoder().decode(Network.self, from: yaml) + } + + @Test("an explicit name is interpolated") + func explicitNameInterpolates() throws { + let net = try network("name: ${INGRESS_NET}") + let resolved = ComposeUp.resolvedNetworkName( + key: "reverse-proxy", + config: net, + environment: ["INGRESS_NET": "prod-ingress"] + ) + #expect(resolved == "prod-ingress") + } + + @Test("a default value is used when the variable is unset") + func defaultValueIsHonoured() throws { + let net = try network("name: ${INGRESS_NET:-local-ingress}") + let resolved = ComposeUp.resolvedNetworkName( + key: "reverse-proxy", + config: net, + environment: [:] + ) + #expect(resolved == "local-ingress") + } + + @Test("the environment wins over the default") + func environmentBeatsDefault() throws { + let net = try network("name: ${INGRESS_NET:-local-ingress}") + let resolved = ComposeUp.resolvedNetworkName( + key: "reverse-proxy", + config: net, + environment: ["INGRESS_NET": "dev-cluster-ingress"] + ) + #expect(resolved == "dev-cluster-ingress") + } + + @Test("without an explicit name the key is used, and is itself interpolated") + func keyFallbackAlsoInterpolates() throws { + #expect( + ComposeUp.resolvedNetworkName(key: "dcf-db", config: nil, environment: [:]) + == "dcf-db" + ) + #expect( + ComposeUp.resolvedNetworkName( + key: "${NET_PREFIX}-db", + config: nil, + environment: ["NET_PREFIX": "dcf"] + ) == "dcf-db" + ) + } + + @Test("a plain name is passed through unchanged") + func plainNameUnchanged() throws { + let net = try network("name: dcf-vpc") + #expect( + ComposeUp.resolvedNetworkName(key: "vpc", config: net, environment: ["X": "y"]) + == "dcf-vpc" + ) + } +} From c04707f4c26fb5f7da5684b0a67497ae3f32485c Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Wed, 2 Sep 2026 06:40:51 +0000 Subject: [PATCH 2/5] fix(up): interpolate image references and label values Third and fourth instance of the same shape as the network name. This project resolves ${VAR} field by field rather than in one pass after parsing, so each field is a separate chance to forget. An audit of ComposeUp.swift found 17 call sites that resolve and these that did not: image, labels, container_name, user, platform, entrypoint, command, volumes. Measured against a real 23-service compose file, only three fields carried variables: image (19 services), labels (2) and ports (7, already resolved). The first two are fixed here; the rest are left alone rather than changed speculatively, and the audit is in the PR description so the shape is visible. Without this, `image: repo/name:${TAG:-1.0}` reaches container verbatim: Pulling Image docker.io/lldap/lldap:${LLDAP_VERSION:-2026-05-26-alpine}... Error: invalid format for image reference The test suite is renamed to Compose Value Interpolation and documents which fields are covered, so the next field is added to a list rather than to a one-off test somewhere else. It also pins a rule that was previously unguarded: resolveVariable gives the process environment precedence over the map it is handed, which is the Compose rule (a shell value beats a .env value). An earlier draft of these tests used a variable name that happened to be exported and failed only on this machine; without a test naming the rule, the obvious "fix" is to invert the merge and silently break it. --- .../Commands/ComposeUp.swift | 18 ++- .../NetworkNameInterpolationTests.swift | 85 ----------- .../ValueInterpolationTests.swift | 139 ++++++++++++++++++ 3 files changed, 151 insertions(+), 91 deletions(-) delete mode 100644 Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift create mode 100644 Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index b0b28c3..9410d96 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -841,10 +841,13 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { if let buildConfig = service.build { imageToRun = try await buildService(buildConfig, for: service, serviceName: serviceName) } else if let img = service.image { - // Use specified image if no build config - // Pull image if necessary - try await pullImage(img, platform: service.platform) - imageToRun = img + // Use specified image if no build config. + // Interpolated: pinning an image per environment with + // `image: repo/name:${TAG:-1.0}` is the common case, and container + // rejects the raw form with "invalid format for image reference". + let resolvedImage = resolveVariable(img, with: environmentVariables) + try await pullImage(resolvedImage, platform: service.platform) + imageToRun = resolvedImage } else { // Should not happen due to Service init validation, but as a fallback throw ComposeError.imageNotFound(serviceName) @@ -904,7 +907,8 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { labels["com.docker.compose.project"] = projectName labels["com.docker.compose.service"] = serviceName for key in labels.keys.sorted() { - runCommandArgs.append(contentsOf: ["--label", "\(key)=\(labels[key] ?? "")"]) + let value = resolveVariable(labels[key] ?? "", with: environmentVariables) + runCommandArgs.append(contentsOf: ["--label", "\(key)=\(value)"]) } // REMOVED: Restart policy is not supported by `container run` @@ -1339,7 +1343,9 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { /// - Returns: Image Name (`String`) private func buildService(_ buildConfig: Build, for service: Service, serviceName: String) async throws -> String { // Determine image tag for built image - let imageToRun = service.image ?? "\(serviceName):latest" + let imageToRun = service.image + .map { resolveVariable($0, with: environmentVariables) } + ?? "\(serviceName):latest" let imageList = try await ClientImage.list() if !rebuild, imageList.contains(where: { $0.description.reference.components(separatedBy: "/").last == imageToRun }) { return imageToRun diff --git a/Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift b/Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift deleted file mode 100644 index 09ea433..0000000 --- a/Tests/Container-Compose-StaticTests/NetworkNameInterpolationTests.swift +++ /dev/null @@ -1,85 +0,0 @@ -//===----------------------------------------------------------------------===// -// Copyright © 2025 Morris Richman and the Container-Compose 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 Testing -import Foundation -@testable import Yams -@testable import ContainerComposeCore - -@Suite("Top-level Network Name Interpolation") -struct NetworkNameInterpolationTests { - - private func network(_ yaml: String) throws -> Network { - try YAMLDecoder().decode(Network.self, from: yaml) - } - - @Test("an explicit name is interpolated") - func explicitNameInterpolates() throws { - let net = try network("name: ${INGRESS_NET}") - let resolved = ComposeUp.resolvedNetworkName( - key: "reverse-proxy", - config: net, - environment: ["INGRESS_NET": "prod-ingress"] - ) - #expect(resolved == "prod-ingress") - } - - @Test("a default value is used when the variable is unset") - func defaultValueIsHonoured() throws { - let net = try network("name: ${INGRESS_NET:-local-ingress}") - let resolved = ComposeUp.resolvedNetworkName( - key: "reverse-proxy", - config: net, - environment: [:] - ) - #expect(resolved == "local-ingress") - } - - @Test("the environment wins over the default") - func environmentBeatsDefault() throws { - let net = try network("name: ${INGRESS_NET:-local-ingress}") - let resolved = ComposeUp.resolvedNetworkName( - key: "reverse-proxy", - config: net, - environment: ["INGRESS_NET": "dev-cluster-ingress"] - ) - #expect(resolved == "dev-cluster-ingress") - } - - @Test("without an explicit name the key is used, and is itself interpolated") - func keyFallbackAlsoInterpolates() throws { - #expect( - ComposeUp.resolvedNetworkName(key: "dcf-db", config: nil, environment: [:]) - == "dcf-db" - ) - #expect( - ComposeUp.resolvedNetworkName( - key: "${NET_PREFIX}-db", - config: nil, - environment: ["NET_PREFIX": "dcf"] - ) == "dcf-db" - ) - } - - @Test("a plain name is passed through unchanged") - func plainNameUnchanged() throws { - let net = try network("name: dcf-vpc") - #expect( - ComposeUp.resolvedNetworkName(key: "vpc", config: net, environment: ["X": "y"]) - == "dcf-vpc" - ) - } -} diff --git a/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift b/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift new file mode 100644 index 0000000..55d3136 --- /dev/null +++ b/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift @@ -0,0 +1,139 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Morris Richman and the Container-Compose 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 Testing +import Foundation +@testable import Yams +@testable import ContainerComposeCore + +@Suite("Compose Value Interpolation") +struct NetworkNameInterpolationTests { + + /// The fields a compose file is allowed to parameterise are resolved one at a + /// time in this project rather than in a single pass after parsing, so each + /// new field is a chance to forget. This list is the record of which ones are + /// covered; extend it rather than adding a one-off test elsewhere. + /// + /// Measured against a real 23-service compose file: `image` appeared with a + /// variable in 19 services and `labels` in 2, and both reached `container` + /// verbatim before this was fixed ("invalid format for image reference"). + /// Variable names here are deliberately implausible. `resolveVariable` gives + /// the process environment precedence over the map it is handed, which is the + /// Compose rule (a shell value beats a .env value) - so a test that picks a + /// name a developer might really export becomes environment-dependent and + /// fails only on some machines. This bit the author: an earlier draft used + /// LLDAP_VERSION, which was already exported, and the override case failed + /// while the identical network-name test passed. + @Test("image references are interpolated") + func imageInterpolates() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: docker.io/lldap/lldap:${CC_TEST_IMAGE_TAG:-2026-05-26-alpine} + """) + let image = try #require(svc.image) + + let overridden = resolveVariable(image, with: ["CC_TEST_IMAGE_TAG": "2026-06-01"]) + #expect(overridden == "docker.io/lldap/lldap:2026-06-01") + + let defaulted = resolveVariable(image, with: [:]) + #expect(defaulted == "docker.io/lldap/lldap:2026-05-26-alpine") + #expect(defaulted.contains("${") == false) + } + + @Test("the process environment wins over the supplied map, as Compose specifies") + func processEnvironmentTakesPrecedence() throws { + // PATH is always set and never something a compose file would name, so it + // pins the precedence rule without depending on the developer's shell. + let fromProcess = ProcessInfo.processInfo.environment["PATH"] + let resolved = resolveVariable("${PATH}", with: ["PATH": "ignored-by-design"]) + #expect(resolved == fromProcess) + #expect(resolved != "ignored-by-design") + } + + @Test("label values are interpolated") + func labelsInterpolate() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: alpine + labels: + traefik.http.routers.app.rule: Host(`${CC_TEST_APP_HOST:-app.local}`) + """) + let raw = try #require(svc.labels?["traefik.http.routers.app.rule"]) + #expect(resolveVariable(raw, with: ["CC_TEST_APP_HOST": "app.example.com"]) + == "Host(`app.example.com`)") + #expect(resolveVariable(raw, with: [:]) == "Host(`app.local`)") + } + + + private func network(_ yaml: String) throws -> Network { + try YAMLDecoder().decode(Network.self, from: yaml) + } + + @Test("an explicit name is interpolated") + func explicitNameInterpolates() throws { + let net = try network("name: ${CC_TEST_INGRESS_NET}") + let resolved = ComposeUp.resolvedNetworkName( + key: "reverse-proxy", + config: net, + environment: ["CC_TEST_INGRESS_NET": "prod-ingress"] + ) + #expect(resolved == "prod-ingress") + } + + @Test("a default value is used when the variable is unset") + func defaultValueIsHonoured() throws { + let net = try network("name: ${CC_TEST_INGRESS_NET:-local-ingress}") + let resolved = ComposeUp.resolvedNetworkName( + key: "reverse-proxy", + config: net, + environment: [:] + ) + #expect(resolved == "local-ingress") + } + + @Test("the environment wins over the default") + func environmentBeatsDefault() throws { + let net = try network("name: ${CC_TEST_INGRESS_NET:-local-ingress}") + let resolved = ComposeUp.resolvedNetworkName( + key: "reverse-proxy", + config: net, + environment: ["CC_TEST_INGRESS_NET": "dev-cluster-ingress"] + ) + #expect(resolved == "dev-cluster-ingress") + } + + @Test("without an explicit name the key is used, and is itself interpolated") + func keyFallbackAlsoInterpolates() throws { + #expect( + ComposeUp.resolvedNetworkName(key: "dcf-db", config: nil, environment: [:]) + == "dcf-db" + ) + #expect( + ComposeUp.resolvedNetworkName( + key: "${CC_TEST_NET_PREFIX}-db", + config: nil, + environment: ["CC_TEST_NET_PREFIX": "dcf"] + ) == "dcf-db" + ) + } + + @Test("a plain name is passed through unchanged") + func plainNameUnchanged() throws { + let net = try network("name: dcf-vpc") + #expect( + ComposeUp.resolvedNetworkName(key: "vpc", config: net, environment: ["X": "y"]) + == "dcf-vpc" + ) + } +} From ee71f12e881e04de2939135473b846ab02ceddce Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Wed, 2 Sep 2026 06:46:07 +0000 Subject: [PATCH 3/5] refactor(up): give image and label mapping a test seam The previous commit added the interpolation but tested it by calling resolveVariable from the test, which passes whether or not the call site uses it - mutation-checked: removing both call sites left all 8 tests green. That is the same vacuous shape this change set exists to remove, reproduced by the author one commit after arguing against it. Both mappings are now pure functions the tests call directly. Extracting labelRunArgs also brought two documented-but-unguarded behaviours under test: the compose project/service labels overriding a user label of the same key, and the argv being sorted for determinism. --- .../Commands/ComposeUp.swift | 51 +++++++++-- .../ValueInterpolationTests.swift | 85 +++++++++++++++---- 2 files changed, 110 insertions(+), 26 deletions(-) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 9410d96..37bd6a1 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -486,6 +486,40 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { return (value, false) } + /// The image reference a service is actually pulled and run from. + /// + /// Interpolated: pinning an image per environment with + /// `image: repo/name:${TAG:-1.0}` is the common case, and container rejects + /// the raw form with "invalid format for image reference". + /// + /// Pure so the mapping is testable; the surrounding assembly has no seam. + static func resolvedImageReference(_ image: String, environment: [String: String]) -> String { + resolveVariable(image, with: environment) + } + + /// The `--label` arguments for a service, including the two Compose adds. + /// + /// The compose labels are set first so the project/service labels take + /// precedence over a user value for the same key; keys are sorted for a + /// deterministic argv. + static func labelRunArgs( + for service: Service, + serviceName: String, + projectName: String, + environment: [String: String] + ) -> [String] { + var labels = service.labels ?? [:] + labels["com.docker.compose.project"] = projectName + labels["com.docker.compose.service"] = serviceName + + var args: [String] = [] + for key in labels.keys.sorted() { + let value = resolveVariable(labels[key] ?? "", with: environment) + args.append(contentsOf: ["--label", "\(key)=\(value)"]) + } + return args + } + /// The name a top-level network is actually created under. /// /// Interpolated for the same reason the service-level network reference is: @@ -845,7 +879,7 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // Interpolated: pinning an image per environment with // `image: repo/name:${TAG:-1.0}` is the common case, and container // rejects the raw form with "invalid format for image reference". - let resolvedImage = resolveVariable(img, with: environmentVariables) + let resolvedImage = Self.resolvedImageReference(img, environment: environmentVariables) try await pullImage(resolvedImage, platform: service.platform) imageToRun = resolvedImage } else { @@ -903,13 +937,12 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // mis-groups unrelated containers that merely share a prefix). The compose labels are // set last so they take precedence over a user value for the same key; keys are sorted // for a deterministic `container run` argv. - var labels = service.labels ?? [:] - labels["com.docker.compose.project"] = projectName - labels["com.docker.compose.service"] = serviceName - for key in labels.keys.sorted() { - let value = resolveVariable(labels[key] ?? "", with: environmentVariables) - runCommandArgs.append(contentsOf: ["--label", "\(key)=\(value)"]) - } + runCommandArgs.append(contentsOf: Self.labelRunArgs( + for: service, + serviceName: serviceName, + projectName: projectName, + environment: environmentVariables + )) // REMOVED: Restart policy is not supported by `container run` // if let restart = service.restart { @@ -1344,7 +1377,7 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { private func buildService(_ buildConfig: Build, for service: Service, serviceName: String) async throws -> String { // Determine image tag for built image let imageToRun = service.image - .map { resolveVariable($0, with: environmentVariables) } + .map { Self.resolvedImageReference($0, environment: environmentVariables) } ?? "\(serviceName):latest" let imageList = try await ClientImage.list() if !rebuild, imageList.contains(where: { $0.description.reference.components(separatedBy: "/").last == imageToRun }) { diff --git a/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift b/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift index 55d3136..43d8285 100644 --- a/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift +++ b/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift @@ -44,24 +44,16 @@ struct NetworkNameInterpolationTests { """) let image = try #require(svc.image) - let overridden = resolveVariable(image, with: ["CC_TEST_IMAGE_TAG": "2026-06-01"]) - #expect(overridden == "docker.io/lldap/lldap:2026-06-01") + #expect( + ComposeUp.resolvedImageReference(image, environment: ["CC_TEST_IMAGE_TAG": "2026-06-01"]) + == "docker.io/lldap/lldap:2026-06-01" + ) - let defaulted = resolveVariable(image, with: [:]) + let defaulted = ComposeUp.resolvedImageReference(image, environment: [:]) #expect(defaulted == "docker.io/lldap/lldap:2026-05-26-alpine") #expect(defaulted.contains("${") == false) } - @Test("the process environment wins over the supplied map, as Compose specifies") - func processEnvironmentTakesPrecedence() throws { - // PATH is always set and never something a compose file would name, so it - // pins the precedence rule without depending on the developer's shell. - let fromProcess = ProcessInfo.processInfo.environment["PATH"] - let resolved = resolveVariable("${PATH}", with: ["PATH": "ignored-by-design"]) - #expect(resolved == fromProcess) - #expect(resolved != "ignored-by-design") - } - @Test("label values are interpolated") func labelsInterpolate() throws { let svc = try YAMLDecoder().decode(Service.self, from: """ @@ -69,12 +61,71 @@ struct NetworkNameInterpolationTests { labels: traefik.http.routers.app.rule: Host(`${CC_TEST_APP_HOST:-app.local}`) """) - let raw = try #require(svc.labels?["traefik.http.routers.app.rule"]) - #expect(resolveVariable(raw, with: ["CC_TEST_APP_HOST": "app.example.com"]) - == "Host(`app.example.com`)") - #expect(resolveVariable(raw, with: [:]) == "Host(`app.local`)") + let args = ComposeUp.labelRunArgs( + for: svc, + serviceName: "web", + projectName: "proj", + environment: ["CC_TEST_APP_HOST": "app.example.com"] + ) + // labelRunArgs returns a flat argv: ["--label", "key=value", ...] + #expect(args.contains("traefik.http.routers.app.rule=Host(`app.example.com`)")) + #expect(args.contains { $0.contains("${") } == false) + } + + @Test("the compose project and service labels are always emitted") + func composeLabelsAlwaysPresent() throws { + let svc = try YAMLDecoder().decode(Service.self, from: "image: alpine") + let args = ComposeUp.labelRunArgs( + for: svc, + serviceName: "web", + projectName: "proj", + environment: [:] + ) + #expect(args.contains("com.docker.compose.project=proj")) + #expect(args.contains("com.docker.compose.service=web")) } + @Test("the compose labels win over a user label of the same key") + func composeLabelsTakePrecedence() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: alpine + labels: + com.docker.compose.project: user-supplied + """) + let args = ComposeUp.labelRunArgs( + for: svc, + serviceName: "web", + projectName: "proj", + environment: [:] + ) + #expect(args.contains("com.docker.compose.project=proj")) + #expect(args.contains("com.docker.compose.project=user-supplied") == false) + } + + @Test("label argv is sorted, so the command line is deterministic") + func labelArgvIsSorted() throws { + let svc = try YAMLDecoder().decode(Service.self, from: """ + image: alpine + labels: + zzz: last + aaa: first + """) + let args = ComposeUp.labelRunArgs( + for: svc, serviceName: "web", projectName: "proj", environment: [:] + ) + let keys = stride(from: 1, to: args.count, by: 2).map { args[$0].split(separator: "=")[0] } + #expect(keys == keys.sorted()) + } + + @Test("the process environment wins over the supplied map, as Compose specifies") + func processEnvironmentTakesPrecedence() throws { + // PATH is always set and never something a compose file would name, so it + // pins the precedence rule without depending on the developer's shell. + let fromProcess = ProcessInfo.processInfo.environment["PATH"] + let resolved = resolveVariable("${PATH}", with: ["PATH": "ignored-by-design"]) + #expect(resolved == fromProcess) + #expect(resolved != "ignored-by-design") + } private func network(_ yaml: String) throws -> Network { try YAMLDecoder().decode(Network.self, from: yaml) From a30f04aa7e8d6bcdaa2d79793fc68374fb281330 Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Tue, 8 Sep 2026 17:19:43 +0000 Subject: [PATCH 4/5] fix(up): connect and inspect networks under their created name `setupNetwork` creates a top-level network under the interpolated name (`resolvedNetworkName`), but two consumers still took the declared `name:` raw: - the `--network` arg read `networks?[key]??.name` directly, so a compose file declaring `name: ${INGRESS_NET:-dev-cluster-local-ingress}` created `dev-cluster-local-ingress` and then asked `container run` to join the literal `${INGRESS_NET:-dev-cluster-local-ingress}`, which fails with "network ... not found". - the host-gateway lookup interpolated the network *key* and ignored a `name:` override, so `container network inspect` missed and it fell back to the host's LAN default-route gateway. That path only warns, so the wrong address reaches the container silently. Both now go through `resolvedNetworkName`, the same mapping the create path uses. Its doc comment already claimed the service-level reference was interpolated "for the same reason"; that is now true. --- .../Commands/ComposeUp.swift | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 37bd6a1..372ee09 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -1014,9 +1014,14 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // Connect to specified networks if let serviceNetworks = service.networks { for network in serviceNetworks { - let resolvedNetwork = resolveVariable(network, with: environmentVariables) - // Use the explicit network name from top-level definition if available, otherwise resolved name - let networkToConnect = dockerCompose.networks?[network]??.name ?? resolvedNetwork + // Same mapping `setupNetwork` uses to create the network. Taking + // `.name` raw here connected to the literal `${VAR:-default}` while + // the network had been created under the interpolated name, so + // `container run` failed with "network ... not found". + let networkToConnect = Self.resolvedNetworkName( + key: network, + config: dockerCompose.networks?[network] ?? nil, + environment: environmentVariables) runCommandArgs.append("--network") let networkTranslation = Self.networkRunArg( network: networkToConnect, @@ -1077,7 +1082,13 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // empty, silently producing "--add-host foo:" (now "foo:" in /etc/hosts). let resolvedEntries = extraHosts.map { resolveVariable($0, with: environmentVariables) } let needsGateway = resolvedEntries.contains { $0.hasSuffix(":host-gateway") } - let resolvedNetworkName = service.networks?.first.map { resolveVariable($0, with: environmentVariables) } ?? "default" + // Must be the name the network was actually created under: this feeds + // `container network inspect`, and a miss there falls back to the host's + // LAN gateway silently rather than failing. + let resolvedNetworkName = service.networks?.first.map { + Self.resolvedNetworkName( + key: $0, config: dockerCompose.networks?[$0] ?? nil, environment: environmentVariables) + } ?? "default" let hostGatewayIP = needsGateway ? Self.resolveHostGatewayIP(networkName: resolvedNetworkName) : "" var hostsFileLines = ["127.0.0.1 localhost", "::1 localhost"] From 9d38406be0fd039ca3f34df267d74cf73c2accbe Mon Sep 17 00:00:00 2001 From: Mikimoto Date: Tue, 8 Sep 2026 17:23:49 +0000 Subject: [PATCH 5/5] refactor(up): give the network args a test seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit fixed both network-name consumers, but nothing would have caught them regressing: every existing test calls `resolvedNetworkName` directly, and those stayed green for as long as the bug existed precisely because `configService` did not call it. Extract the two decisions `configService` makes into pure functions — `networkRunArgs` (every `--network` argument a service contributes) and `gatewayNetworkName` (the network whose gateway `host-gateway` means) — and test both through a decoded compose document that declares `name: ${CC_TEST_INGRESS:-fallback-ingress}`. --- .../Commands/ComposeUp.swift | 86 ++++++++++++------ .../ValueInterpolationTests.swift | 90 +++++++++++++++++++ 2 files changed, 148 insertions(+), 28 deletions(-) diff --git a/Sources/Container-Compose/Commands/ComposeUp.swift b/Sources/Container-Compose/Commands/ComposeUp.swift index 372ee09..58856b0 100644 --- a/Sources/Container-Compose/Commands/ComposeUp.swift +++ b/Sources/Container-Compose/Commands/ComposeUp.swift @@ -536,6 +536,57 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { resolveVariable(config?.name ?? networkKey, with: environment) } + /// Every `--network` argument a service contributes, plus the warnings the + /// caller should print. + /// + /// Pure so the *call site* is covered: a test that calls + /// `resolvedNetworkName` directly passes whether or not `configService` + /// routes through it, which is exactly how the raw-`.name` connect bug + /// survived. Reverting this body to `networks?[key]??.name` turns the + /// document-level test red. + static func networkRunArgs( + for service: Service, + dockerCompose: DockerCompose, + serviceName: String, + environment: [String: String], + supportsAliases: Bool = networkAliasesSupported + ) -> (args: [String], warnings: [String]) { + var args: [String] = [] + var warnings: [String] = [] + for network in service.networks ?? [] { + let networkToConnect = resolvedNetworkName( + key: network, + config: dockerCompose.networks?[network] ?? nil, + environment: environment) + let translation = networkRunArg( + network: networkToConnect, + aliases: service.networkConfigurations?[network]?.aliases ?? [], + serviceName: serviceName, + environmentVariables: environment, + supportsAliases: supportsAliases) + args.append("--network") + args.append(translation.arg) + if let warning = translation.warning { warnings.append(warning) } + } + return (args, warnings) + } + + /// The network whose gateway `host-gateway` resolves to: a service's first + /// network, under the name it was actually created with. + /// + /// Pure for the same reason as `networkRunArgs`: this feeds + /// `container network inspect`, and a miss there only warns before falling + /// back to the host's LAN default route, so getting it wrong is silent. + static func gatewayNetworkName( + for service: Service, + dockerCompose: DockerCompose, + environment: [String: String] + ) -> String { + guard let key = service.networks?.first else { return "default" } + return resolvedNetworkName( + key: key, config: dockerCompose.networks?[key] ?? nil, environment: environment) + } + static func validateStoppedServiceExitCode(_ exitCode: Int32, serviceName: String) throws { guard exitCode == 0 else { throw ComposeError.containerRunFailed(serviceName, exitCode) @@ -1013,27 +1064,11 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // Connect to specified networks if let serviceNetworks = service.networks { - for network in serviceNetworks { - // Same mapping `setupNetwork` uses to create the network. Taking - // `.name` raw here connected to the literal `${VAR:-default}` while - // the network had been created under the interpolated name, so - // `container run` failed with "network ... not found". - let networkToConnect = Self.resolvedNetworkName( - key: network, - config: dockerCompose.networks?[network] ?? nil, - environment: environmentVariables) - runCommandArgs.append("--network") - let networkTranslation = Self.networkRunArg( - network: networkToConnect, - aliases: service.networkConfigurations?[network]?.aliases ?? [], - serviceName: serviceName, - environmentVariables: environmentVariables - ) - runCommandArgs.append(networkTranslation.arg) - if let warning = networkTranslation.warning { - print(warning) - } - } + let networkArgs = Self.networkRunArgs( + for: service, dockerCompose: dockerCompose, + serviceName: serviceName, environment: environmentVariables) + runCommandArgs.append(contentsOf: networkArgs.args) + for warning in networkArgs.warnings { print(warning) } print( "Info: Service '\(serviceName)' is configured to connect to networks: \(serviceNetworks.joined(separator: ", ")) ascertained from networks attribute in \(composePath)." ) @@ -1082,13 +1117,8 @@ public struct ComposeUp: AsyncParsableCommand, @unchecked Sendable { // empty, silently producing "--add-host foo:" (now "foo:" in /etc/hosts). let resolvedEntries = extraHosts.map { resolveVariable($0, with: environmentVariables) } let needsGateway = resolvedEntries.contains { $0.hasSuffix(":host-gateway") } - // Must be the name the network was actually created under: this feeds - // `container network inspect`, and a miss there falls back to the host's - // LAN gateway silently rather than failing. - let resolvedNetworkName = service.networks?.first.map { - Self.resolvedNetworkName( - key: $0, config: dockerCompose.networks?[$0] ?? nil, environment: environmentVariables) - } ?? "default" + let resolvedNetworkName = Self.gatewayNetworkName( + for: service, dockerCompose: dockerCompose, environment: environmentVariables) let hostGatewayIP = needsGateway ? Self.resolveHostGatewayIP(networkName: resolvedNetworkName) : "" var hostsFileLines = ["127.0.0.1 localhost", "::1 localhost"] diff --git a/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift b/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift index 43d8285..ae82e9c 100644 --- a/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift +++ b/Tests/Container-Compose-StaticTests/ValueInterpolationTests.swift @@ -188,3 +188,93 @@ struct NetworkNameInterpolationTests { ) } } + +/// The suite above pins `resolvedNetworkName` itself. These pin its *consumers*. +/// +/// Both consumers used to read the declared `name:` raw, so a compose file +/// saying `name: ${VAR:-default}` had its network created under the resolved +/// name and then asked `container run` to join the literal — and the +/// host-gateway lookup to inspect it. Every test above stayed green throughout, +/// because none of them went through the code `configService` actually calls. +@Suite("Network Args Through A Compose Document") +struct NetworkRunArgsTests { + + private static let yaml = """ + services: + web: + image: nginx + networks: + - ingress + - plain + loner: + image: nginx + networks: + ingress: + name: ${CC_TEST_INGRESS:-fallback-ingress} + plain: {} + """ + + private func compose() throws -> DockerCompose { + try YAMLDecoder().decode(DockerCompose.self, from: Self.yaml) + } + + private func service(_ name: String) throws -> Service { + try #require(try compose().services[name] ?? nil) + } + + /// `supportsAliases` is passed explicitly everywhere here: its default is + /// probed from the installed `container` CLI, which would make these depend + /// on the host. + @Test("the --network arg carries the created name, not the declared literal") + func connectsUnderCreatedName() throws { + let args = ComposeUp.networkRunArgs( + for: try service("web"), dockerCompose: try compose(), + serviceName: "web", environment: [:], supportsAliases: false + ).args + #expect(args == ["--network", "fallback-ingress", "--network", "plain"]) + } + + @Test("the environment wins over the declared default") + func environmentBeatsDefault() throws { + let args = ComposeUp.networkRunArgs( + for: try service("web"), dockerCompose: try compose(), + serviceName: "web", environment: ["CC_TEST_INGRESS": "prod-ingress"], + supportsAliases: false + ).args + #expect(args == ["--network", "prod-ingress", "--network", "plain"]) + } + + @Test("no placeholder ever reaches the command line") + func noPlaceholderSurvives() throws { + let args = ComposeUp.networkRunArgs( + for: try service("web"), dockerCompose: try compose(), + serviceName: "web", environment: [:], supportsAliases: false + ).args + #expect(!args.contains { $0.contains("${") }) + } + + @Test("a service with no networks contributes no args") + func noNetworksNoArgs() throws { + let result = ComposeUp.networkRunArgs( + for: try service("loner"), dockerCompose: try compose(), + serviceName: "loner", environment: [:], supportsAliases: false) + #expect(result.args.isEmpty) + #expect(result.warnings.isEmpty) + } + + @Test("the host-gateway lookup uses the created name of the first network") + func gatewayUsesCreatedName() throws { + #expect( + ComposeUp.gatewayNetworkName( + for: try service("web"), dockerCompose: try compose(), + environment: ["CC_TEST_INGRESS": "prod-ingress"]) == "prod-ingress") + } + + @Test("a service with no networks falls back to the default network") + func gatewayFallsBackToDefault() throws { + #expect( + ComposeUp.gatewayNetworkName( + for: try service("loner"), dockerCompose: try compose(), environment: [:]) + == "default") + } +}