Merge pull request #5914 from thaJeztah/use_atomicwriter

cli/command: deprecate CopyToFile and reimplement with atomicwriter

Author: thaJeztah

cli/command/container/export.go

@@ -7,6 +7,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
+	"github.com/moby/sys/atomicwriter"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
@@ -41,27 +42,28 @@ func NewExportCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func runExport(ctx context.Context, dockerCli command.Cli, opts exportOptions) error {
-	if opts.output == "" && dockerCli.Out().IsTerminal() {
-		return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
-	}
-
-	if err := command.ValidateOutputPath(opts.output); err != nil {
-		return errors.Wrap(err, "failed to export container")
+func runExport(ctx context.Context, dockerCLI command.Cli, opts exportOptions) error {
+	var output io.Writer
+	if opts.output == "" {
+		if dockerCLI.Out().IsTerminal() {
+			return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
+		}
+		output = dockerCLI.Out()
+	} else {
+		writer, err := atomicwriter.New(opts.output, 0o600)
+		if err != nil {
+			return errors.Wrap(err, "failed to export container")
+		}
+		defer writer.Close()
+		output = writer
 	}
 
-	clnt := dockerCli.Client()
-
-	responseBody, err := clnt.ContainerExport(ctx, opts.container)
+	responseBody, err := dockerCLI.Client().ContainerExport(ctx, opts.container)
 	if err != nil {
 		return err
 	}
 	defer responseBody.Close()
 
-	if opts.output == "" {
-		_, err := io.Copy(dockerCli.Out(), responseBody)
-		return err
-	}
-
-	return command.CopyToFile(opts.output, responseBody)
+	_, err = io.Copy(output, responseBody)
+	return err
 }

cli/command/container/export_test.go

@@ -42,8 +42,6 @@ func TestContainerExportOutputToIrregularFile(t *testing.T) {
 	cmd.SetErr(io.Discard)
 	cmd.SetArgs([]string{"-o", "/dev/random", "container"})
 
-	err := cmd.Execute()
-	assert.Assert(t, err != nil)
-	expected := `"/dev/random" must be a directory or a regular file`
-	assert.ErrorContains(t, err, expected)
+	const expected = `failed to export container: cannot write to a character device file`
+	assert.Error(t, cmd.Execute(), expected)
 }

cli/command/image/save.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/docker/client"
+	"github.com/moby/sys/atomicwriter"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
@@ -48,15 +49,7 @@ func NewSaveCommand(dockerCli command.Cli) *cobra.Command {
 }
 
 // runSave performs a save against the engine based on the specified options
-func runSave(ctx context.Context, dockerCli command.Cli, opts saveOptions) error {
-	if opts.output == "" && dockerCli.Out().IsTerminal() {
-		return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
-	}
-
-	if err := command.ValidateOutputPath(opts.output); err != nil {
-		return errors.Wrap(err, "failed to save image")
-	}
-
+func runSave(ctx context.Context, dockerCLI command.Cli, opts saveOptions) error {
 	var options []client.ImageSaveOption
 	if opts.platform != "" {
 		p, err := platforms.Parse(opts.platform)
@@ -67,16 +60,27 @@ func runSave(ctx context.Context, dockerCli command.Cli, opts saveOptions) error
 		options = append(options, client.ImageSaveWithPlatforms(p))
 	}
 
-	responseBody, err := dockerCli.Client().ImageSave(ctx, opts.images, options...)
-	if err != nil {
-		return err
+	var output io.Writer
+	if opts.output == "" {
+		if dockerCLI.Out().IsTerminal() {
+			return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
+		}
+		output = dockerCLI.Out()
+	} else {
+		writer, err := atomicwriter.New(opts.output, 0o600)
+		if err != nil {
+			return errors.Wrap(err, "failed to save image")
+		}
+		defer writer.Close()
+		output = writer
 	}
-	defer responseBody.Close()
 
-	if opts.output == "" {
-		_, err := io.Copy(dockerCli.Out(), responseBody)
+	responseBody, err := dockerCLI.Client().ImageSave(ctx, opts.images, options...)
+	if err != nil {
 		return err
 	}
+	defer responseBody.Close()
 
-	return command.CopyToFile(opts.output, responseBody)
+	_, err = io.Copy(output, responseBody)
+	return err
 }

cli/command/image/save_test.go

@@ -44,12 +44,12 @@ func TestNewSaveCommandErrors(t *testing.T) {
 		{
 			name:          "output directory does not exist",
 			args:          []string{"-o", "fakedir/out.tar", "arg1"},
-			expectedError: "failed to save image: invalid output path: directory \"fakedir\" does not exist",
+			expectedError: `failed to save image: invalid output path: stat fakedir: no such file or directory`,
 		},
 		{
 			name:          "output file is irregular",
 			args:          []string{"-o", "/dev/null", "arg1"},
-			expectedError: "failed to save image: invalid output path: \"/dev/null\" must be a directory or a regular file",
+			expectedError: `failed to save image: cannot write to a character device file`,
 		},
 		{
 			name:          "invalid platform",

cli/command/utils.go

@@ -17,37 +17,23 @@ import (
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/errdefs"
-	"github.com/moby/sys/sequential"
+	"github.com/moby/sys/atomicwriter"
 	"github.com/moby/term"
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
 )
 
 // CopyToFile writes the content of the reader to the specified file
+//
+// Deprecated: use [atomicwriter.New].
 func CopyToFile(outfile string, r io.Reader) error {
-	// We use sequential file access here to avoid depleting the standby list
-	// on Windows. On Linux, this is a call directly to os.CreateTemp
-	tmpFile, err := sequential.CreateTemp(filepath.Dir(outfile), ".docker_temp_")
-	if err != nil {
-		return err
-	}
-
-	tmpPath := tmpFile.Name()
-
-	_, err = io.Copy(tmpFile, r)
-	tmpFile.Close()
-
+	writer, err := atomicwriter.New(outfile, 0o600)
 	if err != nil {
-		os.Remove(tmpPath)
-		return err
-	}
-
-	if err = os.Rename(tmpPath, outfile); err != nil {
-		os.Remove(tmpPath)
 		return err
 	}
-
-	return nil
+	defer writer.Close()
+	_, err = io.Copy(writer, r)
+	return err
 }
 
 var ErrPromptTerminated = errdefs.Cancelled(errors.New("prompt terminated"))
@@ -187,7 +173,7 @@ func AddPlatformFlag(flags *pflag.FlagSet, target *string) {
 	_ = flags.SetAnnotation("platform", "version", []string{"1.32"})
 }
 
-// ValidateOutputPath validates the output paths of the `export` and `save` commands.
+// ValidateOutputPath validates the output paths of the "docker cp" command.
 func ValidateOutputPath(path string) error {
 	dir := filepath.Dir(filepath.Clean(path))
 	if dir != "" && dir != "." {
@@ -213,8 +199,8 @@ func ValidateOutputPath(path string) error {
 	return nil
 }
 
-// ValidateOutputPathFileMode validates the output paths of the `cp` command and serves as a
-// helper to `ValidateOutputPath`
+// ValidateOutputPathFileMode validates the output paths of the "docker cp" command
+// and serves as a helper to [ValidateOutputPath]
 func ValidateOutputPathFileMode(fileMode os.FileMode) error {
 	switch {
 	case fileMode&os.ModeDevice != 0: