From 84fe1a1b5b91d7b4ce79a9978fad8dad30a47fce Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 20 Dec 2017 15:04:41 +0100 Subject: [PATCH] Add support for experimental Cli configuration Allow to mark some commands and flags experimental on cli (i.e. not depending to the state of the daemon). This will allow more flexibility on experimentation with the cli. Marking `docker trust` as cli experimental as it is documented so. Signed-off-by: Vincent Demeester --- cli/command/cli.go | 55 +++++++++++++++++++++++-------- cli/command/cli_test.go | 47 +++++++++++++++++++++++--- cli/command/system/version.go | 3 ++ cli/command/trust/cmd.go | 9 ++--- cli/config/configfile/file.go | 1 + cmd/docker/docker.go | 19 +++++++++++ e2e/internal/fixtures/fixtures.go | 3 +- 7 files changed, 115 insertions(+), 22 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index ff4b78adc6..12c24ca0fe 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -42,24 +42,25 @@ type Cli interface { SetIn(in *InStream) ConfigFile() *configfile.ConfigFile ServerInfo() ServerInfo + ClientInfo() ClientInfo NotaryClient(imgRefAndAuth trust.ImageRefAndAuth, actions []string) (notaryclient.Repository, error) } // DockerCli is an instance the docker command line client. // Instances of the client can be returned from NewDockerCli. type DockerCli struct { - configFile *configfile.ConfigFile - in *InStream - out *OutStream - err io.Writer - client client.APIClient - defaultVersion string - server ServerInfo + configFile *configfile.ConfigFile + in *InStream + out *OutStream + err io.Writer + client client.APIClient + serverInfo ServerInfo + clientInfo ClientInfo } // DefaultVersion returns api.defaultVersion or DOCKER_API_VERSION if specified. func (cli *DockerCli) DefaultVersion() string { - return cli.defaultVersion + return cli.clientInfo.DefaultVersion } // Client returns the APIClient @@ -104,7 +105,12 @@ func (cli *DockerCli) ConfigFile() *configfile.ConfigFile { // ServerInfo returns the server version details for the host this client is // connected to func (cli *DockerCli) ServerInfo() ServerInfo { - return cli.server + return cli.serverInfo +} + +// ClientInfo returns the client details for the cli +func (cli *DockerCli) ClientInfo() ClientInfo { + return cli.clientInfo } // Initialize the dockerCli runs initialization that must happen after command @@ -125,17 +131,34 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error { if err != nil { return err } + hasExperimental, err := isEnabled(cli.configFile.Experimental) + if err != nil { + return errors.Wrap(err, "Experimental field") + } + cli.clientInfo = ClientInfo{ + DefaultVersion: cli.client.ClientVersion(), + HasExperimental: hasExperimental, + } cli.initializeFromClient() return nil } -func (cli *DockerCli) initializeFromClient() { - cli.defaultVersion = cli.client.ClientVersion() +func isEnabled(value string) (bool, error) { + switch value { + case "enabled": + return true, nil + case "", "disabled": + return false, nil + default: + return false, errors.Errorf("%q is not valid, should be either enabled or disabled", value) + } +} +func (cli *DockerCli) initializeFromClient() { ping, err := cli.client.Ping(context.Background()) if err != nil { // Default to true if we fail to connect to daemon - cli.server = ServerInfo{HasExperimental: true} + cli.serverInfo = ServerInfo{HasExperimental: true} if ping.APIVersion != "" { cli.client.NegotiateAPIVersionPing(ping) @@ -143,7 +166,7 @@ func (cli *DockerCli) initializeFromClient() { return } - cli.server = ServerInfo{ + cli.serverInfo = ServerInfo{ HasExperimental: ping.Experimental, OSType: ping.OSType, } @@ -176,6 +199,12 @@ type ServerInfo struct { OSType string } +// ClientInfo stores details about the supported features of the client +type ClientInfo struct { + HasExperimental bool + 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) *DockerCli { return &DockerCli{in: NewInStream(in), out: NewOutStream(out), err: err} diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 16ba7c4ff9..d2cf1352d6 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -1,17 +1,18 @@ package command import ( + "crypto/x509" "os" "testing" - "crypto/x509" - + cliconfig "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/flags" "github.com/docker/cli/internal/test/testutil" "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/client" + "github.com/gotestyourself/gotestyourself/fs" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -124,13 +125,51 @@ func TestInitializeFromClient(t *testing.T) { cli := &DockerCli{client: apiclient} cli.initializeFromClient() - assert.Equal(t, defaultVersion, cli.defaultVersion) - assert.Equal(t, testcase.expectedServer, cli.server) + assert.Equal(t, testcase.expectedServer, cli.serverInfo) assert.Equal(t, testcase.negotiated, apiclient.negotiated) }) } } +func TestExperimentalCLI(t *testing.T) { + defaultVersion := "v1.55" + + var testcases = []struct { + doc string + configfile string + expectedExperimentalCLI bool + }{ + { + doc: "default", + configfile: `{}`, + expectedExperimentalCLI: false, + }, + { + doc: "experimental", + configfile: `{ + "experimental": "enabled" +}`, + expectedExperimentalCLI: true, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.doc, func(t *testing.T) { + dir := fs.NewDir(t, testcase.doc, fs.WithFile("config.json", testcase.configfile)) + defer dir.Remove() + apiclient := &fakeClient{ + version: defaultVersion, + } + + cli := &DockerCli{client: apiclient, err: os.Stderr} + cliconfig.SetDir(dir.Path()) + err := cli.Initialize(flags.NewClientOptions()) + assert.NoError(t, err) + assert.Equal(t, testcase.expectedExperimentalCLI, cli.ClientInfo().HasExperimental) + }) + } +} + func TestGetClientWithPassword(t *testing.T) { expected := "password" diff --git a/cli/command/system/version.go b/cli/command/system/version.go index d063045d5c..45e34d761a 100644 --- a/cli/command/system/version.go +++ b/cli/command/system/version.go @@ -23,6 +23,7 @@ Client:{{if ne .Platform.Name ""}} {{.Platform.Name}}{{end}} Git commit: {{.GitCommit}} Built: {{.BuildTime}} OS/Arch: {{.Os}}/{{.Arch}} + Experimental: {{.Experimental}} {{- end}} {{- if .ServerOK}}{{with .Server}} @@ -69,6 +70,7 @@ type clientVersion struct { Os string Arch string BuildTime string `json:",omitempty"` + Experimental bool } // ServerOK returns true when the client could connect to the docker server @@ -133,6 +135,7 @@ func runVersion(dockerCli *command.DockerCli, opts *versionOptions) error { BuildTime: cli.BuildTime, Os: runtime.GOOS, Arch: runtime.GOARCH, + Experimental: dockerCli.ClientInfo().HasExperimental, }, } vd.Client.Platform.Name = cli.PlatformName diff --git a/cli/command/trust/cmd.go b/cli/command/trust/cmd.go index 11131be6df..6ffc57d05f 100644 --- a/cli/command/trust/cmd.go +++ b/cli/command/trust/cmd.go @@ -9,10 +9,11 @@ import ( // NewTrustCommand returns a cobra command for `trust` subcommands func NewTrustCommand(dockerCli command.Cli) *cobra.Command { cmd := &cobra.Command{ - Use: "trust", - Short: "Manage trust on Docker images (experimental)", - Args: cli.NoArgs, - RunE: command.ShowHelp(dockerCli.Err()), + Use: "trust", + Short: "Manage trust on Docker images (experimental)", + Args: cli.NoArgs, + RunE: command.ShowHelp(dockerCli.Err()), + Annotations: map[string]string{"experimentalCLI": ""}, } cmd.AddCommand( newViewCommand(dockerCli), diff --git a/cli/config/configfile/file.go b/cli/config/configfile/file.go index 9a923d3241..2a2bfe04d5 100644 --- a/cli/config/configfile/file.go +++ b/cli/config/configfile/file.go @@ -44,6 +44,7 @@ type ConfigFile struct { NodesFormat string `json:"nodesFormat,omitempty"` PruneFilters []string `json:"pruneFilters,omitempty"` Proxies map[string]ProxyConfig `json:"proxies,omitempty"` + Experimental string `json:"experimental,omitempty"` } // ProxyConfig contains proxy configuration settings diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 84e9e2520a..6c3cba61fc 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -193,6 +193,7 @@ func dockerPreRun(opts *cliflags.ClientOptions) { type versionDetails interface { Client() client.APIClient + ClientInfo() command.ClientInfo ServerInfo() command.ServerInfo } @@ -200,6 +201,7 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) { clientVersion := details.Client().ClientVersion() osType := details.ServerInfo().OSType hasExperimental := details.ServerInfo().HasExperimental + hasExperimentalCLI := details.ClientInfo().HasExperimental cmd.Flags().VisitAll(func(f *pflag.Flag) { // hide experimental flags @@ -208,6 +210,11 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) { f.Hidden = true } } + if !hasExperimentalCLI { + if _, ok := f.Annotations["experimentalCLI"]; ok { + f.Hidden = true + } + } // hide flags not supported by the server if !isOSTypeSupported(f, osType) || !isVersionSupported(f, clientVersion) { @@ -222,6 +229,11 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) { subcmd.Hidden = true } } + if !hasExperimentalCLI { + if _, ok := subcmd.Annotations["experimentalCLI"]; ok { + subcmd.Hidden = true + } + } // hide subcommands not supported by the server if subcmdVersion, ok := subcmd.Annotations["version"]; ok && versions.LessThan(clientVersion, subcmdVersion) { @@ -234,6 +246,7 @@ func isSupported(cmd *cobra.Command, details versionDetails) error { clientVersion := details.Client().ClientVersion() osType := details.ServerInfo().OSType hasExperimental := details.ServerInfo().HasExperimental + hasExperimentalCLI := details.ClientInfo().HasExperimental // Check recursively so that, e.g., `docker stack ls` returns the same output as `docker stack` for curr := cmd; curr != nil; curr = curr.Parent() { @@ -243,6 +256,9 @@ func isSupported(cmd *cobra.Command, details versionDetails) error { if _, ok := curr.Annotations["experimental"]; ok && !hasExperimental { return fmt.Errorf("%s is only supported on a Docker daemon with experimental features enabled", cmd.CommandPath()) } + if _, ok := curr.Annotations["experimentalCLI"]; ok && !hasExperimentalCLI { + return fmt.Errorf("%s is only supported when experimental cli features are enabled", cmd.CommandPath()) + } } errs := []string{} @@ -260,6 +276,9 @@ func isSupported(cmd *cobra.Command, details versionDetails) error { if _, ok := f.Annotations["experimental"]; ok && !hasExperimental { errs = append(errs, fmt.Sprintf("\"--%s\" is only supported on a Docker daemon with experimental features enabled", f.Name)) } + if _, ok := f.Annotations["experimentalCLI"]; ok && !hasExperimentalCLI { + errs = append(errs, fmt.Sprintf("\"--%s\" is only supported when experimental cli features are enabled", f.Name)) + } } }) if len(errs) > 0 { diff --git a/e2e/internal/fixtures/fixtures.go b/e2e/internal/fixtures/fixtures.go index e9a56da823..ce6518e384 100644 --- a/e2e/internal/fixtures/fixtures.go +++ b/e2e/internal/fixtures/fixtures.go @@ -32,7 +32,8 @@ func SetupConfigFile(t *testing.T) fs.Dir { "https://notary-server:4443": { "auth": "ZWlhaXM6cGFzc3dvcmQK" } - } + }, + "experimental": "enabled" } `)) return *dir