From 6e2db882e62d54dc1ae69e62f165c82456cbaebf Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sat, 27 Sep 2014 11:11:57 -0700 Subject: [PATCH 1/5] script: move Script into config package --- config/config.go | 11 +++++++++++ config/script.go | 16 ++++++++++++++++ coreos-cloudinit.go | 6 +++--- initialize/user_data.go | 21 +++++++-------------- initialize/workspace.go | 2 +- system/unit.go | 2 -- 6 files changed, 38 insertions(+), 20 deletions(-) create mode 100644 config/script.go diff --git a/config/config.go b/config/config.go index 6049d2f..d2b5a9a 100644 --- a/config/config.go +++ b/config/config.go @@ -45,6 +45,17 @@ type CloudConfig struct { NetworkConfig string `yaml:"-"` } +func IsCloudConfig(userdata string) bool { + header := strings.SplitN(userdata, "\n", 2)[0] + + // Explicitly trim the header so we can handle user-data from + // non-unix operating systems. The rest of the file is parsed + // by yaml, which correctly handles CRLF. + header = strings.TrimSuffix(header, "\r") + + return (header == "#cloud-config") +} + // NewCloudConfig instantiates a new CloudConfig from the given contents (a // string of YAML), returning any error encountered. It will ignore unknown // fields but log encountering them. diff --git a/config/script.go b/config/script.go new file mode 100644 index 0000000..c8de6c2 --- /dev/null +++ b/config/script.go @@ -0,0 +1,16 @@ +package config + +import ( + "strings" +) + +type Script []byte + +func IsScript(userdata string) bool { + header := strings.SplitN(userdata, "\n", 2)[0] + return strings.HasPrefix(header, "#!") +} + +func NewScript(userdata string) (Script, error) { + return Script(userdata), nil +} diff --git a/coreos-cloudinit.go b/coreos-cloudinit.go index 20def1d..cb7f879 100644 --- a/coreos-cloudinit.go +++ b/coreos-cloudinit.go @@ -180,7 +180,7 @@ func main() { userdata := env.Apply(string(userdataBytes)) var ccm, ccu *config.CloudConfig - var script *system.Script + var script *config.Script if ccm, err = initialize.ParseMetaData(string(metadataBytes)); err != nil { fmt.Printf("Failed to parse meta-data: %v\n", err) os.Exit(1) @@ -203,7 +203,7 @@ func main() { switch t := ud.(type) { case *config.CloudConfig: ccu = t - case system.Script: + case config.Script: script = &t } } @@ -362,7 +362,7 @@ func selectDatasource(sources []datasource.Datasource) datasource.Datasource { } // TODO(jonboulle): this should probably be refactored and moved into a different module -func runScript(script system.Script, env *initialize.Environment) error { +func runScript(script config.Script, env *initialize.Environment) error { err := initialize.PrepWorkspace(env.Workspace()) if err != nil { fmt.Printf("Failed preparing workspace: %v\n", err) diff --git a/initialize/user_data.go b/initialize/user_data.go index cb477c4..b2b71be 100644 --- a/initialize/user_data.go +++ b/initialize/user_data.go @@ -17,32 +17,25 @@ package initialize import ( - "fmt" + "errors" "log" - "strings" "github.com/coreos/coreos-cloudinit/config" - "github.com/coreos/coreos-cloudinit/system" ) func ParseUserData(contents string) (interface{}, error) { if len(contents) == 0 { return nil, nil } - header := strings.SplitN(contents, "\n", 2)[0] - // Explicitly trim the header so we can handle user-data from - // non-unix operating systems. The rest of the file is parsed - // by yaml, which correctly handles CRLF. - header = strings.TrimSpace(header) - - if strings.HasPrefix(header, "#!") { + switch { + case config.IsScript(contents): log.Printf("Parsing user-data as script") - return system.Script(contents), nil - } else if header == "#cloud-config" { + return config.NewScript(contents) + case config.IsCloudConfig(contents): log.Printf("Parsing user-data as cloud-config") return config.NewCloudConfig(contents) - } else { - return nil, fmt.Errorf("Unrecognized user-data header: %s", header) + default: + return nil, errors.New("Unrecognized user-data format") } } diff --git a/initialize/workspace.go b/initialize/workspace.go index 8af319d..b9d8fc8 100644 --- a/initialize/workspace.go +++ b/initialize/workspace.go @@ -38,7 +38,7 @@ func PrepWorkspace(workspace string) error { return nil } -func PersistScriptInWorkspace(script system.Script, workspace string) (string, error) { +func PersistScriptInWorkspace(script config.Script, workspace string) (string, error) { scriptsPath := path.Join(workspace, "scripts") tmp, err := ioutil.TempFile(scriptsPath, "") if err != nil { diff --git a/system/unit.go b/system/unit.go index 5d34e99..26e9bbc 100644 --- a/system/unit.go +++ b/system/unit.go @@ -41,8 +41,6 @@ type Unit struct { config.Unit } -type Script []byte - // Destination builds the appropriate absolute file path for // the Unit. The root argument indicates the effective base // directory of the system (similar to a chroot). From 88e8265cd6783acf8792cae863067789a789a345 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 28 Oct 2014 20:04:43 -0700 Subject: [PATCH 2/5] config: seperate AssertValid and AssertStructValid Added an error structure to make it possible to get the specifics of the failure. --- config/config.go | 55 +++++++++++++++++++++++++++---------------- config/config_test.go | 9 ++++--- system/update.go | 2 +- system/update_test.go | 3 +-- 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/config/config.go b/config/config.go index d2b5a9a..6dedc2b 100644 --- a/config/config.go +++ b/config/config.go @@ -90,10 +90,20 @@ func IsZero(c interface{}) bool { return isZero(reflect.ValueOf(c)) } -// AssertValid checks the fields in the structure and makes sure that they -// contain valid values as specified by the 'valid' flag. Empty fields are +type ErrorValid struct { + Value string + Valid []string + Field string +} + +func (e ErrorValid) Error() string { + return fmt.Sprintf("invalid value %q for option %q (valid options: %q)", e.Value, e.Field, e.Valid) +} + +// AssertStructValid checks the fields in the structure and makes sure that +// they contain valid values as specified by the 'valid' flag. Empty fields are // implicitly valid. -func AssertValid(c interface{}) error { +func AssertStructValid(c interface{}) error { ct := reflect.TypeOf(c) cv := reflect.ValueOf(c) for i := 0; i < ct.NumField(); i++ { @@ -102,15 +112,33 @@ func AssertValid(c interface{}) error { continue } - valid := ft.Tag.Get("valid") - val := cv.Field(i) - if !isValid(val, valid) { - return fmt.Errorf("invalid value \"%v\" for option %q (valid options: %q)", val.Interface(), ft.Name, valid) + if err := AssertValid(cv.Field(i), ft.Tag.Get("valid")); err != nil { + err.Field = ft.Name + return err } } return nil } +// AssertValid checks to make sure that the given value is in the list of +// valid values. Zero values are implicitly valid. +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 + } + } + return &ErrorValid{ + Value: vs, + Valid: valids, + } +} + func isZero(v reflect.Value) bool { switch v.Kind() { case reflect.Struct: @@ -130,19 +158,6 @@ func isFieldExported(f reflect.StructField) bool { return f.PkgPath == "" } -func isValid(v reflect.Value, valid string) bool { - if valid == "" || isZero(v) { - return true - } - vs := fmt.Sprintf("%v", v.Interface()) - for _, valid := range strings.Split(valid, ",") { - if vs == valid { - return true - } - } - return false -} - type warner func(format string, v ...interface{}) // warnOnUnrecognizedKeys parses the contents of a cloud-config file and calls diff --git a/config/config_test.go b/config/config_test.go index 1d776d6..ea68947 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -17,7 +17,6 @@ package config import ( - "errors" "fmt" "reflect" "strings" @@ -43,7 +42,7 @@ func TestIsZero(t *testing.T) { } } -func TestAssertValid(t *testing.T) { +func TestAssertStructValid(t *testing.T) { for _, tt := range []struct { c interface{} err error @@ -60,7 +59,7 @@ func TestAssertValid(t *testing.T) { }{A: "1", b: "hello"}, nil}, {struct { A, b string `valid:"1,2"` - }{A: "hello", b: "2"}, errors.New("invalid value \"hello\" for option \"A\" (valid options: \"1,2\")")}, + }{A: "hello", b: "2"}, &ErrorValid{Value: "hello", Field: "A", Valid: []string{"1", "2"}}}, {struct { A, b int `valid:"1,2"` }{}, nil}, @@ -72,9 +71,9 @@ func TestAssertValid(t *testing.T) { }{A: 1, b: 9}, nil}, {struct { A, b int `valid:"1,2"` - }{A: 9, b: 2}, errors.New("invalid value \"9\" for option \"A\" (valid options: \"1,2\")")}, + }{A: 9, b: 2}, &ErrorValid{Value: "9", Field: "A", Valid: []string{"1", "2"}}}, } { - if err := AssertValid(tt.c); !reflect.DeepEqual(tt.err, err) { + if err := AssertStructValid(tt.c); !reflect.DeepEqual(tt.err, err) { t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.err, err) } } diff --git a/system/update.go b/system/update.go index 2038def..5d6b066 100644 --- a/system/update.go +++ b/system/update.go @@ -61,7 +61,7 @@ func (uc Update) File() (*File, error) { if config.IsZero(uc.Update) { return nil, nil } - if err := config.AssertValid(uc.Update); err != nil { + if err := config.AssertStructValid(uc.Update); err != nil { return nil, err } diff --git a/system/update_test.go b/system/update_test.go index a75348e..7b68172 100644 --- a/system/update_test.go +++ b/system/update_test.go @@ -17,7 +17,6 @@ package system import ( - "errors" "io" "reflect" "strings" @@ -101,7 +100,7 @@ func TestUpdateFile(t *testing.T) { }, { config: config.Update{RebootStrategy: "wizzlewazzle"}, - err: errors.New("invalid value \"wizzlewazzle\" for option \"RebootStrategy\" (valid options: \"best-effort,etcd-lock,reboot,false\")"), + err: &config.ErrorValid{Value: "wizzlewazzle", Field: "RebootStrategy", Valid: []string{"best-effort", "etcd-lock", "reboot", "false"}}, }, { config: config.Update{Group: "master", Server: "http://foo.com"}, From 51f37100a176fdd8e5970e47e4f9e3f0010ad3f1 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Wed, 22 Oct 2014 11:43:26 -0700 Subject: [PATCH 3/5] config: remove config validator --- config/config.go | 86 ------------------------------------------- config/config_test.go | 24 ------------ 2 files changed, 110 deletions(-) diff --git a/config/config.go b/config/config.go index 6dedc2b..9ac6d69 100644 --- a/config/config.go +++ b/config/config.go @@ -18,7 +18,6 @@ package config import ( "fmt" - "log" "reflect" "strings" @@ -68,7 +67,6 @@ func NewCloudConfig(contents string) (*CloudConfig, error) { if err = yaml.Unmarshal(ncontents, &cfg); err != nil { return &cfg, err } - warnOnUnrecognizedKeys(contents, log.Printf) return &cfg, nil } @@ -158,90 +156,6 @@ func isFieldExported(f reflect.StructField) bool { return f.PkgPath == "" } -type warner func(format string, v ...interface{}) - -// warnOnUnrecognizedKeys parses the contents of a cloud-config file and calls -// warn(msg, key) for every unrecognized key (i.e. those not present in CloudConfig) -func warnOnUnrecognizedKeys(contents string, warn warner) { - // Generate a map of all understood cloud config options - var cc map[string]interface{} - b, _ := yaml.Marshal(&CloudConfig{}) - yaml.Unmarshal(b, &cc) - - // Now unmarshal the entire provided contents - var c map[string]interface{} - yaml.Unmarshal([]byte(contents), &c) - - // Check that every key in the contents exists in the cloud config - for k, _ := range c { - if _, ok := cc[k]; !ok { - warn("Warning: unrecognized key %q in provided cloud config - ignoring section", k) - } - } - - // Check for unrecognized coreos options, if any are set - if coreos, ok := c["coreos"]; ok { - if set, ok := coreos.(map[interface{}]interface{}); ok { - known := cc["coreos"].(map[interface{}]interface{}) - for k, _ := range set { - if key, ok := k.(string); ok { - if _, ok := known[key]; !ok { - warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", key) - } - } else { - warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", k) - } - } - } - } - - // Check for any badly-specified users, if any are set - if users, ok := c["users"]; ok { - var known map[string]interface{} - b, _ := yaml.Marshal(&User{}) - yaml.Unmarshal(b, &known) - - if set, ok := users.([]interface{}); ok { - for _, u := range set { - if user, ok := u.(map[interface{}]interface{}); ok { - for k, _ := range user { - if key, ok := k.(string); ok { - if _, ok := known[key]; !ok { - warn("Warning: unrecognized key %q in user section of cloud config - ignoring", key) - } - } else { - warn("Warning: unrecognized key %q in user section of cloud config - ignoring", k) - } - } - } - } - } - } - - // Check for any badly-specified files, if any are set - if files, ok := c["write_files"]; ok { - var known map[string]interface{} - b, _ := yaml.Marshal(&File{}) - yaml.Unmarshal(b, &known) - - if set, ok := files.([]interface{}); ok { - for _, f := range set { - if file, ok := f.(map[interface{}]interface{}); ok { - for k, _ := range file { - if key, ok := k.(string); ok { - if _, ok := known[key]; !ok { - warn("Warning: unrecognized key %q in file section of cloud config - ignoring", key) - } - } else { - warn("Warning: unrecognized key %q in file section of cloud config - ignoring", k) - } - } - } - } - } - } -} - func normalizeConfig(config string) ([]byte, error) { var cfg map[interface{}]interface{} if err := yaml.Unmarshal([]byte(config), &cfg); err != nil { diff --git a/config/config_test.go b/config/config_test.go index ea68947..dbcbc12 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -17,7 +17,6 @@ package config import ( - "fmt" "reflect" "strings" "testing" @@ -146,29 +145,6 @@ hostname: if len(cfg.Users) < 1 || cfg.Users[0].Name != "fry" || cfg.Users[0].PasswordHash != "somehash" { t.Fatalf("users section not correctly set when invalid keys are present") } - - var warnings string - catchWarn := func(f string, v ...interface{}) { - warnings += fmt.Sprintf(f, v...) - } - - warnOnUnrecognizedKeys(contents, catchWarn) - - if !strings.Contains(warnings, "coreos_unknown") { - t.Errorf("warnings did not catch unrecognized coreos option coreos_unknown") - } - if !strings.Contains(warnings, "bare_unknown") { - t.Errorf("warnings did not catch unrecognized key bare_unknown") - } - if !strings.Contains(warnings, "section_unknown") { - t.Errorf("warnings did not catch unrecognized key section_unknown") - } - if !strings.Contains(warnings, "user_unknown") { - t.Errorf("warnings did not catch unrecognized user key user_unknown") - } - if !strings.Contains(warnings, "file_unknown") { - t.Errorf("warnings did not catch unrecognized file key file_unknown") - } } // Assert that the parsing of a cloud config file "generally works" From 055a3c339a45a0e92b9e40fe29c087db3f97ab22 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Wed, 22 Oct 2014 11:27:51 -0700 Subject: [PATCH 4/5] config/validate: add new config validator This validator is still experimental and is going to need new rules in the future. This lays out the general framework. --- config/script.go | 16 ++ config/unit.go | 2 +- config/validate/context.go | 54 ++++++ config/validate/context_test.go | 133 ++++++++++++++ config/validate/node.go | 159 +++++++++++++++++ config/validate/node_test.go | 286 +++++++++++++++++++++++++++++++ config/validate/report.go | 90 ++++++++++ config/validate/report_test.go | 98 +++++++++++ config/validate/rules.go | 115 +++++++++++++ config/validate/rules_test.go | 251 +++++++++++++++++++++++++++ config/validate/validate.go | 113 ++++++++++++ config/validate/validate_test.go | 121 +++++++++++++ test | 1 + 13 files changed, 1438 insertions(+), 1 deletion(-) create mode 100644 config/validate/context.go create mode 100644 config/validate/context_test.go create mode 100644 config/validate/node.go create mode 100644 config/validate/node_test.go create mode 100644 config/validate/report.go create mode 100644 config/validate/report_test.go create mode 100644 config/validate/rules.go create mode 100644 config/validate/rules_test.go create mode 100644 config/validate/validate.go create mode 100644 config/validate/validate_test.go diff --git a/config/script.go b/config/script.go index c8de6c2..64f05c8 100644 --- a/config/script.go +++ b/config/script.go @@ -1,3 +1,19 @@ +/* + 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 ( diff --git a/config/unit.go b/config/unit.go index 94e20e5..1f56a1e 100644 --- a/config/unit.go +++ b/config/unit.go @@ -27,7 +27,7 @@ type Unit struct { Enable bool `yaml:"enable"` Runtime bool `yaml:"runtime"` Content string `yaml:"content"` - Command string `yaml:"command"` + Command string `yaml:"command" valid:"start,stop,restart,reload,try-restart,reload-or-restart,reload-or-try-restart"` // For drop-in units, a cloudinit.conf is generated. // This is currently unbound in YAML (and hence unsettable in cloud-config files) diff --git a/config/validate/context.go b/config/validate/context.go new file mode 100644 index 0000000..2328ac0 --- /dev/null +++ b/config/validate/context.go @@ -0,0 +1,54 @@ +/* + 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 validate + +import ( + "strings" +) + +// context represents the current position within a newline-delimited string. +// Each line is loaded, one by one, into currentLine (newline omitted) and +// lineNumber keeps track of its position within the original string. +type context struct { + currentLine string + remainingLines string + lineNumber int +} + +// Increment moves the context to the next line (if available). +func (c *context) Increment() { + if c.currentLine == "" && c.remainingLines == "" { + return + } + + lines := strings.SplitN(c.remainingLines, "\n", 2) + c.currentLine = lines[0] + if len(lines) == 2 { + c.remainingLines = lines[1] + } else { + c.remainingLines = "" + } + c.lineNumber++ +} + +// NewContext creates a context from the provided data. It strips out all +// carriage returns and moves to the first line (if available). +func NewContext(content []byte) context { + c := context{remainingLines: strings.Replace(string(content), "\r", "", -1)} + c.Increment() + return c +} diff --git a/config/validate/context_test.go b/config/validate/context_test.go new file mode 100644 index 0000000..bea8a6b --- /dev/null +++ b/config/validate/context_test.go @@ -0,0 +1,133 @@ +/* + 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 validate + +import ( + "reflect" + "testing" +) + +func TestNewContext(t *testing.T) { + tests := []struct { + in string + + out context + }{ + { + out: context{ + currentLine: "", + remainingLines: "", + lineNumber: 0, + }, + }, + { + in: "this\r\nis\r\na\r\ntest", + out: context{ + currentLine: "this", + remainingLines: "is\na\ntest", + lineNumber: 1, + }, + }, + } + + for _, tt := range tests { + if out := NewContext([]byte(tt.in)); !reflect.DeepEqual(tt.out, out) { + t.Errorf("bad context (%q): want %#v, got %#v", tt.in, tt.out, out) + } + } +} + +func TestIncrement(t *testing.T) { + tests := []struct { + init context + op func(c *context) + + res context + }{ + { + init: context{ + currentLine: "", + remainingLines: "", + lineNumber: 0, + }, + res: context{ + currentLine: "", + remainingLines: "", + lineNumber: 0, + }, + op: func(c *context) { + c.Increment() + }, + }, + { + init: context{ + currentLine: "test", + remainingLines: "", + lineNumber: 1, + }, + res: context{ + currentLine: "", + remainingLines: "", + lineNumber: 2, + }, + op: func(c *context) { + c.Increment() + c.Increment() + c.Increment() + }, + }, + { + init: context{ + currentLine: "this", + remainingLines: "is\na\ntest", + lineNumber: 1, + }, + res: context{ + currentLine: "is", + remainingLines: "a\ntest", + lineNumber: 2, + }, + op: func(c *context) { + c.Increment() + }, + }, + { + init: context{ + currentLine: "this", + remainingLines: "is\na\ntest", + lineNumber: 1, + }, + res: context{ + currentLine: "test", + remainingLines: "", + lineNumber: 4, + }, + op: func(c *context) { + c.Increment() + c.Increment() + c.Increment() + }, + }, + } + + for i, tt := range tests { + res := tt.init + if tt.op(&res); !reflect.DeepEqual(tt.res, res) { + t.Errorf("bad context (%d, %#v): want %#v, got %#v", i, tt.init, tt.res, res) + } + } +} diff --git a/config/validate/node.go b/config/validate/node.go new file mode 100644 index 0000000..e010257 --- /dev/null +++ b/config/validate/node.go @@ -0,0 +1,159 @@ +/* + 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 validate + +import ( + "fmt" + "reflect" + "regexp" +) + +var ( + yamlKey = regexp.MustCompile(`^ *-? ?(?P.*?):`) + yamlElem = regexp.MustCompile(`^ *-`) +) + +type node struct { + name string + line int + children []node + field reflect.StructField + reflect.Value +} + +// Child attempts to find the child with the given name in the node's list of +// children. If no such child is found, an invalid node is returned. +func (n node) Child(name string) node { + for _, c := range n.children { + if c.name == name { + return c + } + } + return node{} +} + +// HumanType returns the human-consumable string representation of the type of +// the node. +func (n node) HumanType() string { + switch k := n.Kind(); k { + case reflect.Slice: + c := n.Type().Elem() + return "[]" + node{Value: reflect.New(c).Elem()}.HumanType() + default: + return k.String() + } +} + +// NewNode returns the node representation of the given value. The context +// will be used in an attempt to determine line numbers for the given value. +func NewNode(value interface{}, context context) node { + var n node + toNode(value, context, &n) + return n +} + +// toNode converts the given value into a node and then recursively processes +// each of the nodes components (e.g. fields, array elements, keys). +func toNode(v interface{}, c context, n *node) { + vv := reflect.ValueOf(v) + if !vv.IsValid() { + return + } + + n.Value = vv + switch vv.Kind() { + case reflect.Struct: + // Walk over each field in the structure, skipping unexported fields, + // and create a node for it. + for i := 0; i < vv.Type().NumField(); i++ { + ft := vv.Type().Field(i) + k := ft.Tag.Get("yaml") + if k == "-" || k == "" { + continue + } + + cn := node{name: k, field: ft} + c, ok := findKey(cn.name, c) + if ok { + cn.line = c.lineNumber + } + toNode(vv.Field(i).Interface(), c, &cn) + n.children = append(n.children, cn) + } + case reflect.Map: + // Walk over each key in the map and create a node for it. + v := v.(map[interface{}]interface{}) + for k, cv := range v { + cn := node{name: fmt.Sprintf("%s", k)} + c, ok := findKey(cn.name, c) + if ok { + cn.line = c.lineNumber + } + toNode(cv, c, &cn) + n.children = append(n.children, cn) + } + case reflect.Slice: + // Walk over each element in the slice and create a node for it. + // While iterating over the slice, preserve the context after it + // is modified. This allows the line numbers to reflect the current + // element instead of the first. + for i := 0; i < vv.Len(); i++ { + cn := node{ + name: fmt.Sprintf("%s[%d]", n.name, i), + field: n.field, + } + var ok bool + c, ok = findElem(c) + if ok { + cn.line = c.lineNumber + } + toNode(vv.Index(i).Interface(), c, &cn) + n.children = append(n.children, cn) + c.Increment() + } + case reflect.String, reflect.Int, reflect.Bool: + default: + panic(fmt.Sprintf("toNode(): unhandled kind %s", vv.Kind())) + } +} + +// findKey attempts to find the requested key within the provided context. +// A modified copy of the context is returned with every line up to the key +// incremented past. A boolean, true if the key was found, is also returned. +func findKey(key string, context context) (context, bool) { + return find(yamlKey, key, context) +} + +// findElem attempts to find an array element within the provided context. +// A modified copy of the context is returned with every line up to the array +// element incremented past. A boolean, true if the key was found, is also +// returned. +func findElem(context context) (context, bool) { + return find(yamlElem, "", context) +} + +func find(exp *regexp.Regexp, key string, context context) (context, bool) { + for len(context.currentLine) > 0 || len(context.remainingLines) > 0 { + matches := exp.FindStringSubmatch(context.currentLine) + if len(matches) > 0 && (key == "" || matches[1] == key) { + return context, true + } + + context.Increment() + } + return context, false +} diff --git a/config/validate/node_test.go b/config/validate/node_test.go new file mode 100644 index 0000000..8cda6f3 --- /dev/null +++ b/config/validate/node_test.go @@ -0,0 +1,286 @@ +/* + 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 validate + +import ( + "reflect" + "testing" +) + +func TestChild(t *testing.T) { + tests := []struct { + parent node + name string + + child node + }{ + {}, + { + name: "c1", + }, + { + parent: node{ + children: []node{ + node{name: "c1"}, + node{name: "c2"}, + node{name: "c3"}, + }, + }, + }, + { + parent: node{ + children: []node{ + node{name: "c1"}, + node{name: "c2"}, + node{name: "c3"}, + }, + }, + name: "c2", + child: node{name: "c2"}, + }, + } + + for _, tt := range tests { + if child := tt.parent.Child(tt.name); !reflect.DeepEqual(tt.child, child) { + t.Errorf("bad child (%q): want %#v, got %#v", tt.name, tt.child, child) + } + } +} + +func TestHumanType(t *testing.T) { + tests := []struct { + node node + + humanType string + }{ + { + humanType: "invalid", + }, + { + node: node{Value: reflect.ValueOf("hello")}, + humanType: "string", + }, + { + node: node{ + Value: reflect.ValueOf([]int{1, 2}), + children: []node{ + node{Value: reflect.ValueOf(1)}, + node{Value: reflect.ValueOf(2)}, + }}, + humanType: "[]int", + }, + } + + for _, tt := range tests { + if humanType := tt.node.HumanType(); tt.humanType != humanType { + t.Errorf("bad type (%q): want %q, got %q", tt.node, tt.humanType, humanType) + } + } +} + +func TestToNode(t *testing.T) { + tests := []struct { + value interface{} + context context + + node node + }{ + {}, + { + value: struct{}{}, + node: node{Value: reflect.ValueOf(struct{}{})}, + }, + { + value: struct { + A int `yaml:"a"` + }{}, + node: node{ + children: []node{ + node{ + name: "a", + field: reflect.TypeOf(struct { + A int `yaml:"a"` + }{}).Field(0), + }, + }, + }, + }, + { + value: struct { + A []int `yaml:"a"` + }{}, + node: node{ + children: []node{ + node{ + name: "a", + field: reflect.TypeOf(struct { + A []int `yaml:"a"` + }{}).Field(0), + }, + }, + }, + }, + { + value: map[interface{}]interface{}{ + "a": map[interface{}]interface{}{ + "b": 2, + }, + }, + context: NewContext([]byte("a:\n b: 2")), + node: node{ + children: []node{ + node{ + line: 1, + name: "a", + children: []node{ + node{name: "b", line: 2}, + }, + }, + }, + }, + }, + { + value: struct { + A struct { + Jon bool `yaml:"b"` + } `yaml:"a"` + }{}, + node: node{ + children: []node{ + node{ + name: "a", + children: []node{ + node{ + name: "b", + field: reflect.TypeOf(struct { + Jon bool `yaml:"b"` + }{}).Field(0), + Value: reflect.ValueOf(false), + }, + }, + field: reflect.TypeOf(struct { + A struct { + Jon bool `yaml:"b"` + } `yaml:"a"` + }{}).Field(0), + Value: reflect.ValueOf(struct { + Jon bool `yaml:"b"` + }{}), + }, + }, + Value: reflect.ValueOf(struct { + A struct { + Jon bool `yaml:"b"` + } `yaml:"a"` + }{}), + }, + }, + } + + for _, tt := range tests { + var node node + toNode(tt.value, tt.context, &node) + if !nodesEqual(tt.node, node) { + t.Errorf("bad node (%#v): want %#v, got %#v", tt.value, tt.node, node) + } + } +} + +func TestFindKey(t *testing.T) { + tests := []struct { + key string + context context + + found bool + }{ + {}, + { + key: "key1", + context: NewContext([]byte("key1: hi")), + found: true, + }, + { + key: "key2", + context: NewContext([]byte("key1: hi")), + found: false, + }, + { + key: "key3", + context: NewContext([]byte("key1:\n key2:\n key3: hi")), + found: true, + }, + { + key: "key4", + context: NewContext([]byte("key1:\n - key4: hi")), + found: true, + }, + { + key: "key5", + context: NewContext([]byte("#key5")), + found: false, + }, + } + + for _, tt := range tests { + if _, found := findKey(tt.key, tt.context); tt.found != found { + t.Errorf("bad find (%q): want %t, got %t", tt.key, tt.found, found) + } + } +} + +func TestFindElem(t *testing.T) { + tests := []struct { + context context + + found bool + }{ + {}, + { + context: NewContext([]byte("test: hi")), + found: false, + }, + { + context: NewContext([]byte("test:\n - a\n -b")), + found: true, + }, + { + context: NewContext([]byte("test:\n -\n a")), + found: true, + }, + } + + for _, tt := range tests { + if _, found := findElem(tt.context); tt.found != found { + t.Errorf("bad find (%q): want %t, got %t", tt.context, tt.found, found) + } + } +} + +func nodesEqual(a, b node) bool { + if a.name != b.name || + a.line != b.line || + !reflect.DeepEqual(a.field, b.field) || + len(a.children) != len(b.children) { + return false + } + for i := 0; i < len(a.children); i++ { + if !nodesEqual(a.children[i], b.children[i]) { + return false + } + } + return true +} diff --git a/config/validate/report.go b/config/validate/report.go new file mode 100644 index 0000000..b518504 --- /dev/null +++ b/config/validate/report.go @@ -0,0 +1,90 @@ +/* + 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 validate + +import ( + "encoding/json" + "fmt" +) + +// Report represents the list of entries resulting from validation. +type Report struct { + entries []Entry +} + +// Error adds an error entry to the report. +func (r *Report) Error(line int, message string) { + r.entries = append(r.entries, Entry{entryError, message, line}) +} + +// Warning adds a warning entry to the report. +func (r *Report) Warning(line int, message string) { + r.entries = append(r.entries, Entry{entryWarning, message, line}) +} + +// Info adds an info entry to the report. +func (r *Report) Info(line int, message string) { + r.entries = append(r.entries, Entry{entryInfo, message, line}) +} + +// Entries returns the list of entries in the report. +func (r *Report) Entries() []Entry { + return r.entries +} + +// Entry represents a single generic item in the report. +type Entry struct { + kind entryKind + message string + line int +} + +// String returns a human-readable representation of the entry. +func (e Entry) String() string { + return fmt.Sprintf("line %d: %s: %s", e.line, e.kind, e.message) +} + +// MarshalJSON satisfies the json.Marshaler interface, returning the entry +// encoded as a JSON object. +func (e Entry) MarshalJSON() ([]byte, error) { + return json.Marshal(map[string]interface{}{ + "kind": e.kind.String(), + "message": e.message, + "line": e.line, + }) +} + +type entryKind int + +const ( + entryError entryKind = iota + entryWarning + entryInfo +) + +func (k entryKind) String() string { + switch k { + case entryError: + return "error" + case entryWarning: + return "warning" + case entryInfo: + return "info" + default: + panic(fmt.Sprintf("invalid kind %d", k)) + } +} diff --git a/config/validate/report_test.go b/config/validate/report_test.go new file mode 100644 index 0000000..c5fdb85 --- /dev/null +++ b/config/validate/report_test.go @@ -0,0 +1,98 @@ +/* + 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 validate + +import ( + "bytes" + "reflect" + "testing" +) + +func TestEntry(t *testing.T) { + tests := []struct { + entry Entry + + str string + json []byte + }{ + { + Entry{entryInfo, "test info", 1}, + "line 1: info: test info", + []byte(`{"kind":"info","line":1,"message":"test info"}`), + }, + { + Entry{entryWarning, "test warning", 1}, + "line 1: warning: test warning", + []byte(`{"kind":"warning","line":1,"message":"test warning"}`), + }, + { + Entry{entryError, "test error", 2}, + "line 2: error: test error", + []byte(`{"kind":"error","line":2,"message":"test error"}`), + }, + } + + for _, tt := range tests { + if str := tt.entry.String(); tt.str != str { + t.Errorf("bad string (%q): want %q, got %q", tt.entry, tt.str, str) + } + json, err := tt.entry.MarshalJSON() + if err != nil { + t.Errorf("bad error (%q): want %v, got %q", tt.entry, nil, err) + } + if !bytes.Equal(tt.json, json) { + t.Errorf("bad JSON (%q): want %q, got %q", tt.entry, tt.json, json) + } + } +} + +func TestReport(t *testing.T) { + type reportFunc struct { + fn func(*Report, int, string) + line int + message string + } + + tests := []struct { + fs []reportFunc + + es []Entry + }{ + { + []reportFunc{ + {(*Report).Warning, 1, "test warning 1"}, + {(*Report).Error, 2, "test error 2"}, + {(*Report).Info, 10, "test info 10"}, + }, + []Entry{ + Entry{entryWarning, "test warning 1", 1}, + Entry{entryError, "test error 2", 2}, + Entry{entryInfo, "test info 10", 10}, + }, + }, + } + + for _, tt := range tests { + r := Report{} + for _, f := range tt.fs { + f.fn(&r, f.line, f.message) + } + if es := r.Entries(); !reflect.DeepEqual(tt.es, es) { + t.Errorf("bad entries (%v): want %#v, got %#v", tt.fs, tt.es, es) + } + } +} diff --git a/config/validate/rules.go b/config/validate/rules.go new file mode 100644 index 0000000..9c29dce --- /dev/null +++ b/config/validate/rules.go @@ -0,0 +1,115 @@ +/* + 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 validate + +import ( + "fmt" + "reflect" + + "github.com/coreos/coreos-cloudinit/config" +) + +type rule func(config node, report *Report) + +// Rules contains all of the validation rules. +var Rules []rule = []rule{ + checkStructure, + checkValidity, +} + +// checkStructure compares the provided config to the empty config.CloudConfig +// structure. Each node is checked to make sure that it exists in the known +// structure and that its type is compatible. +func checkStructure(cfg node, report *Report) { + g := NewNode(config.CloudConfig{}, NewContext([]byte{})) + checkNodeStructure(cfg, g, report) +} + +func checkNodeStructure(n, g node, r *Report) { + if !isCompatible(n.Kind(), g.Kind()) { + r.Warning(n.line, fmt.Sprintf("incorrect type for %q (want %s)", n.name, g.HumanType())) + return + } + + switch g.Kind() { + case reflect.Struct: + for _, cn := range n.children { + if cg := g.Child(cn.name); cg.IsValid() { + checkNodeStructure(cn, cg, r) + } else { + r.Warning(cn.line, fmt.Sprintf("unrecognized key %q", cn.name)) + } + } + case reflect.Slice: + for _, cn := range n.children { + var cg node + c := g.Type().Elem() + toNode(reflect.New(c).Elem().Interface(), context{}, &cg) + checkNodeStructure(cn, cg, r) + } + case reflect.String, reflect.Int, reflect.Bool: + default: + panic(fmt.Sprintf("checkNodeStructure(): unhandled kind %s", g.Kind())) + } +} + +// checkValidity checks the value of every node in the provided config by +// running config.AssertValid() on it. +func checkValidity(cfg node, report *Report) { + g := NewNode(config.CloudConfig{}, NewContext([]byte{})) + checkNodeValidity(cfg, g, report) +} + +func checkNodeValidity(n, g node, r *Report) { + if err := config.AssertValid(n.Value, g.field.Tag.Get("valid")); err != nil { + r.Warning(n.line, fmt.Sprintf("invalid value %v", n.Value)) + } + switch g.Kind() { + case reflect.Struct: + for _, cn := range n.children { + if cg := g.Child(cn.name); cg.IsValid() { + checkNodeValidity(cn, cg, r) + } + } + case reflect.Slice: + for _, cn := range n.children { + var cg node + c := g.Type().Elem() + toNode(reflect.New(c).Elem().Interface(), context{}, &cg) + checkNodeValidity(cn, cg, r) + } + case reflect.String, reflect.Int, reflect.Bool: + default: + panic(fmt.Sprintf("checkNodeValidity(): unhandled kind %s", g.Kind())) + } +} + +// isCompatible determines if the type of kind n can be converted to the type +// of kind g in the context of YAML. This is not an exhaustive list, but its +// enough for the purposes of cloud-config validation. +func isCompatible(n, g reflect.Kind) bool { + switch g { + case reflect.String: + return n == reflect.String || n == reflect.Int || n == reflect.Bool + case reflect.Struct: + return n == reflect.Struct || n == reflect.Map + case reflect.Bool, reflect.Slice: + return n == g + default: + panic(fmt.Sprintf("isCompatible(): unhandled kind %s", g)) + } +} diff --git a/config/validate/rules_test.go b/config/validate/rules_test.go new file mode 100644 index 0000000..e3ec1f6 --- /dev/null +++ b/config/validate/rules_test.go @@ -0,0 +1,251 @@ +/* + 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 validate + +import ( + "reflect" + "testing" +) + +func TestCheckStructure(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + {}, + + // Test for unrecognized keys + { + config: "test:", + entries: []Entry{{entryWarning, "unrecognized key \"test\"", 1}}, + }, + { + config: "coreos:\n etcd:\n bad:", + entries: []Entry{{entryWarning, "unrecognized key \"bad\"", 3}}, + }, + { + config: "coreos:\n etcd:\n discovery: good", + }, + + // Test for error on list of nodes + { + config: "coreos:\n units:\n - hello\n - goodbye", + entries: []Entry{ + {entryWarning, "incorrect type for \"units[0]\" (want struct)", 3}, + {entryWarning, "incorrect type for \"units[1]\" (want struct)", 4}, + }, + }, + + // Test for incorrect types + // Want boolean + { + config: "coreos:\n units:\n - enable: true", + }, + { + config: "coreos:\n units:\n - enable: 4", + entries: []Entry{{entryWarning, "incorrect type for \"enable\" (want bool)", 3}}, + }, + { + config: "coreos:\n units:\n - enable: bad", + entries: []Entry{{entryWarning, "incorrect type for \"enable\" (want bool)", 3}}, + }, + { + config: "coreos:\n units:\n - enable:\n bad:", + entries: []Entry{{entryWarning, "incorrect type for \"enable\" (want bool)", 3}}, + }, + { + config: "coreos:\n units:\n - enable:\n - bad", + entries: []Entry{{entryWarning, "incorrect type for \"enable\" (want bool)", 3}}, + }, + // Want string + { + config: "hostname: true", + }, + { + config: "hostname: 4", + }, + { + config: "hostname: host", + }, + { + config: "hostname:\n name:", + entries: []Entry{{entryWarning, "incorrect type for \"hostname\" (want string)", 1}}, + }, + { + config: "hostname:\n - name", + entries: []Entry{{entryWarning, "incorrect type for \"hostname\" (want string)", 1}}, + }, + // Want struct + { + config: "coreos: true", + entries: []Entry{{entryWarning, "incorrect type for \"coreos\" (want struct)", 1}}, + }, + { + config: "coreos: 4", + entries: []Entry{{entryWarning, "incorrect type for \"coreos\" (want struct)", 1}}, + }, + { + config: "coreos: hello", + entries: []Entry{{entryWarning, "incorrect type for \"coreos\" (want struct)", 1}}, + }, + { + config: "coreos:\n etcd:\n discovery: fire in the disco", + }, + { + config: "coreos:\n - hello", + entries: []Entry{{entryWarning, "incorrect type for \"coreos\" (want struct)", 1}}, + }, + // Want []string + { + config: "ssh_authorized_keys: true", + entries: []Entry{{entryWarning, "incorrect type for \"ssh_authorized_keys\" (want []string)", 1}}, + }, + { + config: "ssh_authorized_keys: 4", + entries: []Entry{{entryWarning, "incorrect type for \"ssh_authorized_keys\" (want []string)", 1}}, + }, + { + config: "ssh_authorized_keys: key", + entries: []Entry{{entryWarning, "incorrect type for \"ssh_authorized_keys\" (want []string)", 1}}, + }, + { + config: "ssh_authorized_keys:\n key: value", + entries: []Entry{{entryWarning, "incorrect type for \"ssh_authorized_keys\" (want []string)", 1}}, + }, + { + config: "ssh_authorized_keys:\n - key", + }, + { + config: "ssh_authorized_keys:\n - key: value", + entries: []Entry{{entryWarning, "incorrect type for \"ssh_authorized_keys[0]\" (want string)", 2}}, + }, + // Want []struct + { + config: "users:\n true", + entries: []Entry{{entryWarning, "incorrect type for \"users\" (want []struct)", 1}}, + }, + { + config: "users:\n 4", + entries: []Entry{{entryWarning, "incorrect type for \"users\" (want []struct)", 1}}, + }, + { + config: "users:\n bad", + entries: []Entry{{entryWarning, "incorrect type for \"users\" (want []struct)", 1}}, + }, + { + config: "users:\n bad:", + entries: []Entry{{entryWarning, "incorrect type for \"users\" (want []struct)", 1}}, + }, + { + config: "users:\n - name: good", + }, + // Want struct within array + { + config: "users:\n - true", + entries: []Entry{{entryWarning, "incorrect type for \"users[0]\" (want struct)", 2}}, + }, + { + config: "users:\n - name: hi\n - true", + entries: []Entry{{entryWarning, "incorrect type for \"users[1]\" (want struct)", 3}}, + }, + { + config: "users:\n - 4", + entries: []Entry{{entryWarning, "incorrect type for \"users[0]\" (want struct)", 2}}, + }, + { + config: "users:\n - bad", + entries: []Entry{{entryWarning, "incorrect type for \"users[0]\" (want struct)", 2}}, + }, + { + config: "users:\n - - bad", + entries: []Entry{{entryWarning, "incorrect type for \"users[0]\" (want struct)", 2}}, + }, + } + + for i, tt := range tests { + r := Report{} + n, err := parseCloudConfig([]byte(tt.config), &r) + if err != nil { + panic(err) + } + checkStructure(n, &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%d, %q): want %#v, got %#v", i, tt.config, tt.entries, e) + } + } +} + +func TestCheckValidity(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + // string + { + config: "hostname: test", + }, + + // int + { + config: "coreos:\n fleet:\n verbosity: 2", + }, + + // bool + { + config: "coreos:\n units:\n - enable: true", + }, + + // slice + { + config: "coreos:\n units:\n - command: start\n - name: stop", + }, + { + config: "coreos:\n units:\n - command: lol", + entries: []Entry{{entryWarning, "invalid value lol", 3}}, + }, + + // struct + { + config: "coreos:\n update:\n reboot_strategy: off", + }, + { + config: "coreos:\n update:\n reboot_strategy: always", + entries: []Entry{{entryWarning, "invalid value always", 3}}, + }, + + // unknown + { + config: "unknown: hi", + }, + } + + for i, tt := range tests { + r := Report{} + n, err := parseCloudConfig([]byte(tt.config), &r) + if err != nil { + panic(err) + } + checkValidity(n, &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%d, %q): want %#v, got %#v", i, tt.config, tt.entries, e) + } + } +} diff --git a/config/validate/validate.go b/config/validate/validate.go new file mode 100644 index 0000000..150eba4 --- /dev/null +++ b/config/validate/validate.go @@ -0,0 +1,113 @@ +/* + 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 validate + +import ( + "errors" + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/coreos/coreos-cloudinit/config" + + "github.com/coreos/coreos-cloudinit/Godeps/_workspace/src/gopkg.in/yaml.v1" +) + +var ( + yamlLineError = regexp.MustCompile(`^YAML error: line (?P[[:digit:]]+): (?P.*)$`) + yamlError = regexp.MustCompile(`^YAML error: (?P.*)$`) +) + +// Validate runs a series of validation tests against the given userdata and +// returns a report detailing all of the issues. Presently, only cloud-configs +// can be validated. +func Validate(userdataBytes []byte) (Report, error) { + switch { + case config.IsScript(string(userdataBytes)): + return Report{}, nil + case config.IsCloudConfig(string(userdataBytes)): + return validateCloudConfig(userdataBytes, Rules) + default: + return Report{entries: []Entry{ + Entry{kind: entryError, message: `must be "#cloud-config" or begin with "#!"`}, + }}, nil + } +} + +// validateCloudConfig runs all of the validation rules in Rules and returns +// the resulting report and any errors encountered. +func validateCloudConfig(config []byte, rules []rule) (report Report, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%v", r) + } + }() + + c, err := parseCloudConfig(config, &report) + if err != nil { + return report, err + } + + c = normalizeNodeNames(c, &report) + for _, r := range rules { + r(c, &report) + } + return report, nil +} + +// parseCloudConfig parses the provided config into a node structure and logs +// any parsing issues into the provided report. Unrecoverable errors are +// returned as an error. +func parseCloudConfig(config []byte, report *Report) (n node, err error) { + var raw map[interface{}]interface{} + if err := yaml.Unmarshal(config, &raw); err != nil { + matches := yamlLineError.FindStringSubmatch(err.Error()) + if len(matches) == 3 { + line, err := strconv.Atoi(matches[1]) + if err != nil { + return n, err + } + msg := matches[2] + report.Error(line, msg) + return n, nil + } + + matches = yamlError.FindStringSubmatch(err.Error()) + if len(matches) == 2 { + report.Error(1, matches[1]) + return n, nil + } + + return n, errors.New("couldn't parse yaml error") + } + + return NewNode(raw, NewContext(config)), nil +} + +// normalizeNodeNames replaces all occurences of '-' with '_' within key names +// and makes a note of each replacement in the report. +func normalizeNodeNames(node node, report *Report) node { + if strings.Contains(node.name, "-") { + report.Info(node.line, fmt.Sprintf("%q uses '-' instead of '_'", node.name)) + node.name = strings.Replace(node.name, "-", "_", -1) + } + for i := range node.children { + node.children[i] = normalizeNodeNames(node.children[i], report) + } + return node +} diff --git a/config/validate/validate_test.go b/config/validate/validate_test.go new file mode 100644 index 0000000..021b1d8 --- /dev/null +++ b/config/validate/validate_test.go @@ -0,0 +1,121 @@ +/* + 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 validate + +import ( + "errors" + "reflect" + "testing" +) + +func TestParseCloudConfig(t *testing.T) { + tests := []struct { + config string + + entries []Entry + }{ + {}, + { + config: " ", + entries: []Entry{{entryError, "found character that cannot start any token", 1}}, + }, + { + config: "a:\na", + entries: []Entry{{entryError, "could not find expected ':'", 2}}, + }, + { + config: "#hello\na:\na", + entries: []Entry{{entryError, "could not find expected ':'", 3}}, + }, + } + + for _, tt := range tests { + r := Report{} + parseCloudConfig([]byte(tt.config), &r) + + if e := r.Entries(); !reflect.DeepEqual(tt.entries, e) { + t.Errorf("bad report (%s): want %#v, got %#v", tt.config, tt.entries, e) + } + } +} + +func TestValidateCloudConfig(t *testing.T) { + tests := []struct { + config string + rules []rule + + report Report + err error + }{ + { + rules: []rule{func(_ node, _ *Report) { panic("something happened") }}, + err: errors.New("something happened"), + }, + } + + for _, tt := range tests { + r, err := validateCloudConfig([]byte(tt.config), tt.rules) + if !reflect.DeepEqual(tt.err, err) { + t.Errorf("bad error (%s): want %v, got %v", tt.config, tt.err, err) + } + if !reflect.DeepEqual(tt.report, r) { + t.Errorf("bad report (%s): want %+v, got %+v", tt.config, tt.report, r) + } + } +} + +func BenchmarkValidate(b *testing.B) { + config := `#cloud-config +hostname: test + +coreos: + etcd: + name: node001 + discovery: https://discovery.etcd.io/disco + addr: $public_ipv4:4001 + peer-addr: $private_ipv4:7001 + fleet: + verbosity: 2 + metadata: "hi" + update: + reboot-strategy: off + units: + - name: hi.service + command: start + enable: true + - name: bye.service + command: stop + +ssh_authorized_keys: + - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC0g+ZTxC7weoIJLUafOgrm+h... + - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC0g+ZTxC7weoIJLUafOgrm+h... + +users: + - name: me + +write_files: + - path: /etc/yes + content: "Hi" + +manage_etc_hosts: localhost` + + for i := 0; i < b.N; i++ { + if _, err := Validate([]byte(config)); err != nil { + panic(err) + } + } +} diff --git a/test b/test index e251d81..38279e1 100755 --- a/test +++ b/test @@ -15,6 +15,7 @@ source ./build declare -a TESTPKGS=( config + config/validate datasource datasource/configdrive datasource/file From dda314b518c5a62ffcffed4c4d43683b0caf81b2 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 15 Sep 2014 15:38:31 -0700 Subject: [PATCH 5/5] flags: add validate flag This will allow the user to run a standalone validation. --- coreos-cloudinit.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/coreos-cloudinit.go b/coreos-cloudinit.go index cb7f879..a482d94 100644 --- a/coreos-cloudinit.go +++ b/coreos-cloudinit.go @@ -24,6 +24,7 @@ import ( "time" "github.com/coreos/coreos-cloudinit/config" + "github.com/coreos/coreos-cloudinit/config/validate" "github.com/coreos/coreos-cloudinit/datasource" "github.com/coreos/coreos-cloudinit/datasource/configdrive" "github.com/coreos/coreos-cloudinit/datasource/file" @@ -64,6 +65,7 @@ var ( workspace string sshKeyName string oem string + validate bool }{} ) @@ -83,6 +85,7 @@ func init() { flag.StringVar(&flags.convertNetconf, "convert-netconf", "", "Read the network config provided in cloud-drive and translate it from the specified format into networkd unit files") flag.StringVar(&flags.workspace, "workspace", "/var/lib/coreos-cloudinit", "Base directory coreos-cloudinit should use to store data") flag.StringVar(&flags.sshKeyName, "ssh-key-name", initialize.DefaultSSHKeyName, "Add SSH keys to the system with the given name") + flag.BoolVar(&flags.validate, "validate", false, "[EXPERIMENTAL] Validate the user-data but do not apply it to the system") } type oemConfig map[string]string @@ -158,6 +161,22 @@ func main() { failure = true } + if report, err := validate.Validate(userdataBytes); err == nil { + ret := 0 + for _, e := range report.Entries() { + fmt.Println(e) + ret = 1 + } + if flags.validate { + os.Exit(ret) + } + } else { + fmt.Printf("Failed while validating user_data (%q)\n", err) + if flags.validate { + os.Exit(1) + } + } + fmt.Printf("Fetching meta-data from datasource of type %q\n", ds.Type()) metadataBytes, err := ds.FetchMetadata() if err != nil {