Fixes #971: Rechecks config option constraints after CLI flags applied

control/config.go

@@ -67,12 +67,12 @@ type pluginConfigItem struct {
 //         UnmarshalJSON method in this same file needs to be modified to
 //         match the field mapping that is defined here
 type Config struct {
-	MaxRunningPlugins int               `json:"max_running_plugins,omitempty"yaml:"max_running_plugins,omitempty"`
-	PluginTrust       int               `json:"plugin_trust_level,omitempty"yaml:"plugin_trust_level,omitempty"`
-	AutoDiscoverPath  string            `json:"auto_discover_path,omitempty"yaml:"auto_discover_path,omitempty"`
-	KeyringPaths      string            `json:"keyring_paths,omitempty"yaml:"keyring_paths,omitempty"`
-	CacheExpiration   jsonutil.Duration `json:"cache_expiration,omitempty"yaml:"cache_expiration,omitempty"`
-	Plugins           *pluginConfig     `json:"plugins,omitempty"yaml:"plugins,omitempty"`
+	MaxRunningPlugins int               `json:"max_running_plugins"yaml:"max_running_plugins"`
+	PluginTrust       int               `json:"plugin_trust_level"yaml:"plugin_trust_level"`
+	AutoDiscoverPath  string            `json:"auto_discover_path"yaml:"auto_discover_path"`
+	KeyringPaths      string            `json:"keyring_paths"yaml:"keyring_paths"`
+	CacheExpiration   jsonutil.Duration `json:"cache_expiration"yaml:"cache_expiration"`
+	Plugins           *pluginConfig     `json:"plugins"yaml:"plugins"`
 }
 
 const (

mgmt/rest/server.go

@@ -72,14 +72,14 @@ var (
 //         UnmarshalJSON method in this same file needs to be modified to
 //         match the field mapping that is defined here
 type Config struct {
-	Enable           bool   `json:"enable,omitempty"yaml:"enable,omitempty"`
-	Port             int    `json:"port,omitempty"yaml:"port,omitempty"`
-	Address          string `json:"addr,omitempty"yaml:"addr,omitempty"`
-	HTTPS            bool   `json:"https,omitempty"yaml:"https,omitempty"`
-	RestCertificate  string `json:"rest_certificate,omitempty"yaml:"rest_certificate,omitempty"`
-	RestKey          string `json:"rest_key,omitempty"yaml:"rest_key,omitempty"`
-	RestAuth         bool   `json:"rest_auth,omitempty"yaml:"rest_auth,omitempty"`
-	RestAuthPassword string `json:"rest_auth_password,omitempty"yaml:"rest_auth_password,omitempty"`
+	Enable           bool   `json:"enable"yaml:"enable"`
+	Port             int    `json:"port"yaml:"port"`
+	Address          string `json:"addr"yaml:"addr"`
+	HTTPS            bool   `json:"https"yaml:"https"`
+	RestCertificate  string `json:"rest_certificate"yaml:"rest_certificate"`
+	RestKey          string `json:"rest_key"yaml:"rest_key"`
+	RestAuth         bool   `json:"rest_auth"yaml:"rest_auth"`
+	RestAuthPassword string `json:"rest_auth_password"yaml:"rest_auth_password"`
 }
 
 const (

mgmt/tribe/config.go

@@ -47,11 +47,11 @@ const (
 //         UnmarshalJSON method in this same file needs to be modified to
 //         match the field mapping that is defined here
 type Config struct {
-	Name                      string             `json:"name,omitempty"yaml:"name,omitempty"`
-	Enable                    bool               `json:"enable,omitempty"yaml:"enable,omitempty"`
-	BindAddr                  string             `json:"bind_addr,omitempty"yaml:"bind_addr,omitempty"`
-	BindPort                  int                `json:"bind_port,omitempty"yaml:"bind_port,omitempty"`
-	Seed                      string             `json:"seed,omitempty"yaml:"seed,omitempty"`
+	Name                      string             `json:"name"yaml:"name"`
+	Enable                    bool               `json:"enable"yaml:"enable"`
+	BindAddr                  string             `json:"bind_addr"yaml:"bind_addr"`
+	BindPort                  int                `json:"bind_port"yaml:"bind_port"`
+	Seed                      string             `json:"seed"yaml:"seed"`
 	MemberlistConfig          *memberlist.Config `json:"-"yaml:"-"`
 	RestAPIProto              string             `json:"-"yaml:"-"`
 	RestAPIPassword           string             `json:"-"yaml:"-"`

pkg/cfgfile/cfgfile.go

@@ -54,14 +54,14 @@ var cfgReader reader
 // validating the schema; first, create an interface to wrap the underlying
 // method that will be used to validate the schema
 type schemaValidator interface {
-	ValidateSchema(schema, cfg string) []serror.SnapError
+	validateSchema(schema, cfg string) []serror.SnapError
 }
 
 // then, define a type (struct) that will validate the underlying schema
 type schemaValidatorType struct{}
 
 // and define an implementation for that type that performs the schema validation
-func (r *schemaValidatorType) ValidateSchema(schema, cfg string) []serror.SnapError {
+func (r *schemaValidatorType) validateSchema(schema, cfg string) []serror.SnapError {
 	schemaLoader := gojsonschema.NewStringLoader(schema)
 	testDoc := gojsonschema.NewStringLoader(cfg)
 	result, err := gojsonschema.Validate(schemaLoader, testDoc)
@@ -110,7 +110,7 @@ func Read(path string, v interface{}, schema string) []serror.SnapError {
 		return []serror.SnapError{serror.New(fmt.Errorf("error converting YAML to JSON: %v", err))}
 	}
 	// validate the resulting JSON against the input the schema
-	if errors := cfgValidator.ValidateSchema(schema, string(jb)); errors != nil {
+	if errors := cfgValidator.validateSchema(schema, string(jb)); errors != nil {
 		// if invalid, construct (and return?) a SnapError from the errors identified
 		// during schema validation
 		return errors
@@ -128,3 +128,9 @@ func Read(path string, v interface{}, schema string) []serror.SnapError {
 	}
 	return nil
 }
+
+// Validate an input JSON string against the input schema (and return a set of errors
+// or nil if the input JSON string matches the constraints defined in that schema)
+func ValidateSchema(schema, cfg string) []serror.SnapError {
+	return cfgValidator.validateSchema(schema, cfg)
+}

pkg/cfgfile/cfgfile_small_test.go

@@ -112,8 +112,8 @@ type mockSchemaValidator struct {
 	returnError bool
 }
 
-// and redefine the ValidateSchema method to always return nil (no errors found)
-func (r *mockSchemaValidator) ValidateSchema(schema, cfg string) []serror.SnapError {
+// and redefine the validateSchema method to always return nil (no errors found)
+func (r *mockSchemaValidator) validateSchema(schema, cfg string) []serror.SnapError {
 	if r.returnError {
 		return []serror.SnapError{serror.New(errors.New("Invalid schema"))}
 	}
@@ -185,23 +185,23 @@ func TestValidateSchema(t *testing.T) {
 		config := testConfig{"Tom", "Justin"}
 		cfgValidator = &schemaValidatorType{}
 		jb, _ := json.Marshal(config)
-		errs := cfgValidator.ValidateSchema(MOCK_CONSTRAINTS, string(jb))
+		errs := ValidateSchema(MOCK_CONSTRAINTS, string(jb))
 		So(errs, ShouldBeNil)
 	})
 
 	Convey("Test invalid schema", t, func() {
 		config := testConfig{"Tom", "Justin"}
 		cfgValidator = &schemaValidatorType{}
 		jb, _ := json.Marshal(config)
-		errs := cfgValidator.ValidateSchema(INVALID_MOCK_CONSTRAINTS, string(jb))
+		errs := ValidateSchema(INVALID_MOCK_CONSTRAINTS, string(jb))
 		So(errs, ShouldNotBeNil)
 	})
 
 	Convey("Test invalid json", t, func() {
 		config := testConfig{"Tom", "Justin"}
 		cfgValidator = &schemaValidatorType{}
 		yb, _ := yaml.Marshal(config)
-		errs := cfgValidator.ValidateSchema(MOCK_CONSTRAINTS, string(yb))
+		errs := ValidateSchema(MOCK_CONSTRAINTS, string(yb))
 		So(errs, ShouldNotBeNil)
 	})
 }

scheduler/config.go

@@ -35,8 +35,8 @@ const (
 //         UnmarshalJSON method in this same file needs to be modified to
 //         match the field mapping that is defined here
 type Config struct {
-	WorkManagerQueueSize uint `json:"work_manager_queue_size,omitempty"yaml:"work_manager_queue_size,omitempty"`
-	WorkManagerPoolSize  uint `json:"work_manager_pool_size,omitempty"yaml:"work_manager_pool_size,omitempty"`
+	WorkManagerQueueSize uint `json:"work_manager_queue_size"yaml:"work_manager_queue_size"`
+	WorkManagerPoolSize  uint `json:"work_manager_pool_size"yaml:"work_manager_pool_size"`
 }
 
 const (

snapd.go

@@ -104,13 +104,13 @@ const (
 //         UnmarshalJSON method in this same file needs to be modified to
 //         match the field mapping that is defined here
 type Config struct {
-	LogLevel   int               `json:"log_level,omitempty"yaml:"log_level,omitempty"`
-	GoMaxProcs int               `json:"gomaxprocs,omitempty"yaml:"gomaxprocs,omitempty"`
-	LogPath    string            `json:"log_path,omitempty"yaml:"log_path,omitempty"`
-	Control    *control.Config   `json:"control,omitempty"yaml:"control,omitempty"`
-	Scheduler  *scheduler.Config `json:"scheduler,omitempty"yaml:"scheduler,omitempty"`
-	RestAPI    *rest.Config      `json:"restapi,omitempty"yaml:"restapi,omitempty"`
-	Tribe      *tribe.Config     `json:"tribe,omitempty"yaml:"tribe,omitempty"`
+	LogLevel   int               `json:"log_level"yaml:"log_level"`
+	GoMaxProcs int               `json:"gomaxprocs"yaml:"gomaxprocs"`
+	LogPath    string            `json:"log_path"yaml:"log_path"`
+	Control    *control.Config   `json:"control"yaml:"control"`
+	Scheduler  *scheduler.Config `json:"scheduler"yaml:"scheduler"`
+	RestAPI    *rest.Config      `json:"restapi"yaml:"restapi"`
+	Tribe      *tribe.Config     `json:"tribe"yaml:"tribe"`
 }
 
 const (
@@ -204,6 +204,18 @@ func action(ctx *cli.Context) {
 	// same variables in that configuration
 	applyCmdLineFlags(cfg, ctx)
 
+	// test the resulting configuration to ensure the values it contains still pass the
+	// constraints after applying the environment variables and command-line parameters;
+	// if errors are found, report them and exit with a fatal error
+	jb, _ := json.Marshal(cfg)
+	serrs := cfgfile.ValidateSchema(CONFIG_CONSTRAINTS, string(jb))
+	if serrs != nil {
+		for _, serr := range serrs {
+			log.WithFields(serr.Fields()).Error(serr.Error())
+		}
+		log.Fatal("Errors found after applying command-line flags")
+	}
+
 	// If logPath is set, we verify the logPath and set it so that all logging
 	// goes to the log file instead of stdout.
 	logPath := cfg.LogPath