client: remove ImageLoadResult.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.

We should consider deprecating the "quiet" option, as it's really the client's
responsibility to show or hide progress-bars, but we can do this separately.

This patch removes 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]: 9fd2c0feb0
[moby#19177]: https://github.com/moby/moby/issues/19177
[moby@96d7db6]: 96d7db665b
[moby@2f27632]: 2f27632cde
[`imageRouter.postImagesLoad`]: 7b9d2ef6e5/api/server/router/image/image_routes.go (L248-L255)

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

client/image_load.go

@@ -47,36 +47,13 @@ func (cli *Client) ImageLoad(ctx context.Context, input io.Reader, loadOpts ...I
 	}
 	return ImageLoadResult{
 		body: resp.Body,
-		JSON: resp.Header.Get("Content-Type") == "application/json",
 	}, nil
 }
 
 // ImageLoadResult returns information to the client about a load process.
-//
-// TODO(thaJeztah): remove this type, and just use an io.ReadCloser
-//
-// This type was added in https://github.com/moby/moby/pull/18878, related
-// to https://github.com/moby/moby/issues/19177;
-//
-// 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.
-//
-// However the "load" endpoint unconditionally returns JSON;
-// https://github.com/moby/moby/blob/7b9d2ef6e5518a3d3f3cc418459f8df786cfbbd1/api/server/router/image/image_routes.go#L248-L255
-//
-// PR https://github.com/moby/moby/pull/21959 made the response-type depend
-// on whether "quiet" was set, but this logic got changed in a follow-up
-// https://github.com/moby/moby/pull/25557, which made the JSON response-type
-// unconditionally, but the output produced depend on whether"quiet" was set.
-//
-// We should deprecated the "quiet" option, as it's really a client
-// responsibility.
 type ImageLoadResult struct {
 	// Body must be closed to avoid a resource leak
 	body io.ReadCloser
-	JSON bool
 }
 
 func (r ImageLoadResult) Read(p []byte) (n int, err error) {

client/image_load_test.go

@@ -3,6 +3,7 @@ package client
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"io"
 	"net/http"
 	"net/url"
@@ -27,52 +28,41 @@ func TestImageLoad(t *testing.T) {
 		expectedURL         = "/images/load"
 		expectedContentType = "application/x-tar"
 		expectedInput       = "inputBody"
-		expectedOutput      = "outputBody"
+		expectedOutput      = `{"stream":"Loaded image: busybox:latest\n"}`
 	)
 	tests := []struct {
-		doc                  string
-		quiet                bool
-		platforms            []ocispec.Platform
-		responseContentType  string
-		expectedResponseJSON bool
-		expectedQueryParams  url.Values
+		doc                 string
+		quiet               bool
+		platforms           []ocispec.Platform
+		expectedQueryParams url.Values
 	}{
 		{
-			doc:                  "plain-text",
-			quiet:                false,
-			responseContentType:  "text/plain",
-			expectedResponseJSON: false,
+			doc: "no options",
 			expectedQueryParams: url.Values{
 				"quiet": {"0"},
 			},
 		},
 		{
-			doc:                  "json quiet",
-			quiet:                true,
-			responseContentType:  "application/json",
-			expectedResponseJSON: true,
+			doc:   "quiet",
+			quiet: true,
 			expectedQueryParams: url.Values{
 				"quiet": {"1"},
 			},
 		},
 		{
-			doc:                  "json with platform",
-			platforms:            []ocispec.Platform{{Architecture: "arm64", OS: "linux", Variant: "v8"}},
-			responseContentType:  "application/json",
-			expectedResponseJSON: true,
+			doc:       "with platform",
+			platforms: []ocispec.Platform{{Architecture: "arm64", OS: "linux", Variant: "v8"}},
 			expectedQueryParams: url.Values{
 				"platform": {`{"architecture":"arm64","os":"linux","variant":"v8"}`},
 				"quiet":    {"0"},
 			},
 		},
 		{
-			doc: "json with multiple platforms",
+			doc: "multiple platforms",
 			platforms: []ocispec.Platform{
 				{Architecture: "arm64", OS: "linux", Variant: "v8"},
 				{Architecture: "amd64", OS: "linux"},
 			},
-			responseContentType:  "application/json",
-			expectedResponseJSON: true,
 			expectedQueryParams: url.Values{
 				"platform": {`{"architecture":"arm64","os":"linux","variant":"v8"}`, `{"architecture":"amd64","os":"linux"}`},
 				"quiet":    {"0"},
@@ -86,8 +76,7 @@ func TestImageLoad(t *testing.T) {
 				assert.Check(t, is.Equal(req.Header.Get("Content-Type"), expectedContentType))
 				assert.Check(t, is.DeepEqual(req.URL.Query(), tc.expectedQueryParams))
 
-				hdr := http.Header{"Content-Type": []string{tc.responseContentType}}
-				return mockResponse(http.StatusOK, hdr, expectedOutput)(req)
+				return mockJSONResponse(http.StatusOK, nil, json.RawMessage(expectedOutput))(req)
 			}))
 			assert.NilError(t, err)
 
@@ -97,7 +86,6 @@ func TestImageLoad(t *testing.T) {
 				ImageLoadWithPlatforms(tc.platforms...),
 			)
 			assert.NilError(t, err)
-			assert.Check(t, is.Equal(imageLoadResponse.JSON, tc.expectedResponseJSON))
 
 			body, err := io.ReadAll(imageLoadResponse)
 			assert.NilError(t, err)