Introduce functional arguments to NewDockerCli for a more stable API.

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>

Author: silvin-lubecki

cli/command/cli.go

@@ -19,12 +19,15 @@ import (
 	cliflags "github.com/docker/cli/cli/flags"
 	manifeststore "github.com/docker/cli/cli/manifest/store"
 	registryclient "github.com/docker/cli/cli/registry/client"
+	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/cli/trust"
+	"github.com/docker/cli/internal/containerizedengine"
 	dopts "github.com/docker/cli/opts"
 	clitypes "github.com/docker/cli/types"
 	"github.com/docker/docker/api/types"
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/client"
+	"github.com/docker/docker/pkg/term"
 	"github.com/docker/go-connections/tlsconfig"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -35,18 +38,19 @@ import (
 
 // Streams is an interface which exposes the standard input and output streams
 type Streams interface {
-	In() *InStream
-	Out() *OutStream
+	In() *streams.In
+	Out() *streams.Out
 	Err() io.Writer
 }
 
 // Cli represents the docker command line client.
 type Cli interface {
 	Client() client.APIClient
-	Out() *OutStream
+	Out() *streams.Out
 	Err() io.Writer
-	In() *InStream
-	SetIn(in *InStream)
+	In() *streams.In
+	SetIn(in *streams.In)
+	Apply(ops ...DockerCliOption) error
 	ConfigFile() *configfile.ConfigFile
 	ServerInfo() ServerInfo
 	ClientInfo() ClientInfo
@@ -66,8 +70,8 @@ type Cli interface {
 // Instances of the client can be returned from NewDockerCli.
 type DockerCli struct {
 	configFile            *configfile.ConfigFile
-	in                    *InStream
-	out                   *OutStream
+	in                    *streams.In
+	out                   *streams.Out
 	err                   io.Writer
 	client                client.APIClient
 	serverInfo            ServerInfo
@@ -96,7 +100,7 @@ func (cli *DockerCli) Client() client.APIClient {
 }
 
 // Out returns the writer used for stdout
-func (cli *DockerCli) Out() *OutStream {
+func (cli *DockerCli) Out() *streams.Out {
 	return cli.out
 }
 
@@ -106,12 +110,12 @@ func (cli *DockerCli) Err() io.Writer {
 }
 
 // SetIn sets the reader used for stdin
-func (cli *DockerCli) SetIn(in *InStream) {
+func (cli *DockerCli) SetIn(in *streams.In) {
 	cli.in = in
 }
 
 // In returns the reader used for stdin
-func (cli *DockerCli) In() *InStream {
+func (cli *DockerCli) In() *streams.In {
 	return cli.in
 }
 
@@ -393,6 +397,16 @@ func (cli *DockerCli) DockerEndpoint() docker.Endpoint {
 	return cli.dockerEndpoint
 }
 
+// Apply all the operation on the cli
+func (cli *DockerCli) Apply(ops ...DockerCliOption) error {
+	for _, op := range ops {
+		if err := op(cli); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // ServerInfo stores details about the supported features and platform of the
 // server
 type ServerInfo struct {
@@ -407,9 +421,32 @@ type ClientInfo struct {
 	DefaultVersion  string
 }
 
-// NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.
-func NewDockerCli(in io.ReadCloser, out, err io.Writer, isTrusted bool, containerizedFn func(string) (clitypes.ContainerizedClient, error)) *DockerCli {
-	return &DockerCli{in: NewInStream(in), out: NewOutStream(out), err: err, contentTrust: isTrusted, newContainerizeClient: containerizedFn}
+// NewDockerCli returns a DockerCli instance with all operators applied on it.
+// It applies by default the standard streams, the content trust from
+// environment and the default containerized client constructor operations.
+func NewDockerCli(ops ...DockerCliOption) (*DockerCli, error) {
+	cli := &DockerCli{}
+	defaultOps := []DockerCliOption{
+		WithContentTrustFromEnv(),
+		WithContainerizedClient(containerizedengine.NewClient),
+	}
+	ops = append(defaultOps, ops...)
+	if err := cli.Apply(ops...); err != nil {
+		return nil, err
+	}
+	if cli.out == nil || cli.in == nil || cli.err == nil {
+		stdin, stdout, stderr := term.StdStreams()
+		if cli.in == nil {
+			cli.in = streams.NewIn(stdin)
+		}
+		if cli.out == nil {
+			cli.out = streams.NewOut(stdout)
+		}
+		if cli.err == nil {
+			cli.err = stderr
+		}
+	}
+	return cli, nil
 }
 
 func getServerHost(hosts []string, tlsOptions *tlsconfig.Options) (string, error) {

cli/command/cli_options.go

@@ -0,0 +1,89 @@
+package command
+
+import (
+	"io"
+	"os"
+	"strconv"
+
+	"github.com/docker/cli/cli/streams"
+	clitypes "github.com/docker/cli/types"
+	"github.com/docker/docker/pkg/term"
+)
+
+// DockerCliOption applies a modification on a DockerCli.
+type DockerCliOption func(cli *DockerCli) error
+
+// WithStandardStreams sets a cli in, out and err streams with the standard streams.
+func WithStandardStreams() DockerCliOption {
+	return func(cli *DockerCli) error {
+		// Set terminal emulation based on platform as required.
+		stdin, stdout, stderr := term.StdStreams()
+		cli.in = streams.NewIn(stdin)
+		cli.out = streams.NewOut(stdout)
+		cli.err = stderr
+		return nil
+	}
+}
+
+// WithCombinedStreams uses the same stream for the output and error streams.
+func WithCombinedStreams(combined io.Writer) DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.out = streams.NewOut(combined)
+		cli.err = combined
+		return nil
+	}
+}
+
+// WithInputStream sets a cli input stream.
+func WithInputStream(in io.ReadCloser) DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.in = streams.NewIn(in)
+		return nil
+	}
+}
+
+// WithOutputStream sets a cli output stream.
+func WithOutputStream(out io.Writer) DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.out = streams.NewOut(out)
+		return nil
+	}
+}
+
+// WithErrorStream sets a cli error stream.
+func WithErrorStream(err io.Writer) DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.err = err
+		return nil
+	}
+}
+
+// WithContentTrustFromEnv enables content trust on a cli from environment variable DOCKER_CONTENT_TRUST value.
+func WithContentTrustFromEnv() DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.contentTrust = false
+		if e := os.Getenv("DOCKER_CONTENT_TRUST"); e != "" {
+			if t, err := strconv.ParseBool(e); t || err != nil {
+				// treat any other value as true
+				cli.contentTrust = true
+			}
+		}
+		return nil
+	}
+}
+
+// WithContentTrust enables content trust on a cli.
+func WithContentTrust(enabled bool) DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.contentTrust = enabled
+		return nil
+	}
+}
+
+// WithContainerizedClient sets the containerized client constructor on a cli.
+func WithContainerizedClient(containerizedFn func(string) (clitypes.ContainerizedClient, error)) DockerCliOption {
+	return func(cli *DockerCli) error {
+		cli.newContainerizeClient = containerizedFn
+		return nil
+	}
+}

cli/command/cli_test.go

@@ -1,15 +1,19 @@
 package command
 
 import (
+	"bytes"
 	"context"
 	"crypto/x509"
+	"fmt"
+	"io/ioutil"
 	"os"
 	"runtime"
 	"testing"
 
 	cliconfig "github.com/docker/cli/cli/config"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/flags"
+	clitypes "github.com/docker/cli/types"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
@@ -247,3 +251,44 @@ func TestGetClientWithPassword(t *testing.T) {
 		})
 	}
 }
