diff --git a/api/server/router/image/image_routes.go b/api/server/router/image/image_routes.go index 1573fa1ce5..2e8a94b98e 100644 --- a/api/server/router/image/image_routes.go +++ b/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, - } -} diff --git a/api/server/router/image/inspect_response.go b/api/server/router/image/inspect_response.go new file mode 100644 index 0000000000..a188af4d81 --- /dev/null +++ b/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) +} diff --git a/api/server/router/image/inspect_response_test.go b/api/server/router/image/inspect_response_test.go new file mode 100644 index 0000000000..acfd7bbd30 --- /dev/null +++ b/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)) + }) + } +} diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 59f1fab57f..8cdfa294b8 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/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) {