Add Endpoint webhook ensuring unique MAC addresses (#187)

* Add `Endpoint` webhook validating unique MAC addresses * Rename variable for readabilty * Implement review findings - improve wording - fix typos
ironcore-dev · Nov 28, 2024 · efee1bf · efee1bf
1 parent cd0a3dc
commit efee1bf
Show file tree

Hide file tree

Showing 9 changed files with 506 additions and 0 deletions.
diff --git a/PROJECT b/PROJECT
@@ -16,6 +16,9 @@ resources:
   kind: Endpoint
   path: github.com/ironcore-dev/metal-operator/api/v1alpha1
   version: v1alpha1
+  webhooks:
+    validation: true
+    webhookVersion: v1
 - api:
     crdVersion: v1
   controller: true

diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -10,6 +10,8 @@ import (
 	"os"
 	"time"
 
+	webhookmetalv1alpha1 "github.com/ironcore-dev/metal-operator/internal/webhook/v1alpha1"
+
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
@@ -252,6 +254,13 @@ func main() {
 		setupLog.Error(err, "unable to create controller", "controller", "ServerClaim")
 		os.Exit(1)
 	}
+	// nolint:goconst
+	if os.Getenv("ENABLE_WEBHOOKS") != "false" {
+		if err = webhookmetalv1alpha1.SetupEndpointWebhookWithManager(mgr); err != nil {
+			setupLog.Error(err, "unable to create webhook", "webhook", "Endpoint")
+			os.Exit(1)
+		}
+	}
 	//+kubebuilder:scaffold:builder
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {

diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml
@@ -0,0 +1,6 @@
+resources:
+- manifests.yaml
+- service.yaml
+
+configurations:
+- kustomizeconfig.yaml
diff --git a/config/webhook/kustomizeconfig.yaml b/config/webhook/kustomizeconfig.yaml
@@ -0,0 +1,22 @@
+# the following config is for teaching kustomize where to look at when substituting nameReference.
+# It requires kustomize v2.1.0 or newer to work properly.
+nameReference:
+- kind: Service
+  version: v1
+  fieldSpecs:
+  - kind: MutatingWebhookConfiguration
+    group: admissionregistration.k8s.io
+    path: webhooks/clientConfig/service/name
+  - kind: ValidatingWebhookConfiguration
+    group: admissionregistration.k8s.io
+    path: webhooks/clientConfig/service/name
+
+namespace:
+- kind: MutatingWebhookConfiguration
+  group: admissionregistration.k8s.io
+  path: webhooks/clientConfig/service/namespace
+  create: true
+- kind: ValidatingWebhookConfiguration
+  group: admissionregistration.k8s.io
+  path: webhooks/clientConfig/service/namespace
+  create: true
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -0,0 +1,26 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  name: validating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-metal-ironcore-dev-v1alpha1-endpoint
+  failurePolicy: Fail
+  name: vendpoint-v1alpha1.kb.io
+  rules:
+  - apiGroups:
+    - metal.ironcore.dev
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - endpoints
+  sideEffects: None
diff --git a/config/webhook/service.yaml b/config/webhook/service.yaml
@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    app.kubernetes.io/name: metal-operator
+    app.kubernetes.io/managed-by: kustomize
+  name: webhook-service
+  namespace: system
+spec:
+  ports:
+    - port: 443
+      protocol: TCP
+      targetPort: 9443
+  selector:
+    control-plane: controller-manager
diff --git a/internal/webhook/v1alpha1/endpoint_webhook.go b/internal/webhook/v1alpha1/endpoint_webhook.go
@@ -0,0 +1,137 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha1
+
+import (
+	"context"
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var endpointlog = logf.Log.WithName("endpoint-resource")
+
+// SetupEndpointWebhookWithManager registers the webhook for Endpoint in the manager.
+func SetupEndpointWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).For(&metalv1alpha1.Endpoint{}).
+		WithValidator(&EndpointCustomValidator{Client: mgr.GetClient()}).
+		Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-metal-ironcore-dev-v1alpha1-endpoint,mutating=false,failurePolicy=fail,sideEffects=None,groups=metal.ironcore.dev,resources=endpoints,verbs=create;update,versions=v1alpha1,name=vendpoint-v1alpha1.kb.io,admissionReviewVersions=v1
+
+// EndpointCustomValidator struct is responsible for validating the Endpoint resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type EndpointCustomValidator struct {
+	Client client.Client
+}
+
+var _ webhook.CustomValidator = &EndpointCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Endpoint.
+func (v *EndpointCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	allErrs := field.ErrorList{}
+
+	endpoint, ok := obj.(*metalv1alpha1.Endpoint)
+	if !ok {
+		return nil, fmt.Errorf("expected an Endpoint object but got %T", obj)
+	}
+	endpointlog.Info("Validation for Endpoint upon creation", "name", endpoint.GetName())
+
+	allErrs = append(allErrs, ValidateMACAddressCreate(ctx, v.Client, endpoint.Spec, field.NewPath("spec"))...)
+
+	if len(allErrs) != 0 {
+		return nil, apierrors.NewInvalid(
+			schema.GroupKind{Group: "metal.ironcore.dev", Kind: "Endpoint"},
+			endpoint.GetName(), allErrs)
+	}
+
+	return nil, nil
+}
+
+// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type Endpoint.
+func (v *EndpointCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+	allErrs := field.ErrorList{}
+
+	endpoint, ok := newObj.(*metalv1alpha1.Endpoint)
+	if !ok {
+		return nil, fmt.Errorf("expected an Endpoint object for the newObj but got %T", newObj)
+	}
+	endpointlog.Info("Validation for Endpoint upon update", "name", endpoint.GetName())
+
+	allErrs = append(allErrs, ValidateMACAddressUpdate(ctx, v.Client, endpoint, field.NewPath("spec"))...)
+
+	if len(allErrs) != 0 {
+		return nil, apierrors.NewInvalid(
+			schema.GroupKind{Group: "metal.ironcore.dev", Kind: "Endpoint"},
+			endpoint.GetName(), allErrs)
+	}
+
+	return nil, nil
+}
+
+// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type Endpoint.
+func (v *EndpointCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	endpoint, ok := obj.(*metalv1alpha1.Endpoint)
+	if !ok {
+		return nil, fmt.Errorf("expected an Endpoint object but got %T", obj)
+	}
+	endpointlog.Info("Validation for Endpoint upon deletion", "name", endpoint.GetName())
+
+	// TODO(user): fill in your validation logic upon object deletion.
+
+	return nil, nil
+}
+
+func ValidateMACAddressCreate(ctx context.Context, c client.Client, spec metalv1alpha1.EndpointSpec, path *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	endpoints := &metalv1alpha1.EndpointList{}
+	if err := c.List(ctx, endpoints); err != nil {
+		allErrs = append(allErrs, field.InternalError(path, fmt.Errorf("failed to list Endpoints: %w", err)))
+	}
+
+	for _, e := range endpoints.Items {
+		if e.Spec.MACAddress == spec.MACAddress {
+			allErrs = append(allErrs, field.Duplicate(field.NewPath("spec").Child("MACAddress"), e.Spec.MACAddress))
+		}
+	}
+
+	return allErrs
+}
+
+func ValidateMACAddressUpdate(ctx context.Context, c client.Client, updatedEndpoint *metalv1alpha1.Endpoint, path *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	endpoints := &metalv1alpha1.EndpointList{}
+	if err := c.List(ctx, endpoints); err != nil {
+		allErrs = append(allErrs, field.InternalError(path, fmt.Errorf("failed to list Endpoints: %w", err)))
+	}
+
+	for _, e := range endpoints.Items {
+		if e.Spec.MACAddress == updatedEndpoint.Spec.MACAddress && e.Name != updatedEndpoint.Name {
+			allErrs = append(allErrs, field.Duplicate(field.NewPath("spec").Child("MACAddress"), e.Spec.MACAddress))
+		}
+	}
+
+	return allErrs
+}
diff --git a/internal/webhook/v1alpha1/endpoint_webhook_test.go b/internal/webhook/v1alpha1/endpoint_webhook_test.go
@@ -0,0 +1,140 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha1
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1"
+)
+
+var _ = Describe("Endpoint Webhook", func() {
+	var (
+		obj       *metalv1alpha1.Endpoint
+		oldObj    *metalv1alpha1.Endpoint
+		validator EndpointCustomValidator
+	)
+
+	BeforeEach(func() {
+		obj = &metalv1alpha1.Endpoint{}
+		oldObj = &metalv1alpha1.Endpoint{}
+		validator = EndpointCustomValidator{
+			Client: k8sClient,
+		}
+		Expect(validator).NotTo(BeNil(), "Expected validator to be initialized")
+		Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized")
+		Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
+	})
+
+	Context("When creating or updating an Endpoint under Validating Webhook", func() {
+		It("Should deny creation if an Endpoint has a duplicate MAC address", func(ctx SpecContext) {
+			By("Creating an Endpoint")
+			endpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("1.1.1.1"),
+					MACAddress: "foo",
+				},
+			}
+			Expect(k8sClient.Create(ctx, endpoint)).To(Succeed())
+			DeferCleanup(k8sClient.Delete, endpoint)
+
+			By("Creating an Endpoint with existing MAC address")
+			existingEndpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("2.2.2.2"),
+					MACAddress: "foo",
+				},
+			}
+			Expect(validator.ValidateCreate(ctx, existingEndpoint)).Error().To(HaveOccurred())
+		})
+
+		It("Should allow creation if an Endpoint has a unique MAC address", func(ctx SpecContext) {
+			By("Creating an Endpoint")
+			endpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("1.1.1.1"),
+					MACAddress: "foo",
+				},
+			}
+			Expect(k8sClient.Create(ctx, endpoint)).ToNot(HaveOccurred())
+			DeferCleanup(k8sClient.Delete, endpoint)
+
+			By("Creating an Endpoint with non-existing MAC address")
+			existingEndpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("2.2.2.2"),
+					MACAddress: "bar",
+				},
+			}
+			Expect(validator.ValidateCreate(ctx, existingEndpoint)).Error().ToNot(HaveOccurred())
+		})
+
+		It("Should deny update of an Endpoint with existing MAC address", func() {
+			By("Creating an Endpoint")
+			endpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("1.1.1.1"),
+					MACAddress: "foo",
+				},
+			}
+			Expect(k8sClient.Create(ctx, endpoint)).To(Succeed())
+			DeferCleanup(k8sClient.Delete, endpoint)
+
+			By("Creating an Endpoint with different MAC address")
+			existingEndpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("2.2.2.2"),
+					MACAddress: "bar",
+				},
+			}
+			Expect(k8sClient.Create(ctx, existingEndpoint)).To(Succeed())
+			DeferCleanup(k8sClient.Delete, existingEndpoint)
+
+			By("Updating an Endpoint to conflicting MAC address")
+			updatedEndpoint := endpoint.DeepCopy()
+			updatedEndpoint.Spec.MACAddress = "bar"
+			Expect(validator.ValidateUpdate(ctx, endpoint, updatedEndpoint)).Error().To(HaveOccurred())
+		})
+
+		It("Should allow update an IP address of the same Endpoint", func() {
+			By("Creating an Endpoint")
+			existingEndpoint := &metalv1alpha1.Endpoint{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "test-",
+				},
+				Spec: metalv1alpha1.EndpointSpec{
+					IP:         metalv1alpha1.MustParseIP("1.1.1.1"),
+					MACAddress: "foo",
+				},
+			}
+			Expect(k8sClient.Create(ctx, existingEndpoint)).To(Succeed())
+			DeferCleanup(k8sClient.Delete, existingEndpoint)
+
+			By("Updating an Endpoint IP address")
+			updatedEndpoint := existingEndpoint.DeepCopy()
+			updatedEndpoint.Spec.IP = metalv1alpha1.MustParseIP("2.2.2.2")
+			Expect(validator.ValidateUpdate(ctx, existingEndpoint, updatedEndpoint)).Error().ToNot(HaveOccurred())
+		})
+	})
+})