From af8e5905758066b683ebdbbe600ab2d8e08b8bb3 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sat, 20 Dec 2014 23:26:05 -0800 Subject: [PATCH] config: change valid tag to use regexp A regular expression is much more useful than a list of strings. --- config/config.go | 14 ++++++------- config/config_test.go | 48 ++++++++++++++++++++++++++++++++++--------- config/file.go | 2 +- config/file_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++ config/unit.go | 2 +- config/unit_test.go | 46 +++++++++++++++++++++++++++++++++++++++++ config/update.go | 2 +- config/update_test.go | 43 ++++++++++++++++++++++++++++++++++++++ system/update_test.go | 2 +- 9 files changed, 186 insertions(+), 21 deletions(-) create mode 100644 config/file_test.go create mode 100644 config/unit_test.go create mode 100644 config/update_test.go diff --git a/config/config.go b/config/config.go index 9a80ff3..33eaef9 100644 --- a/config/config.go +++ b/config/config.go @@ -19,6 +19,7 @@ package config import ( "fmt" "reflect" + "regexp" "strings" "github.com/coreos/coreos-cloudinit/Godeps/_workspace/src/github.com/coreos/yaml" @@ -91,7 +92,7 @@ func IsZero(c interface{}) bool { type ErrorValid struct { Value string - Valid []string + Valid string Field string } @@ -125,16 +126,15 @@ func AssertValid(value reflect.Value, valid string) *ErrorValid { if valid == "" || isZero(value) { return nil } + vs := fmt.Sprintf("%v", value.Interface()) - valids := strings.Split(valid, ",") - for _, valid := range valids { - if vs == valid { - return nil - } + if m, _ := regexp.MatchString(valid, vs); m { + return nil } + return &ErrorValid{ Value: vs, - Valid: valids, + Valid: valid, } } diff --git a/config/config_test.go b/config/config_test.go index b1516be..f4b7bc4 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -18,6 +18,7 @@ package config import ( "reflect" + "regexp" "strings" "testing" ) @@ -88,29 +89,29 @@ func TestAssertStructValid(t *testing.T) { }{ {struct{}{}, nil}, {struct { - A, b string `valid:"1,2"` + A, b string `valid:"^1|2$"` }{}, nil}, {struct { - A, b string `valid:"1,2"` + A, b string `valid:"^1|2$"` }{A: "1", b: "2"}, nil}, {struct { - A, b string `valid:"1,2"` + A, b string `valid:"^1|2$"` }{A: "1", b: "hello"}, nil}, {struct { - A, b string `valid:"1,2"` - }{A: "hello", b: "2"}, &ErrorValid{Value: "hello", Field: "A", Valid: []string{"1", "2"}}}, + A, b string `valid:"^1|2$"` + }{A: "hello", b: "2"}, &ErrorValid{Value: "hello", Field: "A", Valid: "^1|2$"}}, {struct { - A, b int `valid:"1,2"` + A, b int `valid:"^1|2$"` }{}, nil}, {struct { - A, b int `valid:"1,2"` + A, b int `valid:"^1|2$"` }{A: 1, b: 2}, nil}, {struct { - A, b int `valid:"1,2"` + A, b int `valid:"^1|2$"` }{A: 1, b: 9}, nil}, {struct { - A, b int `valid:"1,2"` - }{A: 9, b: 2}, &ErrorValid{Value: "9", Field: "A", Valid: []string{"1", "2"}}}, + A, b int `valid:"^1|2$"` + }{A: 9, b: 2}, &ErrorValid{Value: "9", Field: "A", Valid: "^1|2$"}}, } for _, tt := range tests { @@ -120,6 +121,33 @@ func TestAssertStructValid(t *testing.T) { } } +func TestConfigCompile(t *testing.T) { + tests := []interface{}{ + Etcd{}, + File{}, + Flannel{}, + Fleet{}, + Locksmith{}, + OEM{}, + Unit{}, + Update{}, + } + + for _, tt := range tests { + ttt := reflect.TypeOf(tt) + for i := 0; i < ttt.NumField(); i++ { + ft := ttt.Field(i) + if !isFieldExported(ft) { + continue + } + + if _, err := regexp.Compile(ft.Tag.Get("valid")); err != nil { + t.Errorf("bad regexp(%s.%s): want %v, got %s", ttt.Name(), ft.Name, nil, err) + } + } + } +} + func TestCloudConfigUnknownKeys(t *testing.T) { contents := ` coreos: diff --git a/config/file.go b/config/file.go index 26dbc99..04f12c2 100644 --- a/config/file.go +++ b/config/file.go @@ -17,7 +17,7 @@ package config type File struct { - Encoding string `yaml:"encoding" valid:"base64,b64,gz,gzip,gz+base64,gzip+base64,gz+b64,gzip+b64"` + Encoding string `yaml:"encoding" valid:"^(base64|b64|gz|gzip|gz\\+base64|gzip\\+base64|gz\\+b64|gzip\\+b64)$"` Content string `yaml:"content"` Owner string `yaml:"owner"` Path string `yaml:"path"` diff --git a/config/file_test.go b/config/file_test.go new file mode 100644 index 0000000..a1e03e2 --- /dev/null +++ b/config/file_test.go @@ -0,0 +1,48 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package config + +import ( + "testing" +) + +func TestEncodingValid(t *testing.T) { + tests := []struct { + value string + + isValid bool + }{ + {value: "base64", isValid: true}, + {value: "b64", isValid: true}, + {value: "gz", isValid: true}, + {value: "gzip", isValid: true}, + {value: "gz+base64", isValid: true}, + {value: "gzip+base64", isValid: true}, + {value: "gz+b64", isValid: true}, + {value: "gzip+b64", isValid: true}, + {value: "gzzzzbase64", isValid: false}, + {value: "gzipppbase64", isValid: false}, + {value: "unknown", isValid: false}, + } + + for _, tt := range tests { + isValid := (nil == AssertStructValid(File{Encoding: tt.value})) + if tt.isValid != isValid { + t.Errorf("bad assert (%s): want %t, got %t", tt.value, tt.isValid, isValid) + } + } +} diff --git a/config/unit.go b/config/unit.go index 3a09bbb..a86bc8c 100644 --- a/config/unit.go +++ b/config/unit.go @@ -22,7 +22,7 @@ type Unit struct { Enable bool `yaml:"enable"` Runtime bool `yaml:"runtime"` Content string `yaml:"content"` - Command string `yaml:"command" valid:"start,stop,restart,reload,try-restart,reload-or-restart,reload-or-try-restart"` + Command string `yaml:"command" valid:"^(start|stop|restart|reload|try-restart|reload-or-restart|reload-or-try-restart)$"` DropIns []UnitDropIn `yaml:"drop_ins"` } diff --git a/config/unit_test.go b/config/unit_test.go new file mode 100644 index 0000000..d94dd8b --- /dev/null +++ b/config/unit_test.go @@ -0,0 +1,46 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package config + +import ( + "testing" +) + +func TestCommandValid(t *testing.T) { + tests := []struct { + value string + + isValid bool + }{ + {value: "start", isValid: true}, + {value: "stop", isValid: true}, + {value: "restart", isValid: true}, + {value: "reload", isValid: true}, + {value: "try-restart", isValid: true}, + {value: "reload-or-restart", isValid: true}, + {value: "reload-or-try-restart", isValid: true}, + {value: "tryrestart", isValid: false}, + {value: "unknown", isValid: false}, + } + + for _, tt := range tests { + isValid := (nil == AssertStructValid(Unit{Command: tt.value})) + if tt.isValid != isValid { + t.Errorf("bad assert (%s): want %t, got %t", tt.value, tt.isValid, isValid) + } + } +} diff --git a/config/update.go b/config/update.go index a42a3e1..147085d 100644 --- a/config/update.go +++ b/config/update.go @@ -17,7 +17,7 @@ package config type Update struct { - RebootStrategy string `yaml:"reboot_strategy" env:"REBOOT_STRATEGY" valid:"best-effort,etcd-lock,reboot,off"` + RebootStrategy string `yaml:"reboot_strategy" env:"REBOOT_STRATEGY" valid:"^(best-effort|etcd-lock|reboot|off)$"` Group string `yaml:"group" env:"GROUP"` Server string `yaml:"server" env:"SERVER"` } diff --git a/config/update_test.go b/config/update_test.go new file mode 100644 index 0000000..2b0851e --- /dev/null +++ b/config/update_test.go @@ -0,0 +1,43 @@ +/* + Copyright 2014 CoreOS, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package config + +import ( + "testing" +) + +func TestRebootStrategyValid(t *testing.T) { + tests := []struct { + value string + + isValid bool + }{ + {value: "best-effort", isValid: true}, + {value: "etcd-lock", isValid: true}, + {value: "reboot", isValid: true}, + {value: "off", isValid: true}, + {value: "besteffort", isValid: false}, + {value: "unknown", isValid: false}, + } + + for _, tt := range tests { + isValid := (nil == AssertStructValid(Update{RebootStrategy: tt.value})) + if tt.isValid != isValid { + t.Errorf("bad assert (%s): want %t, got %t", tt.value, tt.isValid, isValid) + } + } +} diff --git a/system/update_test.go b/system/update_test.go index 822add0..deeb66f 100644 --- a/system/update_test.go +++ b/system/update_test.go @@ -100,7 +100,7 @@ func TestUpdateFile(t *testing.T) { }, { config: config.Update{RebootStrategy: "wizzlewazzle"}, - err: &config.ErrorValid{Value: "wizzlewazzle", Field: "RebootStrategy", Valid: []string{"best-effort", "etcd-lock", "reboot", "off"}}, + err: &config.ErrorValid{Value: "wizzlewazzle", Field: "RebootStrategy", Valid: "^(best-effort|etcd-lock|reboot|off)$"}, }, { config: config.Update{Group: "master", Server: "http://foo.com"},