From e7ac79043e6f402350ceeb57fb80a5c8992a7d56 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 21 Nov 2024 17:11:07 -0700 Subject: [PATCH 1/6] Add static ipam to kernel parameters: This adds static IPAM to kernel parameters when patching an ISO. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 79 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 9 deletions(-) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 5014eb64..0999f47c 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "net/http/httputil" + "net/netip" "net/url" "path" "path/filepath" @@ -112,7 +113,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } - f, err := getFacility(req.Context(), ha, h.Backend) + f, d, err := getFacility(req.Context(), ha, h.Backend) if err != nil { log.Info("unable to get the hardware object", "error", err, "mac", ha) if apierrors.IsNotFound(err) { @@ -240,7 +241,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { dup := make([]byte, len(b)) copy(dup, b) copy(dup[i:], magicStringPadding) - copy(dup[i:], []byte(h.constructPatch(consoles, ha.String()))) + copy(dup[i:], []byte(h.constructPatch(consoles, ha.String(), d))) b = dup } @@ -250,13 +251,14 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { return resp, nil } -func (h *Handler) constructPatch(console, mac string) string { +func (h *Handler) constructPatch(console, mac string, d *data.DHCP) string { syslogHost := fmt.Sprintf("syslog_host=%s", h.Syslog) grpcAuthority := fmt.Sprintf("grpc_authority=%s", h.TinkServerGRPCAddr) tinkerbellTLS := fmt.Sprintf("tinkerbell_tls=%v", h.TinkServerTLS) workerID := fmt.Sprintf("worker_id=%s", mac) + ipam := parseIPAM(d) - return strings.Join([]string{strings.Join(h.ExtraKernelParams, " "), console, syslogHost, grpcAuthority, tinkerbellTLS, workerID}, " ") + return strings.Join([]string{strings.Join(h.ExtraKernelParams, " "), console, syslogHost, grpcAuthority, tinkerbellTLS, workerID, ipam}, " ") } func getMAC(urlPath string) (net.HardwareAddr, error) { @@ -269,18 +271,18 @@ func getMAC(urlPath string) (net.HardwareAddr, error) { return hw, nil } -func getFacility(ctx context.Context, mac net.HardwareAddr, br BackendReader) (string, error) { +func getFacility(ctx context.Context, mac net.HardwareAddr, br BackendReader) (string, *data.DHCP, error) { if br == nil { - return "", errors.New("backend is nil") + return "", nil, errors.New("backend is nil") } // TODO(jacobweinstock): Pass DHCP info to kernel cmdline parameters for static IP assignment. - _, n, err := br.GetByMac(ctx, mac) + d, n, err := br.GetByMac(ctx, mac) if err != nil { - return "", err + return "", nil, err } - return n.Facility, nil + return n.Facility, d, nil } func randomPercentage(precision int64) float64 { @@ -291,3 +293,62 @@ func randomPercentage(precision int64) float64 { return float64(random.Int64()) / float64(precision) } + +func parseIPAM(d *data.DHCP) string { + // return format is ipam=:::::::: + ipam := make([]string, 9) + ipam[0] = func () string { + m := d.MACAddress.String() + + return strings.ReplaceAll(m, ":", "-") + }() + ipam[1] = func() string { + if d.VLANID != "" { + return d.VLANID + } + return "" + }() + ipam[2] = func() string { + if d.IPAddress.Compare(netip.Addr{}) != 0 { + return d.IPAddress.String() + } + return "" + }() + ipam[3] = func() string { + if d.SubnetMask != nil { + return net.IP(d.SubnetMask).String() + } + return "" + }() + ipam[4] = func() string { + if d.DefaultGateway.Compare(netip.Addr{}) != 0 { + return d.DefaultGateway.String() + } + return "" + }() + ipam[5] = d.Hostname + ipam[6] = func() string { + var nameservers []string + for _, e := range d.NameServers { + nameservers = append(nameservers, e.String()) + } + if len(nameservers) > 0 { + return strings.Join(nameservers, ",") + } + + return "" + }() + ipam[8] = func() string { + var ntp []string + for _, e := range d.NTPServers { + ntp = append(ntp, e.String()) + } + if len(ntp) > 0 { + return strings.Join(ntp, ",") + } + + return "" + }() + + return fmt.Sprintf("ipam=%s", strings.Join(ipam, ":")) +} From ea85fe021254fbbbbf89d41ca2f290271e8f1cba Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 21 Nov 2024 18:31:19 -0700 Subject: [PATCH 2/6] Make ISO static IPAM optional: CLI Flag to enable/disable ISO static IPAM. Signed-off-by: Jacob Weinstock --- cmd/smee/flag.go | 7 +++--- cmd/smee/flag_test.go | 7 +++--- cmd/smee/main.go | 8 ++++--- internal/iso/iso.go | 54 ++++++++++++++++++++++++------------------- 4 files changed, 43 insertions(+), 33 deletions(-) diff --git a/cmd/smee/flag.go b/cmd/smee/flag.go index 61152ea9..670292cb 100644 --- a/cmd/smee/flag.go +++ b/cmd/smee/flag.go @@ -154,9 +154,10 @@ func otelFlags(c *config, fs *flag.FlagSet) { } func isoFlags(c *config, fs *flag.FlagSet) { - fs.BoolVar(&c.iso.enabled, "iso-enabled", false, "[iso] enable serving Hook as an iso") - fs.StringVar(&c.iso.url, "iso-url", "", "[iso] the url for source iso before binary patching") - fs.StringVar(&c.iso.magicString, "iso-magic-string", "", "[iso] the string pattern to match for in the source iso, if not set the default from HookOS is used") + fs.BoolVar(&c.iso.enabled, "iso-enabled", false, "[iso] enable patching an OSIE ISO") + fs.StringVar(&c.iso.url, "iso-url", "", "[iso] an ISO source URL target for patching") + fs.StringVar(&c.iso.magicString, "iso-magic-string", "", "[iso] the string pattern to match for in the source ISO, defaults to the one defined in HookOS") + fs.BoolVar(&c.iso.staticIPAMEnabled, "iso-static-ipam-enabled", false, "[iso] enable static IPAM for HookOS") } func setFlags(c *config, fs *flag.FlagSet) { diff --git a/cmd/smee/flag_test.go b/cmd/smee/flag_test.go index d6867e2f..1d59492f 100644 --- a/cmd/smee/flag_test.go +++ b/cmd/smee/flag_test.go @@ -153,9 +153,10 @@ FLAGS -tink-server-insecure-tls [http] use insecure TLS for Tink server (default "false") -tink-server-tls [http] use TLS for Tink server (default "false") -trusted-proxies [http] comma separated list of trusted proxies in CIDR notation - -iso-enabled [iso] enable serving Hook as an iso (default "false") - -iso-magic-string [iso] the string pattern to match for in the source iso, if not set the default from HookOS is used - -iso-url [iso] the url for source iso before binary patching + -iso-enabled [iso] enable patching an OSIE ISO (default "false") + -iso-magic-string [iso] the string pattern to match for in the source ISO, defaults to the one defined in HookOS + -iso-static-ipam-enabled [iso] enable static IPAM for HookOS (default "false") + -iso-url [iso] an ISO source URL target for patching -otel-endpoint [otel] OpenTelemetry collector endpoint -otel-insecure [otel] OpenTelemetry collector insecure (default "true") -syslog-addr [syslog] local IP to listen on for Syslog messages (default "%[1]v") diff --git a/cmd/smee/main.go b/cmd/smee/main.go index 94426659..6689e308 100644 --- a/cmd/smee/main.go +++ b/cmd/smee/main.go @@ -142,9 +142,10 @@ type otelConfig struct { } type isoConfig struct { - enabled bool - url string - magicString string + enabled bool + url string + magicString string + staticIPAMEnabled bool } func main() { @@ -262,6 +263,7 @@ func main() { Syslog: cfg.dhcp.syslogIP, TinkServerTLS: cfg.ipxeHTTPScript.tinkServerUseTLS, TinkServerGRPCAddr: cfg.ipxeHTTPScript.tinkServer, + StaticIPAMEnabled: cfg.iso.staticIPAMEnabled, MagicString: func() string { if cfg.iso.magicString == "" { return magicString diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 0999f47c..0ad611b7 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -35,21 +35,6 @@ type BackendReader interface { GetByMac(context.Context, net.HardwareAddr) (*data.DHCP, *data.Netboot, error) } -// HandlerFunc returns a reverse proxy HTTP handler function that performs ISO patching. -func (h *Handler) HandlerFunc() (http.HandlerFunc, error) { - target, err := url.Parse(h.SourceISO) - if err != nil { - return nil, err - } - h.parsedURL = target - proxy := httputil.NewSingleHostReverseProxy(target) - - proxy.Transport = h - proxy.FlushInterval = -1 - - return proxy.ServeHTTP, nil -} - // Handler is a struct that contains the necessary fields to patch an ISO file with // relevant information for the Tink worker. type Handler struct { @@ -67,11 +52,27 @@ type Handler struct { Syslog string TinkServerTLS bool TinkServerGRPCAddr string + StaticIPAMEnabled bool // parsedURL derives a url.URL from the SourceISO field. // It needed for validation of SourceISO and easier modification. parsedURL *url.URL } +// HandlerFunc returns a reverse proxy HTTP handler function that performs ISO patching. +func (h *Handler) HandlerFunc() (http.HandlerFunc, error) { + target, err := url.Parse(h.SourceISO) + if err != nil { + return nil, err + } + h.parsedURL = target + proxy := httputil.NewSingleHostReverseProxy(target) + + proxy.Transport = h + proxy.FlushInterval = -1 + + return proxy.ServeHTTP, nil +} + // RoundTrip is a method on the Handler struct that implements the http.RoundTripper interface. // This method is called by the httputil.NewSingleHostReverseProxy to handle the incoming request. // The method is responsible for validating the incoming request, reading the source ISO, patching the ISO. @@ -113,7 +114,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } - f, d, err := getFacility(req.Context(), ha, h.Backend) + fac, dhcpData, err := h.getFacility(req.Context(), ha, h.Backend) if err != nil { log.Info("unable to get the hardware object", "error", err, "mac", ha) if apierrors.IsNotFound(err) { @@ -223,10 +224,10 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { // historically the facility is used as a way to define consoles on a per Hardware basis. var consoles string switch { - case f != "" && strings.Contains(f, "console="): - consoles = fmt.Sprintf("facility=%s", f) - case f != "": - consoles = fmt.Sprintf("facility=%s %s", f, defaultConsoles) + case fac != "" && strings.Contains(fac, "console="): + consoles = fmt.Sprintf("facility=%s", fac) + case fac != "": + consoles = fmt.Sprintf("facility=%s %s", fac, defaultConsoles) default: consoles = defaultConsoles } @@ -241,7 +242,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) { dup := make([]byte, len(b)) copy(dup, b) copy(dup[i:], magicStringPadding) - copy(dup[i:], []byte(h.constructPatch(consoles, ha.String(), d))) + copy(dup[i:], []byte(h.constructPatch(consoles, ha.String(), dhcpData))) b = dup } @@ -271,16 +272,18 @@ func getMAC(urlPath string) (net.HardwareAddr, error) { return hw, nil } -func getFacility(ctx context.Context, mac net.HardwareAddr, br BackendReader) (string, *data.DHCP, error) { +func (h *Handler) getFacility(ctx context.Context, mac net.HardwareAddr, br BackendReader) (string, *data.DHCP, error) { if br == nil { return "", nil, errors.New("backend is nil") } - // TODO(jacobweinstock): Pass DHCP info to kernel cmdline parameters for static IP assignment. d, n, err := br.GetByMac(ctx, mac) if err != nil { return "", nil, err } + if !h.StaticIPAMEnabled { + d = nil + } return n.Facility, d, nil } @@ -295,9 +298,12 @@ func randomPercentage(precision int64) float64 { } func parseIPAM(d *data.DHCP) string { + if d == nil { + return "" + } // return format is ipam=:::::::: ipam := make([]string, 9) - ipam[0] = func () string { + ipam[0] = func() string { m := d.MACAddress.String() return strings.ReplaceAll(m, ":", "-") From 60225de2068211a1d9cb187ef620fdac1cf894d3 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Fri, 22 Nov 2024 18:23:18 -0700 Subject: [PATCH 3/6] Add vlan_id and hw_addr to patched cmdline: These are needed for proper VLAN interface creation. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 9 ++++++++- internal/iso/iso_test.go | 14 ++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 0ad611b7..62d91861 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -257,9 +257,16 @@ func (h *Handler) constructPatch(console, mac string, d *data.DHCP) string { grpcAuthority := fmt.Sprintf("grpc_authority=%s", h.TinkServerGRPCAddr) tinkerbellTLS := fmt.Sprintf("tinkerbell_tls=%v", h.TinkServerTLS) workerID := fmt.Sprintf("worker_id=%s", mac) + vlanID := func() string { + if d != nil { + return fmt.Sprintf("vlan_id=%s", d.VLANID) + } + return "" + }() + hwAddr := fmt.Sprintf("hw_addr=%s", mac) ipam := parseIPAM(d) - return strings.Join([]string{strings.Join(h.ExtraKernelParams, " "), console, syslogHost, grpcAuthority, tinkerbellTLS, workerID, ipam}, " ") + return strings.Join([]string{strings.Join(h.ExtraKernelParams, " "), console, vlanID, hwAddr, syslogHost, grpcAuthority, tinkerbellTLS, workerID, ipam}, " ") } func getMAC(urlPath string) (net.HardwareAddr, error) { diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 66e9f489..0d99aeb1 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -110,13 +110,13 @@ func TestPatching(t *testing.T) { // patch the ISO file // mount the ISO file and check if the magic string was patched + // If anything changes here the space padding will be different. Be sure to update it accordingly. wantGrubCfg := `set timeout=0 set gfxpayload=text menuentry 'LinuxKit ISO Image' { - linuxefi /kernel facility=test console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1 syslog_host=127.0.0.1:514 grpc_authority=127.0.0.1:42113 tinkerbell_tls=false worker_id=de:ed:be:ef:fe:ed text + linuxefi /kernel facility=test console=ttyAMA0 console=ttyS0 console=tty0 console=tty1 console=ttyS1 hw_addr=de:ed:be:ef:fe:ed syslog_host=127.0.0.1:514 grpc_authority=127.0.0.1:42113 tinkerbell_tls=false worker_id=de:ed:be:ef:fe:ed text initrdefi /initrd.img -} -` +}` // This expects that testdata/output.iso exists. Run the TestCreateISO test to create it. // serve it with a http server @@ -141,6 +141,8 @@ menuentry 'LinuxKit ISO Image' { parsedURL: parsedURL, MagicString: magicString, } + // for debugging enable a logger + // h.Logger = logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true})) rurl := hs.URL + "/iso/de:ed:be:ef:fe:ed/output.iso" purl, _ := url.Parse(rurl) @@ -164,14 +166,14 @@ menuentry 'LinuxKit ISO Image' { t.Fatal(err) } - idx := bytes.Index(isoContents, []byte(wantGrubCfg)) + idx := bytes.Index(isoContents, []byte(`set timeout=0`)) if idx == -1 { - t.Fatalf("could not find grub.cfg in the ISO") + t.Fatalf("could not find the expected grub.cfg contents in the ISO") } contents := isoContents[idx : idx+len(wantGrubCfg)] if diff := cmp.Diff(wantGrubCfg, string(contents)); diff != "" { - t.Fatalf("unexpected grub.cfg file: %s", diff) + t.Fatalf("patched grub.cfg contents don't match expected: %v", diff) } } From c64185bb3d2347383d68fffda4fae631465459e3 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Fri, 22 Nov 2024 18:25:38 -0700 Subject: [PATCH 4/6] Update readme with new ISO flag and wording: Signed-off-by: Jacob Weinstock --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 373905fc..1aa7e0a2 100644 --- a/README.md +++ b/README.md @@ -149,9 +149,9 @@ FLAGS -tink-server [http] IP:Port for the Tink server -tink-server-tls [http] use TLS for Tink server (default "false") -trusted-proxies [http] comma separated list of trusted proxies in CIDR notation - -iso-enabled [iso] enable serving Hook as an iso (default "false") - -iso-magic-string [iso] the string pattern to match for in the source iso, if not set the default from HookOS is used - -iso-url [iso] the url for source iso before binary patching + -iso-enabled [iso] enable patching an OSIE ISO (default "false") + -iso-magic-string [iso] the string pattern to match for in the source ISO, defaults to the one defined in HookOS + -iso-static-ipam-enabled [iso] enable static IPAM for HookOS (default "false") -otel-endpoint [otel] OpenTelemetry collector endpoint -otel-insecure [otel] OpenTelemetry collector insecure (default "true") -syslog-addr [syslog] local IP to listen on for Syslog messages (default "172.17.0.3") From b7f62517b2068aeb8bb996d64c4547777973370f Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Fri, 22 Nov 2024 18:44:12 -0700 Subject: [PATCH 5/6] Reorganize where the gating of static IPAM occurs: This moves the gating to the constructing of the kernel parameters and makes it explicit to whether ipam is included or not. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 62d91861..98b3a769 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -258,15 +258,18 @@ func (h *Handler) constructPatch(console, mac string, d *data.DHCP) string { tinkerbellTLS := fmt.Sprintf("tinkerbell_tls=%v", h.TinkServerTLS) workerID := fmt.Sprintf("worker_id=%s", mac) vlanID := func() string { - if d != nil { + if d != nil && d.VLANID != "" { return fmt.Sprintf("vlan_id=%s", d.VLANID) } return "" }() hwAddr := fmt.Sprintf("hw_addr=%s", mac) - ipam := parseIPAM(d) + all := []string{strings.Join(h.ExtraKernelParams, " "), console, vlanID, hwAddr, syslogHost, grpcAuthority, tinkerbellTLS, workerID} + if h.StaticIPAMEnabled { + all = append(all, parseIPAM(d)) + } - return strings.Join([]string{strings.Join(h.ExtraKernelParams, " "), console, vlanID, hwAddr, syslogHost, grpcAuthority, tinkerbellTLS, workerID, ipam}, " ") + return strings.Join(all, " ") } func getMAC(urlPath string) (net.HardwareAddr, error) { @@ -288,9 +291,6 @@ func (h *Handler) getFacility(ctx context.Context, mac net.HardwareAddr, br Back if err != nil { return "", nil, err } - if !h.StaticIPAMEnabled { - d = nil - } return n.Facility, d, nil } From c4fa09d0f35bca76bb4b8a4e6974fc77dc160832 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Sat, 23 Nov 2024 08:02:48 -0700 Subject: [PATCH 6/6] Add search domains to parseIPAM, add test: Search domains was missing from being parsed. Some basic tests were added. Signed-off-by: Jacob Weinstock --- internal/iso/iso.go | 7 +++++++ internal/iso/iso_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 98b3a769..c122e948 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -351,6 +351,13 @@ func parseIPAM(d *data.DHCP) string { return "" }() + ipam[7] = func() string { + if len(d.DomainSearch) > 0 { + return strings.Join(d.DomainSearch, ",") + } + + return "" + }() ipam[8] = func() string { var ntp []string for _, e := range d.NTPServers { diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index 0d99aeb1..f5c7f4c3 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/netip" "net/url" "os" "testing" @@ -194,3 +195,39 @@ func (m *mockBackend) GetByIP(context.Context, net.IP) (*data.DHCP, *data.Netboo } return d, n, nil } + +func TestParseIPAM(t *testing.T) { + tests := map[string]struct { + input *data.DHCP + want string + }{ + "empty": {}, + "only MAC": { + input: &data.DHCP{MACAddress: net.HardwareAddr{0xde, 0xed, 0xbe, 0xef, 0xfe, 0xed}}, + want: "ipam=de-ed-be-ef-fe-ed::::::::", + }, + "everything": { + input: &data.DHCP{ + MACAddress: net.HardwareAddr{0xde, 0xed, 0xbe, 0xef, 0xfe, 0xed}, + IPAddress: netip.AddrFrom4([4]byte{127, 0, 0, 1}), + SubnetMask: net.IPv4Mask(255, 255, 255, 0), + DefaultGateway: netip.AddrFrom4([4]byte{127, 0, 0, 2}), + NameServers: []net.IP{{1, 1, 1, 1}, {4, 4, 4, 4}}, + Hostname: "myhost", + NTPServers: []net.IP{{129, 6, 15, 28}, {129, 6, 15, 29}}, + DomainSearch: []string{"example.com", "example.org"}, + VLANID: "400", + }, + want: "ipam=de-ed-be-ef-fe-ed:400:127.0.0.1:255.255.255.0:127.0.0.2:myhost:1.1.1.1,4.4.4.4:example.com,example.org:129.6.15.28,129.6.15.29", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got := parseIPAM(tt.input) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Fatalf("diff: %v", diff) + } + }) + } +}