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

thaJeztah · 2024-09-10T08:20:42Z

relates to:

commit af0cdc3 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 3df03d8

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.47/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.46/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
}

- What I did

- How I did it

- How to verify it

- Description for the changelog

API: Deprecated: `GET /images/{name}/json` no longer returns the following fields: `Config`, `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, were marked deprecated in API v1.46, and are now omitted.

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2024-09-10T08:22:04Z

Keeping this in draft for now; it looks like the image-spec types use omitempty for most fields, which means that fields that were not set will be omitted.

thaJeztah · 2024-09-10T08:22:47Z

api/server/router/image/image_routes.go

+// 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 {


This was copied from

moby/daemon/containerd/imagespec.go

Lines 107 to 128 in 6b61769

func dockerOCIImageConfigToContainerConfig(cfg imagespec.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,

}

}

We should probably look at moving that to an internal/ package so that we can re-use those functions

Or even move it to https://github.com/moby/docker-image-spec? 🤔

Yeah, but that would require a docker-specific type to be imported there, so probably won't work.

thaJeztah · 2024-09-10T08:24:10Z

daemon/images/image_inspect.go

@@ -53,6 +53,7 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, _ backe
 		layers = append(layers, l.String())
 	}

+	imgConfig := containerConfigToDockerOCIImageConfig(img.Config)


Also wondering if we should consider (and if it'd be possible) to change the image-type we store on disk to use the new type so that we don't have to perform conversion (other than when committing a container to an image)

thaJeztah · 2024-09-10T08:32:19Z

Looks like docker-py may need an update, as I think it's checking for an empty OnBuild field to be present;

=================================== FAILURES ===================================
__________________ BuildTest.test_build_container_with_target __________________
tests/integration/api_build_test.py:278: in test_build_container_with_target
    assert not info['Config']['OnBuild']
E   KeyError: 'OnBuild'

edit: hm.. not sure now; it's assert not ? https://github.com/docker/docker-py/blob/a3652028b1ead708bd9191efb286f909ba6c2a49/tests/integration/api_build_test.py#L258-L278

edit2: ah; It's building the first target, which doesn't have OnBuild, so indeed it tries to verify that OnBuild is empty, but fails because the fields is not there (due to the omitempty.

☝️ opened integration: adjust tests for omitted "OnBuild" docker/docker-py#3336

thaJeztah · 2025-05-22T08:47:31Z

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.pyy::BuildTest::test_build_container_with_target"


Arf;

Suggested change

PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_build_test.pyy::BuildTest::test_build_container_with_target"

PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_build_test.py::BuildTest::test_build_container_with_target"

Maybe instead of skipping, let's just pull docker-py by commit?

moby/hack/make/test-docker-py

Line 8 in 04d3817

: "${DOCKER_PY_COMMIT:=7.1.0}"

Yeah, there's a bunch of other tests that are also waiting for a new release; didn't want to include all of those in this PR (and ideally, look if we can have a new release of docker-py); let me keep that for a follow-up.

Mostly dusted-off this PR because we kept on forgetting about it 😂

Either way, fixed the typo for now 👍

vvoland · 2025-05-22T09:25:32Z

Looks like test is failing:

=== FAIL: amd64.integration-cli TestDockerRegistrySuite/TestBuildByDigest (0.81s)
    docker_cli_by_digest_test.go:191: assertion failed:  (res string) != sha256:56f26145b42bd638db4caef515cd8c72c9acfba61961545d7fe2ef0f4bc38f9e (imageID string)
    check_test.go:468: [db544858fd8a5] daemon is not started
    --- FAIL: TestDockerRegistrySuite/TestBuildByDigest (0.81s)

thaJeztah · 2025-05-22T09:58:40Z

Heh. Looks like that test piggy-backed on the Config.Image field being set (from the container's config);

moby/integration-cli/docker_cli_by_digest_test.go

Lines 188 to 189 in f4ffeb8

    
           // get the build's image id 
        
           res := inspectField(c, name, "Config.Image")

thaJeztah · 2025-05-22T10:11:10Z

integration-cli/docker_cli_by_digest_test.go

+	// verify the build was ok
+	res := inspectField(c, name, "Config.Cmd")
+	assert.Equal(c, res, `[/bin/echo Hello World]`)


Changed it to a sanity check that the build indeed "built" (may even be redundant)

make BIND_DIR=. TEST_FILTER='TestBuildByDigest' test-integration ... === RUN TestDockerRegistrySuite/TestBuildByDigest check_test.go:468: [de4fbab56e382] daemon is not started --- PASS: TestDockerRegistrySuite (1.80s) --- PASS: TestDockerRegistrySuite/TestBuildByDigest (1.80s)

thaJeztah · 2025-05-26T10:27:15Z

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]


Possibly this would help reduce the conversion juggling - I'd have to check though exactly where this field is all used 🤔

vvoland · 2025-05-26T14:48:46Z

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: `GT /images/{name}/json` no longer returns the following `Config`


Suggested change

* Deprecated: `GT /images/{name}/json` no longer returns the following `Config`

* Deprecated: `GET /images/{name}/json` no longer returns the following `Config`

commit af0cdc3 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 3df03d8 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 <[email protected]>

Signed-off-by: Sebastiaan van Stijn <[email protected]>

JonathonReinhart · 2025-06-08T07:16:31Z

Was it actually intended that Entrypoint is now omitted from the output, if it is not specified in the Dockerfile?

This change resulted in a bug in Scuba (JonathonReinhart/scuba#262) which was essentially doing this:

cp = subprocess.run("docker", "inspect", "--type", "image", image, capture=True)
info = json.loads(cp.stdout)[0]
entrypoint = info["Config"]["Entrypoint"]
#                           ^^^^^^^^^^^^  KeyError on docker-ce 28.2

The commit messages for 4dc961d and referenced depreciation commit af0cdc3 only talk about erroneous fields not part of the image; it is never mentioned that the behavior for actual image fields (e.g. Entrypoint) will be modified. Yet AFAICT this PR caused the field to be absent (rather than none) if not set.

This seems like a regression to me.

Edit: This appears to apply to Cmd, too.

Without Entrypoint

https://github.com/JonathonReinhart/scuba/blob/main/test-docker-images/hello/Dockerfile

FROM alpine
ADD hello.sh /hello.sh
RUN chmod 755 /hello.sh
CMD ["/hello.sh"]

$ docker inspect --type image scuba/hello | jq '.[0].Config'
{
  "Env": [
    "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
  ],
  "Cmd": [
    "/hello.sh"
  ],
  "WorkingDir": "/",
  "ArgsEscaped": true
}

With Entrypoint

https://github.com/JonathonReinhart/scuba/blob/main/test-docker-images/entrypoint-test/Dockerfile

FROM alpine:latest
COPY entrypoint.sh /entrypoint.sh
RUN chmod 755 /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]

$ docker inspect --type image scuba/entrypoint-test | jq '.[0].Config'
{
  "Env": [
    "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
  ],
  "Entrypoint": [
    "/entrypoint.sh"
  ],
  "WorkingDir": "/"
}

thaJeztah · 2025-06-11T11:02:00Z

@JonathonReinhart see;

Empty values will likely be omitted in future though, so In general, I'd recommend adjusting how those are handled to either unmarshal to a type on the receiving end, or to handle missing fields gracefully. Omitting the fields is a closer alignment to the OCI image specification, which also omits Config options that are not set; https://github.com/opencontainers/image-spec/blob/v1.1.1/specs-go/v1/config.go#L23-L62

thaJeztah commented Sep 10, 2024

View reviewed changes

thaJeztah force-pushed the api_remove_deprecated_fields branch 5 times, most recently from 3df5ef6 to 6feb664 Compare May 21, 2025 12:50

thaJeztah added area/api status/2-code-review impact/api impact/changelog impact/deprecation area/go-sdk Changes affecting the Go SDK labels May 21, 2025

thaJeztah marked this pull request as ready for review May 21, 2025 12:52

thaJeztah force-pushed the api_remove_deprecated_fields branch from 6feb664 to c6e7214 Compare May 21, 2025 23:43

thaJeztah requested a review from tianon as a code owner May 21, 2025 23:43

thaJeztah mentioned this pull request May 21, 2025

integration: adjust tests for omitted "OnBuild" docker/docker-py#3336

Merged

thaJeztah force-pushed the api_remove_deprecated_fields branch from c6e7214 to 4b7dd37 Compare May 21, 2025 23:49

thaJeztah added this to the 28.2.0 milestone May 21, 2025

thaJeztah requested a review from vvoland May 21, 2025 23:55

thaJeztah commented May 22, 2025

View reviewed changes

thaJeztah force-pushed the api_remove_deprecated_fields branch from 4b7dd37 to 9da4321 Compare May 22, 2025 08:59

thaJeztah force-pushed the api_remove_deprecated_fields branch from 9da4321 to 505bd61 Compare May 22, 2025 10:09

thaJeztah commented May 22, 2025

View reviewed changes

This was referenced May 22, 2025

vendor: github.com/docker/docker 26db31fdab62 (v28.2-dev) docker/cli#6099

Merged

docs/api: Cut docs for API v1.50 #50069

Merged

vvoland approved these changes May 26, 2025

View reviewed changes

thaJeztah commented May 26, 2025

View reviewed changes

vvoland reviewed May 26, 2025

View reviewed changes

thaJeztah added 2 commits May 26, 2025 17:14

docs/api: update deprecation version for erroneous fields

d7cca3f

Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the api_remove_deprecated_fields branch from 505bd61 to d7cca3f Compare May 26, 2025 15:16

vvoland merged commit d0ad135 into moby:master May 26, 2025
160 checks passed

thaJeztah deleted the api_remove_deprecated_fields branch May 26, 2025 16:06

thaJeztah mentioned this pull request May 27, 2025

docs: deprecated: update status for non-standard fields in image inspect docker/cli#6111

Merged

vitus-prediger mentioned this pull request Jun 2, 2025

docker inspect_image API response changed after upgrading to 1.11.1 orbstack/orbstack#1963

Closed

This was referenced Jun 3, 2025

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

Merged

fix thelper linter #50022

Merged

JonathonReinhart mentioned this pull request Jun 8, 2025

Upgrading docker-ce=5:28.2.1-1~debian.12~bookworm makes scuba fail JonathonReinhart/scuba#262

Closed

		// 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 {

	func dockerOCIImageConfigToContainerConfig(cfg imagespec.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,
	}
	}

	PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_build_test.pyy::BuildTest::test_build_container_with_target"
	PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_build_test.py::BuildTest::test_build_container_with_target"

	* Deprecated: `GT /images/{name}/json` no longer returns the following `Config`
	* Deprecated: `GET /images/{name}/json` no longer returns the following `Config`

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

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

Uh oh!

Conversation

thaJeztah commented Sep 10, 2024 • edited by vvoland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vvoland May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vvoland May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vvoland commented May 22, 2025

Uh oh!

thaJeztah commented May 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JonathonReinhart commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Without Entrypoint

With Entrypoint

Uh oh!

thaJeztah commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

thaJeztah commented Sep 10, 2024 •

edited by vvoland

Loading

thaJeztah commented Sep 10, 2024 •

edited

Loading

vvoland May 26, 2025 •

edited

Loading

thaJeztah commented Sep 10, 2024 •

edited

Loading

vvoland May 22, 2025 •

edited

Loading

JonathonReinhart commented Jun 8, 2025 •

edited

Loading

thaJeztah commented Jun 11, 2025 •

edited

Loading