cli/command/context: use errors.Join

Use stdlib multi-errors instead of creating our own; also
touch-up one error and some minor cleanups.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Author: thaJeztah

cli/command/context/options.go

@@ -1,14 +1,14 @@
 package context
 
 import (
+	"errors"
+	"fmt"
 	"strconv"
-	"strings"
 
 	"github.com/docker/cli/cli/context"
 	"github.com/docker/cli/cli/context/docker"
 	"github.com/docker/cli/cli/context/store"
 	"github.com/docker/docker/client"
-	"github.com/pkg/errors"
 )
 
 const (
@@ -68,20 +68,17 @@ func parseBool(config map[string]string, name string) (bool, error) {
 		return false, nil
 	}
 	res, err := strconv.ParseBool(strVal)
-	return res, errors.Wrap(err, name)
+	return res, fmt.Errorf("name: %w", err)
 }
 
 func validateConfig(config map[string]string, allowedKeys map[string]struct{}) error {
-	var errs []string
+	var errs []error
 	for k := range config {
 		if _, ok := allowedKeys[k]; !ok {
-			errs = append(errs, "unrecognized config key: "+k)
+			errs = append(errs, errors.New("unrecognized config key: "+k))
 		}
 	}
-	if len(errs) == 0 {
-		return nil
-	}
-	return errors.New(strings.Join(errs, "\n"))
+	return errors.Join(errs...)
 }
 
 func getDockerEndpoint(contextStore store.Reader, config map[string]string) (docker.Endpoint, error) {
@@ -96,7 +93,7 @@ func getDockerEndpoint(contextStore store.Reader, config map[string]string) (doc
 		if ep, ok := metadata.Endpoints[docker.DockerEndpoint].(docker.EndpointMeta); ok {
 			return docker.Endpoint{EndpointMeta: ep}, nil
 		}
-		return docker.Endpoint{}, errors.Errorf("unable to get endpoint from context %q", contextName)
+		return docker.Endpoint{}, fmt.Errorf("unable to get endpoint from context %q", contextName)
 	}
 	tlsData, err := context.TLSDataFromFiles(config[keyCA], config[keyCert], config[keyKey])
 	if err != nil {
@@ -116,10 +113,11 @@ func getDockerEndpoint(contextStore store.Reader, config map[string]string) (doc
 	// try to resolve a docker client, validating the configuration
 	opts, err := ep.ClientOpts()
 	if err != nil {
-		return docker.Endpoint{}, errors.Wrap(err, "invalid docker endpoint options")
+		return docker.Endpoint{}, fmt.Errorf("invalid docker endpoint options: %w", err)
 	}
+	// FIXME(thaJeztah): this creates a new client (but discards it) only to validate the options; are the validation steps above not enough?
 	if _, err := client.NewClientWithOpts(opts...); err != nil {
-		return docker.Endpoint{}, errors.Wrap(err, "unable to apply docker endpoint options")
+		return docker.Endpoint{}, fmt.Errorf("unable to apply docker endpoint options: %w", err)
 	}
 	return ep, nil
 }

cli/command/context/remove.go

@@ -1,14 +1,13 @@
 package context
 
 import (
+	"errors"
 	"fmt"
 	"os"
-	"strings"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/docker/errdefs"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -33,28 +32,25 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command {
 }
 
 // RunRemove removes one or more contexts
-func RunRemove(dockerCli command.Cli, opts RemoveOptions, names []string) error {
-	var errs []string
-	currentCtx := dockerCli.CurrentContext()
+func RunRemove(dockerCLI command.Cli, opts RemoveOptions, names []string) error {
+	var errs []error
+	currentCtx := dockerCLI.CurrentContext()
 	for _, name := range names {
 		if name == "default" {
-			errs = append(errs, `default: context "default" cannot be removed`)
-		} else if err := doRemove(dockerCli, name, name == currentCtx, opts.Force); err != nil {
-			errs = append(errs, err.Error())
+			errs = append(errs, errors.New(`context "default" cannot be removed`))
+		} else if err := doRemove(dockerCLI, name, name == currentCtx, opts.Force); err != nil {
+			errs = append(errs, err)
 		} else {
-			fmt.Fprintln(dockerCli.Out(), name)
+			_, _ = fmt.Fprintln(dockerCLI.Out(), name)
 		}
 	}
-	if len(errs) > 0 {
-		return errors.New(strings.Join(errs, "\n"))
-	}
-	return nil
+	return errors.Join(errs...)
 }
 
 func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error {
 	if isCurrent {
 		if !force {
-			return errors.Errorf("context %q is in use, set -f flag to force remove", name)
+			return fmt.Errorf("context %q is in use, set -f flag to force remove", name)
 		}
 		// fallback to DOCKER_HOST
 		cfg := dockerCli.ConfigFile()
@@ -65,6 +61,7 @@ func doRemove(dockerCli command.Cli, name string, isCurrent, force bool) error {
 	}
 
 	if !force {
+		// TODO(thaJeztah): instead of checking before removing, can we make ContextStore().Remove() return a proper errdef and ignore "not found" errors?
 		if err := checkContextExists(dockerCli, name); err != nil {
 			return err
 		}
@@ -77,7 +74,7 @@ func checkContextExists(dockerCli command.Cli, name string) error {
 	contextDir := dockerCli.ContextStore().GetStorageInfo(name).MetadataPath
 	_, err := os.Stat(contextDir)
 	if os.IsNotExist(err) {
-		return errdefs.NotFound(errors.Errorf("context %q does not exist", name))
+		return errdefs.NotFound(fmt.Errorf("context %q does not exist", name))
 	}
 	// Ignore other errors; if relevant, they will produce an error when
 	// performing the actual delete.

cli/command/context/remove_test.go

@@ -60,5 +60,5 @@ func TestRemoveDefault(t *testing.T) {
 	createTestContext(t, cli, "other", nil)
 	cli.SetCurrentContext("current")
 	err := RunRemove(cli, RemoveOptions{}, []string{"default"})
-	assert.ErrorContains(t, err, `default: context "default" cannot be removed`)
+	assert.ErrorContains(t, err, `context "default" cannot be removed`)
 }