+
+func TestNewDockerCliAndOperators(t *testing.T) {
+	// Test default operations and also overriding default ones
+	cli, err := NewDockerCli(
+		WithContentTrust(true),
+		WithContainerizedClient(func(string) (clitypes.ContainerizedClient, error) { return nil, nil }),
+	)
+	assert.NilError(t, err)
+	// Check streams are initialized
+	assert.Check(t, cli.In() != nil)
+	assert.Check(t, cli.Out() != nil)
+	assert.Check(t, cli.Err() != nil)
+	assert.Equal(t, cli.ContentTrustEnabled(), true)
+	client, err := cli.NewContainerizedEngineClient("")
+	assert.NilError(t, err)
+	assert.Equal(t, client, nil)
+
+	// Apply can modify a dockerCli after construction
+	inbuf := bytes.NewBuffer([]byte("input"))
+	outbuf := bytes.NewBuffer(nil)
+	errbuf := bytes.NewBuffer(nil)
+	cli.Apply(
+		WithInputStream(ioutil.NopCloser(inbuf)),
+		WithOutputStream(outbuf),
+		WithErrorStream(errbuf),
+	)
+	// Check input stream
+	inputStream, err := ioutil.ReadAll(cli.In())
+	assert.NilError(t, err)
+	assert.Equal(t, string(inputStream), "input")
+	// Check output stream
+	fmt.Fprintf(cli.Out(), "output")
+	outputStream, err := ioutil.ReadAll(outbuf)
+	assert.NilError(t, err)
+	assert.Equal(t, string(outputStream), "output")
+	// Check error stream
+	fmt.Fprintf(cli.Err(), "error")
+	errStream, err := ioutil.ReadAll(errbuf)
+	assert.NilError(t, err)
+	assert.Equal(t, string(errStream), "error")
+}

