metadata: simplify merging of metadata

Add an internal field for CloudConfig to make it easier to distinguish.
Instead of creating two CloudConfigs and merging them, just merge the
metadata into the existing CloudConfig.

config/config.go

@@ -33,8 +33,7 @@ type CloudConfig struct {
 	Hostname          string    `yaml:"hostname"`
 	Users             []User    `yaml:"users"`
 	ManageEtcHosts    EtcHosts  `yaml:"manage_etc_hosts"`
-	NetworkConfigPath string   `yaml:"-"`
-	NetworkConfig     string   `yaml:"-"`
+	Internal          Internals `yaml:"-"`
 }
 
 type CoreOS struct {
@@ -47,6 +46,10 @@ type CoreOS struct {
 	Units     []Unit    `yaml:"units"`
 }
 
+type Internals struct {
+	NetworkConfig []byte
+}
+
 func IsCloudConfig(userdata string) bool {
 	header := strings.SplitN(userdata, "\n", 2)[0]
 

coreos-cloudinit.go

@@ -189,19 +189,8 @@ func main() {
 	env := initialize.NewEnvironment("/", ds.ConfigRoot(), flags.workspace, flags.convertNetconf, flags.sshKeyName, subs)
 	userdata := env.Apply(string(userdataBytes))
 
-	var ccm, ccu *config.CloudConfig
+	var ccu *config.CloudConfig
 	var script *config.Script
-	ccm = initialize.ParseMetaData(metadata)
-
-	if ccm != nil && flags.convertNetconf != "" {
-		fmt.Printf("Fetching network config from datasource of type %q\n", ds.Type())
-		netconfBytes, err := ds.FetchNetworkConfig(ccm.NetworkConfigPath)
-		if err != nil {
-			fmt.Printf("Failed fetching network config from datasource: %v\n", err)
-			os.Exit(1)
-		}
-		ccm.NetworkConfig = string(netconfBytes)
-	}
 
 	if ud, err := initialize.ParseUserData(userdata); err != nil {
 		fmt.Printf("Failed to parse user-data: %v\nContinuing...\n", err)
@@ -215,26 +204,22 @@ func main() {
 		}
 	}
 
-	var cc *config.CloudConfig
-	if ccm != nil && ccu != nil {
 	fmt.Println("Merging cloud-config from meta-data and user-data")
-		merged := mergeCloudConfig(*ccm, *ccu)
-		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.")
-	}
+	cc := mergeConfigs(ccu, metadata)
 
-	if cc != nil {
-		if err = initialize.Apply(*cc, env); err != nil {
-			fmt.Printf("Failed to apply cloud-config: %v\n", err)
+	if flags.convertNetconf != "" {
+		fmt.Printf("Fetching network config from datasource of type %q\n", ds.Type())
+		netconfBytes, err := ds.FetchNetworkConfig(metadata.NetworkConfigPath)
+		if err != nil {
+			fmt.Printf("Failed fetching network config from datasource: %v\n", err)
 			os.Exit(1)
 		}
+		cc.Internal.NetworkConfig = netconfBytes
+	}
+
+	if err = initialize.Apply(cc, env); err != nil {
+		fmt.Printf("Failed to apply cloud-config: %v\n", err)
+		os.Exit(1)
 	}
 
 	if script != nil {
@@ -249,38 +234,25 @@ func main() {
 	}
 }
 
-// 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 config.CloudConfig) (cc config.CloudConfig) {
-	if mdcc.Hostname != "" {
-		if udcc.Hostname != "" {
-			fmt.Printf("Warning: user-data hostname (%s) overrides metadata hostname (%s)\n", udcc.Hostname, mdcc.Hostname)
-		} else {
-			udcc.Hostname = mdcc.Hostname
+// mergeConfigs merges certain options from md (meta-data from the datasource)
+// onto cc (a CloudConfig derived from user-data), if they are not already set
+// on cc (i.e. user-data always takes precedence)
+func mergeConfigs(cc *config.CloudConfig, md datasource.Metadata) (out config.CloudConfig) {
+	if cc != nil {
+		out = *cc
 	}
 
-	}
-	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\n", udcc.NetworkConfigPath, mdcc.NetworkConfigPath)
+	if md.Hostname != "" {
+		if out.Hostname != "" {
+			fmt.Printf("Warning: user-data hostname (%s) overrides metadata hostname (%s)\n", out.Hostname, md.Hostname)
 		} else {
-			udcc.NetworkConfigPath = mdcc.NetworkConfigPath
+			out.Hostname = md.Hostname
 		}
 	}
-	if mdcc.NetworkConfig != "" {
-		if udcc.NetworkConfig != "" {
-			fmt.Printf("Warning: user-data NetworkConfig %s overrides metadata NetworkConfig %s\n", udcc.NetworkConfig, mdcc.NetworkConfig)
-		} else {
-			udcc.NetworkConfig = mdcc.NetworkConfig
+	for _, key := range md.SSHPublicKeys {
+		out.SSHAuthorizedKeys = append(out.SSHAuthorizedKeys, key)
 	}
-	}
-	return udcc
+	return
 }
 
 // getDatasources creates a slice of possible Datasources for cloudinit based

coreos-cloudinit_test.go

@@ -19,116 +19,71 @@ import (
 	"testing"
 
 	"github.com/coreos/coreos-cloudinit/config"
+	"github.com/coreos/coreos-cloudinit/datasource"
 )
 
-func TestMergeCloudConfig(t *testing.T) {
-	simplecc := config.CloudConfig{
-		SSHAuthorizedKeys: []string{"abc", "def"},
-		Hostname:          "foobar",
-		NetworkConfigPath: "/path/somewhere",
-		NetworkConfig:     `{}`,
-	}
-	for i, tt := range []struct {
-		udcc config.CloudConfig
-		mdcc config.CloudConfig
-		want config.CloudConfig
+func TestMergeConfigs(t *testing.T) {
+	tests := []struct {
+		cc *config.CloudConfig
+		md datasource.Metadata
+
+		out config.CloudConfig
 	}{
 		{
-			// If mdcc is empty, udcc should be returned unchanged
-			simplecc,
-			config.CloudConfig{},
-			simplecc,
+			// If md is empty and cc is nil, result should be empty
+			out: config.CloudConfig{},
 		},
 		{
-			// If udcc is empty, mdcc should be returned unchanged(overridden)
-			config.CloudConfig{},
-			simplecc,
-			simplecc,
+			// If md and cc are empty, result should be empty
+			cc:  &config.CloudConfig{},
+			out: config.CloudConfig{},
+		},
+		{
+			// If cc is empty, cc should be returned unchanged
+			cc:  &config.CloudConfig{SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "cc-host"},
+			out: config.CloudConfig{SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "cc-host"},
+		},
+		{
+			// If cc is empty, cc should be returned unchanged(overridden)
+			cc:  &config.CloudConfig{},
+			md:  datasource.Metadata{Hostname: "md-host", SSHPublicKeys: map[string]string{"key": "ghi"}},
+			out: config.CloudConfig{SSHAuthorizedKeys: []string{"ghi"}, Hostname: "md-host"},
+		},
+		{
+			// If cc is nil, cc should be returned unchanged(overridden)
+			md:  datasource.Metadata{Hostname: "md-host", SSHPublicKeys: map[string]string{"key": "ghi"}},
+			out: config.CloudConfig{SSHAuthorizedKeys: []string{"ghi"}, Hostname: "md-host"},
 		},
 		{
 			// user-data should override completely in the case of conflicts
-			simplecc,
-			config.CloudConfig{
-				Hostname:          "meta-hostname",
-				NetworkConfigPath: "/path/meta",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-			simplecc,
+			cc:  &config.CloudConfig{SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "cc-host"},
+			md:  datasource.Metadata{Hostname: "md-host"},
+			out: config.CloudConfig{SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "cc-host"},
 		},
 		{
 			// Mixed merge should succeed
-			config.CloudConfig{
-				SSHAuthorizedKeys: []string{"abc", "def"},
-				Hostname:          "user-hostname",
-				NetworkConfigPath: "/path/somewhere",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-			config.CloudConfig{
-				SSHAuthorizedKeys: []string{"woof", "qux"},
-				Hostname:          "meta-hostname",
-			},
-			config.CloudConfig{
-				SSHAuthorizedKeys: []string{"abc", "def", "woof", "qux"},
-				Hostname:          "user-hostname",
-				NetworkConfigPath: "/path/somewhere",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
+			cc:  &config.CloudConfig{SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "cc-host"},
+			md:  datasource.Metadata{Hostname: "md-host", SSHPublicKeys: map[string]string{"key": "ghi"}},
+			out: config.CloudConfig{SSHAuthorizedKeys: []string{"abc", "def", "ghi"}, Hostname: "cc-host"},
 		},
 		{
 			// Completely non-conflicting merge should be fine
-			config.CloudConfig{
-				Hostname: "supercool",
-			},
-			config.CloudConfig{
-				SSHAuthorizedKeys: []string{"zaphod", "beeblebrox"},
-				NetworkConfigPath: "/dev/fun",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-			config.CloudConfig{
-				Hostname:          "supercool",
-				SSHAuthorizedKeys: []string{"zaphod", "beeblebrox"},
-				NetworkConfigPath: "/dev/fun",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
+			cc:  &config.CloudConfig{Hostname: "cc-host"},
+			md:  datasource.Metadata{SSHPublicKeys: map[string]string{"zaphod": "beeblebrox"}},
+			out: config.CloudConfig{Hostname: "cc-host", SSHAuthorizedKeys: []string{"beeblebrox"}},
 		},
 		{
 			// Non-mergeable settings in user-data should not be affected
-			config.CloudConfig{
-				Hostname:       "mememe",
-				ManageEtcHosts: config.EtcHosts("lolz"),
+			cc:  &config.CloudConfig{Hostname: "cc-host", ManageEtcHosts: config.EtcHosts("lolz")},
+			md:  datasource.Metadata{Hostname: "md-host"},
+			out: config.CloudConfig{Hostname: "cc-host", ManageEtcHosts: config.EtcHosts("lolz")},
 		},
-			config.CloudConfig{
-				Hostname:          "youyouyou",
-				NetworkConfigPath: "meta-meta-yo",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-			config.CloudConfig{
-				Hostname:          "mememe",
-				ManageEtcHosts:    config.EtcHosts("lolz"),
-				NetworkConfigPath: "meta-meta-yo",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-		},
-		{
-			// Non-mergeable (unexpected) settings in meta-data are ignored
-			config.CloudConfig{
-				Hostname: "mememe",
-			},
-			config.CloudConfig{
-				ManageEtcHosts:    config.EtcHosts("lolz"),
-				NetworkConfigPath: "meta-meta-yo",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-			config.CloudConfig{
-				Hostname:          "mememe",
-				NetworkConfigPath: "meta-meta-yo",
-				NetworkConfig:     `{"hostname":"test"}`,
-			},
-		},
-	} {
-		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)
+	}
+
+	for i, tt := range tests {
+		out := mergeConfigs(tt.cc, tt.md)
+		if !reflect.DeepEqual(tt.out, out) {
+			t.Errorf("bad config (%d): want %#v, got %#v", i, tt.out, out)
 		}
 	}
 }

initialize/config.go

@@ -170,9 +170,9 @@ func Apply(cfg config.CloudConfig, env *Environment) error {
 		var err error
 		switch env.NetconfType() {
 		case "debian":
-			interfaces, err = network.ProcessDebianNetconf(cfg.NetworkConfig)
+			interfaces, err = network.ProcessDebianNetconf(cfg.Internal.NetworkConfig)
 		case "digitalocean":
-			interfaces, err = network.ProcessDigitalOceanNetconf(cfg.NetworkConfig)
+			interfaces, err = network.ProcessDigitalOceanNetconf(cfg.Internal.NetworkConfig)
 		default:
 			err = fmt.Errorf("Unsupported network config format %q", env.NetconfType())
 		}

initialize/meta_data.go

@@ -17,25 +17,9 @@ package initialize
 import (
 	"sort"
 
-	"github.com/coreos/coreos-cloudinit/config"
 	"github.com/coreos/coreos-cloudinit/datasource"
 )
 
-// ParseMetaData parses a JSON blob in the OpenStack metadata service format,
-// and converts it to a partially hydrated CloudConfig.
-func ParseMetaData(metadata datasource.Metadata) *config.CloudConfig {
-	var cfg config.CloudConfig
-	if len(metadata.SSHPublicKeys) > 0 {
-		cfg.SSHAuthorizedKeys = make([]string, 0, len(metadata.SSHPublicKeys))
-		for _, name := range sortedKeys(metadata.SSHPublicKeys) {
-			cfg.SSHAuthorizedKeys = append(cfg.SSHAuthorizedKeys, metadata.SSHPublicKeys[name])
-		}
-	}
-	cfg.Hostname = metadata.Hostname
-	cfg.NetworkConfigPath = metadata.NetworkConfigPath
-	return &cfg
-}
-
 // ExtractIPsFromMetaData parses a JSON blob in the OpenStack metadata service
 // format and returns a substitution map possibly containing private_ipv4,
 // public_ipv4, private_ipv6, and public_ipv6 addresses.

network/debian.go

@@ -19,9 +19,9 @@ import (
 	"strings"
 )
 
-func ProcessDebianNetconf(config string) ([]InterfaceGenerator, error) {
+func ProcessDebianNetconf(config []byte) ([]InterfaceGenerator, error) {
 	log.Println("Processing Debian network config")
-	lines := formatConfig(config)
+	lines := formatConfig(string(config))
 	stanzas, err := parseStanzas(lines)
 	if err != nil {
 		return nil, err

network/debian_test.go

@@ -44,7 +44,7 @@ func TestProcessDebianNetconf(t *testing.T) {
 		{"auto eth1\nauto eth2", false, 0},
 		{"iface eth1 inet manual", false, 1},
 	} {
-		interfaces, err := ProcessDebianNetconf(tt.in)
+		interfaces, err := ProcessDebianNetconf([]byte(tt.in))
 		failed := err != nil
 		if tt.fail != failed {
 			t.Fatalf("bad failure state for %q: got %t, want %t", tt.in, failed, tt.fail)

network/digitalocean.go

@@ -23,14 +23,14 @@ import (
 	"github.com/coreos/coreos-cloudinit/datasource/metadata/digitalocean"
 )
 
-func ProcessDigitalOceanNetconf(config string) ([]InterfaceGenerator, error) {
+func ProcessDigitalOceanNetconf(config []byte) ([]InterfaceGenerator, error) {
 	log.Println("Processing DigitalOcean network config")
-	if config == "" {
+	if len(config) == 0 {
 		return nil, nil
 	}
 
 	var cfg digitalocean.Metadata
-	if err := json.Unmarshal([]byte(config), &cfg); err != nil {
+	if err := json.Unmarshal(config, &cfg); err != nil {
 		return nil, err
 	}
 

network/digitalocean_test.go

@@ -378,7 +378,7 @@ func TestProcessDigitalOceanNetconf(t *testing.T) {
 			ifaces: []InterfaceGenerator{},
 		},
 	} {
-		ifaces, err := ProcessDigitalOceanNetconf(tt.cfg)
+		ifaces, err := ProcessDigitalOceanNetconf([]byte(tt.cfg))
 		if !errorsEqual(tt.err, err) {
 			t.Fatalf("bad error (%q): want %q, got %q", tt.cfg, tt.err, err)
 		}