From 0c3bb6c0a4dd6da921739399ab141d3fa8659303 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 8 Sep 2025 19:42:12 +0200 Subject: [PATCH] cli/command/container: rename: remove renameOptions Also remove redundant validation that's already performed by the client or daemon. Signed-off-by: Sebastiaan van Stijn --- cli/command/container/rename.go | 31 +++++++++------------------- cli/command/container/rename_test.go | 25 +++++----------------- 2 files changed, 15 insertions(+), 41 deletions(-) diff --git a/cli/command/container/rename.go b/cli/command/container/rename.go index 53020d1ef5..a4367c0867 100644 --- a/cli/command/container/rename.go +++ b/cli/command/container/rename.go @@ -2,33 +2,24 @@ package container import ( "context" + "errors" "fmt" "strings" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/completion" - "github.com/pkg/errors" "github.com/spf13/cobra" ) -type renameOptions struct { - oldName string - newName string -} - // newRenameCommand creates a new cobra.Command for "docker container rename". func newRenameCommand(dockerCLI command.Cli) *cobra.Command { - var opts renameOptions - cmd := &cobra.Command{ Use: "rename CONTAINER NEW_NAME", Short: "Rename a container", Args: cli.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - opts.oldName = args[0] - opts.newName = args[1] - return runRename(cmd.Context(), dockerCLI, &opts) + return runRename(cmd.Context(), dockerCLI, args[0], args[1]) }, Annotations: map[string]string{ "aliases": "docker container rename, docker rename", @@ -39,17 +30,15 @@ func newRenameCommand(dockerCLI command.Cli) *cobra.Command { return cmd } -func runRename(ctx context.Context, dockerCli command.Cli, opts *renameOptions) error { - oldName := strings.TrimSpace(opts.oldName) - newName := strings.TrimSpace(opts.newName) - - if oldName == "" || newName == "" { - return errors.New("Error: Neither old nor new names may be empty") +func runRename(ctx context.Context, dockerCLI command.Cli, oldName, newName string) error { + newName = strings.TrimSpace(newName) + if newName == "" { + // TODO(thaJeztah): improve validation in ContainerRename and daemon; the daemon returns an obscure error when providing whitespace-only new-name: + // Error response from daemon: Error when allocating new name: Invalid container name (/ ), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed + return errors.New("new name cannot be blank") } - - if err := dockerCli.Client().ContainerRename(ctx, oldName, newName); err != nil { - fmt.Fprintln(dockerCli.Err(), err) - return errors.Errorf("Error: failed to rename container named %s", oldName) + if err := dockerCLI.Client().ContainerRename(ctx, oldName, newName); err != nil { + return fmt.Errorf("failed to rename container: %w", err) } return nil } diff --git a/cli/command/container/rename_test.go b/cli/command/container/rename_test.go index 2e14e771e5..4ca80c70f1 100644 --- a/cli/command/container/rename_test.go +++ b/cli/command/container/rename_test.go @@ -8,7 +8,6 @@ import ( "github.com/docker/cli/internal/test" "gotest.tools/v3/assert" - is "gotest.tools/v3/assert/cmp" ) func TestRunRename(t *testing.T) { @@ -25,13 +24,13 @@ func TestRunRename(t *testing.T) { doc: "empty old name", oldName: "", newName: "newName", - expectedErr: "Error: Neither old nor new names may be empty", + expectedErr: "invalid container name or ID: value is empty", }, { doc: "empty new name", oldName: "oldName", newName: "", - expectedErr: "Error: Neither old nor new names may be empty", + expectedErr: "new name cannot be blank", }, } @@ -39,6 +38,9 @@ func TestRunRename(t *testing.T) { t.Run(tc.doc, func(t *testing.T) { cli := test.NewFakeCli(&fakeClient{ containerRenameFunc: func(ctx context.Context, oldName, newName string) error { + if oldName == "" { + return errors.New("invalid container name or ID: value is empty") + } return nil }, }) @@ -58,20 +60,3 @@ func TestRunRename(t *testing.T) { }) } } - -func TestRunRenameClientError(t *testing.T) { - cli := test.NewFakeCli(&fakeClient{ - containerRenameFunc: func(ctx context.Context, oldName, newName string) error { - return errors.New("client error") - }, - }) - - cmd := newRenameCommand(cli) - cmd.SetOut(io.Discard) - cmd.SetErr(io.Discard) - cmd.SetArgs([]string{"oldName", "newName"}) - - err := cmd.Execute() - - assert.Check(t, is.Error(err, "Error: failed to rename container named oldName")) -}