Merge pull request #19517 from calavera/validate_config_keys

Verify that the configuration keys in the file are valid.
This commit is contained in:
Phil Estes 2016-01-22 15:01:29 -05:00
commit 34a83f9f2c
8 changed files with 170 additions and 45 deletions

View File

@ -56,7 +56,6 @@ type CommonConfig struct {
GraphDriver string `json:"storage-driver,omitempty"`
GraphOptions []string `json:"storage-opts,omitempty"`
Labels []string `json:"labels,omitempty"`
LogConfig LogConfig `json:"log-config,omitempty"`
Mtu int `json:"mtu,omitempty"`
Pidfile string `json:"pidfile,omitempty"`
Root string `json:"graph,omitempty"`
@ -76,12 +75,16 @@ type CommonConfig struct {
// reachable by other hosts.
ClusterAdvertise string `json:"cluster-advertise,omitempty"`
Debug bool `json:"debug,omitempty"`
Hosts []string `json:"hosts,omitempty"`
LogLevel string `json:"log-level,omitempty"`
TLS bool `json:"tls,omitempty"`
TLSVerify bool `json:"tls-verify,omitempty"`
TLSOptions CommonTLSOptions `json:"tls-opts,omitempty"`
Debug bool `json:"debug,omitempty"`
Hosts []string `json:"hosts,omitempty"`
LogLevel string `json:"log-level,omitempty"`
TLS bool `json:"tls,omitempty"`
TLSVerify bool `json:"tlsverify,omitempty"`
// Embedded structs that allow config
// deserialization without the full struct.
CommonTLSOptions
LogConfig
reloadLock sync.Mutex
valuesSet map[string]interface{}
@ -215,16 +218,44 @@ 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 values that implement NamedOption.
// Their configuration name differs from their flag name, like `labels` and `label`.
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 +274,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, ", "))

View File

@ -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)
}
}

View File

@ -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
}

View File

@ -204,9 +204,9 @@ func (cli *DaemonCli) CmdDaemon(args ...string) error {
defaultHost := opts.DefaultHost
if cli.Config.TLS {
tlsOptions := tlsconfig.Options{
CAFile: cli.Config.TLSOptions.CAFile,
CertFile: cli.Config.TLSOptions.CertFile,
KeyFile: cli.Config.TLSOptions.KeyFile,
CAFile: cli.Config.CommonTLSOptions.CAFile,
CertFile: cli.Config.CommonTLSOptions.CertFile,
KeyFile: cli.Config.CommonTLSOptions.KeyFile,
}
if cli.Config.TLSVerify {
@ -338,12 +338,12 @@ func loadDaemonCliConfig(config *daemon.Config, daemonFlags *flag.FlagSet, commo
config.LogLevel = commonConfig.LogLevel
config.TLS = commonConfig.TLS
config.TLSVerify = commonConfig.TLSVerify
config.TLSOptions = daemon.CommonTLSOptions{}
config.CommonTLSOptions = daemon.CommonTLSOptions{}
if commonConfig.TLSOptions != nil {
config.TLSOptions.CAFile = commonConfig.TLSOptions.CAFile
config.TLSOptions.CertFile = commonConfig.TLSOptions.CertFile
config.TLSOptions.KeyFile = commonConfig.TLSOptions.KeyFile
config.CommonTLSOptions.CAFile = commonConfig.TLSOptions.CAFile
config.CommonTLSOptions.CertFile = commonConfig.TLSOptions.CertFile
config.CommonTLSOptions.KeyFile = commonConfig.TLSOptions.KeyFile
}
if configFile != "" {
@ -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
}

View File

@ -51,8 +51,8 @@ func TestLoadDaemonCliConfigWithTLS(t *testing.T) {
if loadedConfig == nil {
t.Fatalf("expected configuration %v, got nil", c)
}
if loadedConfig.TLSOptions.CAFile != "/tmp/ca.pem" {
t.Fatalf("expected /tmp/ca.pem, got %s: %q", loadedConfig.TLSOptions.CAFile, loadedConfig)
if loadedConfig.CommonTLSOptions.CAFile != "/tmp/ca.pem" {
t.Fatalf("expected /tmp/ca.pem, got %s: %q", loadedConfig.CommonTLSOptions.CAFile, loadedConfig)
}
}
@ -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,34 @@ func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) {
t.Fatalf("expected warn log level, got %v", logrus.GetLevel())
}
}
func TestLoadDaemonConfigWithEmbeddedOptions(t *testing.T) {
c := &daemon.Config{}
common := &cli.CommonFlags{}
flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
flags.String([]string{"-tlscacert"}, "", "")
flags.String([]string{"-log-driver"}, "", "")
f, err := ioutil.TempFile("", "docker-config-")
if err != nil {
t.Fatal(err)
}
configFile := f.Name()
f.Write([]byte(`{"tlscacert": "/etc/certs/ca.pem", "log-driver": "syslog"}`))
f.Close()
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.CommonTLSOptions.CAFile != "/etc/certs/ca.pem" {
t.Fatalf("expected CA file path /etc/certs/ca.pem, got %v", loadedConfig.CommonTLSOptions.CAFile)
}
if loadedConfig.LogConfig.Type != "syslog" {
t.Fatalf("expected LogConfig type syslog, got %v", loadedConfig.LogConfig.Type)
}
}

View File

@ -838,10 +838,8 @@ This is a full example of the allowed configuration options in the file:
"storage-driver": "",
"storage-opts": "",
"labels": [],
"log-config": {
"log-driver": "",
"log-opts": []
},
"log-driver": "",
"log-opts": [],
"mtu": 0,
"pidfile": "",
"graph": "",
@ -852,12 +850,10 @@ This is a full example of the allowed configuration options in the file:
"hosts": [],
"log-level": "",
"tls": true,
"tls-verify": true,
"tls-opts": {
"tlscacert": "",
"tlscert": "",
"tlskey": ""
},
"tlsverify": true,
"tlscacert": "",
"tlscert": "",
"tlskey": "",
"api-cors-headers": "",
"selinux-enabled": false,
"userns-remap": "",

View File

@ -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
}
}

View File

@ -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))
}
}