Skip to content

Commit 96b29f5

Browse files
thaJeztahthaJeztah
committed
client: remove support for negotiating API version < v1.44 (docker 25.0)
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 <[email protected]>
1 parent 9169ed2 commit 96b29f5

3 files changed

Lines changed: 43 additions & 20 deletions

File tree

‎client/client.go‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"sync/atomic"
5555
"time"
5656

57+
cerrdefs "github.com/containerd/errdefs"
5758
"github.com/docker/go-connections/sockets"
5859
"github.com/moby/moby/api/types"
5960
"github.com/moby/moby/api/types/versions"
@@ -102,7 +103,7 @@ const MaxAPIVersion = "1.52"
102103
// fallbackAPIVersion is the version to fall back to if API-version negotiation
103104
// fails. API versions below this version are not supported by the client,
104105
// and not considered when negotiating.
105-
const fallbackAPIVersion = "1.24"
106+
const fallbackAPIVersion = "1.44"
106107

107108
// Ensure that Client always implements APIClient.
108109
var _ APIClient = &Client{}
@@ -273,7 +274,7 @@ func (cli *Client) checkVersion(ctx context.Context) error {
273274
if err != nil {
274275
return err
275276
}
276-
cli.negotiateAPIVersionPing(ping.APIVersion)
277+
return cli.negotiateAPIVersion(ping.APIVersion)
277278
}
278279
return nil
279280
}
@@ -321,7 +322,8 @@ func (cli *Client) NegotiateAPIVersion(ctx context.Context) {
321322
// FIXME(thaJeztah): Ping returns an error when failing to connect to the API; we should not swallow the error here, and instead returning it.
322323
return
323324
}
324-
cli.negotiateAPIVersionPing(ping.APIVersion)
325+
// FIXME(thaJeztah): we should not swallow the error here, and instead returning it.
326+
_ = cli.negotiateAPIVersion(ping.APIVersion)
325327
}
326328
}
327329

@@ -342,16 +344,22 @@ func (cli *Client) NegotiateAPIVersionPing(pingResponse types.Ping) {
342344
cli.negotiateLock.Lock()
343345
defer cli.negotiateLock.Unlock()
344346

345-
cli.negotiateAPIVersionPing(pingResponse.APIVersion)
347+
// FIXME(thaJeztah): we should not swallow the error here, and instead returning it.
348+
_ = cli.negotiateAPIVersion(pingResponse.APIVersion)
346349
}
347350
}
348351

349-
// negotiateAPIVersionPing queries the API and updates the version to match the
350-
// API version from the ping response.
351-
func (cli *Client) negotiateAPIVersionPing(pingVersion string) {
352+
// negotiateAPIVersion updates the version to match the API version from
353+
// the ping response. It falls back to the lowest version supported if the
354+
// API version is empty, or returns an error if the API version is lower than
355+
// the lowest supported API version, in which case the version is not modified.
356+
func (cli *Client) negotiateAPIVersion(pingVersion string) error {
352357
pingVersion = strings.TrimPrefix(pingVersion, "v")
353358
if pingVersion == "" {
359+
// TODO(thaJeztah): consider returning an error on empty value or not falling back; see https://github.com/moby/moby/pull/51119#discussion_r2413148487
354360
pingVersion = fallbackAPIVersion
361+
} else if versions.LessThan(pingVersion, fallbackAPIVersion) {
362+
return cerrdefs.ErrInvalidArgument.WithMessage(fmt.Sprintf("API version %s is not supported by this client: the minimum supported API version is %s", pingVersion, fallbackAPIVersion))
355363
}
356364

357365
// if the client is not initialized with a version, start with the latest supported version
@@ -369,6 +377,7 @@ func (cli *Client) negotiateAPIVersionPing(pingVersion string) {
369377
if cli.negotiateVersion {
370378
cli.negotiated.Store(true)
371379
}
380+
return nil
372381
}
373382

374383
// DaemonHost returns the host address used by the client

‎client/client_test.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ func TestNewClientWithOpsFromEnv(t *testing.T) {
8282
},
8383
expectedVersion: "1.50",
8484
},
85+
{
86+
doc: "override with unsupported api version",
87+
envs: map[string]string{
88+
"DOCKER_API_VERSION": "1.0",
89+
},
90+
expectedVersion: "1.0",
91+
},
8592
}
8693

8794
for _, tc := range testcases {
@@ -296,13 +303,11 @@ func TestNegotiateAPIVersion(t *testing.T) {
296303
expectedVersion: fallbackAPIVersion,
297304
},
298305
{
299-
// client should downgrade to the version reported by the daemon.
300-
// version negotiation was added in API 1.25, so this is theoretical,
301-
// but it should negotiate to versions before that if the daemon
302-
// gives that as a response.
303-
doc: "downgrade old",
306+
// client should not downgrade to the version reported by the daemon
307+
// if the version is not supported.
308+
doc: "no downgrade old",
304309
pingVersion: "1.19",
305-
expectedVersion: "1.19",
310+
expectedVersion: MaxAPIVersion,
306311
},
307312
{
308313
// client should not upgrade to a newer version if a version was set,

‎vendor/github.com/moby/moby/client/client.go‎

Lines changed: 16 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)