Merge pull request tinkerbell#558 from jacobweinstock/iso-static-ipam

Add static ipam to kernel parameters for ISO patching:

## Description

<!--- Please describe what this PR is going to change -->
This adds optional static IPAM data to kernel parameters when patching an ISO.

## Why is this needed

<!--- Link to issue you have raised -->

Fixes: #

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->


## How are existing users impacted? What migration steps/scripts do we need?

<!--- Fixes a bug, unblocks installation, removes a component of the stack etc -->
<!--- Requires a DB migration script, etc. -->


## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade

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")

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) {

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")

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

internal/iso/iso.go

@@ -11,6 +11,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httputil"
+	"net/netip"
 	"net/url"
 	"path"
 	"path/filepath"
@@ -34,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 {
@@ -66,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.
@@ -112,7 +114,7 @@ func (h *Handler) RoundTrip(req *http.Request) (*http.Response, error) {
 		}, nil
 	}
 
-	f, 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) {
@@ -222,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
 	}
@@ -240,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())))
+		copy(dup[i:], []byte(h.constructPatch(consoles, ha.String(), dhcpData)))
 		b = dup
 	}
 
@@ -250,13 +252,24 @@ 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)
+	vlanID := func() string {
+		if d != nil && d.VLANID != "" {
+			return fmt.Sprintf("vlan_id=%s", d.VLANID)
+		}
+		return ""
+	}()
+	hwAddr := fmt.Sprintf("hw_addr=%s", mac)
+	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, syslogHost, grpcAuthority, tinkerbellTLS, workerID}, " ")
+	return strings.Join(all, " ")
 }
 
 func getMAC(urlPath string) (net.HardwareAddr, error) {
@@ -269,18 +282,17 @@ func getMAC(urlPath string) (net.HardwareAddr, error) {
 	return hw, nil
 }
 
-func getFacility(ctx context.Context, mac net.HardwareAddr, br BackendReader) (string, error) {
+func (h *Handler) 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 +303,72 @@ func randomPercentage(precision int64) float64 {
 
 	return float64(random.Int64()) / float64(precision)
 }
+
+func parseIPAM(d *data.DHCP) string {
+	if d == nil {
+		return ""
+	}
+	// return format is ipam=<mac-address>:<vlan-id>:<ip-address>:<netmask>:<gateway>:<hostname>:<dns>:<search-domains>:<ntp>
+	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[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 {
+			ntp = append(ntp, e.String())
+		}
+		if len(ntp) > 0 {
+			return strings.Join(ntp, ",")
+		}
+
+		return ""
+	}()
+
+	return fmt.Sprintf("ipam=%s", strings.Join(ipam, ":"))
+}

internal/iso/iso_test.go

@@ -8,6 +8,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/netip"
 	"net/url"
 	"os"
 	"testing"
@@ -110,13 +111,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 +142,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 +167,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)
 	}
 }
 
@@ -192,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)
+			}
+		})
+	}
+}