Add v1alpha2 Hardware validating admission webhook

api/v1alpha2/hardware.go

@@ -41,19 +41,31 @@ type NetworkInterfaces map[MAC]NetworkInterface
 
 // NetworkInterface is the desired configuration for a particular network interface.
 type NetworkInterface struct {
-	// DHCP is the basic network information for serving DHCP requests.
-	DHCP DHCP `json:"dhcp,omitempty"`
+	// DHCP is the basic network information for serving DHCP requests. Required when DisbaleDHCP
+	// is false.
+	// +optional
+	DHCP *DHCP `json:"dhcp,omitempty"`
 
 	// DisableDHCP disables DHCP for this interface. Implies DisableNetboot.
 	// +kubebuilder:default=false
 	DisableDHCP bool `json:"disableDhcp,omitempty"`
 
 	// DisableNetboot disables netbooting for this interface. The interface will still receive
-	// network information speified on by DHCP.
+	// network information specified by DHCP.
 	// +kubebuilder:default=false
 	DisableNetboot bool `json:"disableNetboot,omitempty"`
 }
 
+// IsDHCPEnabled checks if DHCP is enabled for ni.
+func (ni NetworkInterface) IsDHCPEnabled() bool {
+	return !ni.DisableDHCP
+}
+
+// IsNetbootEnabled checks if Netboot is enabled for ni.
+func (ni NetworkInterface) IsNetbootEnabled() bool {
+	return !ni.DisableNetboot
+}
+
 // DHCP describes basic network configuration to be served in DHCP OFFER responses. It can be
 // considered a DHCP reservation.
 type DHCP struct {
@@ -165,6 +177,26 @@ type Hardware struct {
 	Spec HardwareSpec `json:"spec,omitempty"`
 }
 
+// GetMACs retrieves all MACs associated with h.
+func (h *Hardware) GetMACs() []string {
+	var macs []string
+	for m := range h.Spec.NetworkInterfaces {
+		macs = append(macs, string(m))
+	}
+	return macs
+}
+
+// GetIPs retrieves all IP addresses. It does not consider the DisableDHCP flag.
+func (h *Hardware) GetIPs() []string {
+	var ips []string
+	for _, ni := range h.Spec.NetworkInterfaces {
+		if ni.DHCP != nil {
+			ips = append(ips, ni.DHCP.IP)
+		}
+	}
+	return ips
+}
+
 // +kubebuilder:object:root=true
 
 type HardwareList struct {

config/crd/bases/tinkerbell.org_hardware.yaml

@@ -421,7 +421,7 @@ spec:
                     description: NetworkInterface is the desired configuration for a particular network interface.
                     properties:
                       dhcp:
-                        description: DHCP is the basic network information for serving DHCP requests.
+                        description: DHCP is the basic network information for serving DHCP requests. Requires when DisbaleDHCP is false.
                         properties:
                           gateway:
                             description: Gateway is the default gateway address to serve.
@@ -469,7 +469,7 @@ spec:
                         type: boolean
                       disableNetboot:
                         default: false
-                        description: DisableNetboot disables netbooting for this interface. The interface will still receive network information speified on by DHCP.
+                        description: DisableNetboot disables netbooting for this interface. The interface will still receive network information specified by DHCP.
                         type: boolean
                     type: object
                   description: NetworkInterfaces defines the desired DHCP and netboot configuration for a network interface.

go.mod

@@ -97,11 +97,11 @@ require (
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/subosito/gotenv v1.4.2 // indirect
 	go.opentelemetry.io/otel v1.14.0 // indirect
-	go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.11.2 // indirect
-	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.11.2 // indirect
-	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.11.2 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.14.0 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.14.0 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.14.0 // indirect
 	go.opentelemetry.io/otel/metric v0.37.0 // indirect
-	go.opentelemetry.io/otel/sdk v1.11.2 // indirect
+	go.opentelemetry.io/otel/sdk v1.14.0 // indirect
 	go.opentelemetry.io/otel/trace v1.14.0 // indirect
 	go.opentelemetry.io/proto/otlp v0.19.0 // indirect
 	go.uber.org/atomic v1.10.0 // indirect

go.sum

@@ -823,16 +823,16 @@ go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.4
 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.40.0/go.mod h1:UMklln0+MRhZC4e3PwmN3pCtq4DyIadWw4yikh6bNrw=
 go.opentelemetry.io/otel v1.14.0 h1:/79Huy8wbf5DnIPhemGB+zEPVwnN6fuQybr/SRXa6hM=
 go.opentelemetry.io/otel v1.14.0/go.mod h1:o4buv+dJzx8rohcUeRmWUZhqupFvzWis188WlggnNeU=
-go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.11.2 h1:htgM8vZIF8oPSCxa341e3IZ4yr/sKxgu8KZYllByiVY=
-go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.11.2/go.mod h1:rqbht/LlhVBgn5+k3M5QK96K5Xb0DvXpMJ5SFQpY6uw=
-go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.11.2 h1:fqR1kli93643au1RKo0Uma3d2aPQKT+WBKfTSBaKbOc=
-go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.11.2/go.mod h1:5Qn6qvgkMsLDX+sYK64rHb1FPhpn0UtxF+ouX1uhyJE=
-go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.11.2 h1:ERwKPn9Aer7Gxsc0+ZlutlH1bEEAUXAUhqm3Y45ABbk=
-go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.11.2/go.mod h1:jWZUM2MWhWCJ9J9xVbRx7tzK1mXKpAlze4CeulycwVY=
+go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.14.0 h1:/fXHZHGvro6MVqV34fJzDhi7sHGpX3Ej/Qjmfn003ho=
+go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.14.0/go.mod h1:UFG7EBMRdXyFstOwH028U0sVf+AvukSGhF0g8+dmNG8=
+go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.14.0 h1:TKf2uAs2ueguzLaxOCBXNpHxfO/aC7PAdDsSH0IbeRQ=
+go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.14.0/go.mod h1:HrbCVv40OOLTABmOn1ZWty6CHXkU8DK/Urc43tHug70=
+go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.14.0 h1:ap+y8RXX3Mu9apKVtOkM6WSFESLM8K3wNQyOU8sWHcc=
+go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.14.0/go.mod h1:5w41DY6S9gZrbjuq6Y+753e96WfPha5IcsOSZTtullM=
 go.opentelemetry.io/otel/metric v0.37.0 h1:pHDQuLQOZwYD+Km0eb657A25NaRzy0a+eLyKfDXedEs=
 go.opentelemetry.io/otel/metric v0.37.0/go.mod h1:DmdaHfGt54iV6UKxsV9slj2bBRJcKC1B1uvDLIioc1s=
-go.opentelemetry.io/otel/sdk v1.11.2 h1:GF4JoaEx7iihdMFu30sOyRx52HDHOkl9xQ8SMqNXUiU=
-go.opentelemetry.io/otel/sdk v1.11.2/go.mod h1:wZ1WxImwpq+lVRo4vsmSOxdd+xwoUJ6rqyLc3SyX9aU=
+go.opentelemetry.io/otel/sdk v1.14.0 h1:PDCppFRDq8A1jL9v6KMI6dYesaq+DFcDZvjsoGvxGzY=
+go.opentelemetry.io/otel/sdk v1.14.0/go.mod h1:bwIC5TjrNG6QDCHNWvW4HLHtUQ4I+VQDsnjhvyZCALM=
 go.opentelemetry.io/otel/trace v1.14.0 h1:wp2Mmvj41tDsyAJXiWDWpfNsOiIyd38fy85pyKcFq/M=
 go.opentelemetry.io/otel/trace v1.14.0/go.mod h1:8avnQLK+CG77yNLUae4ea2JDQ6iT+gozhnZjy/rw9G8=
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
@@ -847,7 +847,7 @@ go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0
 go.uber.org/automaxprocs v1.4.0/go.mod h1:/mTEdr7LvHhs0v7mjdxDreTz1OG5zdZGqgOnhWiR/+Q=
 go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
 go.uber.org/goleak v1.1.11-0.20210813005559-691160354723/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=
-go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk=
+go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A=
 go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
 go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
 go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI=

internal/hardware/admission.go

@@ -0,0 +1,105 @@
+package hardware
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+
+	"github.com/tinkerbell/tink/api/v1alpha2"
+	"github.com/tinkerbell/tink/internal/hardware/internal"
+	ctrl "sigs.k8s.io/controller-runtime"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// admissionWebhookEndpoint is the endpoint serving the Admission handler.
+const admissionWebhookEndpoint = "/validate-tinkerbell-org-v1alpha2-hardware"
+
+// +kubebuilder:webhook:path=/validate-tinkerbell-org-v1alpha2-hardware,mutating=false,failurePolicy=fail,groups="",resources=hardware,verbs=create;update,versions=v1alpha2,name=hardware.tinkerbell.org
+
+// Admission handles complex validation for admitting a Hardware object to the cluster.
+type Admission struct {
+	client  ctrlclient.Client
+	decoder *admission.Decoder
+}
+
+// Handle satisfies controller-runtime/pkg/webhook/admission#Handler. It is responsible for deciding
+// if the given req is valid and should be admitted to the cluster.
+func (a *Admission) Handle(ctx context.Context, req admission.Request) admission.Response {
+	if a.client == nil {
+		return admission.Errored(http.StatusInternalServerError, errors.New("misconfigured client"))
+	}
+
+	var hw v1alpha2.Hardware
+	if err := a.decoder.Decode(req, &hw); err != nil {
+		return admission.Errored(http.StatusBadRequest, err)
+	}
+
+	// Ensure conditionally optional fields are valid
+	if resp := a.validateConditionalFields(&hw); !resp.Allowed {
+		return resp
+	}
+
+	// Ensure MACs on the hardware are valid.
+	if resp := a.validateMACs(&hw); !resp.Allowed {
+		return resp
+	}
+
+	// Ensure there's no hardware in the cluster with the same MAC addresses.
+	if resp := a.validateUniqueMACs(ctx, &hw); !resp.Allowed {
+		return resp
+	}
+
+	// Ensure there's no hardware in the cluster with the same IP addresses.
+	if resp := a.validateUniqueIPs(ctx, &hw); !resp.Allowed {
+		return resp
+	}
+
+	return admission.Allowed("")
+}
+
+// InjectDecoder satisfies controller-runtime/pkg/webhook/admission#DecoderInjector. It is used
+// when registering the webhook to inject the decoder used by the controller manager.
+func (a *Admission) InjectDecoder(d *admission.Decoder) error {
+	a.decoder = d
+	return nil
+}
+
+// SetClient sets a's internal Kubernetes client.
+func (a *Admission) SetClient(c ctrlclient.Client) {
+	a.client = c
+}
+
+// SetupWithManager registers a with mgr as a webhook served from AdmissionWebhookEndpoint.
+func (a *Admission) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
+	idx := mgr.GetFieldIndexer()
+
+	err := idx.IndexField(
+		ctx,
+		&v1alpha2.Hardware{},
+		internal.HardwareByMACAddr,
+		internal.HardwareByMACAddrFunc,
+	)
+	if err != nil {
+		return fmt.Errorf("register index %s: %w", internal.HardwareByMACAddr, err)
+	}
+
+	err = idx.IndexField(
+		ctx,
+		&v1alpha2.Hardware{},
+		internal.HardwareByIPAddr,
+		internal.HardwareByIPAddrFunc,
+	)
+	if err != nil {
+		return fmt.Errorf("register index %s: %w", internal.HardwareByIPAddr, err)
+	}
+
+	mgr.GetWebhookServer().Register(
+		admissionWebhookEndpoint,
+		&webhook.Admission{Handler: a},
+	)
+
+	return nil
+}

internal/hardware/admission_conditional.go

@@ -0,0 +1,22 @@
+package hardware
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/tinkerbell/tink/api/v1alpha2"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+func (a *Admission) validateConditionalFields(hw *v1alpha2.Hardware) admission.Response {
+	for mac, ni := range hw.Spec.NetworkInterfaces {
+		if ni.IsDHCPEnabled() && ni.DHCP == nil {
+			return admission.Errored(http.StatusBadRequest, fmt.Errorf(
+				"network interface for %v has DHCP enabled but no DHCP config",
+				mac,
+			))
+		}
+	}
+
+	return admission.Allowed("")
+}

internal/hardware/admission_ip.go

@@ -0,0 +1,56 @@
+package hardware
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"strings"
+
+	"github.com/tinkerbell/tink/api/v1alpha2"
+	"github.com/tinkerbell/tink/internal/hardware/internal"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+func (a *Admission) validateUniqueIPs(ctx context.Context, hw *v1alpha2.Hardware) admission.Response {
+	// Determine if there are IP duplicates within the hw object.
+	seen := map[string]struct{}{}
+	var dupOnHw []string
+	for _, ip := range hw.GetIPs() {
+		if _, ok := seen[ip]; ok {
+			dupOnHw = append(dupOnHw, ip)
+		}
+		seen[ip] = struct{}{}
+	}
+
+	if len(dupOnHw) > 0 {
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf(
+			"duplicate IPs on Hardware: %v",
+			strings.Join(dupOnHw, ", "),
+		))
+	}
+
+	// Determine if there are IP duplicates with other Hardware objects.
+	dups := duplicates{}
+	for _, ip := range hw.GetIPs() {
+		var hwWithIP v1alpha2.HardwareList
+		err := a.client.List(ctx, &hwWithIP, ctrlclient.MatchingFields{
+			internal.HardwareByIPAddr: ip,
+		})
+		if err != nil {
+			return admission.Errored(http.StatusInternalServerError, err)
+		}
+		if len(hwWithIP.Items) > 0 {
+			dups.AppendTo(ip, hwWithIP.Items...)
+		}
+	}
+
+	if len(dups) > 0 {
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf(
+			"IP associated with existing Hardware: %v",
+			dups.String(),
+		))
+	}
+
+	return admission.Allowed("")
+}

