From 1e44f114436d1430de498a5f188c516613af7e56 Mon Sep 17 00:00:00 2001 From: Tom McSweeney Date: Wed, 4 May 2016 12:40:55 -0700 Subject: [PATCH] adds schema validation to the config file parsing process --- Godeps/Godeps.json | 12 +++++++++ control/config.go | 34 +++++++++++++++++++++++ control/config_test.go | 24 ++++++++++++++--- mgmt/rest/rest_func_test.go | 20 +++++++++++++- mgmt/rest/server.go | 34 +++++++++++++++++++++++ mgmt/rest/server_test.go | 4 +-- mgmt/tribe/config.go | 28 +++++++++++++++++++ mgmt/tribe/config_test.go | 22 +++++++++++++-- pkg/cfgfile/cfgfile.go | 54 ++++++++++++++++++++++++++++++++----- pkg/cfgfile/cfgfile_test.go | 21 +++++++++++++-- scheduler/config.go | 19 +++++++++++++ scheduler/config_test.go | 22 +++++++++++++-- snapd.go | 45 ++++++++++++++++++++++++++++--- 13 files changed, 317 insertions(+), 22 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 3c72c3598..37d356aaa 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -5,6 +5,18 @@ "./..." ], "Deps": [ + { + "ImportPath": "github.com/xeipuuv/gojsonpointer", + "Rev": "e0fe6f68307607d540ed8eac07a342c33fa1b54a" + }, + { + "ImportPath": "github.com/xeipuuv/gojsonreference", + "Rev": "e02fc20de94c78484cd5ffb007f8af96be030a45" + }, + { + "ImportPath": "github.com/xeipuuv/gojsonschema", + "Rev": "d3178baac32433047aa76f07317f84fbe2be6cda" + }, { "ImportPath": "github.com/Sirupsen/logrus", "Comment": "v0.7.3", diff --git a/control/config.go b/control/config.go index 296b8abde..f97450a13 100644 --- a/control/config.go +++ b/control/config.go @@ -75,6 +75,40 @@ type Config struct { Plugins *pluginConfig `json:"plugins,omitempty"yaml:"plugins,omitempty"` } +const ( + CONFIG_CONSTRAINTS = ` + "control" : { + "type": ["object", "null"], + "properties": { + "plugin_trust_level": { + "type": "integer", + "minimum": 0, + "maximum": 2 + }, + "auto_discover_path": { + "type": "string" + }, + "cache_expiration": { + "type": "string" + }, + "max_running_plugins": { + "type": "integer", + "minimum": 1 + }, + "keyring_paths" : { + "type": "string" + }, + "plugins": { + "type": ["object", "null"], + "properties" : {}, + "additionalProperties": true + } + }, + "additionalProperties": false + } + ` +) + // get the default snapd configuration func GetDefaultConfig() *Config { return &Config{ diff --git a/control/config_test.go b/control/config_test.go index 22ababa77..f2f86762f 100644 --- a/control/config_test.go +++ b/control/config_test.go @@ -30,6 +30,24 @@ import ( . "github.com/smartystreets/goconvey/convey" ) +const ( + MOCK_CONSTRAINTS = `{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "snapd global config schema", + "type": ["object", "null"], + "properties": { + "control": { "$ref": "#/definitions/control" }, + "scheduler": { "$ref": "#/definitions/scheduler"}, + "restapi" : { "$ref": "#/definitions/restapi"}, + "tribe": { "$ref": "#/definitions/tribe"} + }, + "additionalProperties": true, + "definitions": { ` + + CONFIG_CONSTRAINTS + `,` + `"scheduler": {}, "restapi": {}, "tribe":{}` + + `}` + + `}` +) + type mockConfig struct { Control *Config } @@ -68,7 +86,7 @@ func TestPluginConfig(t *testing.T) { Control: GetDefaultConfig(), } path := "../examples/configs/snap-config-sample.json" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) So(err, ShouldBeNil) cfg := config.Control So(cfg.Plugins, ShouldNotBeNil) @@ -124,7 +142,7 @@ func TestControlConfigJSON(t *testing.T) { Control: GetDefaultConfig(), } path := "../examples/configs/snap-config-sample.json" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.Control @@ -187,7 +205,7 @@ func TestControlConfigYaml(t *testing.T) { Control: GetDefaultConfig(), } path := "../examples/configs/snap-config-sample.yaml" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.Control diff --git a/mgmt/rest/rest_func_test.go b/mgmt/rest/rest_func_test.go index 6c6e0579d..cd47cb1f6 100644 --- a/mgmt/rest/rest_func_test.go +++ b/mgmt/rest/rest_func_test.go @@ -64,6 +64,24 @@ var ( UploadCount = 0 ) +const ( + MOCK_CONSTRAINTS = `{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "snapd global config schema", + "type": ["object", "null"], + "properties": { + "control": { "$ref": "#/definitions/control" }, + "scheduler": { "$ref": "#/definitions/scheduler"}, + "restapi" : { "$ref": "#/definitions/restapi"}, + "tribe": { "$ref": "#/definitions/tribe"} + }, + "additionalProperties": true, + "definitions": { ` + + `"control": {}, "scheduler": {}, ` + CONFIG_CONSTRAINTS + `, "tribe":{}` + + `}` + + `}` +) + type restAPIInstance struct { port int server *Server @@ -560,7 +578,7 @@ func TestPluginRestCalls(t *testing.T) { }) Convey("Plugin config is set at startup", func() { cfg := getDefaultMockConfig() - err := cfgfile.Read("../../examples/configs/snap-config-sample.json", &cfg) + err := cfgfile.Read("../../examples/configs/snap-config-sample.json", &cfg, MOCK_CONSTRAINTS) So(err, ShouldBeNil) r := startAPI(cfg) port := r.port diff --git a/mgmt/rest/server.go b/mgmt/rest/server.go index ce0332bb7..0098c8655 100644 --- a/mgmt/rest/server.go +++ b/mgmt/rest/server.go @@ -80,6 +80,40 @@ type Config struct { RestAuthPassword string `json:"rest_auth_password,omitempty"yaml:"rest_auth_password,omitempty"` } +const ( + CONFIG_CONSTRAINTS = ` + "restapi" : { + "type": ["object", "null"], + "properties" : { + "enable": { + "type": "boolean" + }, + "https" : { + "type": "boolean" + }, + "rest_auth": { + "type": "boolean" + }, + "rest_auth_password": { + "type": "string" + }, + "rest_certificate": { + "type": "string" + }, + "rest_key" : { + "type": "string" + }, + "port" : { + "type": "integer", + "minimum": 0, + "maximum": 65535 + } + }, + "additionalProperties": false + } + ` +) + type managesMetrics interface { MetricCatalog() ([]core.CatalogedMetric, error) FetchMetrics(core.Namespace, int) ([]core.CatalogedMetric, error) diff --git a/mgmt/rest/server_test.go b/mgmt/rest/server_test.go index c699e4ae1..89602a294 100644 --- a/mgmt/rest/server_test.go +++ b/mgmt/rest/server_test.go @@ -35,7 +35,7 @@ func TestRestAPIConfigJSON(t *testing.T) { RestAPI: GetDefaultConfig(), } path := "../../examples/configs/snap-config-sample.json" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.RestAPI @@ -74,7 +74,7 @@ func TestRestAPIConfigYaml(t *testing.T) { RestAPI: GetDefaultConfig(), } path := "../../examples/configs/snap-config-sample.yaml" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.RestAPI diff --git a/mgmt/tribe/config.go b/mgmt/tribe/config.go index c4ee5813c..0ea4c7771 100644 --- a/mgmt/tribe/config.go +++ b/mgmt/tribe/config.go @@ -59,6 +59,34 @@ type Config struct { RestAPIInsecureSkipVerify string `json:"-"yaml:"-"` } +const ( + CONFIG_CONSTRAINTS = ` + "tribe": { + "type": ["object", "null"], + "properties": { + "enable" : { + "type": "boolean" + }, + "bind_addr": { + "type": "string" + }, + "bind_port": { + "type": "integer", + "minimum": 0, + "maximum": 65535 + }, + "name": { + "type": "string" + }, + "seed": { + "type" : "string" + } + }, + "additionalProperties": false + } + ` +) + // get the default snapd configuration func GetDefaultConfig() *Config { mlCfg := memberlist.DefaultLANConfig() diff --git a/mgmt/tribe/config_test.go b/mgmt/tribe/config_test.go index d47eb7b43..292b9a413 100644 --- a/mgmt/tribe/config_test.go +++ b/mgmt/tribe/config_test.go @@ -27,6 +27,24 @@ import ( . "github.com/smartystreets/goconvey/convey" ) +const ( + MOCK_CONSTRAINTS = `{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "snapd global config schema", + "type": ["object", "null"], + "properties": { + "control": { "$ref": "#/definitions/control" }, + "scheduler": { "$ref": "#/definitions/scheduler"}, + "restapi" : { "$ref": "#/definitions/restapi"}, + "tribe": { "$ref": "#/definitions/tribe"} + }, + "additionalProperties": true, + "definitions": { ` + + `"control": {}, "scheduler": {}, "restapi":{}, ` + CONFIG_CONSTRAINTS + + `}` + + `}` +) + type mockConfig struct { Tribe *Config } @@ -36,7 +54,7 @@ func TestTribeConfigJSON(t *testing.T) { Tribe: GetDefaultConfig(), } path := "../../examples/configs/snap-config-sample.json" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.Tribe @@ -69,7 +87,7 @@ func TestTribeConfigYaml(t *testing.T) { Tribe: GetDefaultConfig(), } path := "../../examples/configs/snap-config-sample.yaml" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.Tribe diff --git a/pkg/cfgfile/cfgfile.go b/pkg/cfgfile/cfgfile.go index 8093d483f..0bd4f4256 100644 --- a/pkg/cfgfile/cfgfile.go +++ b/pkg/cfgfile/cfgfile.go @@ -20,25 +20,38 @@ limitations under the License. package cfgfile import ( + "encoding/json" + "errors" "fmt" "io/ioutil" "strings" "github.com/ghodss/yaml" + "github.com/intelsdi-x/snap/core/serror" + "github.com/xeipuuv/gojsonschema" ) // Read an input configuration file, parsing it (as YAML or JSON) // into the input 'interface{}'', v -func Read(path string, v interface{}) error { +func Read(path string, v interface{}, schema string) []serror.SnapError { // read bytes from file b, err := ioutil.ReadFile(path) if err != nil { - return err + return []serror.SnapError{serror.New(err)} } - // parse the byte-stream read (above); Note that we can parse both JSON - // and YAML using the same yaml.Unmarshal() method since a JSON string is - // a valid YAML string (but the converse is not true) - if parseErr := yaml.Unmarshal(b, v); parseErr != nil { + // convert from YAML to JSON (remember, JSON is actually valid YAML) + jb, err := yaml.YAMLToJSON(b) + if err != nil { + 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 := ValidateSchema(schema, string(jb)); errors != nil { + // if invalid, construct (and return?) a SnapError from the errors identified + // during schema validation + return errors + } + // if valid, parse the JSON byte-stream (above) + if parseErr := json.Unmarshal(jb, v); parseErr != nil { // remove any YAML-specific prefix that might have been added by then // yaml.Unmarshal() method or JSON-specific prefix that might have been // added if the resulting JSON string could not be marshalled into our @@ -46,7 +59,34 @@ func Read(path string, v interface{}) error { // these prefixes then the error message will be passed through unchanged) tmpErr := strings.TrimPrefix(parseErr.Error(), "error converting YAML to JSON: yaml: ") errRet := strings.TrimPrefix(tmpErr, "error unmarshaling JSON: json: ") - return fmt.Errorf("Error while parsing configuration file: %v", errRet) + return []serror.SnapError{serror.New(fmt.Errorf("Error while parsing configuration file: %v", errRet))} } return nil } + +// Validate the configuration (JSON) string against the input schema +func ValidateSchema(schema, cfg string) []serror.SnapError { + schemaLoader := gojsonschema.NewStringLoader(schema) + testDoc := gojsonschema.NewStringLoader(cfg) + result, err := gojsonschema.Validate(schemaLoader, testDoc) + var serrors []serror.SnapError + // Check for invalid json + if err != nil { + serrors = append(serrors, serror.New(err)) + return serrors + } + // check if result passes validation + if result.Valid() { + return nil + } + for _, err := range result.Errors() { + serr := serror.New(errors.New("Validate schema error")) + serr.SetFields(map[string]interface{}{ + "value": err.Value(), + "context": err.Context().String("::"), + "description": err.Description(), + }) + serrors = append(serrors, serr) + } + return serrors +} diff --git a/pkg/cfgfile/cfgfile_test.go b/pkg/cfgfile/cfgfile_test.go index b240538d3..5d3eec302 100644 --- a/pkg/cfgfile/cfgfile_test.go +++ b/pkg/cfgfile/cfgfile_test.go @@ -29,6 +29,23 @@ import ( . "github.com/smartystreets/goconvey/convey" ) +const ( + MOCK_CONSTRAINTS = `{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "test schema", + "type": ["object", "null"], + "properties": { + "Foo": { + "type": "string" + }, + "Bar": { + "type": "string" + } + } + } + ` +) + type testConfig struct { Foo string Bar string @@ -85,7 +102,7 @@ func TestReadConfig(t *testing.T) { Convey("Unmarshal yaml file", t, func() { config := testConfig{} - err := Read(yamlFile, &config) + err := Read(yamlFile, &config, MOCK_CONSTRAINTS) So(err, ShouldBeNil) So(config.Foo, ShouldResemble, "Tom") So(config.Bar, ShouldResemble, "Justin") @@ -93,7 +110,7 @@ func TestReadConfig(t *testing.T) { Convey("Unmarshal json file", t, func() { config := testConfig{} - err := Read(jsonFile, &config) + err := Read(jsonFile, &config, MOCK_CONSTRAINTS) So(err, ShouldBeNil) So(config.Foo, ShouldResemble, "Tom") So(config.Bar, ShouldResemble, "Justin") diff --git a/scheduler/config.go b/scheduler/config.go index 2c173d4bf..eb71d5238 100644 --- a/scheduler/config.go +++ b/scheduler/config.go @@ -39,6 +39,25 @@ type Config struct { WorkManagerPoolSize uint `json:"work_manager_pool_size,omitempty"yaml:"work_manager_pool_size,omitempty"` } +const ( + CONFIG_CONSTRAINTS = ` + "scheduler": { + "type": ["object", "null"], + "properties" : { + "work_manager_queue_size" : { + "type": "integer", + "minimum": 1 + }, + "work_manager_pool_size" : { + "type": "integer", + "minimum": 1 + } + }, + "additionalProperties": false + } + ` +) + // get the default snapd configuration func GetDefaultConfig() *Config { return &Config{ diff --git a/scheduler/config_test.go b/scheduler/config_test.go index b45c58079..d3f607833 100644 --- a/scheduler/config_test.go +++ b/scheduler/config_test.go @@ -26,6 +26,24 @@ import ( . "github.com/smartystreets/goconvey/convey" ) +const ( + MOCK_CONSTRAINTS = `{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "snapd global config schema", + "type": ["object", "null"], + "properties": { + "control": { "$ref": "#/definitions/control" }, + "scheduler": { "$ref": "#/definitions/scheduler"}, + "restapi" : { "$ref": "#/definitions/restapi"}, + "tribe": { "$ref": "#/definitions/tribe"} + }, + "additionalProperties": true, + "definitions": { ` + + `"control": {}, ` + CONFIG_CONSTRAINTS + `, "restapi": {}, "tribe":{}` + + `}` + + `}` +) + type mockConfig struct { Scheduler *Config } @@ -35,7 +53,7 @@ func TestSchedulerConfigJSON(t *testing.T) { Scheduler: GetDefaultConfig(), } path := "../examples/configs/snap-config-sample.json" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.Scheduler @@ -59,7 +77,7 @@ func TestSchedulerConfigYaml(t *testing.T) { Scheduler: GetDefaultConfig(), } path := "../examples/configs/snap-config-sample.yaml" - err := cfgfile.Read(path, &config) + err := cfgfile.Read(path, &config, MOCK_CONSTRAINTS) var cfg *Config if err == nil { cfg = config.Scheduler diff --git a/snapd.go b/snapd.go index d89a2ba52..5fa0ef2e4 100644 --- a/snapd.go +++ b/snapd.go @@ -161,6 +161,42 @@ type Config struct { Tribe *tribe.Config `json:"tribe,omitempty"yaml:"tribe,omitempty"` } +const ( + CONFIG_CONSTRAINTS = `{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "snapd global config schema", + "type": ["object", "null"], + "properties": { + "log_level": { + "description": "log verbosity level for snapd. Range between 1: debug, 2: info, 3: warning, 4: error, 5: fatal", + "type": "integer", + "minimum": 1, + "maximum": 5 + }, + "log_path": { + "description": "path to log file for snapd to use", + "type": "string" + }, + "gomaxprocs": { + "description": "value to be used for gomaxprocs", + "type": "integer", + "minimum": 1 + }, + "control": { "$ref": "#/definitions/control" }, + "scheduler": { "$ref": "#/definitions/scheduler"}, + "restapi" : { "$ref": "#/definitions/restapi"}, + "tribe": { "$ref": "#/definitions/tribe"} + }, + "additionalProperties": false, + "definitions": { ` + + control.CONFIG_CONSTRAINTS + `,` + + scheduler.CONFIG_CONSTRAINTS + `,` + + rest.CONFIG_CONSTRAINTS + `,` + + tribe.CONFIG_CONSTRAINTS + + `}` + + `}` +) + type coreModule interface { Start() error Stop() @@ -551,9 +587,12 @@ func readConfig(cfg *Config, fpath string) { path = fpath } - err := cfgfile.Read(path, &cfg) - if err != nil { - log.Fatal(err) + serrs := cfgfile.Read(path, &cfg, CONFIG_CONSTRAINTS) + if serrs != nil { + for _, serr := range serrs { + log.WithFields(serr.Fields()).Error(serr.Error()) + } + log.Fatal("Errors found while parsing global configuration file") } }