From cf548d0fdda9fcc94ea78aa068a980271d6eac4f Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Tue, 10 Aug 2021 09:08:43 -0500 Subject: [PATCH 1/7] add os support for specifying an ipxe configuration for its installer Signed-off-by: Mike Mason --- cmd/boots/main.go | 1 + installers/ipxe/errors.go | 7 + installers/ipxe/main.go | 82 +++++++++++ installers/ipxe/main_test.go | 254 +++++++++++++++++++++++++++++++++++ job/ipxe.go | 14 ++ job/mock.go | 8 ++ packet/models.go | 12 +- 7 files changed, 373 insertions(+), 5 deletions(-) create mode 100644 installers/ipxe/errors.go create mode 100644 installers/ipxe/main.go create mode 100644 installers/ipxe/main_test.go diff --git a/cmd/boots/main.go b/cmd/boots/main.go index a872c271..59350f55 100644 --- a/cmd/boots/main.go +++ b/cmd/boots/main.go @@ -19,6 +19,7 @@ import ( _ "github.com/tinkerbell/boots/installers/coreos" _ "github.com/tinkerbell/boots/installers/custom_ipxe" + _ "github.com/tinkerbell/boots/installers/ipxe" _ "github.com/tinkerbell/boots/installers/nixos" _ "github.com/tinkerbell/boots/installers/osie" _ "github.com/tinkerbell/boots/installers/rancher" diff --git a/installers/ipxe/errors.go b/installers/ipxe/errors.go new file mode 100644 index 00000000..f9779e7d --- /dev/null +++ b/installers/ipxe/errors.go @@ -0,0 +1,7 @@ +package ipxe + +import "errors" + +var ( + ErrEmptyIpxeConfig = errors.New("ipxe config URL or Script must be defined") +) diff --git a/installers/ipxe/main.go b/installers/ipxe/main.go new file mode 100644 index 00000000..62f8593f --- /dev/null +++ b/installers/ipxe/main.go @@ -0,0 +1,82 @@ +package ipxe + +import ( + "encoding/json" + "strings" + + "github.com/packethost/pkg/log" + "github.com/tinkerbell/boots/installers" + "github.com/tinkerbell/boots/ipxe" + "github.com/tinkerbell/boots/job" +) + +func init() { + job.RegisterInstaller("ipxe", ipxeScript) +} + +func ipxeScript(j job.Job, s *ipxe.Script) { + logger := installers.Logger("ipxe") + if j.InstanceID() != "" { + logger = logger.With("instance.id", j.InstanceID()) + } + + cfg, err := ipxeConfigFromJob(j) + if err != nil { + s.Echo("Failed to decode installer data") + s.Shell() + logger.Error(err, "decoding installer data") + + return + } + + IpxeScriptFromConfig(logger, cfg, j, s) +} + +func IpxeScriptFromConfig(logger log.Logger, cfg *Config, j job.Job, s *ipxe.Script) { + if err := cfg.validate(); err != nil { + s.Echo("Invalid ipxe configuration") + s.Shell() + logger.Error(err, "validating ipxe config") + + return + } + + s.PhoneHome("provisioning.104.01") + s.Set("packet_facility", j.FacilityCode()) + s.Set("packet_plan", j.PlanSlug()) + + if cfg.Chain != "" { + s.Chain(cfg.Chain) + } else if cfg.Script != "" { + s.AppendString(strings.TrimPrefix(cfg.Script, "#!ipxe")) + } else { + s.Echo("Unknown ipxe config path") + s.Shell() + } +} + +func ipxeConfigFromJob(j job.Job) (*Config, error) { + data := j.OperatingSystem().InstallerData + + cfg := &Config{} + + err := json.NewDecoder(strings.NewReader(data)).Decode(&cfg) + if err != nil { + return nil, err + } + + return cfg, nil +} + +type Config struct { + Chain string `json:"chain,omitempty"` + Script string `json:"script,omitempty"` +} + +func (c *Config) validate() error { + if c.Chain == "" && c.Script == "" { + return ErrEmptyIpxeConfig + } + + return nil +} diff --git a/installers/ipxe/main_test.go b/installers/ipxe/main_test.go new file mode 100644 index 00000000..3e37a9e0 --- /dev/null +++ b/installers/ipxe/main_test.go @@ -0,0 +1,254 @@ +package ipxe + +import ( + "os" + "regexp" + "testing" + + l "github.com/packethost/pkg/log" + "github.com/stretchr/testify/require" + "github.com/tinkerbell/boots/installers" + "github.com/tinkerbell/boots/ipxe" + "github.com/tinkerbell/boots/job" +) + +var ( + testLogger l.Logger +) + +func TestMain(m *testing.M) { + os.Setenv("PACKET_ENV", "test") + os.Setenv("PACKET_VERSION", "0") + os.Setenv("ROLLBAR_DISABLE", "1") + os.Setenv("ROLLBAR_TOKEN", "1") + + logger, _ := l.Init("github.com/tinkerbell/boots") + job.Init(logger) + installers.Init(logger) + testLogger = logger + os.Exit(m.Run()) +} + +func TestIpxeScript(t *testing.T) { + var testCases = []struct { + name string + installerData string + want string + }{ + { + "invalid config", + "", + `#!ipxe + + echo Failed to decode installer data + shell + `, + }, + { + "valid config", + `{"chain": "http://url/path.ipxe"}`, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + chain --autofree http://url/path.ipxe + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + mockJob := job.NewMock(t, "test.slug", "test.facility") + script := ipxe.NewScript() + + mockJob.SetOSInstallerData(tc.installerData) + + ipxeScript(mockJob.Job(), script) + + assert.Equal(dedent(tc.want), string(script.Bytes())) + }) + } +} + +func TestIpxeScriptFromConfig(t *testing.T) { + var testCases = []struct { + name string + config *Config + want string + }{ + { + "invalid config", + &Config{}, + `#!ipxe + + echo Invalid ipxe configuration + shell + `, + }, + { + "valid chain", + &Config{Chain: "http://url/path.ipxe"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + chain --autofree http://url/path.ipxe + `, + }, + { + "valid script", + &Config{Script: "echo my test script"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + echo my test script + `, + }, + { + "valid script with header", + &Config{Script: "#!ipxe\necho my test script"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + + echo my test script + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + mockJob := job.NewMock(t, "test.slug", "test.facility") + script := ipxe.NewScript() + + IpxeScriptFromConfig(testLogger, tc.config, mockJob.Job(), script) + + assert.Equal(dedent(tc.want), string(script.Bytes())) + }) + } +} + +func TestIpxeConfigFromJob(t *testing.T) { + var testCases = []struct { + name string + installerData string + want *Config + expectError string + }{ + { + "valid chain", + `{"chain": "http://url/path.ipxe"}`, + &Config{Chain: "http://url/path.ipxe"}, + "", + }, + { + "valid script", + `{"script": "echo script"}`, + &Config{Script: "echo script"}, + "", + }, + { + "empty json error", + ``, + nil, + "EOF", + }, + { + "invalid json error", + `{"error"`, + nil, + "unexpected EOF", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + + mockJob := job.NewMock(t, "test.slug", "test.facility") + + mockJob.SetOSInstallerData(tc.installerData) + + cfg, err := ipxeConfigFromJob(mockJob.Job()) + + if tc.expectError == "" { + assert.Nil(err) + } else { + assert.EqualError(err, tc.expectError) + } + + assert.Equal(tc.want, cfg) + }) + } +} + +func TestConfigValidate(t *testing.T) { + var testCases = []struct { + name string + chain string + script string + want string + }{ + {"error when empty", "", "", "ipxe config URL or Script must be defined"}, + {"using chain", "http://chain.url/script.ipxe", "", ""}, + {"using script", "", "#!ipxe\necho ipxe script", ""}, + {"using both", "http://path", "ipxe script", ""}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + + cfg := &Config{ + Chain: tc.chain, + Script: tc.script, + } + + got := cfg.validate() + + if tc.want == "" { + assert.Nil(got) + } else { + assert.EqualError(got, tc.want) + } + }) + } +} + +var dedentRegexp = regexp.MustCompile(`(?m)^[^\S\n]+`) + +func dedent(s string) string { + return dedentRegexp.ReplaceAllString(s, "") +} diff --git a/job/ipxe.go b/job/ipxe.go index 15aed51c..9698d3d9 100644 --- a/job/ipxe.go +++ b/job/ipxe.go @@ -10,6 +10,7 @@ import ( var ( byDistro = make(map[string]BootScript) + byInstaller = make(map[string]BootScript) bySlug = make(map[string]BootScript) defaultInstaller BootScript scripts = map[string]BootScript{ @@ -36,6 +37,14 @@ func RegisterDistro(name string, builder BootScript) { byDistro[name] = builder } +func RegisterInstaller(name string, builder BootScript) { + if _, ok := byInstaller[name]; ok { + err := errors.Errorf("installer %q already registered!", name) + joblog.Fatal(err, "installer", name) + } + byInstaller[name] = builder +} + func RegisterSlug(name string, builder BootScript) { if _, ok := bySlug[name]; ok { err := errors.Errorf("slug %q already registered!", name) @@ -77,6 +86,11 @@ func auto(j Job, s *ipxe.Script) { return } + if f, ok := byInstaller[j.hardware.OperatingSystem().Installer]; ok { + f(j, s) + + return + } if f, ok := bySlug[j.hardware.OperatingSystem().Slug]; ok { f(j, s) diff --git a/job/mock.go b/job/mock.go index 196ddbed..92e4cbe5 100644 --- a/job/mock.go +++ b/job/mock.go @@ -111,6 +111,14 @@ func (m *Mock) SetOSImageTag(tag string) { m.hardware.OperatingSystem().ImageTag = tag } +func (m *Mock) SetOSInstaller(installer string) { + m.hardware.OperatingSystem().Installer = installer +} + +func (m *Mock) SetOSInstallerData(installerData string) { + m.hardware.OperatingSystem().InstallerData = installerData +} + func (m *Mock) SetPassword(password string) { m.instance.CryptedRootPassword = "insecure" m.instance.PasswordHash = "insecure" diff --git a/packet/models.go b/packet/models.go index 2dd34b81..5e32e039 100644 --- a/packet/models.go +++ b/packet/models.go @@ -221,11 +221,13 @@ type IP struct { // OperatingSystem holds details for the operating system type OperatingSystem struct { - Slug string `json:"slug"` - Distro string `json:"distro"` - Version string `json:"version"` - ImageTag string `json:"image_tag"` - OsSlug string `json:"os_slug"` + Slug string `json:"slug"` + Distro string `json:"distro"` + Version string `json:"version"` + ImageTag string `json:"image_tag"` + OsSlug string `json:"os_slug"` + Installer string `json:"installer,omitempty"` + InstallerData string `json:"installer_data,omitempty"` } // Port represents a network port From c1cb592b33b9e296e066f27aee1bdd3f3821f68b Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Tue, 10 Aug 2021 16:08:50 -0500 Subject: [PATCH 2/7] allow custom_ipxe to handle both paths for booting an ipxe script Signed-off-by: Mike Mason --- cmd/boots/main.go | 1 - installers/{ipxe => custom_ipxe}/errors.go | 2 +- installers/custom_ipxe/ipxe_script_test.go | 2 +- installers/custom_ipxe/main.go | 82 ++++++- installers/custom_ipxe/main_test.go | 235 +++++++++++++++++++ installers/ipxe/main.go | 82 ------- installers/ipxe/main_test.go | 254 --------------------- 7 files changed, 314 insertions(+), 344 deletions(-) rename installers/{ipxe => custom_ipxe}/errors.go (83%) delete mode 100644 installers/ipxe/main.go delete mode 100644 installers/ipxe/main_test.go diff --git a/cmd/boots/main.go b/cmd/boots/main.go index 59350f55..a872c271 100644 --- a/cmd/boots/main.go +++ b/cmd/boots/main.go @@ -19,7 +19,6 @@ import ( _ "github.com/tinkerbell/boots/installers/coreos" _ "github.com/tinkerbell/boots/installers/custom_ipxe" - _ "github.com/tinkerbell/boots/installers/ipxe" _ "github.com/tinkerbell/boots/installers/nixos" _ "github.com/tinkerbell/boots/installers/osie" _ "github.com/tinkerbell/boots/installers/rancher" diff --git a/installers/ipxe/errors.go b/installers/custom_ipxe/errors.go similarity index 83% rename from installers/ipxe/errors.go rename to installers/custom_ipxe/errors.go index f9779e7d..e2cd57fa 100644 --- a/installers/ipxe/errors.go +++ b/installers/custom_ipxe/errors.go @@ -1,4 +1,4 @@ -package ipxe +package custom_ipxe import "errors" diff --git a/installers/custom_ipxe/ipxe_script_test.go b/installers/custom_ipxe/ipxe_script_test.go index b759f724..4d4a6238 100644 --- a/installers/custom_ipxe/ipxe_script_test.go +++ b/installers/custom_ipxe/ipxe_script_test.go @@ -30,7 +30,7 @@ func TestScript(t *testing.T) { s.Set("tinkerbell", "http://127.0.0.1") s.Set("ipxe_cloud_config", "packet") - bootScript(m.Job(), &s) + ipxeScript(m.Job(), &s) got := string(s.Bytes()) if script != got { t.Fatalf("%s bad iPXE script:\n%v", typ, diff.LineDiff(script, got)) diff --git a/installers/custom_ipxe/main.go b/installers/custom_ipxe/main.go index ab2eab43..829fd507 100644 --- a/installers/custom_ipxe/main.go +++ b/installers/custom_ipxe/main.go @@ -1,24 +1,96 @@ package custom_ipxe import ( + "encoding/json" "strings" + "github.com/packethost/pkg/log" + "github.com/tinkerbell/boots/installers" "github.com/tinkerbell/boots/ipxe" "github.com/tinkerbell/boots/job" ) func init() { - job.RegisterSlug("custom_ipxe", bootScript) + job.RegisterInstaller("ipxe", ipxeScript) + job.RegisterSlug("custom_ipxe", ipxeScript) } -func bootScript(j job.Job, s *ipxe.Script) { +func ipxeScript(j job.Job, s *ipxe.Script) { + logger := installers.Logger("ipxe") + if j.InstanceID() != "" { + logger = logger.With("instance.id", j.InstanceID()) + } + + var cfg *Config + var err error + + if j.OperatingSystem().Installer == "ipxe" { + cfg, err = ipxeConfigFromJob(j) + if err != nil { + s.Echo("Failed to decode installer data") + s.Shell() + logger.Error(err, "decoding installer data") + + return + } + } else { + cfg = &Config{} + + if strings.HasPrefix(j.UserData(), "#!ipxe") { + cfg.Script = j.UserData() + } else { + cfg.Chain = j.IPXEScriptURL() + } + } + + IpxeScriptFromConfig(logger, cfg, j, s) +} + +func IpxeScriptFromConfig(logger log.Logger, cfg *Config, j job.Job, s *ipxe.Script) { + if err := cfg.validate(); err != nil { + s.Echo("Invalid ipxe configuration") + s.Shell() + logger.Error(err, "validating ipxe config") + + return + } + s.PhoneHome("provisioning.104.01") s.Set("packet_facility", j.FacilityCode()) s.Set("packet_plan", j.PlanSlug()) - if strings.HasPrefix(j.UserData(), "#!ipxe") { - s.AppendString(strings.TrimPrefix(j.UserData(), "#!ipxe")) + if cfg.Chain != "" { + s.Chain(cfg.Chain) + } else if cfg.Script != "" { + s.AppendString(strings.TrimPrefix(cfg.Script, "#!ipxe")) } else { - s.Chain(j.IPXEScriptURL()) + s.Echo("Unknown ipxe config path") + s.Shell() } } + +func ipxeConfigFromJob(j job.Job) (*Config, error) { + data := j.OperatingSystem().InstallerData + + cfg := &Config{} + + err := json.NewDecoder(strings.NewReader(data)).Decode(&cfg) + if err != nil { + return nil, err + } + + return cfg, nil +} + +type Config struct { + Chain string `json:"chain,omitempty"` + Script string `json:"script,omitempty"` +} + +func (c *Config) validate() error { + if c.Chain == "" && c.Script == "" { + return ErrEmptyIpxeConfig + } + + return nil +} diff --git a/installers/custom_ipxe/main_test.go b/installers/custom_ipxe/main_test.go index afcd7bdf..65073cfc 100644 --- a/installers/custom_ipxe/main_test.go +++ b/installers/custom_ipxe/main_test.go @@ -2,12 +2,20 @@ package custom_ipxe import ( "os" + "regexp" "testing" l "github.com/packethost/pkg/log" + "github.com/stretchr/testify/require" + "github.com/tinkerbell/boots/installers" + "github.com/tinkerbell/boots/ipxe" "github.com/tinkerbell/boots/job" ) +var ( + testLogger l.Logger +) + func TestMain(m *testing.M) { os.Setenv("PACKET_ENV", "test") os.Setenv("PACKET_VERSION", "0") @@ -16,5 +24,232 @@ func TestMain(m *testing.M) { logger, _ := l.Init("github.com/tinkerbell/boots") job.Init(logger) + installers.Init(logger) + testLogger = logger os.Exit(m.Run()) } + +func TestIpxeScript(t *testing.T) { + var testCases = []struct { + name string + installerData string + want string + }{ + { + "invalid config", + "", + `#!ipxe + + echo Failed to decode installer data + shell + `, + }, + { + "valid config", + `{"chain": "http://url/path.ipxe"}`, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + chain --autofree http://url/path.ipxe + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + mockJob := job.NewMock(t, "test.slug", "test.facility") + script := ipxe.NewScript() + + mockJob.SetOSInstaller("ipxe") + mockJob.SetOSInstallerData(tc.installerData) + + ipxeScript(mockJob.Job(), script) + + assert.Equal(dedent(tc.want), string(script.Bytes())) + }) + } +} + +func TestIpxeScriptFromConfig(t *testing.T) { + var testCases = []struct { + name string + config *Config + want string + }{ + { + "invalid config", + &Config{}, + `#!ipxe + + echo Invalid ipxe configuration + shell + `, + }, + { + "valid chain", + &Config{Chain: "http://url/path.ipxe"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + chain --autofree http://url/path.ipxe + `, + }, + { + "valid script", + &Config{Script: "echo my test script"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + echo my test script + `, + }, + { + "valid script with header", + &Config{Script: "#!ipxe\necho my test script"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + + echo my test script + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + mockJob := job.NewMock(t, "test.slug", "test.facility") + script := ipxe.NewScript() + + IpxeScriptFromConfig(testLogger, tc.config, mockJob.Job(), script) + + assert.Equal(dedent(tc.want), string(script.Bytes())) + }) + } +} + +func TestIpxeConfigFromJob(t *testing.T) { + var testCases = []struct { + name string + installerData string + want *Config + expectError string + }{ + { + "valid chain", + `{"chain": "http://url/path.ipxe"}`, + &Config{Chain: "http://url/path.ipxe"}, + "", + }, + { + "valid script", + `{"script": "echo script"}`, + &Config{Script: "echo script"}, + "", + }, + { + "empty json error", + ``, + nil, + "EOF", + }, + { + "invalid json error", + `{"error"`, + nil, + "unexpected EOF", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + + mockJob := job.NewMock(t, "test.slug", "test.facility") + + mockJob.SetOSInstallerData(tc.installerData) + + cfg, err := ipxeConfigFromJob(mockJob.Job()) + + if tc.expectError == "" { + assert.Nil(err) + } else { + assert.EqualError(err, tc.expectError) + } + + assert.Equal(tc.want, cfg) + }) + } +} + +func TestConfigValidate(t *testing.T) { + var testCases = []struct { + name string + chain string + script string + want string + }{ + {"error when empty", "", "", "ipxe config URL or Script must be defined"}, + {"using chain", "http://chain.url/script.ipxe", "", ""}, + {"using script", "", "#!ipxe\necho ipxe script", ""}, + {"using both", "http://path", "ipxe script", ""}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert := require.New(t) + + cfg := &Config{ + Chain: tc.chain, + Script: tc.script, + } + + got := cfg.validate() + + if tc.want == "" { + assert.Nil(got) + } else { + assert.EqualError(got, tc.want) + } + }) + } +} + +var dedentRegexp = regexp.MustCompile(`(?m)^[^\S\n]+`) + +func dedent(s string) string { + return dedentRegexp.ReplaceAllString(s, "") +} diff --git a/installers/ipxe/main.go b/installers/ipxe/main.go deleted file mode 100644 index 62f8593f..00000000 --- a/installers/ipxe/main.go +++ /dev/null @@ -1,82 +0,0 @@ -package ipxe - -import ( - "encoding/json" - "strings" - - "github.com/packethost/pkg/log" - "github.com/tinkerbell/boots/installers" - "github.com/tinkerbell/boots/ipxe" - "github.com/tinkerbell/boots/job" -) - -func init() { - job.RegisterInstaller("ipxe", ipxeScript) -} - -func ipxeScript(j job.Job, s *ipxe.Script) { - logger := installers.Logger("ipxe") - if j.InstanceID() != "" { - logger = logger.With("instance.id", j.InstanceID()) - } - - cfg, err := ipxeConfigFromJob(j) - if err != nil { - s.Echo("Failed to decode installer data") - s.Shell() - logger.Error(err, "decoding installer data") - - return - } - - IpxeScriptFromConfig(logger, cfg, j, s) -} - -func IpxeScriptFromConfig(logger log.Logger, cfg *Config, j job.Job, s *ipxe.Script) { - if err := cfg.validate(); err != nil { - s.Echo("Invalid ipxe configuration") - s.Shell() - logger.Error(err, "validating ipxe config") - - return - } - - s.PhoneHome("provisioning.104.01") - s.Set("packet_facility", j.FacilityCode()) - s.Set("packet_plan", j.PlanSlug()) - - if cfg.Chain != "" { - s.Chain(cfg.Chain) - } else if cfg.Script != "" { - s.AppendString(strings.TrimPrefix(cfg.Script, "#!ipxe")) - } else { - s.Echo("Unknown ipxe config path") - s.Shell() - } -} - -func ipxeConfigFromJob(j job.Job) (*Config, error) { - data := j.OperatingSystem().InstallerData - - cfg := &Config{} - - err := json.NewDecoder(strings.NewReader(data)).Decode(&cfg) - if err != nil { - return nil, err - } - - return cfg, nil -} - -type Config struct { - Chain string `json:"chain,omitempty"` - Script string `json:"script,omitempty"` -} - -func (c *Config) validate() error { - if c.Chain == "" && c.Script == "" { - return ErrEmptyIpxeConfig - } - - return nil -} diff --git a/installers/ipxe/main_test.go b/installers/ipxe/main_test.go deleted file mode 100644 index 3e37a9e0..00000000 --- a/installers/ipxe/main_test.go +++ /dev/null @@ -1,254 +0,0 @@ -package ipxe - -import ( - "os" - "regexp" - "testing" - - l "github.com/packethost/pkg/log" - "github.com/stretchr/testify/require" - "github.com/tinkerbell/boots/installers" - "github.com/tinkerbell/boots/ipxe" - "github.com/tinkerbell/boots/job" -) - -var ( - testLogger l.Logger -) - -func TestMain(m *testing.M) { - os.Setenv("PACKET_ENV", "test") - os.Setenv("PACKET_VERSION", "0") - os.Setenv("ROLLBAR_DISABLE", "1") - os.Setenv("ROLLBAR_TOKEN", "1") - - logger, _ := l.Init("github.com/tinkerbell/boots") - job.Init(logger) - installers.Init(logger) - testLogger = logger - os.Exit(m.Run()) -} - -func TestIpxeScript(t *testing.T) { - var testCases = []struct { - name string - installerData string - want string - }{ - { - "invalid config", - "", - `#!ipxe - - echo Failed to decode installer data - shell - `, - }, - { - "valid config", - `{"chain": "http://url/path.ipxe"}`, - `#!ipxe - - - params - param body Device connected to DHCP system - param type provisioning.104.01 - imgfetch ${tinkerbell}/phone-home##params - imgfree - - set packet_facility test.facility - set packet_plan test.slug - chain --autofree http://url/path.ipxe - `, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert := require.New(t) - mockJob := job.NewMock(t, "test.slug", "test.facility") - script := ipxe.NewScript() - - mockJob.SetOSInstallerData(tc.installerData) - - ipxeScript(mockJob.Job(), script) - - assert.Equal(dedent(tc.want), string(script.Bytes())) - }) - } -} - -func TestIpxeScriptFromConfig(t *testing.T) { - var testCases = []struct { - name string - config *Config - want string - }{ - { - "invalid config", - &Config{}, - `#!ipxe - - echo Invalid ipxe configuration - shell - `, - }, - { - "valid chain", - &Config{Chain: "http://url/path.ipxe"}, - `#!ipxe - - - params - param body Device connected to DHCP system - param type provisioning.104.01 - imgfetch ${tinkerbell}/phone-home##params - imgfree - - set packet_facility test.facility - set packet_plan test.slug - chain --autofree http://url/path.ipxe - `, - }, - { - "valid script", - &Config{Script: "echo my test script"}, - `#!ipxe - - - params - param body Device connected to DHCP system - param type provisioning.104.01 - imgfetch ${tinkerbell}/phone-home##params - imgfree - - set packet_facility test.facility - set packet_plan test.slug - echo my test script - `, - }, - { - "valid script with header", - &Config{Script: "#!ipxe\necho my test script"}, - `#!ipxe - - - params - param body Device connected to DHCP system - param type provisioning.104.01 - imgfetch ${tinkerbell}/phone-home##params - imgfree - - set packet_facility test.facility - set packet_plan test.slug - - echo my test script - `, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert := require.New(t) - mockJob := job.NewMock(t, "test.slug", "test.facility") - script := ipxe.NewScript() - - IpxeScriptFromConfig(testLogger, tc.config, mockJob.Job(), script) - - assert.Equal(dedent(tc.want), string(script.Bytes())) - }) - } -} - -func TestIpxeConfigFromJob(t *testing.T) { - var testCases = []struct { - name string - installerData string - want *Config - expectError string - }{ - { - "valid chain", - `{"chain": "http://url/path.ipxe"}`, - &Config{Chain: "http://url/path.ipxe"}, - "", - }, - { - "valid script", - `{"script": "echo script"}`, - &Config{Script: "echo script"}, - "", - }, - { - "empty json error", - ``, - nil, - "EOF", - }, - { - "invalid json error", - `{"error"`, - nil, - "unexpected EOF", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert := require.New(t) - - mockJob := job.NewMock(t, "test.slug", "test.facility") - - mockJob.SetOSInstallerData(tc.installerData) - - cfg, err := ipxeConfigFromJob(mockJob.Job()) - - if tc.expectError == "" { - assert.Nil(err) - } else { - assert.EqualError(err, tc.expectError) - } - - assert.Equal(tc.want, cfg) - }) - } -} - -func TestConfigValidate(t *testing.T) { - var testCases = []struct { - name string - chain string - script string - want string - }{ - {"error when empty", "", "", "ipxe config URL or Script must be defined"}, - {"using chain", "http://chain.url/script.ipxe", "", ""}, - {"using script", "", "#!ipxe\necho ipxe script", ""}, - {"using both", "http://path", "ipxe script", ""}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert := require.New(t) - - cfg := &Config{ - Chain: tc.chain, - Script: tc.script, - } - - got := cfg.validate() - - if tc.want == "" { - assert.Nil(got) - } else { - assert.EqualError(got, tc.want) - } - }) - } -} - -var dedentRegexp = regexp.MustCompile(`(?m)^[^\S\n]+`) - -func dedent(s string) string { - return dedentRegexp.ReplaceAllString(s, "") -} From 6fd00dca115703acd3ae2b1df935cb447a927f83 Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Tue, 10 Aug 2021 21:10:15 -0500 Subject: [PATCH 3/7] adjustments from pr review Signed-off-by: Mike Mason --- installers/custom_ipxe/main.go | 49 +++++------------- installers/custom_ipxe/main_test.go | 79 +++++------------------------ job/mock.go | 2 +- packet/models.go | 20 +++++--- 4 files changed, 40 insertions(+), 110 deletions(-) diff --git a/installers/custom_ipxe/main.go b/installers/custom_ipxe/main.go index 829fd507..dac843f6 100644 --- a/installers/custom_ipxe/main.go +++ b/installers/custom_ipxe/main.go @@ -1,13 +1,13 @@ package custom_ipxe import ( - "encoding/json" "strings" "github.com/packethost/pkg/log" "github.com/tinkerbell/boots/installers" "github.com/tinkerbell/boots/ipxe" "github.com/tinkerbell/boots/job" + "github.com/tinkerbell/boots/packet" ) func init() { @@ -16,39 +16,37 @@ func init() { } func ipxeScript(j job.Job, s *ipxe.Script) { - logger := installers.Logger("ipxe") + logger := installers.Logger("custom_ipxe") if j.InstanceID() != "" { logger = logger.With("instance.id", j.InstanceID()) } - var cfg *Config + var cfg *packet.InstallerData var err error if j.OperatingSystem().Installer == "ipxe" { - cfg, err = ipxeConfigFromJob(j) - if err != nil { - s.Echo("Failed to decode installer data") + cfg = j.OperatingSystem().InstallerData + if cfg == nil { + s.Echo("Installer data not provided") s.Shell() - logger.Error(err, "decoding installer data") + logger.Error(err, "empty installer data") return } } else { - cfg = &Config{} - if strings.HasPrefix(j.UserData(), "#!ipxe") { - cfg.Script = j.UserData() + cfg = &packet.InstallerData{Script: j.UserData()} } else { - cfg.Chain = j.IPXEScriptURL() + cfg = &packet.InstallerData{Chain: j.IPXEScriptURL()} } } IpxeScriptFromConfig(logger, cfg, j, s) } -func IpxeScriptFromConfig(logger log.Logger, cfg *Config, j job.Job, s *ipxe.Script) { - if err := cfg.validate(); err != nil { - s.Echo("Invalid ipxe configuration") +func IpxeScriptFromConfig(logger log.Logger, cfg *packet.InstallerData, j job.Job, s *ipxe.Script) { + if err := validateConfig(cfg); err != nil { + s.Echo(err.Error()) s.Shell() logger.Error(err, "validating ipxe config") @@ -63,31 +61,10 @@ func IpxeScriptFromConfig(logger log.Logger, cfg *Config, j job.Job, s *ipxe.Scr s.Chain(cfg.Chain) } else if cfg.Script != "" { s.AppendString(strings.TrimPrefix(cfg.Script, "#!ipxe")) - } else { - s.Echo("Unknown ipxe config path") - s.Shell() } } -func ipxeConfigFromJob(j job.Job) (*Config, error) { - data := j.OperatingSystem().InstallerData - - cfg := &Config{} - - err := json.NewDecoder(strings.NewReader(data)).Decode(&cfg) - if err != nil { - return nil, err - } - - return cfg, nil -} - -type Config struct { - Chain string `json:"chain,omitempty"` - Script string `json:"script,omitempty"` -} - -func (c *Config) validate() error { +func validateConfig(c *packet.InstallerData) error { if c.Chain == "" && c.Script == "" { return ErrEmptyIpxeConfig } diff --git a/installers/custom_ipxe/main_test.go b/installers/custom_ipxe/main_test.go index 65073cfc..d7f9fe0a 100644 --- a/installers/custom_ipxe/main_test.go +++ b/installers/custom_ipxe/main_test.go @@ -10,6 +10,7 @@ import ( "github.com/tinkerbell/boots/installers" "github.com/tinkerbell/boots/ipxe" "github.com/tinkerbell/boots/job" + "github.com/tinkerbell/boots/packet" ) var ( @@ -32,21 +33,21 @@ func TestMain(m *testing.M) { func TestIpxeScript(t *testing.T) { var testCases = []struct { name string - installerData string + installerData *packet.InstallerData want string }{ { "invalid config", - "", + nil, `#!ipxe - echo Failed to decode installer data + echo Installer data not provided shell `, }, { "valid config", - `{"chain": "http://url/path.ipxe"}`, + &packet.InstallerData{Chain: "http://url/path.ipxe"}, `#!ipxe @@ -82,21 +83,21 @@ func TestIpxeScript(t *testing.T) { func TestIpxeScriptFromConfig(t *testing.T) { var testCases = []struct { name string - config *Config + config *packet.InstallerData want string }{ { "invalid config", - &Config{}, + &packet.InstallerData{}, `#!ipxe - echo Invalid ipxe configuration + echo ipxe config URL or Script must be defined shell `, }, { "valid chain", - &Config{Chain: "http://url/path.ipxe"}, + &packet.InstallerData{Chain: "http://url/path.ipxe"}, `#!ipxe @@ -113,7 +114,7 @@ func TestIpxeScriptFromConfig(t *testing.T) { }, { "valid script", - &Config{Script: "echo my test script"}, + &packet.InstallerData{Script: "echo my test script"}, `#!ipxe @@ -130,7 +131,7 @@ func TestIpxeScriptFromConfig(t *testing.T) { }, { "valid script with header", - &Config{Script: "#!ipxe\necho my test script"}, + &packet.InstallerData{Script: "#!ipxe\necho my test script"}, `#!ipxe @@ -161,60 +162,6 @@ func TestIpxeScriptFromConfig(t *testing.T) { } } -func TestIpxeConfigFromJob(t *testing.T) { - var testCases = []struct { - name string - installerData string - want *Config - expectError string - }{ - { - "valid chain", - `{"chain": "http://url/path.ipxe"}`, - &Config{Chain: "http://url/path.ipxe"}, - "", - }, - { - "valid script", - `{"script": "echo script"}`, - &Config{Script: "echo script"}, - "", - }, - { - "empty json error", - ``, - nil, - "EOF", - }, - { - "invalid json error", - `{"error"`, - nil, - "unexpected EOF", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - assert := require.New(t) - - mockJob := job.NewMock(t, "test.slug", "test.facility") - - mockJob.SetOSInstallerData(tc.installerData) - - cfg, err := ipxeConfigFromJob(mockJob.Job()) - - if tc.expectError == "" { - assert.Nil(err) - } else { - assert.EqualError(err, tc.expectError) - } - - assert.Equal(tc.want, cfg) - }) - } -} - func TestConfigValidate(t *testing.T) { var testCases = []struct { name string @@ -232,12 +179,12 @@ func TestConfigValidate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { assert := require.New(t) - cfg := &Config{ + cfg := &packet.InstallerData{ Chain: tc.chain, Script: tc.script, } - got := cfg.validate() + got := validateConfig(cfg) if tc.want == "" { assert.Nil(got) diff --git a/job/mock.go b/job/mock.go index 92e4cbe5..718f29ef 100644 --- a/job/mock.go +++ b/job/mock.go @@ -115,7 +115,7 @@ func (m *Mock) SetOSInstaller(installer string) { m.hardware.OperatingSystem().Installer = installer } -func (m *Mock) SetOSInstallerData(installerData string) { +func (m *Mock) SetOSInstallerData(installerData *packet.InstallerData) { m.hardware.OperatingSystem().InstallerData = installerData } diff --git a/packet/models.go b/packet/models.go index 5e32e039..a58c48b6 100644 --- a/packet/models.go +++ b/packet/models.go @@ -221,13 +221,19 @@ type IP struct { // OperatingSystem holds details for the operating system type OperatingSystem struct { - Slug string `json:"slug"` - Distro string `json:"distro"` - Version string `json:"version"` - ImageTag string `json:"image_tag"` - OsSlug string `json:"os_slug"` - Installer string `json:"installer,omitempty"` - InstallerData string `json:"installer_data,omitempty"` + Slug string `json:"slug"` + Distro string `json:"distro"` + Version string `json:"version"` + ImageTag string `json:"image_tag"` + OsSlug string `json:"os_slug"` + Installer string `json:"installer,omitempty"` + InstallerData *InstallerData `json:"installer_data,omitempty"` +} + +// InstallerData holds a number of fields that may be used by an installer. +type InstallerData struct { + Chain string `json:"chain,omitempty"` + Script string `json:"script,omitempty"` } // Port represents a network port From 943bbe5097995607ac4a2df2040667d0cbbbb88b Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Wed, 11 Aug 2021 10:11:33 -0500 Subject: [PATCH 4/7] function should not be exported Signed-off-by: Mike Mason --- installers/custom_ipxe/main.go | 4 ++-- installers/custom_ipxe/main_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/installers/custom_ipxe/main.go b/installers/custom_ipxe/main.go index dac843f6..26f9f24a 100644 --- a/installers/custom_ipxe/main.go +++ b/installers/custom_ipxe/main.go @@ -41,10 +41,10 @@ func ipxeScript(j job.Job, s *ipxe.Script) { } } - IpxeScriptFromConfig(logger, cfg, j, s) + ipxeScriptFromConfig(logger, cfg, j, s) } -func IpxeScriptFromConfig(logger log.Logger, cfg *packet.InstallerData, j job.Job, s *ipxe.Script) { +func ipxeScriptFromConfig(logger log.Logger, cfg *packet.InstallerData, j job.Job, s *ipxe.Script) { if err := validateConfig(cfg); err != nil { s.Echo(err.Error()) s.Shell() diff --git a/installers/custom_ipxe/main_test.go b/installers/custom_ipxe/main_test.go index d7f9fe0a..38f2bb42 100644 --- a/installers/custom_ipxe/main_test.go +++ b/installers/custom_ipxe/main_test.go @@ -155,7 +155,7 @@ func TestIpxeScriptFromConfig(t *testing.T) { mockJob := job.NewMock(t, "test.slug", "test.facility") script := ipxe.NewScript() - IpxeScriptFromConfig(testLogger, tc.config, mockJob.Job(), script) + ipxeScriptFromConfig(testLogger, tc.config, mockJob.Job(), script) assert.Equal(dedent(tc.want), string(script.Bytes())) }) From 6727da08e4d1d54bf8cdf1a1f177f5a02091f8fb Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Wed, 11 Aug 2021 10:39:37 -0500 Subject: [PATCH 5/7] match package name Signed-off-by: Mike Mason --- installers/custom_ipxe/main.go | 4 ++-- installers/custom_ipxe/main_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/installers/custom_ipxe/main.go b/installers/custom_ipxe/main.go index 26f9f24a..05bfde0b 100644 --- a/installers/custom_ipxe/main.go +++ b/installers/custom_ipxe/main.go @@ -11,7 +11,7 @@ import ( ) func init() { - job.RegisterInstaller("ipxe", ipxeScript) + job.RegisterInstaller("custom_ipxe", ipxeScript) job.RegisterSlug("custom_ipxe", ipxeScript) } @@ -24,7 +24,7 @@ func ipxeScript(j job.Job, s *ipxe.Script) { var cfg *packet.InstallerData var err error - if j.OperatingSystem().Installer == "ipxe" { + if j.OperatingSystem().Installer == "custom_ipxe" { cfg = j.OperatingSystem().InstallerData if cfg == nil { s.Echo("Installer data not provided") diff --git a/installers/custom_ipxe/main_test.go b/installers/custom_ipxe/main_test.go index 38f2bb42..bc1cabc5 100644 --- a/installers/custom_ipxe/main_test.go +++ b/installers/custom_ipxe/main_test.go @@ -70,7 +70,7 @@ func TestIpxeScript(t *testing.T) { mockJob := job.NewMock(t, "test.slug", "test.facility") script := ipxe.NewScript() - mockJob.SetOSInstaller("ipxe") + mockJob.SetOSInstaller("custom_ipxe") mockJob.SetOSInstallerData(tc.installerData) ipxeScript(mockJob.Job(), script) From b910cdf043deb2c0436ae575cbabd90b79649743 Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Wed, 11 Aug 2021 16:32:00 -0500 Subject: [PATCH 6/7] use job logger instead of creating a new one Signed-off-by: Mike Mason --- installers/custom_ipxe/main.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/installers/custom_ipxe/main.go b/installers/custom_ipxe/main.go index 05bfde0b..72d2d61b 100644 --- a/installers/custom_ipxe/main.go +++ b/installers/custom_ipxe/main.go @@ -4,7 +4,6 @@ import ( "strings" "github.com/packethost/pkg/log" - "github.com/tinkerbell/boots/installers" "github.com/tinkerbell/boots/ipxe" "github.com/tinkerbell/boots/job" "github.com/tinkerbell/boots/packet" @@ -16,20 +15,16 @@ func init() { } func ipxeScript(j job.Job, s *ipxe.Script) { - logger := installers.Logger("custom_ipxe") - if j.InstanceID() != "" { - logger = logger.With("instance.id", j.InstanceID()) - } + logger := j.Logger.With("installer", "custom_ipxe") var cfg *packet.InstallerData - var err error if j.OperatingSystem().Installer == "custom_ipxe" { cfg = j.OperatingSystem().InstallerData if cfg == nil { s.Echo("Installer data not provided") s.Shell() - logger.Error(err, "empty installer data") + logger.Error(ErrEmptyIpxeConfig, "installer data not provided") return } From 1f966ce09c757259087289d6dece9ac11a516e73 Mon Sep 17 00:00:00 2001 From: Mike Mason Date: Wed, 11 Aug 2021 16:32:55 -0500 Subject: [PATCH 7/7] handle instance path with no data gracefully Signed-off-by: Mike Mason --- installers/custom_ipxe/main.go | 14 +++-- installers/custom_ipxe/main_test.go | 79 +++++++++++++++++++++++++++-- job/mock.go | 4 ++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/installers/custom_ipxe/main.go b/installers/custom_ipxe/main.go index 72d2d61b..d5cd40f7 100644 --- a/installers/custom_ipxe/main.go +++ b/installers/custom_ipxe/main.go @@ -28,12 +28,16 @@ func ipxeScript(j job.Job, s *ipxe.Script) { return } + } else if strings.HasPrefix(j.UserData(), "#!ipxe") { + cfg = &packet.InstallerData{Script: j.UserData()} + } else if j.IPXEScriptURL() != "" { + cfg = &packet.InstallerData{Chain: j.IPXEScriptURL()} } else { - if strings.HasPrefix(j.UserData(), "#!ipxe") { - cfg = &packet.InstallerData{Script: j.UserData()} - } else { - cfg = &packet.InstallerData{Chain: j.IPXEScriptURL()} - } + s.Echo("Unknown ipxe configuration") + s.Shell() + logger.Error(ErrEmptyIpxeConfig, "unknown ipxe configuration") + + return } ipxeScriptFromConfig(logger, cfg, j, s) diff --git a/installers/custom_ipxe/main_test.go b/installers/custom_ipxe/main_test.go index bc1cabc5..7369aa6c 100644 --- a/installers/custom_ipxe/main_test.go +++ b/installers/custom_ipxe/main_test.go @@ -33,11 +33,13 @@ func TestMain(m *testing.M) { func TestIpxeScript(t *testing.T) { var testCases = []struct { name string + installer string installerData *packet.InstallerData want string }{ { - "invalid config", + "installer: invalid config", + "custom_ipxe", nil, `#!ipxe @@ -47,6 +49,53 @@ func TestIpxeScript(t *testing.T) { }, { "valid config", + "custom_ipxe", + &packet.InstallerData{Chain: "http://url/path.ipxe"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + chain --autofree http://url/path.ipxe + `, + }, + { + "installer: valid config", + "", + &packet.InstallerData{Chain: "http://url/path.ipxe"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + chain --autofree http://url/path.ipxe + `, + }, + { + "instance: no config", + "", + nil, + `#!ipxe + + echo Unknown ipxe configuration + shell + `, + }, + { + "instance: ipxe script url", + "", &packet.InstallerData{Chain: "http://url/path.ipxe"}, `#!ipxe @@ -62,6 +111,25 @@ func TestIpxeScript(t *testing.T) { chain --autofree http://url/path.ipxe `, }, + { + "instance: userdata script", + "", + &packet.InstallerData{Script: "#!ipxe\necho userdata script"}, + `#!ipxe + + + params + param body Device connected to DHCP system + param type provisioning.104.01 + imgfetch ${tinkerbell}/phone-home##params + imgfree + + set packet_facility test.facility + set packet_plan test.slug + + echo userdata script + `, + }, } for _, tc := range testCases { @@ -70,8 +138,13 @@ func TestIpxeScript(t *testing.T) { mockJob := job.NewMock(t, "test.slug", "test.facility") script := ipxe.NewScript() - mockJob.SetOSInstaller("custom_ipxe") - mockJob.SetOSInstallerData(tc.installerData) + if tc.installer == "custom_ipxe" { + mockJob.SetOSInstaller("custom_ipxe") + mockJob.SetOSInstallerData(tc.installerData) + } else if tc.installerData != nil { + mockJob.SetIPXEScriptURL(tc.installerData.Chain) + mockJob.SetUserData(tc.installerData.Script) + } ipxeScript(mockJob.Job(), script) diff --git a/job/mock.go b/job/mock.go index 718f29ef..7650477a 100644 --- a/job/mock.go +++ b/job/mock.go @@ -80,6 +80,10 @@ func (m *Mock) SetIPXEScriptURL(url string) { m.instance.IPXEScriptURL = url } +func (m *Mock) SetUserData(userdata string) { + m.instance.UserData = userdata +} + func (m *Mock) SetMAC(mac string) { _m, err := net.ParseMAC(mac) if err != nil {