diff --git a/daemon/config.go b/daemon/config.go index 04931b24e9..97818d98dc 100644 --- a/daemon/config.go +++ b/daemon/config.go @@ -80,7 +80,7 @@ type CommonConfig struct { Hosts []string `json:"hosts,omitempty"` LogLevel string `json:"log-level,omitempty"` TLS bool `json:"tls,omitempty"` - TLSVerify bool `json:"tls-verify,omitempty"` + TLSVerify bool `json:"tlsverify,omitempty"` TLSOptions CommonTLSOptions `json:"tls-opts,omitempty"` reloadLock sync.Mutex @@ -215,16 +215,43 @@ func configValuesSet(config map[string]interface{}) map[string]interface{} { } // findConfigurationConflicts iterates over the provided flags searching for -// duplicated configurations. It returns an error with all the conflicts if +// duplicated configurations and unknown keys. It returns an error with all the conflicts if // it finds any. func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagSet) error { - var conflicts []string + // 1. Search keys from the file that we don't recognize as flags. + unknownKeys := make(map[string]interface{}) + for key, value := range config { + flagName := "-" + key + if flag := flags.Lookup(flagName); flag == nil { + unknownKeys[key] = value + } + } + // 2. Discard keys that might have a given name, like `labels`. + unknownNamedConflicts := func(f *flag.Flag) { + if namedOption, ok := f.Value.(opts.NamedOption); ok { + if _, valid := unknownKeys[namedOption.Name()]; valid { + delete(unknownKeys, namedOption.Name()) + } + } + } + flags.VisitAll(unknownNamedConflicts) + + if len(unknownKeys) > 0 { + var unknown []string + for key := range unknownKeys { + unknown = append(unknown, key) + } + return fmt.Errorf("the following directives don't match any configuration option: %s", strings.Join(unknown, ", ")) + } + + var conflicts []string printConflict := func(name string, flagValue, fileValue interface{}) string { return fmt.Sprintf("%s: (from flag: %v, from file: %v)", name, flagValue, fileValue) } - collectConflicts := func(f *flag.Flag) { + // 3. Search keys that are present as a flag and as a file option. + duplicatedConflicts := func(f *flag.Flag) { // search option name in the json configuration payload if the value is a named option if namedOption, ok := f.Value.(opts.NamedOption); ok { if optsValue, ok := config[namedOption.Name()]; ok { @@ -243,7 +270,7 @@ func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagS } } - flags.Visit(collectConflicts) + flags.Visit(duplicatedConflicts) if len(conflicts) > 0 { return fmt.Errorf("the following directives are specified both as a flag and in the configuration file: %s", strings.Join(conflicts, ", ")) diff --git a/daemon/config_test.go b/daemon/config_test.go index 69a199e162..dc1c3bc7da 100644 --- a/daemon/config_test.go +++ b/daemon/config_test.go @@ -89,21 +89,16 @@ func TestFindConfigurationConflicts(t *testing.T) { config := map[string]interface{}{"authorization-plugins": "foobar"} flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + flags.String([]string{"-authorization-plugins"}, "", "") + if err := flags.Set("-authorization-plugins", "asdf"); err != nil { + t.Fatal(err) + } + err := findConfigurationConflicts(config, flags) - if err != nil { - t.Fatal(err) - } - - flags.String([]string{"authorization-plugins"}, "", "") - if err := flags.Set("authorization-plugins", "asdf"); err != nil { - t.Fatal(err) - } - - err = findConfigurationConflicts(config, flags) if err == nil { t.Fatal("expected error, got nil") } - if !strings.Contains(err.Error(), "authorization-plugins") { + if !strings.Contains(err.Error(), "authorization-plugins: (from flag: asdf, from file: foobar)") { t.Fatalf("expected authorization-plugins conflict, got %v", err) } } @@ -175,3 +170,41 @@ func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) { t.Fatalf("expected tlscacert conflict, got %v", err) } } + +func TestFindConfigurationConflictsWithUnknownKeys(t *testing.T) { + config := map[string]interface{}{"tls-verify": "true"} + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + + flags.Bool([]string{"-tlsverify"}, false, "") + err := findConfigurationConflicts(config, flags) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "the following directives don't match any configuration option: tls-verify") { + t.Fatalf("expected tls-verify conflict, got %v", err) + } +} + +func TestFindConfigurationConflictsWithMergedValues(t *testing.T) { + var hosts []string + config := map[string]interface{}{"hosts": "tcp://127.0.0.1:2345"} + base := mflag.NewFlagSet("base", mflag.ContinueOnError) + base.Var(opts.NewNamedListOptsRef("hosts", &hosts, nil), []string{"H", "-host"}, "") + + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + mflag.Merge(flags, base) + + err := findConfigurationConflicts(config, flags) + if err != nil { + t.Fatal(err) + } + + flags.Set("-host", "unix:///var/run/docker.sock") + err = findConfigurationConflicts(config, flags) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "hosts: (from flag: [unix:///var/run/docker.sock], from file: tcp://127.0.0.1:2345)") { + t.Fatalf("expected hosts conflict, got %v", err) + } +} diff --git a/docker/common.go b/docker/common.go index 5652424ed2..6028f79da0 100644 --- a/docker/common.go +++ b/docker/common.go @@ -18,6 +18,7 @@ const ( defaultCaFile = "ca.pem" defaultKeyFile = "key.pem" defaultCertFile = "cert.pem" + tlsVerifyKey = "tlsverify" ) var ( @@ -60,7 +61,7 @@ func postParseCommon() { // 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 cmd.IsSet("-tlsverify") || commonFlags.TLSVerify { + if cmd.IsSet("-"+tlsVerifyKey) || commonFlags.TLSVerify { commonFlags.TLS = true } diff --git a/docker/daemon.go b/docker/daemon.go index ba4c9e41dd..5e46dc249d 100644 --- a/docker/daemon.go +++ b/docker/daemon.go @@ -362,7 +362,7 @@ func loadDaemonCliConfig(config *daemon.Config, daemonFlags *flag.FlagSet, commo // Regardless of whether the user sets it to true or false, if they // specify TLSVerify at all then we need to turn on TLS - if config.IsValueSet("tls-verify") { + if config.IsValueSet(tlsVerifyKey) { config.TLS = true } diff --git a/docker/daemon_test.go b/docker/daemon_test.go index 992172de4f..11e4a2e181 100644 --- a/docker/daemon_test.go +++ b/docker/daemon_test.go @@ -105,10 +105,11 @@ func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) { } configFile := f.Name() - f.Write([]byte(`{"tls-verify": true}`)) + f.Write([]byte(`{"tlsverify": true}`)) f.Close() flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + flags.Bool([]string{"-tlsverify"}, false, "") loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) if err != nil { t.Fatal(err) @@ -136,10 +137,11 @@ func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) { } configFile := f.Name() - f.Write([]byte(`{"tls-verify": false}`)) + f.Write([]byte(`{"tlsverify": false}`)) f.Close() flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + flags.Bool([]string{"-tlsverify"}, false, "") loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) if err != nil { t.Fatal(err) @@ -198,6 +200,7 @@ func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) { f.Close() flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + flags.String([]string{"-log-level"}, "", "") loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) if err != nil { t.Fatal(err) @@ -213,3 +216,30 @@ func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) { t.Fatalf("expected warn log level, got %v", logrus.GetLevel()) } } + +func TestLoadDaemonCliConfigWithTLSOptions(t *testing.T) { + c := &daemon.Config{} + common := &cli.CommonFlags{} + + f, err := ioutil.TempFile("", "docker-config-") + if err != nil { + t.Fatal(err) + } + + configFile := f.Name() + f.Write([]byte(`{"tls-opts": {"tlscacert": "/etc/certs/ca.pem"}}`)) + f.Close() + + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + flags.String([]string{"-tlscacert"}, "", "") + loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) + if err != nil { + t.Fatal(err) + } + if loadedConfig == nil { + t.Fatalf("expected configuration %v, got nil", c) + } + if loadedConfig.TLSOptions.CAFile != "/etc/certs/ca.pem" { + t.Fatalf("expected CA file path /etc/certs/ca.pem, got %v", loadedConfig.TLSOptions.CAFile) + } +} diff --git a/docs/reference/commandline/daemon.md b/docs/reference/commandline/daemon.md index 856d913782..d2110b7183 100644 --- a/docs/reference/commandline/daemon.md +++ b/docs/reference/commandline/daemon.md @@ -852,7 +852,7 @@ This is a full example of the allowed configuration options in the file: "hosts": [], "log-level": "", "tls": true, - "tls-verify": true, + "tlsverify": true, "tls-opts": { "tlscacert": "", "tlscert": "", diff --git a/pkg/mflag/flag.go b/pkg/mflag/flag.go index 2ad299accd..e7fabe0a9e 100644 --- a/pkg/mflag/flag.go +++ b/pkg/mflag/flag.go @@ -1223,11 +1223,27 @@ func (v mergeVal) IsBoolFlag() bool { return false } +// Name returns the name of a mergeVal. +// If the original value had a name, return the original name, +// otherwise, return the key asinged to this mergeVal. +func (v mergeVal) Name() string { + type namedValue interface { + Name() string + } + if nVal, ok := v.Value.(namedValue); ok { + return nVal.Name() + } + return v.key +} + // Merge is an helper function that merges n FlagSets into a single dest FlagSet // In case of name collision between the flagsets it will apply // the destination FlagSet's errorHandling behavior. func Merge(dest *FlagSet, flagsets ...*FlagSet) error { for _, fset := range flagsets { + if fset.formal == nil { + continue + } for k, f := range fset.formal { if _, ok := dest.formal[k]; ok { var err error @@ -1249,6 +1265,9 @@ func Merge(dest *FlagSet, flagsets ...*FlagSet) error { } newF := *f newF.Value = mergeVal{f.Value, k, fset} + if dest.formal == nil { + dest.formal = make(map[string]*Flag) + } dest.formal[k] = &newF } } diff --git a/pkg/mflag/flag_test.go b/pkg/mflag/flag_test.go index c28deda896..138355546e 100644 --- a/pkg/mflag/flag_test.go +++ b/pkg/mflag/flag_test.go @@ -514,3 +514,14 @@ func TestSortFlags(t *testing.T) { t.Fatalf("NFlag (%d) != fs.NFlag() (%d) of elements visited", nflag, fs.NFlag()) } } + +func TestMergeFlags(t *testing.T) { + base := NewFlagSet("base", ContinueOnError) + base.String([]string{"f"}, "", "") + + fs := NewFlagSet("test", ContinueOnError) + Merge(fs, base) + if len(fs.formal) != 1 { + t.Fatalf("FlagCount (%d) != number (1) of elements merged", len(fs.formal)) + } +}