From 27b1fcf13f52f5ad39a5cc0ba7ecc980ad027368 Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Sun, 20 Sep 2026 13:38:24 +0300 Subject: [PATCH 1/2] fix(dockerclient): version API requests when no API version was configured go-dockerclient only parses an API version into the client`s internal requestedAPIVersion when the caller passes one, and getURL() consults that field alone when building a request path. With DOCKER_API_VERSION unset - the default for a plain "mint build" - every request therefore goes out without a "/vX.Y/" segment. A daemon reached through a proxy, which is what dind gives a CI pipeline, reads an unversioned request as coming from the oldest client it supports and rejects it with "client version ... is too old". Setting DOCKER_API_VERSION by hand is the only workaround the issue thread found, and that is precisely because it is the only way the field gets populated. Probe the daemon once and rebuild the client with the version it reports, so requestedAPIVersion ends up set exactly as if the caller had configured it. If the probe fails, hand back the original client so the caller still sees the real connection error. Fixes #95 Signed-off-by: Eljees <3.14hell@gmail.com> --- pkg/crt/docker/dockerclient/client.go | 83 ++++++++++++++----- .../dockerclient/client_api_version_test.go | 70 ++++++++++++++++ 2 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 pkg/crt/docker/dockerclient/client_api_version_test.go diff --git a/pkg/crt/docker/dockerclient/client.go b/pkg/crt/docker/dockerclient/client.go index 635fe4e2e..67fa7d7a1 100644 --- a/pkg/crt/docker/dockerclient/client.go +++ b/pkg/crt/docker/dockerclient/client.go @@ -174,6 +174,65 @@ func GetUnixSocketAddr() (*SocketInfo, error) { return nil, fmt.Errorf("docker socket not found") } +// newVersionedClient builds a *docker.Client for host, honoring an explicit +// apiVersion when the caller set one. When apiVersion is empty (a plain +// "mint build"/"slim build" with no DOCKER_API_VERSION set - the common +// case in every mintoolkit/mint#95 and slimtoolkit/slim#646 report), +// go-dockerclient leaves the client's internal requestedAPIVersion nil for +// its whole life (it only parses config.APIVersion into that field when the +// string is non-empty), so every request the client sends omits the +// "/vX.Y/" path segment. A daemon reached through a proxy - dind +// (Docker-in-Docker: a CI runner's own container running a Docker daemon, +// the topology behind every #95/#646 report) - reads an unversioned +// request as coming from the oldest client it supports and rejects it +// ("client version ... is too old"). Probing the daemon's real API version +// with an initial Version() call and rebuilding the client with that +// version populates requestedAPIVersion exactly as if the caller had set +// DOCKER_API_VERSION by hand, which is the only workaround either issue +// thread ever found. +func newVersionedClient(host, apiVersion string) (*docker.Client, error) { + client, err := docker.NewVersionedClient(host, apiVersion) + if err != nil { + return nil, err + } + + if apiVersion != "" { + client.SkipServerVersionCheck = true + return client, nil + } + + env, err := client.Version() + if err != nil { + // Daemon unreachable or otherwise misbehaving: hand back the + // original unversioned client so callers see the same connection + // error they always would have, instead of masking it here. + return client, nil + } + + discovered := env.Get("ApiVersion") + if discovered == "" { + return client, nil + } + + versioned, err := docker.NewVersionedClient(host, discovered) + if err != nil { + return client, nil + } + + // The just-discovered version must be trusted as-is: go-dockerclient's + // own internal version bootstrap (triggered lazily by the first request + // when SkipServerVersionCheck is false) builds its own "/version" probe + // through getURL(), which - now that requestedAPIVersion is set - + // prefixes even that bootstrap call with "/vX.Y/", so it would ask a + // real daemon for "/v1.44/version" instead of "/version" and fail to + // parse the (missing) result. Skipping that redundant self-check is + // exactly what happens today when a caller sets DOCKER_API_VERSION by + // hand (see the config.APIVersion != "" branches above). + versioned.SkipServerVersionCheck = true + + return versioned, nil +} + // New creates a new Docker client instance func New(config *config.DockerClient) (*docker.Client, error) { var client *docker.Client @@ -290,15 +349,11 @@ func New(config *config.DockerClient) (*docker.Client, error) { case config.Host != "" && !config.UseTLS: - client, err = docker.NewVersionedClient(config.Host, config.APIVersion) + client, err = newVersionedClient(config.Host, config.APIVersion) if err != nil { return nil, err } - if config.APIVersion != "" { - client.SkipServerVersionCheck = true - } - log.Debug("dockerclient.New: new Docker client [3]") case config.Host == "" && @@ -342,15 +397,11 @@ func New(config *config.DockerClient) (*docker.Client, error) { } config.Host = socketInfo.Address - client, err = docker.NewVersionedClient(config.Host, config.APIVersion) + client, err = newVersionedClient(config.Host, config.APIVersion) if err != nil { return nil, err } - if config.APIVersion != "" { - client.SkipServerVersionCheck = true - } - case config.Host == "" && config.Env[EnvDockerHost] == "" && contextHost != "": log.Debugf("dockerclient.New: new Docker client - from context ('%s') contextVerifyTLS=%v", contextHost, contextVerifyTLS) if strings.HasPrefix(contextHost, "/") || @@ -374,15 +425,11 @@ func New(config *config.DockerClient) (*docker.Client, error) { } config.Host = socketInfo.Address - client, err = docker.NewVersionedClient(config.Host, config.APIVersion) + client, err = newVersionedClient(config.Host, config.APIVersion) if err != nil { return nil, err } - if config.APIVersion != "" { - client.SkipServerVersionCheck = true - } - log.Debugf("dockerclient.New: new Docker client - from context ('%s') - [7]", contextHost) } else { log.Debugf("dockerclient.New: new Docker client - from context - non-unix socket host (%s) [todo]", contextHost) @@ -403,15 +450,11 @@ func New(config *config.DockerClient) (*docker.Client, error) { } config.Host = socketInfo.Address - client, err = docker.NewVersionedClient(config.Host, config.APIVersion) + client, err = newVersionedClient(config.Host, config.APIVersion) if err != nil { return nil, err } - if config.APIVersion != "" { - client.SkipServerVersionCheck = true - } - log.Debug("dockerclient.New: new Docker client (default) [6]") default: diff --git a/pkg/crt/docker/dockerclient/client_api_version_test.go b/pkg/crt/docker/dockerclient/client_api_version_test.go new file mode 100644 index 000000000..e83866c58 --- /dev/null +++ b/pkg/crt/docker/dockerclient/client_api_version_test.go @@ -0,0 +1,70 @@ +package dockerclient + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/mintoolkit/mint/pkg/app/master/config" +) + +// TestNewClientVersionsRequestsWithoutExplicitAPIVersion covers #95. +// +// When the caller leaves DOCKER_API_VERSION / config.APIVersion empty - the +// default for a plain "mint build" - go-dockerclient never parses an API +// version into the client's requestedAPIVersion, and getURL() consults that +// field alone when it builds a request path. Every request the client sends +// therefore omits the "/vX.Y/" segment. A daemon reached through a proxy, +// which is what dind gives a CI pipeline, reads an unversioned request as +// coming from the oldest client it supports and rejects it with +// "client version ... is too old". +// +// The client's own /version negotiation does not help: its result is stored +// in expectedAPIVersion and never copied into requestedAPIVersion. So the +// request worth asserting on is the next one - here Info()'s GET /info - +// which is the one a proxying daemon actually rejects. +func TestNewClientVersionsRequestsWithoutExplicitAPIVersion(t *testing.T) { + var infoPath string + sawInfoRequest := false + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if strings.HasSuffix(r.URL.Path, "/version") { + // Answer the version probe truthfully so the client proceeds. + _, _ = w.Write([]byte(`{"ApiVersion":"1.44"}`)) + return + } + infoPath = r.URL.Path + sawInfoRequest = true + _, _ = w.Write([]byte(`{}`)) + })) + defer srv.Close() + + cfg := &config.DockerClient{ + Host: srv.URL, + UseTLS: false, + // Left empty on purpose: this is the path every report hit. + APIVersion: "", + } + + client, err := New(cfg) + if err != nil { + t.Fatalf("New() with an empty APIVersion must succeed: %v", err) + } + + if _, err := client.Info(); err != nil { + t.Fatalf("Info() must succeed against a daemon that answers /version: %v", err) + } + + if !sawInfoRequest { + t.Fatal("expected the fake daemon to receive the /info request, got none") + } + + if !strings.Contains(infoPath, "/v") { + t.Fatalf("the /info request path %q carries no API version segment although the daemon "+ + "answered the version probe; a daemon behind a proxy rejects such a request as coming "+ + "from a client that is too old (#95)", infoPath) + } +} From b6c112dc3888b6fcfe0b70054e46a53b6e506cad Mon Sep 17 00:00:00 2001 From: Eljees <57435526+Eljees@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:48:19 +0300 Subject: [PATCH 2/2] fix(dockerclient): version requests on the TLS and DOCKER_HOST paths too The first commit covered only the plain-host and socket paths. The TLS branches build their clients through NewVersionedTLSClientFromBytes and the DOCKER_HOST branch through NewClientFromEnv; both leave requestedAPIVersion unset when no version is configured, so a TLS or environment-configured daemon behind a proxy hits the same unversioned-request rejection. Version discovery now lives in withDiscoveredAPIVersion, which takes the constructor to rebuild with. Each path hands over its own, so the rebuilt client keeps that path's transport, certificates and endpoint: - plain host and socket paths: docker.NewVersionedClient - TLS paths: the file's own newTLSClient closure - DOCKER_HOST path: docker.NewVersionedClientFromEnv Tests: - the regression test no longer leaks DOCKER_HOST into the process environment (t.Setenv puts the original back) and matches the unversioned "/version" probe exactly, so a later "/v1.44/version" cannot be taken for it - a second end-to-end test covers the DOCKER_HOST path - four unit tests cover the shared discovery logic, including that the caller's own constructor receives the discovered version, which is what the TLS paths rely on go test ./pkg/crt/docker/dockerclient/... gives 6 green tests with this change. With master's client.go in place, both end-to-end tests fail on `the /info request path "/info" carries no API version segment`. Signed-off-by: Eljees <57435526+Eljees@users.noreply.github.com> --- pkg/crt/docker/dockerclient/client.go | 135 ++++++++++++------ .../client_api_version_internal_test.go | 121 ++++++++++++++++ .../dockerclient/client_api_version_test.go | 82 ++++++++--- 3 files changed, 275 insertions(+), 63 deletions(-) create mode 100644 pkg/crt/docker/dockerclient/client_api_version_internal_test.go diff --git a/pkg/crt/docker/dockerclient/client.go b/pkg/crt/docker/dockerclient/client.go index 67fa7d7a1..9de82e03f 100644 --- a/pkg/crt/docker/dockerclient/client.go +++ b/pkg/crt/docker/dockerclient/client.go @@ -174,63 +174,86 @@ func GetUnixSocketAddr() (*SocketInfo, error) { return nil, fmt.Errorf("docker socket not found") } -// newVersionedClient builds a *docker.Client for host, honoring an explicit -// apiVersion when the caller set one. When apiVersion is empty (a plain -// "mint build"/"slim build" with no DOCKER_API_VERSION set - the common -// case in every mintoolkit/mint#95 and slimtoolkit/slim#646 report), -// go-dockerclient leaves the client's internal requestedAPIVersion nil for -// its whole life (it only parses config.APIVersion into that field when the -// string is non-empty), so every request the client sends omits the -// "/vX.Y/" path segment. A daemon reached through a proxy - dind -// (Docker-in-Docker: a CI runner's own container running a Docker daemon, -// the topology behind every #95/#646 report) - reads an unversioned -// request as coming from the oldest client it supports and rejects it -// ("client version ... is too old"). Probing the daemon's real API version -// with an initial Version() call and rebuilding the client with that -// version populates requestedAPIVersion exactly as if the caller had set -// DOCKER_API_VERSION by hand, which is the only workaround either issue -// thread ever found. -func newVersionedClient(host, apiVersion string) (*docker.Client, error) { - client, err := docker.NewVersionedClient(host, apiVersion) +// discoverAPIVersion asks the daemon behind client which API version it +// speaks. An unreachable or otherwise misbehaving daemon yields "", which +// leaves the caller with the client it already had, so a connection problem +// still surfaces as the same error it always did. +func discoverAPIVersion(client *docker.Client) string { + env, err := client.Version() if err != nil { - return nil, err + return "" } - if apiVersion != "" { - client.SkipServerVersionCheck = true - return client, nil + return env.Get("ApiVersion") +} + +// withDiscoveredAPIVersion returns a client whose requests carry the "/vX.Y/" +// path segment even when the caller configured no API version. +// +// go-dockerclient parses an API version into the client's internal +// requestedAPIVersion only when the string it is given is non-empty, and +// getURL() consults that field alone when it builds a request path. With no +// version configured - a plain "mint build"/"slim build" with no +// DOCKER_API_VERSION set, which is the case in every mintoolkit/mint#95 and +// slimtoolkit/slim#646 report - every request the client sends omits the +// version segment. A daemon reached through a proxy, dind (Docker-in-Docker: +// a CI runner's own container running a Docker daemon, the topology behind +// those reports), reads an unversioned request as coming from the oldest +// client it supports and rejects it ("client version ... is too old"). The +// client's own /version negotiation does not help here: it stores its result +// in expectedAPIVersion and never copies it into requestedAPIVersion. +// +// rebuild constructs the replacement client for the discovered version. Each +// construction path passes its own constructor, so the rebuilt client keeps +// that path's transport, credentials and endpoint; this is what lets the TLS +// and environment paths share this logic with the plain one. +func withDiscoveredAPIVersion( + client *docker.Client, + apiVersion string, + rebuild func(apiVersion string) (*docker.Client, error), +) *docker.Client { + if client == nil { + return client } - env, err := client.Version() - if err != nil { - // Daemon unreachable or otherwise misbehaving: hand back the - // original unversioned client so callers see the same connection - // error they always would have, instead of masking it here. - return client, nil + if apiVersion != "" { + // An explicitly configured version already populates + // requestedAPIVersion, so the lazy self-check buys nothing. This is + // what the call sites did by hand before. + client.SkipServerVersionCheck = true + return client } - discovered := env.Get("ApiVersion") + discovered := discoverAPIVersion(client) if discovered == "" { - return client, nil + return client } - versioned, err := docker.NewVersionedClient(host, discovered) - if err != nil { - return client, nil + versioned, err := rebuild(discovered) + if err != nil || versioned == nil { + return client } - // The just-discovered version must be trusted as-is: go-dockerclient's - // own internal version bootstrap (triggered lazily by the first request - // when SkipServerVersionCheck is false) builds its own "/version" probe - // through getURL(), which - now that requestedAPIVersion is set - - // prefixes even that bootstrap call with "/vX.Y/", so it would ask a - // real daemon for "/v1.44/version" instead of "/version" and fail to - // parse the (missing) result. Skipping that redundant self-check is - // exactly what happens today when a caller sets DOCKER_API_VERSION by - // hand (see the config.APIVersion != "" branches above). + // The version was just read from this very daemon, so the lazy self-check + // that the first request would otherwise trigger only repeats the probe + // that has already happened. Skipping it is exactly what happens today + // when a caller sets DOCKER_API_VERSION by hand. versioned.SkipServerVersionCheck = true - return versioned, nil + return versioned +} + +// newVersionedClient builds a plain (non-TLS) client for host and gives it the +// daemon's API version when the caller configured none. +func newVersionedClient(host, apiVersion string) (*docker.Client, error) { + client, err := docker.NewVersionedClient(host, apiVersion) + if err != nil { + return nil, err + } + + return withDiscoveredAPIVersion(client, apiVersion, func(apiVersion string) (*docker.Client, error) { + return docker.NewVersionedClient(host, apiVersion) + }), nil } // New creates a new Docker client instance @@ -264,6 +287,20 @@ func New(config *config.DockerClient) (*docker.Client, error) { return docker.NewVersionedTLSClientFromBytes(host, cert, key, ca, apiVersion) } + // newVersionedTLSClient is newTLSClient plus the version discovery the + // plain path gets: same certificates, same transport, and a "/vX.Y/" + // segment on every request when the caller configured no version. + newVersionedTLSClient := func(host string, certPath string, verify bool, apiVersion string) (*docker.Client, error) { + client, err := newTLSClient(host, certPath, verify, apiVersion) + if err != nil { + return nil, err + } + + return withDiscoveredAPIVersion(client, apiVersion, func(apiVersion string) (*docker.Client, error) { + return newTLSClient(host, certPath, verify, apiVersion) + }), nil + } + //NOTE: //go-dockerclient doesn't support DOCKER_CONTEXT natively //so we need to lookup the context first to extract its connection info @@ -329,7 +366,7 @@ func New(config *config.DockerClient) (*docker.Client, error) { config.UseTLS && config.VerifyTLS && config.TLSCertPath != "": - client, err = newTLSClient(config.Host, config.TLSCertPath, true, config.APIVersion) + client, err = newVersionedTLSClient(config.Host, config.TLSCertPath, true, config.APIVersion) if err != nil { return nil, err } @@ -340,7 +377,7 @@ func New(config *config.DockerClient) (*docker.Client, error) { config.UseTLS && !config.VerifyTLS && config.TLSCertPath != "": - client, err = newTLSClient(config.Host, config.TLSCertPath, false, config.APIVersion) + client, err = newVersionedTLSClient(config.Host, config.TLSCertPath, false, config.APIVersion) if err != nil { return nil, err } @@ -361,7 +398,7 @@ func New(config *config.DockerClient) (*docker.Client, error) { config.Env[EnvDockerTLSVerify] == "1" && config.Env[EnvDockerCertPath] != "" && config.Env[EnvDockerHost] != "": - client, err = newTLSClient(config.Env[EnvDockerHost], config.Env[EnvDockerCertPath], false, config.APIVersion) + client, err = newVersionedTLSClient(config.Env[EnvDockerHost], config.Env[EnvDockerCertPath], false, config.APIVersion) if err != nil { return nil, err } @@ -374,6 +411,12 @@ func New(config *config.DockerClient) (*docker.Client, error) { return nil, err } + // NewClientFromEnv reads DOCKER_API_VERSION itself, so the same string + // decides here whether a version was configured at all. + client = withDiscoveredAPIVersion(client, os.Getenv(EnvDockerAPIVer), func(apiVersion string) (*docker.Client, error) { + return docker.NewVersionedClientFromEnv(apiVersion) + }) + log.Debug("dockerclient.New: new Docker client (env) [5]") case config.Host != "" && (strings.HasPrefix(config.Host, "/") || strings.HasPrefix(config.Host, "unix://")): diff --git a/pkg/crt/docker/dockerclient/client_api_version_internal_test.go b/pkg/crt/docker/dockerclient/client_api_version_internal_test.go new file mode 100644 index 000000000..a5e2ff608 --- /dev/null +++ b/pkg/crt/docker/dockerclient/client_api_version_internal_test.go @@ -0,0 +1,121 @@ +package dockerclient + +import ( + "errors" + "testing" + + docker "github.com/fsouza/go-dockerclient" +) + +// TestWithDiscoveredAPIVersionKeepsAnExplicitVersion asserts the branch every +// caller relied on before: a configured version is left alone, and the +// redundant self-check is skipped. +func TestWithDiscoveredAPIVersionKeepsAnExplicitVersion(t *testing.T) { + client, err := docker.NewVersionedClient("http://127.0.0.1:1", "1.41") + if err != nil { + t.Fatalf("building the fixture client: %v", err) + } + + rebuilt := false + got := withDiscoveredAPIVersion(client, "1.41", func(string) (*docker.Client, error) { + rebuilt = true + return nil, nil + }) + + if rebuilt { + t.Fatal("a configured API version must not trigger a probe or a rebuild") + } + + if got != client { + t.Fatal("a configured API version must leave the client as it is") + } + + if !got.SkipServerVersionCheck { + t.Fatal("a configured API version must skip the redundant server version check") + } +} + +// TestWithDiscoveredAPIVersionKeepsTheClientWhenTheDaemonIsUnreachable makes +// sure the probe never turns a connection problem into a different error: the +// caller gets the client it would have had, and fails where it always did. +func TestWithDiscoveredAPIVersionKeepsTheClientWhenTheDaemonIsUnreachable(t *testing.T) { + // Port 1 refuses connections, which is what an absent daemon looks like. + client, err := docker.NewVersionedClient("http://127.0.0.1:1", "") + if err != nil { + t.Fatalf("building the fixture client: %v", err) + } + + rebuilt := false + got := withDiscoveredAPIVersion(client, "", func(string) (*docker.Client, error) { + rebuilt = true + return nil, nil + }) + + if rebuilt { + t.Fatal("an unreachable daemon must not produce a rebuilt client") + } + + if got != client { + t.Fatal("an unreachable daemon must leave the original client in place") + } +} + +// TestWithDiscoveredAPIVersionRebuildsThroughTheCallersConstructor is the +// TLS and environment paths' coverage: they differ from the plain path only +// in the constructor they hand over, so this asserts that the discovered +// version reaches that constructor and that its client is the one returned. +func TestWithDiscoveredAPIVersionRebuildsThroughTheCallersConstructor(t *testing.T) { + var probePath string + srv := daemonAnsweringVersion(t, &probePath) + defer srv.Close() + + client, err := docker.NewVersionedClient(srv.URL, "") + if err != nil { + t.Fatalf("building the fixture client: %v", err) + } + + replacement, err := docker.NewVersionedClient(srv.URL, "1.44") + if err != nil { + t.Fatalf("building the replacement client: %v", err) + } + + var handed string + got := withDiscoveredAPIVersion(client, "", func(apiVersion string) (*docker.Client, error) { + handed = apiVersion + return replacement, nil + }) + + if handed != "1.44" { + t.Fatalf("the constructor was handed %q, expected the version the daemon reported", handed) + } + + if got != replacement { + t.Fatal("the client built by the caller's own constructor must be the one returned") + } + + if !got.SkipServerVersionCheck { + t.Fatal("the rebuilt client must skip the self-check that repeats the probe just made") + } +} + +// TestWithDiscoveredAPIVersionKeepsTheClientWhenTheRebuildFails covers the +// remaining branch: a constructor that fails leaves the working client in +// place rather than dropping the caller into a nil. +func TestWithDiscoveredAPIVersionKeepsTheClientWhenTheRebuildFails(t *testing.T) { + var probePath string + srv := daemonAnsweringVersion(t, &probePath) + defer srv.Close() + + client, err := docker.NewVersionedClient(srv.URL, "") + if err != nil { + t.Fatalf("building the fixture client: %v", err) + } + + got := withDiscoveredAPIVersion(client, "", func(string) (*docker.Client, error) { + return nil, errors.New("no certificates here") + }) + + if got != client { + t.Fatal("a failed rebuild must leave the original client in place") + } +} diff --git a/pkg/crt/docker/dockerclient/client_api_version_test.go b/pkg/crt/docker/dockerclient/client_api_version_test.go index e83866c58..a6680528c 100644 --- a/pkg/crt/docker/dockerclient/client_api_version_test.go +++ b/pkg/crt/docker/dockerclient/client_api_version_test.go @@ -9,7 +9,31 @@ import ( "github.com/mintoolkit/mint/pkg/app/master/config" ) -// TestNewClientVersionsRequestsWithoutExplicitAPIVersion covers #95. +// daemonAnsweringVersion is a stand-in daemon that answers the unversioned +// "/version" probe and records the path of the next request it receives. +// +// The match on "/version" is exact on purpose: once the client knows an API +// version it prefixes every path, so a lenient match would take a later +// "/v1.44/version" for the probe and hide the very thing under test. +func daemonAnsweringVersion(t *testing.T, seen *string) *httptest.Server { + t.Helper() + + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + + if r.URL.Path == "/version" { + _, _ = w.Write([]byte(`{"ApiVersion":"1.44"}`)) + return + } + + *seen = r.URL.Path + _, _ = w.Write([]byte(`{}`)) + })) +} + +// TestNewClientVersionsRequestsWithoutExplicitAPIVersion covers #95 on the +// plain host path. // // When the caller leaves DOCKER_API_VERSION / config.APIVersion empty - the // default for a plain "mint build" - go-dockerclient never parses an API @@ -25,21 +49,14 @@ import ( // request worth asserting on is the next one - here Info()'s GET /info - // which is the one a proxying daemon actually rejects. func TestNewClientVersionsRequestsWithoutExplicitAPIVersion(t *testing.T) { - var infoPath string - sawInfoRequest := false + // New() writes DOCKER_HOST (and DOCKER_API_VERSION) into the process + // environment. t.Setenv restores whatever was there when the test ends, + // so a closed test server's URL cannot leak into later tests. + t.Setenv(EnvDockerHost, "") + t.Setenv(EnvDockerAPIVer, "") - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - if strings.HasSuffix(r.URL.Path, "/version") { - // Answer the version probe truthfully so the client proceeds. - _, _ = w.Write([]byte(`{"ApiVersion":"1.44"}`)) - return - } - infoPath = r.URL.Path - sawInfoRequest = true - _, _ = w.Write([]byte(`{}`)) - })) + var infoPath string + srv := daemonAnsweringVersion(t, &infoPath) defer srv.Close() cfg := &config.DockerClient{ @@ -58,13 +75,44 @@ func TestNewClientVersionsRequestsWithoutExplicitAPIVersion(t *testing.T) { t.Fatalf("Info() must succeed against a daemon that answers /version: %v", err) } - if !sawInfoRequest { + if infoPath == "" { t.Fatal("expected the fake daemon to receive the /info request, got none") } - if !strings.Contains(infoPath, "/v") { + if !strings.HasPrefix(infoPath, "/v1.44/") { t.Fatalf("the /info request path %q carries no API version segment although the daemon "+ "answered the version probe; a daemon behind a proxy rejects such a request as coming "+ "from a client that is too old (#95)", infoPath) } } + +// TestNewClientFromEnvVersionsRequests covers the same defect on the +// DOCKER_HOST path, which reaches the daemon through +// docker.NewClientFromEnv() instead of an explicit host. +func TestNewClientFromEnvVersionsRequests(t *testing.T) { + var infoPath string + srv := daemonAnsweringVersion(t, &infoPath) + defer srv.Close() + + t.Setenv(EnvDockerHost, srv.URL) + t.Setenv(EnvDockerAPIVer, "") + t.Setenv(EnvDockerTLSVerify, "") + + cfg := &config.DockerClient{ + Env: map[string]string{EnvDockerHost: srv.URL}, + } + + client, err := New(cfg) + if err != nil { + t.Fatalf("New() from the environment must succeed: %v", err) + } + + if _, err := client.Info(); err != nil { + t.Fatalf("Info() must succeed against a daemon that answers /version: %v", err) + } + + if !strings.HasPrefix(infoPath, "/v1.44/") { + t.Fatalf("the /info request path %q carries no API version segment on the DOCKER_HOST "+ + "path (#95)", infoPath) + } +}