From 216ac8770964707b441982aed3161e57feb22c3f Mon Sep 17 00:00:00 2001 From: AlexKantor87 Date: Thu, 2 Apr 2026 10:28:38 +0100 Subject: [PATCH 1/3] fix: add Docker API version negotiation to all client creation sites The Docker SDK bump from v28.0.4 to v28.3.2 changed the default API version from 1.48 to 1.51. This breaks any environment where the Docker daemon is older than 28.3 (e.g. GitHub Actions ubuntu-24.04 runners which ship Docker Engine 28.0.4, max API 1.48). Adding client.WithAPIVersionNegotiation() makes the client auto-downgrade to match whatever API version the daemon supports, as recommended by Docker's own documentation. In internal/docker/docker.go, a newDockerClient() helper centralises client creation so the negotiation option cannot be accidentally omitted in future call sites. Includes a regression test that verifies the client can Ping the daemon and that the negotiated API version does not exceed the server maximum. Co-Authored-By: Claude Opus 4.6 --- cmd/kosli/snapshotDocker.go | 2 +- internal/digest/digest.go | 4 +- internal/docker/docker.go | 20 +++-- internal/docker/docker_test.go | 141 +++++++++++++++++++++++---------- 4 files changed, 115 insertions(+), 52 deletions(-) diff --git a/cmd/kosli/snapshotDocker.go b/cmd/kosli/snapshotDocker.go index 125224d7e..4ad545cb7 100644 --- a/cmd/kosli/snapshotDocker.go +++ b/cmd/kosli/snapshotDocker.go @@ -88,7 +88,7 @@ func (o *snapshotDockerOptions) run(args []string) error { func CreateDockerArtifactsData() ([]*server.ServerData, error) { result := []*server.ServerData{} - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) if err != nil { return result, err } diff --git a/internal/digest/digest.go b/internal/digest/digest.go index d8581d69d..d534ee94c 100644 --- a/internal/digest/digest.go +++ b/internal/digest/digest.go @@ -251,7 +251,7 @@ func FileSha256(filepath string, logger *logger.Logger) (string, error) { // It requires the docker daemon to be accessible and the docker image to be locally present. // The docker image must have been pushed into a registry to have a digest. func DockerImageSha256(imageID string) (string, error) { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) if err != nil { return "", err } @@ -323,7 +323,7 @@ func requestManifestFromRegistry(registryEndPoint, imageName, imageTag, registry // RemoteDockerImageSha256 returns a sha256 digest of a docker image by reading it from // remote docker registry func RemoteDockerImageSha256(imageName, imageTag, registryEndPoint, registryToken string, logger *logger.Logger) (string, error) { - // Some docker images have Manifest list, aka “fat manifest” which combines + // Some docker images have Manifest list, aka "fat manifest" which combines // image manifests for one or more platforms. Other images don't have such manifest. // The response Content-Type header specifies whether an image has it or not. // More details here: https://docs.docker.com/registry/spec/manifest-v2-2/ diff --git a/internal/docker/docker.go b/internal/docker/docker.go index cd1f3a70d..bc348d392 100644 --- a/internal/docker/docker.go +++ b/internal/docker/docker.go @@ -14,9 +14,17 @@ import ( "github.com/docker/docker/client" ) +// newDockerClient creates a Docker client with API version negotiation enabled. +// This ensures the client automatically downgrades its API version to match +// the daemon, preventing "client version X is too new" errors when the SDK +// is newer than the Docker Engine. +func newDockerClient() (*client.Client, error) { + return client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) +} + // PullDockerImage pulls a docker image or returns an error func PullDockerImage(imageName string) error { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := newDockerClient() if err != nil { return err } @@ -41,7 +49,7 @@ func PullDockerImage(imageName string) error { // PushDockerImage pushes a docker image to the local registry or returns an error func PushDockerImage(imageName string) error { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := newDockerClient() if err != nil { return err } @@ -73,7 +81,7 @@ func PushDockerImage(imageName string) error { // TagDockerImage tags a docker image or returns an error func TagDockerImage(sourceName, targetName string) error { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := newDockerClient() if err != nil { return err } @@ -83,7 +91,7 @@ func TagDockerImage(sourceName, targetName string) error { // RemoveDockerImage deletes a docker image or return an error func RemoveDockerImage(imageName string) error { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := newDockerClient() if err != nil { return err } @@ -98,7 +106,7 @@ func RemoveDockerImage(imageName string) error { // RunDockerContainer runs a docker container that sleeps for 6 minutes and returns its ID or returns an error func RunDockerContainer(imageName string) (string, error) { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := newDockerClient() if err != nil { return "", err } @@ -117,7 +125,7 @@ func RunDockerContainer(imageName string) (string, error) { // RemoveDockerContainer remove a docker container or returns an error func RemoveDockerContainer(containerID string) error { - cli, err := client.NewClientWithOpts(client.FromEnv) + cli, err := newDockerClient() if err != nil { return err } diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go index b48af72eb..d261fe7ba 100644 --- a/internal/docker/docker_test.go +++ b/internal/docker/docker_test.go @@ -1,8 +1,10 @@ package docker import ( + "context" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -13,7 +15,7 @@ type DockerTestSuite struct { } func (suite *DockerTestSuite) SetupSuite() { - suite.testImageName = "library/alpine@sha256:e15947432b813e8ffa90165da919953e2ce850bef511a0ad1287d7cb86de84b5" + suite.testImageName = "library/alpine:latest" } func (suite *DockerTestSuite) SetupTest() { @@ -21,26 +23,45 @@ func (suite *DockerTestSuite) SetupTest() { require.NoError(suite.T(), err) } +func (suite *DockerTestSuite) TestNewDockerClientNegotiatesAPIVersion() { + cli, err := newDockerClient() + suite.Require().NoError(err) + + // Ping triggers version negotiation. If WithAPIVersionNegotiation() is + // missing and the SDK default exceeds the daemon's max API version, + // this call will fail with "client version X is too new". + ping, err := cli.Ping(context.Background()) + suite.Require().NoError(err, + "Docker client should be able to ping the daemon; "+ + "if this fails with 'client version X is too new', "+ + "WithAPIVersionNegotiation() may be missing from newDockerClient()") + + // After negotiation the client version must not exceed the server's max. + suite.Assert().NotEmpty(ping.APIVersion, + "server should report its API version in the ping response") + suite.Assert().LessOrEqual(cli.ClientVersion(), ping.APIVersion, + "negotiated client API version should be <= server max API version") +} + func (suite *DockerTestSuite) TestPullDockerImage() { - tests := []struct { + for _, t := range []struct { name string imageName string wantErr bool }{ { - name: "pulling a real image works", + name: "pulling an existing image works", imageName: suite.testImageName, }, { - name: "pulling a fictional image fails", - imageName: "library/non-existing", + name: "pulling a non-existing image fails", + imageName: "non-existing:latest", wantErr: true, }, - } - for _, tt := range tests { - suite.Run(tt.name, func() { - err := PullDockerImage(tt.imageName) - if tt.wantErr { + } { + suite.Run(t.name, func() { + err := PullDockerImage(t.imageName) + if t.wantErr { require.Error(suite.T(), err) } else { require.NoError(suite.T(), err) @@ -50,34 +71,29 @@ func (suite *DockerTestSuite) TestPullDockerImage() { } func (suite *DockerTestSuite) TestPushDockerImage() { - tests := []struct { - name string - imageName string - tagImageAs string - wantErr bool + for _, t := range []struct { + name string + imageName string + wantErr bool }{ { - name: "pushing a real image works", - imageName: suite.testImageName, - tagImageAs: "localhost:5001/alpine:v1", + name: "pushing an existing image works", + imageName: "localhost:5001/alpine:latest", }, { - name: "pushing to a repo without permission to push fails", - imageName: suite.testImageName, + name: "pushing a non-existing image fails", + imageName: "non-existing:latest", wantErr: true, }, - } - for _, tt := range tests { - suite.Run(tt.name, func() { - if tt.tagImageAs != "" { - err := TagDockerImage(tt.imageName, tt.tagImageAs) + } { + suite.Run(t.name, func() { + if !t.wantErr { + err := TagDockerImage(suite.testImageName, t.imageName) require.NoError(suite.T(), err) - } else { - tt.tagImageAs = tt.imageName } - err := PushDockerImage(tt.tagImageAs) - if tt.wantErr { + err := PushDockerImage(t.imageName) + if t.wantErr { require.Error(suite.T(), err) } else { require.NoError(suite.T(), err) @@ -87,27 +103,66 @@ func (suite *DockerTestSuite) TestPushDockerImage() { } func (suite *DockerTestSuite) TestTagDockerImage() { - err := TagDockerImage(suite.testImageName, "new-tag") - require.NoError(suite.T(), err) + for _, t := range []struct { + name string + imageName string + wantErr bool + }{ + { + name: "tagging an existing image works", + imageName: suite.testImageName, + }, + { + name: "tagging a non-existing image fails", + imageName: "non-existing:latest", + wantErr: true, + }, + } { + suite.Run(t.name, func() { + err := TagDockerImage(t.imageName, "new-tag:latest") + if t.wantErr { + require.Error(suite.T(), err) + } else { + require.NoError(suite.T(), err) + } + }) + } } func (suite *DockerTestSuite) TestRemoveDockerImage() { - err := RemoveDockerImage(suite.testImageName) - require.NoError(suite.T(), err) - err = RemoveDockerImage("non-existing-image") - require.Error(suite.T(), err) + for _, t := range []struct { + name string + imageName string + wantErr bool + }{ + { + name: "removing an existing image works", + imageName: suite.testImageName, + }, + { + name: "removing a non-existing image fails", + imageName: "non-existing:latest", + wantErr: true, + }, + } { + suite.Run(t.name, func() { + err := RemoveDockerImage(t.imageName) + if t.wantErr { + require.Error(suite.T(), err) + } else { + require.NoError(suite.T(), err) + } + }) + } } -func (suite *DockerTestSuite) TestRunDockerContainer() { - id, err := RunDockerContainer(suite.testImageName) - require.NoError(suite.T(), err) - require.NotEmpty(suite.T(), id) - err = RemoveDockerContainer(id) +func (suite *DockerTestSuite) TestRunAndRemoveDockerContainer() { + containerID, err := RunDockerContainer(suite.testImageName) require.NoError(suite.T(), err) + assert.NotEmpty(suite.T(), containerID) - id, err = RunDockerContainer("not-existing-image") - require.Error(suite.T(), err) - require.Empty(suite.T(), id) + err = RemoveDockerContainer(containerID) + require.NoError(suite.T(), err) } func TestDockerTestSuite(t *testing.T) { From c42e2ab562dd777e2682125ecb6d2c1f16683fd9 Mon Sep 17 00:00:00 2001 From: AlexKantor87 Date: Thu, 2 Apr 2026 10:32:46 +0100 Subject: [PATCH 2/3] fix: use pinned alpine digest in test, preserve existing test structure Restores the original docker_test.go and only adds the new TestNewDockerClientNegotiatesAPIVersion regression test. Co-Authored-By: Claude Opus 4.6 --- internal/docker/docker_test.go | 120 ++++++++++++--------------------- 1 file changed, 43 insertions(+), 77 deletions(-) diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go index d261fe7ba..7a0d1bf08 100644 --- a/internal/docker/docker_test.go +++ b/internal/docker/docker_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -15,7 +14,7 @@ type DockerTestSuite struct { } func (suite *DockerTestSuite) SetupSuite() { - suite.testImageName = "library/alpine:latest" + suite.testImageName = "library/alpine@sha256:e15947432b813e8ffa90165da919953e2ce850bef511a0ad1287d7cb86de84b5" } func (suite *DockerTestSuite) SetupTest() { @@ -44,24 +43,25 @@ func (suite *DockerTestSuite) TestNewDockerClientNegotiatesAPIVersion() { } func (suite *DockerTestSuite) TestPullDockerImage() { - for _, t := range []struct { + tests := []struct { name string imageName string wantErr bool }{ { - name: "pulling an existing image works", + name: "pulling a real image works", imageName: suite.testImageName, }, { - name: "pulling a non-existing image fails", - imageName: "non-existing:latest", + name: "pulling a fictional image fails", + imageName: "library/non-existing", wantErr: true, }, - } { - suite.Run(t.name, func() { - err := PullDockerImage(t.imageName) - if t.wantErr { + } + for _, tt := range tests { + suite.Run(tt.name, func() { + err := PullDockerImage(tt.imageName) + if tt.wantErr { require.Error(suite.T(), err) } else { require.NoError(suite.T(), err) @@ -71,29 +71,34 @@ func (suite *DockerTestSuite) TestPullDockerImage() { } func (suite *DockerTestSuite) TestPushDockerImage() { - for _, t := range []struct { - name string - imageName string - wantErr bool + tests := []struct { + name string + imageName string + tagImageAs string + wantErr bool }{ { - name: "pushing an existing image works", - imageName: "localhost:5001/alpine:latest", + name: "pushing a real image works", + imageName: suite.testImageName, + tagImageAs: "localhost:5001/alpine:v1", }, { - name: "pushing a non-existing image fails", - imageName: "non-existing:latest", + name: "pushing to a repo without permission to push fails", + imageName: suite.testImageName, wantErr: true, }, - } { - suite.Run(t.name, func() { - if !t.wantErr { - err := TagDockerImage(suite.testImageName, t.imageName) + } + for _, tt := range tests { + suite.Run(tt.name, func() { + if tt.tagImageAs != "" { + err := TagDockerImage(tt.imageName, tt.tagImageAs) require.NoError(suite.T(), err) + } else { + tt.tagImageAs = tt.imageName } - err := PushDockerImage(t.imageName) - if t.wantErr { + err := PushDockerImage(tt.tagImageAs) + if tt.wantErr { require.Error(suite.T(), err) } else { require.NoError(suite.T(), err) @@ -103,66 +108,27 @@ func (suite *DockerTestSuite) TestPushDockerImage() { } func (suite *DockerTestSuite) TestTagDockerImage() { - for _, t := range []struct { - name string - imageName string - wantErr bool - }{ - { - name: "tagging an existing image works", - imageName: suite.testImageName, - }, - { - name: "tagging a non-existing image fails", - imageName: "non-existing:latest", - wantErr: true, - }, - } { - suite.Run(t.name, func() { - err := TagDockerImage(t.imageName, "new-tag:latest") - if t.wantErr { - require.Error(suite.T(), err) - } else { - require.NoError(suite.T(), err) - } - }) - } + err := TagDockerImage(suite.testImageName, "new-tag") + require.NoError(suite.T(), err) } func (suite *DockerTestSuite) TestRemoveDockerImage() { - for _, t := range []struct { - name string - imageName string - wantErr bool - }{ - { - name: "removing an existing image works", - imageName: suite.testImageName, - }, - { - name: "removing a non-existing image fails", - imageName: "non-existing:latest", - wantErr: true, - }, - } { - suite.Run(t.name, func() { - err := RemoveDockerImage(t.imageName) - if t.wantErr { - require.Error(suite.T(), err) - } else { - require.NoError(suite.T(), err) - } - }) - } + err := RemoveDockerImage(suite.testImageName) + require.NoError(suite.T(), err) + err = RemoveDockerImage("non-existing-image") + require.Error(suite.T(), err) } -func (suite *DockerTestSuite) TestRunAndRemoveDockerContainer() { - containerID, err := RunDockerContainer(suite.testImageName) +func (suite *DockerTestSuite) TestRunDockerContainer() { + id, err := RunDockerContainer(suite.testImageName) require.NoError(suite.T(), err) - assert.NotEmpty(suite.T(), containerID) - - err = RemoveDockerContainer(containerID) + require.NotEmpty(suite.T(), id) + err = RemoveDockerContainer(id) require.NoError(suite.T(), err) + + id, err = RunDockerContainer("not-existing-image") + require.Error(suite.T(), err) + require.Empty(suite.T(), id) } func TestDockerTestSuite(t *testing.T) { From 09981bd2d8cf4efdfda925151cba1199aae7c16a Mon Sep 17 00:00:00 2001 From: AlexKantor87 Date: Thu, 2 Apr 2026 10:41:09 +0100 Subject: [PATCH 3/3] remove DOCKER_API_VERSION pin from test.yml and drop redundant test The DOCKER_API_VERSION env var was masking version mismatch issues in CI by forcing all Docker clients to use API 1.45 regardless of the SDK default. Removing it means the existing Docker integration tests will naturally fail if any call site is missing WithAPIVersionNegotiation(), making the custom negotiation test redundant. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/test.yml | 2 -- internal/docker/docker_test.go | 21 --------------------- 2 files changed, 23 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6488bb7d2..1fcc42ba0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -163,7 +163,6 @@ jobs: INTEGRATION_TEST_AZURE_CLIENT_SECRET: ${{ secrets.azure_client_secret }} INTEGRATION_TEST_AZURE_CLIENT_ID: ${{ secrets.azure_client_id }} KOSLI_SONAR_API_TOKEN: ${{ secrets.sonarqube_token }} - DOCKER_API_VERSION: "1.45" KOSLI_API_TOKEN_PROD: ${{ secrets.kosli_querying_api_token }} run: | # some tests use git operations, therefore the git author on the CI VM needs to be set @@ -273,4 +272,3 @@ jobs: --trail ${{ inputs.TRAIL_NAME }} --scan-results snyk-dependency.json --org ${{ inputs.KOSLI_ORG }} - diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go index 7a0d1bf08..b48af72eb 100644 --- a/internal/docker/docker_test.go +++ b/internal/docker/docker_test.go @@ -1,7 +1,6 @@ package docker import ( - "context" "testing" "github.com/stretchr/testify/require" @@ -22,26 +21,6 @@ func (suite *DockerTestSuite) SetupTest() { require.NoError(suite.T(), err) } -func (suite *DockerTestSuite) TestNewDockerClientNegotiatesAPIVersion() { - cli, err := newDockerClient() - suite.Require().NoError(err) - - // Ping triggers version negotiation. If WithAPIVersionNegotiation() is - // missing and the SDK default exceeds the daemon's max API version, - // this call will fail with "client version X is too new". - ping, err := cli.Ping(context.Background()) - suite.Require().NoError(err, - "Docker client should be able to ping the daemon; "+ - "if this fails with 'client version X is too new', "+ - "WithAPIVersionNegotiation() may be missing from newDockerClient()") - - // After negotiation the client version must not exceed the server's max. - suite.Assert().NotEmpty(ping.APIVersion, - "server should report its API version in the ping response") - suite.Assert().LessOrEqual(cli.ClientVersion(), ping.APIVersion, - "negotiated client API version should be <= server max API version") -} - func (suite *DockerTestSuite) TestPullDockerImage() { tests := []struct { name string