From 1aabacc76946cefcfde7d73a87a9535ec5513a8d Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Thu, 26 Jun 2014 17:17:58 -0700 Subject: [PATCH 1/3] cloudinit: merge cloudconfig info from user-data and meta-data This attempts to retrieve cloudconfigs from two sources: the meta-data service, and the user-data service. If only one cloudconfig is found, that is applied to the system. If both services return a cloudconfig, the two are merged into a single cloudconfig which is then applied to the system. Only a subset of parameters are merged (because the meta-data service currently only partially populates a cloudconfig). In the event of any conflicts, parameters in the user-data cloudconfig take precedence over those in the meta-data cloudconfig. --- coreos-cloudinit.go | 137 ++++++++++++++++++++++------------- coreos-cloudinit_test.go | 110 ++++++++++++++++++++++++++++ initialize/meta_data.go | 4 +- initialize/user_data.go | 7 +- initialize/user_data_test.go | 2 +- test | 2 +- 6 files changed, 200 insertions(+), 62 deletions(-) create mode 100644 coreos-cloudinit_test.go diff --git a/coreos-cloudinit.go b/coreos-cloudinit.go index 652298b..f3700e5 100644 --- a/coreos-cloudinit.go +++ b/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) - die() - } + 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() + } + if ud, err := initialize.ParseUserData(string(userdataBytes)); err != nil { + fmt.Printf("Failed to parse user-data: %v\n", err) + die() } else { - fmt.Println("No meta-data to handle.") + switch t := ud.(type) { + case *initialize.CloudConfig: + ccu = t + case system.Script: + script = t + } } - if len(userdataBytes) > 0 { - if err := processUserdata(string(userdataBytes), env); err != nil { - fmt.Printf("Failed to process user-data: %v\n", err) - if !ignoreFailure { - die() - } - } + 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 user-data to handle.") + 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()) - if err == nil { - var name string - name, err = system.ExecuteScript(path) - initialize.PersistUnitNameInWorkspace(name, 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) -} diff --git a/coreos-cloudinit_test.go b/coreos-cloudinit_test.go new file mode 100644 index 0000000..6c6e7f0 --- /dev/null +++ b/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) + } + } +} diff --git a/initialize/meta_data.go b/initialize/meta_data.go index 5168d36..05c0153 100644 --- a/initialize/meta_data.go +++ b/initialize/meta_data.go @@ -3,8 +3,8 @@ 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) (cfg *CloudConfig, err error) { var metadata struct { SSHAuthorizedKeyMap map[string]string `json:"public_keys"` Hostname string `json:"hostname"` diff --git a/initialize/user_data.go b/initialize/user_data.go index a34d534..4cd4fa4 100644 --- a/initialize/user_data.go +++ b/initialize/user_data.go @@ -19,14 +19,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) } diff --git a/initialize/user_data_test.go b/initialize/user_data_test.go index c764eb2..984d073 100644 --- a/initialize/user_data_test.go +++ b/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") diff --git a/test b/test index 24e556b..53dfc91 100755 --- a/test +++ b/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/ From 231c0fa20bed5360c03782747aa2087868bb4e74 Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Fri, 27 Jun 2014 18:31:55 -0700 Subject: [PATCH 2/3] initialize: add tests for ParseMetadata --- initialize/meta_data.go | 20 +++++++++++++------- initialize/meta_data_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/initialize/meta_data.go b/initialize/meta_data.go index 05c0153..8fb1e77 100644 --- a/initialize/meta_data.go +++ b/initialize/meta_data.go @@ -4,7 +4,10 @@ import "encoding/json" // ParseMetaData parses a JSON blob in the OpenStack metadata service format, and // converts it to a partially hydrated CloudConfig -func ParseMetaData(contents string) (cfg *CloudConfig, err error) { +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 } - cfg.SSHAuthorizedKeys = make([]string, 0, len(metadata.SSHAuthorizedKeyMap)) - for _, key := range metadata.SSHAuthorizedKeyMap { - cfg.SSHAuthorizedKeys = append(cfg.SSHAuthorizedKeys, key) + 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, diff --git a/initialize/meta_data_test.go b/initialize/meta_data_test.go index a362243..2553e31 100644 --- a/initialize/meta_data_test.go +++ b/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 From d4e048a1f4e13617bba8934ac372125e36080b50 Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Fri, 27 Jun 2014 23:58:27 -0700 Subject: [PATCH 3/3] ParseUserData: return nil on empty input string --- coreos-cloudinit.go | 6 +++--- initialize/user_data.go | 3 +++ initialize/user_data_test.go | 9 +++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/coreos-cloudinit.go b/coreos-cloudinit.go index f3700e5..948a047 100644 --- a/coreos-cloudinit.go +++ b/coreos-cloudinit.go @@ -115,7 +115,7 @@ func main() { env := initialize.NewEnvironment("/", ds.ConfigRoot(), workspace, convertNetconf, sshKeyName, subs) var ccm, ccu *initialize.CloudConfig - var script system.Script + var script *system.Script if ccm, err = initialize.ParseMetaData(string(metadataBytes)); err != nil { fmt.Printf("Failed to parse meta-data: %v\n", err) die() @@ -128,7 +128,7 @@ func main() { case *initialize.CloudConfig: ccu = t case system.Script: - script = t + script = &t } } @@ -155,7 +155,7 @@ func main() { } if script != nil { - if err = runScript(script, env); err != nil { + if err = runScript(*script, env); err != nil { fmt.Printf("Failed to run script: %v\n", err) die() } diff --git a/initialize/user_data.go b/initialize/user_data.go index 4cd4fa4..280cad6 100644 --- a/initialize/user_data.go +++ b/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 diff --git a/initialize/user_data_test.go b/initialize/user_data_test.go index 984d073..006f85f 100644 --- a/initialize/user_data_test.go +++ b/initialize/user_data_test.go @@ -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") + } +}