Merge pull request #4939 from Benehiko/prompt-termination

feat: standardize error for prompt
docker · Apr 2, 2024 · 9ca30bd · 9ca30bd
2 parents 155dc5e + 910d5d0
commit 9ca30bd
Show file tree

Hide file tree

Showing 29 changed files with 425 additions and 165 deletions.
diff --git a/cli/command/builder/prune.go b/cli/command/builder/prune.go
@@ -2,6 +2,7 @@ package builder
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -10,6 +11,7 @@ import (
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/errdefs"
 	units "github.com/docker/go-units"
 	"github.com/spf13/cobra"
 )
@@ -67,9 +69,13 @@ func runPrune(ctx context.Context, dockerCli command.Cli, options pruneOptions)
 		warning = allCacheWarning
 	}
 	if !options.force {
-		if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning); !r || err != nil {
+		r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning)
+		if err != nil {
 			return 0, "", err
 		}
+		if !r {
+			return 0, "", errdefs.Cancelled(errors.New("builder prune has been cancelled"))
+		}
 	}
 
 	report, err := dockerCli.Client().BuildCachePrune(ctx, types.BuildCachePruneOptions{

diff --git a/cli/command/builder/prune_test.go b/cli/command/builder/prune_test.go
@@ -5,10 +5,8 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
-	"gotest.tools/v3/assert"
 )
 
 func TestBuilderPromptTermination(t *testing.T) {
@@ -21,8 +19,5 @@ func TestBuilderPromptTermination(t *testing.T) {
 		},
 	})
 	cmd := NewPruneCommand(cli)
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 }
diff --git a/cli/command/container/prune.go b/cli/command/container/prune.go
@@ -8,7 +8,9 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/opts"
+	"github.com/docker/docker/errdefs"
 	units "github.com/docker/go-units"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -54,9 +56,13 @@ func runPrune(ctx context.Context, dockerCli command.Cli, options pruneOptions)
 	pruneFilters := command.PruneFilters(dockerCli, options.filter.Value())
 
 	if !options.force {
-		if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning); !r || err != nil {
+		r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning)
+		if err != nil {
 			return 0, "", err
 		}
+		if !r {
+			return 0, "", errdefs.Cancelled(errors.New("container prune has been cancelled"))
+		}
 	}
 
 	report, err := dockerCli.Client().ContainersPrune(ctx, pruneFilters)

diff --git a/cli/command/container/prune_test.go b/cli/command/container/prune_test.go
@@ -4,12 +4,10 @@ import (
 	"context"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/pkg/errors"
-	"gotest.tools/v3/assert"
 )
 
 func TestContainerPrunePromptTermination(t *testing.T) {
@@ -22,8 +20,5 @@ func TestContainerPrunePromptTermination(t *testing.T) {
 		},
 	})
 	cmd := NewPruneCommand(cli)
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 }
diff --git a/cli/command/image/prune.go b/cli/command/image/prune.go
@@ -10,7 +10,9 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/opts"
+	"github.com/docker/docker/errdefs"
 	units "github.com/docker/go-units"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -68,9 +70,13 @@ func runPrune(ctx context.Context, dockerCli command.Cli, options pruneOptions)
 		warning = allImageWarning
 	}
 	if !options.force {
-		if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning); !r || err != nil {
+		r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning)
+		if err != nil {
 			return 0, "", err
 		}
+		if !r {
+			return 0, "", errdefs.Cancelled(errors.New("image prune has been cancelled"))
+		}
 	}
 
 	report, err := dockerCli.Client().ImagesPrune(ctx, pruneFilters)

