cli/flags: merge CommonOptions into ClientOptions

CommonOptions was inherited from when the cli and daemon were in the same
repository, and some options would be shared between them. That's no longer
the case, and some options are even "incorrect" (for example, while the
daemon can be configured to run on multiple hosts, the CLI can only connect
with a single host / connection). This patch does not (yet) address that,
but merges the CommonOptions into the ClientOptions.

An alias is created for the old type, although it doesn't appear there's
any external consumers using the CommonOptions type (or its constructor).

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

cli/cobra.go

@@ -28,7 +28,7 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
 	flags := rootCmd.Flags()
 
 	flags.StringVar(&opts.ConfigDir, "config", config.Dir(), "Location of client config files")
-	opts.Common.InstallFlags(flags)
+	opts.InstallFlags(flags)
 
 	cobra.AddTemplateFunc("add", func(a, b int) int { return a + b })
 	cobra.AddTemplateFunc("hasAliases", hasAliases)
@@ -172,7 +172,7 @@ func (tcmd *TopLevelCommand) HandleGlobalFlags() (*cobra.Command, []string, erro
 
 // Initialize finalises global option parsing and initializes the docker client.
 func (tcmd *TopLevelCommand) Initialize(ops ...command.InitializeOpt) error {
-	tcmd.opts.Common.SetDefaultOptions(tcmd.flags)
+	tcmd.opts.SetDefaultOptions(tcmd.flags)
 	return tcmd.dockerCli.Initialize(tcmd.opts, ops...)
 }
 

cli/command/cli.go

@@ -203,13 +203,13 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
 			return err
 		}
 	}
-	cliflags.SetLogLevel(opts.Common.LogLevel)
+	cliflags.SetLogLevel(opts.LogLevel)
 
 	if opts.ConfigDir != "" {
 		config.SetDir(opts.ConfigDir)
 	}
 
-	if opts.Common.Debug {
+	if opts.Debug {
 		debug.Enable()
 	}
 
@@ -219,10 +219,10 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
 	cli.contextStore = &ContextStoreWithDefault{
 		Store: baseContextStore,
 		Resolver: func() (*DefaultContext, error) {
-			return ResolveDefaultContext(opts.Common, cli.contextStoreConfig)
+			return ResolveDefaultContext(opts, cli.contextStoreConfig)
 		},
 	}
-	cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore)
+	cli.currentContext, err = resolveContextName(opts, cli.configFile, cli.contextStore)
 	if err != nil {
 		return err
 	}
@@ -242,7 +242,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
 }
 
 // NewAPIClientFromFlags creates a new APIClient from command line flags
