From c255739a932ac7a2b9eaf487a4303ae37f75770a Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 19:37:39 -0700 Subject: [PATCH 1/8] etcd: refactor config - Explicitly specify all of the valid options for etcd - Remove the default name generation (ETCD_NAME is set by its unit file now) - Seperate the etcd config from Units() - Remove support for DISCOVERY_URL - Add YAML tags for the fields --- config/etcd.go | 32 +++++++ initialize/config.go | 5 +- initialize/config_test.go | 2 +- initialize/etcd.go | 63 ------------- initialize/etcd_test.go | 184 -------------------------------------- system/env.go | 26 ++++++ system/etcd.go | 25 ++++++ system/etcd_test.go | 60 +++++++++++++ 8 files changed, 147 insertions(+), 250 deletions(-) create mode 100644 config/etcd.go delete mode 100644 initialize/etcd.go delete mode 100644 initialize/etcd_test.go create mode 100644 system/env.go create mode 100644 system/etcd.go create mode 100644 system/etcd_test.go diff --git a/config/etcd.go b/config/etcd.go new file mode 100644 index 0000000..b0073ed --- /dev/null +++ b/config/etcd.go @@ -0,0 +1,32 @@ +package config + +type Etcd struct { + Addr string `yaml:"addr" env:"ETCD_ADDR"` + BindAddr string `yaml:"bind-addr" env:"ETCD_BIND_ADDR"` + CAFile string `yaml:"ca-file" env:"ETCD_CA_FILE"` + CertFile string `yaml:"cert-file" env:"ETCD_CERT_FILE"` + ClusterActiveSize string `yaml:"cluster-active-size" env:"ETCD_CLUSTER_ACTIVE_SIZE"` + ClusterRemoveDelay string `yaml:"cluster-remove-delay" env:"ETCD_CLUSTER_REMOVE_DELAY"` + ClusterSyncInterval string `yaml:"cluster-sync-interval" env:"ETCD_CLUSTER_SYNC_INTERVAL"` + Cors string `yaml:"cors" env:"ETCD_CORS"` + CPUProfileFile string `yaml:"cpu-profile-file" env:"ETCD_CPU_PROFILE_FILE"` + DataDir string `yaml:"data-dir" env:"ETCD_DATA_DIR"` + Discovery string `yaml:"discovery" env:"ETCD_DISCOVERY"` + HTTPReadTimeout string `yaml:"http-read-timeout" env:"ETCD_HTTP_READ_TIMEOUT"` + HTTPWriteTimeout string `yaml:"http-write-timeout" env:"ETCD_HTTP_WRITE_TIMEOUT"` + KeyFile string `yaml:"key-file" env:"ETCD_KEY_FILE"` + MaxClusterSize string `yaml:"max-cluster-size" env:"ETCD_MAX_CLUSTER_SIZE"` + MaxResultBuffer string `yaml:"max-result-buffer" env:"ETCD_MAX_RESULT_BUFFER"` + MaxRetryAttempts string `yaml:"max-retry-attempts" env:"ETCD_MAX_RETRY_ATTEMPTS"` + Name string `yaml:"name" env:"ETCD_NAME"` + PeerAddr string `yaml:"peer-addr" env:"ETCD_PEER_ADDR"` + PeerBindAddr string `yaml:"peer-bind-addr" env:"ETCD_PEER_BIND_ADDR"` + PeerCAFile string `yaml:"peer-ca-file" env:"ETCD_PEER_CA_FILE"` + PeerCertFile string `yaml:"peer-cert-file" env:"ETCD_PEER_CERT_FILE"` + PeerKeyFile string `yaml:"peer-key-file" env:"ETCD_PEER_KEY_FILE"` + Peers string `yaml:"peers" env:"ETCD_PEERS"` + PeersFile string `yaml:"peers-file" env:"ETCD_PEERS_FILE"` + Snapshot string `yaml:"snapshot" env:"ETCD_SNAPSHOT"` + Verbose string `yaml:"verbose" env:"ETCD_VERBOSE"` + VeryVerbose string `yaml:"very-verbose" env:"ETCD_VERY_VERBOSE"` +} diff --git a/initialize/config.go b/initialize/config.go index 0ccaef4..384b9ba 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -8,6 +8,7 @@ import ( "github.com/coreos/coreos-cloudinit/third_party/gopkg.in/yaml.v1" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/network" "github.com/coreos/coreos-cloudinit/system" ) @@ -30,7 +31,7 @@ type CloudConfigUnit interface { type CloudConfig struct { SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"` Coreos struct { - Etcd EtcdEnvironment + Etcd config.Etcd Fleet FleetEnvironment OEM OEMRelease Update UpdateConfig @@ -226,7 +227,7 @@ func Apply(cfg CloudConfig, env *Environment) error { } } - for _, ccu := range []CloudConfigUnit{cfg.Coreos.Etcd, cfg.Coreos.Fleet, cfg.Coreos.Update} { + for _, ccu := range []CloudConfigUnit{system.Etcd{cfg.Coreos.Etcd}, cfg.Coreos.Fleet, cfg.Coreos.Update} { u, err := ccu.Units(env.Root()) if err != nil { return err diff --git a/initialize/config_test.go b/initialize/config_test.go index 0e91933..b098a79 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -66,7 +66,7 @@ hostname: if cfg.Hostname != "foo" { t.Fatalf("hostname not correctly set when invalid keys are present") } - if len(cfg.Coreos.Etcd) < 1 { + if cfg.Coreos.Etcd.Discovery != "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" { 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" { diff --git a/initialize/etcd.go b/initialize/etcd.go deleted file mode 100644 index e2011f2..0000000 --- a/initialize/etcd.go +++ /dev/null @@ -1,63 +0,0 @@ -package initialize - -import ( - "errors" - "fmt" - "sort" - - "github.com/coreos/coreos-cloudinit/system" -) - -type EtcdEnvironment map[string]string - -func (ee EtcdEnvironment) String() (out string) { - norm := normalizeSvcEnv(ee) - - if val, ok := norm["DISCOVERY_URL"]; ok { - delete(norm, "DISCOVERY_URL") - if _, ok := norm["DISCOVERY"]; !ok { - norm["DISCOVERY"] = val - } - } - - var sorted sort.StringSlice - for k, _ := range norm { - sorted = append(sorted, k) - } - sorted.Sort() - - out += "[Service]\n" - - for _, key := range sorted { - val := norm[key] - out += fmt.Sprintf("Environment=\"ETCD_%s=%s\"\n", key, val) - } - - return -} - -// Units creates a Unit file drop-in for etcd, using any configured -// options and adding a default MachineID if unset. -func (ee EtcdEnvironment) Units(root string) ([]system.Unit, error) { - if len(ee) < 1 { - return nil, nil - } - - if _, ok := ee["name"]; !ok { - if machineID := system.MachineID(root); machineID != "" { - ee["name"] = machineID - } else if hostname, err := system.Hostname(); err == nil { - ee["name"] = hostname - } else { - return nil, errors.New("Unable to determine default etcd name") - } - } - - etcd := system.Unit{ - Name: "etcd.service", - Runtime: true, - DropIn: true, - Content: ee.String(), - } - return []system.Unit{etcd}, nil -} diff --git a/initialize/etcd_test.go b/initialize/etcd_test.go deleted file mode 100644 index 4f7a9dc..0000000 --- a/initialize/etcd_test.go +++ /dev/null @@ -1,184 +0,0 @@ -package initialize - -import ( - "io/ioutil" - "os" - "path" - "testing" - - "github.com/coreos/coreos-cloudinit/system" -) - -func TestEtcdEnvironment(t *testing.T) { - cfg := make(EtcdEnvironment, 0) - cfg["discovery"] = "http://disco.example.com/foobar" - cfg["peer-bind-addr"] = "127.0.0.1:7002" - - env := cfg.String() - expect := `[Service] -Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" -Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" -` - - if env != expect { - t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect) - } -} - -func TestEtcdEnvironmentDiscoveryURLTranslated(t *testing.T) { - cfg := make(EtcdEnvironment, 0) - cfg["discovery_url"] = "http://disco.example.com/foobar" - cfg["peer-bind-addr"] = "127.0.0.1:7002" - - env := cfg.String() - expect := `[Service] -Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" -Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" -` - - if env != expect { - t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect) - } -} - -func TestEtcdEnvironmentDiscoveryOverridesDiscoveryURL(t *testing.T) { - cfg := make(EtcdEnvironment, 0) - cfg["discovery_url"] = "ping" - cfg["discovery"] = "pong" - cfg["peer-bind-addr"] = "127.0.0.1:7002" - - env := cfg.String() - expect := `[Service] -Environment="ETCD_DISCOVERY=pong" -Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" -` - - if env != expect { - t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect) - } -} - -func TestEtcdEnvironmentWrittenToDisk(t *testing.T) { - ee := EtcdEnvironment{ - "name": "node001", - "discovery": "http://disco.example.com/foobar", - "peer-bind-addr": "127.0.0.1:7002", - } - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - - sd := system.NewUnitManager(dir) - - uu, err := ee.Units(dir) - if err != nil { - t.Fatalf("Generating etcd unit failed: %v", err) - } - if len(uu) != 1 { - t.Fatalf("Expected 1 unit to be returned, got %d", len(uu)) - } - u := uu[0] - - dst := u.Destination(dir) - os.Stderr.WriteString("writing to " + dir + "\n") - if err := sd.PlaceUnit(&u, dst); err != nil { - t.Fatalf("Writing of EtcdEnvironment failed: %v", err) - } - - fullPath := path.Join(dir, "run", "systemd", "system", "etcd.service.d", "20-cloudinit.conf") - - fi, err := os.Stat(fullPath) - if err != nil { - t.Fatalf("Unable to stat file: %v", err) - } - - if fi.Mode() != os.FileMode(0644) { - t.Errorf("File has incorrect mode: %v", fi.Mode()) - } - - contents, err := ioutil.ReadFile(fullPath) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } - - expect := `[Service] -Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" -Environment="ETCD_NAME=node001" -Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" -` - if string(contents) != expect { - t.Fatalf("File has incorrect contents") - } -} - -func TestEtcdEnvironmentEmptyNoOp(t *testing.T) { - ee := EtcdEnvironment{} - uu, err := ee.Units("") - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if len(uu) > 0 { - t.Fatalf("Generated etcd units unexpectedly: %v") - } -} - -func TestEtcdEnvironmentWrittenToDiskDefaultToMachineID(t *testing.T) { - ee := EtcdEnvironment{"foo": "bar"} - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - - sd := system.NewUnitManager(dir) - - os.Mkdir(path.Join(dir, "etc"), os.FileMode(0755)) - err = ioutil.WriteFile(path.Join(dir, "etc", "machine-id"), []byte("node007"), os.FileMode(0444)) - if err != nil { - t.Fatalf("Failed writing out /etc/machine-id: %v", err) - } - - uu, err := ee.Units(dir) - if err != nil { - t.Fatalf("Generating etcd unit failed: %v", err) - } - if len(uu) == 0 { - t.Fatalf("Returned empty etcd units unexpectedly") - } - u := uu[0] - - dst := u.Destination(dir) - os.Stderr.WriteString("writing to " + dir + "\n") - if err := sd.PlaceUnit(&u, dst); err != nil { - t.Fatalf("Writing of EtcdEnvironment failed: %v", err) - } - - fullPath := path.Join(dir, "run", "systemd", "system", "etcd.service.d", "20-cloudinit.conf") - - contents, err := ioutil.ReadFile(fullPath) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } - - expect := `[Service] -Environment="ETCD_FOO=bar" -Environment="ETCD_NAME=node007" -` - if string(contents) != expect { - t.Fatalf("File has incorrect contents") - } -} - -func TestEtcdEnvironmentWhenNil(t *testing.T) { - // EtcdEnvironment will be a nil map if it wasn't in the yaml - var ee EtcdEnvironment - if ee != nil { - t.Fatalf("EtcdEnvironment is not nil") - } - uu, err := ee.Units("") - if len(uu) != 0 || err != nil { - t.Fatalf("Units returned value for nil input") - } -} diff --git a/system/env.go b/system/env.go new file mode 100644 index 0000000..acb413a --- /dev/null +++ b/system/env.go @@ -0,0 +1,26 @@ +package system + +import ( + "fmt" + "reflect" +) + +// dropinContents generates the contents for a drop-in unit given the config. +// The argument must be a struct from the 'config' package. +func dropinContents(e interface{}) string { + et := reflect.TypeOf(e) + ev := reflect.ValueOf(e) + + var out string + for i := 0; i < et.NumField(); i++ { + if val := ev.Field(i).String(); val != "" { + key := et.Field(i).Tag.Get("env") + out += fmt.Sprintf("Environment=\"%s=%s\"\n", key, val) + } + } + + if out == "" { + return "" + } + return "[Service]\n" + out +} diff --git a/system/etcd.go b/system/etcd.go new file mode 100644 index 0000000..86cb6c1 --- /dev/null +++ b/system/etcd.go @@ -0,0 +1,25 @@ +package system + +import ( + "github.com/coreos/coreos-cloudinit/config" +) + +// Etcd is a top-level structure which embeds its underlying configuration, +// config.Etcd, and provides the system-specific Unit(). +type Etcd struct { + config.Etcd +} + +// Units creates a Unit file drop-in for etcd, using any configured options. +func (ee Etcd) Units(_ string) ([]Unit, error) { + content := dropinContents(ee.Etcd) + if content == "" { + return nil, nil + } + return []Unit{{ + Name: "etcd.service", + Runtime: true, + DropIn: true, + Content: content, + }}, nil +} diff --git a/system/etcd_test.go b/system/etcd_test.go new file mode 100644 index 0000000..68aee41 --- /dev/null +++ b/system/etcd_test.go @@ -0,0 +1,60 @@ +package system + +import ( + "reflect" + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) + +func TestEtcdUnits(t *testing.T) { + for _, tt := range []struct { + config config.Etcd + units []Unit + }{ + { + config.Etcd{}, + nil, + }, + { + config.Etcd{ + Discovery: "http://disco.example.com/foobar", + PeerBindAddr: "127.0.0.1:7002", + }, + []Unit{{ + Name: "etcd.service", + Runtime: true, + DropIn: true, + Content: `[Service] +Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" +Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" +`, + }}, + }, + { + config.Etcd{ + Name: "node001", + Discovery: "http://disco.example.com/foobar", + PeerBindAddr: "127.0.0.1:7002", + }, + []Unit{{ + Name: "etcd.service", + Runtime: true, + DropIn: true, + Content: `[Service] +Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" +Environment="ETCD_NAME=node001" +Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" +`, + }}, + }, + } { + units, err := Etcd{tt.config}.Units("") + if err != nil { + t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err) + } + if !reflect.DeepEqual(tt.units, units) { + t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.units, units) + } + } +} From 9454522033b75e54d30474086c615dacfa7c31d2 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 19:28:04 -0700 Subject: [PATCH 2/8] fleet: refactor config - Explicitly specify all of the valid options for fleet - Seperate the config from Units() - Add YAML tags for the fields --- config/fleet.go | 14 +++++++++++++ initialize/config.go | 4 ++-- initialize/env.go | 13 ------------ initialize/fleet.go | 35 -------------------------------- initialize/fleet_test.go | 43 ---------------------------------------- system/fleet.go | 26 ++++++++++++++++++++++++ system/fleet_test.go | 41 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 83 insertions(+), 93 deletions(-) create mode 100644 config/fleet.go delete mode 100644 initialize/fleet.go delete mode 100644 initialize/fleet_test.go create mode 100644 system/fleet.go create mode 100644 system/fleet_test.go diff --git a/config/fleet.go b/config/fleet.go new file mode 100644 index 0000000..41268b7 --- /dev/null +++ b/config/fleet.go @@ -0,0 +1,14 @@ +package config + +type Fleet struct { + AgentTTL string `yaml:"agent-ttl" env:"FLEET_AGENT_TTL"` + EngineReconcileInterval string `yaml:"engine-reconcile-interval" env:"FLEET_ENGINE_RECONCILE_INTERVAL"` + EtcdCAFile string `yaml:"etcd-cafile" env:"FLEET_ETCD_CAFILE"` + EtcdCertFile string `yaml:"etcd-certfile" env:"FLEET_ETCD_CERTFILE"` + EtcdKeyFile string `yaml:"etcd-keyfile" env:"FLEET_ETCD_KEYFILE"` + EtcdRequestTimeout string `yaml:"etcd-request-timeout" env:"FLEET_ETCD_REQUEST_TIMEOUT"` + EtcdServers string `yaml:"etcd-servers" env:"FLEET_ETCD_SERVERS"` + Metadata string `yaml:"metadata" env:"FLEET_METADATA"` + PublicIP string `yaml:"public-ip" env:"FLEET_PUBLIC_IP"` + Verbosity string `yaml:"verbosity" env:"FLEET_VERBOSITY"` +} diff --git a/initialize/config.go b/initialize/config.go index 384b9ba..957abfc 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -32,7 +32,7 @@ type CloudConfig struct { SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"` Coreos struct { Etcd config.Etcd - Fleet FleetEnvironment + Fleet config.Fleet OEM OEMRelease Update UpdateConfig Units []system.Unit @@ -227,7 +227,7 @@ func Apply(cfg CloudConfig, env *Environment) error { } } - for _, ccu := range []CloudConfigUnit{system.Etcd{cfg.Coreos.Etcd}, cfg.Coreos.Fleet, cfg.Coreos.Update} { + for _, ccu := range []CloudConfigUnit{system.Etcd{cfg.Coreos.Etcd}, system.Fleet{cfg.Coreos.Fleet}, cfg.Coreos.Update} { u, err := ccu.Units(env.Root()) if err != nil { return err diff --git a/initialize/env.go b/initialize/env.go index e34c268..69e528e 100644 --- a/initialize/env.go +++ b/initialize/env.go @@ -104,16 +104,3 @@ func (e *Environment) DefaultEnvironmentFile() *system.EnvFile { return &ef } } - -// normalizeSvcEnv standardizes the keys of the map (environment variables for a service) -// by replacing any dashes with underscores and ensuring they are entirely upper case. -// For example, "some-env" --> "SOME_ENV" -func normalizeSvcEnv(m map[string]string) map[string]string { - out := make(map[string]string, len(m)) - for key, val := range m { - key = strings.ToUpper(key) - key = strings.Replace(key, "-", "_", -1) - out[key] = val - } - return out -} diff --git a/initialize/fleet.go b/initialize/fleet.go deleted file mode 100644 index c03e1d1..0000000 --- a/initialize/fleet.go +++ /dev/null @@ -1,35 +0,0 @@ -package initialize - -import ( - "fmt" - - "github.com/coreos/coreos-cloudinit/system" -) - -type FleetEnvironment map[string]string - -func (fe FleetEnvironment) String() (out string) { - norm := normalizeSvcEnv(fe) - out += "[Service]\n" - - for key, val := range norm { - out += fmt.Sprintf("Environment=\"FLEET_%s=%s\"\n", key, val) - } - - return -} - -// Units generates a Unit file drop-in for fleet, if any fleet options were -// configured in cloud-config -func (fe FleetEnvironment) Units(root string) ([]system.Unit, error) { - if len(fe) < 1 { - return nil, nil - } - fleet := system.Unit{ - Name: "fleet.service", - Runtime: true, - DropIn: true, - Content: fe.String(), - } - return []system.Unit{fleet}, nil -} diff --git a/initialize/fleet_test.go b/initialize/fleet_test.go deleted file mode 100644 index 481a8de..0000000 --- a/initialize/fleet_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package initialize - -import "testing" - -func TestFleetEnvironment(t *testing.T) { - cfg := make(FleetEnvironment, 0) - cfg["public-ip"] = "12.34.56.78" - - env := cfg.String() - - expect := `[Service] -Environment="FLEET_PUBLIC_IP=12.34.56.78" -` - - if env != expect { - t.Errorf("Generated environment:\n%s\nExpected environment:\n%s", env, expect) - } -} - -func TestFleetUnit(t *testing.T) { - cfg := make(FleetEnvironment, 0) - uu, err := cfg.Units("/") - if len(uu) != 0 { - t.Errorf("unexpectedly generated unit with empty FleetEnvironment") - } - - cfg["public-ip"] = "12.34.56.78" - - uu, err = cfg.Units("/") - if err != nil { - t.Errorf("error generating fleet unit: %v", err) - } - if len(uu) != 1 { - t.Fatalf("expected 1 unit generated, got %d", len(uu)) - } - u := uu[0] - if !u.Runtime { - t.Errorf("bad Runtime for generated fleet unit!") - } - if !u.DropIn { - t.Errorf("bad DropIn for generated fleet unit!") - } -} diff --git a/system/fleet.go b/system/fleet.go new file mode 100644 index 0000000..50f288a --- /dev/null +++ b/system/fleet.go @@ -0,0 +1,26 @@ +package system + +import ( + "github.com/coreos/coreos-cloudinit/config" +) + +// Fleet is a top-level structure which embeds its underlying configuration, +// config.Fleet, and provides the system-specific Unit(). +type Fleet struct { + config.Fleet +} + +// Units generates a Unit file drop-in for fleet, if any fleet options were +// configured in cloud-config +func (fe Fleet) Units(_ string) ([]Unit, error) { + content := dropinContents(fe.Fleet) + if content == "" { + return nil, nil + } + return []Unit{{ + Name: "fleet.service", + Runtime: true, + DropIn: true, + Content: content, + }}, nil +} diff --git a/system/fleet_test.go b/system/fleet_test.go new file mode 100644 index 0000000..4618d5d --- /dev/null +++ b/system/fleet_test.go @@ -0,0 +1,41 @@ +package system + +import ( + "reflect" + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) + +func TestFleetUnits(t *testing.T) { + for _, tt := range []struct { + config config.Fleet + units []Unit + }{ + { + config.Fleet{}, + nil, + }, + { + config.Fleet{ + PublicIP: "12.34.56.78", + }, + []Unit{{ + Name: "fleet.service", + Content: `[Service] +Environment="FLEET_PUBLIC_IP=12.34.56.78" +`, + Runtime: true, + DropIn: true, + }}, + }, + } { + units, err := Fleet{tt.config}.Units("") + if err != nil { + t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err) + } + if !reflect.DeepEqual(units, tt.units) { + t.Errorf("bad units (%q): want %q, got %q", tt.config, tt.units, units) + } + } +} From 6730cb7227889d3795e16d91703b6e03e61ec67e Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 12:14:00 -0700 Subject: [PATCH 3/8] oem: refactor the config - Seperate the config from File() - Add YAML tags for the fields --- config/oem.go | 9 ++++++ initialize/config.go | 4 +-- initialize/oem.go | 41 --------------------------- initialize/oem_test.go | 63 ------------------------------------------ system/oem.go | 32 +++++++++++++++++++++ system/oem_test.go | 47 +++++++++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 106 deletions(-) create mode 100644 config/oem.go delete mode 100644 initialize/oem.go delete mode 100644 initialize/oem_test.go create mode 100644 system/oem.go create mode 100644 system/oem_test.go diff --git a/config/oem.go b/config/oem.go new file mode 100644 index 0000000..61d0ae9 --- /dev/null +++ b/config/oem.go @@ -0,0 +1,9 @@ +package config + +type OEM struct { + ID string `yaml:"id"` + Name string `yaml:"name"` + VersionID string `yaml:"version-id"` + HomeURL string `yaml:"home-url"` + BugReportURL string `yaml:"bug-report-url"` +} diff --git a/initialize/config.go b/initialize/config.go index 957abfc..4e28579 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -33,7 +33,7 @@ type CloudConfig struct { Coreos struct { Etcd config.Etcd Fleet config.Fleet - OEM OEMRelease + OEM config.OEM Update UpdateConfig Units []system.Unit } @@ -217,7 +217,7 @@ func Apply(cfg CloudConfig, env *Environment) error { } } - for _, ccf := range []CloudConfigFile{cfg.Coreos.OEM, cfg.Coreos.Update, cfg.ManageEtcHosts} { + for _, ccf := range []CloudConfigFile{system.OEM{cfg.Coreos.OEM}, cfg.Coreos.Update, cfg.ManageEtcHosts} { f, err := ccf.File(env.Root()) if err != nil { return err diff --git a/initialize/oem.go b/initialize/oem.go deleted file mode 100644 index aea43bc..0000000 --- a/initialize/oem.go +++ /dev/null @@ -1,41 +0,0 @@ -package initialize - -import ( - "fmt" - "path" - "strings" - - "github.com/coreos/coreos-cloudinit/system" -) - -type OEMRelease struct { - ID string `yaml:"id"` - Name string `yaml:"name"` - VersionID string `yaml:"version-id"` - HomeURL string `yaml:"home-url"` - BugReportURL string `yaml:"bug-report-url"` -} - -func (oem OEMRelease) String() string { - fields := []string{ - fmt.Sprintf("ID=%s", oem.ID), - fmt.Sprintf("VERSION_ID=%s", oem.VersionID), - fmt.Sprintf("NAME=%q", oem.Name), - fmt.Sprintf("HOME_URL=%q", oem.HomeURL), - fmt.Sprintf("BUG_REPORT_URL=%q", oem.BugReportURL), - } - - return strings.Join(fields, "\n") + "\n" -} - -func (oem OEMRelease) File(root string) (*system.File, error) { - if oem.ID == "" { - return nil, nil - } - - return &system.File{ - Path: path.Join("etc", "oem-release"), - RawFilePermissions: "0644", - Content: oem.String(), - }, nil -} diff --git a/initialize/oem_test.go b/initialize/oem_test.go deleted file mode 100644 index a2eae46..0000000 --- a/initialize/oem_test.go +++ /dev/null @@ -1,63 +0,0 @@ -package initialize - -import ( - "io/ioutil" - "os" - "path" - "testing" - - "github.com/coreos/coreos-cloudinit/system" -) - -func TestOEMReleaseWrittenToDisk(t *testing.T) { - oem := OEMRelease{ - ID: "rackspace", - Name: "Rackspace Cloud Servers", - VersionID: "168.0.0", - HomeURL: "https://www.rackspace.com/cloud/servers/", - BugReportURL: "https://github.com/coreos/coreos-overlay", - } - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - - f, err := oem.File(dir) - if err != nil { - t.Fatalf("Processing of OEMRelease failed: %v", err) - } - if f == nil { - t.Fatalf("OEMRelease returned nil file unexpectedly") - } - - if _, err := system.WriteFile(f, dir); err != nil { - t.Fatalf("Writing of OEMRelease failed: %v", err) - } - - fullPath := path.Join(dir, "etc", "oem-release") - - fi, err := os.Stat(fullPath) - if err != nil { - t.Fatalf("Unable to stat file: %v", err) - } - - if fi.Mode() != os.FileMode(0644) { - t.Errorf("File has incorrect mode: %v", fi.Mode()) - } - - contents, err := ioutil.ReadFile(fullPath) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } - - expect := `ID=rackspace -VERSION_ID=168.0.0 -NAME="Rackspace Cloud Servers" -HOME_URL="https://www.rackspace.com/cloud/servers/" -BUG_REPORT_URL="https://github.com/coreos/coreos-overlay" -` - if string(contents) != expect { - t.Fatalf("File has incorrect contents") - } -} diff --git a/system/oem.go b/system/oem.go new file mode 100644 index 0000000..ba3634f --- /dev/null +++ b/system/oem.go @@ -0,0 +1,32 @@ +package system + +import ( + "fmt" + "path" + + "github.com/coreos/coreos-cloudinit/config" +) + +// OEM is a top-level structure which embeds its underlying configuration, +// config.OEM, and provides the system-specific File(). +type OEM struct { + config.OEM +} + +func (oem OEM) File(_ string) (*File, error) { + if oem.ID == "" { + return nil, nil + } + + content := fmt.Sprintf("ID=%s\n", oem.ID) + content += fmt.Sprintf("VERSION_ID=%s\n", oem.VersionID) + content += fmt.Sprintf("NAME=%q\n", oem.Name) + content += fmt.Sprintf("HOME_URL=%q\n", oem.HomeURL) + content += fmt.Sprintf("BUG_REPORT_URL=%q\n", oem.BugReportURL) + + return &File{ + Path: path.Join("etc", "oem-release"), + RawFilePermissions: "0644", + Content: content, + }, nil +} diff --git a/system/oem_test.go b/system/oem_test.go new file mode 100644 index 0000000..4a8029e --- /dev/null +++ b/system/oem_test.go @@ -0,0 +1,47 @@ +package system + +import ( + "reflect" + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) + +func TestOEMFile(t *testing.T) { + for _, tt := range []struct { + config config.OEM + file *File + }{ + { + config.OEM{}, + nil, + }, + { + config.OEM{ + ID: "rackspace", + Name: "Rackspace Cloud Servers", + VersionID: "168.0.0", + HomeURL: "https://www.rackspace.com/cloud/servers/", + BugReportURL: "https://github.com/coreos/coreos-overlay", + }, + &File{ + Path: "etc/oem-release", + RawFilePermissions: "0644", + Content: `ID=rackspace +VERSION_ID=168.0.0 +NAME="Rackspace Cloud Servers" +HOME_URL="https://www.rackspace.com/cloud/servers/" +BUG_REPORT_URL="https://github.com/coreos/coreos-overlay" +`, + }, + }, + } { + file, err := OEM{tt.config}.File("") + if err != nil { + t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err) + } + if !reflect.DeepEqual(tt.file, file) { + t.Errorf("bad file (%q): want %#v, got %#v", tt.config, tt.file, file) + } + } +} From 667dbd8fb7d5010a6234f0d6b71d20acc448d1c4 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 19:22:13 -0700 Subject: [PATCH 4/8] update: refactor config - Explicitly specify all of the valid options for Update - Seperate the config from File() and Units() - Add YAML tags for the fields --- config/config.go | 66 +++++ config/config_test.go | 63 +++++ config/etc_hosts.go | 3 + config/update.go | 7 + coreos-cloudinit_test.go | 7 +- initialize/config.go | 24 +- initialize/config_test.go | 2 +- initialize/manage_etc_hosts_test.go | 83 ------- initialize/update.go | 165 ------------- initialize/update_test.go | 232 ------------------ .../etc_hosts.go | 16 +- system/etc_hosts_test.go | 46 ++++ system/etcd.go | 2 +- system/etcd_test.go | 2 +- system/fleet.go | 2 +- system/fleet_test.go | 2 +- system/oem.go | 2 +- system/oem_test.go | 2 +- system/update.go | 137 +++++++++++ system/update_test.go | 151 ++++++++++++ test | 29 ++- 21 files changed, 525 insertions(+), 518 deletions(-) create mode 100644 config/config.go create mode 100644 config/config_test.go create mode 100644 config/etc_hosts.go create mode 100644 config/update.go delete mode 100644 initialize/manage_etc_hosts_test.go delete mode 100644 initialize/update.go delete mode 100644 initialize/update_test.go rename initialize/manage_etc_hosts.go => system/etc_hosts.go (73%) create mode 100644 system/etc_hosts_test.go create mode 100644 system/update.go create mode 100644 system/update_test.go diff --git a/config/config.go b/config/config.go new file mode 100644 index 0000000..7ca7a42 --- /dev/null +++ b/config/config.go @@ -0,0 +1,66 @@ +package config + +import ( + "fmt" + "reflect" + "strings" +) + +// IsZero returns whether or not the parameter is the zero value for its type. +// If the parameter is a struct, only the exported fields are considered. +func IsZero(c interface{}) bool { + return isZero(reflect.ValueOf(c)) +} + +// AssertValid checks the fields in the structure and makes sure that they +// contain valid values as specified by the 'valid' flag. Empty fields are +// implicitly valid. +func AssertValid(c interface{}) error { + ct := reflect.TypeOf(c) + cv := reflect.ValueOf(c) + for i := 0; i < ct.NumField(); i++ { + ft := ct.Field(i) + if !isFieldExported(ft) { + continue + } + + valid := ft.Tag.Get("valid") + val := cv.Field(i) + if !isValid(val, valid) { + return fmt.Errorf("invalid value \"%v\" for option %q (valid options: %q)", val.Interface(), ft.Name, valid) + } + } + return nil +} + +func isZero(v reflect.Value) bool { + switch v.Kind() { + case reflect.Struct: + vt := v.Type() + for i := 0; i < v.NumField(); i++ { + if isFieldExported(vt.Field(i)) && !isZero(v.Field(i)) { + return false + } + } + return true + default: + return v.Interface() == reflect.Zero(v.Type()).Interface() + } +} + +func isFieldExported(f reflect.StructField) bool { + return f.PkgPath == "" +} + +func isValid(v reflect.Value, valid string) bool { + if valid == "" || isZero(v) { + return true + } + vs := fmt.Sprintf("%v", v.Interface()) + for _, valid := range strings.Split(valid, ",") { + if vs == valid { + return true + } + } + return false +} diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 0000000..9042f87 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,63 @@ +package config + +import ( + "errors" + "reflect" + "testing" +) + +func TestIsZero(t *testing.T) { + for _, tt := range []struct { + c interface{} + empty bool + }{ + {struct{}{}, true}, + {struct{ a, b string }{}, true}, + {struct{ A, b string }{}, true}, + {struct{ A, B string }{}, true}, + {struct{ A string }{A: "hello"}, false}, + {struct{ A int }{}, true}, + {struct{ A int }{A: 1}, false}, + } { + if empty := IsZero(tt.c); tt.empty != empty { + t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.empty, empty) + } + } +} + +func TestAssertValid(t *testing.T) { + for _, tt := range []struct { + c interface{} + err error + }{ + {struct{}{}, nil}, + {struct { + A, b string `valid:"1,2"` + }{}, nil}, + {struct { + A, b string `valid:"1,2"` + }{A: "1", b: "2"}, nil}, + {struct { + A, b string `valid:"1,2"` + }{A: "1", b: "hello"}, nil}, + {struct { + A, b string `valid:"1,2"` + }{A: "hello", b: "2"}, errors.New("invalid value \"hello\" for option \"A\" (valid options: \"1,2\")")}, + {struct { + A, b int `valid:"1,2"` + }{}, nil}, + {struct { + A, b int `valid:"1,2"` + }{A: 1, b: 2}, nil}, + {struct { + A, b int `valid:"1,2"` + }{A: 1, b: 9}, nil}, + {struct { + A, b int `valid:"1,2"` + }{A: 9, b: 2}, errors.New("invalid value \"9\" for option \"A\" (valid options: \"1,2\")")}, + } { + if err := AssertValid(tt.c); !reflect.DeepEqual(tt.err, err) { + t.Errorf("bad result (%q): want %q, got %q", tt.c, tt.err, err) + } + } +} diff --git a/config/etc_hosts.go b/config/etc_hosts.go new file mode 100644 index 0000000..5c1a9f7 --- /dev/null +++ b/config/etc_hosts.go @@ -0,0 +1,3 @@ +package config + +type EtcHosts string diff --git a/config/update.go b/config/update.go new file mode 100644 index 0000000..0352f9a --- /dev/null +++ b/config/update.go @@ -0,0 +1,7 @@ +package config + +type Update struct { + RebootStrategy string `yaml:"reboot-strategy" env:"REBOOT_STRATEGY" valid:"best-effort,etcd-lock,reboot,off"` + Group string `yaml:"group" env:"GROUP"` + Server string `yaml:"server" env:"SERVER"` +} diff --git a/coreos-cloudinit_test.go b/coreos-cloudinit_test.go index 81687da..cd86284 100644 --- a/coreos-cloudinit_test.go +++ b/coreos-cloudinit_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/coreos/coreos-cloudinit/initialize" + "github.com/coreos/coreos-cloudinit/config" ) func TestMergeCloudConfig(t *testing.T) { @@ -81,7 +82,7 @@ func TestMergeCloudConfig(t *testing.T) { // Non-mergeable settings in user-data should not be affected initialize.CloudConfig{ Hostname: "mememe", - ManageEtcHosts: initialize.EtcHosts("lolz"), + ManageEtcHosts: config.EtcHosts("lolz"), }, initialize.CloudConfig{ Hostname: "youyouyou", @@ -90,7 +91,7 @@ func TestMergeCloudConfig(t *testing.T) { }, initialize.CloudConfig{ Hostname: "mememe", - ManageEtcHosts: initialize.EtcHosts("lolz"), + ManageEtcHosts: config.EtcHosts("lolz"), NetworkConfigPath: "meta-meta-yo", NetworkConfig: `{"hostname":"test"}`, }, @@ -101,7 +102,7 @@ func TestMergeCloudConfig(t *testing.T) { Hostname: "mememe", }, initialize.CloudConfig{ - ManageEtcHosts: initialize.EtcHosts("lolz"), + ManageEtcHosts: config.EtcHosts("lolz"), NetworkConfigPath: "meta-meta-yo", NetworkConfig: `{"hostname":"test"}`, }, diff --git a/initialize/config.go b/initialize/config.go index 4e28579..0eb6163 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -18,13 +18,13 @@ import ( type CloudConfigFile interface { // File should either return (*system.File, error), or (nil, nil) if nothing // needs to be done for this configuration option. - File(root string) (*system.File, error) + File() (*system.File, error) } // CloudConfigUnit represents a CoreOS specific configuration option that can generate // associated system.Units to be created/enabled appropriately type CloudConfigUnit interface { - Units(root string) ([]system.Unit, error) + Units() ([]system.Unit, error) } // CloudConfig encapsulates the entire cloud-config configuration file and maps directly to YAML @@ -34,13 +34,13 @@ type CloudConfig struct { Etcd config.Etcd Fleet config.Fleet OEM config.OEM - Update UpdateConfig + Update config.Update Units []system.Unit } WriteFiles []system.File `yaml:"write_files"` Hostname string Users []system.User - ManageEtcHosts EtcHosts `yaml:"manage_etc_hosts"` + ManageEtcHosts config.EtcHosts `yaml:"manage_etc_hosts"` NetworkConfigPath string NetworkConfig string } @@ -217,8 +217,12 @@ func Apply(cfg CloudConfig, env *Environment) error { } } - for _, ccf := range []CloudConfigFile{system.OEM{cfg.Coreos.OEM}, cfg.Coreos.Update, cfg.ManageEtcHosts} { - f, err := ccf.File(env.Root()) + for _, ccf := range []CloudConfigFile{ + system.OEM{cfg.Coreos.OEM}, + system.Update{cfg.Coreos.Update, system.DefaultReadConfig}, + system.EtcHosts{cfg.ManageEtcHosts}, + } { + f, err := ccf.File() if err != nil { return err } @@ -227,8 +231,12 @@ func Apply(cfg CloudConfig, env *Environment) error { } } - for _, ccu := range []CloudConfigUnit{system.Etcd{cfg.Coreos.Etcd}, system.Fleet{cfg.Coreos.Fleet}, cfg.Coreos.Update} { - u, err := ccu.Units(env.Root()) + for _, ccu := range []CloudConfigUnit{ + system.Etcd{cfg.Coreos.Etcd}, + system.Fleet{cfg.Coreos.Fleet}, + system.Update{cfg.Coreos.Update, system.DefaultReadConfig}, + } { + u, err := ccu.Units() if err != nil { return err } diff --git a/initialize/config_test.go b/initialize/config_test.go index b098a79..cd84438 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -226,7 +226,7 @@ Address=10.209.171.177/19 if cfg.Hostname != "trontastic" { t.Errorf("Failed to parse hostname") } - if cfg.Coreos.Update["reboot-strategy"] != "reboot" { + if cfg.Coreos.Update.RebootStrategy != "reboot" { t.Errorf("Failed to parse locksmith strategy") } } diff --git a/initialize/manage_etc_hosts_test.go b/initialize/manage_etc_hosts_test.go deleted file mode 100644 index 3e23b10..0000000 --- a/initialize/manage_etc_hosts_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package initialize - -import ( - "fmt" - "io/ioutil" - "os" - "path" - "testing" - - "github.com/coreos/coreos-cloudinit/system" -) - -func TestCloudConfigManageEtcHosts(t *testing.T) { - contents := ` -manage_etc_hosts: localhost -` - cfg, err := NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error: %v", err) - } - - manageEtcHosts := cfg.ManageEtcHosts - - if manageEtcHosts != "localhost" { - t.Errorf("ManageEtcHosts value is %q, expected 'localhost'", manageEtcHosts) - } -} - -func TestManageEtcHostsInvalidValue(t *testing.T) { - eh := EtcHosts("invalid") - if f, err := eh.File(""); err == nil || f != nil { - t.Fatalf("EtcHosts File succeeded with invalid value!") - } -} - -func TestEtcHostsWrittenToDisk(t *testing.T) { - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - - eh := EtcHosts("localhost") - - f, err := eh.File(dir) - if err != nil { - t.Fatalf("Error calling File on EtcHosts: %v", err) - } - if f == nil { - t.Fatalf("manageEtcHosts returned nil file unexpectedly") - } - - if _, err := system.WriteFile(f, dir); err != nil { - t.Fatalf("Error writing EtcHosts: %v", err) - } - - fullPath := path.Join(dir, "etc", "hosts") - - fi, err := os.Stat(fullPath) - if err != nil { - t.Fatalf("Unable to stat file: %v", err) - } - - if fi.Mode() != os.FileMode(0644) { - t.Errorf("File has incorrect mode: %v", fi.Mode()) - } - - contents, err := ioutil.ReadFile(fullPath) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } - - hostname, err := os.Hostname() - if err != nil { - t.Fatalf("Unable to read OS hostname: %v", err) - } - - expect := fmt.Sprintf("%s %s\n", DefaultIpv4Address, hostname) - - if string(contents) != expect { - t.Fatalf("File has incorrect contents") - } -} diff --git a/initialize/update.go b/initialize/update.go deleted file mode 100644 index d7a5ecc..0000000 --- a/initialize/update.go +++ /dev/null @@ -1,165 +0,0 @@ -package initialize - -import ( - "bufio" - "errors" - "fmt" - "os" - "path" - "strings" - - "github.com/coreos/coreos-cloudinit/system" -) - -const ( - locksmithUnit = "locksmithd.service" - updateEngineUnit = "update-engine.service" -) - -// updateOption represents a configurable update option, which, if set, will be -// written into update.conf, replacing any existing value for the option -type updateOption struct { - key string // key used to configure this option in cloud-config - valid []string // valid values for the option - prefix string // prefix for the option in the update.conf file - value string // used to store the new value in update.conf (including prefix) - seen bool // whether the option has been seen in any existing update.conf -} - -// updateOptions defines the update options understood by cloud-config. -// The keys represent the string used in cloud-config to configure the option. -var updateOptions = []*updateOption{ - &updateOption{ - key: "reboot-strategy", - prefix: "REBOOT_STRATEGY=", - valid: []string{"best-effort", "etcd-lock", "reboot", "off"}, - }, - &updateOption{ - key: "group", - prefix: "GROUP=", - }, - &updateOption{ - key: "server", - prefix: "SERVER=", - }, -} - -// isValid checks whether a supplied value is valid for this option -func (uo updateOption) isValid(val string) bool { - if len(uo.valid) == 0 { - return true - } - for _, v := range uo.valid { - if val == v { - return true - } - } - return false -} - -type UpdateConfig map[string]string - -// File generates an `/etc/coreos/update.conf` file (if any update -// configuration options are set in cloud-config) by either rewriting the -// existing file on disk, or starting from `/usr/share/coreos/update.conf` -func (uc UpdateConfig) File(root string) (*system.File, error) { - if len(uc) < 1 { - return nil, nil - } - - var out string - - // Generate the list of possible substitutions to be performed based on the options that are configured - subs := make([]*updateOption, 0) - for _, uo := range updateOptions { - val, ok := uc[uo.key] - if !ok { - continue - } - if !uo.isValid(val) { - return nil, errors.New(fmt.Sprintf("invalid value %v for option %v (valid options: %v)", val, uo.key, uo.valid)) - } - uo.value = uo.prefix + val - subs = append(subs, uo) - } - - etcUpdate := path.Join(root, "etc", "coreos", "update.conf") - usrUpdate := path.Join(root, "usr", "share", "coreos", "update.conf") - - conf, err := os.Open(etcUpdate) - if os.IsNotExist(err) { - conf, err = os.Open(usrUpdate) - } - if err != nil { - return nil, err - } - - scanner := bufio.NewScanner(conf) - - for scanner.Scan() { - line := scanner.Text() - for _, s := range subs { - if strings.HasPrefix(line, s.prefix) { - line = s.value - s.seen = true - break - } - } - out += line - out += "\n" - if err := scanner.Err(); err != nil { - return nil, err - } - } - - for _, s := range subs { - if !s.seen { - out += s.value - out += "\n" - } - } - - return &system.File{ - Path: path.Join("etc", "coreos", "update.conf"), - RawFilePermissions: "0644", - Content: out, - }, nil -} - -// Units generates units for the cloud-init initializer to act on: -// - a locksmith system.Unit, if "reboot-strategy" was set in cloud-config -// - an update_engine system.Unit, if "group" was set in cloud-config -func (uc UpdateConfig) Units(root string) ([]system.Unit, error) { - var units []system.Unit - if strategy, ok := uc["reboot-strategy"]; ok { - ls := &system.Unit{ - Name: locksmithUnit, - Command: "restart", - Mask: false, - Runtime: true, - } - - if strategy == "off" { - ls.Command = "stop" - ls.Mask = true - } - units = append(units, *ls) - } - - rue := false - if _, ok := uc["group"]; ok { - rue = true - } - if _, ok := uc["server"]; ok { - rue = true - } - if rue { - ue := system.Unit{ - Name: updateEngineUnit, - Command: "restart", - } - units = append(units, ue) - } - - return units, nil -} diff --git a/initialize/update_test.go b/initialize/update_test.go deleted file mode 100644 index a1f87e2..0000000 --- a/initialize/update_test.go +++ /dev/null @@ -1,232 +0,0 @@ -package initialize - -import ( - "io/ioutil" - "os" - "path" - "sort" - "strings" - "testing" - - "github.com/coreos/coreos-cloudinit/system" -) - -const ( - base = `SERVER=https://example.com -GROUP=thegroupc` - configured = base + ` -REBOOT_STRATEGY=awesome -` - expected = base + ` -REBOOT_STRATEGY=etcd-lock -` -) - -func setupFixtures(dir string) { - os.MkdirAll(path.Join(dir, "usr", "share", "coreos"), 0755) - os.MkdirAll(path.Join(dir, "run", "systemd", "system"), 0755) - - ioutil.WriteFile(path.Join(dir, "usr", "share", "coreos", "update.conf"), []byte(base), 0644) -} - -func TestEmptyUpdateConfig(t *testing.T) { - uc := &UpdateConfig{} - f, err := uc.File("") - if err != nil { - t.Error("unexpected error getting file from empty UpdateConfig") - } - if f != nil { - t.Errorf("getting file from empty UpdateConfig should have returned nil, got %v", f) - } - uu, err := uc.Units("") - if err != nil { - t.Error("unexpected error getting unit from empty UpdateConfig") - } - if len(uu) != 0 { - t.Errorf("getting unit from empty UpdateConfig should have returned zero units, got %d", len(uu)) - } -} - -func TestInvalidUpdateOptions(t *testing.T) { - uon := &updateOption{ - key: "numbers", - prefix: "numero_", - valid: []string{"one", "two"}, - } - uoa := &updateOption{ - key: "any_will_do", - prefix: "any_", - } - - if !uon.isValid("one") { - t.Error("update option did not accept valid option \"one\"") - } - if uon.isValid("three") { - t.Error("update option accepted invalid option \"three\"") - } - for _, s := range []string{"one", "asdf", "foobarbaz"} { - if !uoa.isValid(s) { - t.Errorf("update option with no \"valid\" field did not accept %q", s) - } - } - - uc := &UpdateConfig{"reboot-strategy": "wizzlewazzle"} - f, err := uc.File("") - if err == nil { - t.Errorf("File did not give an error on invalid UpdateOption") - } - if f != nil { - t.Errorf("File did not return a nil file on invalid UpdateOption") - } -} - -func TestServerGroupOptions(t *testing.T) { - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - setupFixtures(dir) - u := &UpdateConfig{"group": "master", "server": "http://foo.com"} - - want := ` -GROUP=master -SERVER=http://foo.com` - - f, err := u.File(dir) - if err != nil { - t.Errorf("unexpected error getting file from UpdateConfig: %v", err) - } else if f == nil { - t.Error("unexpectedly got empty file from UpdateConfig") - } else { - out := strings.Split(f.Content, "\n") - sort.Strings(out) - got := strings.Join(out, "\n") - if got != want { - t.Errorf("File has incorrect contents, got %v, want %v", got, want) - } - } - - uu, err := u.Units(dir) - if err != nil { - t.Errorf("unexpected error getting units from UpdateConfig: %v", err) - } else if len(uu) != 1 { - t.Errorf("unexpected number of files returned from UpdateConfig: want 1, got %d", len(uu)) - } else { - unit := uu[0] - if unit.Name != "update-engine.service" { - t.Errorf("bad name for generated unit: want update-engine.service, got %s", unit.Name) - } - if unit.Command != "restart" { - t.Errorf("bad command for generated unit: want restart, got %s", unit.Command) - } - } -} - -func TestRebootStrategies(t *testing.T) { - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - setupFixtures(dir) - strategies := []struct { - name string - line string - uMask bool - uCommand string - }{ - {"best-effort", "REBOOT_STRATEGY=best-effort", false, "restart"}, - {"etcd-lock", "REBOOT_STRATEGY=etcd-lock", false, "restart"}, - {"reboot", "REBOOT_STRATEGY=reboot", false, "restart"}, - {"off", "REBOOT_STRATEGY=off", true, "stop"}, - } - for _, s := range strategies { - uc := &UpdateConfig{"reboot-strategy": s.name} - f, err := uc.File(dir) - if err != nil { - t.Errorf("update failed to generate file for reboot-strategy=%v: %v", s.name, err) - } else if f == nil { - t.Errorf("generated empty file for reboot-strategy=%v", s.name) - } else { - seen := false - for _, line := range strings.Split(f.Content, "\n") { - if line == s.line { - seen = true - break - } - } - if !seen { - t.Errorf("couldn't find expected line %v for reboot-strategy=%v", s.line) - } - } - uu, err := uc.Units(dir) - if err != nil { - t.Errorf("failed to generate unit for reboot-strategy=%v!", s.name) - } else if len(uu) != 1 { - t.Errorf("unexpected number of units for reboot-strategy=%v: %d", s.name, len(uu)) - } else { - u := uu[0] - if u.Name != locksmithUnit { - t.Errorf("unit generated for reboot strategy=%v had bad name: %v", s.name, u.Name) - } - if u.Mask != s.uMask { - t.Errorf("unit generated for reboot strategy=%v had bad mask: %t", s.name, u.Mask) - } - if u.Command != s.uCommand { - t.Errorf("unit generated for reboot strategy=%v had bad command: %v", s.name, u.Command) - } - } - } - -} - -func TestUpdateConfWrittenToDisk(t *testing.T) { - dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") - if err != nil { - t.Fatalf("Unable to create tempdir: %v", err) - } - defer os.RemoveAll(dir) - setupFixtures(dir) - - for i := 0; i < 2; i++ { - if i == 1 { - err = ioutil.WriteFile(path.Join(dir, "etc", "coreos", "update.conf"), []byte(configured), 0644) - if err != nil { - t.Fatal(err) - } - } - uc := &UpdateConfig{"reboot-strategy": "etcd-lock"} - - f, err := uc.File(dir) - if err != nil { - t.Fatalf("Processing UpdateConfig failed: %v", err) - } else if f == nil { - t.Fatal("Unexpectedly got nil updateconfig file") - } - - if _, err := system.WriteFile(f, dir); err != nil { - t.Fatalf("Error writing update config: %v", err) - } - - fullPath := path.Join(dir, "etc", "coreos", "update.conf") - - fi, err := os.Stat(fullPath) - if err != nil { - t.Fatalf("Unable to stat file: %v", err) - } - - if fi.Mode() != os.FileMode(0644) { - t.Errorf("File has incorrect mode: %v", fi.Mode()) - } - - contents, err := ioutil.ReadFile(fullPath) - if err != nil { - t.Fatalf("Unable to read expected file: %v", err) - } - - if string(contents) != expected { - t.Fatalf("File has incorrect contents, got %v, wanted %v", string(contents), expected) - } - } -} diff --git a/initialize/manage_etc_hosts.go b/system/etc_hosts.go similarity index 73% rename from initialize/manage_etc_hosts.go rename to system/etc_hosts.go index b4bd3f6..9d81ae5 100644 --- a/initialize/manage_etc_hosts.go +++ b/system/etc_hosts.go @@ -1,4 +1,4 @@ -package initialize +package system import ( "errors" @@ -6,15 +6,17 @@ import ( "os" "path" - "github.com/coreos/coreos-cloudinit/system" + "github.com/coreos/coreos-cloudinit/config" ) const DefaultIpv4Address = "127.0.0.1" -type EtcHosts string +type EtcHosts struct { + Config config.EtcHosts +} func (eh EtcHosts) generateEtcHosts() (out string, err error) { - if eh != "localhost" { + if eh.Config != "localhost" { return "", errors.New("Invalid option to manage_etc_hosts") } @@ -28,8 +30,8 @@ func (eh EtcHosts) generateEtcHosts() (out string, err error) { } -func (eh EtcHosts) File(root string) (*system.File, error) { - if eh == "" { +func (eh EtcHosts) File() (*File, error) { + if eh.Config == "" { return nil, nil } @@ -38,7 +40,7 @@ func (eh EtcHosts) File(root string) (*system.File, error) { return nil, err } - return &system.File{ + return &File{ Path: path.Join("etc", "hosts"), RawFilePermissions: "0644", Content: etcHosts, diff --git a/system/etc_hosts_test.go b/system/etc_hosts_test.go new file mode 100644 index 0000000..b9380fe --- /dev/null +++ b/system/etc_hosts_test.go @@ -0,0 +1,46 @@ +package system + +import ( + "fmt" + "os" + "reflect" + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) + +func TestEtcdHostsFile(t *testing.T) { + hostname, err := os.Hostname() + if err != nil { + panic(err) + } + + for _, tt := range []struct { + config config.EtcHosts + file *File + err error + }{ + { + "invalid", + nil, + fmt.Errorf("Invalid option to manage_etc_hosts"), + }, + { + "localhost", + &File{ + Content: fmt.Sprintf("127.0.0.1 %s\n", hostname), + Path: "etc/hosts", + RawFilePermissions: "0644", + }, + nil, + }, + } { + file, err := EtcHosts{tt.config}.File() + if !reflect.DeepEqual(tt.err, err) { + t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err) + } + if !reflect.DeepEqual(tt.file, file) { + t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.file, file) + } + } +} diff --git a/system/etcd.go b/system/etcd.go index 86cb6c1..f52b8d1 100644 --- a/system/etcd.go +++ b/system/etcd.go @@ -11,7 +11,7 @@ type Etcd struct { } // Units creates a Unit file drop-in for etcd, using any configured options. -func (ee Etcd) Units(_ string) ([]Unit, error) { +func (ee Etcd) Units() ([]Unit, error) { content := dropinContents(ee.Etcd) if content == "" { return nil, nil diff --git a/system/etcd_test.go b/system/etcd_test.go index 68aee41..c962178 100644 --- a/system/etcd_test.go +++ b/system/etcd_test.go @@ -49,7 +49,7 @@ Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" }}, }, } { - units, err := Etcd{tt.config}.Units("") + units, err := Etcd{tt.config}.Units() if err != nil { t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err) } diff --git a/system/fleet.go b/system/fleet.go index 50f288a..6eb6f18 100644 --- a/system/fleet.go +++ b/system/fleet.go @@ -12,7 +12,7 @@ type Fleet struct { // Units generates a Unit file drop-in for fleet, if any fleet options were // configured in cloud-config -func (fe Fleet) Units(_ string) ([]Unit, error) { +func (fe Fleet) Units() ([]Unit, error) { content := dropinContents(fe.Fleet) if content == "" { return nil, nil diff --git a/system/fleet_test.go b/system/fleet_test.go index 4618d5d..5d01ec9 100644 --- a/system/fleet_test.go +++ b/system/fleet_test.go @@ -30,7 +30,7 @@ Environment="FLEET_PUBLIC_IP=12.34.56.78" }}, }, } { - units, err := Fleet{tt.config}.Units("") + units, err := Fleet{tt.config}.Units() if err != nil { t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err) } diff --git a/system/oem.go b/system/oem.go index ba3634f..38e3071 100644 --- a/system/oem.go +++ b/system/oem.go @@ -13,7 +13,7 @@ type OEM struct { config.OEM } -func (oem OEM) File(_ string) (*File, error) { +func (oem OEM) File() (*File, error) { if oem.ID == "" { return nil, nil } diff --git a/system/oem_test.go b/system/oem_test.go index 4a8029e..62ebbbf 100644 --- a/system/oem_test.go +++ b/system/oem_test.go @@ -36,7 +36,7 @@ BUG_REPORT_URL="https://github.com/coreos/coreos-overlay" }, }, } { - file, err := OEM{tt.config}.File("") + file, err := OEM{tt.config}.File() if err != nil { t.Errorf("bad error (%q): want %q, got %q", tt.config, nil, err) } diff --git a/system/update.go b/system/update.go new file mode 100644 index 0000000..5e172af --- /dev/null +++ b/system/update.go @@ -0,0 +1,137 @@ +package system + +import ( + "bufio" + "fmt" + "io" + "os" + "path" + "reflect" + "sort" + "strings" + + "github.com/coreos/coreos-cloudinit/config" +) + +const ( + locksmithUnit = "locksmithd.service" + updateEngineUnit = "update-engine.service" +) + +// Update is a top-level structure which contains its underlying configuration, +// config.Update, a function for reading the configuration (the default +// implementation reading from the filesystem), and provides the system-specific +// File() and Unit(). +type Update struct { + Config config.Update + ReadConfig func() (io.Reader, error) +} + +func DefaultReadConfig() (io.Reader, error) { + etcUpdate := path.Join("/etc", "coreos", "update.conf") + usrUpdate := path.Join("/usr", "share", "coreos", "update.conf") + + f, err := os.Open(etcUpdate) + if os.IsNotExist(err) { + f, err = os.Open(usrUpdate) + } + return f, err +} + +// File generates an `/etc/coreos/update.conf` file (if any update +// configuration options are set in cloud-config) by either rewriting the +// existing file on disk, or starting from `/usr/share/coreos/update.conf` +func (uc Update) File() (*File, error) { + if config.IsZero(uc.Config) { + return nil, nil + } + if err := config.AssertValid(uc.Config); err != nil { + return nil, err + } + + // Generate the list of possible substitutions to be performed based on the options that are configured + subs := map[string]string{} + uct := reflect.TypeOf(uc.Config) + ucv := reflect.ValueOf(uc.Config) + for i := 0; i < uct.NumField(); i++ { + val := ucv.Field(i).String() + if val == "" { + continue + } + env := uct.Field(i).Tag.Get("env") + subs[env] = fmt.Sprintf("%s=%s", env, val) + } + + conf, err := uc.ReadConfig() + if err != nil { + return nil, err + } + scanner := bufio.NewScanner(conf) + + var out string + for scanner.Scan() { + line := scanner.Text() + for env, value := range subs { + if strings.HasPrefix(line, env) { + line = value + delete(subs, env) + break + } + } + out += line + out += "\n" + if err := scanner.Err(); err != nil { + return nil, err + } + } + + for _, key := range sortedKeys(subs) { + out += subs[key] + out += "\n" + } + + return &File{ + Path: path.Join("etc", "coreos", "update.conf"), + RawFilePermissions: "0644", + Content: out, + }, nil +} + +// Units generates units for the cloud-init initializer to act on: +// - a locksmith Unit, if "reboot-strategy" was set in cloud-config +// - an update_engine Unit, if "group" or "server" was set in cloud-config +func (uc Update) Units() ([]Unit, error) { + var units []Unit + if uc.Config.RebootStrategy != "" { + ls := &Unit{ + Name: locksmithUnit, + Command: "restart", + Mask: false, + Runtime: true, + } + + if uc.Config.RebootStrategy == "off" { + ls.Command = "stop" + ls.Mask = true + } + units = append(units, *ls) + } + + if uc.Config.Group != "" || uc.Config.Server != "" { + ue := Unit{ + Name: updateEngineUnit, + Command: "restart", + } + units = append(units, ue) + } + + return units, nil +} + +func sortedKeys(m map[string]string) (keys []string) { + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + return +} diff --git a/system/update_test.go b/system/update_test.go new file mode 100644 index 0000000..ea82113 --- /dev/null +++ b/system/update_test.go @@ -0,0 +1,151 @@ +package system + +import ( + "errors" + "io" + "reflect" + "strings" + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) + +func testReadConfig(config string) func() (io.Reader, error) { + return func() (io.Reader, error) { + return strings.NewReader(config), nil + } +} + +func TestUpdateUnits(t *testing.T) { + for _, tt := range []struct { + config config.Update + units []Unit + err error + }{ + { + config: config.Update{}, + }, + { + config: config.Update{Group: "master", Server: "http://foo.com"}, + units: []Unit{{ + Name: "update-engine.service", + Command: "restart", + }}, + }, + { + config: config.Update{RebootStrategy: "best-effort"}, + units: []Unit{{ + Name: "locksmithd.service", + Command: "restart", + Runtime: true, + }}, + }, + { + config: config.Update{RebootStrategy: "etcd-lock"}, + units: []Unit{{ + Name: "locksmithd.service", + Command: "restart", + Runtime: true, + }}, + }, + { + config: config.Update{RebootStrategy: "reboot"}, + units: []Unit{{ + Name: "locksmithd.service", + Command: "restart", + Runtime: true, + }}, + }, + { + config: config.Update{RebootStrategy: "off"}, + units: []Unit{{ + Name: "locksmithd.service", + Command: "stop", + Runtime: true, + Mask: true, + }}, + }, + } { + units, err := Update{tt.config, testReadConfig("")}.Units() + if !reflect.DeepEqual(tt.err, err) { + t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err) + } + if !reflect.DeepEqual(tt.units, units) { + t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.units, units) + } + } +} + +func TestUpdateFile(t *testing.T) { + for _, tt := range []struct { + config config.Update + orig string + file *File + err error + }{ + { + config: config.Update{}, + }, + { + config: config.Update{RebootStrategy: "wizzlewazzle"}, + err: errors.New("invalid value \"wizzlewazzle\" for option \"RebootStrategy\" (valid options: \"best-effort,etcd-lock,reboot,off\")"), + }, + { + config: config.Update{Group: "master", Server: "http://foo.com"}, + file: &File{ + Content: "GROUP=master\nSERVER=http://foo.com\n", + Path: "etc/coreos/update.conf", + RawFilePermissions: "0644", + }, + }, + { + config: config.Update{RebootStrategy: "best-effort"}, + file: &File{ + Content: "REBOOT_STRATEGY=best-effort\n", + Path: "etc/coreos/update.conf", + RawFilePermissions: "0644", + }, + }, + { + config: config.Update{RebootStrategy: "etcd-lock"}, + file: &File{ + Content: "REBOOT_STRATEGY=etcd-lock\n", + Path: "etc/coreos/update.conf", + RawFilePermissions: "0644", + }, + }, + { + config: config.Update{RebootStrategy: "reboot"}, + file: &File{ + Content: "REBOOT_STRATEGY=reboot\n", + Path: "etc/coreos/update.conf", + RawFilePermissions: "0644", + }, + }, + { + config: config.Update{RebootStrategy: "off"}, + file: &File{ + Content: "REBOOT_STRATEGY=off\n", + Path: "etc/coreos/update.conf", + RawFilePermissions: "0644", + }, + }, + { + config: config.Update{RebootStrategy: "etcd-lock"}, + orig: "SERVER=https://example.com\nGROUP=thegroupc\nREBOOT_STRATEGY=awesome", + file: &File{ + Content: "SERVER=https://example.com\nGROUP=thegroupc\nREBOOT_STRATEGY=etcd-lock\n", + Path: "etc/coreos/update.conf", + RawFilePermissions: "0644", + }, + }, + } { + file, err := Update{tt.config, testReadConfig(tt.orig)}.File() + if !reflect.DeepEqual(tt.err, err) { + t.Errorf("bad error (%q): want %q, got %q", tt.config, tt.err, err) + } + if !reflect.DeepEqual(tt.file, file) { + t.Errorf("bad units (%q): want %#v, got %#v", tt.config, tt.file, file) + } + } +} diff --git a/test b/test index 8292c2a..d34ff5b 100755 --- a/test +++ b/test @@ -13,19 +13,22 @@ COVER=${COVER:-"-cover"} source ./build -declare -a TESTPKGS=(initialize - system - datasource - datasource/configdrive - datasource/file - datasource/metadata - datasource/metadata/cloudsigma - datasource/metadata/digitalocean - datasource/metadata/ec2 - datasource/proc_cmdline - datasource/url - pkg - network) +declare -a TESTPKGS=( + config + datasource + datasource/configdrive + datasource/file + datasource/metadata + datasource/metadata/cloudsigma + datasource/metadata/digitalocean + datasource/metadata/ec2 + datasource/proc_cmdline + datasource/url + initialize + network + pkg + system +) if [ -z "$PKG" ]; then GOFMTPATH="${TESTPKGS[*]} coreos-cloudinit.go" From 1fbbaaec19f4d132a7ab17f2aa2bfc8b2c5ef4ce Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 19:22:38 -0700 Subject: [PATCH 5/8] unit: refactor config - Seperate the config from Destination() - Add YAML tags for the fields --- config/unit.go | 34 ++++++++++++++++++++++++++++++++++ initialize/config.go | 11 ++++++++--- initialize/config_test.go | 21 +++++++++++---------- system/etcd.go | 4 ++-- system/etcd_test.go | 8 ++++---- system/fleet.go | 4 ++-- system/fleet_test.go | 4 ++-- system/systemd_test.go | 24 +++++++++++++----------- system/unit.go | 33 +++++---------------------------- system/update.go | 8 ++++---- system/update_test.go | 20 ++++++++++---------- 11 files changed, 95 insertions(+), 76 deletions(-) create mode 100644 config/unit.go diff --git a/config/unit.go b/config/unit.go new file mode 100644 index 0000000..7d35b28 --- /dev/null +++ b/config/unit.go @@ -0,0 +1,34 @@ +package config + +import ( + "path/filepath" + "strings" +) + +type Unit struct { + Name string `yaml:"name"` + Mask bool `yaml:"mask"` + Enable bool `yaml:"enable"` + Runtime bool `yaml:"runtime"` + Content string `yaml:"content"` + Command string `yaml:"command"` + + // For drop-in units, a cloudinit.conf is generated. + // This is currently unbound in YAML (and hence unsettable in cloud-config files) + // until the correct behaviour for multiple drop-in units is determined. + DropIn bool `yaml:"-"` +} + +func (u *Unit) Type() string { + ext := filepath.Ext(u.Name) + return strings.TrimLeft(ext, ".") +} + +func (u *Unit) Group() string { + switch u.Type() { + case "network", "netdev", "link": + return "network" + default: + return "system" + } +} diff --git a/initialize/config.go b/initialize/config.go index 0eb6163..210c234 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -35,7 +35,7 @@ type CloudConfig struct { Fleet config.Fleet OEM config.OEM Update config.Update - Units []system.Unit + Units []config.Unit } WriteFiles []system.File `yaml:"write_files"` Hostname string @@ -231,6 +231,11 @@ func Apply(cfg CloudConfig, env *Environment) error { } } + var units []system.Unit + for _, u := range cfg.Coreos.Units { + units = append(units, system.Unit{u}) + } + for _, ccu := range []CloudConfigUnit{ system.Etcd{cfg.Coreos.Etcd}, system.Fleet{cfg.Coreos.Fleet}, @@ -240,7 +245,7 @@ func Apply(cfg CloudConfig, env *Environment) error { if err != nil { return err } - cfg.Coreos.Units = append(cfg.Coreos.Units, u...) + units = append(units, u...) } wroteEnvironment := false @@ -291,7 +296,7 @@ func Apply(cfg CloudConfig, env *Environment) error { } um := system.NewUnitManager(env.Root()) - return processUnits(cfg.Coreos.Units, env.Root(), um) + return processUnits(units, env.Root(), um) } diff --git a/initialize/config_test.go b/initialize/config_test.go index cd84438..9426551 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/system" ) @@ -401,10 +402,10 @@ func (tum *TestUnitManager) UnmaskUnit(unit *system.Unit) error { func TestProcessUnits(t *testing.T) { tum := &TestUnitManager{} units := []system.Unit{ - system.Unit{ + system.Unit{config.Unit{ Name: "foo", Mask: true, - }, + }}, } if err := processUnits(units, "", tum); err != nil { t.Fatalf("unexpected error calling processUnits: %v", err) @@ -415,9 +416,9 @@ func TestProcessUnits(t *testing.T) { tum = &TestUnitManager{} units = []system.Unit{ - system.Unit{ + system.Unit{config.Unit{ Name: "bar.network", - }, + }}, } if err := processUnits(units, "", tum); err != nil { t.Fatalf("unexpected error calling processUnits: %v", err) @@ -428,10 +429,10 @@ func TestProcessUnits(t *testing.T) { tum = &TestUnitManager{} units = []system.Unit{ - system.Unit{ + system.Unit{config.Unit{ Name: "baz.service", Content: "[Service]\nExecStart=/bin/true", - }, + }}, } if err := processUnits(units, "", tum); err != nil { t.Fatalf("unexpected error calling processUnits: %v", err) @@ -442,10 +443,10 @@ func TestProcessUnits(t *testing.T) { tum = &TestUnitManager{} units = []system.Unit{ - system.Unit{ + system.Unit{config.Unit{ Name: "locksmithd.service", Runtime: true, - }, + }}, } if err := processUnits(units, "", tum); err != nil { t.Fatalf("unexpected error calling processUnits: %v", err) @@ -456,10 +457,10 @@ func TestProcessUnits(t *testing.T) { tum = &TestUnitManager{} units = []system.Unit{ - system.Unit{ + system.Unit{config.Unit{ Name: "woof", Enable: true, - }, + }}, } if err := processUnits(units, "", tum); err != nil { t.Fatalf("unexpected error calling processUnits: %v", err) diff --git a/system/etcd.go b/system/etcd.go index f52b8d1..dd7664f 100644 --- a/system/etcd.go +++ b/system/etcd.go @@ -16,10 +16,10 @@ func (ee Etcd) Units() ([]Unit, error) { if content == "" { return nil, nil } - return []Unit{{ + return []Unit{{config.Unit{ Name: "etcd.service", Runtime: true, DropIn: true, Content: content, - }}, nil + }}}, nil } diff --git a/system/etcd_test.go b/system/etcd_test.go index c962178..2e65768 100644 --- a/system/etcd_test.go +++ b/system/etcd_test.go @@ -21,7 +21,7 @@ func TestEtcdUnits(t *testing.T) { Discovery: "http://disco.example.com/foobar", PeerBindAddr: "127.0.0.1:7002", }, - []Unit{{ + []Unit{{config.Unit{ Name: "etcd.service", Runtime: true, DropIn: true, @@ -29,7 +29,7 @@ func TestEtcdUnits(t *testing.T) { Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" `, - }}, + }}}, }, { config.Etcd{ @@ -37,7 +37,7 @@ Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" Discovery: "http://disco.example.com/foobar", PeerBindAddr: "127.0.0.1:7002", }, - []Unit{{ + []Unit{{config.Unit{ Name: "etcd.service", Runtime: true, DropIn: true, @@ -46,7 +46,7 @@ Environment="ETCD_DISCOVERY=http://disco.example.com/foobar" Environment="ETCD_NAME=node001" Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002" `, - }}, + }}}, }, } { units, err := Etcd{tt.config}.Units() diff --git a/system/fleet.go b/system/fleet.go index 6eb6f18..a4ce127 100644 --- a/system/fleet.go +++ b/system/fleet.go @@ -17,10 +17,10 @@ func (fe Fleet) Units() ([]Unit, error) { if content == "" { return nil, nil } - return []Unit{{ + return []Unit{{config.Unit{ Name: "fleet.service", Runtime: true, DropIn: true, Content: content, - }}, nil + }}}, nil } diff --git a/system/fleet_test.go b/system/fleet_test.go index 5d01ec9..48e52f9 100644 --- a/system/fleet_test.go +++ b/system/fleet_test.go @@ -20,14 +20,14 @@ func TestFleetUnits(t *testing.T) { config.Fleet{ PublicIP: "12.34.56.78", }, - []Unit{{ + []Unit{{config.Unit{ Name: "fleet.service", Content: `[Service] Environment="FLEET_PUBLIC_IP=12.34.56.78" `, Runtime: true, DropIn: true, - }}, + }}}, }, } { units, err := Fleet{tt.config}.Units() diff --git a/system/systemd_test.go b/system/systemd_test.go index de6d4e8..fc4a83b 100644 --- a/system/systemd_test.go +++ b/system/systemd_test.go @@ -5,10 +5,12 @@ import ( "os" "path" "testing" + + "github.com/coreos/coreos-cloudinit/config" ) func TestPlaceNetworkUnit(t *testing.T) { - u := Unit{ + u := Unit{config.Unit{ Name: "50-eth0.network", Runtime: true, Content: `[Match] @@ -17,7 +19,7 @@ Name=eth47 [Network] Address=10.209.171.177/19 `, - } + }} dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") if err != nil { @@ -66,10 +68,10 @@ func TestUnitDestination(t *testing.T) { dir := "/some/dir" name := "foobar.service" - u := Unit{ + u := Unit{config.Unit{ Name: name, DropIn: false, - } + }} dst := u.Destination(dir) expectDst := path.Join(dir, "etc", "systemd", "system", "foobar.service") @@ -87,14 +89,14 @@ func TestUnitDestination(t *testing.T) { } func TestPlaceMountUnit(t *testing.T) { - u := Unit{ + u := Unit{config.Unit{ Name: "media-state.mount", Runtime: false, Content: `[Mount] What=/dev/sdb1 Where=/media/state `, - } + }} dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-") if err != nil { @@ -162,7 +164,7 @@ func TestMaskUnit(t *testing.T) { sd := &systemd{dir} // Ensure mask works with units that do not currently exist - uf := &Unit{Name: "foo.service"} + uf := &Unit{config.Unit{Name: "foo.service"}} if err := sd.MaskUnit(uf); err != nil { t.Fatalf("Unable to mask new unit: %v", err) } @@ -176,7 +178,7 @@ func TestMaskUnit(t *testing.T) { } // Ensure mask works with unit files that already exist - ub := &Unit{Name: "bar.service"} + ub := &Unit{config.Unit{Name: "bar.service"}} barPath := path.Join(dir, "etc", "systemd", "system", "bar.service") if _, err := os.Create(barPath); err != nil { t.Fatalf("Error creating new unit file: %v", err) @@ -202,12 +204,12 @@ func TestUnmaskUnit(t *testing.T) { sd := &systemd{dir} - nilUnit := &Unit{Name: "null.service"} + nilUnit := &Unit{config.Unit{Name: "null.service"}} if err := sd.UnmaskUnit(nilUnit); err != nil { t.Errorf("unexpected error from unmasking nonexistent unit: %v", err) } - uf := &Unit{Name: "foo.service", Content: "[Service]\nExecStart=/bin/true"} + uf := &Unit{config.Unit{Name: "foo.service", Content: "[Service]\nExecStart=/bin/true"}} dst := uf.Destination(dir) if err := os.MkdirAll(path.Dir(dst), os.FileMode(0755)); err != nil { t.Fatalf("Unable to create unit directory: %v", err) @@ -227,7 +229,7 @@ func TestUnmaskUnit(t *testing.T) { t.Errorf("unmask of non-empty unit mutated unit contents unexpectedly") } - ub := &Unit{Name: "bar.service"} + ub := &Unit{config.Unit{Name: "bar.service"}} dst = ub.Destination(dir) if err := os.Symlink("/dev/null", dst); err != nil { t.Fatalf("Unable to create masked unit: %v", err) diff --git a/system/unit.go b/system/unit.go index df4d384..d4d2421 100644 --- a/system/unit.go +++ b/system/unit.go @@ -3,8 +3,8 @@ package system import ( "fmt" "path" - "path/filepath" - "strings" + + "github.com/coreos/coreos-cloudinit/config" ) // Name for drop-in service configuration files created by cloudconfig @@ -19,33 +19,10 @@ type UnitManager interface { UnmaskUnit(unit *Unit) error } +// Unit is a top-level structure which embeds its underlying configuration, +// config.Unit, and provides the system-specific Destination(). type Unit struct { - Name string - Mask bool - Enable bool - Runtime bool - Content string - Command string - - // For drop-in units, a cloudinit.conf is generated. - // This is currently unbound in YAML (and hence unsettable in cloud-config files) - // until the correct behaviour for multiple drop-in units is determined. - DropIn bool `yaml:"-"` -} - -func (u *Unit) Type() string { - ext := filepath.Ext(u.Name) - return strings.TrimLeft(ext, ".") -} - -func (u *Unit) Group() (group string) { - t := u.Type() - if t == "network" || t == "netdev" || t == "link" { - group = "network" - } else { - group = "system" - } - return + config.Unit } type Script []byte diff --git a/system/update.go b/system/update.go index 5e172af..8ee18ac 100644 --- a/system/update.go +++ b/system/update.go @@ -103,12 +103,12 @@ func (uc Update) File() (*File, error) { func (uc Update) Units() ([]Unit, error) { var units []Unit if uc.Config.RebootStrategy != "" { - ls := &Unit{ + ls := &Unit{config.Unit{ Name: locksmithUnit, Command: "restart", Mask: false, Runtime: true, - } + }} if uc.Config.RebootStrategy == "off" { ls.Command = "stop" @@ -118,10 +118,10 @@ func (uc Update) Units() ([]Unit, error) { } if uc.Config.Group != "" || uc.Config.Server != "" { - ue := Unit{ + ue := Unit{config.Unit{ Name: updateEngineUnit, Command: "restart", - } + }} units = append(units, ue) } diff --git a/system/update_test.go b/system/update_test.go index ea82113..1c37d9e 100644 --- a/system/update_test.go +++ b/system/update_test.go @@ -27,43 +27,43 @@ func TestUpdateUnits(t *testing.T) { }, { config: config.Update{Group: "master", Server: "http://foo.com"}, - units: []Unit{{ + units: []Unit{{config.Unit{ Name: "update-engine.service", Command: "restart", - }}, + }}}, }, { config: config.Update{RebootStrategy: "best-effort"}, - units: []Unit{{ + units: []Unit{{config.Unit{ Name: "locksmithd.service", Command: "restart", Runtime: true, - }}, + }}}, }, { config: config.Update{RebootStrategy: "etcd-lock"}, - units: []Unit{{ + units: []Unit{{config.Unit{ Name: "locksmithd.service", Command: "restart", Runtime: true, - }}, + }}}, }, { config: config.Update{RebootStrategy: "reboot"}, - units: []Unit{{ + units: []Unit{{config.Unit{ Name: "locksmithd.service", Command: "restart", Runtime: true, - }}, + }}}, }, { config: config.Update{RebootStrategy: "off"}, - units: []Unit{{ + units: []Unit{{config.Unit{ Name: "locksmithd.service", Command: "stop", Runtime: true, Mask: true, - }}, + }}}, }, } { units, err := Update{tt.config, testReadConfig("")}.Units() From 85b8d804c896d4ec5faebfbdbda2f0369c9851f2 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 19:22:27 -0700 Subject: [PATCH 6/8] file: refactor config - Seperate the config from Permissions() - Add YAML tags for the fields --- config/file.go | 9 +++++++++ initialize/config.go | 11 +++++++--- initialize/config_test.go | 2 +- initialize/env.go | 5 +++-- initialize/workspace.go | 9 +++++---- system/env_file_test.go | 42 ++++++++++++++++++++------------------- system/etc_hosts.go | 4 ++-- system/etc_hosts_test.go | 4 ++-- system/file.go | 10 +++++----- system/file_test.go | 18 +++++++++-------- system/networkd.go | 7 ++++--- system/oem.go | 4 ++-- system/oem_test.go | 4 ++-- system/systemd.go | 5 +++-- system/update.go | 4 ++-- system/update_test.go | 24 +++++++++++----------- 16 files changed, 92 insertions(+), 70 deletions(-) create mode 100644 config/file.go diff --git a/config/file.go b/config/file.go new file mode 100644 index 0000000..69ab5a4 --- /dev/null +++ b/config/file.go @@ -0,0 +1,9 @@ +package config + +type File struct { + Encoding string `yaml:"-"` + Content string `yaml:"content"` + Owner string `yaml:"owner"` + Path string `yaml:"path"` + RawFilePermissions string `yaml:"permissions"` +} diff --git a/initialize/config.go b/initialize/config.go index 210c234..79ec5e7 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -37,7 +37,7 @@ type CloudConfig struct { Update config.Update Units []config.Unit } - WriteFiles []system.File `yaml:"write_files"` + WriteFiles []config.File `yaml:"write_files"` Hostname string Users []system.User ManageEtcHosts config.EtcHosts `yaml:"manage_etc_hosts"` @@ -217,6 +217,11 @@ func Apply(cfg CloudConfig, env *Environment) error { } } + var writeFiles []system.File + for _, file := range cfg.WriteFiles { + writeFiles = append(writeFiles, system.File{file}) + } + for _, ccf := range []CloudConfigFile{ system.OEM{cfg.Coreos.OEM}, system.Update{cfg.Coreos.Update, system.DefaultReadConfig}, @@ -227,7 +232,7 @@ func Apply(cfg CloudConfig, env *Environment) error { return err } if f != nil { - cfg.WriteFiles = append(cfg.WriteFiles, *f) + writeFiles = append(writeFiles, *f) } } @@ -249,7 +254,7 @@ func Apply(cfg CloudConfig, env *Environment) error { } wroteEnvironment := false - for _, file := range cfg.WriteFiles { + for _, file := range writeFiles { fullPath, err := system.WriteFile(&file, env.Root()) if err != nil { return err diff --git a/initialize/config_test.go b/initialize/config_test.go index 9426551..fa5ed93 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -178,7 +178,7 @@ hostname: trontastic if len(cfg.WriteFiles) != 1 { t.Error("Failed to parse correct number of write_files") } else { - wf := cfg.WriteFiles[0] + wf := system.File{cfg.WriteFiles[0]} if wf.Content != "penny\nelroy\n" { t.Errorf("WriteFile has incorrect contents '%s'", wf.Content) } diff --git a/initialize/env.go b/initialize/env.go index 69e528e..76ac68e 100644 --- a/initialize/env.go +++ b/initialize/env.go @@ -6,6 +6,7 @@ import ( "regexp" "strings" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/system" ) @@ -81,9 +82,9 @@ func (e *Environment) Apply(data string) string { func (e *Environment) DefaultEnvironmentFile() *system.EnvFile { ef := system.EnvFile{ - File: &system.File{ + File: &system.File{config.File{ Path: "/etc/environment", - }, + }}, Vars: map[string]string{}, } if ip, ok := e.substitutions["$public_ipv4"]; ok && len(ip) > 0 { diff --git a/initialize/workspace.go b/initialize/workspace.go index ed1cf93..944e955 100644 --- a/initialize/workspace.go +++ b/initialize/workspace.go @@ -5,6 +5,7 @@ import ( "path" "strings" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/system" ) @@ -31,21 +32,21 @@ func PersistScriptInWorkspace(script system.Script, workspace string) (string, e relpath := strings.TrimPrefix(tmp.Name(), workspace) - file := system.File{ + file := system.File{config.File{ Path: relpath, RawFilePermissions: "0744", Content: string(script), - } + }} return system.WriteFile(&file, workspace) } func PersistUnitNameInWorkspace(name string, workspace string) error { - file := system.File{ + file := system.File{config.File{ Path: path.Join("scripts", "unit-name"), RawFilePermissions: "0644", Content: name, - } + }} _, err := system.WriteFile(&file, workspace) return err } diff --git a/system/env_file_test.go b/system/env_file_test.go index e827576..8ebdeac 100644 --- a/system/env_file_test.go +++ b/system/env_file_test.go @@ -7,6 +7,8 @@ import ( "strings" "syscall" "testing" + + "github.com/coreos/coreos-cloudinit/config" ) const ( @@ -48,9 +50,9 @@ func TestWriteEnvFileUpdate(t *testing.T) { } ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueUpdate, } @@ -95,9 +97,9 @@ func TestWriteEnvFileUpdateNoNewline(t *testing.T) { } ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueUpdate, } @@ -136,9 +138,9 @@ func TestWriteEnvFileCreate(t *testing.T) { fullPath := path.Join(dir, name) ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueUpdate, } @@ -174,9 +176,9 @@ func TestWriteEnvFileNoop(t *testing.T) { } ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueNoop, } @@ -221,9 +223,9 @@ func TestWriteEnvFileUpdateDos(t *testing.T) { } ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueUpdate, } @@ -270,9 +272,9 @@ func TestWriteEnvFileDos2Unix(t *testing.T) { } ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueNoop, } @@ -318,9 +320,9 @@ func TestWriteEnvFileEmpty(t *testing.T) { } ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueEmpty, } @@ -360,9 +362,9 @@ func TestWriteEnvFileEmptyNoCreate(t *testing.T) { fullPath := path.Join(dir, name) ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueEmpty, } @@ -391,9 +393,9 @@ func TestWriteEnvFilePermFailure(t *testing.T) { ioutil.WriteFile(fullPath, []byte(base), 0000) ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueUpdate, } @@ -413,9 +415,9 @@ func TestWriteEnvFileNameFailure(t *testing.T) { name := "foo.conf" ef := EnvFile{ - File: &File{ + File: &File{config.File{ Path: name, - }, + }}, Vars: valueInvalid, } diff --git a/system/etc_hosts.go b/system/etc_hosts.go index 9d81ae5..1adccd7 100644 --- a/system/etc_hosts.go +++ b/system/etc_hosts.go @@ -40,9 +40,9 @@ func (eh EtcHosts) File() (*File, error) { return nil, err } - return &File{ + return &File{config.File{ Path: path.Join("etc", "hosts"), RawFilePermissions: "0644", Content: etcHosts, - }, nil + }}, nil } diff --git a/system/etc_hosts_test.go b/system/etc_hosts_test.go index b9380fe..1bd4650 100644 --- a/system/etc_hosts_test.go +++ b/system/etc_hosts_test.go @@ -27,11 +27,11 @@ func TestEtcdHostsFile(t *testing.T) { }, { "localhost", - &File{ + &File{config.File{ Content: fmt.Sprintf("127.0.0.1 %s\n", hostname), Path: "etc/hosts", RawFilePermissions: "0644", - }, + }}, nil, }, } { diff --git a/system/file.go b/system/file.go index e9f40b2..733ccef 100644 --- a/system/file.go +++ b/system/file.go @@ -8,14 +8,14 @@ import ( "os/exec" "path" "strconv" + + "github.com/coreos/coreos-cloudinit/config" ) +// File is a top-level structure which embeds its underlying configuration, +// config.File, and provides the system-specific Permissions(). type File struct { - Encoding string - Content string - Owner string - Path string - RawFilePermissions string `yaml:"permissions"` + config.File } func (f *File) Permissions() (os.FileMode, error) { diff --git a/system/file_test.go b/system/file_test.go index 949ac2a..29fa9a8 100644 --- a/system/file_test.go +++ b/system/file_test.go @@ -5,6 +5,8 @@ import ( "os" "path" "testing" + + "github.com/coreos/coreos-cloudinit/config" ) func TestWriteFileUnencodedContent(t *testing.T) { @@ -17,11 +19,11 @@ func TestWriteFileUnencodedContent(t *testing.T) { fn := "foo" fullPath := path.Join(dir, fn) - wf := File{ + wf := File{config.File{ Path: fn, Content: "bar", RawFilePermissions: "0644", - } + }} path, err := WriteFile(&wf, dir) if err != nil { @@ -56,11 +58,11 @@ func TestWriteFileInvalidPermission(t *testing.T) { } defer os.RemoveAll(dir) - wf := File{ + wf := File{config.File{ Path: path.Join(dir, "tmp", "foo"), Content: "bar", RawFilePermissions: "pants", - } + }} if _, err := WriteFile(&wf, dir); err == nil { t.Fatalf("Expected error to be raised when writing file with invalid permission") @@ -77,10 +79,10 @@ func TestWriteFilePermissions(t *testing.T) { fn := "foo" fullPath := path.Join(dir, fn) - wf := File{ + wf := File{config.File{ Path: fn, RawFilePermissions: "0755", - } + }} path, err := WriteFile(&wf, dir) if err != nil { @@ -106,11 +108,11 @@ func TestWriteFileEncodedContent(t *testing.T) { } defer os.RemoveAll(dir) - wf := File{ + wf := File{config.File{ Path: path.Join(dir, "tmp", "foo"), Content: "", Encoding: "base64", - } + }} if _, err := WriteFile(&wf, dir); err == nil { t.Fatalf("Expected error to be raised when writing file with encoding") diff --git a/system/networkd.go b/system/networkd.go index 0749c9c..9fb6409 100644 --- a/system/networkd.go +++ b/system/networkd.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/network" "github.com/coreos/coreos-cloudinit/third_party/github.com/dotcloud/docker/pkg/netlink" ) @@ -108,11 +109,11 @@ func WriteNetworkdConfigs(interfaces []network.InterfaceGenerator) error { return nil } -func writeConfig(filename string, config string) error { - if config == "" { +func writeConfig(filename string, content string) error { + if content == "" { return nil } log.Printf("Writing networkd unit %q\n", filename) - _, err := WriteFile(&File{Content: config, Path: filename}, runtimeNetworkPath) + _, err := WriteFile(&File{config.File{Content: content, Path: filename}}, runtimeNetworkPath) return err } diff --git a/system/oem.go b/system/oem.go index 38e3071..4d21230 100644 --- a/system/oem.go +++ b/system/oem.go @@ -24,9 +24,9 @@ func (oem OEM) File() (*File, error) { content += fmt.Sprintf("HOME_URL=%q\n", oem.HomeURL) content += fmt.Sprintf("BUG_REPORT_URL=%q\n", oem.BugReportURL) - return &File{ + return &File{config.File{ Path: path.Join("etc", "oem-release"), RawFilePermissions: "0644", Content: content, - }, nil + }}, nil } diff --git a/system/oem_test.go b/system/oem_test.go index 62ebbbf..a2755c5 100644 --- a/system/oem_test.go +++ b/system/oem_test.go @@ -24,7 +24,7 @@ func TestOEMFile(t *testing.T) { HomeURL: "https://www.rackspace.com/cloud/servers/", BugReportURL: "https://github.com/coreos/coreos-overlay", }, - &File{ + &File{config.File{ Path: "etc/oem-release", RawFilePermissions: "0644", Content: `ID=rackspace @@ -33,7 +33,7 @@ NAME="Rackspace Cloud Servers" HOME_URL="https://www.rackspace.com/cloud/servers/" BUG_REPORT_URL="https://github.com/coreos/coreos-overlay" `, - }, + }}, }, } { file, err := OEM{tt.config}.File() diff --git a/system/systemd.go b/system/systemd.go index 73e90ad..01462ff 100644 --- a/system/systemd.go +++ b/system/systemd.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/third_party/github.com/coreos/go-systemd/dbus" ) @@ -35,11 +36,11 @@ func (s *systemd) PlaceUnit(u *Unit, dst string) error { } } - file := File{ + file := File{config.File{ Path: filepath.Base(dst), Content: u.Content, RawFilePermissions: "0644", - } + }} _, err := WriteFile(&file, dir) if err != nil { diff --git a/system/update.go b/system/update.go index 8ee18ac..7f98dfe 100644 --- a/system/update.go +++ b/system/update.go @@ -90,11 +90,11 @@ func (uc Update) File() (*File, error) { out += "\n" } - return &File{ + return &File{config.File{ Path: path.Join("etc", "coreos", "update.conf"), RawFilePermissions: "0644", Content: out, - }, nil + }}, nil } // Units generates units for the cloud-init initializer to act on: diff --git a/system/update_test.go b/system/update_test.go index 1c37d9e..2ff95d2 100644 --- a/system/update_test.go +++ b/system/update_test.go @@ -92,52 +92,52 @@ func TestUpdateFile(t *testing.T) { }, { config: config.Update{Group: "master", Server: "http://foo.com"}, - file: &File{ + file: &File{config.File{ Content: "GROUP=master\nSERVER=http://foo.com\n", Path: "etc/coreos/update.conf", RawFilePermissions: "0644", - }, + }}, }, { config: config.Update{RebootStrategy: "best-effort"}, - file: &File{ + file: &File{config.File{ Content: "REBOOT_STRATEGY=best-effort\n", Path: "etc/coreos/update.conf", RawFilePermissions: "0644", - }, + }}, }, { config: config.Update{RebootStrategy: "etcd-lock"}, - file: &File{ + file: &File{config.File{ Content: "REBOOT_STRATEGY=etcd-lock\n", Path: "etc/coreos/update.conf", RawFilePermissions: "0644", - }, + }}, }, { config: config.Update{RebootStrategy: "reboot"}, - file: &File{ + file: &File{config.File{ Content: "REBOOT_STRATEGY=reboot\n", Path: "etc/coreos/update.conf", RawFilePermissions: "0644", - }, + }}, }, { config: config.Update{RebootStrategy: "off"}, - file: &File{ + file: &File{config.File{ Content: "REBOOT_STRATEGY=off\n", Path: "etc/coreos/update.conf", RawFilePermissions: "0644", - }, + }}, }, { config: config.Update{RebootStrategy: "etcd-lock"}, orig: "SERVER=https://example.com\nGROUP=thegroupc\nREBOOT_STRATEGY=awesome", - file: &File{ + file: &File{config.File{ Content: "SERVER=https://example.com\nGROUP=thegroupc\nREBOOT_STRATEGY=etcd-lock\n", Path: "etc/coreos/update.conf", RawFilePermissions: "0644", - }, + }}, }, } { file, err := Update{tt.config, testReadConfig(tt.orig)}.File() From 4b472795c458a874daf76220c8be2f6f593b3dec Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 16:33:01 -0700 Subject: [PATCH 7/8] user: move User into config package - Add YAML tags for the fields --- config/user.go | 17 +++++++++++++++++ initialize/config.go | 4 ++-- system/user.go | 22 ++++------------------ 3 files changed, 23 insertions(+), 20 deletions(-) create mode 100644 config/user.go diff --git a/config/user.go b/config/user.go new file mode 100644 index 0000000..d66fbe8 --- /dev/null +++ b/config/user.go @@ -0,0 +1,17 @@ +package config + +type User struct { + Name string `yaml:"name"` + PasswordHash string `yaml:"passwd"` + SSHAuthorizedKeys []string `yaml:"ssh-authorized-keys"` + SSHImportGithubUser string `yaml:"coreos-ssh-import-github"` + SSHImportURL string `yaml:"coreos-ssh-import-url"` + GECOS string `yaml:"gecos"` + Homedir string `yaml:"homedir"` + NoCreateHome bool `yaml:"no-create-home"` + PrimaryGroup string `yaml:"primary-group"` + Groups []string `yaml:"groups"` + NoUserGroup bool `yaml:"no-user-group"` + System bool `yaml:"system"` + NoLogInit bool `yaml:"no-log-init"` +} diff --git a/initialize/config.go b/initialize/config.go index 79ec5e7..61ac7a4 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -39,7 +39,7 @@ type CloudConfig struct { } WriteFiles []config.File `yaml:"write_files"` Hostname string - Users []system.User + Users []config.User ManageEtcHosts config.EtcHosts `yaml:"manage_etc_hosts"` NetworkConfigPath string NetworkConfig string @@ -85,7 +85,7 @@ func warnOnUnrecognizedKeys(contents string, warn warner) { // Check for any badly-specified users, if any are set if users, ok := c["users"]; ok { var known map[string]interface{} - b, _ := yaml.Marshal(&system.User{}) + b, _ := yaml.Marshal(&config.User{}) yaml.Unmarshal(b, &known) if set, ok := users.([]interface{}); ok { diff --git a/system/user.go b/system/user.go index 9e627f0..849e735 100644 --- a/system/user.go +++ b/system/user.go @@ -6,30 +6,16 @@ import ( "os/exec" "os/user" "strings" + + "github.com/coreos/coreos-cloudinit/config" ) -type User struct { - Name string `yaml:"name"` - PasswordHash string `yaml:"passwd"` - SSHAuthorizedKeys []string `yaml:"ssh-authorized-keys"` - SSHImportGithubUser string `yaml:"coreos-ssh-import-github"` - SSHImportURL string `yaml:"coreos-ssh-import-url"` - GECOS string `yaml:"gecos"` - Homedir string `yaml:"homedir"` - NoCreateHome bool `yaml:"no-create-home"` - PrimaryGroup string `yaml:"primary-group"` - Groups []string `yaml:"groups"` - NoUserGroup bool `yaml:"no-user-group"` - System bool `yaml:"system"` - NoLogInit bool `yaml:"no-log-init"` -} - -func UserExists(u *User) bool { +func UserExists(u *config.User) bool { _, err := user.Lookup(u.Name) return err == nil } -func CreateUser(u *User) error { +func CreateUser(u *config.User) error { args := []string{} if u.PasswordHash != "" { From 18e2f98414330eef1480995f778b00f9c7271be3 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Sun, 21 Sep 2014 16:46:58 -0700 Subject: [PATCH 8/8] cloudconfig: refactor config - Move CloudConfig into config package - Add YAML tags to CloudConfig --- config/config.go | 132 +++++++++++ config/config_test.go | 410 +++++++++++++++++++++++++++++++++++ coreos-cloudinit.go | 9 +- coreos-cloudinit_test.go | 39 ++-- initialize/config.go | 131 +---------- initialize/config_test.go | 357 ------------------------------ initialize/github_test.go | 32 --- initialize/meta_data.go | 6 +- initialize/meta_data_test.go | 20 +- initialize/ssh_keys_test.go | 27 --- initialize/user_data.go | 3 +- initialize/user_data_test.go | 4 +- 12 files changed, 588 insertions(+), 582 deletions(-) delete mode 100644 initialize/github_test.go diff --git a/config/config.go b/config/config.go index 7ca7a42..3fdc036 100644 --- a/config/config.go +++ b/config/config.go @@ -2,10 +2,58 @@ package config import ( "fmt" + "log" "reflect" "strings" + + "github.com/coreos/coreos-cloudinit/third_party/gopkg.in/yaml.v1" ) +// CloudConfig encapsulates the entire cloud-config configuration file and maps +// directly to YAML. Fields that cannot be set in the cloud-config (fields +// used for internal use) have the YAML tag '-' so that they aren't marshalled. +type CloudConfig struct { + SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"` + Coreos struct { + Etcd Etcd `yaml:"etcd"` + Fleet Fleet `yaml:"fleet"` + OEM OEM `yaml:"oem"` + Update Update `yaml:"update"` + Units []Unit `yaml:"units"` + } `yaml:"coreos"` + WriteFiles []File `yaml:"write_files"` + Hostname string `yaml:"hostname"` + Users []User `yaml:"users"` + ManageEtcHosts EtcHosts `yaml:"manage_etc_hosts"` + NetworkConfigPath string `yaml:"-"` + NetworkConfig string `yaml:"-"` +} + +// 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 := yaml.Unmarshal([]byte(contents), &cfg) + if err != nil { + return &cfg, err + } + warnOnUnrecognizedKeys(contents, log.Printf) + return &cfg, nil +} + +func (cc CloudConfig) String() string { + bytes, err := yaml.Marshal(cc) + if err != nil { + return "" + } + + stringified := string(bytes) + stringified = fmt.Sprintf("#cloud-config\n%s", stringified) + + return stringified +} + // IsZero returns whether or not the parameter is the zero value for its type. // If the parameter is a struct, only the exported fields are considered. func IsZero(c interface{}) bool { @@ -64,3 +112,87 @@ func isValid(v reflect.Value, valid string) bool { } return false } + +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, _ := yaml.Marshal(&CloudConfig{}) + yaml.Unmarshal(b, &cc) + + // Now unmarshal the entire provided contents + var c map[string]interface{} + yaml.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 + if coreos, ok := c["coreos"]; ok { + if set, ok := coreos.(map[interface{}]interface{}); ok { + known := cc["coreos"].(map[interface{}]interface{}) + for k, _ := range set { + if key, ok := k.(string); ok { + if _, ok := known[key]; !ok { + warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", key) + } + } else { + warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", k) + } + } + } + } + + // Check for any badly-specified users, if any are set + if users, ok := c["users"]; ok { + var known map[string]interface{} + b, _ := yaml.Marshal(&User{}) + yaml.Unmarshal(b, &known) + + if set, ok := users.([]interface{}); ok { + for _, u := range set { + if user, ok := u.(map[interface{}]interface{}); ok { + for k, _ := range user { + if key, ok := k.(string); ok { + if _, ok := known[key]; !ok { + warn("Warning: unrecognized key %q in user section of cloud config - ignoring", key) + } + } else { + warn("Warning: unrecognized key %q in user section of cloud config - ignoring", k) + } + } + } + } + } + } + + // Check for any badly-specified files, if any are set + if files, ok := c["write_files"]; ok { + var known map[string]interface{} + b, _ := yaml.Marshal(&File{}) + yaml.Unmarshal(b, &known) + + if set, ok := files.([]interface{}); ok { + for _, f := range set { + if file, ok := f.(map[interface{}]interface{}); ok { + for k, _ := range file { + if key, ok := k.(string); ok { + if _, ok := known[key]; !ok { + warn("Warning: unrecognized key %q in file section of cloud config - ignoring", key) + } + } else { + warn("Warning: unrecognized key %q in file section of cloud config - ignoring", k) + } + } + } + } + } + } +} diff --git a/config/config_test.go b/config/config_test.go index 9042f87..86da82e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -2,7 +2,9 @@ package config import ( "errors" + "fmt" "reflect" + "strings" "testing" ) @@ -61,3 +63,411 @@ func TestAssertValid(t *testing.T) { } } } + +func TestCloudConfigInvalidKeys(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic while instantiating CloudConfig with nil keys: %v", r) + } + }() + + for _, tt := range []struct { + contents string + }{ + {"coreos:"}, + {"ssh_authorized_keys:"}, + {"ssh_authorized_keys:\n -"}, + {"ssh_authorized_keys:\n - 0:"}, + {"write_files:"}, + {"write_files:\n -"}, + {"write_files:\n - 0:"}, + {"users:"}, + {"users:\n -"}, + {"users:\n - 0:"}, + } { + _, err := NewCloudConfig(tt.contents) + if err != nil { + t.Fatalf("error instantiating CloudConfig with invalid keys: %v", err) + } + } +} + +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 cfg.Coreos.Etcd.Discovery != "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" { + 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("") + if err != nil { + t.Fatalf("Encountered unexpected error :%v", err) + } + + keys := cfg.SSHAuthorizedKeys + if len(keys) != 0 { + t.Error("Parsed incorrect number of SSH keys") + } + + if len(cfg.WriteFiles) != 0 { + t.Error("Expected zero WriteFiles") + } + + if cfg.Hostname != "" { + t.Errorf("Expected hostname to be empty, got '%s'", cfg.Hostname) + } +} + +// Assert that the parsing of a cloud config file "generally works" +func TestCloudConfig(t *testing.T) { + contents := ` +coreos: + etcd: + discovery: "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" + update: + reboot-strategy: reboot + units: + - name: 50-eth0.network + runtime: yes + content: '[Match] + + Name=eth47 + + + [Network] + + Address=10.209.171.177/19 + +' + oem: + id: rackspace + name: Rackspace Cloud Servers + version-id: 168.0.0 + home-url: https://www.rackspace.com/cloud/servers/ + bug-report-url: https://github.com/coreos/coreos-overlay +ssh_authorized_keys: + - foobar + - foobaz +write_files: + - content: | + penny + elroy + path: /etc/dogepack.conf + permissions: '0644' + owner: root:dogepack +hostname: trontastic +` + cfg, err := NewCloudConfig(contents) + if err != nil { + t.Fatalf("Encountered unexpected error :%v", err) + } + + keys := cfg.SSHAuthorizedKeys + if len(keys) != 2 { + t.Error("Parsed incorrect number of SSH keys") + } else if keys[0] != "foobar" { + t.Error("Expected first SSH key to be 'foobar'") + } else if keys[1] != "foobaz" { + t.Error("Expected first SSH key to be 'foobaz'") + } + + if len(cfg.WriteFiles) != 1 { + t.Error("Failed to parse correct number of write_files") + } else { + wf := cfg.WriteFiles[0] + if wf.Content != "penny\nelroy\n" { + t.Errorf("WriteFile has incorrect contents '%s'", wf.Content) + } + if wf.Encoding != "" { + t.Errorf("WriteFile has incorrect encoding %s", wf.Encoding) + } + if wf.RawFilePermissions != "0644" { + t.Errorf("WriteFile has incorrect permissions %s", wf.RawFilePermissions) + } + if wf.Path != "/etc/dogepack.conf" { + t.Errorf("WriteFile has incorrect path %s", wf.Path) + } + if wf.Owner != "root:dogepack" { + t.Errorf("WriteFile has incorrect owner %s", wf.Owner) + } + } + + if len(cfg.Coreos.Units) != 1 { + t.Error("Failed to parse correct number of units") + } else { + u := cfg.Coreos.Units[0] + expect := `[Match] +Name=eth47 + +[Network] +Address=10.209.171.177/19 +` + if u.Content != expect { + t.Errorf("Unit has incorrect contents '%s'.\nExpected '%s'.", u.Content, expect) + } + if u.Runtime != true { + t.Errorf("Unit has incorrect runtime value") + } + if u.Name != "50-eth0.network" { + t.Errorf("Unit has incorrect name %s", u.Name) + } + if u.Type() != "network" { + t.Errorf("Unit has incorrect type '%s'", u.Type()) + } + } + + if cfg.Coreos.OEM.ID != "rackspace" { + t.Errorf("Failed parsing coreos.oem. Expected ID 'rackspace', got %q.", cfg.Coreos.OEM.ID) + } + + if cfg.Hostname != "trontastic" { + t.Errorf("Failed to parse hostname") + } + if cfg.Coreos.Update.RebootStrategy != "reboot" { + t.Errorf("Failed to parse locksmith strategy") + } +} + +// Assert that our interface conversion doesn't panic +func TestCloudConfigKeysNotList(t *testing.T) { + contents := ` +ssh_authorized_keys: + - foo: bar +` + cfg, err := NewCloudConfig(contents) + if err != nil { + t.Fatalf("Encountered unexpected error: %v", err) + } + + keys := cfg.SSHAuthorizedKeys + if len(keys) != 0 { + t.Error("Parsed incorrect number of SSH keys") + } +} + +func TestCloudConfigSerializationHeader(t *testing.T) { + cfg, _ := NewCloudConfig("") + contents := cfg.String() + header := strings.SplitN(contents, "\n", 2)[0] + if header != "#cloud-config" { + t.Fatalf("Serialized config did not have expected header") + } +} + +// TestDropInIgnored asserts that users are unable to set DropIn=True on units +func TestDropInIgnored(t *testing.T) { + contents := ` +coreos: + units: + - name: test + dropin: true +` + cfg, err := NewCloudConfig(contents) + if err != nil || len(cfg.Coreos.Units) != 1 { + t.Fatalf("Encountered unexpected error: %v", err) + } + if len(cfg.Coreos.Units) != 1 || cfg.Coreos.Units[0].Name != "test" { + t.Fatalf("Expected 1 unit, but got %d: %v", len(cfg.Coreos.Units), cfg.Coreos.Units) + } + if cfg.Coreos.Units[0].DropIn { + t.Errorf("dropin option on unit in cloud-config was not ignored!") + } +} + +func TestCloudConfigUsers(t *testing.T) { + contents := ` +users: + - name: elroy + passwd: somehash + ssh-authorized-keys: + - somekey + gecos: arbitrary comment + homedir: /home/place + no-create-home: yes + primary-group: things + groups: + - ping + - pong + no-user-group: true + system: y + no-log-init: True +` + cfg, err := NewCloudConfig(contents) + if err != nil { + t.Fatalf("Encountered unexpected error: %v", err) + } + + if len(cfg.Users) != 1 { + t.Fatalf("Parsed %d users, expected 1", cfg.Users) + } + + user := cfg.Users[0] + + if user.Name != "elroy" { + t.Errorf("User name is %q, expected 'elroy'", user.Name) + } + + if user.PasswordHash != "somehash" { + t.Errorf("User passwd is %q, expected 'somehash'", user.PasswordHash) + } + + if keys := user.SSHAuthorizedKeys; len(keys) != 1 { + t.Errorf("Parsed %d ssh keys, expected 1", len(keys)) + } else { + key := user.SSHAuthorizedKeys[0] + if key != "somekey" { + t.Errorf("User SSH key is %q, expected 'somekey'", key) + } + } + + if user.GECOS != "arbitrary comment" { + t.Errorf("Failed to parse gecos field, got %q", user.GECOS) + } + + if user.Homedir != "/home/place" { + t.Errorf("Failed to parse homedir field, got %q", user.Homedir) + } + + if !user.NoCreateHome { + t.Errorf("Failed to parse no-create-home field") + } + + if user.PrimaryGroup != "things" { + t.Errorf("Failed to parse primary-group field, got %q", user.PrimaryGroup) + } + + if len(user.Groups) != 2 { + t.Errorf("Failed to parse 2 goups, got %d", len(user.Groups)) + } else { + if user.Groups[0] != "ping" { + t.Errorf("First group was %q, not expected value 'ping'", user.Groups[0]) + } + if user.Groups[1] != "pong" { + t.Errorf("First group was %q, not expected value 'pong'", user.Groups[1]) + } + } + + if !user.NoUserGroup { + t.Errorf("Failed to parse no-user-group field") + } + + if !user.System { + t.Errorf("Failed to parse system field") + } + + if !user.NoLogInit { + t.Errorf("Failed to parse no-log-init field") + } +} + +func TestCloudConfigUsersGithubUser(t *testing.T) { + + contents := ` +users: + - name: elroy + coreos-ssh-import-github: bcwaldon +` + cfg, err := NewCloudConfig(contents) + if err != nil { + t.Fatalf("Encountered unexpected error: %v", err) + } + + if len(cfg.Users) != 1 { + t.Fatalf("Parsed %d users, expected 1", cfg.Users) + } + + user := cfg.Users[0] + + if user.Name != "elroy" { + t.Errorf("User name is %q, expected 'elroy'", user.Name) + } + + if user.SSHImportGithubUser != "bcwaldon" { + t.Errorf("github user is %q, expected 'bcwaldon'", user.SSHImportGithubUser) + } +} + +func TestCloudConfigUsersSSHImportURL(t *testing.T) { + contents := ` +users: + - name: elroy + coreos-ssh-import-url: https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys +` + cfg, err := NewCloudConfig(contents) + if err != nil { + t.Fatalf("Encountered unexpected error: %v", err) + } + + if len(cfg.Users) != 1 { + t.Fatalf("Parsed %d users, expected 1", cfg.Users) + } + + user := cfg.Users[0] + + if user.Name != "elroy" { + t.Errorf("User name is %q, expected 'elroy'", user.Name) + } + + if user.SSHImportURL != "https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys" { + t.Errorf("ssh import url is %q, expected 'https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys'", user.SSHImportURL) + } +} diff --git a/coreos-cloudinit.go b/coreos-cloudinit.go index 214f4b3..8e72ec6 100644 --- a/coreos-cloudinit.go +++ b/coreos-cloudinit.go @@ -7,6 +7,7 @@ import ( "sync" "time" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/datasource" "github.com/coreos/coreos-cloudinit/datasource/configdrive" "github.com/coreos/coreos-cloudinit/datasource/file" @@ -156,7 +157,7 @@ func main() { env := initialize.NewEnvironment("/", ds.ConfigRoot(), flags.workspace, flags.convertNetconf, flags.sshKeyName, subs) userdata := env.Apply(string(userdataBytes)) - var ccm, ccu *initialize.CloudConfig + var ccm, ccu *config.CloudConfig var script *system.Script if ccm, err = initialize.ParseMetaData(string(metadataBytes)); err != nil { fmt.Printf("Failed to parse meta-data: %v\n", err) @@ -178,14 +179,14 @@ func main() { failure = true } else { switch t := ud.(type) { - case *initialize.CloudConfig: + case *config.CloudConfig: ccu = t case system.Script: script = &t } } - var cc *initialize.CloudConfig + var cc *config.CloudConfig if ccm != nil && ccu != nil { fmt.Println("Merging cloud-config from meta-data and user-data") merged := mergeCloudConfig(*ccm, *ccu) @@ -224,7 +225,7 @@ func main() { // 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) { +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) diff --git a/coreos-cloudinit_test.go b/coreos-cloudinit_test.go index cd86284..a69db06 100644 --- a/coreos-cloudinit_test.go +++ b/coreos-cloudinit_test.go @@ -4,38 +4,37 @@ import ( "reflect" "testing" - "github.com/coreos/coreos-cloudinit/initialize" "github.com/coreos/coreos-cloudinit/config" ) func TestMergeCloudConfig(t *testing.T) { - simplecc := initialize.CloudConfig{ + simplecc := config.CloudConfig{ SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "foobar", NetworkConfigPath: "/path/somewhere", NetworkConfig: `{}`, } for i, tt := range []struct { - udcc initialize.CloudConfig - mdcc initialize.CloudConfig - want initialize.CloudConfig + udcc config.CloudConfig + mdcc config.CloudConfig + want config.CloudConfig }{ { // If mdcc is empty, udcc should be returned unchanged simplecc, - initialize.CloudConfig{}, + config.CloudConfig{}, simplecc, }, { // If udcc is empty, mdcc should be returned unchanged(overridden) - initialize.CloudConfig{}, + config.CloudConfig{}, simplecc, simplecc, }, { // user-data should override completely in the case of conflicts simplecc, - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "meta-hostname", NetworkConfigPath: "/path/meta", NetworkConfig: `{"hostname":"test"}`, @@ -44,17 +43,17 @@ func TestMergeCloudConfig(t *testing.T) { }, { // Mixed merge should succeed - initialize.CloudConfig{ + config.CloudConfig{ SSHAuthorizedKeys: []string{"abc", "def"}, Hostname: "user-hostname", NetworkConfigPath: "/path/somewhere", NetworkConfig: `{"hostname":"test"}`, }, - initialize.CloudConfig{ + config.CloudConfig{ SSHAuthorizedKeys: []string{"woof", "qux"}, Hostname: "meta-hostname", }, - initialize.CloudConfig{ + config.CloudConfig{ SSHAuthorizedKeys: []string{"abc", "def", "woof", "qux"}, Hostname: "user-hostname", NetworkConfigPath: "/path/somewhere", @@ -63,15 +62,15 @@ func TestMergeCloudConfig(t *testing.T) { }, { // Completely non-conflicting merge should be fine - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "supercool", }, - initialize.CloudConfig{ + config.CloudConfig{ SSHAuthorizedKeys: []string{"zaphod", "beeblebrox"}, NetworkConfigPath: "/dev/fun", NetworkConfig: `{"hostname":"test"}`, }, - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "supercool", SSHAuthorizedKeys: []string{"zaphod", "beeblebrox"}, NetworkConfigPath: "/dev/fun", @@ -80,16 +79,16 @@ func TestMergeCloudConfig(t *testing.T) { }, { // Non-mergeable settings in user-data should not be affected - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "mememe", ManageEtcHosts: config.EtcHosts("lolz"), }, - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "youyouyou", NetworkConfigPath: "meta-meta-yo", NetworkConfig: `{"hostname":"test"}`, }, - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "mememe", ManageEtcHosts: config.EtcHosts("lolz"), NetworkConfigPath: "meta-meta-yo", @@ -98,15 +97,15 @@ func TestMergeCloudConfig(t *testing.T) { }, { // Non-mergeable (unexpected) settings in meta-data are ignored - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "mememe", }, - initialize.CloudConfig{ + config.CloudConfig{ ManageEtcHosts: config.EtcHosts("lolz"), NetworkConfigPath: "meta-meta-yo", NetworkConfig: `{"hostname":"test"}`, }, - initialize.CloudConfig{ + config.CloudConfig{ Hostname: "mememe", NetworkConfigPath: "meta-meta-yo", NetworkConfig: `{"hostname":"test"}`, diff --git a/initialize/config.go b/initialize/config.go index 61ac7a4..752a21a 100644 --- a/initialize/config.go +++ b/initialize/config.go @@ -6,8 +6,6 @@ import ( "log" "path" - "github.com/coreos/coreos-cloudinit/third_party/gopkg.in/yaml.v1" - "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/network" "github.com/coreos/coreos-cloudinit/system" @@ -27,137 +25,10 @@ type CloudConfigUnit interface { Units() ([]system.Unit, error) } -// CloudConfig encapsulates the entire cloud-config configuration file and maps directly to YAML -type CloudConfig struct { - SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"` - Coreos struct { - Etcd config.Etcd - Fleet config.Fleet - OEM config.OEM - Update config.Update - Units []config.Unit - } - WriteFiles []config.File `yaml:"write_files"` - Hostname string - Users []config.User - ManageEtcHosts config.EtcHosts `yaml:"manage_etc_hosts"` - NetworkConfigPath string - NetworkConfig string -} - -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, _ := yaml.Marshal(&CloudConfig{}) - yaml.Unmarshal(b, &cc) - - // Now unmarshal the entire provided contents - var c map[string]interface{} - yaml.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 - if coreos, ok := c["coreos"]; ok { - if set, ok := coreos.(map[interface{}]interface{}); ok { - known := cc["coreos"].(map[interface{}]interface{}) - for k, _ := range set { - if key, ok := k.(string); ok { - if _, ok := known[key]; !ok { - warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", key) - } - } else { - warn("Warning: unrecognized key %q in coreos section of provided cloud config - ignoring", k) - } - } - } - } - - // Check for any badly-specified users, if any are set - if users, ok := c["users"]; ok { - var known map[string]interface{} - b, _ := yaml.Marshal(&config.User{}) - yaml.Unmarshal(b, &known) - - if set, ok := users.([]interface{}); ok { - for _, u := range set { - if user, ok := u.(map[interface{}]interface{}); ok { - for k, _ := range user { - if key, ok := k.(string); ok { - if _, ok := known[key]; !ok { - warn("Warning: unrecognized key %q in user section of cloud config - ignoring", key) - } - } else { - warn("Warning: unrecognized key %q in user section of cloud config - ignoring", k) - } - } - } - } - } - } - - // Check for any badly-specified files, if any are set - if files, ok := c["write_files"]; ok { - var known map[string]interface{} - b, _ := yaml.Marshal(&system.File{}) - yaml.Unmarshal(b, &known) - - if set, ok := files.([]interface{}); ok { - for _, f := range set { - if file, ok := f.(map[interface{}]interface{}); ok { - for k, _ := range file { - if key, ok := k.(string); ok { - if _, ok := known[key]; !ok { - warn("Warning: unrecognized key %q in file section of cloud config - ignoring", key) - } - } else { - warn("Warning: unrecognized key %q in file section of cloud config - ignoring", k) - } - } - } - } - } - } -} - -// 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 := yaml.Unmarshal([]byte(contents), &cfg) - if err != nil { - return &cfg, err - } - warnOnUnrecognizedKeys(contents, log.Printf) - return &cfg, nil -} - -func (cc CloudConfig) String() string { - bytes, err := yaml.Marshal(cc) - if err != nil { - return "" - } - - stringified := string(bytes) - stringified = fmt.Sprintf("#cloud-config\n%s", stringified) - - return stringified -} - // Apply renders a CloudConfig to an Environment. This can involve things like // configuring the hostname, adding new users, writing various configuration // files to disk, and manipulating systemd services. -func Apply(cfg CloudConfig, env *Environment) error { +func Apply(cfg config.CloudConfig, env *Environment) error { if cfg.Hostname != "" { if err := system.SetHostname(cfg.Hostname); err != nil { return err diff --git a/initialize/config_test.go b/initialize/config_test.go index fa5ed93..82e5f5b 100644 --- a/initialize/config_test.go +++ b/initialize/config_test.go @@ -1,369 +1,12 @@ package initialize import ( - "fmt" - "strings" "testing" "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/system" ) -func TestCloudConfigInvalidKeys(t *testing.T) { - defer func() { - if r := recover(); r != nil { - t.Fatalf("panic while instantiating CloudConfig with nil keys: %v", r) - } - }() - - for _, tt := range []struct { - contents string - }{ - {"coreos:"}, - {"ssh_authorized_keys:"}, - {"ssh_authorized_keys:\n -"}, - {"ssh_authorized_keys:\n - 0:"}, - {"write_files:"}, - {"write_files:\n -"}, - {"write_files:\n - 0:"}, - {"users:"}, - {"users:\n -"}, - {"users:\n - 0:"}, - } { - _, err := NewCloudConfig(tt.contents) - if err != nil { - t.Fatalf("error instantiating CloudConfig with invalid keys: %v", err) - } - } -} - -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 cfg.Coreos.Etcd.Discovery != "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" { - 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("") - if err != nil { - t.Fatalf("Encountered unexpected error :%v", err) - } - - keys := cfg.SSHAuthorizedKeys - if len(keys) != 0 { - t.Error("Parsed incorrect number of SSH keys") - } - - if len(cfg.WriteFiles) != 0 { - t.Error("Expected zero WriteFiles") - } - - if cfg.Hostname != "" { - t.Errorf("Expected hostname to be empty, got '%s'", cfg.Hostname) - } -} - -// Assert that the parsing of a cloud config file "generally works" -func TestCloudConfig(t *testing.T) { - contents := ` -coreos: - etcd: - discovery: "https://discovery.etcd.io/827c73219eeb2fa5530027c37bf18877" - update: - reboot-strategy: reboot - units: - - name: 50-eth0.network - runtime: yes - content: '[Match] - - Name=eth47 - - - [Network] - - Address=10.209.171.177/19 - -' - oem: - id: rackspace - name: Rackspace Cloud Servers - version-id: 168.0.0 - home-url: https://www.rackspace.com/cloud/servers/ - bug-report-url: https://github.com/coreos/coreos-overlay -ssh_authorized_keys: - - foobar - - foobaz -write_files: - - content: | - penny - elroy - path: /etc/dogepack.conf - permissions: '0644' - owner: root:dogepack -hostname: trontastic -` - cfg, err := NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error :%v", err) - } - - keys := cfg.SSHAuthorizedKeys - if len(keys) != 2 { - t.Error("Parsed incorrect number of SSH keys") - } else if keys[0] != "foobar" { - t.Error("Expected first SSH key to be 'foobar'") - } else if keys[1] != "foobaz" { - t.Error("Expected first SSH key to be 'foobaz'") - } - - if len(cfg.WriteFiles) != 1 { - t.Error("Failed to parse correct number of write_files") - } else { - wf := system.File{cfg.WriteFiles[0]} - if wf.Content != "penny\nelroy\n" { - t.Errorf("WriteFile has incorrect contents '%s'", wf.Content) - } - if wf.Encoding != "" { - t.Errorf("WriteFile has incorrect encoding %s", wf.Encoding) - } - if perm, _ := wf.Permissions(); perm != 0644 { - t.Errorf("WriteFile has incorrect permissions %s", perm) - } - if wf.Path != "/etc/dogepack.conf" { - t.Errorf("WriteFile has incorrect path %s", wf.Path) - } - if wf.Owner != "root:dogepack" { - t.Errorf("WriteFile has incorrect owner %s", wf.Owner) - } - } - - if len(cfg.Coreos.Units) != 1 { - t.Error("Failed to parse correct number of units") - } else { - u := cfg.Coreos.Units[0] - expect := `[Match] -Name=eth47 - -[Network] -Address=10.209.171.177/19 -` - if u.Content != expect { - t.Errorf("Unit has incorrect contents '%s'.\nExpected '%s'.", u.Content, expect) - } - if u.Runtime != true { - t.Errorf("Unit has incorrect runtime value") - } - if u.Name != "50-eth0.network" { - t.Errorf("Unit has incorrect name %s", u.Name) - } - if u.Type() != "network" { - t.Errorf("Unit has incorrect type '%s'", u.Type()) - } - } - - if cfg.Coreos.OEM.ID != "rackspace" { - t.Errorf("Failed parsing coreos.oem. Expected ID 'rackspace', got %q.", cfg.Coreos.OEM.ID) - } - - if cfg.Hostname != "trontastic" { - t.Errorf("Failed to parse hostname") - } - if cfg.Coreos.Update.RebootStrategy != "reboot" { - t.Errorf("Failed to parse locksmith strategy") - } -} - -// Assert that our interface conversion doesn't panic -func TestCloudConfigKeysNotList(t *testing.T) { - contents := ` -ssh_authorized_keys: - - foo: bar -` - cfg, err := NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error: %v", err) - } - - keys := cfg.SSHAuthorizedKeys - if len(keys) != 0 { - t.Error("Parsed incorrect number of SSH keys") - } -} - -func TestCloudConfigSerializationHeader(t *testing.T) { - cfg, _ := NewCloudConfig("") - contents := cfg.String() - header := strings.SplitN(contents, "\n", 2)[0] - if header != "#cloud-config" { - t.Fatalf("Serialized config did not have expected header") - } -} - -// TestDropInIgnored asserts that users are unable to set DropIn=True on units -func TestDropInIgnored(t *testing.T) { - contents := ` -coreos: - units: - - name: test - dropin: true -` - cfg, err := NewCloudConfig(contents) - if err != nil || len(cfg.Coreos.Units) != 1 { - t.Fatalf("Encountered unexpected error: %v", err) - } - if len(cfg.Coreos.Units) != 1 || cfg.Coreos.Units[0].Name != "test" { - t.Fatalf("Expected 1 unit, but got %d: %v", len(cfg.Coreos.Units), cfg.Coreos.Units) - } - if cfg.Coreos.Units[0].DropIn { - t.Errorf("dropin option on unit in cloud-config was not ignored!") - } -} - -func TestCloudConfigUsers(t *testing.T) { - contents := ` -users: - - name: elroy - passwd: somehash - ssh-authorized-keys: - - somekey - gecos: arbitrary comment - homedir: /home/place - no-create-home: yes - primary-group: things - groups: - - ping - - pong - no-user-group: true - system: y - no-log-init: True -` - cfg, err := NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error: %v", err) - } - - if len(cfg.Users) != 1 { - t.Fatalf("Parsed %d users, expected 1", cfg.Users) - } - - user := cfg.Users[0] - - if user.Name != "elroy" { - t.Errorf("User name is %q, expected 'elroy'", user.Name) - } - - if user.PasswordHash != "somehash" { - t.Errorf("User passwd is %q, expected 'somehash'", user.PasswordHash) - } - - if keys := user.SSHAuthorizedKeys; len(keys) != 1 { - t.Errorf("Parsed %d ssh keys, expected 1", len(keys)) - } else { - key := user.SSHAuthorizedKeys[0] - if key != "somekey" { - t.Errorf("User SSH key is %q, expected 'somekey'", key) - } - } - - if user.GECOS != "arbitrary comment" { - t.Errorf("Failed to parse gecos field, got %q", user.GECOS) - } - - if user.Homedir != "/home/place" { - t.Errorf("Failed to parse homedir field, got %q", user.Homedir) - } - - if !user.NoCreateHome { - t.Errorf("Failed to parse no-create-home field") - } - - if user.PrimaryGroup != "things" { - t.Errorf("Failed to parse primary-group field, got %q", user.PrimaryGroup) - } - - if len(user.Groups) != 2 { - t.Errorf("Failed to parse 2 goups, got %d", len(user.Groups)) - } else { - if user.Groups[0] != "ping" { - t.Errorf("First group was %q, not expected value 'ping'", user.Groups[0]) - } - if user.Groups[1] != "pong" { - t.Errorf("First group was %q, not expected value 'pong'", user.Groups[1]) - } - } - - if !user.NoUserGroup { - t.Errorf("Failed to parse no-user-group field") - } - - if !user.System { - t.Errorf("Failed to parse system field") - } - - if !user.NoLogInit { - t.Errorf("Failed to parse no-log-init field") - } -} - type TestUnitManager struct { placed []string enabled []string diff --git a/initialize/github_test.go b/initialize/github_test.go deleted file mode 100644 index 2de5728..0000000 --- a/initialize/github_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package initialize - -import ( - "testing" -) - -func TestCloudConfigUsersGithubUser(t *testing.T) { - - contents := ` -users: - - name: elroy - coreos-ssh-import-github: bcwaldon -` - cfg, err := NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error: %v", err) - } - - if len(cfg.Users) != 1 { - t.Fatalf("Parsed %d users, expected 1", cfg.Users) - } - - user := cfg.Users[0] - - if user.Name != "elroy" { - t.Errorf("User name is %q, expected 'elroy'", user.Name) - } - - if user.SSHImportGithubUser != "bcwaldon" { - t.Errorf("github user is %q, expected 'bcwaldon'", user.SSHImportGithubUser) - } -} diff --git a/initialize/meta_data.go b/initialize/meta_data.go index 23ef08c..68c752a 100644 --- a/initialize/meta_data.go +++ b/initialize/meta_data.go @@ -3,11 +3,13 @@ package initialize import ( "encoding/json" "sort" + + "github.com/coreos/coreos-cloudinit/config" ) // ParseMetaData parses a JSON blob in the OpenStack metadata service format, // and converts it to a partially hydrated CloudConfig. -func ParseMetaData(contents string) (*CloudConfig, error) { +func ParseMetaData(contents string) (*config.CloudConfig, error) { if len(contents) == 0 { return nil, nil } @@ -22,7 +24,7 @@ func ParseMetaData(contents string) (*CloudConfig, error) { return nil, err } - var cfg CloudConfig + var cfg config.CloudConfig if len(metadata.SSHAuthorizedKeyMap) > 0 { cfg.SSHAuthorizedKeys = make([]string, 0, len(metadata.SSHAuthorizedKeyMap)) for _, name := range sortedKeys(metadata.SSHAuthorizedKeyMap) { diff --git a/initialize/meta_data_test.go b/initialize/meta_data_test.go index 5b09fbf..47d44e6 100644 --- a/initialize/meta_data_test.go +++ b/initialize/meta_data_test.go @@ -1,21 +1,25 @@ package initialize -import "reflect" -import "testing" +import ( + "reflect" + "testing" + + "github.com/coreos/coreos-cloudinit/config" +) func TestParseMetadata(t *testing.T) { for i, tt := range []struct { in string - want *CloudConfig + want *config.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{"alice", "jill"}}, 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}, + {`{"foo": "bar"}`, &config.CloudConfig{}, false}, + {`{"network_config": {"content_path": "asdf"}}`, &config.CloudConfig{NetworkConfigPath: "asdf"}, false}, + {`{"hostname": "turkleton"}`, &config.CloudConfig{Hostname: "turkleton"}, false}, + {`{"public_keys": {"jack": "jill", "bob": "alice"}}`, &config.CloudConfig{SSHAuthorizedKeys: []string{"alice", "jill"}}, false}, + {`{"unknown": "thing", "hostname": "my_host", "public_keys": {"do": "re", "mi": "fa"}, "network_config": {"content_path": "/root", "blah": "zzz"}}`, &config.CloudConfig{SSHAuthorizedKeys: []string{"re", "fa"}, Hostname: "my_host", NetworkConfigPath: "/root"}, false}, } { got, err := ParseMetaData(tt.in) if tt.err != (err != nil) { diff --git a/initialize/ssh_keys_test.go b/initialize/ssh_keys_test.go index 2b5d1fe..3efa146 100644 --- a/initialize/ssh_keys_test.go +++ b/initialize/ssh_keys_test.go @@ -39,31 +39,4 @@ func TestCloudConfigUsersUrlMarshal(t *testing.T) { if keys[2] != expected { t.Fatalf("expected %s, got %s", expected, keys[2]) } - -} -func TestCloudConfigUsersSSHImportURL(t *testing.T) { - - contents := ` -users: - - name: elroy - coreos-ssh-import-url: https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys -` - cfg, err := NewCloudConfig(contents) - if err != nil { - t.Fatalf("Encountered unexpected error: %v", err) - } - - if len(cfg.Users) != 1 { - t.Fatalf("Parsed %d users, expected 1", cfg.Users) - } - - user := cfg.Users[0] - - if user.Name != "elroy" { - t.Errorf("User name is %q, expected 'elroy'", user.Name) - } - - if user.SSHImportURL != "https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys" { - t.Errorf("ssh import url is %q, expected 'https://token:x-auth-token@github.enterprise.com/api/v3/polvi/keys'", user.SSHImportURL) - } } diff --git a/initialize/user_data.go b/initialize/user_data.go index 7592602..f62b766 100644 --- a/initialize/user_data.go +++ b/initialize/user_data.go @@ -5,6 +5,7 @@ import ( "log" "strings" + "github.com/coreos/coreos-cloudinit/config" "github.com/coreos/coreos-cloudinit/system" ) @@ -24,7 +25,7 @@ func ParseUserData(contents string) (interface{}, error) { return system.Script(contents), nil } else if header == "#cloud-config" { log.Printf("Parsing user-data as cloud-config") - return NewCloudConfig(contents) + return config.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 006f85f..90b1065 100644 --- a/initialize/user_data_test.go +++ b/initialize/user_data_test.go @@ -2,6 +2,8 @@ package initialize import ( "testing" + + "github.com/coreos/coreos-cloudinit/config" ) func TestParseHeaderCRLF(t *testing.T) { @@ -37,7 +39,7 @@ func TestParseConfigCRLF(t *testing.T) { t.Fatalf("Failed parsing config: %v", err) } - cfg := ud.(*CloudConfig) + cfg := ud.(*config.CloudConfig) if cfg.Hostname != "foo" { t.Error("Failed parsing hostname from config")