Skip to content

Commit 7f207f3

Browse files
silvin-lubeckisilvin-lubecki
committed
Introduce functional arguments to NewDockerCli for a more stable API.
Signed-off-by: Silvin Lubecki <[email protected]>
1 parent eb0ba4f commit 7f207f3

7 files changed

Lines changed: 214 additions & 46 deletions

File tree

‎cli/command/cli.go‎

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@ import (
1919
cliflags "github.com/docker/cli/cli/flags"
2020
manifeststore "github.com/docker/cli/cli/manifest/store"
2121
registryclient "github.com/docker/cli/cli/registry/client"
22+
"github.com/docker/cli/cli/streams"
2223
"github.com/docker/cli/cli/trust"
24+
"github.com/docker/cli/internal/containerizedengine"
2325
dopts "github.com/docker/cli/opts"
2426
clitypes "github.com/docker/cli/types"
2527
"github.com/docker/docker/api/types"
2628
registrytypes "github.com/docker/docker/api/types/registry"
2729
"github.com/docker/docker/client"
30+
"github.com/docker/docker/pkg/term"
2831
"github.com/docker/go-connections/tlsconfig"
2932
"github.com/pkg/errors"
3033
"github.com/spf13/cobra"
@@ -35,18 +38,19 @@ import (
3538

3639
// Streams is an interface which exposes the standard input and output streams
3740
type Streams interface {
38-
In() *InStream
39-
Out() *OutStream
41+
In() *streams.In
42+
Out() *streams.Out
4043
Err() io.Writer
4144
}
4245

4346
// Cli represents the docker command line client.
4447
type Cli interface {
4548
Client() client.APIClient
46-
Out() *OutStream
49+
Out() *streams.Out
4750
Err() io.Writer
48-
In() *InStream
49-
SetIn(in *InStream)
51+
In() *streams.In
52+
SetIn(in *streams.In)
53+
Apply(ops ...DockerCliOption) error
5054
ConfigFile() *configfile.ConfigFile
5155
ServerInfo() ServerInfo
5256
ClientInfo() ClientInfo
@@ -66,8 +70,8 @@ type Cli interface {
6670
// Instances of the client can be returned from NewDockerCli.
6771
type DockerCli struct {
6872
configFile *configfile.ConfigFile
69-
in *InStream
70-
out *OutStream
73+
in *streams.In
74+
out *streams.Out
7175
err io.Writer
7276
client client.APIClient
7377
serverInfo ServerInfo
@@ -96,7 +100,7 @@ func (cli *DockerCli) Client() client.APIClient {
96100
}
97101

98102
// Out returns the writer used for stdout
99-
func (cli *DockerCli) Out() *OutStream {
103+
func (cli *DockerCli) Out() *streams.Out {
100104
return cli.out
101105
}
102106

@@ -106,12 +110,12 @@ func (cli *DockerCli) Err() io.Writer {
106110
}
107111

108112
// SetIn sets the reader used for stdin
109-
func (cli *DockerCli) SetIn(in *InStream) {
113+
func (cli *DockerCli) SetIn(in *streams.In) {
110114
cli.in = in
111115
}
112116

113117
// In returns the reader used for stdin
114-
func (cli *DockerCli) In() *InStream {
118+
func (cli *DockerCli) In() *streams.In {
115119
return cli.in
116120
}
117121

@@ -393,6 +397,16 @@ func (cli *DockerCli) DockerEndpoint() docker.Endpoint {
393397
return cli.dockerEndpoint
394398
}
395399

400+
// Apply all the operation on the cli
401+
func (cli *DockerCli) Apply(ops ...DockerCliOption) error {
402+
for _, op := range ops {
403+
if err := op(cli); err != nil {
404+
return err
405+
}
406+
}
407+
return nil
408+
}
409+
396410
// ServerInfo stores details about the supported features and platform of the
397411
// server
398412
type ServerInfo struct {
@@ -407,9 +421,32 @@ type ClientInfo struct {
407421
DefaultVersion string
408422
}
409423

410-
// NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.
411-
func NewDockerCli(in io.ReadCloser, out, err io.Writer, isTrusted bool, containerizedFn func(string) (clitypes.ContainerizedClient, error)) *DockerCli {
412-
return &DockerCli{in: NewInStream(in), out: NewOutStream(out), err: err, contentTrust: isTrusted, newContainerizeClient: containerizedFn}
424+
// NewDockerCli returns a DockerCli instance with all operators applied on it.
425+
// It applies by default the standard streams, the content trust from
426+
// environment and the default containerized client constructor operations.
427+
func NewDockerCli(ops ...DockerCliOption) (*DockerCli, error) {
428+
cli := &DockerCli{}
429+
defaultOps := []DockerCliOption{
430+
WithContentTrustFromEnv(),
431+
WithContainerizedClient(containerizedengine.NewClient),
432+
}
433+
ops = append(defaultOps, ops...)
434+
if err := cli.Apply(ops...); err != nil {
435+
return nil, err
436+
}
437+
if cli.out == nil || cli.in == nil || cli.err == nil {
438+
stdin, stdout, stderr := term.StdStreams()
439+
if cli.in == nil {
440+
cli.in = streams.NewIn(stdin)
441+
}
442+
if cli.out == nil {
443+
cli.out = streams.NewOut(stdout)
444+
}
445+
if cli.err == nil {
446+
cli.err = stderr
447+
}
448+
}
449+
return cli, nil
413450
}
414451

415452
func getServerHost(hosts []string, tlsOptions *tlsconfig.Options) (string, error) {

‎cli/command/cli_options.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package command
2+
3+
import (
4+
"io"
5+
"os"
6+
"strconv"
7+
8+
"github.com/docker/cli/cli/streams"
9+
clitypes "github.com/docker/cli/types"
10+
"github.com/docker/docker/pkg/term"
11+
)
12+
13+
// DockerCliOption applies a modification on a DockerCli.
14+
type DockerCliOption func(cli *DockerCli) error
15+
16+
// WithStandardStreams sets a cli in, out and err streams with the standard streams.
17+
func WithStandardStreams() DockerCliOption {
18+
return func(cli *DockerCli) error {
19+
// Set terminal emulation based on platform as required.
20+
stdin, stdout, stderr := term.StdStreams()
21+
cli.in = streams.NewIn(stdin)
22+
cli.out = streams.NewOut(stdout)
23+
cli.err = stderr
24+
return nil
25+
}
26+
}
27+
28+
// WithCombinedStreams uses the same stream for the output and error streams.
29+
func WithCombinedStreams(combined io.Writer) DockerCliOption {
30+
return func(cli *DockerCli) error {
31+
cli.out = streams.NewOut(combined)
32+
cli.err = combined
33+
return nil
34+
}
35+
}
36+
37+
// WithInputStream sets a cli input stream.
38+
func WithInputStream(in io.ReadCloser) DockerCliOption {
39+
return func(cli *DockerCli) error {
40+
cli.in = streams.NewIn(in)
41+
return nil
42+
}
43+
}
44+
45+
// WithOutputStream sets a cli output stream.
46+
func WithOutputStream(out io.Writer) DockerCliOption {
47+
return func(cli *DockerCli) error {
48+
cli.out = streams.NewOut(out)
49+
return nil
50+
}
51+
}
52+
53+
// WithErrorStream sets a cli error stream.
54+
func WithErrorStream(err io.Writer) DockerCliOption {
55+
return func(cli *DockerCli) error {
56+
cli.err = err
57+
return nil
58+
}
59+
}
60+
61+
// WithContentTrustFromEnv enables content trust on a cli from environment variable DOCKER_CONTENT_TRUST value.
62+
func WithContentTrustFromEnv() DockerCliOption {
63+
return func(cli *DockerCli) error {
64+
cli.contentTrust = false
65+
if e := os.Getenv("DOCKER_CONTENT_TRUST"); e != "" {
66+
if t, err := strconv.ParseBool(e); t || err != nil {
67+
// treat any other value as true
68+
cli.contentTrust = true
69+
}
70+
}
71+
return nil
72+
}
73+
}
74+
75+
// WithContentTrust enables content trust on a cli.
76+
func WithContentTrust(enabled bool) DockerCliOption {
77+
return func(cli *DockerCli) error {
78+
cli.contentTrust = enabled
79+
return nil
80+
}
81+
}
82+
83+
// WithContainerizedClient sets the containerized client constructor on a cli.
84+
func WithContainerizedClient(containerizedFn func(string) (clitypes.ContainerizedClient, error)) DockerCliOption {
85+
return func(cli *DockerCli) error {
86+
cli.newContainerizeClient = containerizedFn
87+
return nil
88+
}
89+
}

‎cli/command/cli_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
package command
22

33
import (
4+
"bytes"
45
"context"
56
"crypto/x509"
7+
"fmt"
8+
"io/ioutil"
69
"os"
710
"runtime"
811
"testing"
912

1013
cliconfig "github.com/docker/cli/cli/config"
1114
"github.com/docker/cli/cli/config/configfile"
1215
"github.com/docker/cli/cli/flags"
16+
clitypes "github.com/docker/cli/types"
1317
"github.com/docker/docker/api"
1418
"github.com/docker/docker/api/types"
1519
"github.com/docker/docker/client"
@@ -247,3 +251,44 @@ func TestGetClientWithPassword(t *testing.T) {
247251
})
248252
}
249253
}
254+
255+
func TestNewDockerCliAndOperators(t *testing.T) {
256+
// Test default operations and also overriding default ones
257+
cli, err := NewDockerCli(
258+
WithContentTrust(true),
259+
WithContainerizedClient(func(string) (clitypes.ContainerizedClient, error) { return nil, nil }),
260+
)
261+
assert.NilError(t, err)
262+
// Check streams are initialized
263+
assert.Check(t, cli.In() != nil)
264+
assert.Check(t, cli.Out() != nil)
265+
assert.Check(t, cli.Err() != nil)
266+
assert.Equal(t, cli.ContentTrustEnabled(), true)
267+
client, err := cli.NewContainerizedEngineClient("")
268+
assert.NilError(t, err)
269+
assert.Equal(t, client, nil)
270+
271+
// Apply can modify a dockerCli after construction
272+
inbuf := bytes.NewBuffer([]byte("input"))
273+
outbuf := bytes.NewBuffer(nil)
274+
errbuf := bytes.NewBuffer(nil)
275+
cli.Apply(
276+
WithInputStream(ioutil.NopCloser(inbuf)),
277+
WithOutputStream(outbuf),
278+
WithErrorStream(errbuf),
279+
)
280+
// Check input stream
281+
inputStream, err := ioutil.ReadAll(cli.In())
282+
assert.NilError(t, err)
283+
assert.Equal(t, string(inputStream), "input")
284+
// Check output stream
285+
fmt.Fprintf(cli.Out(), "output")
286+
outputStream, err := ioutil.ReadAll(outbuf)
287+
assert.NilError(t, err)
288+
assert.Equal(t, string(outputStream), "output")
289+
// Check error stream
290+
fmt.Fprintf(cli.Err(), "error")
291+
errStream, err := ioutil.ReadAll(errbuf)
292+
assert.NilError(t, err)
293+
assert.Equal(t, string(errStream), "error")
294+
}

‎cmd/docker/docker.go‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"errors"
55
"fmt"
66
"os"
7-
"strconv"
87
"strings"
98

109
"github.com/docker/cli/cli"
@@ -13,10 +12,8 @@ import (
1312
cliconfig "github.com/docker/cli/cli/config"
1413
"github.com/docker/cli/cli/debug"
1514
cliflags "github.com/docker/cli/cli/flags"
16-
"github.com/docker/cli/internal/containerizedengine"
1715
"github.com/docker/docker/api/types/versions"
1816
"github.com/docker/docker/client"
19-
"github.com/docker/docker/pkg/term"
2017
"github.com/sirupsen/logrus"
2118
"github.com/spf13/cobra"
2219
"github.com/spf13/pflag"
@@ -170,17 +167,19 @@ func noArgs(cmd *cobra.Command, args []string) error {
170167
}
171168

172169
func main() {
173-
// Set terminal emulation based on platform as required.
174-
stdin, stdout, stderr := term.StdStreams()
175-
logrus.SetOutput(stderr)
170+
dockerCli, err := command.NewDockerCli()
171+
if err != nil {
172+
fmt.Fprintln(dockerCli.Err(), err)
173+
os.Exit(1)
174+
}
175+
logrus.SetOutput(dockerCli.Err())
176176

177-
dockerCli := command.NewDockerCli(stdin, stdout, stderr, contentTrustEnabled(), containerizedengine.NewClient)
178177
cmd := newDockerCommand(dockerCli)
179178

180179
if err := cmd.Execute(); err != nil {
181180
if sterr, ok := err.(cli.StatusError); ok {
182181
if sterr.Status != "" {
183-
fmt.Fprintln(stderr, sterr.Status)
182+
fmt.Fprintln(dockerCli.Err(), sterr.Status)
184183
}
185184
// StatusError should only be used for errors, and all errors should
186185
// have a non-zero exit status, so never exit with 0
@@ -189,21 +188,11 @@ func main() {
189188
}
190189
os.Exit(sterr.StatusCode)
191190
}
192-
fmt.Fprintln(stderr, err)
191+
fmt.Fprintln(dockerCli.Err(), err)
193192
os.Exit(1)
194193
}
195194
}
196195

197-
func contentTrustEnabled() bool {
198-
if e := os.Getenv("DOCKER_CONTENT_TRUST"); e != "" {
199-
if t, err := strconv.ParseBool(e); t || err != nil {
200-
// treat any other value as true
201-
return true
202-
}
203-
}
204-
return false
205-
}
206-
207196
func dockerPreRun(opts *cliflags.ClientOptions) {
208197
cliflags.SetLogLevel(opts.Common.LogLevel)
209198

‎cmd/docker/docker_test.go‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,27 +25,33 @@ func TestClientDebugEnabled(t *testing.T) {
2525
assert.Check(t, is.Equal(logrus.DebugLevel, logrus.GetLevel()))
2626
}
2727

28+
var discard = ioutil.NopCloser(bytes.NewBuffer(nil))
29+
2830
func TestExitStatusForInvalidSubcommandWithHelpFlag(t *testing.T) {
29-
discard := ioutil.Discard
30-
cmd := newDockerCommand(command.NewDockerCli(os.Stdin, discard, discard, false, nil))
31+
cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(ioutil.Discard))
32+
assert.NilError(t, err)
33+
cmd := newDockerCommand(cli)
3134
cmd.SetArgs([]string{"help", "invalid"})
32-
err := cmd.Execute()
35+
err = cmd.Execute()
3336
assert.Error(t, err, "unknown help topic: invalid")
3437
}
3538

3639
func TestExitStatusForInvalidSubcommand(t *testing.T) {
37-
discard := ioutil.Discard
38-
cmd := newDockerCommand(command.NewDockerCli(os.Stdin, discard, discard, false, nil))
40+
cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(ioutil.Discard))
41+
assert.NilError(t, err)
42+
cmd := newDockerCommand(cli)
3943
cmd.SetArgs([]string{"invalid"})
40-
err := cmd.Execute()
44+
err = cmd.Execute()
4145
assert.Check(t, is.ErrorContains(err, "docker: 'invalid' is not a docker command."))
4246
}
4347

4448
func TestVersion(t *testing.T) {
4549
var b bytes.Buffer
46-
cmd := newDockerCommand(command.NewDockerCli(os.Stdin, &b, &b, false, nil))
50+
cli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b))
51+
assert.NilError(t, err)
52+
cmd := newDockerCommand(cli)
4753
cmd.SetArgs([]string{"--version"})
48-
err := cmd.Execute()
54+
err = cmd.Execute()
4955
assert.NilError(t, err)
5056
assert.Check(t, is.Contains(b.String(), "Docker version"))
5157
}

0 commit comments

Comments
 (0)