Merge pull request #290 from crawford/yaml

Improved YAML parsing

Godeps/Godeps.json

@@ -15,7 +15,7 @@
 		},
 		{
 			"ImportPath": "github.com/coreos/yaml",
-			"Rev": "9f9df34309c04878acc86042b16630b0f696e1de"
+			"Rev": "6b16a5714269b2f70720a45406b1babd947a17ef"
 		},
 		{
 			"ImportPath": "github.com/dotcloud/docker/pkg/netlink",

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

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
+	parser    yaml_parser_t
-	event  yaml_event_t
+	event     yaml_event_t
-	doc    *node
+	doc       *node
+	transform transformString
 }
 
-func newParser(b []byte) *parser {
+func newParser(b []byte, t transformString) *parser {
-	p := 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

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:

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

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
 

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 {
+	CoreOS            CoreOS   `yaml:"coreos"`
-		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"`
 	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)
+	err := yaml.Unmarshal([]byte(contents), &cfg)
-	if err != nil {
+	return &cfg, err
-		return &cfg, err
-	}
-	if err = yaml.Unmarshal(ncontents, &cfg); err != nil {
-		return &cfg, err
-	}
-	return &cfg, nil
 }
 
 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, ",")
+	if m, _ := regexp.MatchString(valid, vs); m {
-	for _, valid := range valids {
+		return nil
-		if vs == valid {
-			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
-}

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 {
+	tests := []struct {
-		c     interface{}
+		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 {
+	tests := []struct {
-		c   interface{}
+		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, b string `valid:"^1|2$"`
-		}{A: "hello", b: "2"}, &ErrorValid{Value: "hello", Field: "A", Valid: []string{"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, b int `valid:"^1|2$"`
-		}{A: 9, b: 2}, &ErrorValid{Value: "9", Field: "A", Valid: []string{"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) {
+func TestConfigCompile(t *testing.T) {
-	defer func() {
+	tests := []interface{}{
-		if r := recover(); r != nil {
+		Etcd{},
-			t.Fatalf("panic while instantiating CloudConfig with nil keys: %v", r)
+		File{},
-		}
+		Flannel{},
-	}()
+		Fleet{},
+		Locksmith{},
+		OEM{},
+		Unit{},
+		Update{},
+	}
 
-	for _, tt := range []struct {
+	for _, tt := range tests {
-		contents string
+		ttt := reflect.TypeOf(tt)
-	}{
+		for i := 0; i < ttt.NumField(); i++ {
-		{"coreos:"},
+			ft := ttt.Field(i)
-		{"ssh_authorized_keys:"},
+			if !isFieldExported(ft) {
-		{"ssh_authorized_keys:\n  -"},
+				continue
-		{"ssh_authorized_keys:\n  - 0:"},
+			}
-		{"write_files:"},
+
-		{"write_files:\n  -"},
+			if _, err := regexp.Compile(ft.Tag.Get("valid")); err != nil {
-		{"write_files:\n  - 0:"},
+				t.Errorf("bad regexp(%s.%s): want %v, got %s", ttt.Name(), ft.Name, nil, err)
-		{"users:"},
+			}
-		{"users:\n  -"},
-		{"users:\n  - 0:"},
-	} {
-		_, err := NewCloudConfig(tt.contents)
-		if err != nil {
-			t.Fatalf("error instantiating CloudConfig with invalid keys: %v", 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" {
+	if cfg.CoreOS.OEM.ID != "rackspace" {
-		t.Errorf("Failed parsing coreos.oem. Expected ID 'rackspace', got %q.", cfg.Coreos.OEM.ID)
+		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)
-		}
-	}
-}

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}$"`
 }

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)
+		}
+	}
+}

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"`
 }
 

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)
+		}
+	}
+}

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"`
 }

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)
+		}
+	}
+}

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:

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) {
+func parseCloudConfig(cfg []byte, report *Report) (node, error) {
-	var raw map[interface{}]interface{}
+	yaml.UnmarshalMappingKeyTransform = func(nameIn string) (nameOut string) {
-	if err := yaml.Unmarshal(config, &raw); err != nil {
+		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

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 {

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.OEM{OEM: cfg.CoreOS.OEM},
-		system.Update{Update: cfg.Coreos.Update, ReadConfig: system.DefaultReadConfig},
+		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.Etcd{Etcd: cfg.CoreOS.Etcd},
-		system.Fleet{Fleet: cfg.Coreos.Fleet},
+		system.Fleet{Fleet: cfg.CoreOS.Fleet},
-		system.Locksmith{Locksmith: cfg.Coreos.Locksmith},
+		system.Locksmith{Locksmith: cfg.CoreOS.Locksmith},
-		system.Update{Update: cfg.Coreos.Update, ReadConfig: system.DefaultReadConfig},
+		system.Update{Update: cfg.CoreOS.Update, ReadConfig: system.DefaultReadConfig},
 	} {
 		units = append(units, ccu.Units()...)
 	}

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)
 	}

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)

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
 		}

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{