From c13a4731c70dfc0baee381018f1c9672018fa2de Mon Sep 17 00:00:00 2001 From: Ivan Smirnov Date: Sat, 14 Jul 2018 12:43:22 -0700 Subject: [PATCH] Add logic for validating config. - detects unknown keys - detects bad data types under known keys Note: does not detect duplicate keys due to limitations in YAML parser libraries. --- config.go | 50 +++++++++++++++++++++++++++ config_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++ main.go | 12 +++++++ main_test.go | 9 +++-- text_test.go | 16 ++++----- web_test.go | 6 ++-- 6 files changed, 172 insertions(+), 14 deletions(-) create mode 100644 config_test.go diff --git a/config.go b/config.go index 37407b5..15bb900 100644 --- a/config.go +++ b/config.go @@ -12,6 +12,7 @@ import ( "github.com/Jeffail/gabs" "github.com/fsnotify/fsnotify" "github.com/ghodss/yaml" + "github.com/hashicorp/go-multierror" ) const ( @@ -20,8 +21,12 @@ const ( expandKey = "expand" queryKey = "query" sslKey = "ssl_off" + ) +// Sentinel value used to indicate set membership. +var exists = struct{}{} + // parseYaml takes a file name and returns a gabs config object. func parseYaml(fname string) (*gabs.Container, error) { data, err := ioutil.ReadFile(fname) @@ -38,6 +43,51 @@ func parseYaml(fname string) (*gabs.Container, error) { return j, nil } +// validateConfig verifies that there are no unexpected values in the config file. +// at each level of the config, we should either have a KV for expansions, or a leaf node +// with the values oneof "expand", "query", "ssl_off" that map to a string. +func validateConfig(c *gabs.Container) (error) { + var errors *multierror.Error + children, _ := c.ChildrenMap() + seenKeys := make(map[string]struct{}) + for k,v := range children { + // Check if key already seen + if _, ok := seenKeys[k]; ok { + log.Printf("%s detected again", k) + errors = multierror.Append(errors, fmt.Errorf("duplicate key detected %s", k)) + } else { + seenKeys[k] = exists // mark key as seen + } + + // Validate all children + switch k { + case + "expand", + "query": + // check that v is a string, else return error. + if _, ok := v.Data().(string); !ok { + errors = multierror.Append(errors, fmt.Errorf("expected string value for %T, got: %v", k, v.Data())) + } + case "ssl_off": + // check that v is a boolean, else return error. + if _, ok := v.Data().(bool); !ok { + errors = multierror.Append(errors, fmt.Errorf("expected bool value for %T, got: %v", k, v.Data())) + } + default: + // Check if we have an unknown string here. + if _, ok := v.Data().(string); ok { + fmt.Printf("unexpected key %s", k) + errors = multierror.Append(errors, fmt.Errorf("unexpected string value under key %s, got: %v", k, v.Data())) + } + // recurse, collect any errors. + if err := validateConfig(v); err != nil { + errors = multierror.Append(errors, err) + } + } + } + return errors.ErrorOrNil() +} + func watchChanges(watcher *fsnotify.Watcher, fname string, cb func()) { for { select { diff --git a/config_test.go b/config_test.go new file mode 100644 index 0000000..cbd2fcb --- /dev/null +++ b/config_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "testing" + + . "github.com/smartystreets/goconvey/convey" +) + +const okYAML = ` +e: + expand: example.com + a: + expand: apples + b: + expand: bananas +g: + expand: github.com + d: + expand: issmirnov/dotfiles + z: + expand: issmirnov/zap + s: + query: "search?q=" +z: + expand: zero.com + ssl_off: yes +zz: + expand: zero.ssl.on.com + ssl_off: no +` + +const duplicatedYAML = ` +e: + expand: example.com + a: + expand: apples + b: + expand: bananas +g: + expand: github.com + d: + expand: issmirnov/dotfiles + z: + expand: issmirnov/zap + s: + query: "search?q=" +z: + expand: zero.com + ssl_off: yes +zz: + expand: zero.ssl.on.com + ssl_off: no +zz: + expand: secondaryexpansion.com +` + +const badkeysYAML = ` +e: + bad_key: example.com + a: + expand: apples +g: + expand: github.com + d: + expand: issmirnov/dotfiles +` + + +func TestValidateConfig(t *testing.T) { + Convey("Given a correctly formatted yaml config", t, func() { + conf, _ := parseYamlString(okYAML) + //fmt.Printf(err.Error()) + Convey("The validator should pass", func() { + So(validateConfig(conf), ShouldBeNil) + }) + }) + + // The YAML libraries don't have support for detecting duplicate keys + // at parse time. Users will have to figure this out themselves. + //Convey("Given a yaml config with duplicated keys", t, func() { + // conf, _ := parseYamlString(duplicatedYAML) + // Convey("The validator should complain", func() { + // So(validateConfig(conf), ShouldNotBeNil) + // }) + //}) + + Convey("Given a YAML config with unknown keys", t, func() { + conf, _ := parseYamlString(badkeysYAML) + Convey("The validator should raise an error", func() { + So(validateConfig(conf), ShouldNotBeNil) + }) + }) +} diff --git a/main.go b/main.go index 19dfab6..4813821 100644 --- a/main.go +++ b/main.go @@ -24,6 +24,7 @@ func main() { port = flag.Int("port", 8927, "port to bind to") host = flag.String("host", "127.0.0.1", "host interface") v = flag.Bool("v", false, "print version info") + validate = flag.Bool("validate", false, "load config file and check for errors") ) flag.Parse() @@ -38,6 +39,17 @@ func main() { log.Printf("Error parsing config file. Please fix syntax: %s\n", err) return } + + // Perform extended validation of config. + if *validate { + if err := validateConfig(c); err != nil { + fmt.Println(err.Error()) + os.Exit(1) + } + fmt.Println("No errors detected.") + os.Exit(0) + } + context := &context{config: c} updateHosts(context) // sync changes since last run. diff --git a/main_test.go b/main_test.go index 4641751..47d25a0 100644 --- a/main_test.go +++ b/main_test.go @@ -30,13 +30,16 @@ zz: ssl_off: no ` -func parseDummyYaml() (*gabs.Container, error) { - d, jsonErr := yaml.YAMLToJSON([]byte(cYaml)) +func loadTestYaml() (*gabs.Container, error) { + return parseYamlString(cYaml) +} + +func parseYamlString(config string) (*gabs.Container, error) { + d, jsonErr := yaml.YAMLToJSON([]byte(config)) if jsonErr != nil { fmt.Printf("Error encoding input to JSON.\n%s\n", jsonErr.Error()) return nil, jsonErr } j, _ := gabs.ParseJSON(d) return j, nil - } diff --git a/text_test.go b/text_test.go index 28d349d..5e0885e 100644 --- a/text_test.go +++ b/text_test.go @@ -42,7 +42,7 @@ func TestTokenizer(t *testing.T) { func TestExpander(t *testing.T) { Convey("Given 'g/z'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/z") var res bytes.Buffer res.WriteString("https:/") @@ -54,7 +54,7 @@ func TestExpander(t *testing.T) { }) }) // Convey("Given 'e/n'", t, func() { - // c, _ := parseDummyYaml() + // c, _ := loadTestYaml() // l := tokenize("e/n ") // var res bytes.Buffer // res.WriteString("https:/") @@ -66,7 +66,7 @@ func TestExpander(t *testing.T) { // }) // }) Convey("Given 'g/z/extratext'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/z/extratext") var res bytes.Buffer res.WriteString("https:/") @@ -78,7 +78,7 @@ func TestExpander(t *testing.T) { }) }) Convey("Given 'g/'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/") var res bytes.Buffer res.WriteString("https:/") @@ -90,7 +90,7 @@ func TestExpander(t *testing.T) { }) }) Convey("Given 'g/z/very/deep/path'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/z/very/deep/path") var res bytes.Buffer res.WriteString("https:/") @@ -102,7 +102,7 @@ func TestExpander(t *testing.T) { }) }) Convey("Given 'g/s/foobar'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/s/foobar") var res bytes.Buffer res.WriteString("https:/") @@ -114,7 +114,7 @@ func TestExpander(t *testing.T) { }) }) Convey("Given 'g/s/foo/bar/baz'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/s/foo/bar/baz") var res bytes.Buffer res.WriteString("https:/") @@ -126,7 +126,7 @@ func TestExpander(t *testing.T) { }) }) Convey("Given 'g/s/foo/bar/baz/'", t, func() { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() l := tokenize("g/s/foo/bar/baz/") var res bytes.Buffer res.WriteString("https:/") diff --git a/web_test.go b/web_test.go index 3b5f3af..310a77d 100644 --- a/web_test.go +++ b/web_test.go @@ -15,7 +15,7 @@ import ( // See https://elithrar.github.io/article/testing-http-handlers-go/ for comments. func TestIndexHandler(t *testing.T) { Convey("Given app is set up with default config", t, func() { - c, err := parseDummyYaml() + c, err := loadTestYaml() So(err, ShouldBeNil) context := &context{config: c} appHandler := &ctxWrapper{context, IndexHandler} @@ -155,7 +155,7 @@ func TestIndexHandler(t *testing.T) { // BenchmarkIndexHandler tests request processing geed when context is preloaded. // Run with go test -run=BenchmarkIndexHandler -bench=. // results: 500000x 2555 ns/op func BenchmarkIndexHandler(b *testing.B) { - c, _ := parseDummyYaml() + c, _ := loadTestYaml() context := &context{config: c} appHandler := &ctxWrapper{context, IndexHandler} handler := http.Handler(appHandler) @@ -187,7 +187,7 @@ func TestHealthCheckHandler(t *testing.T) { func TestVarzHandler(t *testing.T) { Convey("Given app is set up with default config", t, func() { - c, err := parseDummyYaml() + c, err := loadTestYaml() So(err, ShouldBeNil) context := &context{config: c}