Merge pull request #1652 from ijc/plugins-config

Add a field to the config file for plugin use.

cli-plugins/examples/helloworld/main.go

@@ -41,12 +41,21 @@ func main() {
 			// the path where a plugin overrides this
 			// hook.
 			PersistentPreRunE: plugin.PersistentPreRunE,
-			Run: func(cmd *cobra.Command, args []string) {
+			RunE: func(cmd *cobra.Command, args []string) error {
+				if who == "" {
+					who, _ = dockerCli.ConfigFile().PluginConfig("helloworld", "who")
+				}
+				if who == "" {
+					who = "World"
+				}
+
 				fmt.Fprintf(dockerCli.Out(), "Hello %s!\n", who)
+				dockerCli.ConfigFile().SetPluginConfig("helloworld", "lastwho", who)
+				return dockerCli.ConfigFile().Save()
 			},
 		}
 		flags := cmd.Flags()
-		flags.StringVar(&who, "who", "World", "Who are we addressing?")
+		flags.StringVar(&who, "who", "", "Who are we addressing?")
 
 		cmd.AddCommand(goodbye, apiversion)
 		return cmd

cli/config/configfile/file.go

@@ -24,31 +24,32 @@ const (
 
 // ConfigFile ~/.docker/config.json file info
 type ConfigFile struct {
-	AuthConfigs          map[string]types.AuthConfig `json:"auths"`
+	AuthConfigs          map[string]types.AuthConfig  `json:"auths"`
-	HTTPHeaders          map[string]string           `json:"HttpHeaders,omitempty"`
+	HTTPHeaders          map[string]string            `json:"HttpHeaders,omitempty"`
-	PsFormat             string                      `json:"psFormat,omitempty"`
+	PsFormat             string                       `json:"psFormat,omitempty"`
-	ImagesFormat         string                      `json:"imagesFormat,omitempty"`
+	ImagesFormat         string                       `json:"imagesFormat,omitempty"`
-	NetworksFormat       string                      `json:"networksFormat,omitempty"`
+	NetworksFormat       string                       `json:"networksFormat,omitempty"`
-	PluginsFormat        string                      `json:"pluginsFormat,omitempty"`
+	PluginsFormat        string                       `json:"pluginsFormat,omitempty"`
-	VolumesFormat        string                      `json:"volumesFormat,omitempty"`
+	VolumesFormat        string                       `json:"volumesFormat,omitempty"`
-	StatsFormat          string                      `json:"statsFormat,omitempty"`
+	StatsFormat          string                       `json:"statsFormat,omitempty"`
-	DetachKeys           string                      `json:"detachKeys,omitempty"`
+	DetachKeys           string                       `json:"detachKeys,omitempty"`
-	CredentialsStore     string                      `json:"credsStore,omitempty"`
+	CredentialsStore     string                       `json:"credsStore,omitempty"`
-	CredentialHelpers    map[string]string           `json:"credHelpers,omitempty"`
+	CredentialHelpers    map[string]string            `json:"credHelpers,omitempty"`
-	Filename             string                      `json:"-"` // Note: for internal use only
+	Filename             string                       `json:"-"` // Note: for internal use only
-	ServiceInspectFormat string                      `json:"serviceInspectFormat,omitempty"`
+	ServiceInspectFormat string                       `json:"serviceInspectFormat,omitempty"`
-	ServicesFormat       string                      `json:"servicesFormat,omitempty"`
+	ServicesFormat       string                       `json:"servicesFormat,omitempty"`
-	TasksFormat          string                      `json:"tasksFormat,omitempty"`
+	TasksFormat          string                       `json:"tasksFormat,omitempty"`
-	SecretFormat         string                      `json:"secretFormat,omitempty"`
+	SecretFormat         string                       `json:"secretFormat,omitempty"`
-	ConfigFormat         string                      `json:"configFormat,omitempty"`
+	ConfigFormat         string                       `json:"configFormat,omitempty"`
-	NodesFormat          string                      `json:"nodesFormat,omitempty"`
+	NodesFormat          string                       `json:"nodesFormat,omitempty"`
-	PruneFilters         []string                    `json:"pruneFilters,omitempty"`
+	PruneFilters         []string                     `json:"pruneFilters,omitempty"`
-	Proxies              map[string]ProxyConfig      `json:"proxies,omitempty"`
+	Proxies              map[string]ProxyConfig       `json:"proxies,omitempty"`
-	Experimental         string                      `json:"experimental,omitempty"`
+	Experimental         string                       `json:"experimental,omitempty"`
-	StackOrchestrator    string                      `json:"stackOrchestrator,omitempty"`
+	StackOrchestrator    string                       `json:"stackOrchestrator,omitempty"`
-	Kubernetes           *KubernetesConfig           `json:"kubernetes,omitempty"`
+	Kubernetes           *KubernetesConfig            `json:"kubernetes,omitempty"`
-	CurrentContext       string                      `json:"currentContext,omitempty"`
+	CurrentContext       string                       `json:"currentContext,omitempty"`
-	CLIPluginsExtraDirs  []string                    `json:"cliPluginsExtraDirs,omitempty"`
+	CLIPluginsExtraDirs  []string                     `json:"cliPluginsExtraDirs,omitempty"`
+	Plugins              map[string]map[string]string `json:"plugins,omitempty"`
 }
 
 // ProxyConfig contains proxy configuration settings
@@ -70,6 +71,7 @@ func New(fn string) *ConfigFile {
 		AuthConfigs: make(map[string]types.AuthConfig),
 		HTTPHeaders: make(map[string]string),
 		Filename:    fn,
+		Plugins:     make(map[string]map[string]string),
 	}
 }
 
@@ -330,6 +332,42 @@ func (configFile *ConfigFile) GetFilename() string {
 	return configFile.Filename
 }
 
+// PluginConfig retrieves the requested option for the given plugin.
+func (configFile *ConfigFile) PluginConfig(pluginname, option string) (string, bool) {
+	if configFile.Plugins == nil {
+		return "", false
+	}
+	pluginConfig, ok := configFile.Plugins[pluginname]
+	if !ok {
+		return "", false
+	}
+	value, ok := pluginConfig[option]
+	return value, ok
+}
+
+// SetPluginConfig sets the option to the given value for the given
+// plugin. Passing a value of "" will remove the option. If removing
+// the final config item for a given plugin then also cleans up the
+// overall plugin entry.
+func (configFile *ConfigFile) SetPluginConfig(pluginname, option, value string) {
+	if configFile.Plugins == nil {
+		configFile.Plugins = make(map[string]map[string]string)
+	}
+	pluginConfig, ok := configFile.Plugins[pluginname]
+	if !ok {
+		pluginConfig = make(map[string]string)
+		configFile.Plugins[pluginname] = pluginConfig
+	}
+	if value != "" {
+		pluginConfig[option] = value
+	} else {
+		delete(pluginConfig, option)
+	}
+	if len(pluginConfig) == 0 {
+		delete(configFile.Plugins, pluginname)
+	}
+}
+
 func checkKubernetesConfiguration(kubeConfig *KubernetesConfig) error {
 	if kubeConfig == nil {
 		return nil

cli/config/configfile/file_test.go

@@ -1,6 +1,7 @@
 package configfile
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -9,6 +10,7 @@ import (
 	"github.com/docker/cli/cli/config/types"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
+	"gotest.tools/golden"
 )
 
 func TestEncodeAuth(t *testing.T) {
@@ -429,3 +431,68 @@ func TestSave(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(string(cfg), "{\n	\"auths\": {}\n}"))
 }
+
+func TestPluginConfig(t *testing.T) {
+	configFile := New("test-plugin")
+	defer os.Remove("test-plugin")
+
+	// Populate some initial values
+	configFile.SetPluginConfig("plugin1", "data1", "some string")
+	configFile.SetPluginConfig("plugin1", "data2", "42")
+	configFile.SetPluginConfig("plugin2", "data3", "some other string")
+
+	// Save a config file with some plugin config
+	err := configFile.Save()
+	assert.NilError(t, err)
+
+	// Read it back and check it has the expected content
+	cfg, err := ioutil.ReadFile("test-plugin")
+	assert.NilError(t, err)
+	golden.Assert(t, string(cfg), "plugin-config.golden")
+
+	// Load it, resave and check again that the content is
+	// preserved through a load/save cycle.
+	configFile = New("test-plugin2")
+	defer os.Remove("test-plugin2")
+	assert.NilError(t, configFile.LoadFromReader(bytes.NewReader(cfg)))
+	err = configFile.Save()
+	assert.NilError(t, err)
+	cfg, err = ioutil.ReadFile("test-plugin2")
+	assert.NilError(t, err)
+	golden.Assert(t, string(cfg), "plugin-config.golden")
+
+	// Check that the contents was reloaded properly
+	v, ok := configFile.PluginConfig("plugin1", "data1")
+	assert.Assert(t, ok)
+	assert.Equal(t, v, "some string")
+	v, ok = configFile.PluginConfig("plugin1", "data2")
+	assert.Assert(t, ok)
+	assert.Equal(t, v, "42")
+	v, ok = configFile.PluginConfig("plugin1", "data3")
+	assert.Assert(t, !ok)
+	assert.Equal(t, v, "")
+	v, ok = configFile.PluginConfig("plugin2", "data3")
+	assert.Assert(t, ok)
+	assert.Equal(t, v, "some other string")
+	v, ok = configFile.PluginConfig("plugin2", "data4")
+	assert.Assert(t, !ok)
+	assert.Equal(t, v, "")
+	v, ok = configFile.PluginConfig("plugin3", "data5")
+	assert.Assert(t, !ok)
+	assert.Equal(t, v, "")
+
+	// Add, remove and modify
+	configFile.SetPluginConfig("plugin1", "data1", "some replacement string") // replacing a key
+	configFile.SetPluginConfig("plugin1", "data2", "")                        // deleting a key
+	configFile.SetPluginConfig("plugin1", "data3", "some additional string")  // new key
+	configFile.SetPluginConfig("plugin2", "data3", "")                        // delete the whole plugin, since this was the only data
+	configFile.SetPluginConfig("plugin3", "data5", "a new plugin")            // add a new plugin
+
+	err = configFile.Save()
+	assert.NilError(t, err)
+
+	// Read it back and check it has the expected content again
+	cfg, err = ioutil.ReadFile("test-plugin2")
+	assert.NilError(t, err)
+	golden.Assert(t, string(cfg), "plugin-config-2.golden")
+}

cli/config/configfile/testdata/plugin-config-2.golden

@@ -0,0 +1,12 @@
+{
+	"auths": {},
+	"plugins": {
+		"plugin1": {
+			"data1": "some replacement string",
+			"data3": "some additional string"
+		},
+		"plugin3": {
+			"data5": "a new plugin"
+		}
+	}
+}

cli/config/configfile/testdata/plugin-config.golden

@@ -0,0 +1,12 @@
+{
+	"auths": {},
+	"plugins": {
+		"plugin1": {
+			"data1": "some string",
+			"data2": "42"
+		},
+		"plugin2": {
+			"data3": "some other string"
+		}
+	}
+}

docs/extend/cli_plugins.md

@@ -75,6 +75,18 @@ 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`.
 
+## Configuration
+
+Plugins are expected to make use of existing global configuration
+where it makes sense and likewise to consider extending the global
+configuration (by patching `docker/cli` to add new fields) where that
+is sensible.
+
+Where plugins unavoidably require specific configuration the
+`.plugins.«name»` key in the global `config.json` is reserved for
+their use. However the preference should be for shared/global
+configuration whenever that makes sense.
+
 ## Connecting to the docker engine
 
 For consistency plugins should prefer to dial the engine by using the

docs/reference/commandline/cli.md

@@ -223,6 +223,10 @@ Users can override your custom or the default key sequence on a per-container
 basis. To do this, the user specifies the `--detach-keys` flag with the `docker
 attach`, `docker exec`, `docker run` or `docker start` command.
 
+The property `plugins` contains settings specific to CLI plugins. The
+key is the plugin name, while the value is a further map of options,
+which are specific to that plugin.
+
 Following is a sample `config.json` file:
 
 ```json
@@ -246,7 +250,16 @@ Following is a sample `config.json` file:
     "awesomereg.example.org": "hip-star",
     "unicorn.example.com": "vcbait"
   },
-  "stackOrchestrator": "kubernetes"
+  "stackOrchestrator": "kubernetes",
+  "plugins": {
+    "plugin1": {
+      "option": "value"
+    },
+    "plugin2": {
+      "anotheroption": "anothervalue",
+      "athirdoption": "athirdvalue"
+    }
+  }
 }
 {% endraw %}
 ```

e2e/cli-plugins/config_test.go

@@ -0,0 +1,34 @@
+package cliplugins
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/docker/cli/cli/config"
+	"gotest.tools/assert"
+	"gotest.tools/icmd"
+)
+
+func TestConfig(t *testing.T) {
+	run, cfg, cleanup := prepare(t)
+	defer cleanup()
+
+	cfg.SetPluginConfig("helloworld", "who", "Cambridge")
+	err := cfg.Save()
+	assert.NilError(t, err)
+
+	res := icmd.RunCmd(run("helloworld"))
+	res.Assert(t, icmd.Expected{
+		ExitCode: 0,
+		Out:      "Hello Cambridge!",
+	})
+
+	cfg2, err := config.Load(filepath.Dir(cfg.GetFilename()))
+	assert.NilError(t, err)
+	assert.DeepEqual(t, cfg2.Plugins, map[string]map[string]string{
+		"helloworld": {
+			"who":     "Cambridge",
+			"lastwho": "Cambridge",
+		},
+	})
+}

e2e/cli-plugins/help_test.go

@@ -13,7 +13,7 @@ import (
 
 // TestGlobalHelp ensures correct behaviour when running `docker help`
 func TestGlobalHelp(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("help"))

e2e/cli-plugins/run_test.go

@@ -11,7 +11,7 @@ import (
 
 // TestRunNonexisting ensures correct behaviour when running a nonexistent plugin.
 func TestRunNonexisting(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("nonexistent"))
@@ -24,7 +24,7 @@ func TestRunNonexisting(t *testing.T) {
 
 // TestHelpNonexisting ensures correct behaviour when invoking help on a nonexistent plugin.
 func TestHelpNonexisting(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("help", "nonexistent"))
@@ -38,7 +38,7 @@ func TestHelpNonexisting(t *testing.T) {
 // TestNonexistingHelp ensures correct behaviour when invoking a
 // nonexistent plugin with `--help`.
 func TestNonexistingHelp(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("nonexistent", "--help"))
@@ -53,7 +53,7 @@ func TestNonexistingHelp(t *testing.T) {
 
 // TestRunBad ensures correct behaviour when running an existent but invalid plugin
 func TestRunBad(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("badmeta"))
@@ -66,7 +66,7 @@ func TestRunBad(t *testing.T) {
 
 // TestHelpBad ensures correct behaviour when invoking help on a existent but invalid plugin.
 func TestHelpBad(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("help", "badmeta"))
@@ -80,7 +80,7 @@ func TestHelpBad(t *testing.T) {
 // TestBadHelp ensures correct behaviour when invoking an
 // existent but invalid plugin with `--help`.
 func TestBadHelp(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("badmeta", "--help"))
@@ -95,7 +95,7 @@ func TestBadHelp(t *testing.T) {
 
 // TestRunGood ensures correct behaviour when running a valid plugin
 func TestRunGood(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("helloworld"))
@@ -109,7 +109,7 @@ func TestRunGood(t *testing.T) {
 // valid plugin. A global argument is included to ensure it does not
 // interfere.
 func TestHelpGood(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("-D", "help", "helloworld"))
@@ -122,7 +122,7 @@ func TestHelpGood(t *testing.T) {
 // with `--help`. A global argument is used to ensure it does not
 // interfere.
 func TestGoodHelp(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("-D", "helloworld", "--help"))
@@ -134,7 +134,7 @@ func TestGoodHelp(t *testing.T) {
 
 // TestRunGoodSubcommand ensures correct behaviour when running a valid plugin with a subcommand
 func TestRunGoodSubcommand(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("helloworld", "goodbye"))
@@ -146,7 +146,7 @@ func TestRunGoodSubcommand(t *testing.T) {
 
 // TestRunGoodArgument ensures correct behaviour when running a valid plugin with an `--argument`.
 func TestRunGoodArgument(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("helloworld", "--who", "Cleveland"))
@@ -160,7 +160,7 @@ func TestRunGoodArgument(t *testing.T) {
 // valid plugin subcommand. A global argument is included to ensure it does not
 // interfere.
 func TestHelpGoodSubcommand(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("-D", "help", "helloworld", "goodbye"))
@@ -173,7 +173,7 @@ func TestHelpGoodSubcommand(t *testing.T) {
 // with a subcommand and `--help`. A global argument is used to ensure it does not
 // interfere.
 func TestGoodSubcommandHelp(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("-D", "helloworld", "goodbye", "--help"))
@@ -186,7 +186,7 @@ func TestGoodSubcommandHelp(t *testing.T) {
 // TestCliInitialized tests the code paths which ensure that the Cli
 // object is initialized even if the plugin uses PersistentRunE
 func TestCliInitialized(t *testing.T) {
-	run, cleanup := prepare(t)
+	run, _, cleanup := prepare(t)
 	defer cleanup()
 
 	res := icmd.RunCmd(run("helloworld", "apiversion"))

e2e/cli-plugins/testdata/docker-help-helloworld.golden

@@ -4,7 +4,7 @@ Usage:	docker helloworld [OPTIONS] COMMAND
 A basic Hello World plugin for tests
 
 Options:
-      --who string   Who are we addressing? (default "World")
+      --who string   Who are we addressing?
 
 Commands:
   apiversion  Print the API version of the server

e2e/cli-plugins/util_test.go

@@ -5,11 +5,14 @@ import (
 	"os"
 	"testing"
 
+	"github.com/docker/cli/cli/config"
+	"github.com/docker/cli/cli/config/configfile"
+	"gotest.tools/assert"
 	"gotest.tools/fs"
 	"gotest.tools/icmd"
 )
 
-func prepare(t *testing.T) (func(args ...string) icmd.Cmd, func()) {
+func prepare(t *testing.T) (func(args ...string) icmd.Cmd, *configfile.ConfigFile, func()) {
 	cfg := fs.NewDir(t, "plugin-test",
 		fs.WithFile("config.json", fmt.Sprintf(`{"cliPluginsExtraDirs": [%q]}`, os.Getenv("DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS"))),
 	)
@@ -19,6 +22,9 @@ func prepare(t *testing.T) (func(args ...string) icmd.Cmd, func()) {
 	cleanup := func() {
 		cfg.Remove()
 	}
-	return run, cleanup
+	cfgfile, err := config.Load(cfg.Path())
+	assert.NilError(t, err)
+
+	return run, cfgfile, cleanup
 
 }