Merge pull request #3876 from thaJeztah/merge_common_options

cli/flags: merge CommonOptions into ClientOptions

Author: thaJeztah

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

@@ -43,68 +43,69 @@ var (
 	dockerTLS = os.Getenv("DOCKER_TLS") != ""
 )
 
-// CommonOptions are options common to both the client and the daemon.
-type CommonOptions struct {
+// ClientOptions are the options used to configure the client cli.
+type ClientOptions struct {
 	Debug      bool
 	Hosts      []string
 	LogLevel   string
 	TLS        bool
 	TLSVerify  bool
 	TLSOptions *tlsconfig.Options
 	Context    string
+	ConfigDir  string
 }
 
-// NewCommonOptions returns a new CommonOptions
-func NewCommonOptions() *CommonOptions {
-	return &CommonOptions{}
+// NewClientOptions returns a new ClientOptions.
+func NewClientOptions() *ClientOptions {
+	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.StringVarP(&commonOpts.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(&commonOpts.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote")
+	flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode")
+	flags.StringVarP(&o.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`)
+	flags.BoolVar(&o.TLS, "tls", dockerTLS, "Use TLS; implied by --tlsverify")
+	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 {
-		commonOpts.TLS = true
+	if flags.Changed(FlagTLSVerify) || o.TLSVerify {
+		o.TLS = true
 	}
 
-	if !commonOpts.TLS {
-		commonOpts.TLSOptions = nil
+	if !o.TLS {
+		o.TLSOptions = nil
 	} else {
-		tlsOptions := commonOpts.TLSOptions
-		tlsOptions.InsecureSkipVerify = !commonOpts.TLSVerify
+		tlsOptions := o.TLSOptions
+		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/common.go cli/flags/options.gocli/flags/common.go renamed to cli/flags/options.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_deprecated.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{})

cli/flags/common_test.go cli/flags/options_test.gocli/flags/common_test.go renamed to cli/flags/options_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))
 			}