p4u1/docker-cli
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #4939 from Benehiko/prompt-termination

Browse Source 
feat: standardize error for prompt
This commit is contained in:
Sebastiaan van Stijn
2024-04-02 19:09:12 +02:00
committed by GitHub
parent 155dc5e4e4 910d5d0247
commit 9ca30bd2ac
29 changed files with 425 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
cli/command/builder/prune.go
View File

@ -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{

7
cli/command/builder/prune_test.go
View File

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

8
cli/command/container/prune.go
View File

@ -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)

7
cli/command/container/prune_test.go
View File

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

8
cli/command/image/prune.go
View File

@ -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)

27
cli/command/image/prune_test.go
View File

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

8
cli/command/network/prune.go
View File

@ -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)

7
cli/command/network/prune_test.go
View File

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

6
cli/command/network/remove_test.go
View File

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

2
cli/command/plugin/testdata/plugin-upgrade-terminate.golden vendored
View File

@ -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]

12
cli/command/plugin/upgrade.go
View File

@ -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"))
}
}

7
cli/command/plugin/upgrade_test.go
View File

@ -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")
}

8
cli/command/system/prune.go
View File

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

10
cli/command/system/prune_test.go
View File

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

7
cli/command/trust/revoke.go
View File

@ -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"))
}
}

25
cli/command/trust/revoke_test.go
View File

