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 <github@gone.nl>

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())
 }

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
 			},
 		},
 	}

cli/command/image/testdata/load-command-success.input-file.golden

@@ -1 +1 @@
-Success
+input-file: success

cli/command/image/testdata/load-command-success.json.golden

@@ -1 +0,0 @@
-1: 

cli/command/image/testdata/load-command-success.simple.golden

@@ -1 +1 @@
-Success
+simple: success

cli/command/image/testdata/load-command-success.with-comma-separated-platforms.golden

@@ -1 +1 @@
-Success
+with-comma-separated-platforms: success

cli/command/image/testdata/load-command-success.with-multiple-platform-options.golden

@@ -1 +1 @@
-Success
+with-multiple-platform-options: success

cli/command/image/testdata/load-command-success.with-single-platform.golden

@@ -1 +1 @@
-Success
+single-platform: success