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 <github@gone.nl>

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
 }

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