image-inspect: remove Config fields that are not part of the image

commit af0cdc36c7 marked these fields as
deprecated and to be removed in API v1.47 (which was targeted for v28.0).
We shipped v1.47 with the v27.2 release, but did not yet remove the erroneous
fields, so the version to deprecate was updated to v1.48 through
3df03d8e66

This patch removes fields that are not part of the image by replacing the
type with the Config struct from the docker image-spec.

    curl -s --unix-socket /var/run/docker.sock http://localhost/v1.50/images/alpine/json | jq .Config
    {
      "Env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
      ],
      "Cmd": [
        "/bin/sh"
      ]
    }

    curl -s --unix-socket /var/run/docker.sock http://localhost/v1.49/images/alpine/json | jq .Config
    {
      "Hostname": "",
      "Domainname": "",
      "User": "",
      "AttachStdin": false,
      "AttachStdout": false,
      "AttachStderr": false,
      "Tty": false,
      "OpenStdin": false,
      "StdinOnce": false,
      "Env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
      ],
      "Cmd": [
        "/bin/sh"
      ],
      "Image": "",
      "Volumes": null,
      "WorkingDir": "",
      "Entrypoint": null,
      "OnBuild": null,
      "Labels": null
    }

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

api/server/router/image/image_routes.go

@@ -15,6 +15,7 @@ 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"
@@ -26,6 +27,8 @@ 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"
@@ -388,6 +391,17 @@ func (ir *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWrite
 	if versions.LessThan(version, "1.48") {
 		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),
+		})
+	}
+
 	return httputils.WriteJSON(w, http.StatusOK, imageInspect)
 }
 
@@ -571,3 +585,27 @@ 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/swagger.yaml

@@ -1428,63 +1428,10 @@ definitions:
       when starting a container from the image.
     type: "object"
     properties:
-      Hostname:
-        description: |
-          The hostname to use for the container, as a valid RFC 1123 hostname.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always empty. It must not be used, and will be removed in API v1.48.
-        type: "string"
-        example: ""
-      Domainname:
-        description: |
-          The domain name to use for the container.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always empty. It must not be used, and will be removed in API v1.48.
-        type: "string"
-        example: ""
       User:
         description: "The user that commands are run as inside the container."
         type: "string"
         example: "web:web"
-      AttachStdin:
-        description: |
-          Whether to attach to `stdin`.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always false. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
-      AttachStdout:
-        description: |
-          Whether to attach to `stdout`.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always false. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
-      AttachStderr:
-        description: |
-          Whether to attach to `stderr`.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always false. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
       ExposedPorts:
         description: |
           An object mapping ports to an empty object in the form:
@@ -1501,39 +1448,6 @@ definitions:
           "80/tcp": {},
           "443/tcp": {}
         }
-      Tty:
-        description: |
-          Attach standard streams to a TTY, including `stdin` if it is not closed.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always false. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
-      OpenStdin:
-        description: |
-          Open `stdin`
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always false. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
-      StdinOnce:
-        description: |
-          Close `stdin` after one attached client disconnects.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always false. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
       Env:
         description: |
           A list of environment variables to set inside the container in the
@@ -1559,18 +1473,6 @@ definitions:
         default: false
         example: false
         x-nullable: true
-      Image:
-        description: |
-          The name (or reference) of the image to use when creating the container,
-          or which was used when the container was created.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always empty. It must not be used, and will be removed in API v1.48.
-        type: "string"
-        default: ""
-        example: ""
       Volumes:
         description: |
           An object mapping mount point paths inside the container to empty
@@ -1599,30 +1501,6 @@ definitions:
         items:
           type: "string"
         example: []
-      NetworkDisabled:
-        description: |
-          Disable networking for the container.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always omitted. It must not be used, and will be removed in API v1.48.
-        type: "boolean"
-        default: false
-        example: false
-        x-nullable: true
-      MacAddress:
-        description: |
-          MAC address of the container.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always omitted. It must not be used, and will be removed in API v1.48.
-        type: "string"
-        default: ""
-        example: ""
-        x-nullable: true
       OnBuild:
         description: |
           `ONBUILD` metadata that were defined in the image's `Dockerfile`.
@@ -1645,17 +1523,6 @@ definitions:
         type: "string"
         example: "SIGTERM"
         x-nullable: true
