- Return an error if any of the keys don't match valid flags.
- Fix an issue ignoring merged values as named values.
- Fix tlsverify configuration key.
- Fix bug in mflag to avoid panics when one of the flag set doesn't have any flag.
Signed-off-by: David Calavera <david.calavera@gmail.com>
| ... | ... |
@@ -80,7 +80,7 @@ type CommonConfig struct {
|
| 80 | 80 |
Hosts []string `json:"hosts,omitempty"` |
| 81 | 81 |
LogLevel string `json:"log-level,omitempty"` |
| 82 | 82 |
TLS bool `json:"tls,omitempty"` |
| 83 |
- TLSVerify bool `json:"tls-verify,omitempty"` |
|
| 83 |
+ TLSVerify bool `json:"tlsverify,omitempty"` |
|
| 84 | 84 |
TLSOptions CommonTLSOptions `json:"tls-opts,omitempty"` |
| 85 | 85 |
|
| 86 | 86 |
reloadLock sync.Mutex |
| ... | ... |
@@ -215,16 +215,43 @@ func configValuesSet(config map[string]interface{}) map[string]interface{} {
|
| 215 | 215 |
} |
| 216 | 216 |
|
| 217 | 217 |
// findConfigurationConflicts iterates over the provided flags searching for |
| 218 |
-// duplicated configurations. It returns an error with all the conflicts if |
|
| 218 |
+// duplicated configurations and unknown keys. It returns an error with all the conflicts if |
|
| 219 | 219 |
// it finds any. |
| 220 | 220 |
func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagSet) error {
|
| 221 |
- var conflicts []string |
|
| 221 |
+ // 1. Search keys from the file that we don't recognize as flags. |
|
| 222 |
+ unknownKeys := make(map[string]interface{})
|
|
| 223 |
+ for key, value := range config {
|
|
| 224 |
+ flagName := "-" + key |
|
| 225 |
+ if flag := flags.Lookup(flagName); flag == nil {
|
|
| 226 |
+ unknownKeys[key] = value |
|
| 227 |
+ } |
|
| 228 |
+ } |
|
| 229 |
+ |
|
| 230 |
+ // 2. Discard keys that might have a given name, like `labels`. |
|
| 231 |
+ unknownNamedConflicts := func(f *flag.Flag) {
|
|
| 232 |
+ if namedOption, ok := f.Value.(opts.NamedOption); ok {
|
|
| 233 |
+ if _, valid := unknownKeys[namedOption.Name()]; valid {
|
|
| 234 |
+ delete(unknownKeys, namedOption.Name()) |
|
| 235 |
+ } |
|
| 236 |
+ } |
|
| 237 |
+ } |
|
| 238 |
+ flags.VisitAll(unknownNamedConflicts) |
|
| 239 |
+ |
|
| 240 |
+ if len(unknownKeys) > 0 {
|
|
| 241 |
+ var unknown []string |
|
| 242 |
+ for key := range unknownKeys {
|
|
| 243 |
+ unknown = append(unknown, key) |
|
| 244 |
+ } |
|
| 245 |
+ return fmt.Errorf("the following directives don't match any configuration option: %s", strings.Join(unknown, ", "))
|
|
| 246 |
+ } |
|
| 222 | 247 |
|
| 248 |
+ var conflicts []string |
|
| 223 | 249 |
printConflict := func(name string, flagValue, fileValue interface{}) string {
|
| 224 | 250 |
return fmt.Sprintf("%s: (from flag: %v, from file: %v)", name, flagValue, fileValue)
|
| 225 | 251 |
} |
| 226 | 252 |
|
| 227 |
- collectConflicts := func(f *flag.Flag) {
|
|
| 253 |
+ // 3. Search keys that are present as a flag and as a file option. |
|
| 254 |
+ duplicatedConflicts := func(f *flag.Flag) {
|
|
| 228 | 255 |
// search option name in the json configuration payload if the value is a named option |
| 229 | 256 |
if namedOption, ok := f.Value.(opts.NamedOption); ok {
|
| 230 | 257 |
if optsValue, ok := config[namedOption.Name()]; ok {
|
| ... | ... |
@@ -243,7 +270,7 @@ func findConfigurationConflicts(config map[string]interface{}, flags *flag.FlagS
|
| 243 | 243 |
} |
| 244 | 244 |
} |
| 245 | 245 |
|
| 246 |
- flags.Visit(collectConflicts) |
|
| 246 |
+ flags.Visit(duplicatedConflicts) |
|
| 247 | 247 |
|
| 248 | 248 |
if len(conflicts) > 0 {
|
| 249 | 249 |
return fmt.Errorf("the following directives are specified both as a flag and in the configuration file: %s", strings.Join(conflicts, ", "))
|
| ... | ... |
@@ -89,21 +89,16 @@ func TestFindConfigurationConflicts(t *testing.T) {
|
| 89 | 89 |
config := map[string]interface{}{"authorization-plugins": "foobar"}
|
| 90 | 90 |
flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
| 91 | 91 |
|
| 92 |
- err := findConfigurationConflicts(config, flags) |
|
| 93 |
- if err != nil {
|
|
| 94 |
- t.Fatal(err) |
|
| 95 |
- } |
|
| 96 |
- |
|
| 97 |
- flags.String([]string{"authorization-plugins"}, "", "")
|
|
| 98 |
- if err := flags.Set("authorization-plugins", "asdf"); err != nil {
|
|
| 92 |
+ flags.String([]string{"-authorization-plugins"}, "", "")
|
|
| 93 |
+ if err := flags.Set("-authorization-plugins", "asdf"); err != nil {
|
|
| 99 | 94 |
t.Fatal(err) |
| 100 | 95 |
} |
| 101 | 96 |
|
| 102 |
- err = findConfigurationConflicts(config, flags) |
|
| 97 |
+ err := findConfigurationConflicts(config, flags) |
|
| 103 | 98 |
if err == nil {
|
| 104 | 99 |
t.Fatal("expected error, got nil")
|
| 105 | 100 |
} |
| 106 |
- if !strings.Contains(err.Error(), "authorization-plugins") {
|
|
| 101 |
+ if !strings.Contains(err.Error(), "authorization-plugins: (from flag: asdf, from file: foobar)") {
|
|
| 107 | 102 |
t.Fatalf("expected authorization-plugins conflict, got %v", err)
|
| 108 | 103 |
} |
| 109 | 104 |
} |
| ... | ... |
@@ -175,3 +170,41 @@ func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) {
|
| 175 | 175 |
t.Fatalf("expected tlscacert conflict, got %v", err)
|
| 176 | 176 |
} |
| 177 | 177 |
} |
| 178 |
+ |
|
| 179 |
+func TestFindConfigurationConflictsWithUnknownKeys(t *testing.T) {
|
|
| 180 |
+ config := map[string]interface{}{"tls-verify": "true"}
|
|
| 181 |
+ flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
|
| 182 |
+ |
|
| 183 |
+ flags.Bool([]string{"-tlsverify"}, false, "")
|
|
| 184 |
+ err := findConfigurationConflicts(config, flags) |
|
| 185 |
+ if err == nil {
|
|
| 186 |
+ t.Fatal("expected error, got nil")
|
|
| 187 |
+ } |
|
| 188 |
+ if !strings.Contains(err.Error(), "the following directives don't match any configuration option: tls-verify") {
|
|
| 189 |
+ t.Fatalf("expected tls-verify conflict, got %v", err)
|
|
| 190 |
+ } |
|
| 191 |
+} |
|
| 192 |
+ |
|
| 193 |
+func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
|
|
| 194 |
+ var hosts []string |
|
| 195 |
+ config := map[string]interface{}{"hosts": "tcp://127.0.0.1:2345"}
|
|
| 196 |
+ base := mflag.NewFlagSet("base", mflag.ContinueOnError)
|
|
| 197 |
+ base.Var(opts.NewNamedListOptsRef("hosts", &hosts, nil), []string{"H", "-host"}, "")
|
|
| 198 |
+ |
|
| 199 |
+ flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
|
| 200 |
+ mflag.Merge(flags, base) |
|
| 201 |
+ |
|
| 202 |
+ err := findConfigurationConflicts(config, flags) |
|
| 203 |
+ if err != nil {
|
|
| 204 |
+ t.Fatal(err) |
|
| 205 |
+ } |
|
| 206 |
+ |
|
| 207 |
+ flags.Set("-host", "unix:///var/run/docker.sock")
|
|
| 208 |
+ err = findConfigurationConflicts(config, flags) |
|
| 209 |
+ if err == nil {
|
|
| 210 |
+ t.Fatal("expected error, got nil")
|
|
| 211 |
+ } |
|
| 212 |
+ if !strings.Contains(err.Error(), "hosts: (from flag: [unix:///var/run/docker.sock], from file: tcp://127.0.0.1:2345)") {
|
|
| 213 |
+ t.Fatalf("expected hosts conflict, got %v", err)
|
|
| 214 |
+ } |
|
| 215 |
+} |
| ... | ... |
@@ -18,6 +18,7 @@ const ( |
| 18 | 18 |
defaultCaFile = "ca.pem" |
| 19 | 19 |
defaultKeyFile = "key.pem" |
| 20 | 20 |
defaultCertFile = "cert.pem" |
| 21 |
+ tlsVerifyKey = "tlsverify" |
|
| 21 | 22 |
) |
| 22 | 23 |
|
| 23 | 24 |
var ( |
| ... | ... |
@@ -60,7 +61,7 @@ func postParseCommon() {
|
| 60 | 60 |
// Regardless of whether the user sets it to true or false, if they |
| 61 | 61 |
// specify --tlsverify at all then we need to turn on tls |
| 62 | 62 |
// TLSVerify can be true even if not set due to DOCKER_TLS_VERIFY env var, so we need to check that here as well |
| 63 |
- if cmd.IsSet("-tlsverify") || commonFlags.TLSVerify {
|
|
| 63 |
+ if cmd.IsSet("-"+tlsVerifyKey) || commonFlags.TLSVerify {
|
|
| 64 | 64 |
commonFlags.TLS = true |
| 65 | 65 |
} |
| 66 | 66 |
|
| ... | ... |
@@ -362,7 +362,7 @@ func loadDaemonCliConfig(config *daemon.Config, daemonFlags *flag.FlagSet, commo |
| 362 | 362 |
|
| 363 | 363 |
// Regardless of whether the user sets it to true or false, if they |
| 364 | 364 |
// specify TLSVerify at all then we need to turn on TLS |
| 365 |
- if config.IsValueSet("tls-verify") {
|
|
| 365 |
+ if config.IsValueSet(tlsVerifyKey) {
|
|
| 366 | 366 |
config.TLS = true |
| 367 | 367 |
} |
| 368 | 368 |
|
| ... | ... |
@@ -105,10 +105,11 @@ func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) {
|
| 105 | 105 |
} |
| 106 | 106 |
|
| 107 | 107 |
configFile := f.Name() |
| 108 |
- f.Write([]byte(`{"tls-verify": true}`))
|
|
| 108 |
+ f.Write([]byte(`{"tlsverify": true}`))
|
|
| 109 | 109 |
f.Close() |
| 110 | 110 |
|
| 111 | 111 |
flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
| 112 |
+ flags.Bool([]string{"-tlsverify"}, false, "")
|
|
| 112 | 113 |
loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) |
| 113 | 114 |
if err != nil {
|
| 114 | 115 |
t.Fatal(err) |
| ... | ... |
@@ -136,10 +137,11 @@ func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) {
|
| 136 | 136 |
} |
| 137 | 137 |
|
| 138 | 138 |
configFile := f.Name() |
| 139 |
- f.Write([]byte(`{"tls-verify": false}`))
|
|
| 139 |
+ f.Write([]byte(`{"tlsverify": false}`))
|
|
| 140 | 140 |
f.Close() |
| 141 | 141 |
|
| 142 | 142 |
flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
| 143 |
+ flags.Bool([]string{"-tlsverify"}, false, "")
|
|
| 143 | 144 |
loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) |
| 144 | 145 |
if err != nil {
|
| 145 | 146 |
t.Fatal(err) |
| ... | ... |
@@ -198,6 +200,7 @@ func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) {
|
| 198 | 198 |
f.Close() |
| 199 | 199 |
|
| 200 | 200 |
flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
| 201 |
+ flags.String([]string{"-log-level"}, "", "")
|
|
| 201 | 202 |
loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) |
| 202 | 203 |
if err != nil {
|
| 203 | 204 |
t.Fatal(err) |
| ... | ... |
@@ -213,3 +216,30 @@ func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) {
|
| 213 | 213 |
t.Fatalf("expected warn log level, got %v", logrus.GetLevel())
|
| 214 | 214 |
} |
| 215 | 215 |
} |
| 216 |
+ |
|
| 217 |
+func TestLoadDaemonCliConfigWithTLSOptions(t *testing.T) {
|
|
| 218 |
+ c := &daemon.Config{}
|
|
| 219 |
+ common := &cli.CommonFlags{}
|
|
| 220 |
+ |
|
| 221 |
+ f, err := ioutil.TempFile("", "docker-config-")
|
|
| 222 |
+ if err != nil {
|
|
| 223 |
+ t.Fatal(err) |
|
| 224 |
+ } |
|
| 225 |
+ |
|
| 226 |
+ configFile := f.Name() |
|
| 227 |
+ f.Write([]byte(`{"tls-opts": {"tlscacert": "/etc/certs/ca.pem"}}`))
|
|
| 228 |
+ f.Close() |
|
| 229 |
+ |
|
| 230 |
+ flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
|
|
| 231 |
+ flags.String([]string{"-tlscacert"}, "", "")
|
|
| 232 |
+ loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile) |
|
| 233 |
+ if err != nil {
|
|
| 234 |
+ t.Fatal(err) |
|
| 235 |
+ } |
|
| 236 |
+ if loadedConfig == nil {
|
|
| 237 |
+ t.Fatalf("expected configuration %v, got nil", c)
|
|
| 238 |
+ } |
|
| 239 |
+ if loadedConfig.TLSOptions.CAFile != "/etc/certs/ca.pem" {
|
|
| 240 |
+ t.Fatalf("expected CA file path /etc/certs/ca.pem, got %v", loadedConfig.TLSOptions.CAFile)
|
|
| 241 |
+ } |
|
| 242 |
+} |
| ... | ... |
@@ -1223,11 +1223,27 @@ func (v mergeVal) IsBoolFlag() bool {
|
| 1223 | 1223 |
return false |
| 1224 | 1224 |
} |
| 1225 | 1225 |
|
| 1226 |
+// Name returns the name of a mergeVal. |
|
| 1227 |
+// If the original value had a name, return the original name, |
|
| 1228 |
+// otherwise, return the key asinged to this mergeVal. |
|
| 1229 |
+func (v mergeVal) Name() string {
|
|
| 1230 |
+ type namedValue interface {
|
|
| 1231 |
+ Name() string |
|
| 1232 |
+ } |
|
| 1233 |
+ if nVal, ok := v.Value.(namedValue); ok {
|
|
| 1234 |
+ return nVal.Name() |
|
| 1235 |
+ } |
|
| 1236 |
+ return v.key |
|
| 1237 |
+} |
|
| 1238 |
+ |
|
| 1226 | 1239 |
// Merge is an helper function that merges n FlagSets into a single dest FlagSet |
| 1227 | 1240 |
// In case of name collision between the flagsets it will apply |
| 1228 | 1241 |
// the destination FlagSet's errorHandling behavior. |
| 1229 | 1242 |
func Merge(dest *FlagSet, flagsets ...*FlagSet) error {
|
| 1230 | 1243 |
for _, fset := range flagsets {
|
| 1244 |
+ if fset.formal == nil {
|
|
| 1245 |
+ continue |
|
| 1246 |
+ } |
|
| 1231 | 1247 |
for k, f := range fset.formal {
|
| 1232 | 1248 |
if _, ok := dest.formal[k]; ok {
|
| 1233 | 1249 |
var err error |
| ... | ... |
@@ -1249,6 +1265,9 @@ func Merge(dest *FlagSet, flagsets ...*FlagSet) error {
|
| 1249 | 1249 |
} |
| 1250 | 1250 |
newF := *f |
| 1251 | 1251 |
newF.Value = mergeVal{f.Value, k, fset}
|
| 1252 |
+ if dest.formal == nil {
|
|
| 1253 |
+ dest.formal = make(map[string]*Flag) |
|
| 1254 |
+ } |
|
| 1252 | 1255 |
dest.formal[k] = &newF |
| 1253 | 1256 |
} |
| 1254 | 1257 |
} |
| ... | ... |
@@ -514,3 +514,14 @@ func TestSortFlags(t *testing.T) {
|
| 514 | 514 |
t.Fatalf("NFlag (%d) != fs.NFlag() (%d) of elements visited", nflag, fs.NFlag())
|
| 515 | 515 |
} |
| 516 | 516 |
} |
| 517 |
+ |
|
| 518 |
+func TestMergeFlags(t *testing.T) {
|
|
| 519 |
+ base := NewFlagSet("base", ContinueOnError)
|
|
| 520 |
+ base.String([]string{"f"}, "", "")
|
|
| 521 |
+ |
|
| 522 |
+ fs := NewFlagSet("test", ContinueOnError)
|
|
| 523 |
+ Merge(fs, base) |
|
| 524 |
+ if len(fs.formal) != 1 {
|
|
| 525 |
+ t.Fatalf("FlagCount (%d) != number (1) of elements merged", len(fs.formal))
|
|
| 526 |
+ } |
|
| 527 |
+} |