Conversation
…gured When the caller leaves DOCKER_API_VERSION / config.APIVersion empty (the common case for a plain "slim build" with no explicit override), dockerclient.New() builds a *docker.Client whose internal requestedAPIVersion field is never populated, so every outgoing request omits the "/vX.Y/" path segment. A daemon reached through a proxy - dind (Docker-in-Docker), the topology behind Bitbucket/GitLab self-hosted CI runners - reads an unversioned request as coming from the oldest client it supports and rejects it with "client version ... is too old", exactly as reported in slimtoolkit#646. Add newVersionedClient(), which probes the daemon's real API version with an initial Version() call and rebuilds the client with that version when the caller did not set one explicitly - the same effect as the caller setting DOCKER_API_VERSION by hand. Both call sites in New() now go through it. Adds a regression test that stands up a fake Docker daemon and checks that dockerclient.New()'s client sends a versioned request path even when config.APIVersion was left empty. Fixes slimtoolkit#646 Signed-off-by: Eljees <3.14hell@gmail.com>
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
1 participant
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.
Fixes #646
Root cause
When the caller leaves
DOCKER_API_VERSION/config.APIVersionempty (the common case for a plainslim buildwith no explicit override),dockerclient.New()builds a*docker.Clientwhose internalrequestedAPIVersionfield is never populated, so every outgoing request omits the/vX.Y/path segment.A daemon reached through a proxy - dind (Docker-in-Docker), the topology behind Bitbucket/GitLab self-hosted CI runners - reads an unversioned request as coming from the oldest client it supports and rejects it with
client version ... is too old, exactly as reported in #646.This is the same underlying bug as
mintoolkit/mint#95(dockerclient.New()here uses the samedocker.NewVersionedClient(config.Host, config.APIVersion)+ "SkipServerVersionCheck = trueonly whenconfig.APIVersion != """ pattern), already fixed there in mintoolkit/mint#198.Fix
Add
newVersionedClient(), which probes the daemon's real API version with an initialVersion()call and rebuilds the client with that version when the caller did not set one explicitly - the same effect as the caller settingDOCKER_API_VERSIONby hand. Both call sites inNew()now go through it.Testing
Added
TestNewClientDefaultAPIVersion_Issue646, which stands up a fake Docker daemon withhttptest.Serverand asserts the request path carries a version segment even whenconfig.APIVersionwas left empty./info(no version segment) - reproduces the exact defect from dockerutil.BuildEmptyImage() fails with bad API version #646./v1.44/info.Signed-off-by: Eljees 3.14hell@gmail.com