diff --git a/cli/command/image/prune_test.go b/cli/command/image/prune_test.go
@@ -4,9 +4,10 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
@@ -94,13 +95,18 @@ func TestNewPruneCommandSuccess(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		cli := test.NewFakeCli(&fakeClient{imagesPruneFunc: tc.imagesPruneFunc})
-		cmd := NewPruneCommand(cli)
-		cmd.SetOut(io.Discard)
-		cmd.SetArgs(tc.args)
-		err := cmd.Execute()
-		assert.NilError(t, err)
-		golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("prune-command-success.%s.golden", tc.name))
+		t.Run(tc.name, func(t *testing.T) {
+			cli := test.NewFakeCli(&fakeClient{imagesPruneFunc: tc.imagesPruneFunc})
+			// when prompted, answer "Y" to confirm the prune.
+			// will not be prompted if --force is used.
+			cli.SetIn(streams.NewIn(io.NopCloser(strings.NewReader("Y\n"))))
+			cmd := NewPruneCommand(cli)
+			cmd.SetOut(io.Discard)
+			cmd.SetArgs(tc.args)
+			err := cmd.Execute()
+			assert.NilError(t, err)
+			golden.Assert(t, cli.OutBuffer().String(), fmt.Sprintf("prune-command-success.%s.golden", tc.name))
+		})
 	}
 }
 
@@ -114,8 +120,5 @@ func TestPrunePromptTermination(t *testing.T) {
 		},
 	})
 	cmd := NewPruneCommand(cli)
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 }
diff --git a/cli/command/network/prune.go b/cli/command/network/prune.go
@@ -7,6 +7,8 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/opts"
+	"github.com/docker/docker/errdefs"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -50,9 +52,13 @@ func runPrune(ctx context.Context, dockerCli command.Cli, options pruneOptions)
 	pruneFilters := command.PruneFilters(dockerCli, options.filter.Value())
 
 	if !options.force {
-		if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning); !r || err != nil {
+		r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning)
+		if err != nil {
 			return "", err
 		}
+		if !r {
+			return "", errdefs.Cancelled(errors.New("network prune cancelled has been cancelled"))
+		}
 	}
 
 	report, err := dockerCli.Client().NetworksPrune(ctx, pruneFilters)

diff --git a/cli/command/network/prune_test.go b/cli/command/network/prune_test.go
@@ -4,12 +4,10 @@ import (
 	"context"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/pkg/errors"
-	"gotest.tools/v3/assert"
 )
 
 func TestNetworkPrunePromptTermination(t *testing.T) {
@@ -22,8 +20,5 @@ func TestNetworkPrunePromptTermination(t *testing.T) {
 		},
 	})
 	cmd := NewPruneCommand(cli)
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 }
diff --git a/cli/command/network/remove_test.go b/cli/command/network/remove_test.go
@@ -5,7 +5,6 @@ import (
 	"io"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/errdefs"
@@ -115,8 +114,5 @@ func TestNetworkRemovePromptTermination(t *testing.T) {
 	})
 	cmd := newRemoveCommand(cli)
 	cmd.SetArgs([]string{"existing-network"})
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 }
diff --git a/cli/command/plugin/testdata/plugin-upgrade-terminate.golden b/cli/command/plugin/testdata/plugin-upgrade-terminate.golden
@@ -1,2 +1,2 @@
 Upgrading plugin foo/bar from localhost:5000/foo/bar:v0.1.0 to localhost:5000/foo/bar:v1.0.0
-Plugin images do not match, are you sure? [y/N] 
+Plugin images do not match, are you sure? [y/N] 
diff --git a/cli/command/plugin/upgrade.go b/cli/command/plugin/upgrade.go
@@ -8,6 +8,7 @@ import (
 	"github.com/distribution/reference"
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -63,11 +64,12 @@ func runUpgrade(ctx context.Context, dockerCli command.Cli, opts pluginOptions)
 
 	fmt.Fprintf(dockerCli.Out(), "Upgrading plugin %s from %s to %s\n", p.Name, reference.FamiliarString(old), reference.FamiliarString(remote))
 	if !opts.skipRemoteCheck && remote.String() != old.String() {
-		if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), "Plugin images do not match, are you sure?"); !r || err != nil {
-			if err != nil {
-				return errors.Wrap(err, "canceling upgrade request")
-			}
-			return errors.New("canceling upgrade request")
+		r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), "Plugin images do not match, are you sure?")
+		if err != nil {
+			return err
+		}
+		if !r {
+			return errdefs.Cancelled(errors.New("plugin upgrade has been cancelled"))
 		}
 	}
 

