client: Client.ContainerExport: close reader on context cancellation

Use a cancelReadCloser to automatically close the reader when the context
is cancelled. Consumers are still recommended to manually close the reader,
but the cancelReadCloser makes the Close idempotent.

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

client/container_export.go

@@ -19,6 +19,8 @@ type ContainerExportResult interface {
 // ContainerExport retrieves the raw contents of a container
 // and returns them as an [io.ReadCloser]. It's up to the caller
 // to close the stream.
+//
+// The underlying [io.ReadCloser] is automatically closed if the context is canceled,
 func (cli *Client) ContainerExport(ctx context.Context, containerID string, options ContainerExportOptions) (ContainerExportResult, error) {
 	containerID, err := trimID("container", containerID)
 	if err != nil {
@@ -31,30 +33,15 @@ func (cli *Client) ContainerExport(ctx context.Context, containerID string, opti
 	}
 
 	return &containerExportResult{
-		body: resp.Body,
+		ReadCloser: newCancelReadCloser(ctx, resp.Body),
 	}, nil
 }
 
 type containerExportResult struct {
-	// body must be closed to avoid a resource leak
-	body io.ReadCloser
+	io.ReadCloser
 }
 
 var (
 	_ io.ReadCloser         = (*containerExportResult)(nil)
 	_ ContainerExportResult = (*containerExportResult)(nil)
 )
-
-func (r *containerExportResult) Read(p []byte) (int, error) {
-	if r == nil || r.body == nil {
-		return 0, io.EOF
-	}
-	return r.body.Read(p)
-}
-
-func (r *containerExportResult) Close() error {
-	if r == nil || r.body == nil {
-		return nil
-	}
-	return r.body.Close()
-}

client/container_export_test.go

@@ -1,7 +1,6 @@
 package client
 
 import (
-	"context"
 	"io"
 	"net/http"
 	"testing"
@@ -17,14 +16,14 @@ func TestContainerExportError(t *testing.T) {
 	)
 	assert.NilError(t, err)
 
-	_, err = client.ContainerExport(context.Background(), "nothing", ContainerExportOptions{})
+	_, err = client.ContainerExport(t.Context(), "nothing", ContainerExportOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal))
 
-	_, err = client.ContainerExport(context.Background(), "", ContainerExportOptions{})
+	_, err = client.ContainerExport(t.Context(), "", ContainerExportOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 	assert.Check(t, is.ErrorContains(err, "value is empty"))
 
-	_, err = client.ContainerExport(context.Background(), "    ", ContainerExportOptions{})
+	_, err = client.ContainerExport(t.Context(), "    ", ContainerExportOptions{})
 	assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument))
 	assert.Check(t, is.ErrorContains(err, "value is empty"))
 }
@@ -40,10 +39,10 @@ func TestContainerExport(t *testing.T) {
 		}),
 	)
 	assert.NilError(t, err)
-	body, err := client.ContainerExport(context.Background(), "container_id", ContainerExportOptions{})
+	res, err := client.ContainerExport(t.Context(), "container_id", ContainerExportOptions{})
 	assert.NilError(t, err)
-	defer body.Close()
-	content, err := io.ReadAll(body)
+	defer res.Close()
+	content, err := io.ReadAll(res)
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(string(content), "response"))
 }

integration-cli/docker_api_containers_test.go

