Merge pull request #166 from jonboulle/merge

cloudinit: merge cloudconfig info from user-data and meta-data

coreos-cloudinit.go

@@ -102,6 +102,7 @@ func main() {
 		die()
 	}
 
+	// Extract IPv4 addresses from metadata if possible
 	var subs map[string]string
 	if len(metadataBytes) > 0 {
 		subs, err = initialize.ExtractIPsFromMetadata(metadataBytes)
@@ -110,29 +111,86 @@ func main() {
 			die()
 		}
 	}
+
 	env := initialize.NewEnvironment("/", ds.ConfigRoot(), workspace, convertNetconf, sshKeyName, subs)
 
-	if len(metadataBytes) > 0 {
-		if err := processMetadata(string(metadataBytes), env); err != nil {
-			fmt.Printf("Failed to process meta-data: %v\n", err)
+	var ccm, ccu *initialize.CloudConfig
+	var script *system.Script
+	if ccm, err = initialize.ParseMetaData(string(metadataBytes)); err != nil {
+		fmt.Printf("Failed to parse meta-data: %v\n", err)
 		die()
 	}
-	} else {
-		fmt.Println("No meta-data to handle.")
-	}
-
-	if len(userdataBytes) > 0 {
-		if err := processUserdata(string(userdataBytes), env); err != nil {
-			fmt.Printf("Failed to process user-data: %v\n", err)
-			if !ignoreFailure {
+	if ud, err := initialize.ParseUserData(string(userdataBytes)); err != nil {
+		fmt.Printf("Failed to parse user-data: %v\n", err)
 		die()
-			}
-		}
 	} else {
-		fmt.Println("No user-data to handle.")
+		switch t := ud.(type) {
+		case *initialize.CloudConfig:
+			ccu = t
+		case system.Script:
+			script = &t
 		}
 	}
 
+	var cc *initialize.CloudConfig
+	if ccm != nil && ccu != nil {
+		fmt.Println("Merging cloud-config from meta-data and user-data")
+		merged := mergeCloudConfig(*ccu, *ccm)
+		cc = &merged
+	} else if ccm != nil && ccu == nil {
+		fmt.Println("Processing cloud-config from meta-data")
+		cc = ccm
+	} else if ccm == nil && ccu != nil {
+		fmt.Println("Processing cloud-config from user-data")
+		cc = ccu
+	} else {
+		fmt.Println("No cloud-config data to handle.")
+	}
+
+	if cc != nil {
+		if err = initialize.Apply(*cc, env); err != nil {
+			fmt.Printf("Failed to apply cloud-config: %v\n", err)
+			die()
+		}
+	}
+
+	if script != nil {
+		if err = runScript(*script, env); err != nil {
+			fmt.Printf("Failed to run script: %v\n", err)
+			die()
+		}
+	}
+}
+
+// mergeCloudConfig merges certain options from mdcc (a CloudConfig derived from
+// meta-data) onto udcc (a CloudConfig derived from user-data), if they are
+// not already set on udcc (i.e. user-data always takes precedence)
+// NB: This needs to be kept in sync with ParseMetadata so that it tracks all
+// elements of a CloudConfig which that function can populate.
+func mergeCloudConfig(mdcc, udcc initialize.CloudConfig) (cc initialize.CloudConfig) {
+	if mdcc.Hostname != "" {
+		if udcc.Hostname != "" {
+			fmt.Printf("Warning: user-data hostname (%s) overrides metadata hostname (%s)", udcc.Hostname, mdcc.Hostname)
+		} else {
+			udcc.Hostname = mdcc.Hostname
+		}
+
+	}
+	for _, key := range mdcc.SSHAuthorizedKeys {
+		udcc.SSHAuthorizedKeys = append(udcc.SSHAuthorizedKeys, key)
+	}
+	if mdcc.NetworkConfigPath != "" {
+		if udcc.NetworkConfigPath != "" {
+			fmt.Printf("Warning: user-data NetworkConfigPath %s overrides metadata NetworkConfigPath %s", udcc.NetworkConfigPath, mdcc.NetworkConfigPath)
+		} else {
+			udcc.NetworkConfigPath = mdcc.NetworkConfigPath
+		}
+	}
+	return udcc
+}
+
+// getDatasources creates a slice of possible Datasources for cloudinit based
+// on the different source command-line flags.
 func getDatasources() []datasource.Datasource {
 	dss := make([]datasource.Datasource, 0, 5)
 	if sources.file != "" {
@@ -153,6 +211,11 @@ func getDatasources() []datasource.Datasource {
 	return dss
 }
 
+// selectDatasource attempts to choose a valid Datasource to use based on its
+// current availability. The first Datasource to report to be available is
+// returned. Datasources will be retried if possible if they are not
+// immediately available. If all Datasources are permanently unavailable or
+// datasourceTimeout is reached before one becomes available, nil is returned.
 func selectDatasource(sources []datasource.Datasource) datasource.Datasource {
 	ds := make(chan datasource.Datasource)
 	stop := make(chan struct{})
@@ -199,48 +262,18 @@ func selectDatasource(sources []datasource.Datasource) datasource.Datasource {
 	return s
 }
 
-func processUserdata(userdata string, env *initialize.Environment) error {
-	userdata = env.Apply(userdata)
-
-	parsed, err := initialize.ParseUserData(userdata)
-	if err != nil {
-		fmt.Printf("Failed parsing user-data: %v\n", err)
-		return err
-	}
-
-	err = initialize.PrepWorkspace(env.Workspace())
+// TODO(jonboulle): this should probably be refactored and moved into a different module
+func runScript(script system.Script, env *initialize.Environment) error {
+	err := initialize.PrepWorkspace(env.Workspace())
 	if err != nil {
 		fmt.Printf("Failed preparing workspace: %v\n", err)
 		return err
 	}
-
-	switch t := parsed.(type) {
-	case initialize.CloudConfig:
-		err = initialize.Apply(t, env)
-	case system.Script:
-		var path string
-		path, err = initialize.PersistScriptInWorkspace(t, env.Workspace())
+	path, err := initialize.PersistScriptInWorkspace(script, env.Workspace())
 	if err == nil {
 		var name string
 		name, err = system.ExecuteScript(path)
 		initialize.PersistUnitNameInWorkspace(name, env.Workspace())
 	}
-	}
-
 	return err
 }
-
-func processMetadata(metadata string, env *initialize.Environment) error {
-	parsed, err := initialize.ParseMetaData(metadata)
-	if err != nil {
-		fmt.Printf("Failed parsing meta-data: %v\n", err)
-		return err
-	}
-	err = initialize.PrepWorkspace(env.Workspace())
-	if err != nil {
-		fmt.Printf("Failed preparing workspace: %v\n", err)
-		return err
-	}
-
-	return initialize.Apply(parsed, env)
-}

coreos-cloudinit_test.go

@@ -0,0 +1,110 @@
+package main
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/coreos/coreos-cloudinit/initialize"
+)
+
+func TestMergeCloudConfig(t *testing.T) {
+	simplecc := initialize.CloudConfig{
+		SSHAuthorizedKeys: []string{"abc", "def"},
+		Hostname:          "foobar",
+		NetworkConfigPath: "/path/somewhere",
+	}
+	for i, tt := range []struct {
+		udcc initialize.CloudConfig
+		mdcc initialize.CloudConfig
+		want initialize.CloudConfig
+	}{
+		{
+			// If mdcc is empty, udcc should be returned unchanged
+			simplecc,
+			initialize.CloudConfig{},
+			simplecc,
+		},
+		{
+			// If udcc is empty, mdcc should be returned unchanged(overridden)
+			initialize.CloudConfig{},
+			simplecc,
+			simplecc,
+		},
+		{
+			// user-data should override completely in the case of conflicts
+			simplecc,
+			initialize.CloudConfig{
+				Hostname:          "meta-hostname",
+				NetworkConfigPath: "/path/meta",
+			},
+			simplecc,
+		},
+		{
+			// Mixed merge should succeed
+			initialize.CloudConfig{
+				SSHAuthorizedKeys: []string{"abc", "def"},
+				Hostname:          "user-hostname",
+				NetworkConfigPath: "/path/somewhere",
+			},
+			initialize.CloudConfig{
+				SSHAuthorizedKeys: []string{"woof", "qux"},
+				Hostname:          "meta-hostname",
+			},
+			initialize.CloudConfig{
+				SSHAuthorizedKeys: []string{"abc", "def", "woof", "qux"},
+				Hostname:          "user-hostname",
+				NetworkConfigPath: "/path/somewhere",
+			},
+		},
+		{
+			// Completely non-conflicting merge should be fine
+			initialize.CloudConfig{
+				Hostname: "supercool",
+			},
+			initialize.CloudConfig{
+				SSHAuthorizedKeys: []string{"zaphod", "beeblebrox"},
+				NetworkConfigPath: "/dev/fun",
+			},
+			initialize.CloudConfig{
+				Hostname:          "supercool",
+				SSHAuthorizedKeys: []string{"zaphod", "beeblebrox"},
+				NetworkConfigPath: "/dev/fun",
+			},
+		},
+		{
+			// Non-mergeable settings in user-data should not be affected
+			initialize.CloudConfig{
+				Hostname:       "mememe",
+				ManageEtcHosts: initialize.EtcHosts("lolz"),
+			},
+			initialize.CloudConfig{
+				Hostname:          "youyouyou",
+				NetworkConfigPath: "meta-meta-yo",
+			},
+			initialize.CloudConfig{
+				Hostname:          "mememe",
+				ManageEtcHosts:    initialize.EtcHosts("lolz"),
+				NetworkConfigPath: "meta-meta-yo",
+			},
+		},
+		{
+			// Non-mergeable (unexpected) settings in meta-data are ignored
+			initialize.CloudConfig{
+				Hostname: "mememe",
+			},
+			initialize.CloudConfig{
+				ManageEtcHosts:    initialize.EtcHosts("lolz"),
+				NetworkConfigPath: "meta-meta-yo",
+			},
+			initialize.CloudConfig{
+				Hostname:          "mememe",
+				NetworkConfigPath: "meta-meta-yo",
+			},
+		},
+	} {
+		got := mergeCloudConfig(tt.mdcc, tt.udcc)
+		if !reflect.DeepEqual(got, tt.want) {
+			t.Errorf("case #%d: mergeCloudConfig mutated CloudConfig unexpectedly:\ngot:\n%s\nwant:\n%s", i, got, tt.want)
+		}
+	}
+}

initialize/meta_data.go

@@ -3,8 +3,11 @@ package initialize
 import "encoding/json"
 
 // ParseMetaData parses a JSON blob in the OpenStack metadata service format, and
-// converts it to a CloudConfig
-func ParseMetaData(contents string) (cfg CloudConfig, err error) {
+// converts it to a partially hydrated CloudConfig
+func ParseMetaData(contents string) (*CloudConfig, error) {
+	if len(contents) == 0 {
+		return nil, nil
+	}
 	var metadata struct {
 		SSHAuthorizedKeyMap map[string]string `json:"public_keys"`
 		Hostname            string            `json:"hostname"`
@@ -12,17 +15,20 @@ func ParseMetaData(contents string) (cfg CloudConfig, err error) {
 			ContentPath string `json:"content_path"`
 		} `json:"network_config"`
 	}
-	if err = json.Unmarshal([]byte(contents), &metadata); err != nil {
-		return
+	if err := json.Unmarshal([]byte(contents), &metadata); err != nil {
+		return nil, err
 	}
 
+	var cfg CloudConfig
+	if len(metadata.SSHAuthorizedKeyMap) > 0 {
 		cfg.SSHAuthorizedKeys = make([]string, 0, len(metadata.SSHAuthorizedKeyMap))
 		for _, key := range metadata.SSHAuthorizedKeyMap {
 			cfg.SSHAuthorizedKeys = append(cfg.SSHAuthorizedKeys, key)
 		}
+	}
 	cfg.Hostname = metadata.Hostname
 	cfg.NetworkConfigPath = metadata.NetworkConfig.ContentPath
-	return
+	return &cfg, nil
 }
 
 // ExtractIPsFromMetaData parses a JSON blob in the OpenStack metadata service format,

initialize/meta_data_test.go

@@ -3,6 +3,39 @@ package initialize
 import "reflect"
 import "testing"
 
+func TestParseMetadata(t *testing.T) {
+	for i, tt := range []struct {
+		in   string
+		want *CloudConfig
+		err  bool
+	}{
+		{"", nil, false},
+		{`garbage, invalid json`, nil, true},
+		{`{"foo": "bar"}`, &CloudConfig{}, false},
+		{`{"network_config": {"content_path": "asdf"}}`, &CloudConfig{NetworkConfigPath: "asdf"}, false},
+		{`{"hostname": "turkleton"}`, &CloudConfig{Hostname: "turkleton"}, false},
+		{`{"public_keys": {"jack": "jill", "bob": "alice"}}`, &CloudConfig{SSHAuthorizedKeys: []string{"jill", "alice"}}, false},
+		{`{"unknown": "thing", "hostname": "my_host", "public_keys": {"do": "re", "mi": "fa"}, "network_config": {"content_path": "/root", "blah": "zzz"}}`, &CloudConfig{SSHAuthorizedKeys: []string{"re", "fa"}, Hostname: "my_host", NetworkConfigPath: "/root"}, false},
+	} {
+		got, err := ParseMetaData(tt.in)
+		if tt.err != (err != nil) {
+			t.Errorf("case #%d: bad error state: got %t, want %t (err=%v)", i, (err != nil), tt.err, err)
+		}
+		if got == nil {
+			if tt.want != nil {
+				t.Errorf("case #%d: unexpected nil output", i)
+			}
+		} else if tt.want == nil {
+			t.Errorf("case #%d: unexpected non-nil output", i)
+		} else {
+			if !reflect.DeepEqual(*got, *tt.want) {
+				t.Errorf("case #%d: bad output:\ngot\n%v\nwant\n%v", i, *got, *tt.want)
+			}
+		}
+	}
+
+}
+
 func TestExtractIPsFromMetadata(t *testing.T) {
 	for i, tt := range []struct {
 		in  []byte

initialize/user_data.go

@@ -9,6 +9,9 @@ import (
 )
 
 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
@@ -19,14 +22,9 @@ func ParseUserData(contents string) (interface{}, error) {
 	if strings.HasPrefix(header, "#!") {
 		log.Printf("Parsing user-data as script")
 		return system.Script(contents), nil
-
 	} else if header == "#cloud-config" {
 		log.Printf("Parsing user-data as cloud-config")
-		cfg, err := NewCloudConfig(contents)
-		if err != nil {
-			log.Fatal(err.Error())
-		}
-		return *cfg, nil
+		return NewCloudConfig(contents)
 	} else {
 		return nil, fmt.Errorf("Unrecognized user-data header: %s", header)
 	}

initialize/user_data_test.go

@@ -37,7 +37,7 @@ func TestParseConfigCRLF(t *testing.T) {
 		t.Fatalf("Failed parsing config: %v", err)
 	}
 
-	cfg := ud.(CloudConfig)
+	cfg := ud.(*CloudConfig)
 
 	if cfg.Hostname != "foo" {
 		t.Error("Failed parsing hostname from config")
@@ -47,3 +47,12 @@ func TestParseConfigCRLF(t *testing.T) {
 		t.Error("Parsed incorrect number of SSH keys")
 	}
 }
+
+func TestParseConfigEmpty(t *testing.T) {
+	i, e := ParseUserData(``)
+	if i != nil {
+		t.Error("ParseUserData of empty string returned non-nil unexpectedly")
+	} else if e != nil {
+		t.Error("ParseUserData of empty string returned error unexpectedly")
+	}
+}

test

@@ -18,7 +18,7 @@ declare -a TESTPKGS=(initialize system datasource pkg network)
 if [ -z "$PKG" ]; then
 	GOFMTPATH="$TESTPKGS coreos-cloudinit.go"
 	# prepend repo path to each package
-	TESTPKGS=${TESTPKGS[@]/#/${REPO_PATH}/}
+	TESTPKGS="${TESTPKGS[@]/#/${REPO_PATH}/} ./"
 else
 	GOFMTPATH="$TESTPKGS"
 	# strip out slashes and dots from PKG=./foo/