Merge pull request #1654 from ijc/plugins-dial-stdio

cli-plugins: use system dial-stdio to contact the engine.

cli-plugins/manager/manager.go

@@ -12,6 +12,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// ReexecEnvvar is the name of an ennvar which is set to the command
+// used to originally invoke the docker CLI when executing a
+// plugin. Assuming $PATH and $CWD remain unchanged this should allow
+// the plugin to re-execute the original CLI.
+const ReexecEnvvar = "DOCKER_CLI_PLUGIN_ORIGINAL_CLI_COMMAND"
+
 // errPluginNotFound is the error returned when a plugin could not be found.
 type errPluginNotFound string
 
@@ -155,6 +161,9 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
 
+		cmd.Env = os.Environ()
+		cmd.Env = append(cmd.Env, ReexecEnvvar+"="+os.Args[0])
+
 		return cmd, nil
 	}
 	return nil, errPluginNotFound(name)

cli-plugins/plugin/plugin.go

@@ -9,7 +9,9 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli-plugins/manager"
 	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/connhelper"
 	cliflags "github.com/docker/cli/cli/flags"
+	"github.com/docker/docker/client"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -49,6 +51,7 @@ func Run(makeCmd func(command.Cli) *cobra.Command, meta manager.Metadata) {
 // own use of that hook will shadow anything we add to the top-level
 // command meaning the CLI is never Initialized.
 var options struct {
+	name         string
 	init, prerun sync.Once
 	opts         *cliflags.ClientOptions
 	flags        *pflag.FlagSet
@@ -71,13 +74,45 @@ func PersistentPreRunE(cmd *cobra.Command, args []string) error {
 		}
 		// flags must be the original top-level command flags, not cmd.Flags()
 		options.opts.Common.SetDefaultOptions(options.flags)
-		err = options.dockerCli.Initialize(options.opts)
+		err = options.dockerCli.Initialize(options.opts, withPluginClientConn(options.name))
 	})
 	return err
 }
 
