Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 42 additions & 13 deletions cli/command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -125,25 +131,42 @@ 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we should mention the configuration option here, currently it shows:

$ docker version
"Moby was here" is not valid, should be either enabled or disabled

Perhaps something like:

Invalid value for "experimentalCli": "Moby was here". Valid options are "enabled" and "disabled"

(better suggestions welcome)

}
}

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

cli.server = ServerInfo{
cli.serverInfo = ServerInfo{
HasExperimental: ping.Experimental,
OSType: ping.OSType,
}
Expand Down Expand Up @@ -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}
Expand Down
47 changes: 43 additions & 4 deletions cli/command/cli_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions cli/command/system/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions cli/command/trust/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions cli/config/configfile/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,15 @@ func dockerPreRun(opts *cliflags.ClientOptions) {

type versionDetails interface {
Client() client.APIClient
ClientInfo() command.ClientInfo
ServerInfo() command.ServerInfo
}

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
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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() {
Expand All @@ -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{}
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion e2e/internal/fixtures/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ func SetupConfigFile(t *testing.T) fs.Dir {
"https://notary-server:4443": {
"auth": "ZWlhaXM6cGFzc3dvcmQK"
}
}
},
"experimental": "enabled"
}
`))
return *dir
Expand Down