cmd/docker/docker.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"strconv"
 	"strings"
 
 	"github.com/docker/cli/cli"
@@ -13,10 +12,8 @@ import (
 	cliconfig "github.com/docker/cli/cli/config"
 	"github.com/docker/cli/cli/debug"
 	cliflags "github.com/docker/cli/cli/flags"
-	"github.com/docker/cli/internal/containerizedengine"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/client"
-	"github.com/docker/docker/pkg/term"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -170,17 +167,19 @@ func noArgs(cmd *cobra.Command, args []string) error {
 }
 
 func main() {
-	// Set terminal emulation based on platform as required.
-	stdin, stdout, stderr := term.StdStreams()
-	logrus.SetOutput(stderr)
+	dockerCli, err := command.NewDockerCli()
+	if err != nil {
+		fmt.Fprintln(dockerCli.Err(), err)
+		os.Exit(1)
+	}
+	logrus.SetOutput(dockerCli.Err())
 
-	dockerCli := command.NewDockerCli(stdin, stdout, stderr, contentTrustEnabled(), containerizedengine.NewClient)
 	cmd := newDockerCommand(dockerCli)
 
 	if err := cmd.Execute(); err != nil {
 		if sterr, ok := err.(cli.StatusError); ok {
 			if sterr.Status != "" {
-				fmt.Fprintln(stderr, sterr.Status)
+				fmt.Fprintln(dockerCli.Err(), sterr.Status)
 			}
 			// StatusError should only be used for errors, and all errors should
 			// have a non-zero exit status, so never exit with 0
@@ -189,21 +188,11 @@ func main() {
 			}
 			os.Exit(sterr.StatusCode)
 		}
-		fmt.Fprintln(stderr, err)
+		fmt.Fprintln(dockerCli.Err(), err)
 		os.Exit(1)
 	}
 }
 
-func contentTrustEnabled() bool {
-	if e := os.Getenv("DOCKER_CONTENT_TRUST"); e != "" {
-		if t, err := strconv.ParseBool(e); t || err != nil {
-			// treat any other value as true
-			return true
-		}
-	}
-	return false
-}
-
 func dockerPreRun(opts *cliflags.ClientOptions) {
 	cliflags.SetLogLevel(opts.Common.LogLevel)
 

cmd/docker/docker_test.go

@@ -25,27 +25,33 @@ func TestClientDebugEnabled(t *testing.T) {
 	assert.Check(t, is.Equal(logrus.DebugLevel, logrus.GetLevel()))
 }
 
+var discard = ioutil.NopCloser(bytes.NewBuffer(nil))
+
 func TestExitStatusForInvalidSubcommandWithHelpFlag(t *testing.T) {
-	discard := ioutil.Discard
-	cmd := newDockerCommand(command.NewDockerCli(os.Stdin, discard, discard, false, nil))
+	cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(ioutil.Discard))
+	assert.NilError(t, err)
+	cmd := newDockerCommand(cli)
 	cmd.SetArgs([]string{"help", "invalid"})
-	err := cmd.Execute()
+	err = cmd.Execute()
 	assert.Error(t, err, "unknown help topic: invalid")
 }
 
 func TestExitStatusForInvalidSubcommand(t *testing.T) {
-	discard := ioutil.Discard
-	cmd := newDockerCommand(command.NewDockerCli(os.Stdin, discard, discard, false, nil))
+	cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(ioutil.Discard))
+	assert.NilError(t, err)
+	cmd := newDockerCommand(cli)
 	cmd.SetArgs([]string{"invalid"})
-	err := cmd.Execute()
+	err = cmd.Execute()
 	assert.Check(t, is.ErrorContains(err, "docker: 'invalid' is not a docker command."))
 }
 
 func TestVersion(t *testing.T) {
 	var b bytes.Buffer
-	cmd := newDockerCommand(command.NewDockerCli(os.Stdin, &b, &b, false, nil))
+	cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b))
+	assert.NilError(t, err)
+	cmd := newDockerCommand(cli)
 	cmd.SetArgs([]string{"--version"})
-	err := cmd.Execute()
+	err = cmd.Execute()
 	assert.NilError(t, err)
 	assert.Check(t, is.Contains(b.String(), "Docker version"))
 }