api: image inspect: add back fields that did not omitempty

commit 4dc961d0e9 removed deprecated
fields from the image inspect response for API v1.50 and up. As
part of that change, it changed the type used for the Config field
to use the docker image spect structs, which embeds the OCI image
spec structs.

While the OCI image spect struct contains the same fields as we
used before, those fields also have "omitempty" set, which means
they are now omitted when empty.

We should probably consider deprecating that behavior in the API,
and call out that these fields are omitted if not set, but in the
meantime, we can add them back with their default (zero) value.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/server/router/image/image_routes.go

@@ -15,7 +15,6 @@ import (
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/server/httputils"
 	"github.com/docker/docker/api/types/backend"
-	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
 	imagetypes "github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/api/types/registry"
@@ -27,8 +26,6 @@ import (
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
-	"github.com/docker/go-connections/nat"
-	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -370,7 +367,7 @@ func (ir *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWrite
 		return errdefs.InvalidParameter(errors.New("conflicting options: manifests and platform options cannot both be set"))
 	}
 
-	imageInspect, err := ir.backend.ImageInspect(ctx, vars["name"], backend.ImageInspectOpts{
+	resp, err := ir.backend.ImageInspect(ctx, vars["name"], backend.ImageInspectOpts{
 		Manifests: manifests,
 		Platform:  platform,
 	})
@@ -378,6 +375,14 @@ func (ir *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWrite
 		return err
 	}
 
+	// inspectResponse preserves fields in the response that have an
+	// "omitempty" in the OCI spec, but didn't omit such fields in
+	// legacy responses before API v1.50.
+	imageInspect := &inspectCompatResponse{
+		InspectResponse: resp,
+		legacyConfig:    legacyConfigFields["current"],
+	}
+
 	// Make sure we output empty arrays instead of nil. While Go nil slice is functionally equivalent to an empty slice,
 	// it matters for the JSON representation.
 	if imageInspect.RepoTags == nil {
@@ -405,14 +410,7 @@ func (ir *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWrite
 		imageInspect.Descriptor = nil
 	}
 	if versions.LessThan(version, "1.50") {
-		type imageInspectLegacy struct {
-			imagetypes.InspectResponse
-			LegacyConfig *container.Config `json:"Config"`
-		}
-		return httputils.WriteJSON(w, http.StatusOK, imageInspectLegacy{
-			InspectResponse: *imageInspect,
-			LegacyConfig:    dockerOCIImageConfigToContainerConfig(*imageInspect.Config),
-		})
+		imageInspect.legacyConfig = legacyConfigFields["v1.49"]
 	}
 
 	return httputils.WriteJSON(w, http.StatusOK, imageInspect)
@@ -598,27 +596,3 @@ func validateRepoName(name reference.Named) error {
 	}
 	return nil
 }
-
-// FIXME(thaJeztah): this is a copy of dockerOCIImageConfigToContainerConfig in daemon/containerd: https://github.com/moby/moby/blob/6b617699c500522aa6526cfcae4558333911b11f/daemon/containerd/imagespec.go#L107-L128
-func dockerOCIImageConfigToContainerConfig(cfg dockerspec.DockerOCIImageConfig) *container.Config {
-	exposedPorts := make(nat.PortSet, len(cfg.ExposedPorts))
-	for k, v := range cfg.ExposedPorts {
-		exposedPorts[nat.Port(k)] = v
-	}
-
-	return &container.Config{
-		Entrypoint:   cfg.Entrypoint,
-		Env:          cfg.Env,
-		Cmd:          cfg.Cmd,
-		User:         cfg.User,
-		WorkingDir:   cfg.WorkingDir,
-		ExposedPorts: exposedPorts,
-		Volumes:      cfg.Volumes,
-		Labels:       cfg.Labels,
-		ArgsEscaped:  cfg.ArgsEscaped, //nolint:staticcheck // Ignore SA1019. Need to keep it in image.
-		StopSignal:   cfg.StopSignal,
-		Healthcheck:  cfg.Healthcheck,
-		OnBuild:      cfg.OnBuild,
-		Shell:        cfg.Shell,
-	}
-}

api/server/router/image/inspect_response.go

@@ -0,0 +1,88 @@
+// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
+//go:build go1.23
+
+package image
+
+import (
+	"encoding/json"
+	"maps"
+
+	"github.com/docker/docker/api/types/image"
+)
+
+// legacyConfigFields defines legacy image-config fields to include in
+// API responses on older API versions.
+var legacyConfigFields = map[string]map[string]any{
+	// Legacy fields for API v1.49 and lower. These fields are deprecated
+	// and omitted in newer API versions; see https://github.com/moby/moby/pull/48457
+	"v1.49": {
+		"AttachStderr": false,
+		"AttachStdin":  false,
+		"AttachStdout": false,
+		"Cmd":          nil,
+		"Domainname":   "",
+		"Entrypoint":   nil,
+		"Env":          nil,
+		"Hostname":     "",
+		"Image":        "",
+		"Labels":       nil,
+		"OnBuild":      nil,
+		"OpenStdin":    false,
+		"StdinOnce":    false,
+		"Tty":          false,
+		"User":         "",
+		"Volumes":      nil,
+		"WorkingDir":   "",
+	},
+	// Legacy fields for current API versions (v1.50 and up). These fields
+	// did not have an "omitempty" and were always included in the response,
+	// even if not set; see https://github.com/moby/moby/issues/50134
+	"current": {
+		"Cmd":        nil,
+		"Entrypoint": nil,
+		"Env":        nil,
+		"Labels":     nil,
+		"OnBuild":    nil,
+		"User":       "",
+		"Volumes":    nil,
+		"WorkingDir": "",
+	},
+}
+
+// inspectCompatResponse is a wrapper around [image.InspectResponse] with a
+// custom marshal function for legacy [api/types/container.Config} fields
+// that have been removed, or did not have omitempty.
+type inspectCompatResponse struct {
+	*image.InspectResponse
+	legacyConfig map[string]any
+}
+
+// MarshalJSON implements a custom marshaler to include legacy fields
+// in API responses.
+func (ir *inspectCompatResponse) MarshalJSON() ([]byte, error) {
+	type tmp *image.InspectResponse
+	base, err := json.Marshal((tmp)(ir.InspectResponse))
+	if err != nil {
+		return nil, err
+	}
+	if len(ir.legacyConfig) == 0 {
+		return base, nil
+	}
+
+	type resp struct {
+		*image.InspectResponse
+		Config map[string]any
+	}
+
+	var merged resp
+	err = json.Unmarshal(base, &merged)
+	if err != nil {
+		return base, nil
+	}
+
+	// prevent mutating legacyConfigFields.
+	cfg := maps.Clone(ir.legacyConfig)
+	maps.Copy(cfg, merged.Config)
+	merged.Config = cfg
+	return json.Marshal(merged)
+}

api/server/router/image/inspect_response_test.go

@@ -0,0 +1,74 @@
+package image
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/docker/docker/api/types/image"
+	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestInspectResponse(t *testing.T) {
+	tests := []struct {
+		doc          string
+		cfg          *ocispec.ImageConfig
+		legacyConfig map[string]any
+		expected     string
+	}{
+		{
+			doc:      "empty",
+			expected: `null`,
+		},
+		{
+			doc: "no legacy config",
+			cfg: &ocispec.ImageConfig{
+				Cmd:        []string{"/bin/sh"},
+				StopSignal: "SIGQUIT",
+			},
+			expected: `{"Cmd":["/bin/sh"],"StopSignal":"SIGQUIT"}`,
+		},
+		{
+			doc: "api < v1.50",
+			cfg: &ocispec.ImageConfig{
+				Cmd:        []string{"/bin/sh"},
+				StopSignal: "SIGQUIT",
+			},
+			legacyConfig: legacyConfigFields["v1.49"],
+			expected:     `{"AttachStderr":false,"AttachStdin":false,"AttachStdout":false,"Cmd":["/bin/sh"],"Domainname":"","Entrypoint":null,"Env":null,"Hostname":"","Image":"","Labels":null,"OnBuild":null,"OpenStdin":false,"StdinOnce":false,"StopSignal":"SIGQUIT","Tty":false,"User":"","Volumes":null,"WorkingDir":""}`,
+		},
+		{
+			doc: "api >= v1.50",
+			cfg: &ocispec.ImageConfig{
+				Cmd:        []string{"/bin/sh"},
+				StopSignal: "SIGQUIT",
+			},
+			legacyConfig: legacyConfigFields["current"],
+			expected:     `{"Cmd":["/bin/sh"],"Entrypoint":null,"Env":null,"Labels":null,"OnBuild":null,"StopSignal":"SIGQUIT","User":"","Volumes":null,"WorkingDir":""}`,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			imgInspect := &image.InspectResponse{}
+			if tc.cfg != nil {
+				// Verify that fields that are set override the legacy values,
+				// or appended if not part of the legacy values.
+				imgInspect.Config = &dockerspec.DockerOCIImageConfig{
+					ImageConfig: *tc.cfg,
+				}
+			}
+			out, err := json.Marshal(&inspectCompatResponse{
+				InspectResponse: imgInspect,
+				legacyConfig:    tc.legacyConfig,
+			})
+			assert.NilError(t, err)
+
+			var outMap struct{ Config json.RawMessage }
+			err = json.Unmarshal(out, &outMap)
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(string(outMap.Config), tc.expected))
+		})
+	}
+}

integration-cli/docker_cli_build_test.go

@@ -3138,13 +3138,7 @@ func (s *DockerCLIBuildSuite) TestBuildClearCmd(c *testing.T) {
    CMD []`))
 
 	cmd := inspectFieldJSON(c, name, "Config.Cmd")
-	// OCI types specify `omitempty` JSON annotation which doesn't serialize
-	// empty arrays and the Cmd will not be present at all.
-	if testEnv.UsingSnapshotter() {
-		assert.Check(c, is.Equal(cmd, "null"))
-	} else {
-		assert.Check(c, is.Equal(cmd, "[]"))
-	}
+	assert.Check(c, is.Equal(cmd, "null"))
 }
 
 func (s *DockerCLIBuildSuite) TestBuildEmptyCmd(c *testing.T) {