From f594a7f09bfae5a3b5f838b0cc5d6274d2da3a24 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 27 Oct 2025 20:52:19 +0100 Subject: [PATCH] cli/command/image: remove uses of JSON field The JSON field was added in [moby@9fd2c0f], to address [moby#19177], which reported an incompatibility with Classic (V1) Swarm, which produced a non- standard response; > Make docker load to output json when the response content type is json > Swarm hijacks the response from docker load and returns JSON rather > than plain text like the Engine does. This makes the API library to return > information to figure that out. A later change in [moby@96d7db6] added additional logic to make sure the correct content-type was returned, depending on whether the `quiet` option was set (which produced a non-JSON response). This caused inconsistency in the API response, and [moby@2f27632] changed the endpoint to always produce JSON (only skipping the "progress" output if `quiet` was set). This means that the "load" endpoint ([`imageRouter.postImagesLoad`]) now unconditionally returns JSON, making the `JSON` field fully redundant. This patch removes the use of the JSON field, as it's redundant, and the way it handles the content-type is incorrect because it would not handle correct, but different formatted response-headers (`application/json; charset=utf-8`), which could result in malformed output on the client. [moby@9fd2c0f]: https://github.com/moby/moby/commit/9fd2c0feb0c131d01d727d50baa7183b976c7bdc [moby#19177]: https://github.com/moby/moby/issues/19177 [moby@96d7db6]: https://github.com/moby/moby/commit/96d7db665b06cc0bbede22d818c69dc5f6921f66 [moby@2f27632]: https://github.com/moby/moby/commit/2f27632cde2f0e514bd3a8de77cc1934e5193a83 [`imageRouter.postImagesLoad`]: https://github.com/moby/moby/blob/7b9d2ef6e5518a3d3f3cc418459f8df786cfbbd1/api/server/router/image/image_routes.go#L248-L255 Signed-off-by: Sebastiaan van Stijn --- cli/command/image/load.go | 11 ++---- cli/command/image/load_test.go | 34 +++++++------------ .../load-command-success.input-file.golden | 2 +- .../testdata/load-command-success.json.golden | 1 - .../load-command-success.simple.golden | 2 +- ...cess.with-comma-separated-platforms.golden | 2 +- ...cess.with-multiple-platform-options.golden | 2 +- ...ommand-success.with-single-platform.golden | 2 +- 8 files changed, 20 insertions(+), 36 deletions(-) delete mode 100644 cli/command/image/testdata/load-command-success.json.golden diff --git a/cli/command/image/load.go b/cli/command/image/load.go index 4bbf26069..4fc939466 100644 --- a/cli/command/image/load.go +++ b/cli/command/image/load.go @@ -70,7 +70,7 @@ func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error if err != nil { return err } - defer file.Close() + defer func() { _ = file.Close() }() input = file } @@ -95,12 +95,7 @@ func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error if err != nil { return err } - defer res.Close() + defer func() { _ = res.Close() }() - if res.JSON { - return jsonstream.Display(ctx, res, dockerCli.Out()) - } - - _, err = io.Copy(dockerCli.Out(), res) - return err + return jsonstream.Display(ctx, res, dockerCli.Out()) } diff --git a/cli/command/image/load_test.go b/cli/command/image/load_test.go index 91f4273cd..78c1ca045 100644 --- a/cli/command/image/load_test.go +++ b/cli/command/image/load_test.go @@ -74,8 +74,8 @@ func TestNewLoadCommandInvalidInput(t *testing.T) { assert.ErrorContains(t, err, expectedError) } -func mockImageLoadResult(content string, json bool) client.ImageLoadResult { - out := client.ImageLoadResult{JSON: json} +func mockImageLoadResult(content string) client.ImageLoadResult { + out := client.ImageLoadResult{} // Set unexported field "body" v := reflect.ValueOf(&out).Elem() @@ -94,22 +94,12 @@ func TestNewLoadCommandSuccess(t *testing.T) { { name: "simple", args: []string{}, - imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (client.ImageLoadResult, error) { - // FIXME(thaJeztah): how to mock this? - // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader("Success"))}, nil - return mockImageLoadResult(`Success`, false), nil - }, - }, - { - name: "json", - args: []string{}, imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (client.ImageLoadResult, error) { // FIXME(thaJeztah): how to mock this? // return client.ImageLoadResult{ - // Body: io.NopCloser(strings.NewReader(`{"ID": "1"}`)), - // JSON: true, + // Body: io.NopCloser(strings.NewReader(`{"ID":"simple","Status":"success"}`)), // }, nil - return mockImageLoadResult(`{"ID":"1"}`, true), nil + return mockImageLoadResult(`{"ID":"simple","Status":"success"}`), nil }, }, { @@ -117,8 +107,8 @@ func TestNewLoadCommandSuccess(t *testing.T) { args: []string{"--input", "testdata/load-command-success.input.txt"}, imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (client.ImageLoadResult, error) { // FIXME(thaJeztah): how to mock this? - // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader("Success"))}, nil - return mockImageLoadResult(`Success`, false), nil + // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader(`{"ID":"input-file","Status":"success"}`))}, nil + return mockImageLoadResult(`{"ID":"input-file","Status":"success"}`), nil }, }, { @@ -129,8 +119,8 @@ func TestNewLoadCommandSuccess(t *testing.T) { assert.Check(t, len(options) > 0) // can be 1 or two depending on whether a terminal is attached :/ // assert.Check(t, is.Contains(options, client.ImageHistoryWithPlatform(ocispec.Platform{OS: "linux", Architecture: "amd64"}))) // FIXME(thaJeztah): how to mock this? - // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader("Success"))}, nil - return mockImageLoadResult(`Success`, false), nil + // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader(`{"ID":"single-platform","Status":"success"}`))}, nil + return mockImageLoadResult(`{"ID":"single-platform","Status":"success"}`), nil }, }, { @@ -139,8 +129,8 @@ func TestNewLoadCommandSuccess(t *testing.T) { imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (client.ImageLoadResult, error) { assert.Check(t, len(options) > 0) // can be 1 or two depending on whether a terminal is attached :/ // FIXME(thaJeztah): how to mock this? - // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader("Success"))}, nil - return mockImageLoadResult(`Success`, false), nil + // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader(`{"ID":"with-comma-separated-platforms","Status":"success"}`))}, nil + return mockImageLoadResult(`{"ID":"with-comma-separated-platforms","Status":"success"}`), nil }, }, { @@ -149,8 +139,8 @@ func TestNewLoadCommandSuccess(t *testing.T) { imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (client.ImageLoadResult, error) { assert.Check(t, len(options) > 0) // can be 1 or two depending on whether a terminal is attached :/ // FIXME(thaJeztah): how to mock this? - // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader("Success"))}, nil - return mockImageLoadResult(`Success`, false), nil + // return client.ImageLoadResult{Body: io.NopCloser(strings.NewReader(`{"ID":"with-multiple-platform-options","Status":"success"}`))}, nil + return mockImageLoadResult(`{"ID":"with-multiple-platform-options","Status":"success"}`), nil }, }, } diff --git a/cli/command/image/testdata/load-command-success.input-file.golden b/cli/command/image/testdata/load-command-success.input-file.golden index 51da4200a..0f454425f 100644 --- a/cli/command/image/testdata/load-command-success.input-file.golden +++ b/cli/command/image/testdata/load-command-success.input-file.golden @@ -1 +1 @@ -Success \ No newline at end of file +input-file: success diff --git a/cli/command/image/testdata/load-command-success.json.golden b/cli/command/image/testdata/load-command-success.json.golden deleted file mode 100644 index c17f16ecd..000000000 --- a/cli/command/image/testdata/load-command-success.json.golden +++ /dev/null @@ -1 +0,0 @@ -1: diff --git a/cli/command/image/testdata/load-command-success.simple.golden b/cli/command/image/testdata/load-command-success.simple.golden index 51da4200a..f503a70c6 100644 --- a/cli/command/image/testdata/load-command-success.simple.golden +++ b/cli/command/image/testdata/load-command-success.simple.golden @@ -1 +1 @@ -Success \ No newline at end of file +simple: success diff --git a/cli/command/image/testdata/load-command-success.with-comma-separated-platforms.golden b/cli/command/image/testdata/load-command-success.with-comma-separated-platforms.golden index 51da4200a..9cd1f6fc3 100644 --- a/cli/command/image/testdata/load-command-success.with-comma-separated-platforms.golden +++ b/cli/command/image/testdata/load-command-success.with-comma-separated-platforms.golden @@ -1 +1 @@ -Success \ No newline at end of file +with-comma-separated-platforms: success diff --git a/cli/command/image/testdata/load-command-success.with-multiple-platform-options.golden b/cli/command/image/testdata/load-command-success.with-multiple-platform-options.golden index 51da4200a..172a51a1a 100644 --- a/cli/command/image/testdata/load-command-success.with-multiple-platform-options.golden +++ b/cli/command/image/testdata/load-command-success.with-multiple-platform-options.golden @@ -1 +1 @@ -Success \ No newline at end of file +with-multiple-platform-options: success diff --git a/cli/command/image/testdata/load-command-success.with-single-platform.golden b/cli/command/image/testdata/load-command-success.with-single-platform.golden index 51da4200a..59ff47762 100644 --- a/cli/command/image/testdata/load-command-success.with-single-platform.golden +++ b/cli/command/image/testdata/load-command-success.with-single-platform.golden @@ -1 +1 @@ -Success \ No newline at end of file +single-platform: success