internal/hardware/admission_mac.go

@@ -0,0 +1,70 @@
+package hardware
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"regexp"
+	"strings"
+
+	"github.com/tinkerbell/tink/api/v1alpha2"
+	"github.com/tinkerbell/tink/internal/hardware/internal"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+func (a *Admission) validateMACs(hw *v1alpha2.Hardware) admission.Response {
+	// Validate all MACs on hw are valid before we compare them with Hardware in the cluster.
+	if invalidMACs := getInvalidMACs(hw); len(invalidMACs) > 0 {
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf(
+			"invalid MAC address (%v): %v",
+			macRegex.String(),
+			strings.Join(invalidMACs, ", "),
+		))
+	}
+
+	return admission.Allowed("")
+}
+
+// macRegex is taken from the API package documentation. It checks for valid MAC addresses.
+// It expects MACs to be lowercase which is necessary for index lookups on API objects.
+var macRegex = regexp.MustCompile("^([0-9a-f]{2}:){5}([0-9a-f]{2})$")
+
+func getInvalidMACs(hw *v1alpha2.Hardware) []string {
+	var invalidMACs []string
+	for _, mac := range hw.GetMACs() {
+		if mac == "" {
+			mac = "<empty string>"
+		}
+		if !macRegex.MatchString(mac) {
+			invalidMACs = append(invalidMACs, mac)
+		}
+	}
+	return invalidMACs
+}
+
+func (a *Admission) validateUniqueMACs(ctx context.Context, hw *v1alpha2.Hardware) admission.Response {
+	dups := duplicates{}
+	for _, mac := range hw.GetMACs() {
+		var hwWithMAC v1alpha2.HardwareList
+		err := a.client.List(ctx, &hwWithMAC, ctrlclient.MatchingFields{
+			internal.HardwareByMACAddr: mac,
+		})
+		if err != nil {
+			return admission.Errored(http.StatusInternalServerError, err)
+		}
+
+		if len(hwWithMAC.Items) > 0 {
+			dups.AppendTo(mac, hwWithMAC.Items...)
+		}
+	}
+
+	if len(dups) > 0 {
+		return admission.Errored(http.StatusBadRequest, fmt.Errorf(
+			"MAC associated with existing Hardware: %s",
+			dups.String(),
+		))
+	}
+
+	return admission.Allowed("")
+}