-func NewAPIClientFromFlags(opts *cliflags.CommonOptions, configFile *configfile.ConfigFile) (client.APIClient, error) {
+func NewAPIClientFromFlags(opts *cliflags.ClientOptions, configFile *configfile.ConfigFile) (client.APIClient, error) {
 	storeConfig := DefaultContextStoreConfig()
 	contextStore := &ContextStoreWithDefault{
 		Store: store.New(config.ContextStoreDir(), storeConfig),
@@ -288,7 +288,7 @@ func resolveDockerEndpoint(s store.Reader, contextName string) (docker.Endpoint,
 }
 
 // Resolve the Docker endpoint for the default context (based on config, env vars and CLI flags)
-func resolveDefaultDockerEndpoint(opts *cliflags.CommonOptions) (docker.Endpoint, error) {
+func resolveDefaultDockerEndpoint(opts *cliflags.ClientOptions) (docker.Endpoint, error) {
 	host, err := getServerHost(opts.Hosts, opts.TLSOptions)
 	if err != nil {
 		return docker.Endpoint{}, err
@@ -445,7 +445,7 @@ func UserAgent() string {
 // - if DOCKER_CONTEXT is set, use this value
 // - if Config file has a globally set "CurrentContext", use this value
 // - fallbacks to default HOST, uses TLS config from flags/env vars
-func resolveContextName(opts *cliflags.CommonOptions, config *configfile.ConfigFile, contextstore store.Reader) (string, error) {
+func resolveContextName(opts *cliflags.ClientOptions, config *configfile.ConfigFile, contextstore store.Reader) (string, error) {
 	if opts.Context != "" && len(opts.Hosts) > 0 {
 		return "", errors.New("Conflicting options: either specify --host or --context, not both")
 	}

cli/command/cli_test.go

@@ -31,7 +31,7 @@ func TestNewAPIClientFromFlags(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		host = "npipe://./"
 	}
-	opts := &flags.CommonOptions{Hosts: []string{host}}
+	opts := &flags.ClientOptions{Hosts: []string{host}}
 	apiClient, err := NewAPIClientFromFlags(opts, &configfile.ConfigFile{})
 	assert.NilError(t, err)
 	assert.Equal(t, apiClient.DaemonHost(), host)
@@ -44,7 +44,7 @@ func TestNewAPIClientFromFlagsForDefaultSchema(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		slug = "tcp://127.0.0.1"
 	}
-	opts := &flags.CommonOptions{Hosts: []string{host}}
+	opts := &flags.ClientOptions{Hosts: []string{host}}
 	apiClient, err := NewAPIClientFromFlags(opts, &configfile.ConfigFile{})
 	assert.NilError(t, err)
 	assert.Equal(t, apiClient.DaemonHost(), slug+host)
@@ -62,7 +62,7 @@ func TestNewAPIClientFromFlagsWithCustomHeaders(t *testing.T) {
 	}))
 	defer ts.Close()
 	host := strings.Replace(ts.URL, "http://", "tcp://", 1)
-	opts := &flags.CommonOptions{Hosts: []string{host}}
+	opts := &flags.ClientOptions{Hosts: []string{host}}
 	configFile := &configfile.ConfigFile{
 		HTTPHeaders: map[string]string{
 			"My-Header": "Custom-Value",
@@ -91,7 +91,7 @@ func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) {
 	t.Setenv("DOCKER_API_VERSION", customVersion)
 	t.Setenv("DOCKER_HOST", ":2375")
 
-	opts := &flags.CommonOptions{}
+	opts := &flags.ClientOptions{}
 	configFile := &configfile.ConfigFile{}
 	apiclient, err := NewAPIClientFromFlags(opts, configFile)
 	assert.NilError(t, err)
@@ -191,7 +191,7 @@ func TestInitializeFromClientHangs(t *testing.T) {
 	ts.Start()
 	defer ts.Close()
 
-	opts := &flags.CommonOptions{Hosts: []string{fmt.Sprintf("unix://%s", socket)}}
+	opts := &flags.ClientOptions{Hosts: []string{fmt.Sprintf("unix://%s", socket)}}
 	configFile := &configfile.ConfigFile{}
 	apiClient, err := NewAPIClientFromFlags(opts, configFile)
 	assert.NilError(t, err)

cli/command/defaultcontextstore.go

@@ -42,7 +42,7 @@ type EndpointDefaultResolver interface {
 }
 
 // ResolveDefaultContext creates a Metadata for the current CLI invocation parameters
-func ResolveDefaultContext(opts *cliflags.CommonOptions, config store.Config) (*DefaultContext, error) {
+func ResolveDefaultContext(opts *cliflags.ClientOptions, config store.Config) (*DefaultContext, error) {
 	contextTLSData := store.ContextTLSData{
 		Endpoints: make(map[string]store.EndpointTLSData),
 	}

cli/command/defaultcontextstore_test.go

@@ -56,7 +56,7 @@ func TestDefaultContextInitializer(t *testing.T) {
 	assert.NilError(t, err)
 	t.Setenv("DOCKER_HOST", "ssh://someswarmserver")
 	cli.configFile = &configfile.ConfigFile{}
-	ctx, err := ResolveDefaultContext(&cliflags.CommonOptions{
+	ctx, err := ResolveDefaultContext(&cliflags.ClientOptions{
 		TLS: true,
 		TLSOptions: &tlsconfig.Options{
 			CAFile: "./testdata/ca.pem",

cli/flags/client.go

@@ -1,12 +0,0 @@
-package flags
-
-// ClientOptions are the options used to configure the client cli
-type ClientOptions struct {
-	Common    *CommonOptions
-	ConfigDir string
-}
-
-// NewClientOptions returns a new ClientOptions
-func NewClientOptions() *ClientOptions {
-	return &ClientOptions{Common: NewCommonOptions()}
-}

cli/flags/options.go

@@ -43,8 +43,8 @@ var (
 	dockerTLS = os.Getenv("DOCKER_TLS") != ""
 )
 
-// CommonOptions are options common to both the client and the daemon.
+// ClientOptions are the options used to configure the client cli.
-type CommonOptions struct {
+type ClientOptions struct {
 	Debug      bool
 	Hosts      []string
 	LogLevel   string
@@ -52,59 +52,60 @@ type CommonOptions struct {
 	TLSVerify  bool
 	TLSOptions *tlsconfig.Options
 	Context    string
+	ConfigDir  string
 }
 
-// NewCommonOptions returns a new CommonOptions
+// NewClientOptions returns a new ClientOptions.
-func NewCommonOptions() *CommonOptions {
+func NewClientOptions() *ClientOptions {
-	return &CommonOptions{}
+	return &ClientOptions{}
 }
 
 // InstallFlags adds flags for the common options on the FlagSet
-func (commonOpts *CommonOptions) InstallFlags(flags *pflag.FlagSet) {
+func (o *ClientOptions) InstallFlags(flags *pflag.FlagSet) {
 	if dockerCertPath == "" {
 		dockerCertPath = config.Dir()
 	}
 
-	flags.BoolVarP(&commonOpts.Debug, "debug", "D", false, "Enable debug mode")
+	flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode")
-	flags.StringVarP(&commonOpts.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`)
+	flags.StringVarP(&o.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`)
-	flags.BoolVar(&commonOpts.TLS, "tls", dockerTLS, "Use TLS; implied by --tlsverify")
+	flags.BoolVar(&o.TLS, "tls", dockerTLS, "Use TLS; implied by --tlsverify")
-	flags.BoolVar(&commonOpts.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote")
+	flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote")
 
 	// TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file")
 
-	commonOpts.TLSOptions = &tlsconfig.Options{
+	o.TLSOptions = &tlsconfig.Options{
 		CAFile:   filepath.Join(dockerCertPath, DefaultCaFile),
 		CertFile: filepath.Join(dockerCertPath, DefaultCertFile),
 		KeyFile:  filepath.Join(dockerCertPath, DefaultKeyFile),
 	}
-	tlsOptions := commonOpts.TLSOptions
+	tlsOptions := o.TLSOptions
 	flags.Var(opts.NewQuotedString(&tlsOptions.CAFile), "tlscacert", "Trust certs signed only by this CA")
 	flags.Var(opts.NewQuotedString(&tlsOptions.CertFile), "tlscert", "Path to TLS certificate file")
 	flags.Var(opts.NewQuotedString(&tlsOptions.KeyFile), "tlskey", "Path to TLS key file")
 
 	// opts.ValidateHost is not used here, so as to allow connection helpers
-	hostOpt := opts.NewNamedListOptsRef("hosts", &commonOpts.Hosts, nil)
+	hostOpt := opts.NewNamedListOptsRef("hosts", &o.Hosts, nil)
 	flags.VarP(hostOpt, "host", "H", "Daemon socket(s) to connect to")
-	flags.StringVarP(&commonOpts.Context, "context", "c", "",
+	flags.StringVarP(&o.Context, "context", "c", "",
 		`Name of the context to use to connect to the daemon (overrides `+client.EnvOverrideHost+` env var and default context set with "docker context use")`)
 }
 
 // SetDefaultOptions sets default values for options after flag parsing is
 // complete
-func (commonOpts *CommonOptions) SetDefaultOptions(flags *pflag.FlagSet) {
+func (o *ClientOptions) SetDefaultOptions(flags *pflag.FlagSet) {
 	// Regardless of whether the user sets it to true or false, if they
 	// specify --tlsverify at all then we need to turn on TLS
 	// TLSVerify can be true even if not set due to DOCKER_TLS_VERIFY env var, so we need
 	// to check that here as well
-	if flags.Changed(FlagTLSVerify) || commonOpts.TLSVerify {
+	if flags.Changed(FlagTLSVerify) || o.TLSVerify {
-		commonOpts.TLS = true
+		o.TLS = true
 	}
 
-	if !commonOpts.TLS {
+	if !o.TLS {
-		commonOpts.TLSOptions = nil
+		o.TLSOptions = nil
 	} else {
-		tlsOptions := commonOpts.TLSOptions
+		tlsOptions := o.TLSOptions
-		tlsOptions.InsecureSkipVerify = !commonOpts.TLSVerify
+		tlsOptions.InsecureSkipVerify = !o.TLSVerify
 
 		// Reset CertFile and KeyFile to empty string if the user did not specify
 		// the respective flags and the respective default files were not found.

cli/flags/options_deprecated.go

@@ -0,0 +1,11 @@
+package flags
+
+// CommonOptions are options common to both the client and the daemon.
+//
+// Deprecated: use [ClientOptions].
+type CommonOptions = ClientOptions
+
+// NewCommonOptions returns a new CommonOptions
+//
+// Deprecated: use [NewClientOptions].
+var NewCommonOptions = NewClientOptions

cli/flags/options_test.go

@@ -10,9 +10,9 @@ import (
 	is "gotest.tools/v3/assert/cmp"
 )
 
-func TestCommonOptionsInstallFlags(t *testing.T) {
+func TestClientOptionsInstallFlags(t *testing.T) {
 	flags := pflag.NewFlagSet("testing", pflag.ContinueOnError)
-	opts := NewCommonOptions()
+	opts := NewClientOptions()
 	opts.InstallFlags(flags)
 
 	err := flags.Parse([]string{
@@ -30,9 +30,9 @@ func defaultPath(filename string) string {
 	return filepath.Join(config.Dir(), filename)
 }
 
-func TestCommonOptionsInstallFlagsWithDefaults(t *testing.T) {
+func TestClientOptionsInstallFlagsWithDefaults(t *testing.T) {
 	flags := pflag.NewFlagSet("testing", pflag.ContinueOnError)
-	opts := NewCommonOptions()
+	opts := NewClientOptions()
 	opts.InstallFlags(flags)
 
 	err := flags.Parse([]string{})

cmd/docker/builder_test.go

@@ -79,7 +79,7 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD
 					}))
 				}
 				opts := flags.NewClientOptions()
-				opts.Common.Context = tt.context
+				opts.Context = tt.context
 				assert.NilError(t, dockerCli.Initialize(opts))
 			}