diff --git a/cli/command/plugin/upgrade_test.go b/cli/command/plugin/upgrade_test.go
@@ -5,11 +5,9 @@ import (
 	"io"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
 	"github.com/pkg/errors"
-	"gotest.tools/v3/assert"
 	"gotest.tools/v3/golden"
 )
 
@@ -34,9 +32,6 @@ func TestUpgradePromptTermination(t *testing.T) {
 	// need to set a remote address that does not match the plugin
 	// reference sent by the `pluginInspectFunc`
 	cmd.SetArgs([]string{"foo/bar", "localhost:5000/foo/bar:v1.0.0"})
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 	golden.Assert(t, cli.OutBuffer().String(), "plugin-upgrade-terminate.golden")
 }
diff --git a/cli/command/system/prune.go b/cli/command/system/prune.go
@@ -17,8 +17,10 @@ import (
 	"github.com/docker/cli/cli/command/volume"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/go-units"
 	"github.com/fvbommel/sortorder"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -75,9 +77,13 @@ func runPrune(ctx context.Context, dockerCli command.Cli, options pruneOptions)
 		return fmt.Errorf(`ERROR: The "until" filter is not supported with "--volumes"`)
 	}
 	if !options.force {
-		if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), confirmationMessage(dockerCli, options)); !r || err != nil {
+		r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), confirmationMessage(dockerCli, options))
+		if err != nil {
 			return err
 		}
+		if !r {
+			return errdefs.Cancelled(errors.New("system prune has been cancelled"))
+		}
 	}
 	pruneFuncs := []func(ctx context.Context, dockerCli command.Cli, all bool, filter opts.FilterOpt) (uint64, string, error){
 		container.RunPrune,

diff --git a/cli/command/system/prune_test.go b/cli/command/system/prune_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
@@ -18,7 +17,7 @@ func TestPrunePromptPre131DoesNotIncludeBuildCache(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{version: "1.30"})
 	cmd := newPruneCommand(cli)
 	cmd.SetArgs([]string{})
-	assert.NilError(t, cmd.Execute())
+	assert.ErrorContains(t, cmd.Execute(), "system prune has been cancelled")
 	expected := `WARNING! This will remove:
   - all stopped containers
   - all networks not used by at least one container
@@ -36,7 +35,7 @@ func TestPrunePromptFilters(t *testing.T) {
 	cmd := newPruneCommand(cli)
 	cmd.SetArgs([]string{"--filter", "until=24h", "--filter", "label=hello-world", "--filter", "label!=foo=bar", "--filter", "label=bar=baz"})
 
-	assert.NilError(t, cmd.Execute())
+	assert.ErrorContains(t, cmd.Execute(), "system prune has been cancelled")
 	expected := `WARNING! This will remove:
   - all stopped containers
   - all networks not used by at least one container
@@ -69,8 +68,5 @@ func TestSystemPrunePromptTermination(t *testing.T) {
 	})
 
 	cmd := newPruneCommand(cli)
-	test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
-		t.Helper()
-		assert.ErrorIs(t, err, command.ErrPromptTerminated)
-	})
+	test.TerminatePrompt(ctx, t, cmd, cli)
 }
diff --git a/cli/command/trust/revoke.go b/cli/command/trust/revoke.go
@@ -8,6 +8,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/image"
 	"github.com/docker/cli/cli/trust"
+	"github.com/docker/docker/errdefs"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/theupdateframework/notary/client"
@@ -45,12 +46,10 @@ func revokeTrust(ctx context.Context, dockerCLI command.Cli, remote string, opti
 	if imgRefAndAuth.Tag() == "" && !options.forceYes {
 		deleteRemote, err := command.PromptForConfirmation(ctx, dockerCLI.In(), dockerCLI.Out(), fmt.Sprintf("Please confirm you would like to delete all signature data for %s?", remote))
 		if err != nil {
-			fmt.Fprintf(dockerCLI.Out(), "\nAborting action.\n")
-			return errors.Wrap(err, "aborting action")
+			return err
 		}
 		if !deleteRemote {
-			fmt.Fprintf(dockerCLI.Out(), "\nAborting action.\n")
-			return nil
+			return errdefs.Cancelled(errors.New("trust revoke has been cancelled"))
 		}
 	}