config: use a YAML transform to normalize keys

This removes the problematic two-pass unmarshalling.

config/config.go

@@ -63,15 +63,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
-	}
-	if err = yaml.Unmarshal(ncontents, &cfg); err != nil {
-		return &cfg, err
-	}
-	return &cfg, nil
 }
 
 func (cc CloudConfig) String() string {
@@ -159,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

@@ -22,6 +22,42 @@ import (
 	"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"}}},
+		},
+	}
+
+	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) {
 	tests := []struct {
 		c interface{}
@@ -251,40 +287,6 @@ Address=10.209.171.177/19
 	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
@@ -451,31 +453,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)
-		}
-	}
-}