Per-service API client with identifying headers#13963
Open
nicksieger wants to merge 2 commits into
Open
Conversation
Introduce composeService.serviceClient(project, service), a cached APIClient that tags requests with X-Docker-Compose-Project/-Service headers so the daemon can attribute them. Generalizes the pull-only client into shared infrastructure, closed via composeService.Close(), and routes the image pull path through it. Preserves --dry-run by re-wrapping scoped clients in DryRunClient. Signed-off-by: Nick Sieger <nick@nicksieger.com>
Use serviceClient for ContainerCreate, ContainerStart, and the create-time NetworkConnect/ContainerRemove/ContainerInspect calls so each request in the up path carries the project/service identifying headers. Dependency health inspects stay on the shared client (cross-service reads). Signed-off-by: Nick Sieger <nick@nicksieger.com>
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The per-service API client implementation is well-structured and correct. Key aspects reviewed:
- Cache correctness: The mutex-guarded
serviceClientCachecorrectly handles concurrent access from concurrent pull goroutines and container lifecycle calls. Lazy map initialization is safe (inside the lock). - DryRun wrapping:
newServiceClientcorrectly re-wraps the freshly-built client inDryRunClientwhens.dryRun == true, preserving dry-run interception semantics. - Resource lifecycle:
Close()properly drains and closes all cached clients. TheDryRunClient.Close()delegates to the inner client'sClose(), so no resource leak on the dry-run path. - Header precedence: User-configured headers are copied first, then compose-specific headers are set — ensuring compose attribution headers always win, which is the intended behavior.
- API version pinning:
newServiceClientpinsClientVersion()from the shared client, ensuring consistent API version negotiation across all service clients. - Test coverage:
TestServiceClientHeadersexercises header forwarding, CLI header preservation, cache hit/miss, and close semantics.TestResolvePullPlatformscovers the extracted helper.
No bugs introduced by this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Tag the Docker API requests that Compose makes on behalf of a specific service with HTTP headers identifying the project and service the request
originates from:
This lets the daemon (and anything in front of it) attribute an incoming API call to the Compose project/service it came from.
Why
Today every Compose request reaches the daemon indistinguishable from any other. Attributing container-lifecycle calls to their originating service makes daemon-side logging, auditing, and policy far easier, and gives operators a reliable signal for which service triggered a given create/start/pull.
How
The moby client applies custom HTTP headers per client instance, not per request — there is no per-request header hook or request-mutation callback (WithHTTPHeaders is the only lever; the sole hook, WithResponseHook, is response-side). So a single shared client can't carry service-specific headers.
This PR introduces a small per-service client layer:
Call sites routed through the per-service client:
Project-wide calls (networks, volumes, events, project-scoped container listings, Ping) intentionally stay on the shared client. Dependency health-poll inspects (isServiceHealthy/isServiceCompleted) also stay on the shared client, since they read a dependency's containers and don't have unambiguous service attribution.
Testing
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did