+func withPluginClientConn(name string) command.InitializeOpt {
+	return command.WithInitializeClient(func(dockerCli *command.DockerCli) (client.APIClient, error) {
+		cmd := "docker"
+		if x := os.Getenv(manager.ReexecEnvvar); x != "" {
+			cmd = x
+		}
+		var flags []string
+
+		// Accumulate all the global arguments, that is those
+		// up to (but not including) the plugin's name. This
+		// ensures that `docker system dial-stdio` is
+		// evaluating the same set of `--config`, `--tls*` etc
+		// global options as the plugin was called with, which
+		// in turn is the same as what the original docker
+		// invocation was passed.
+		for _, a := range os.Args[1:] {
+			if a == name {
+				break
+			}
+			flags = append(flags, a)
+		}
+		flags = append(flags, "system", "dial-stdio")
+
+		helper, err := connhelper.GetCommandConnectionHelper(cmd, flags...)
+		if err != nil {
+			return nil, err
+		}
+
+		return client.NewClientWithOpts(client.WithDialContext(helper.Dialer))
+	})
+}
+
 func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) *cobra.Command {
-	name := plugin.Use
+	name := plugin.Name()
 	fullname := manager.NamePrefix + name
 
 	cmd := &cobra.Command{
@@ -101,6 +136,7 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta
 	cli.DisableFlagsInUseLine(cmd)
 
 	options.init.Do(func() {
+		options.name = name
 		options.opts = opts
 		options.flags = flags
 		options.dockerCli = dockerCli
@@ -115,6 +151,8 @@ func newMetadataSubcommand(plugin *cobra.Command, meta manager.Metadata) *cobra.
 	cmd := &cobra.Command{
 		Use:    manager.MetadataSubcommandName,
 		Hidden: true,
+		// Suppress the global/parent PersistentPreRunE, which needlessly initializes the client and tries to connect to the daemon.
+		PersistentPreRun: func(cmd *cobra.Command, args []string) {},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			enc := json.NewEncoder(os.Stdout)
 			enc.SetEscapeHTML(false)

cli/command/cli.go

@@ -175,9 +175,28 @@ func (cli *DockerCli) RegistryClient(allowInsecure bool) registryclient.Registry
 	return registryclient.NewRegistryClient(resolver, UserAgent(), allowInsecure)
 }
 
+// InitializeOpt is the type of the functional options passed to DockerCli.Initialize
+type InitializeOpt func(dockerCli *DockerCli) error
+
+// WithInitializeClient is passed to DockerCli.Initialize by callers who wish to set a particular API Client for use by the CLI.
+func WithInitializeClient(makeClient func(dockerCli *DockerCli) (client.APIClient, error)) InitializeOpt {
+	return func(dockerCli *DockerCli) error {
+		var err error
+		dockerCli.client, err = makeClient(dockerCli)
+		return err
+	}
+}
+
 // Initialize the dockerCli runs initialization that must happen after command
 // line flags are parsed.
-func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
+func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...InitializeOpt) error {
+	var err error
+
+	for _, o := range ops {
+		if err := o(cli); err != nil {
+			return err
+		}
+	}
 	cliflags.SetLogLevel(opts.Common.LogLevel)
 
 	if opts.ConfigDir != "" {
@@ -189,29 +208,31 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
 	}
 
 	cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err)
-	var err error
-	cli.contextStore = store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig)
-	cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore)
-	if err != nil {
-		return err
-	}
-	endpoint, err := resolveDockerEndpoint(cli.contextStore, cli.currentContext, opts.Common)
-	if err != nil {
-		return errors.Wrap(err, "unable to resolve docker endpoint")
-	}
-	cli.dockerEndpoint = endpoint
 
-	cli.client, err = newAPIClientFromEndpoint(endpoint, cli.configFile)
-	if tlsconfig.IsErrEncryptedKey(err) {
-		passRetriever := passphrase.PromptRetrieverWithInOut(cli.In(), cli.Out(), nil)
-		newClient := func(password string) (client.APIClient, error) {
-			endpoint.TLSPassword = password
-			return newAPIClientFromEndpoint(endpoint, cli.configFile)
+	if cli.client == nil {
+		cli.contextStore = store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig)
+		cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore)
+		if err != nil {
+			return err
+		}
+		endpoint, err := resolveDockerEndpoint(cli.contextStore, cli.currentContext, opts.Common)
+		if err != nil {
+			return errors.Wrap(err, "unable to resolve docker endpoint")
+		}
+		cli.dockerEndpoint = endpoint
+
+		cli.client, err = newAPIClientFromEndpoint(endpoint, cli.configFile)
+		if tlsconfig.IsErrEncryptedKey(err) {
+			passRetriever := passphrase.PromptRetrieverWithInOut(cli.In(), cli.Out(), nil)
+			newClient := func(password string) (client.APIClient, error) {
+				endpoint.TLSPassword = password
+				return newAPIClientFromEndpoint(endpoint, cli.configFile)
+			}
+			cli.client, err = getClientWithPassword(passRetriever, newClient)
+		}
+		if err != nil {
+			return err
 		}
-		cli.client, err = getClientWithPassword(passRetriever, newClient)
-	}
-	if err != nil {
-		return err
 	}
 	var experimentalValue string
 	// Environment variable always overrides configuration

cli/command/cli_test.go

@@ -175,6 +175,9 @@ func TestExperimentalCLI(t *testing.T) {
 			defer dir.Remove()
 			apiclient := &fakeClient{
 				version: defaultVersion,
+				pingFunc: func() (types.Ping, error) {
+					return types.Ping{Experimental: true, OSType: "linux", APIVersion: defaultVersion}, nil
+				},
 			}
 
 			cli := &DockerCli{client: apiclient, err: os.Stderr}

cli/connhelper/connhelper.go

@@ -53,6 +53,16 @@ func GetConnectionHelper(daemonURL string) (*ConnectionHelper, error) {
 	return nil, err
 }
 
+// GetCommandConnectionHelper returns a ConnectionHelp constructed from an arbitrary command.
+func GetCommandConnectionHelper(cmd string, flags ...string) (*ConnectionHelper, error) {
+	return &ConnectionHelper{
+		Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			return newCommandConn(ctx, cmd, flags...)
+		},
+		Host: "http://docker",
+	}, nil
+}
+
 func newCommandConn(ctx context.Context, cmd string, args ...string) (net.Conn, error) {
 	var (
 		c   commandConn

docs/extend/cli_plugins.md

@@ -75,6 +75,19 @@ A plugin is required to support all of the global options of the
 top-level CLI, i.e. those listed by `man docker 1` with the exception
 of `-v`.
 
+## Connecting to the docker engine
+
+For consistency plugins should prefer to dial the engine by using the
+`system dial-stdio` subcommand of the main Docker CLI binary.
+
+To facilitate this plugins will be executed with the
+`$DOCKER_CLI_PLUGIN_ORIGINAL_CLI_COMMAND` environment variable
+pointing back to the main Docker CLI binary.
+
+All global options (everything from after the binary name up to, but
+not including, the primary entry point subcommand name) should be
+passed back to the CLI.
+
 ## Installation
 
 Plugins distributed in packages for system wide installation on

e2e/cli-plugins/dial_test.go

@@ -0,0 +1,26 @@
+package cliplugins
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/docker/cli/cli-plugins/manager"
+	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
+	"gotest.tools/icmd"
+)
+
+func TestDialStdio(t *testing.T) {
+	// Run the helloworld plugin forcing /bin/true as the `system
+	// dial-stdio` target. It should be passed all arguments from
+	// before the `helloworld` arg, but not the --who=foo which
+	// follows. We observe this from the debug level logging from
+	// the connhelper stuff.
+	helloworld := filepath.Join(os.Getenv("DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS"), "docker-helloworld")
+	cmd := icmd.Command(helloworld, "--config=blah", "--tls", "--log-level", "debug", "helloworld", "--who=foo")
+	res := icmd.RunCmd(cmd, icmd.WithEnv(manager.ReexecEnvvar+"=/bin/true"))
+	res.Assert(t, icmd.Success)
+	assert.Assert(t, is.Contains(res.Stderr(), `msg="connhelper: starting /bin/true with [--config=blah --tls --log-level debug system dial-stdio]"`))
+	assert.Assert(t, is.Equal(res.Stdout(), "Hello foo!\n"))
+}