From 0f45d27028ba2aca0dbe7397b3ec874eacc950c2 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 23 May 2017 14:40:58 -0500 Subject: [PATCH 1/3] #33 validate config --- cmd/root.go | 38 +++++++++++++++++++++++++++++++++++++- cmd/root_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index d50dd05b4..caea25992 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" + "reflect" ) var ( @@ -109,6 +110,11 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error v.AutomaticEnv() c := v.GetString("config") + var flagErr error + validTags := make(map[string]bool) + flags.VisitAll(func(f *pflag.Flag) { + validTags[f.Name] = true + }) // add config file to viper if c != "" { @@ -118,10 +124,16 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error if err != nil { return fmt.Errorf("error reading configuration file '%s': %v", c, err) } + + for _, key := range v.AllKeys() { + if _, ok := validTags[key]; !ok { + return fmt.Errorf("invalid tag: %v", key) + } + } + } // set all values from viper - var flagErr error flags.VisitAll(func(f *pflag.Flag) { if flagErr != nil { return @@ -151,3 +163,27 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error }) return flagErr } + +func GetValidTags(v interface{}) map[string]bool { + validTag := make(map[string]bool) + conf := reflect.ValueOf(v) + + for i := 0; i < conf.Type().NumField(); i++ { + field := conf.Field(i) + if field.Kind() == reflect.Struct { + tag := conf.Type().Field(i).Tag.Get("toml") + tagString := strings.Split(tag, ",") + val := reflect.ValueOf(field.Interface()) + for j := 0; j < val.Type().NumField(); j++ { + subTag := val.Type().Field(j).Tag.Get("toml") + subTagString := strings.Split(subTag, ",") + validTag[fmt.Sprintf("%s.%s", tagString[0], subTagString[0])] = true + } + } else { + tomlTag := conf.Type().Field(i).Tag.Get("toml") + s := strings.Split(tomlTag, ",") + validTag[s[0]] = true + } + } + return validTag +} diff --git a/cmd/root_test.go b/cmd/root_test.go index 16bc82a8a..d3aeeab1f 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -171,3 +171,27 @@ func TestRootCommand(t *testing.T) { t.Fatalf("Expected standard usage message from RootCommand, but err: '%v', output: '%s'", err, outStr) } } + +func TestRootCommand_Config(t *testing.T) { + file, err := ioutil.TempFile("", "test.conf") + if err != nil { + panic(err) + } + config := `data-dir = "/tmp/pil5_0" +bind = "127.0.0.1:15000" + +[cluster] + poll-interval = "2m0s" + replicas = 2 + partitions = 128 + hosts = [ + "127.0.0.1:15000", + "127.0.0.1:15001", + ]` + file.Write([]byte(config)) + file.Close() + _, err = ExecNewRootCommand(t, "server", "--config", file.Name()) + if err.Error() != "invalid tag: cluster.partitions" { + t.Fatalf("Expected invalid tag, but err: '%v'", err) + } +} From 0a19e9f413a6b715b5864453f17586be849f47fb Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 23 May 2017 14:44:16 -0500 Subject: [PATCH 2/3] #33 don't need reflect for get valid field --- cmd/root.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index caea25992..e506f2fed 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,7 +22,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" - "reflect" ) var ( @@ -163,27 +162,3 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error }) return flagErr } - -func GetValidTags(v interface{}) map[string]bool { - validTag := make(map[string]bool) - conf := reflect.ValueOf(v) - - for i := 0; i < conf.Type().NumField(); i++ { - field := conf.Field(i) - if field.Kind() == reflect.Struct { - tag := conf.Type().Field(i).Tag.Get("toml") - tagString := strings.Split(tag, ",") - val := reflect.ValueOf(field.Interface()) - for j := 0; j < val.Type().NumField(); j++ { - subTag := val.Type().Field(j).Tag.Get("toml") - subTagString := strings.Split(subTag, ",") - validTag[fmt.Sprintf("%s.%s", tagString[0], subTagString[0])] = true - } - } else { - tomlTag := conf.Type().Field(i).Tag.Get("toml") - s := strings.Split(tomlTag, ",") - validTag[s[0]] = true - } - } - return validTag -} From 51db1c77848327981a0eeb412401dfbbfde0603e Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 25 May 2017 12:07:09 -0500 Subject: [PATCH 3/3] fix review --- cmd/root.go | 2 +- cmd/root_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index e506f2fed..a34e8b0c7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -126,7 +126,7 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error for _, key := range v.AllKeys() { if _, ok := validTags[key]; !ok { - return fmt.Errorf("invalid tag: %v", key) + return fmt.Errorf("invalid option in configuration file: %v", key) } } diff --git a/cmd/root_test.go b/cmd/root_test.go index d3aeeab1f..e1aaaf31d 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -178,20 +178,20 @@ func TestRootCommand_Config(t *testing.T) { panic(err) } config := `data-dir = "/tmp/pil5_0" -bind = "127.0.0.1:15000" +bind = "127.0.0.1:10101" [cluster] poll-interval = "2m0s" replicas = 2 partitions = 128 hosts = [ - "127.0.0.1:15000", - "127.0.0.1:15001", + "127.0.0.1:10101", + "127.0.0.1:10111", ]` file.Write([]byte(config)) file.Close() _, err = ExecNewRootCommand(t, "server", "--config", file.Name()) - if err.Error() != "invalid tag: cluster.partitions" { - t.Fatalf("Expected invalid tag, but err: '%v'", err) + if err.Error() != "invalid option in configuration file: cluster.partitions" { + t.Fatalf("Expected invalid option in configuration file, but err: '%v'", err) } }