Historically, the client would downgrade to API v1.24 when failing
to negotiate as this was the API version from before API-version
negotiation was introduced.
Given that those daemons are EOL and those API versions no longer
supported, we should not fall back to an older API version, and
just continue using the latest / current version.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make the mocked responses match the API closer;
- Add headers as returned by the daemon's VersionMiddleware
- By default handle "/_ping" requests to allow the client to
perform API-version negotiation as part of tests.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
While a manual overridden version shouldn't perform automatic version
negotiation, the "ForceNegotiate" option could still be used to (re)
negotiate a version. This allows a client to be configured with an
initial API version, then triggered to perform API-version negotiation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Make these options more strict to not allow arbitrary values. Historically,
the `DOCKER_API_VERSION` env-var did not perform any validation as it was
intended for testing-purposes, but given the wider use of this env-var,
we should perform some amount of validation.
Both `WithAPIVersion` and `WithAPIVersionFromEnv` still allow specifying
API versions that are not supported by the client for testing purposes
(e.g. to use API versions beyond `MinAPIVersion` and `MaxAPIVersion`),
but must be well-formed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add WithAPIVersion and WithAPIVersionFromEnv to be more clear on
the intent, and to align with other related options and fields.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use a more idiomatic name so that it can be used as `client.New()`.
We should look if we want `New()` to have different / updated defaults
i.e., enable `WithEnv` as default, and have an opt-out and have API-
version negotiation enabled by default (with an opt-out option).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- Use the `mockResponse` instead
- `bytesBufferClose` was our own implementation of `io.NopCloser`
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The API does not produce these as a response; the fields in the Ping
struct, including the Swarm status are propagated from headers returned
by the /_ping endpoint.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Docker versions below 25.0 have reached EOL; 25.0 is currently maintained
as an LTS version by Mirantis, and we want to allow current versions of the
CLI to be able to connect to such setups.
This patch raises the fallback API version to API v1.44; when negotiating an API
version with a daemon, this will be the lowest version negotiated.
Currently, it still allows manually overriding the version to versions that
are not supported (`WithVersion`, `WithVersionFromEnv`), and no code has
been removed yet that adjusts the client for old API versions, but this
can be done in a follow-up.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
TestGetAPIPath: don't use obsolete API versions in test
This test was using API v1.22 as "old" version to verify the given
version overrode the default. Update it to use a previous API version
that's still supported by the client.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
These consts are no longer used, and separate consts were added in both
the client and daemon packages;
- client: 41da5700a4
- daemon: a632b8495b
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With the client and API migrating to separate modules, users of the Client
module may upgrade the API module to higher versions. Currently, the Client
uses the API's Default version. While the version of the API module is
allowed to be updated (following SemVer), we should not allow the Client
to support higher API versions than it was written for.
This patch introduces a DefaultAPIVersion in the client package that is
used as default version of the API for the client to use.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
As of Go 1.17, `Setenv` can be used to set environment variables
specific to a single test. This also removes a package which gets
vendored just for this.
Signed-off-by: Derek McGowan <derek@mcg.dev>
These comments were added to enforce using the correct import path for
our packages ("github.com/docker/docker", not "github.com/moby/moby").
However, when working in go module mode (not GOPATH / vendor), they have
no effect, so their impact is limited.
Remove these imports in preparation of migrating our code to become an
actual go module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When overriding the API version through DOCKER_API_VERSION, no validation
happens on the given version. However, some code-paths in the client do
some minor normalizing, and strip the "v" prefix (if present) as part of
[`Client.getAPIPath()`][1].
This resulted in some inconsistent handling of the version that's set. For
example, [`Client.checkResponseErr()`][2] decides whether or not the API
response is expected to support errors in JSON format (`types.ErrorResponse`),
which would fail because `versions.GreaterThan()` does not strip the prefix,
therefore making the first element "zero" (ranking lower than any valid version).
Net result was "mixed" because of this; for example in the following, half
the output is handled correctly ("downgraded from 1.47"), but the response
is handled as < 1.23 (so printed as-is);
DOCKER_API_VERSION=v1.23 docker version
Client: Docker Engine - Community
Version: 27.5.1
API version: v1.23 (downgraded from 1.47)
Go version: go1.22.11
Git commit: 9f9e405
Built: Wed Jan 22 13:41:13 2025
OS/Arch: linux/amd64
Context: default
Error response from daemon: {"message":"client version 1.23 is too old. Minimum supported API version is 1.24, please upgrade your client to a newer version"}
Passing the version without v-prefix corrects this problem;
DOCKER_API_VERSION=1.23 docker version
Client: Docker Engine - Community
Version: 27.5.1
API version: 1.99 (downgraded from 1.47)
Go version: go1.22.11
Git commit: 9f9e405
Built: Wed Jan 22 13:41:13 2025
OS/Arch: linux/amd64
Context: default
Error response from daemon: client version 1.99 is too new. Maximum supported API version is 1.47
DOCKER_API_VERSION=v1.99 docker version
Client: Docker Engine - Community
Version: 27.5.1
API version: v1.99 (downgraded from 1.47)
Go version: go1.22.11
Git commit: 9f9e405
Built: Wed Jan 22 13:41:13 2025
OS/Arch: linux/amd64
Context: default
Error response from daemon: {"message":"client version 1.99 is too new. Maximum supported API version is 1.47"}
This patch strips the prefix when setting a custom version, so that
normalization happens consistently. The existing code to strip the
prefix in [`Client.getAPIPath()`][1] is kept for now, in case values
are set through other ways.
[1]: 47dc8d5dd8/client/client.go (L303-L309)
[2]: 47dc8d5dd8/client/request.go (L231-L241)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Go automatically canonicalises HTTP headers, meaning the string `API-Version` passed as a header has always been returned as `Api-Version`. Similarly, `OSType` is returned as `Ostype`.
This commit updates the documentation to reflect this behaviour and modifies the codebase to ensure that input strings are aligned with their canonical output values.
Signed-off-by: maggie44 <64841595+maggie44@users.noreply.github.com>
client/client_test.go:91:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
client/client_test.go:326:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
client/client_test.go:481:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
client/image_list_test.go:183:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
client/image_push_test.go:163:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
client/image_tag_test.go:50:3: The copy of the 'for' variable "repo" can be deleted (Go 1.22+) (copyloopvar)
repo := repo
^
client/image_tag_test.go:61:3: The copy of the 'for' variable "repotag" can be deleted (Go 1.22+) (copyloopvar)
repotag := repotag
^
client/ping_test.go:114:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
client/request_test.go:53:3: The copy of the 'for' variable "tc" can be deleted (Go 1.22+) (copyloopvar)
tc := tc
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Commit e6907243af applied a fix for situations
where the client was configured with API-version negotiation, but did not yet
negotiate a version.
However, the checkVersion() function that was implemented copied the semantics
of cli.NegotiateAPIVersion, which ignored connection failures with the
assumption that connection errors would still surface further down.
However, when using the result of a failed negotiation for NewVersionError,
an API version mismatch error would be produced, masking the actual connection
error.
This patch changes the signature of checkVersion to return unexpected errors,
including failures to connect to the API.
Before this patch:
docker -H unix:///no/such/socket.sock secret ls
"secret list" requires API version 1.25, but the Docker daemon API version is 1.24
With this patch applied:
docker -H unix:///no/such/socket.sock secret ls
Cannot connect to the Docker daemon at unix:///no/such/socket.sock. Is the docker daemon running?
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
NegotiateAPIVersion was ignoring errors returned by Ping. The intent here
was to handle API responses from a daemon that may be in an unhealthy state,
however this case is already handled by Ping itself.
Ping only returns an error when either failing to connect to the API (daemon
not running or permissions errors), or when failing to parse the API response.
Neither of those should be ignored in this code, or considered a successful
"ping", so update the code to return
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- don't use un-keyed structs
- use http consts where possible
- use errors.As instead of manually checking the error-type
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Had this stashed as part of some other changes, so thought I'd push
as a PR; with this patch:
=== RUN TestNewClientWithOpsFromEnv/invalid_cert_path
=== RUN TestNewClientWithOpsFromEnv/default_api_version_with_cert_path
=== RUN TestNewClientWithOpsFromEnv/default_api_version_with_cert_path_and_tls_verify
=== RUN TestNewClientWithOpsFromEnv/default_api_version_with_cert_path_and_host
=== RUN TestNewClientWithOpsFromEnv/invalid_docker_host
=== RUN TestNewClientWithOpsFromEnv/invalid_docker_host,_with_good_format
=== RUN TestNewClientWithOpsFromEnv/override_api_version
--- PASS: TestNewClientWithOpsFromEnv (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/default_api_version (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/invalid_cert_path (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/default_api_version_with_cert_path (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/default_api_version_with_cert_path_and_tls_verify (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/default_api_version_with_cert_path_and_host (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/invalid_docker_host (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/invalid_docker_host,_with_good_format (0.00s)
--- PASS: TestNewClientWithOpsFromEnv/override_api_version (0.00s)
PASS
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit replaces `os.Setenv` with `t.Setenv` in tests. The
environment variable is automatically restored to its original value
when the test and all its subtests complete.
Reference: https://pkg.go.dev/testing#T.Setenv
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
This test was setting the non-exported `Client.basePath` directly, however,
it was setting it to a value that would never realistically happen, because
`NewClientWithOpts()` initializes the Client with the default API version:
ea5b4765d9/client/client.go (L119-L130)
Which is used by `getAPIPath()` to construct the URL/path:
ea5b4765d9/client/client.go (L176-L190)
While this didn't render the test "invalid", using a Client that's constructed
in the usual way, makes it more representative.
Given that we deprecated (but still support) the non-versioned API paths, with
the exception of the `/_ping` API endpoint, we should probably change `getAPIPath()`
to default to the "current version", instead of allowing it to use an empty string.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- avoid accessing non-exported fields where possible, and test using accessors
instead, so that we're closer to how it's actually used.
- use a variable or const for "expected" in some tests, so that "expected" is
printed as part of the test-failure output (instead of just a "value").
- swap the order of "actual" and "expected" for consistency, and to make it
easier to see what the "expected" value is in some cases ("expected" on the
right, so that it reads `val (actual) != val (expected)`).
- don't set fields in the Ping response that are not relevant to the test.
- rename some variables for consistency.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The io/ioutil package has been deprecated in Go 1.16. This commit
replaces the existing io/ioutil functions with their new definitions in
io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
WithAPIVersionNegotiation enables automatic API version negotiation for the client.
With this option enabled, the client automatically negotiates the API version
to use when making requests. API version negotiation is performed on the first
request; subsequent requests will not re-negotiate.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
commit 3d72963ab8 fixed
situations where a version negotiation could override
the version, even though a client was initialized with a
fixed version.
In situations where the "fixed" version is empty, we
should ignore the option, and treat the client as
"not having a fixed version", so that API version
negotiation can still be performed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
