Browse Source

daemon/config: MergeDaemonConfigurations() don't validate intermediates

MergeDaemonConfigurations was validating the configs before and after
merging. However, the "fileConfig" configuration may contain only a
"partial" configuration (options to apply to / override the existing
config). This means that some options may not be set and contain default
or empty values.

Validating such partial configurations can produce validation failures,
so to prevent those, we should validate the configuration _after_
merging, to validate the "final" state.

There's more cleaning up / improvements to be made in this area; for
example, we currently use our "self crafted" `getConflictFreeConfiguration()`
function, which is used to detect options that are not allowed to
be overridden, and which could potentially be handled by mergo.Merge(),
but leaving those changes for a future exercise.

This patch removes the first validation step, changing the function
to only validate the resulting configuration after merging.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 3 years ago
parent
commit
9a54dadc44
1 changed files with 1 additions and 6 deletions
  1. 1 6
      daemon/config/config.go

+ 1 - 6
daemon/config/config.go

@@ -374,17 +374,12 @@ func MergeDaemonConfigurations(flagsConfig *Config, flags *pflag.FlagSet, config
 		return nil, err
 	}
 
-	if err := Validate(fileConfig); err != nil {
-		return nil, errors.Wrap(err, "configuration validation from file failed")
-	}
-
 	// merge flags configuration on top of the file configuration
 	if err := mergo.Merge(fileConfig, flagsConfig); err != nil {
 		return nil, err
 	}
 
-	// We need to validate again once both fileConfig and flagsConfig
-	// have been merged
+	// validate the merged fileConfig and flagsConfig
 	if err := Validate(fileConfig); err != nil {
 		return nil, errors.Wrap(err, "merged configuration validation from file and command line flags failed")
 	}