Skip to content

Commit

Permalink
Add logic for validating config.
Browse files Browse the repository at this point in the history
- detects unknown keys
- detects bad data types under known keys

Note: does not detect duplicate keys due to limitations in YAML parser
libraries.
  • Loading branch information
issmirnov committed Jul 31, 2018
1 parent 8623230 commit c13a473
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 14 deletions.
50 changes: 50 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/Jeffail/gabs"
"github.com/fsnotify/fsnotify"
"github.com/ghodss/yaml"
"github.com/hashicorp/go-multierror"
)

const (
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
93 changes: 93 additions & 0 deletions config_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
})
}
12 changes: 12 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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.

Expand Down
9 changes: 6 additions & 3 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

}
16 changes: 8 additions & 8 deletions text_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:/")
Expand All @@ -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:/")
Expand All @@ -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:/")
Expand All @@ -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:/")
Expand All @@ -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:/")
Expand All @@ -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:/")
Expand All @@ -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:/")
Expand All @@ -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:/")
Expand Down
6 changes: 3 additions & 3 deletions web_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}

Expand Down

0 comments on commit c13a473

Please sign in to comment.