Merge pull request #90 from jonboulle/90

Warn or error on unrecognized keys in cloud-config.yml

initialize/config.go

@@ -43,11 +43,91 @@ type CloudConfig struct {
 	ManageEtcHosts EtcHosts `yaml:"manage_etc_hosts"`
 }
 
+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, _ := goyaml.Marshal(&CloudConfig{})
+	goyaml.Unmarshal(b, &cc)
+
+	// Now unmarshal the entire provided contents
+	var c map[string]interface{}
+	goyaml.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
+	coreos, ok := c["coreos"]
+	if ok {
+		set := coreos.(map[interface{}]interface{})
+		known := cc["coreos"].(map[interface{}]interface{})
+		for k, _ := range set {
+			key := k.(string)
+			if _, ok := known[key]; !ok {
+				warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", key)
+			}
+		}
+	}
+
+	// Check for any badly-specified users, if any are set
+	users, ok := c["users"]
+	if ok {
+		var known map[string]interface{}
+		b, _ := goyaml.Marshal(&system.User{})
+		goyaml.Unmarshal(b, &known)
+
+		set := users.([]interface{})
+		for _, u := range set {
+			user := u.(map[interface{}]interface{})
+			for k, _ := range user {
+				key := k.(string)
+				if _, ok := known[key]; !ok {
+					warn("Warning: unrecognized key %q in user section of cloud config - ignoring", key)
+				}
+			}
+		}
+	}
+
+	// Check for any badly-specified files, if any are set
+	files, ok := c["write_files"]
+	if ok {
+		var known map[string]interface{}
+		b, _ := goyaml.Marshal(&system.File{})
+		goyaml.Unmarshal(b, &known)
+
+		set := files.([]interface{})
+		for _, f := range set {
+			file := f.(map[interface{}]interface{})
+			for k, _ := range file {
+				key := k.(string)
+				if _, ok := known[key]; !ok {
+					warn("Warning: unrecognized key %q in file section of cloud config - ignoring", key)
+				}
+			}
+		}
+	}
+}
+
+// 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.
 func NewCloudConfig(contents string) (*CloudConfig, error) {
 	var cfg CloudConfig
 	err := goyaml.Unmarshal([]byte(contents), &cfg)
+	if err != nil {
 		return &cfg, err
 	}
+	warnOnUnrecognizedKeys(contents, log.Printf)
+	return &cfg, nil
+}
 
 func (cc CloudConfig) String() string {
 	bytes, err := goyaml.Marshal(cc)

initialize/config_test.go

@@ -1,10 +1,75 @@
 package initialize
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 )
 
+func TestCloudConfigUnknownKeys(t *testing.T) {
+	contents := `
+coreos: 
+  etcd:
+    discovery: "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877"
+  coreos_unknown:
+    foo: "bar"
+section_unknown:
+  dunno:
+    something
+bare_unknown:
+  bar
+write_files:
+  - content: fun
+    path: /var/party
+    file_unknown: nofun
+users:
+  - name: fry
+    passwd: somehash
+    user_unknown: philip
+hostname:
+  foo
+`
+	cfg, err := NewCloudConfig(contents)
+	if err != nil {
+		t.Fatalf("error instantiating CloudConfig with unknown keys: %v", err)
+	}
+	if cfg.Hostname != "foo" {
+		t.Fatalf("hostname not correctly set when invalid keys are present")
+	}
+	if len(cfg.Coreos.Etcd) < 1 {
+		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" {
+		t.Fatalf("write_files section not correctly set when invalid keys are present")
+	}
+	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"
 func TestCloudConfigEmpty(t *testing.T) {
 	cfg, err := NewCloudConfig("")