@@ -103,11 +103,11 @@ func (s *DockerAPISuite) TestContainerAPIGetExport(c *testing.T) {
 	assert.NilError(c, err)
 	defer apiClient.Close()
 
-	body, err := apiClient.ContainerExport(testutil.GetContext(c), name, client.ContainerExportOptions{})
+	res, err := apiClient.ContainerExport(testutil.GetContext(c), name, client.ContainerExportOptions{})
 	assert.NilError(c, err)
-	defer body.Close()
+	defer res.Close()
 	found := false
-	for tarReader := tar.NewReader(body); ; {
+	for tarReader := tar.NewReader(res); ; {
 		h, err := tarReader.Next()
 		if errors.Is(err, io.EOF) {
 			break

integration/container/export_test.go

@@ -71,6 +71,7 @@ func TestExportContainerAfterDaemonRestart(t *testing.T) {
 
 	d.Restart(t)
 
-	_, err := c.ContainerExport(ctx, ctrID, client.ContainerExportOptions{})
+	res, err := c.ContainerExport(ctx, ctrID, client.ContainerExportOptions{})
 	assert.NilError(t, err)
+	_ = res.Close()
 }

integration/container/overlayfs_linux_test.go

@@ -32,10 +32,10 @@ func TestNoOverlayfsWarningsAboutUndefinedBehaviors(t *testing.T) {
 			return err
 		}},
 		{name: "export", operation: func(*testing.T) error {
-			rc, err := apiClient.ContainerExport(ctx, cID, client.ContainerExportOptions{})
+			res, err := apiClient.ContainerExport(ctx, cID, client.ContainerExportOptions{})
 			if err == nil {
-				defer rc.Close()
-				_, err = io.Copy(io.Discard, rc)
+				_, err = io.Copy(io.Discard, res)
+				_ = res.Close()
 			}
 			return err
 		}},
@@ -48,8 +48,8 @@ func TestNoOverlayfsWarningsAboutUndefinedBehaviors(t *testing.T) {
 		{name: "cp from container", operation: func(*testing.T) error {
 			res, err := apiClient.CopyFromContainer(ctx, cID, client.CopyFromContainerOptions{SourcePath: "/file"})
 			if err == nil {
-				defer res.Content.Close()
 				_, err = io.Copy(io.Discard, res.Content)
+				_ = res.Content.Close()
 			}
 
 			return err

integration/plugin/authz/authz_plugin_test.go

@@ -363,13 +363,13 @@ func TestAuthZPluginEnsureLoadImportWorking(t *testing.T) {
 
 	cID := container.Run(ctx, t, c)
 
-	responseReader, err := c.ContainerExport(ctx, cID, client.ContainerExportOptions{})
+	res, err := c.ContainerExport(ctx, cID, client.ContainerExportOptions{})
 	assert.NilError(t, err)
-	defer responseReader.Close()
+	defer func() { _ = res.Close() }()
 	file, err := os.Create(exportedImagePath)
 	assert.NilError(t, err)
-	defer file.Close()
-	_, err = io.Copy(file, responseReader)
+	defer func() { _ = file.Close() }()
+	_, err = io.Copy(file, res)
 	assert.NilError(t, err)
 
 	err = imageImport(ctx, c, exportedImagePath)

vendor/github.com/moby/moby/client/container_export.go

@@ -19,6 +19,8 @@ type ContainerExportResult interface {
 // ContainerExport retrieves the raw contents of a container
 // and returns them as an [io.ReadCloser]. It's up to the caller
 // to close the stream.
+//
+// The underlying [io.ReadCloser] is automatically closed if the context is canceled,
 func (cli *Client) ContainerExport(ctx context.Context, containerID string, options ContainerExportOptions) (ContainerExportResult, error) {
 	containerID, err := trimID("container", containerID)
 	if err != nil {
@@ -31,30 +33,15 @@ func (cli *Client) ContainerExport(ctx context.Context, containerID string, opti
 	}
 
 	return &containerExportResult{
-		body: resp.Body,
+		ReadCloser: newCancelReadCloser(ctx, resp.Body),
 	}, nil
 }
 
 type containerExportResult struct {
-	// body must be closed to avoid a resource leak
-	body io.ReadCloser
+	io.ReadCloser
 }
 
 var (
 	_ io.ReadCloser         = (*containerExportResult)(nil)
 	_ ContainerExportResult = (*containerExportResult)(nil)
 )
-
-func (r *containerExportResult) Read(p []byte) (int, error) {
-	if r == nil || r.body == nil {
-		return 0, io.EOF
-	}
-	return r.body.Read(p)
-}
-
-func (r *containerExportResult) Close() error {
-	if r == nil || r.body == nil {
-		return nil
-	}
-	return r.body.Close()
-}