From 36441fc5f6248daeea52b77e962dc2190419c69e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 Nov 2022 13:09:35 +0100 Subject: [PATCH 1/3] cli: NewTopLevelCommand: don't use unnamed assignments This prevents unexpected bugs if fields are added/moved. Signed-off-by: Sebastiaan van Stijn --- cli/cobra.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cli/cobra.go b/cli/cobra.go index 7cf7e810d9..41622064c7 100644 --- a/cli/cobra.go +++ b/cli/cobra.go @@ -116,7 +116,13 @@ type TopLevelCommand struct { // NewTopLevelCommand returns a new TopLevelCommand object func NewTopLevelCommand(cmd *cobra.Command, dockerCli *command.DockerCli, opts *cliflags.ClientOptions, flags *pflag.FlagSet) *TopLevelCommand { - return &TopLevelCommand{cmd, dockerCli, opts, flags, os.Args[1:]} + return &TopLevelCommand{ + cmd: cmd, + dockerCli: dockerCli, + opts: opts, + flags: flags, + args: os.Args[1:], + } } // SetArgs sets the args (default os.Args[:1] used to invoke the command From 181769f18cbdb99debd7e769fad53c59671136dd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 Nov 2022 13:24:07 +0100 Subject: [PATCH 2/3] cli/command: remove DockerCli.loadConfigFile() This makes it more transparent what's happening. Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index b6fb6b5124..e32f8303c7 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -132,16 +132,13 @@ func ShowHelp(err io.Writer) func(*cobra.Command, []string) error { // ConfigFile returns the ConfigFile func (cli *DockerCli) ConfigFile() *configfile.ConfigFile { + // TODO(thaJeztah): when would this happen? Is this only in tests (where cli.Initialize() is not called first?) if cli.configFile == nil { - cli.loadConfigFile() + cli.configFile = config.LoadDefaultConfigFile(cli.err) } return cli.configFile } -func (cli *DockerCli) loadConfigFile() { - cli.configFile = config.LoadDefaultConfigFile(cli.err) -} - // ServerInfo returns the server version details for the host this client is // connected to func (cli *DockerCli) ServerInfo() ServerInfo { @@ -225,7 +222,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize return errors.New("conflicting options: either specify --host or --context, not both") } - cli.loadConfigFile() + cli.configFile = config.LoadDefaultConfigFile(cli.err) baseContextStore := store.New(config.ContextStoreDir(), cli.contextStoreConfig) cli.contextStore = &ContextStoreWithDefault{ From 60987b8d7a9afd5f1d97fcb071d4d5baeb47ee54 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 4 Nov 2022 13:40:46 +0100 Subject: [PATCH 3/3] cli/command: DockerCli: keep reference to options for later use Store a reference to the options in the client, so that they are available for later use. Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index e32f8303c7..0cce3fa505 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -69,6 +69,7 @@ type Cli interface { // Instances of the client can be returned from NewDockerCli. type DockerCli struct { configFile *configfile.ConfigFile + options *cliflags.ClientOptions in *streams.In out *streams.Out err io.Writer @@ -222,17 +223,16 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize return errors.New("conflicting options: either specify --host or --context, not both") } + cli.options = opts cli.configFile = config.LoadDefaultConfigFile(cli.err) - - baseContextStore := store.New(config.ContextStoreDir(), cli.contextStoreConfig) + cli.currentContext = resolveContextName(cli.options, cli.configFile) cli.contextStore = &ContextStoreWithDefault{ - Store: baseContextStore, + Store: store.New(config.ContextStoreDir(), cli.contextStoreConfig), Resolver: func() (*DefaultContext, error) { - return ResolveDefaultContext(opts, cli.contextStoreConfig) + return ResolveDefaultContext(cli.options, cli.contextStoreConfig) }, } - cli.currentContext = resolveContextName(opts, cli.configFile) - cli.dockerEndpoint, err = resolveDockerEndpoint(cli.contextStore, cli.currentContext) + cli.dockerEndpoint, err = resolveDockerEndpoint(cli.contextStore, resolveContextName(opts, cli.configFile)) if err != nil { return errors.Wrap(err, "unable to resolve docker endpoint") }