-      StopTimeout:
-        description: |
-          Timeout to stop a container in seconds.
-
-          <p><br /></p>
-
-          > **Deprecated**: this field is not part of the image specification and is
-          > always omitted. It must not be used, and will be removed in API v1.48.
-        type: "integer"
-        default: 10
-        x-nullable: true
       Shell:
         description: |
           Shell for when `RUN`, `CMD`, and `ENTRYPOINT` uses a shell.
@@ -1666,19 +1533,11 @@ definitions:
         example: ["/bin/sh", "-c"]
     # FIXME(thaJeztah): temporarily using a full example to remove some "omitempty" fields. Remove once the fields are removed.
     example:
-      "Hostname": ""
-      "Domainname": ""
       "User": "web:web"
-      "AttachStdin": false
-      "AttachStdout": false
-      "AttachStderr": false
       "ExposedPorts": {
         "80/tcp": {},
         "443/tcp": {}
       }
-      "Tty": false
-      "OpenStdin": false
-      "StdinOnce": false
       "Env": ["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"]
       "Cmd": ["/bin/sh"]
       "Healthcheck": {
@@ -1690,7 +1549,6 @@ definitions:
         "StartInterval": 0
       }
       "ArgsEscaped": true
-      "Image": ""
       "Volumes": {
         "/app/data": {},
         "/app/config": {}

api/types/backend/backend.go

@@ -160,7 +160,7 @@ type ImageInspectOpts struct {
 type CommitConfig struct {
 	Author              string
 	Comment             string
-	Config              *container.Config
+	Config              *container.Config // TODO(thaJeztah); change this to [dockerspec.DockerOCIImageConfig]
 	ContainerConfig     *container.Config
 	ContainerID         string
 	ContainerMountLabel string

api/types/image/image_inspect.go

@@ -3,6 +3,7 @@ package image
 import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/storage"
+	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
@@ -84,7 +85,7 @@ type InspectResponse struct {
 	// Author is the name of the author that was specified when committing the
 	// image, or as specified through MAINTAINER (deprecated) in the Dockerfile.
 	Author string
-	Config *container.Config
+	Config *dockerspec.DockerOCIImageConfig
 
 	// Architecture is the hardware CPU architecture that the image runs on.
 	Architecture string

daemon/containerd/image_inspect.go

@@ -108,8 +108,9 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts ba
 	}
 
 	if multi.Best != nil {
+		imgConfig := img.Config
 		resp.Author = img.Author
-		resp.Config = dockerOCIImageConfigToContainerConfig(img.Config)
+		resp.Config = &imgConfig
 		resp.Architecture = img.Architecture
 		resp.Variant = img.Variant
 		resp.Os = img.OS

daemon/images/image_inspect.go

@@ -53,6 +53,7 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts ba
 		layers = append(layers, l.String())
 	}
 
+	imgConfig := containerConfigToDockerOCIImageConfig(img.Config)
 	return &imagetypes.InspectResponse{
 		ID:              img.ID().String(),
 		RepoTags:        repoTags,
@@ -64,7 +65,7 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts ba
 		ContainerConfig: &img.ContainerConfig, //nolint:staticcheck // ignore SA1019: field is deprecated, but still set on API < v1.45.
 		DockerVersion:   img.DockerVersion,
 		Author:          img.Author,
-		Config:          img.Config,
+		Config:          &imgConfig,
 		Architecture:    img.Architecture,
 		Variant:         img.Variant,
 		Os:              img.OperatingSystem(),

daemon/images/imagespec.go

@@ -0,0 +1,41 @@
+package images
+
+import (
+	"github.com/docker/docker/api/types/container"
+	imagespec "github.com/moby/docker-image-spec/specs-go/v1"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
+func containerConfigToDockerOCIImageConfig(cfg *container.Config) imagespec.DockerOCIImageConfig {
+	var ociCfg ocispec.ImageConfig
+	var ext imagespec.DockerOCIImageConfigExt
+
+	if cfg != nil {
+		ociCfg = ocispec.ImageConfig{
+			User:        cfg.User,
+			Env:         cfg.Env,
+			Entrypoint:  cfg.Entrypoint,
+			Cmd:         cfg.Cmd,
+			Volumes:     cfg.Volumes,
+			WorkingDir:  cfg.WorkingDir,
+			Labels:      cfg.Labels,
+			StopSignal:  cfg.StopSignal,
+			ArgsEscaped: cfg.ArgsEscaped, //nolint:staticcheck // Ignore SA1019. Need to keep it in image.
+		}
+
+		if len(cfg.ExposedPorts) > 0 {
+			ociCfg.ExposedPorts = map[string]struct{}{}
+			for k, v := range cfg.ExposedPorts {
+				ociCfg.ExposedPorts[string(k)] = v
+			}
+		}
+		ext.Healthcheck = cfg.Healthcheck
+		ext.OnBuild = cfg.OnBuild
+		ext.Shell = cfg.Shell
+	}
+
+	return imagespec.DockerOCIImageConfig{
+		ImageConfig:             ociCfg,
+		DockerOCIImageConfigExt: ext,
+	}
+}

docs/api/version-history.md

@@ -24,6 +24,14 @@ keywords: "API, Docker, rcli, REST, documentation"
 * Deprecated: The `BridgeNfIptables` and `BridgeNfIp6tables` fields in the
   `GET /info` response were deprecated in API v1.48, and are now omitted
   in API v1.50.
+* Deprecated: `GET /images/{name}/json` no longer returns the following `Config`
+  fields; `Hostname`, `Domainname`, `AttachStdin`, `AttachStdout`, `AttachStderr`
+  `Tty`, `OpenStdin`, `StdinOnce`, `Image`, `NetworkDisabled` (already omitted unless set),
+  `MacAddress` (already omitted unless set), `StopTimeout` (already omitted unless set).
+  These additional fields were included in the response due to an implementation
+  detail but not part of the image's Configuration. These fields were marked
+  deprecated in API v1.46, and are now omitted. Older versions of the API still
+  return these fields, but they are always empty.
 
 ## v1.49 API changes
 

hack/make/test-docker-py

@@ -22,6 +22,9 @@ if [ -n "$TEST_INTEGRATION_USE_SNAPSHOTTER" ]; then
 	PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_build_test.py::BuildTest::test_build_squash"
 fi
 
+# TODO(thaJeztah) re-enable after https://github.com/docker/docker-py/pull/3336 is in the DOCKER_PY_COMMIT release.
+PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_build_test.py::BuildTest::test_build_container_with_target"
+
 # TODO(vvoland): re-enable after https://github.com/docker/docker-py/pull/3203 is included in the DOCKER_PY_COMMIT release.
 PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_image_test.py::CommitTest::test_commit"
 PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_image_test.py::CommitTest::test_commit_with_changes"

integration-cli/docker_cli_by_digest_test.go

@@ -166,7 +166,6 @@ func (s *DockerRegistrySuite) TestRemoveImageByDigest(c *testing.T) {
 }
 
 func (s *DockerRegistrySuite) TestBuildByDigest(c *testing.T) {
-	skip.If(c, testEnv.UsingSnapshotter(), "Config.Image is not created with containerd, buildkit doesn't set it either")
 	imgDigest, err := setupImage(c)
 	assert.NilError(c, err, "error setting up image")
 
@@ -175,9 +174,6 @@ func (s *DockerRegistrySuite) TestBuildByDigest(c *testing.T) {
 	// pull from the registry using the <name>@<digest> reference
 	cli.DockerCmd(c, "pull", imageReference)
 
-	// get the image id
-	imageID := inspectField(c, imageReference, "Id")
-
 	// do the build
 	const name = "buildbydigest"
 	buildImageSuccessfully(c, name, build.WithDockerfile(fmt.Sprintf(
@@ -185,10 +181,9 @@ func (s *DockerRegistrySuite) TestBuildByDigest(c *testing.T) {
      CMD ["/bin/echo", "Hello World"]`, imageReference)))
 	assert.NilError(c, err)
 
-	// get the build's image id
+	// verify the build was ok
-	res := inspectField(c, name, "Config.Image")
+	res := inspectField(c, name, "Config.Cmd")
-	// make sure they match
+	assert.Equal(c, res, `[/bin/echo Hello World]`)
-	assert.Equal(c, res, imageID)
 }
 
 func (s *DockerRegistrySuite) TestTagByDigest(c *testing.T) {