From cd3446972e968639684f2b65bfc11c099a25f1b0 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Tue, 19 Jan 2016 14:16:07 -0500 Subject: [PATCH] Fix post config verification without flags. - Set the daemon log level to what's set in the configuration. - Enable TLS when TLSVerify is enabled. Signed-off-by: David Calavera --- daemon/config.go | 38 ++++++++++--- docker/common.go | 24 ++++---- docker/daemon.go | 9 +++ docker/daemon_test.go | 124 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 19 deletions(-) diff --git a/daemon/config.go b/daemon/config.go index a75178faef..04931b24e9 100644 --- a/daemon/config.go +++ b/daemon/config.go @@ -84,6 +84,7 @@ type CommonConfig struct { TLSOptions CommonTLSOptions `json:"tls-opts,omitempty"` reloadLock sync.Mutex + valuesSet map[string]interface{} } // InstallCommonFlags adds command-line options to the top-level flag parser for @@ -112,6 +113,16 @@ func (config *Config) InstallCommonFlags(cmd *flag.FlagSet, usageFn func(string) cmd.Var(opts.NewNamedMapOpts("cluster-store-opts", config.ClusterOpts, nil), []string{"-cluster-store-opt"}, usageFn("Set cluster store options")) } +// IsValueSet returns true if a configuration value +// was explicitly set in the configuration file. +func (config *Config) IsValueSet(name string) bool { + if config.valuesSet == nil { + return false + } + _, ok := config.valuesSet[name] + return ok +} + func parseClusterAdvertiseSettings(clusterStore, clusterAdvertise string) (string, error) { if clusterAdvertise == "" { return "", errDiscoveryDisabled @@ -165,6 +176,7 @@ func getConflictFreeConfiguration(configFile string, flags *flag.FlagSet) (*Conf return nil, err } + var config Config var reader io.Reader if flags != nil { var jsonConfig map[string]interface{} @@ -173,22 +185,22 @@ func getConflictFreeConfiguration(configFile string, flags *flag.FlagSet) (*Conf return nil, err } - if err := findConfigurationConflicts(jsonConfig, flags); err != nil { + configSet := configValuesSet(jsonConfig) + + if err := findConfigurationConflicts(configSet, flags); err != nil { return nil, err } + + config.valuesSet = configSet } - var config Config reader = bytes.NewReader(b) err = json.NewDecoder(reader).Decode(&config) return &config, err } -// findConfigurationConflicts iterates over the provided flags searching for -// duplicated configurations. 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 +// configValuesSet returns the configuration values explicitly set in the file. +func configValuesSet(config map[string]interface{}) map[string]interface{} { flatten := make(map[string]interface{}) for k, v := range config { if m, ok := v.(map[string]interface{}); ok { @@ -199,6 +211,14 @@ func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagS flatten[k] = v } } + return flatten +} + +// findConfigurationConflicts iterates over the provided flags searching for +// duplicated configurations. 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 printConflict := func(name string, flagValue, fileValue interface{}) string { return fmt.Sprintf("%s: (from flag: %v, from file: %v)", name, flagValue, fileValue) @@ -207,7 +227,7 @@ func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagS collectConflicts := 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 := flatten[namedOption.Name()]; ok { + if optsValue, ok := config[namedOption.Name()]; ok { conflicts = append(conflicts, printConflict(namedOption.Name(), f.Value.String(), optsValue)) } } else { @@ -215,7 +235,7 @@ func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagS for _, name := range f.Names { name = strings.TrimLeft(name, "-") - if value, ok := flatten[name]; ok { + if value, ok := config[name]; ok { conflicts = append(conflicts, printConflict(name, f.Value.String(), value)) break } diff --git a/docker/common.go b/docker/common.go index 893de7109e..5652424ed2 100644 --- a/docker/common.go +++ b/docker/common.go @@ -55,16 +55,7 @@ func init() { func postParseCommon() { cmd := commonFlags.FlagSet - if commonFlags.LogLevel != "" { - lvl, err := logrus.ParseLevel(commonFlags.LogLevel) - if err != nil { - fmt.Fprintf(os.Stderr, "Unable to parse logging level: %s\n", commonFlags.LogLevel) - os.Exit(1) - } - logrus.SetLevel(lvl) - } else { - logrus.SetLevel(logrus.InfoLevel) - } + setDaemonLogLevel(commonFlags.LogLevel) // Regardless of whether the user sets it to true or false, if they // specify --tlsverify at all then we need to turn on tls @@ -93,3 +84,16 @@ func postParseCommon() { } } } + +func setDaemonLogLevel(logLevel string) { + if logLevel != "" { + lvl, err := logrus.ParseLevel(logLevel) + if err != nil { + fmt.Fprintf(os.Stderr, "Unable to parse logging level: %s\n", logLevel) + os.Exit(1) + } + logrus.SetLevel(lvl) + } else { + logrus.SetLevel(logrus.InfoLevel) + } +} diff --git a/docker/daemon.go b/docker/daemon.go index a8422122f7..ba4c9e41dd 100644 --- a/docker/daemon.go +++ b/docker/daemon.go @@ -360,5 +360,14 @@ 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") { + config.TLS = true + } + + // ensure that the log level is the one set after merging configurations + setDaemonLogLevel(config.LogLevel) + return config, nil } diff --git a/docker/daemon_test.go b/docker/daemon_test.go index bc519e7467..992172de4f 100644 --- a/docker/daemon_test.go +++ b/docker/daemon_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/Sirupsen/logrus" "github.com/docker/docker/cli" "github.com/docker/docker/daemon" "github.com/docker/docker/opts" @@ -89,3 +90,126 @@ func TestLoadDaemonCliConfigWithConflicts(t *testing.T) { t.Fatalf("expected labels conflict, got %v", err) } } + +func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) { + c := &daemon.Config{} + common := &cli.CommonFlags{ + TLSOptions: &tlsconfig.Options{ + CAFile: "/tmp/ca.pem", + }, + } + + f, err := ioutil.TempFile("", "docker-config-") + if err != nil { + t.Fatal(err) + } + + configFile := f.Name() + f.Write([]byte(`{"tls-verify": true}`)) + f.Close() + + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + 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.TLS { + t.Fatalf("expected TLS enabled, got %q", loadedConfig) + } +} + +func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) { + c := &daemon.Config{} + common := &cli.CommonFlags{ + TLSOptions: &tlsconfig.Options{ + CAFile: "/tmp/ca.pem", + }, + } + + f, err := ioutil.TempFile("", "docker-config-") + if err != nil { + t.Fatal(err) + } + + configFile := f.Name() + f.Write([]byte(`{"tls-verify": false}`)) + f.Close() + + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + 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.TLS { + t.Fatalf("expected TLS enabled, got %q", loadedConfig) + } +} + +func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) { + c := &daemon.Config{} + common := &cli.CommonFlags{ + TLSOptions: &tlsconfig.Options{ + CAFile: "/tmp/ca.pem", + }, + } + + f, err := ioutil.TempFile("", "docker-config-") + if err != nil { + t.Fatal(err) + } + + configFile := f.Name() + f.Write([]byte(`{}`)) + f.Close() + + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + 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.TLS { + t.Fatalf("expected TLS disabled, got %q", loadedConfig) + } +} + +func TestLoadDaemonCliConfigWithLogLevel(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(`{"log-level": "warn"}`)) + f.Close() + + flags := mflag.NewFlagSet("test", mflag.ContinueOnError) + 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.LogLevel != "warn" { + t.Fatalf("expected warn log level, got %v", loadedConfig.LogLevel) + } + + if logrus.GetLevel() != logrus.WarnLevel { + t.Fatalf("expected warn log level, got %v", logrus.GetLevel()) + } +}