fix: allow disabled services as build additional contexts - #931
Conversation
Validate build-only dependencies without enabling their profiles. Reproduces docker/compose#14223 through the loader and preserves missing and non-buildable target errors. Assisted-by: OpenAI Codex Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>
ndeloof
left a comment
ndeloof
left a comment
There was a problem hiding this comment.
Fix is correct and minimal — mirrors the tolerance the depends_on check already has a few lines below (errors.Is(err, errdefs.ErrDisabled)), and the loader-level regression test correctly locks in all three outcomes (buildable / image-only / missing) plus p.DisabledServices["base"].Build != nil — i.e. the service stays disabled at the project level, which is the right scope for this fix.
I traced the follow-on question this raises: once a disabled service is reachable this way, does re-enabling it for the build risk it also getting started? On docker/compose's side (pkg/compose/build.go), s.build calls project.WithServicesEnabled(...) — but that method deepCopy()s before mutating (per its own doc comment, "returns a new Project instance ... keeps the original unchanged"), and s.build only ever returns a digest map, never the re-enabled project, back to its caller (ensureImagesExists → preparePlan → create/up). So the enabled-for-build copy is scoped to the build step and never reaches the reconciler that decides what to create/start — base gets built, not started. That safety property is real today, but it's an emergent consequence of a narrow return type + immutable-Project convention holding together, not something asserted by a test anywhere. Worth a docker/compose-side e2e case (once this dependency bumps) pinning "disabled build-context service is built but never started" — not a blocker for this PR, since it's out of loader scope, but flagging so it doesn't get silently relied upon forever.
…d service A service reachable only as another service's build additional_contexts must still build even when its own profile is inactive: it's referenced for its image, not started as a workload -- the same tolerance the depends_on consistency check already grants a disabled, non-required dependency, and the same case addBuildDependencies already special-cases on the docker/compose side. compose-go's own consistency check doesn't grant that tolerance yet: it rejects the reference outright, failing the whole project load before addBuildDependencies (or anything else, up included) ever runs -- for up, build and up --no-build alike, exactly as reported. This is expected to fail until compose-go bumps to include compose-spec/compose-go#931. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
compose-spec/compose-go#931 is merged: TestBuildAdditionalContextDisabledService, red until now by design, passes for real against the released compose-go pin. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A service reachable only as another service's build additional_contexts must still build even when its own profile is inactive: it's referenced for its image, not started as a workload -- the same tolerance the depends_on consistency check already grants a disabled, non-required dependency, and the same case addBuildDependencies already special-cases on the docker/compose side. compose-go's own consistency check doesn't grant that tolerance yet: it rejects the reference outright, failing the whole project load before addBuildDependencies (or anything else, up included) ever runs -- for up, build and up --no-build alike, exactly as reported. This is expected to fail until compose-go bumps to include compose-spec/compose-go#931. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
compose-spec/compose-go#931 is merged: TestBuildAdditionalContextDisabledService, red until now by design, passes for real against the released compose-go pin. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
2 participants
A service used only as a build additional context can have an inactive profile. The consistency check currently rejects that service as unknown, before Docker Compose can include it in the build graph. This reproduces docker/compose#14223.
Look up disabled services for this build-context check while leaving their profile state unchanged. Missing targets still report an unknown service, and image-only targets still report a non-buildable service. The regression exercises the full loader with profile filtering and consistency checks enabled and verifies that only classroom is active.
Validation:
go test ./...passes on Linux, Go 1.24.13 and 1.26.3.Repository-pinned golangci-lint 2.1.6 also passes (0 issues, Go 1.24.13).
No Docker daemon/image-build test was run. This addresses the loader rejection; Docker Compose needs to consume the updated dependency.
Prepared with OpenAI Codex assistance.