@ -5,7 +5,6 @@ import (
"io"
"testing"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/trust"
"github.com/docker/cli/internal/test"
"github.com/docker/cli/internal/test/notary"
@ -58,6 +57,8 @@ func TestTrustRevokeCommandErrors(t *testing.T) {
}
func TestTrustRevokeCommand(t *testing.T) {
revokeCancelledError := "trust revoke has been cancelled"
testCases := []struct {
doc string
notaryRepository func(trust.ImageRefAndAuth, []string) (client.Repository, error)
@ -69,7 +70,8 @@ func TestTrustRevokeCommand(t *testing.T) {
doc: "OfflineErrors_Confirm",
notaryRepository: notary.GetOfflineNotaryRepository,
args: []string{"reg-name.io/image"},
expectedMessage: "Please confirm you would like to delete all signature data for reg-name.io/image? [y/N] \nAborting action.",
expectedMessage: "Please confirm you would like to delete all signature data for reg-name.io/image? [y/N] ",
expectedErr: revokeCancelledError,
},
{
doc: "OfflineErrors_Offline",
@ -87,7 +89,8 @@ func TestTrustRevokeCommand(t *testing.T) {
doc: "UninitializedErrors_Confirm",
notaryRepository: notary.GetUninitializedNotaryRepository,
args: []string{"reg-name.io/image"},
expectedMessage: "Please confirm you would like to delete all signature data for reg-name.io/image? [y/N] \nAborting action.",
expectedMessage: "Please confirm you would like to delete all signature data for reg-name.io/image? [y/N] ",
expectedErr: revokeCancelledError,
},
{
doc: "UninitializedErrors_NoTrustData",
@ -105,7 +108,8 @@ func TestTrustRevokeCommand(t *testing.T) {
doc: "EmptyNotaryRepo_Confirm",
notaryRepository: notary.GetEmptyTargetsNotaryRepository,
args: []string{"reg-name.io/image"},
expectedMessage: "Please confirm you would like to delete all signature data for reg-name.io/image? [y/N] \nAborting action.",
expectedMessage: "Please confirm you would like to delete all signature data for reg-name.io/image? [y/N] ",
expectedErr: revokeCancelledError,
},
{
doc: "EmptyNotaryRepo_NoSignedTags",
@ -123,7 +127,8 @@ func TestTrustRevokeCommand(t *testing.T) {
doc: "AllSigConfirmation",
notaryRepository: notary.GetEmptyTargetsNotaryRepository,
args: []string{"alpine"},
expectedMessage: "Please confirm you would like to delete all signature data for alpine? [y/N] \nAborting action.",
expectedMessage: "Please confirm you would like to delete all signature data for alpine? [y/N] ",
expectedErr: revokeCancelledError,
},
}
@ -136,9 +141,9 @@ func TestTrustRevokeCommand(t *testing.T) {
cmd.SetOut(io.Discard)
if tc.expectedErr != "" {
assert.ErrorContains(t, cmd.Execute(), tc.expectedErr)
return
} else {
assert.NilError(t, cmd.Execute())
}
assert.NilError(t, cmd.Execute())
assert.Check(t, is.Contains(cli.OutBuffer().String(), tc.expectedMessage))
})
}
@ -159,10 +164,6 @@ func TestRevokeTrustPromptTermination(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{})
cmd := newRevokeCommand(cli)
cmd.SetArgs([]string{"example/trust-demo"})
test.TerminatePrompt(ctx, t, cmd, cli, func(t *testing.T, err error) {
t.Helper()
assert.ErrorIs(t, err, command.ErrPromptTerminated)
})
assert.Equal(t, cli.ErrBuffer().String(), "")
test.TerminatePrompt(ctx, t, cmd, cli)
golden.Assert(t, cli.OutBuffer().String(), "trust-revoke-prompt-termination.golden")
}

6
cli/command/trust/signer_remove.go
View File

@ -132,10 +132,12 @@ func removeSingleSigner(ctx context.Context, dockerCLI command.Cli, repoName, si
}
ok, err := maybePromptForSignerRemoval(ctx, dockerCLI, repoName, signerName, isLastSigner, forceYes)
if err != nil || !ok {
fmt.Fprintf(dockerCLI.Out(), "\nAborting action.\n")
if err != nil {
return false, err
}
if !ok {
return false, nil
}
if err := notaryRepo.RemoveDelegationKeys(releasesRoleTUFName, role.KeyIDs); err != nil {
return false, err

1
cli/command/trust/testdata/trust-revoke-prompt-termination.golden vendored
View File

@ -1,2 +1 @@
Please confirm you would like to delete all signature data for example/trust-demo? [y/N]
Aborting action.

7
cli/command/utils.go
View File

@ -19,6 +19,7 @@ import (
"github.com/docker/docker/api/types/filters"
mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/errdefs"
"github.com/moby/sys/sequential"
"github.com/pkg/errors"
"github.com/spf13/pflag"
@ -75,9 +76,7 @@ func PrettyPrint(i any) string {
}
}
type PromptError error
var ErrPromptTerminated = PromptError(errors.New("prompt terminated"))
var ErrPromptTerminated = errdefs.Cancelled(errors.New("prompt terminated"))
// PromptForConfirmation requests and checks confirmation from the user.
// This will display the provided message followed by ' [y/N] '. If the user
@ -123,6 +122,8 @@ func PromptForConfirmation(ctx context.Context, ins io.Reader, outs io.Writer, m
select {
case <-notifyCtx.Done():
// print a newline on termination
_, _ = fmt.Fprintln(outs, "")
return false, ErrPromptTerminated
case r := <-result:
return r, nil

12
cli/command/volume/prune.go
View File

@ -32,10 +32,6 @@ func NewPruneCommand(dockerCli command.Cli) *cobra.Command {
RunE: func(cmd *cobra.Command, args []string) error {
spaceReclaimed, output, err := runPrune(cmd.Context(), dockerCli, options)
if err != nil {
if errdefs.IsCancelled(err) {
fmt.Fprintln(dockerCli.Out(), output)
return nil
}
return err
}
if output != "" {
@ -81,8 +77,12 @@ func runPrune(ctx context.Context, dockerCli command.Cli, options pruneOptions)
warning = allVolumesWarning
}
if !options.force {
if r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning); !r || err != nil {
return 0, "", errdefs.Cancelled(errors.New("user cancelled operation"))
r, err := command.PromptForConfirmation(ctx, dockerCli.In(), dockerCli.Out(), warning)
if err != nil {
return 0, "", err
}
if !r {
return 0, "", errdefs.Cancelled(errors.New("volume prune has been cancelled"))
}
}

8
cli/command/volume/prune_test.go
View File

@ -155,6 +155,7 @@ func TestVolumePrunePromptYes(t *testing.T) {
cli.SetIn(streams.NewIn(io.NopCloser(strings.NewReader(input))))
cmd := NewPruneCommand(cli)
cmd.SetArgs([]string{})
assert.NilError(t, cmd.Execute())
golden.Assert(t, cli.OutBuffer().String(), "volume-prune-yes.golden")
}
@ -171,7 +172,8 @@ func TestVolumePrunePromptNo(t *testing.T) {
cli.SetIn(streams.NewIn(io.NopCloser(strings.NewReader(input))))
cmd := NewPruneCommand(cli)
assert.NilError(t, cmd.Execute())
cmd.SetArgs([]string{})
assert.ErrorContains(t, cmd.Execute(), "volume prune has been cancelled")
golden.Assert(t, cli.OutBuffer().String(), "volume-prune-no.golden")
}
}
@ -196,7 +198,7 @@ func TestVolumePrunePromptTerminate(t *testing.T) {
})
cmd := NewPruneCommand(cli)
test.TerminatePrompt(ctx, t, cmd, cli, nil)
cmd.SetArgs([]string{})
test.TerminatePrompt(ctx, t, cmd, cli)
golden.Assert(t, cli.OutBuffer().String(), "volume-prune-terminate.golden")
}

2
cli/command/volume/testdata/volume-prune-no.golden vendored
View File

@ -1,2 +1,2 @@
WARNING! This will remove anonymous local volumes not used by at least one container.
Are you sure you want to continue? [y/N]
Are you sure you want to continue? [y/N]