Skip to content

test(e2e): reproduce #14223 -- additional_contexts to a disabled service - #14242

Draft
ndeloof wants to merge 2 commits into
docker:mainfrom
ndeloof:14223-additional-contexts-disabled-service
Draft

ndeloof wants to merge 2 commits into
docker:mainfrom
ndeloof:14223-additional-contexts-disabled-service

Conversation

@ndeloof

@ndeloof ndeloof commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Draft: adds a regression test for #14223, expected to fail until compose-go bumps to include the fix.

What this PR does, in one sentence

Locks in, as an e2e scenario, that a service reachable only through another service's build.additional_contexts must build even while its own profile is inactive — and must never start.

Context

Reported in #14223: a build-only base service with an inactive profile, referenced by classroom via additional_contexts: base: service:base, makes the whole project fail to load — up, build and up --no-build alike — with service "classroom" declares unknown service "base" as additional contexts base.

The root cause lives entirely in compose-go's own consistency check (loader/validate.go): it validates additional_contexts targets via GetService, which errors on a disabled service, and — unlike the depends_on check a few lines below, which already tolerates a disabled, non-required dependency — has no such tolerance for additional_contexts. This blocks the project at load time, before docker/compose's own addBuildDependencies (pkg/compose/build.go) ever gets a chance to run — and that function already special-cases exactly this: it walks additional_contexts into DisabledServices and re-enables what it finds, scoped to the build step only (see WithServicesEnabled, called on a deepCopy, with the result only ever surfaced as a digest map — the enabled-for-build project never reaches the code that decides what to create or start). In other words, docker/compose already does the right thing here; it just never gets the chance to.

A fix for the compose-go side is up at compose-spec/compose-go#931.

What the PR brings

  • TestBuildAdditionalContextDisabledService (pkg/e2e/build_test.go): a classroom service (active profile) building FROM base via additional_contexts, base (inactive profile) providing only a Build. Asserts classroom reaches running, base's image gets built, and base itself is never created.
  • Left as a draft, failing today at the exact reported error, until docker/compose's compose-go pin bumps past Fig could pull and build images in parallel to speed things up. #931.

🤖 Generated with Claude Code

…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>

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

⚠️ Verification inconclusive: The verifier returned a malformed response (duplicate finding_id in the verdicts array), so findings below are surfaced unverified from the drafter. Please evaluate them manually.

Unverified findings (verification inconclusive)

  • [medium] pkg/e2e/build_test.go:619 — up -d without --build may not exercise the build path the regression test targets

    The step uses ComposeCmd("--profile", "classroom", "up", "-d") without the --build flag. Docker Compose only builds an image during up if no local image already exists for that service. If e2e-additional-context-base:latest or e2e-additional-context-classroom:latest are present in the local daemon (e.g. leaked from another test, or from a prior run whose Defer cleanup raced), up will skip the build entirely — the ImageExists check passes trivially, and the regression (which lives in the build path for additional_contexts targeting a disabled-profile service) is never actually exercised. Every other build-focused test in this file that verifies building behaviour uses ComposeCmd("up", "--build", ...) or a separate build step. Fix: add "--build" to the up invocation so the build path is always triggered.

  • [low] pkg/e2e/testdata/TestBuildAdditionalContextDisabledService/compose.yaml:20 — classroom service relies solely on ENTRYPOINT in dockerfile_inline; no command at compose level

    The classroom service sets ENTRYPOINT ["sleep", "infinity"] in the inline Dockerfile. While ENTRYPOINT is reliable for Alpine, a command: sleep infinity in the compose file would be more explicit. This is low severity and likely not a real bug.

docker-agent review: without --build, an image tagged
e2e-additional-context-base:latest left over from a previous run would
let up skip the build entirely, passing the test without exercising
the additional_contexts-to-a-disabled-service path it exists to lock
in.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof

ndeloof commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in f29bd3d: added --build so a stale image from a previous run cannot let this step skip the build path the test exists to exercise.

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

This review covers only the commits pushed since b22e94e.

The single change — adding --build to compose up -d in TestBuildAdditionalContextDisabledService — is correct and well-motivated. Without it, a stale image from a previous run could silently bypass the build step the test exists to exercise. The flag, its placement in the command, and the accompanying comment are all accurate. No bugs introduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants