diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index ee26a08..77a58e9 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -15,7 +15,7 @@ }, { "ImportPath": "github.com/coreos/yaml", - "Rev": "9f9df34309c04878acc86042b16630b0f696e1de" + "Rev": "6b16a5714269b2f70720a45406b1babd947a17ef" }, { "ImportPath": "github.com/dotcloud/docker/pkg/netlink", diff --git a/Godeps/_workspace/src/github.com/coreos/yaml/README.md b/Godeps/_workspace/src/github.com/coreos/yaml/README.md index af07056..4427005 100644 --- a/Godeps/_workspace/src/github.com/coreos/yaml/README.md +++ b/Godeps/_workspace/src/github.com/coreos/yaml/README.md @@ -1,3 +1,6 @@ +Note: This is a fork of https://github.com/go-yaml/yaml. The following README +doesn't necessarily apply to this fork. + # YAML support for the Go language Introduction diff --git a/Godeps/_workspace/src/github.com/coreos/yaml/decode.go b/Godeps/_workspace/src/github.com/coreos/yaml/decode.go index a098626..e219d4b 100644 --- a/Godeps/_workspace/src/github.com/coreos/yaml/decode.go +++ b/Godeps/_workspace/src/github.com/coreos/yaml/decode.go @@ -30,13 +30,15 @@ type node struct { // Parser, produces a node tree out of a libyaml event stream. type parser struct { - parser yaml_parser_t - event yaml_event_t - doc *node + parser yaml_parser_t + event yaml_event_t + doc *node + transform transformString } -func newParser(b []byte) *parser { - p := parser{} +func newParser(b []byte, t transformString) *parser { + p := parser{transform: t} + if !yaml_parser_initialize(&p.parser) { panic("Failed to initialize YAML emitter") } @@ -175,7 +177,10 @@ func (p *parser) mapping() *node { p.anchor(n, p.event.anchor) p.skip() for p.event.typ != yaml_MAPPING_END_EVENT { - n.children = append(n.children, p.parse(), p.parse()) + key := p.parse() + key.value = p.transform(key.value) + value := p.parse() + n.children = append(n.children, key, value) } p.skip() return n diff --git a/Godeps/_workspace/src/github.com/coreos/yaml/decode_test.go b/Godeps/_workspace/src/github.com/coreos/yaml/decode_test.go index ef3d37f..349bee7 100644 --- a/Godeps/_workspace/src/github.com/coreos/yaml/decode_test.go +++ b/Godeps/_workspace/src/github.com/coreos/yaml/decode_test.go @@ -1,8 +1,8 @@ package yaml_test import ( + "github.com/coreos/yaml" . "gopkg.in/check.v1" - "gopkg.in/yaml.v1" "math" "reflect" "strings" @@ -557,6 +557,23 @@ func (s *S) TestUnmarshalWithFalseSetterIgnoresValue(c *C) { c.Assert(m["ghi"].value, Equals, 3) } +func (s *S) TestUnmarshalWithTransform(c *C) { + data := `{a_b: 1, c-d: 2, e-f_g: 3, h_i-j: 4}` + expect := map[string]int{ + "a_b": 1, + "c_d": 2, + "e_f_g": 3, + "h_i_j": 4, + } + m := map[string]int{} + yaml.UnmarshalMappingKeyTransform = func(i string) string { + return strings.Replace(i, "-", "_", -1) + } + err := yaml.Unmarshal([]byte(data), m) + c.Assert(err, IsNil) + c.Assert(m, DeepEquals, expect) +} + // From http://yaml.org/type/merge.html var mergeTests = ` anchors: diff --git a/Godeps/_workspace/src/github.com/coreos/yaml/encode_test.go b/Godeps/_workspace/src/github.com/coreos/yaml/encode_test.go index c9febc2..2cd0ea7 100644 --- a/Godeps/_workspace/src/github.com/coreos/yaml/encode_test.go +++ b/Godeps/_workspace/src/github.com/coreos/yaml/encode_test.go @@ -7,8 +7,8 @@ import ( "strings" "time" + "github.com/coreos/yaml" . "gopkg.in/check.v1" - "gopkg.in/yaml.v1" ) var marshalIntTest = 123 diff --git a/Godeps/_workspace/src/github.com/coreos/yaml/yaml.go b/Godeps/_workspace/src/github.com/coreos/yaml/yaml.go index f1c390e..16e1365 100644 --- a/Godeps/_workspace/src/github.com/coreos/yaml/yaml.go +++ b/Godeps/_workspace/src/github.com/coreos/yaml/yaml.go @@ -84,7 +84,7 @@ type Getter interface { func Unmarshal(in []byte, out interface{}) (err error) { defer handleErr(&err) d := newDecoder() - p := newParser(in) + p := newParser(in, UnmarshalMappingKeyTransform) defer p.destroy() node := p.parse() if node != nil { @@ -146,6 +146,17 @@ func Marshal(in interface{}) (out []byte, err error) { return } +// UnmarshalMappingKeyTransform is a string transformation that is applied to +// each mapping key in a YAML document before it is unmarshalled. By default, +// UnmarshalMappingKeyTransform is an identity transform (no modification). +var UnmarshalMappingKeyTransform transformString = identityTransform + +type transformString func(in string) (out string) + +func identityTransform(in string) (out string) { + return in +} + // -------------------------------------------------------------------------- // Maintain a mapping of keys to structure field indexes diff --git a/config/config.go b/config/config.go index 9278ecc..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" @@ -29,15 +30,7 @@ import ( // used for internal use) have the YAML tag '-' so that they aren't marshalled. type CloudConfig struct { SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"` - Coreos struct { - Etcd Etcd `yaml:"etcd"` - Flannel Flannel `yaml:"flannel"` - Fleet Fleet `yaml:"fleet"` - Locksmith Locksmith `yaml:"locksmith"` - OEM OEM `yaml:"oem"` - Update Update `yaml:"update"` - Units []Unit `yaml:"units"` - } `yaml:"coreos"` + CoreOS CoreOS `yaml:"coreos"` WriteFiles []File `yaml:"write_files"` Hostname string `yaml:"hostname"` Users []User `yaml:"users"` @@ -46,6 +39,16 @@ type CloudConfig struct { NetworkConfig string `yaml:"-"` } +type CoreOS struct { + Etcd Etcd `yaml:"etcd"` + Flannel Flannel `yaml:"flannel"` + Fleet Fleet `yaml:"fleet"` + Locksmith Locksmith `yaml:"locksmith"` + OEM OEM `yaml:"oem"` + Update Update `yaml:"update"` + Units []Unit `yaml:"units"` +} + func IsCloudConfig(userdata string) bool { header := strings.SplitN(userdata, "\n", 2)[0] @@ -61,15 +64,12 @@ func IsCloudConfig(userdata string) bool { // string of YAML), returning any error encountered. It will ignore unknown // fields but log encountering them. func NewCloudConfig(contents string) (*CloudConfig, error) { + yaml.UnmarshalMappingKeyTransform = func(nameIn string) (nameOut string) { + return strings.Replace(nameIn, "-", "_", -1) + } var cfg CloudConfig - ncontents, err := normalizeConfig(contents) - if err != nil { - return &cfg, err - } - if err = yaml.Unmarshal(ncontents, &cfg); err != nil { - return &cfg, err - } - return &cfg, nil + err := yaml.Unmarshal([]byte(contents), &cfg) + return &cfg, err } func (cc CloudConfig) String() string { @@ -92,7 +92,7 @@ func IsZero(c interface{}) bool { type ErrorValid struct { Value string - Valid []string + Valid string Field string } @@ -126,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, } } @@ -157,31 +156,3 @@ func isZero(v reflect.Value) bool { func isFieldExported(f reflect.StructField) bool { return f.PkgPath == "" } - -func normalizeConfig(config string) ([]byte, error) { - var cfg map[interface{}]interface{} - if err := yaml.Unmarshal([]byte(config), &cfg); err != nil { - return nil, err - } - return yaml.Marshal(normalizeKeys(cfg)) -} - -func normalizeKeys(m map[interface{}]interface{}) map[interface{}]interface{} { - for k, v := range m { - if m, ok := m[k].(map[interface{}]interface{}); ok { - normalizeKeys(m) - } - - if s, ok := m[k].([]interface{}); ok { - for _, e := range s { - if m, ok := e.(map[interface{}]interface{}); ok { - normalizeKeys(m) - } - } - } - - delete(m, k) - m[strings.Replace(fmt.Sprint(k), "-", "_", -1)] = v - } - return m -} diff --git a/config/config_test.go b/config/config_test.go index 29171ad..0eaa4de 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -18,13 +18,67 @@ package config import ( "reflect" + "regexp" "strings" "testing" ) +func TestNewCloudConfig(t *testing.T) { + tests := []struct { + contents string + + config CloudConfig + }{ + {}, + { + contents: "#cloud-config\nwrite_files:\n - path: underscore", + config: CloudConfig{WriteFiles: []File{File{Path: "underscore"}}}, + }, + { + contents: "#cloud-config\nwrite-files:\n - path: hyphen", + config: CloudConfig{WriteFiles: []File{File{Path: "hyphen"}}}, + }, + { + contents: "#cloud-config\ncoreos:\n update:\n reboot-strategy: off", + config: CloudConfig{CoreOS: CoreOS{Update: Update{RebootStrategy: "off"}}}, + }, + { + contents: "#cloud-config\ncoreos:\n update:\n reboot-strategy: false", + config: CloudConfig{CoreOS: CoreOS{Update: Update{RebootStrategy: "false"}}}, + }, + { + contents: "#cloud-config\nwrite_files:\n - permissions: 0744", + config: CloudConfig{WriteFiles: []File{File{RawFilePermissions: "0744"}}}, + }, + { + contents: "#cloud-config\nwrite_files:\n - permissions: 744", + config: CloudConfig{WriteFiles: []File{File{RawFilePermissions: "744"}}}, + }, + { + contents: "#cloud-config\nwrite_files:\n - permissions: '0744'", + config: CloudConfig{WriteFiles: []File{File{RawFilePermissions: "0744"}}}, + }, + { + contents: "#cloud-config\nwrite_files:\n - permissions: '744'", + config: CloudConfig{WriteFiles: []File{File{RawFilePermissions: "744"}}}, + }, + } + + for i, tt := range tests { + config, err := NewCloudConfig(tt.contents) + if err != nil { + t.Errorf("bad error (test case #%d): want %v, got %s", i, nil, err) + } + if !reflect.DeepEqual(&tt.config, config) { + t.Errorf("bad config (test case #%d): want %#v, got %#v", i, tt.config, config) + } + } +} + func TestIsZero(t *testing.T) { - for _, tt := range []struct { - c interface{} + tests := []struct { + c interface{} + empty bool }{ {struct{}{}, true}, @@ -34,7 +88,9 @@ func TestIsZero(t *testing.T) { {struct{ A string }{A: "hello"}, false}, {struct{ A int }{}, true}, {struct{ A int }{A: 1}, false}, - } { + } + + for _, tt := range tests { if empty := IsZero(tt.c); tt.empty != empty { t.Errorf("bad result (%q): want %t, got %t", tt.c, tt.empty, empty) } @@ -42,66 +98,68 @@ func TestIsZero(t *testing.T) { } func TestAssertStructValid(t *testing.T) { - for _, tt := range []struct { - c interface{} + tests := []struct { + c interface{} + err error }{ {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 { if err := AssertStructValid(tt.c); !reflect.DeepEqual(tt.err, err) { t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.err, err) } } } -func TestCloudConfigInvalidKeys(t *testing.T) { - defer func() { - if r := recover(); r != nil { - t.Fatalf("panic while instantiating CloudConfig with nil keys: %v", r) - } - }() +func TestConfigCompile(t *testing.T) { + tests := []interface{}{ + Etcd{}, + File{}, + Flannel{}, + Fleet{}, + Locksmith{}, + OEM{}, + Unit{}, + Update{}, + } - for _, tt := range []struct { - contents string - }{ - {"coreos:"}, - {"ssh_authorized_keys:"}, - {"ssh_authorized_keys:\n -"}, - {"ssh_authorized_keys:\n - 0:"}, - {"write_files:"}, - {"write_files:\n -"}, - {"write_files:\n - 0:"}, - {"users:"}, - {"users:\n -"}, - {"users:\n - 0:"}, - } { - _, err := NewCloudConfig(tt.contents) - if err != nil { - t.Fatalf("error instantiating CloudConfig with invalid keys: %v", err) + 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) + } } } } @@ -136,7 +194,7 @@ hostname: if cfg.Hostname != "foo" { t.Fatalf("hostname not correctly set when invalid keys are present") } - if cfg.Coreos.Etcd.Discovery != "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" { + if cfg.CoreOS.Etcd.Discovery != "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" { t.Fatalf("etcd section not correctly set when invalid keys are present") } if len(cfg.WriteFiles) < 1 || cfg.WriteFiles[0].Content != "fun" || cfg.WriteFiles[0].Path != "/var/party" { @@ -242,10 +300,10 @@ hostname: trontastic } } - if len(cfg.Coreos.Units) != 1 { + if len(cfg.CoreOS.Units) != 1 { t.Error("Failed to parse correct number of units") } else { - u := cfg.Coreos.Units[0] + u := cfg.CoreOS.Units[0] expect := `[Match] Name=eth47 @@ -263,50 +321,16 @@ Address=10.209.171.177/19 } } - if cfg.Coreos.OEM.ID != "rackspace" { - t.Errorf("Failed parsing coreos.oem. Expected ID 'rackspace', got %q.", cfg.Coreos.OEM.ID) + if cfg.CoreOS.OEM.ID != "rackspace" { + t.Errorf("Failed parsing coreos.oem. Expected ID 'rackspace', got %q.", cfg.CoreOS.OEM.ID) } if cfg.Hostname != "trontastic" { t.Errorf("Failed to parse hostname") } - if cfg.Coreos.Update.RebootStrategy != "reboot" { + if cfg.CoreOS.Update.RebootStrategy != "reboot" { t.Errorf("Failed to parse locksmith strategy") } - - contents = ` -coreos: -write_files: - - path: /home/me/notes - permissions: 0744 -` - cfg, err = NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error :%v", err) - } - - if len(cfg.WriteFiles) != 1 { - t.Error("Failed to parse correct number of write_files") - } else { - wf := cfg.WriteFiles[0] - if wf.Content != "" { - t.Errorf("WriteFile has incorrect contents '%s'", wf.Content) - } - if wf.Encoding != "" { - t.Errorf("WriteFile has incorrect encoding %s", wf.Encoding) - } - // Verify that the normalization of the config converted 0744 to its decimal - // representation, 484. - if wf.RawFilePermissions != "484" { - t.Errorf("WriteFile has incorrect permissions %s", wf.RawFilePermissions) - } - if wf.Path != "/home/me/notes" { - t.Errorf("WriteFile has incorrect path %s", wf.Path) - } - if wf.Owner != "" { - t.Errorf("WriteFile has incorrect owner %s", wf.Owner) - } - } } // Assert that our interface conversion doesn't panic @@ -473,31 +497,3 @@ users: t.Errorf("ssh import url is %q, expected 'https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys'", user.SSHImportURL) } } - -func TestNormalizeKeys(t *testing.T) { - for _, tt := range []struct { - in string - out string - }{ - {"my_key_name: the-value\n", "my_key_name: the-value\n"}, - {"my-key_name: the-value\n", "my_key_name: the-value\n"}, - {"my-key-name: the-value\n", "my_key_name: the-value\n"}, - - {"a:\n- key_name: the-value\n", "a:\n- key_name: the-value\n"}, - {"a:\n- key-name: the-value\n", "a:\n- key_name: the-value\n"}, - - {"a:\n b:\n - key_name: the-value\n", "a:\n b:\n - key_name: the-value\n"}, - {"a:\n b:\n - key-name: the-value\n", "a:\n b:\n - key_name: the-value\n"}, - - {"coreos:\n update:\n reboot-strategy: off\n", "coreos:\n update:\n reboot_strategy: false\n"}, - {"coreos:\n update:\n reboot-strategy: 'off'\n", "coreos:\n update:\n reboot_strategy: \"off\"\n"}, - } { - out, err := normalizeConfig(tt.in) - if err != nil { - t.Fatalf("bad error (%q): want nil, got %s", tt.in, err) - } - if string(out) != tt.out { - t.Fatalf("bad normalization (%q): want %q, got %q", tt.in, tt.out, out) - } - } -} diff --git a/config/file.go b/config/file.go index 26dbc99..76b5827 100644 --- a/config/file.go +++ b/config/file.go @@ -17,9 +17,9 @@ 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"` - RawFilePermissions string `yaml:"permissions"` + RawFilePermissions string `yaml:"permissions" valid:"^0?[0-7]{3,4}$"` } diff --git a/config/file_test.go b/config/file_test.go new file mode 100644 index 0000000..e3f0871 --- /dev/null +++ b/config/file_test.go @@ -0,0 +1,71 @@ +/* + 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) + } + } +} + +func TestRawFilePermissionsValid(t *testing.T) { + tests := []struct { + value string + + isValid bool + }{ + {value: "744", isValid: true}, + {value: "0744", isValid: true}, + {value: "1744", isValid: true}, + {value: "01744", isValid: true}, + {value: "11744", isValid: false}, + {value: "rwxr--r--", isValid: false}, + {value: "800", isValid: false}, + } + + for _, tt := range tests { + isValid := (nil == AssertStructValid(File{RawFilePermissions: 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 723f244..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,false"` + 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/config/validate/rules.go b/config/validate/rules.go index 7651819..6da2772 100644 --- a/config/validate/rules.go +++ b/config/validate/rules.go @@ -76,7 +76,7 @@ func checkValidity(cfg node, report *Report) { func checkNodeValidity(n, g node, r *Report) { if err := config.AssertValid(n.Value, g.field.Tag.Get("valid")); err != nil { - r.Error(n.line, fmt.Sprintf("invalid value %v", n.Value)) + r.Error(n.line, fmt.Sprintf("invalid value %v", n.Value.Interface())) } switch g.Kind() { case reflect.Struct: diff --git a/config/validate/validate.go b/config/validate/validate.go index 26ad0b5..02a2baa 100644 --- a/config/validate/validate.go +++ b/config/validate/validate.go @@ -65,7 +65,6 @@ func validateCloudConfig(config []byte, rules []rule) (report Report, err error) return report, err } - c = normalizeNodeNames(c, &report) for _, r := range rules { r(c, &report) } @@ -75,30 +74,79 @@ func validateCloudConfig(config []byte, rules []rule) (report Report, err error) // 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 { +func parseCloudConfig(cfg []byte, report *Report) (node, error) { + yaml.UnmarshalMappingKeyTransform = func(nameIn string) (nameOut string) { + return nameIn + } + // unmarshal the config into an implicitly-typed form. The yaml library + // will implicitly convert types into their normalized form + // (e.g. 0744 -> 484, off -> false). + var weak map[interface{}]interface{} + if err := yaml.Unmarshal(cfg, &weak); err != nil { matches := yamlLineError.FindStringSubmatch(err.Error()) if len(matches) == 3 { line, err := strconv.Atoi(matches[1]) if err != nil { - return n, err + return node{}, err } msg := matches[2] report.Error(line, msg) - return n, nil + return node{}, nil } matches = yamlError.FindStringSubmatch(err.Error()) if len(matches) == 2 { report.Error(1, matches[1]) - return n, nil + return node{}, nil } - return n, errors.New("couldn't parse yaml error") + return node{}, errors.New("couldn't parse yaml error") + } + w := NewNode(weak, NewContext(cfg)) + w = normalizeNodeNames(w, report) + + // unmarshal the config into the explicitly-typed form. + yaml.UnmarshalMappingKeyTransform = func(nameIn string) (nameOut string) { + return strings.Replace(nameIn, "-", "_", -1) + } + var strong config.CloudConfig + if err := yaml.Unmarshal([]byte(cfg), &strong); err != nil { + return node{}, err + } + s := NewNode(strong, NewContext(cfg)) + + // coerceNodes weak nodes and strong nodes. strong nodes replace weak nodes + // if they are compatible types (this happens when the yaml library + // converts the input). + // (e.g. weak 484 is replaced by strong 0744, weak 4 is not replaced by + // strong false) + return coerceNodes(w, s), nil +} + +// coerceNodes recursively evaluates two nodes, returning a new node containing +// either the weak or strong node's value and its recursively processed +// children. The strong node's value is used if the two nodes are leafs, are +// both valid, and are compatible types (defined by isCompatible()). The weak +// node is returned in all other cases. coerceNodes is used to counteract the +// effects of yaml's automatic type conversion. The weak node is the one +// resulting from unmarshalling into an empty interface{} (the type is +// inferred). The strong node is the one resulting from unmarshalling into a +// struct. If the two nodes are of compatible types, the yaml library correctly +// parsed the value into the strongly typed unmarshalling. In this case, we +// prefer the strong node because its actually the type we are expecting. +func coerceNodes(w, s node) node { + n := w + n.children = nil + if len(w.children) == 0 && len(s.children) == 0 && + w.IsValid() && s.IsValid() && + isCompatible(w.Kind(), s.Kind()) { + n.Value = s.Value } - return NewNode(raw, NewContext(config)), nil + for _, cw := range w.children { + n.children = append(n.children, coerceNodes(cw, s.Child(cw.name))) + } + return n } // normalizeNodeNames replaces all occurences of '-' with '_' within key names diff --git a/config/validate/validate_test.go b/config/validate/validate_test.go index b390eda..328878b 100644 --- a/config/validate/validate_test.go +++ b/config/validate/validate_test.go @@ -65,6 +65,31 @@ func TestValidateCloudConfig(t *testing.T) { rules: []rule{func(_ node, _ *Report) { panic("something happened") }}, err: errors.New("something happened"), }, + { + config: "write_files:\n - permissions: 0744", + rules: Rules, + }, + { + config: "write_files:\n - permissions: '0744'", + rules: Rules, + }, + { + config: "write_files:\n - permissions: 744", + rules: Rules, + }, + { + config: "write_files:\n - permissions: '744'", + rules: Rules, + }, + { + config: "coreos:\n update:\n reboot-strategy: off", + rules: Rules, + }, + { + config: "coreos:\n update:\n reboot-strategy: false", + rules: Rules, + report: Report{entries: []Entry{{entryError, "invalid value false", 3}}}, + }, } for _, tt := range tests { diff --git a/initialize/config.go b/initialize/config.go index 99e18c1..034ba97 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -110,10 +110,10 @@ func Apply(cfg config.CloudConfig, env *Environment) error { } for _, ccf := range []CloudConfigFile{ - system.OEM{OEM: cfg.Coreos.OEM}, - system.Update{Update: cfg.Coreos.Update, ReadConfig: system.DefaultReadConfig}, + system.OEM{OEM: cfg.CoreOS.OEM}, + system.Update{Update: cfg.CoreOS.Update, ReadConfig: system.DefaultReadConfig}, system.EtcHosts{EtcHosts: cfg.ManageEtcHosts}, - system.Flannel{Flannel: cfg.Coreos.Flannel}, + system.Flannel{Flannel: cfg.CoreOS.Flannel}, } { f, err := ccf.File() if err != nil { @@ -125,15 +125,15 @@ func Apply(cfg config.CloudConfig, env *Environment) error { } var units []system.Unit - for _, u := range cfg.Coreos.Units { + for _, u := range cfg.CoreOS.Units { units = append(units, system.Unit{Unit: u}) } for _, ccu := range []CloudConfigUnit{ - system.Etcd{Etcd: cfg.Coreos.Etcd}, - system.Fleet{Fleet: cfg.Coreos.Fleet}, - system.Locksmith{Locksmith: cfg.Coreos.Locksmith}, - system.Update{Update: cfg.Coreos.Update, ReadConfig: system.DefaultReadConfig}, + system.Etcd{Etcd: cfg.CoreOS.Etcd}, + system.Fleet{Fleet: cfg.CoreOS.Fleet}, + system.Locksmith{Locksmith: cfg.CoreOS.Locksmith}, + system.Update{Update: cfg.CoreOS.Update, ReadConfig: system.DefaultReadConfig}, } { units = append(units, ccu.Units()...) } diff --git a/system/file.go b/system/file.go index 312caf4..22118c1 100644 --- a/system/file.go +++ b/system/file.go @@ -43,7 +43,7 @@ func (f *File) Permissions() (os.FileMode, error) { } // Parse string representation of file mode as integer - perm, err := strconv.ParseInt(f.RawFilePermissions, 0, 32) + perm, err := strconv.ParseInt(f.RawFilePermissions, 8, 32) if err != nil { return 0, fmt.Errorf("Unable to parse file permissions %q as integer", f.RawFilePermissions) } diff --git a/system/file_test.go b/system/file_test.go index 8bd441b..eb6f947 100644 --- a/system/file_test.go +++ b/system/file_test.go @@ -97,7 +97,7 @@ func TestDecimalFilePermissions(t *testing.T) { wf := File{config.File{ Path: fn, - RawFilePermissions: "484", // Decimal representation of 0744 + RawFilePermissions: "744", }} path, err := WriteFile(&wf, dir) diff --git a/system/update.go b/system/update.go index bd745da..dd21515 100644 --- a/system/update.go +++ b/system/update.go @@ -126,7 +126,7 @@ func (uc Update) Units() []Unit { Runtime: true, }} - if uc.Update.RebootStrategy == "false" || uc.Update.RebootStrategy == "off" { + if uc.Update.RebootStrategy == "off" { ls.Command = "stop" ls.Mask = true } diff --git a/system/update_test.go b/system/update_test.go index 4ebf70b..deeb66f 100644 --- a/system/update_test.go +++ b/system/update_test.go @@ -71,15 +71,6 @@ func TestUpdateUnits(t *testing.T) { Runtime: true, }}}, }, - { - config: config.Update{RebootStrategy: "false"}, - units: []Unit{{config.Unit{ - Name: "locksmithd.service", - Command: "stop", - Runtime: true, - Mask: true, - }}}, - }, { config: config.Update{RebootStrategy: "off"}, units: []Unit{{config.Unit{ @@ -109,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", "false"}}, + err: &config.ErrorValid{Value: "wizzlewazzle", Field: "RebootStrategy", Valid: "^(best-effort|etcd-lock|reboot|off)$"}, }, { config: config.Update{Group: "master", Server: "http://foo.com"}, @@ -143,14 +134,6 @@ func TestUpdateFile(t *testing.T) { RawFilePermissions: "0644", }}, }, - { - config: config.Update{RebootStrategy: "false"}, - file: &File{config.File{ - Content: "REBOOT_STRATEGY=false\n", - Path: "etc/coreos/update.conf", - RawFilePermissions: "0644", - }}, - }, { config: config.Update{RebootStrategy: "off"}, file: &File{config.File{