From f85394dd5d7ea27d5189f22a53eebd69e2acc7f3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 3 Jun 2025 15:39:32 +0200 Subject: [PATCH] api: image inspect: add back fields that did not omitempty commit 4dc961d0e922381b8cd5e05223f172f3145c7494 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 --- api/server/router/image/image_routes.go | 46 +++------- api/server/router/image/inspect_response.go | 88 +++++++++++++++++++ .../router/image/inspect_response_test.go | 74 ++++++++++++++++ integration-cli/docker_cli_build_test.go | 8 +- 4 files changed, 173 insertions(+), 43 deletions(-) create mode 100644 api/server/router/image/inspect_response.go create mode 100644 api/server/router/image/inspect_response_test.go 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) {