The log journal of a container is shared by all its runs, and a followed
log stream has no upper bound: it ends with the run in progress when it
opened. After a restart, compose opens a stream starting at the end of
the previous run (d834777e1), but a stream opened late, once the
container has restarted again (the restart delay is only 100ms, and the
event monitor or the daemon can be slower), also returns the next run's
lines, which that run's own stream then returns a second time.
The streams of one container now run one at a time, in start-event
order, and each starts right after the last line the previous ones
relayed. Line timestamps are the daemon's own, so no client clock is
involved. A stream opened in time still starts at the previous run's
end, as before. TTY containers, whose stream is not multiplexed, are
unchanged.
Signed-off-by: Nicolas De Loof <[email protected]>
When the inspection of an optional (required: false) dependency fails, the
dependency is reported as skipped and the wait succeeds. That also happened
when the failure came from the wait's context being done: a cancellation
was reported as a skipped dependency, and an expired deadline (the wait
timeout, or one inherited from the caller) was turned into a successful
wait, unlike the same deadline expiring between two polls.
A check failing while its context is done now reports "not done yet" for an
optional dependency, so that waitDependency decides what the end of the wait
means. Genuine inspection errors still skip the dependency, and a required
dependency is handled as before.
Signed-off-by: Nicolas De Loof <[email protected]>
The bridge commands never go through the dry-run backend, so the
inherited --dry-run flag was silently ignored: `bridge convert` still
wiped and rewrote the output directory, pulled the transformer image
and ran it, and `bridge transformations create` still wrote its
destination folder.
Fail fast with an explicit error before any side effect instead of
pretending to honor the flag.
Signed-off-by: Guillaume Lours <[email protected]>
Docker Desktop stores Docker Hardened Images credentials under https://dhi.io, so return that as the server address of dhi.io credentials, as StagingIndexServer does for the staging registry.
Signed-off-by: Alano Terblanche <[email protected]>
Docker Hardened Images on dhi.io authenticate with Docker Hub accounts (the registry advertises service=registry.docker.io), so resolve dhi.io credentials from the production Docker Hub session as well, with dhi.io as the server address. Other registries, including look-alikes of dhi.io, still use the Docker CLI credentials.
Signed-off-by: Alano Terblanche <[email protected]>
Rename isExpectedMiss to isSessionUnavailable, after what it reports, and shorten or drop doc comments in the Docker Desktop credentials provider.
Signed-off-by: Alano Terblanche <[email protected]>
A lookup that found no usable session disabled Desktop credentials for the rest of the process. That is right within one command, where retrying a hung engine would add the lookup timeout to every image, but the sessions are process wide and pkg/compose is also used as a library: in a long-running process a transient timeout, or signing in to Desktop later, was never picked up. Hold lookups off for a minute instead of for good.
Also name the expected-miss classification and test it, including dockerhub.ErrNoDefaultProfile, which wraps dockerhub.ErrNoSession.
Signed-off-by: Alano Terblanche <[email protected]>
It covered the resolver credential mapping and containerd grant selection, neither changed by this PR. The token-as-password contract is asserted on the provider output and on the encoded X-Registry-Auth in internal/registry.
Signed-off-by: Alano Terblanche <[email protected]>
Shorten the doc comments added for the Docker Desktop credentials provider to what the code does not already say, and drop them from the tests.
Signed-off-by: Alano Terblanche <[email protected]>
Docker Desktop keeps the Docker Hub staging session in separate secrets engine realms, read with dockerhub.Staging(). The environment is picked per registry host rather than with a global stage flag: the engine resolves docker.io to the production registry even when Desktop runs in stage mode, so the staging session is only used for registry-1-stage.docker.io and the production session only for the production Hub hosts. A token is never sent to the registry of the other environment.
Each environment has its own process-wide session and cache, and falls back to the Docker CLI config file independently. Staging credentials use https://registry-1-stage.docker.io/ as server address.
Signed-off-by: Alano Terblanche <[email protected]>
The session payload may carry no claims, in which case the access token was reused for a minute without any expiry check, so an expired token could reach the registry and fail the pull instead of falling back. Read the exp claim from the JWT itself in that case. An expiry that cannot be read is still tolerated.
Signed-off-by: Alano Terblanche <[email protected]>
Docker Hub credentials are now read first from the session of the account signed in to Docker Desktop, through the Docker Desktop secrets engine (github.com/docker/secrets-engine/client), and from the Docker CLI config file and its credential helpers otherwise. Other registries keep using the config file only.
The OAuth access token is passed as the password of the signed-in account, never as an IdentityToken, which containerd sends as a refresh token. It flows unchanged to the engine in X-Registry-Auth and through the OCI resolver as an OAuth password grant.
Compose falls back to the config file when Desktop is not running, nobody is signed in, the session is expired or about to expire, or the lookup fails (the last logged as a warning). The session is shared process wide, cached until shortly before it expires, and an unusable session is not asked for again. Lookups are bounded by a 5s timeout, as the SDK applies none by default.
Used by pull, push, publish, config --resolve-image-digests, oci:// remote resources and dry-run. The classic builder and use_api_socket keep config file credentials, as a short-lived token copied into a container would go stale.
Signed-off-by: Alano Terblanche <[email protected]>
Two independent reviews (Copilot CLI + a fresh-context agent) of the
previous commit converged on the same finding: execStartContainer's,
execRunPreStart's, and execRunPostStart's ctx.Err() guards still
claimed the TOCTOU window "cannot be closed" and deferred the real fix
to #14081 -- stale now that run()'s failedDependency check closes it
for a direct dependency before executeNode is ever reached. Reworded
to say what's actually still open: defense in depth for ctx canceled
by something other than a direct dependency (an unrelated branch of
the same plan, or an external cancellation).
Also: recordFailure stored the already-wrapped "dependency failed: %w"
error, compounding the prefix at every hop of a multi-node chain.
Store the original error instead, wrapping only the value returned
from this node's own goroutine.
And a stale reference to a test name that was never the one actually
added (TestExecutePlanFailedPreStartGatesStart, not ...GroupDoesNotRestart).
Signed-off-by: Nicolas De Loof <[email protected]>
Two review findings on the same root cause:
- docker-agent: a failing OpRunPreStart emitted a bare Error with no
preceding Working/Starting event, since triggersWorking only fires
on OpStartContainer. onNodeError now emits the group's Working event
first when it hasn't fired yet, keeping the progression well-formed.
- glours (following up on the docker-agent thread): worse than a
missing event -- OpStartContainer's own goroutine was unblocked by
<-done[dep.ID] and ran anyway, since close(done[node.ID]) races
ahead of errgroup's cancel() (same TOCTOU class as the ctx.Err()
guards already tracked under #14081, but deterministic here with the
default unset --parallel limiter: no semaphore wait gives cancel()
time to land first). It fired a second, spurious Starting right
after the sibling's Error, with no further event ever closing it.
Reproduced 143/143 iterations against this PR's own regression test.
run() now records each node's terminal error in a shared, mutex-guarded
map as it closes done[node.ID], and a dependent checks that map right
after its dependency wait, treating a genuine dependency failure
exactly like a context cancellation: no slot acquired, no event
emitted, its own failure recorded and propagated to its own
dependents the same way. This closes the event-ordering race outright
for this specific chain, rather than narrowing it like the existing
ctx.Err() guards -- those stay in place and still guard actual engine
calls, unrelated to event emission.
TestExecutePlanFailedPreStartGatesStart now asserts the full event
sequence (Creating, Created, Starting, Error — nothing after),
verified by reverting the executor.go fix locally and confirming the
test catches the reintroduced spurious Starting.
Signed-off-by: Nicolas De Loof <[email protected]>
Same interim narrowing as execStartContainer and execRunPostStart: the
node hangs off the replica's create node, so when that create fails the
pre_start hooks could still fire in the window before errgroup cancels
the context. The structural fix stays tracked in #14081.
Signed-off-by: Nicolas De Loof <[email protected]>
docker-agent's review of the previous commit's fix correctly pointed
out that the ctx.Err() guard added to execStartContainer/execRunPostStart
narrows the race it targets but does not eliminate it: close(done[...])
still runs inside the failing node's own goroutine, strictly before
errgroup's cancel() on that goroutine's returned error. A dependent
unblocked from <-done[dep.ID] can observe ctx.Err() == nil for a brief
window even though its dependency just failed for a genuine reason.
This is exactly the structural gap the epic (#14081) already flagged
in its "failed-dependency semantics" comment before this PR existed:
the real fix carries each node's success/failure through what
dependents wait on (e.g. a chan error) instead of inferring it from
ctx.Err(), uniformly across every operation type - including the
already-merged execCreateHookContainer, which has the same limit.
That is a dedicated brick of the executor lot, not squeezed into this
PR's diff.
No behavior change: this documents the residual window explicitly at
both guard sites and in the regression test, so the next reader (and
the next PR) doesn't mistake "narrows" for "closes".
Signed-off-by: Nicolas De Loof <[email protected]>
A node's done-channel closes strictly before errgroup cancels its
derived context on that same error - the close happens synchronously
inside the failing node's goroutine, ctx cancellation only after it
returns. A dependent blocked in run()'s
select { case <-done[dep.ID]: ...; case <-ctx.Done(): ... } therefore
always sees its dependency's done-channel ready first, regardless of
whether that dependency failed for a genuine reason or was cancelled.
execCreateHookContainer already guarded against exactly this. The two
other new dispatch functions from the previous commit did not:
execStartContainer's enriched branch could call ContainerStart right
after its OpRunPreStart failed for real (e.g. "no hook runner
container found"), contradicting pre_start.go's documented guarantee
that a non-zero hook exit gates service start - and symmetrically for
execRunPostStart after a failed OpStartContainer.
Both now bail out on ctx.Err() first, mirroring the existing guard.
Pinned by a regression test reproducing the race deterministically: no
ContainerStart expectation is registered, so the unguarded code calls
it from inside the errgroup worker goroutine, where gomock's missing-
expectation failure hangs the test binary instead of failing it
cleanly - a stronger tell than a plain assertion failure would have
been.
Signed-off-by: Nicolas De Loof <[email protected]>
Wire the plan executor to the start-phase vocabulary the reconciler
already produces (#14200): OpWaitCondition, OpRunPreStart, the enriched
OpStartContainer, and OpRunPostStart all had no case in executeNode
and hit "unknown operation type".
Every operation reuses the exact imperative primitive it mirrors —
waitDependency, runPreStart, runHook, injectSecrets/injectConfigs — so
behavior and events cannot drift between the two engines: execWaitCondition
resolves the dependency's live containers and hands off to the same
polling/condition-check functions waitDependencies uses; the enriched
OpStartContainer resolves its target either from the observed container
or, for a replica the plan itself creates, from the CreateContainer
node's result (the mechanism OpRenameContainer already relies on), then
injects secrets/configs before ContainerStart; OpRunPreStart re-checks
the daemon first so a replica that started in the observe-to-execute
window skips a redundant hook run.
A replica's pre_start/start/post_start chain shares one event group,
reporting a single Starting→Started progression exactly like
startServiceContainer does today — Starting fires specifically at
StartContainer, not at the (silent) pre_start hooks preceding it.
pkg/api/event.go documents this per-resource progression contract.
Still inert in production: create() (executePlan's only caller never
builds a plan with start-phase scope, so none of this runs outside
tests until a later brick in epic #14081 wires a real caller.
EOF
)
Signed-off-by: Nicolas De Loof <[email protected]>
down() validates options.Images upfront, but ImagePruner is exported
and can be driven directly with a hand-built ImagePruneOptions that
bypasses that check. Add a direct test for the Valid() guard added in
the previous commit, so the two checks can't silently drift without a
test catching it.
Signed-off-by: Nicolas De Loof <[email protected]>
The legal values of DownOptions.Images lived in pkg/compose
(image_pruner.go) while the field itself was a bare string in pkg/api,
and the only validation fired inside ImagesToPrune — after down had
already removed the project's containers, leaving the teardown half
done on a typo. The CLI validated its --rmi flag separately, so only
SDK callers were exposed.
ImagePruneMode and its three values now live in pkg/api next to the
field they constrain (type-aliased in pkg/compose for existing
consumers), down() rejects any other value before touching a single
resource, and the CLI check reuses the same definition. A unit test
runs Down with expectation-free mocks: one daemon call would fail it.
Epic #14074, B.6.
Signed-off-by: Nicolas De Loof <[email protected]>
docker-agent found that streamImageTo returned a plain error (channel
intact) when the announce write itself failed -- for the success
announce, and for the error announce on a platform-parse/ImageSave
failure. In both cases the provider never received anything to read
and was left blocked on ReadString, while the caller, seeing a plain
error, never closed stdin either -- a deadlock.
Both paths now return streamAbortedError when the announce write
fails, matching the existing mid-stream/close failure handling so the
caller's errors.As check closes stdin and unblocks the provider.
Pinned by two new regression tests using a writer that always errors,
verified by reverting the fix locally and confirming both fail with a
plain error instead of streamAbortedError.
Signed-off-by: Nicolas De Loof <[email protected]>
Three fixes from glours' review:
- ensureProviderImages now skips a provider service declaring neither
image: nor build: (legal for a pure resource provider, e.g. one
backing a remote Amazon RDS instance). Without the guard, such a
service reached pull with a fabricated "<project>-<service>"
reference (GetImageNameOrDefault) under source=registry
(providerImageSource treats Build == nil as registry-authoritative)
-- a name that was never real. Already reachable through existing
e2e fixtures (TestDependsOnMultipleProviders' provider1/provider2),
only a no-op today because the demo pull() short-circuits on an
unset env var, not because of any guard here.
- localImageFacts now resolves --digest through the manifest-aware
localContentDigest instead of the raw ImageInspect ID. Under
BuildKit provenance attestations, the raw ID covers the attestation
manifest too and changes on every build even when the runnable image
content didn't (#13636), which would make the provider re-sync on
every up for no reason -- defeating docs/extension.md's own
"identity test" contract for --digest. Threads service.Platform
through from runProviderPull so the resolved manifest matches what
the provider will actually run.
- streamImageTo now distinguishes a failure before the success
announce (invalid platform, ImageSave error -- a clean, well-formed
error line, channel intact) from one after it (the chunked body
truncated, channel unresynchronizable) via a new streamAbortedError
type. Only the latter closes stdin; the provider protocol supports
several sequential get-image requests per pull invocation, and
closing stdin on a clean pre-stream error was killing any other
independent request in the same invocation for no reason.
Each fix is pinned by a regression test, verified by reverting the
fix locally and confirming the new test catches it.
Signed-off-by: Nicolas De Loof <[email protected]>
A service declaring only build: (no image:) answered get-service-config
with an empty "image" field: service.Image is the YAML-declared value,
and compose-go never fills it in for this case (every other caller of
GetImageNameOrDefault in this package exists precisely because of that).
The provider only ever sees this response, never the model compose
builds internally, so an sbx-style provider building its own runtime
image rejected the service outright with "defines no image".
The response now carries the resolved name (api.GetImageNameOrDefault)
instead - the same one the image phase already built and tagged by the
time a provider asks. The build directive itself is cleared before
marshaling: a provider has no builder to run it against, only an image
identity to run.
Pinned by a regression test using the existing fake-provider harness,
verified by ablation to fail on either half of the fix reverted.
Signed-off-by: Nicolas De Loof <[email protected]>
extension.md gains the Image distribution section: invocation contract
(--image/--digest/--created/--source/--policy), the get-image request
and its chunked image-stream answer (zero-chunk terminated, stdin
exclusivity and drain-before-next-answer contract), the two-branch
decision guidance (digest as identity, created as ordering fallback,
reproducible builds caveat), and the support-by-presence metadata
convention is now stated as the general rule. The example provider
implements pull end to end — request, stock chunked reader, hash
recorded at PROVIDER_PULL_MARKER — and the e2e scenario locks the
whole path: up builds the service image and streams it to the provider
under the local verdict, compose pull re-invokes it under the
freshness contract.
Signed-off-by: Nicolas De Loof <[email protected]>
startService errored with 'no container to start' whenever the
project-wide container listing came back empty, before the per-service
filtering that lets provider services pass through. A project whose
services are all provider-backed legitimately reaches the start phase
with zero containers. The empty-list error is now softened for
provider services only — a relay container deployed for published
endpoints still goes through the regular start path.
Signed-off-by: Nicolas De Loof <[email protected]>
ensureImagesExists and docker compose pull now give provider-backed
services their turn: compose invokes the provider's pull command with
the image identity and the state of the local daemon cache
(--image/--digest/--created), plus two verdicts compose alone can
compute so providers never re-implement that arbitration:
- --source: the authority for this invocation — local (the daemon's
image is the desired state: build-only service, pull_policy build,
or an image the current run just built) or registry (resolve the
reference upstream — including the CI workflow where build is only
the recipe used to publish the image consumers pull);
- --policy: missing on the up path (a usable version suffices) or
always on compose pull (ensure freshness of the authority).
digest/created describe the cache, they are not instructions: the
provider persists them as the bookkeeping keys of what it ingested —
digest as identity test, created as the ordering fallback for backends
that cannot preserve digests. Providers without the metadata block are
skipped: behavior unchanged.
Providers are independent, so their pulls run without fail-fast
cancellation — the first failure no longer kills its siblings mid-run
and mangles their reports into context-canceled noise; every provider
runs to completion and every failure is reported, prefixed by service.
On the pull path the provider phase runs unconditionally — skipping it
because an unrelated service failed to pull would silently leave
provider runtimes stale — and its error is a per-service pull error,
following their exact regime: reported with them, suppressed by
IgnoreFailures. A fatal errgroup error is reported joined with the
accumulated per-service context instead of dropping it. The provider
runs under the root context: the errgroup context is canceled once
Wait returns and would kill the provider process on the spot.
Signed-off-by: Nicolas De Loof <[email protected]>
Providers opt into image distribution by declaring a pull block in
their metadata — the presence of the command block is the declaration
of support, generalizing the stop precedent. During its pull command a
provider can request the service image from the local daemon with a
get-image message (optionally platform-narrowed); compose answers on
stdin with one image-stream JSON line then the tar as HTTP/1.1 chunked
data (RFC 9112): length-prefixed blocks need no in-band delimiter in
binary data, the zero-length chunk marks a COMPLETE transfer, and any
stock chunked reader consumes it. There is deliberately no trailer nor
final CRLF after the zero chunk — a stock reader stops there without
consuming further bytes, so the next stdin answer starts clean for a
provider requesting several images. An export failure is announced
in-band through the error field so the provider is never left waiting;
a failure after the announce closes the answer channel so the
truncation is observable as EOF mid-chunk instead of a stream nobody
will finish. The stream is exclusive on stdin for its whole duration —
the framing invariant behind the lock.
get-image is scoped to the pull command: emitted during up, down or
stop it is a protocol error rather than served, so an image export can
never stall another lifecycle command.
Message dispatch moves out of executePlugin into handlePluginMessage,
which also keeps the function under the complexity threshold.
Signed-off-by: Nicolas De Loof <[email protected]>
Binding the announced gateway alone restricts by destination address, not
by the interface traffic arrives on, so another bridge can still reach it
by IP. A provider that pins the socket to the relay-link bridge's own
interface (SO_BINDTODEVICE on Linux) keeps the endpoint relay-only.
Signed-off-by: Nicolas De Loof <[email protected]>
Unify the up and down paths on one removal helper that tolerates both
NotFound and Conflict and emits the same progress events as removeNetwork,
build one down op per relay-link network gated on the shared limiter (so
the empty-ops warning is reachable again), and share the label filter.
Reword the isolation claim: the relay-link network is joined by nothing
else, but its gateway is a host address that local containers can still
reach by IP; only a provider binding exclusively that address keeps the
endpoint off loopback and LAN-facing addresses.
Signed-off-by: Nicolas De Loof <[email protected]>
get-relay-info answers pointed a locally-bound provider at one of the
dependents' own networks: shared infrastructure every other service on
it could also reach, and no more isolated than any project bridge.
Provision a dedicated, internal:true bridge network scoped to that one
provider service instead, created only when the provider itself sends
get-relay-info -- the provider's own declaration that it binds locally,
never for a provider backing a remote resource. The relay container
joins it alongside the dependents' networks it already joins; it is
torn down with the relay on endpoint changes and on down, including
the no-compose-file down path.
The network's lifecycle is hardened against the races a shared,
project-scoped resource runs into:
- a NetworkCreate conflict on the deterministic name re-checks by
label (not just by name) before adopting the existing network, so a
genuine name clash with an unrelated, user-declared network fails
loudly instead of being silently adopted into the relay's isolation
boundary;
- a NotFound gateway lookup after losing that conflict falls back to
an unresolved answer instead of propagating the error;
- removeRelayLinkNetwork and ensureRelayLinkNetworksDown both tolerate
a NetworkRemove conflict from the daemon's asynchronous endpoint
disconnect, which can race the relay container's own removal;
- ensureRelayLinkNetworksDown accumulates failures with errors.Join
across every relay-link network of a project instead of aborting on
the first one, and skips (warn-only) a network that still has active
endpoints, mirroring removeNetwork's existing tolerance for regular
project networks.
docs/extension.md documents that the "desktop" network name in the
get-relay-info answer (under Docker Desktop) is an opaque placeholder,
never a real Docker network a provider can inspect or connect to.
Signed-off-by: Nicolas De Loof <[email protected]>
compose-go's loader now silently renames both deprecated
x-initialSync and x-develop extensions to their official
initial_sync and develop attributes before compose ever sees them
as extensions, so the legacy lookups and deprecation warnings for
both can no longer be reached. That's why the e2e test asserting on
the x-initialSync warning broke under the bump.
Simplify both lookups to rely on the promoted fields directly, and
rewrite the e2e test to check the legacy attribute still triggers a
real initial sync instead of asserting on an unreachable message.
Signed-off-by: Guillaume Lours <[email protected]>
down's outer ops dispatch bounded only how many ops ran concurrently,
separate from the semaphore already shared by image removal -- a
single network plus --rmi could spend up to 2x the configured
budget. Network and volume removal now share that same semaphore.
up's post-attach log streaming called ContainerInspect directly and
passed no limiter to doLogContainer, unlike `compose logs --follow`'s
equivalent path, so it never shared --parallel's budget either.
The plan executor also held a node's slot for its whole dependency
wait instead of just its own work, and internal/sync/tar.go's file
sync used a hardcoded cap of 16 instead of --parallel.
Signed-off-by: Guillaume Lours <[email protected]>
Co-Authored-By: Claude Sonnet 5 <[email protected]>
`docker compose cp` fanned out to every matched container through a
bare unbounded errgroup, ignoring --parallel entirely -- the same
class of gap already closed everywhere else in this package. Routed
it through the existing forEachContainerConcurrent helper instead of
a bespoke fan-out.
inspectWithSlot and doLogContainer both release a semaphore slot
early via an explicit call rather than a defer, so a panic between
acquire and that release leaked the slot forever. Guarded both with
a panic-safe release, scoped tightly in doLogContainer so a later
panic in the same function can't trigger a double release.
Signed-off-by: Guillaume Lours <[email protected]>
Co-Authored-By: Claude Sonnet 5 <[email protected]>
down and stop rebuilt their per-container limiter on every service
visit, so independent services could each spend a full budget
concurrently -- the same leak already fixed on restart. start's
InDependencyOrder call had no concurrency option, leaving every
`up`'s container-start phase fully unbounded. wait.go goes back to
unbounded on purpose: ContainerWait blocks per container
indefinitely, so capping it would serialize waits meant to run
concurrently.
Also extracts the shared per-container fan-out into one helper reused
by restart/down/stop, and dedupes the concurrency-guard test fixtures.
Signed-off-by: Guillaume Lours <[email protected]>
Logs --follow reused the bounded errgroup for both the indefinite log
streams and the monitor goroutine, so once maxConcurrency streams
opened the monitor never started and further services never showed
logs. restart gave each service its own budget, so ready services
could multiply maxConcurrency instead of sharing it.
Bound both with a semaphore that gates only the connect/restart call,
never the indefinite work that follows, sharing one instance across
restart's services. Adds regression tests, plus one guarding the
topological node order a bounded errgroup now relies on in executor.
Signed-off-by: Guillaume Lours <[email protected]>
Nicolas De Loof