From 9a444c023fa4afa63f9778f8a789db1ebebef137 Mon Sep 17 00:00:00 2001 From: Damian Peckett Date: Sat, 20 Jan 2024 13:49:30 +0100 Subject: [PATCH] feat: negotiate docker client version Signed-off-by: Damian Peckett --- docker/daemon/client.go | 18 ++++++++++-------- docker/daemon/client_test.go | 31 +++++++++++++++++++++++++------ docker/daemon/daemon_dest.go | 2 +- docker/daemon/daemon_src.go | 2 +- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/docker/daemon/client.go b/docker/daemon/client.go index 2c245f54f9..0436dca148 100644 --- a/docker/daemon/client.go +++ b/docker/daemon/client.go @@ -1,6 +1,7 @@ package daemon import ( + "context" "net/http" "path/filepath" @@ -9,13 +10,8 @@ import ( "github.com/docker/go-connections/tlsconfig" ) -const ( - // The default API version to be used in case none is explicitly specified - defaultAPIVersion = "1.22" -) - // NewDockerClient initializes a new API client based on the passed SystemContext. -func newDockerClient(sys *types.SystemContext) (*dockerclient.Client, error) { +func newDockerClient(ctx context.Context, sys *types.SystemContext) (*dockerclient.Client, error) { host := dockerclient.DefaultDockerHost if sys != nil && sys.DockerDaemonHost != "" { host = sys.DockerDaemonHost @@ -23,7 +19,6 @@ func newDockerClient(sys *types.SystemContext) (*dockerclient.Client, error) { opts := []dockerclient.Opt{ dockerclient.WithHost(host), - dockerclient.WithVersion(defaultAPIVersion), } // We conditionalize building the TLS configuration only to TLS sockets: @@ -63,7 +58,14 @@ func newDockerClient(sys *types.SystemContext) (*dockerclient.Client, error) { opts = append(opts, dockerclient.WithHTTPClient(hc)) } - return dockerclient.NewClientWithOpts(opts...) + cli, err := dockerclient.NewClientWithOpts(opts...) + if err != nil { + return nil, err + } + + cli.NegotiateAPIVersion(ctx) + + return cli, nil } func tlsConfig(sys *types.SystemContext) (*http.Client, error) { diff --git a/docker/daemon/client_test.go b/docker/daemon/client_test.go index 161747f750..c33e0f5478 100644 --- a/docker/daemon/client_test.go +++ b/docker/daemon/client_test.go @@ -1,9 +1,12 @@ package daemon import ( + "context" "net/http" "os" "path/filepath" + "strconv" + "strings" "testing" "github.com/containers/image/v5/types" @@ -12,13 +15,23 @@ import ( ) func TestDockerClientFromNilSystemContext(t *testing.T) { - client, err := newDockerClient(nil) + client, err := newDockerClient(context.Background(), nil) assert.Nil(t, err, "There should be no error creating the Docker client") assert.NotNil(t, client, "A Docker client reference should have been returned") assert.Equal(t, dockerclient.DefaultDockerHost, client.DaemonHost(), "The default docker host should have been used") - assert.Equal(t, defaultAPIVersion, client.ClientVersion(), "The default api version should have been used") + + clientVersionNumbers := strings.Split(client.ClientVersion(), ".") + + major, err := strconv.Atoi(clientVersionNumbers[0]) + assert.NoError(t, err, "The client major version should be a number") + + minor, err := strconv.Atoi(clientVersionNumbers[1]) + assert.NoError(t, err, "The client minor version should be a number") + + // The client defaults to 1.24 if negotiation fails. + assert.True(t, major == 1 && minor > 24, client.ClientVersion(), "Should have successfully negotiated a client version") assert.NoError(t, client.Close()) } @@ -33,13 +46,19 @@ func TestDockerClientFromCertContext(t *testing.T) { DockerDaemonInsecureSkipTLSVerify: true, } - client, err := newDockerClient(systemCtx) + client, err := newDockerClient(context.Background(), systemCtx) assert.Nil(t, err, "There should be no error creating the Docker client") assert.NotNil(t, client, "A Docker client reference should have been returned") assert.Equal(t, host, client.DaemonHost()) - assert.Equal(t, "1.22", client.ClientVersion()) + + clientVersionNumbers := strings.Split(client.ClientVersion(), ".") + + major, err := strconv.Atoi(clientVersionNumbers[0]) + assert.NoError(t, err, "The client major version should be a number") + + assert.Equal(t, 1, major, "The client major version should be 1") assert.NoError(t, client.Close()) } @@ -88,11 +107,11 @@ func TestSkipTLSVerifyOnly(t *testing.T) { func TestSpecifyPlainHTTPViaHostScheme(t *testing.T) { host := "http://127.0.0.1:2376" - ctx := &types.SystemContext{ + systemCtx := &types.SystemContext{ DockerDaemonHost: host, } - client, err := newDockerClient(ctx) + client, err := newDockerClient(context.Background(), systemCtx) assert.Nil(t, err, "There should be no error creating the Docker client") assert.NotNil(t, client, "A Docker client reference should have been returned") diff --git a/docker/daemon/daemon_dest.go b/docker/daemon/daemon_dest.go index 55431db13a..c5949aa602 100644 --- a/docker/daemon/daemon_dest.go +++ b/docker/daemon/daemon_dest.go @@ -43,7 +43,7 @@ func newImageDestination(ctx context.Context, sys *types.SystemContext, ref daem mustMatchRuntimeOS = false } - c, err := newDockerClient(sys) + c, err := newDockerClient(ctx, sys) if err != nil { return nil, fmt.Errorf("initializing docker engine client: %w", err) } diff --git a/docker/daemon/daemon_src.go b/docker/daemon/daemon_src.go index 10923c278e..23640cc066 100644 --- a/docker/daemon/daemon_src.go +++ b/docker/daemon/daemon_src.go @@ -24,7 +24,7 @@ type daemonImageSource struct { // is the config, and that the following len(RootFS) files are the layers, but that feels // way too brittle.) func newImageSource(ctx context.Context, sys *types.SystemContext, ref daemonReference) (private.ImageSource, error) { - c, err := newDockerClient(sys) + c, err := newDockerClient(ctx, sys) if err != nil { return nil, fmt.Errorf("initializing docker engine client: %w", err) }