refactor: merge RuntimeVersion and CurrentAPIVersion into RuntimeAPIVersion

After API negotiation, Compose should only rely on the negotiated version
and never use the daemon's raw max version for request shaping. Merge both
functions into a single RuntimeAPIVersion that negotiates via Ping and
returns ClientVersion, erroring if the client reports an empty version
instead of silently falling back to ServerVersion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

Author: 2 people

pkg/compose/compose.go

@@ -216,8 +216,7 @@ type composeService struct {
 	maxConcurrency int
 	dryRun         bool
 
-	runtimeVersion    runtimeVersionCache
-	currentAPIVersion runtimeVersionCache
+	runtimeAPIVersion runtimeVersionCache
 }
 
 // Close releases any connections/resources held by the underlying clients.
@@ -502,34 +501,18 @@ type runtimeVersionCache struct {
 	val string
 }
 
-// RuntimeVersion returns the raw API version reported by the daemon.
-// Callers that need the negotiated/effective client API version should use
-// CurrentAPIVersion instead.
-func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) {
-	s.runtimeVersion.mu.Lock()
-	defer s.runtimeVersion.mu.Unlock()
-	if s.runtimeVersion.val != "" {
-		return s.runtimeVersion.val, nil
-	}
-	version, err := s.apiClient().ServerVersion(ctx, client.ServerVersionOptions{})
-	if err != nil {
-		return "", err
-	}
-	s.runtimeVersion.val = version.APIVersion
-	return s.runtimeVersion.val, nil
-}
-
-// CurrentAPIVersion returns the API version currently used by the Docker client.
-// Trigger negotiation first so version-gated request shaping matches the version
-// that subsequent API calls will actually use.
+// RuntimeAPIVersion returns the negotiated API version that will be used for
+// requests to the Docker daemon. It triggers version negotiation via Ping so
+// that version-gated request shaping matches the version subsequent API calls
+// will actually use.
 //
-// Lock ordering: currentAPIVersion.mu must be acquired before runtimeVersion.mu
-// (via the RuntimeVersion fallback). No code path should reverse this order.
-func (s *composeService) CurrentAPIVersion(ctx context.Context) (string, error) {
-	s.currentAPIVersion.mu.Lock()
-	defer s.currentAPIVersion.mu.Unlock()
-	if s.currentAPIVersion.val != "" {
-		return s.currentAPIVersion.val, nil
+// After negotiation, Compose should never rely on features or request attributes
+// not defined by this API version, even if the daemon's raw version is higher.
+func (s *composeService) RuntimeAPIVersion(ctx context.Context) (string, error) {
+	s.runtimeAPIVersion.mu.Lock()
+	defer s.runtimeAPIVersion.mu.Unlock()
+	if s.runtimeAPIVersion.val != "" {
+		return s.runtimeAPIVersion.val, nil
 	}
 
 	cli := s.apiClient()
@@ -539,17 +522,10 @@ func (s *composeService) CurrentAPIVersion(ctx context.Context) (string, error)
 	}
 
 	version := cli.ClientVersion()
-	if version != "" {
-		s.currentAPIVersion.val = version
-		return s.currentAPIVersion.val, nil
+	if version == "" {
+		return "", fmt.Errorf("docker client returned empty version after successful API negotiation")
 	}
 
-	// Defensive fallback for unexpected client implementations or mocks that
-	// do not populate ClientVersion after a successful negotiated ping.
-	val, err := s.RuntimeVersion(ctx)
-	if err != nil {
-		return "", err
-	}
-	s.currentAPIVersion.val = val
-	return s.currentAPIVersion.val, nil
+	s.runtimeAPIVersion.val = version
+	return s.runtimeAPIVersion.val, nil
 }

pkg/compose/convergence_test.go

@@ -498,7 +498,7 @@ func TestCreateMobyContainer(t *testing.T) {
 	assert.NilError(t, err)
 }
 
-func TestCurrentAPIVersionCachesNegotiation(t *testing.T) {
+func TestRuntimeAPIVersionCachesNegotiation(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
@@ -508,52 +508,25 @@ func TestCurrentAPIVersionCachesNegotiation(t *testing.T) {
 
 	cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
+	// Ping reports the server's max API version (1.44), but after negotiation
+	// the client may settle on a lower version (1.43) — e.g. when the client
+	// SDK caps at an older version. RuntimeAPIVersion must return the negotiated
+	// ClientVersion, not the server's raw APIVersion.
 	apiClient.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}).Return(client.PingResult{
 		APIVersion: "1.44",
 	}, nil).Times(1)
 	apiClient.EXPECT().ClientVersion().Return("1.43").Times(1)
 
-	version, err := tested.CurrentAPIVersion(t.Context())
+	version, err := tested.RuntimeAPIVersion(t.Context())
 	assert.NilError(t, err)
 	assert.Equal(t, version, "1.43")
 
-	version, err = tested.CurrentAPIVersion(t.Context())
+	version, err = tested.RuntimeAPIVersion(t.Context())
 	assert.NilError(t, err)
 	assert.Equal(t, version, "1.43")
 }
 
-func TestRuntimeVersionRetriesOnTransientError(t *testing.T) {
-	mockCtrl := gomock.NewController(t)
-	defer mockCtrl.Finish()
-
-	apiClient := mocks.NewMockAPIClient(mockCtrl)
-	cli := mocks.NewMockCli(mockCtrl)
-	tested := &composeService{dockerCli: cli}
-
-	cli.EXPECT().Client().Return(apiClient).AnyTimes()
-
-	// First call: ServerVersion fails with a transient error
-	firstCall := apiClient.EXPECT().ServerVersion(gomock.Any(), gomock.Any()).
-		Return(client.ServerVersionResult{}, context.DeadlineExceeded).Times(1)
-
-	// Second call: succeeds
-	apiClient.EXPECT().ServerVersion(gomock.Any(), gomock.Any()).
-		Return(client.ServerVersionResult{APIVersion: "1.48"}, nil).Times(1).After(firstCall)
-
-	_, err := tested.RuntimeVersion(t.Context())
-	assert.ErrorIs(t, err, context.DeadlineExceeded)
-
-	version, err := tested.RuntimeVersion(t.Context())
-	assert.NilError(t, err)
-	assert.Equal(t, version, "1.48")
-
-	// Third call returns cached value
-	version, err = tested.RuntimeVersion(t.Context())
-	assert.NilError(t, err)
-	assert.Equal(t, version, "1.48")
-}
-
-func TestCurrentAPIVersionRetriesOnTransientError(t *testing.T) {
+func TestRuntimeAPIVersionRetriesOnTransientError(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
@@ -573,16 +546,16 @@ func TestCurrentAPIVersionRetriesOnTransientError(t *testing.T) {
 	apiClient.EXPECT().ClientVersion().Return("1.44").Times(1)
 
 	// First call should return the transient error
-	_, err := tested.CurrentAPIVersion(t.Context())
+	_, err := tested.RuntimeAPIVersion(t.Context())
 	assert.ErrorIs(t, err, context.DeadlineExceeded)
 
 	// Second call should succeed — error was not cached
-	version, err := tested.CurrentAPIVersion(t.Context())
+	version, err := tested.RuntimeAPIVersion(t.Context())
 	assert.NilError(t, err)
 	assert.Equal(t, version, "1.44")
 
 	// Third call should return the cached value without calling Ping again
-	version, err = tested.CurrentAPIVersion(t.Context())
+	version, err = tested.RuntimeAPIVersion(t.Context())
 	assert.NilError(t, err)
 	assert.Equal(t, version, "1.44")
 }

pkg/compose/create.go

@@ -252,7 +252,7 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
 	if err != nil {
 		return createConfigs{}, err
 	}
-	apiVersion, err := s.CurrentAPIVersion(ctx)
+	apiVersion, err := s.RuntimeAPIVersion(ctx)
 	if err != nil {
 		return createConfigs{}, err
 	}
@@ -899,7 +899,7 @@ func (s *composeService) buildContainerVolumes(
 		case mount.TypeImage:
 			// The daemon validates image mounts against the negotiated API version
 			// from the request path, not the server's own max version.
-			version, err := s.CurrentAPIVersion(ctx)
+			version, err := s.RuntimeAPIVersion(ctx)
 			if err != nil {
 				return nil, nil, err
 			}

pkg/compose/images.go

@@ -58,7 +58,7 @@ func (s *composeService) Images(ctx context.Context, projectName string, options
 
 	// The daemon validates the platform field in ImageInspect against the
 	// negotiated API version from the request path, not the server's own max version.
-	version, err := s.CurrentAPIVersion(ctx)
+	version, err := s.RuntimeAPIVersion(ctx)
 	if err != nil {
 		return nil, err
 	}