Adding PreflightValidation Webhook

Until now users were able to add invalid kernel versions to
PreflightValidation, which made KMM controller panic.
This commit adds a validation webhook for PreflightValidation resource.

Author: TomerNewman

cmd/webhook-server/main.go

@@ -109,8 +109,9 @@ func main() {
 			cmd.FatalError(setupLogger, err, "unable to create conversion webhook", "name", "PreflightValidation/v1beta1")
 		}
 
-		if err = ctrl.NewWebhookManagedBy(mgr).For(&kmmv1beta2.PreflightValidation{}).Complete(); err != nil {
-			cmd.FatalError(setupLogger, err, "unable to create conversion webhook", "name", "PreflightValidation/v1beta2")
+		logger.Info("Enabling PreflightValidation webhook")
+		if err = webhook.NewPreflightValidationValidator(logger).SetupWebhookWithManager(mgr, &kmmv1beta2.PreflightValidation{}); err != nil {
+			cmd.FatalError(setupLogger, err, "unable to create webhook", "webhook", "PreflightValidationValidator")
 		}
 	}
 

config/webhook/manifests.yaml

@@ -43,3 +43,23 @@ webhooks:
     resources:
     - modules
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-kmm-sigs-x-k8s-io-v1beta2-preflightvalidation
+  failurePolicy: Fail
+  name: vpreflightvalidation.kb.io
+  rules:
+  - apiGroups:
+    - kmm.sigs.x-k8s.io
+    apiVersions:
+    - v1beta2
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - preflightvalidations
+  sideEffects: None

internal/module/kernelmapper.go

@@ -149,7 +149,10 @@ func (kh *kernelMapperHelper) prepareModuleLoaderData(mapping *kmmv1beta1.Kernel
 }
 
 func (kh *kernelMapperHelper) replaceTemplates(mld *api.ModuleLoaderData) error {
-	osConfigEnvVars := utils.KernelComponentsAsEnvVars(mld.KernelNormalizedVersion)
+	osConfigEnvVars, err := utils.KernelComponentsAsEnvVars(mld.KernelNormalizedVersion)
+	if err != nil {
+		return fmt.Errorf("failed to get kernel componnents as env variables, %v", err)
+	}
 	osConfigEnvVars = append(osConfigEnvVars, "MOD_NAME="+mld.Name, "MOD_NAMESPACE="+mld.Namespace)
 
 	replacedContainerImage, err := utils.ReplaceInTemplates(osConfigEnvVars, mld.ContainerImage)
@@ -205,9 +208,12 @@ func (kh *kernelMapperHelper) getRelevantSign(moduleSign *kmmv1beta1.Sign, mappi
 		signConfig.FilesToSign = append(signConfig.FilesToSign, mappingSign.FilesToSign...)
 	}
 
-	osConfigEnvVars := utils.KernelComponentsAsEnvVars(
+	osConfigEnvVars, err := utils.KernelComponentsAsEnvVars(
 		kernel.NormalizeVersion(kernelVersion),
 	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get kernel componnents as env variables, %v", err)
+	}
 	unsignedImage, err := utils.ReplaceInTemplates(osConfigEnvVars, signConfig.UnsignedImage)
 	if err != nil {
 		return nil, err

internal/utils/replaceStrings.go

@@ -14,10 +14,13 @@ const (
 	kernelVersionPatchIdx = 2
 )
 
-var kernelRegexp = regexp.MustCompile("[.,-]")
+var KernelRegexp = regexp.MustCompile("[.,-]")
 
-func KernelComponentsAsEnvVars(kernel string) []string {
-	osConfigFieldsList := kernelRegexp.Split(kernel, -1)
+func KernelComponentsAsEnvVars(kernel string) ([]string, error) {
+	osConfigFieldsList := KernelRegexp.Split(kernel, -1)
+	if len(osConfigFieldsList) < 3 {
+		return nil, fmt.Errorf("invalid kernel version %s: expected at least three components (major.minor.patch)", kernel)
+	}
 
 	envvars := []string{
 		"KERNEL_FULL_VERSION=" + kernel,
@@ -28,7 +31,7 @@ func KernelComponentsAsEnvVars(kernel string) []string {
 		"KERNEL_Z=" + osConfigFieldsList[kernelVersionPatchIdx],
 	}
 
-	return envvars
+	return envvars, nil
 }
 
 func ReplaceInTemplates(envvars []string, templates ...string) ([]string, error) {

internal/utils/replaceStrings_test.go

@@ -17,8 +17,16 @@ var _ = Describe("KernelComponentsAsEnvVars", func() {
 			"KERNEL_Y=0",
 			"KERNEL_Z=15",
 		}
+		osConfigEnvVars, err := KernelComponentsAsEnvVars(kernelVersion)
+		Expect(osConfigEnvVars).To(Equal(expected))
+		Expect(err).ToNot(HaveOccurred())
+	})
 
-		Expect(KernelComponentsAsEnvVars(kernelVersion)).To(Equal(expected))
+	It("should fail due to invalid kernel version", func() {
+		const kernelVersion = "test"
+		osConfigEnvVars, err := KernelComponentsAsEnvVars(kernelVersion)
+		Expect(osConfigEnvVars).To(BeNil())
+		Expect(err).To(HaveOccurred())
 	})
 })
 

internal/webhook/preflight.go

@@ -0,0 +1,72 @@
+package webhook
+
+import (
+	"context"
+	"fmt"
+	"github.com/go-logr/logr"
+	kmmv1beta2 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta2"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// PreflightValidationValidator validates PreflightValidation resources.
+type PreflightValidationValidator struct {
+	logger logr.Logger
+}
+
+func NewPreflightValidationValidator(logger logr.Logger) *PreflightValidationValidator {
+	return &PreflightValidationValidator{logger: logger}
+}
+
+func (v *PreflightValidationValidator) SetupWebhookWithManager(mgr ctrl.Manager, pf *kmmv1beta2.PreflightValidation) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(pf).
+		WithValidator(v).
+		Complete()
+}
+
+//+kubebuilder:webhook:path=/validate-kmm-sigs-x-k8s-io-v1beta2-preflightvalidation,mutating=false,failurePolicy=fail,sideEffects=None,groups=kmm.sigs.x-k8s.io,resources=preflightvalidations,verbs=create;update,versions=v1beta2,name=vpreflightvalidation.kb.io,admissionReviewVersions=v1
+
+func (v *PreflightValidationValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	pv, ok := obj.(*kmmv1beta2.PreflightValidation)
+	if !ok {
+		return nil, fmt.Errorf("bad type for the object; expected %v, got %v", pv, obj)
+	}
+
+	v.logger.Info("Validating PreflightValidation creation", "name", pv.Name)
+	return validatePreflight(pv)
+}
+
+func (v *PreflightValidationValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+	oldPV, ok := oldObj.(*kmmv1beta2.PreflightValidation)
+	if !ok {
+		return nil, fmt.Errorf("bad type for the old object; expected %v, got %v", oldPV, oldObj)
+	}
+
+	newPV, ok := newObj.(*kmmv1beta2.PreflightValidation)
+	if !ok {
+		return nil, fmt.Errorf("bad type for the new object; expected %v, got %v", newPV, newObj)
+	}
+
+	v.logger.Info("Validating PreflightValidation update", "name", oldPV.Name)
+	return validatePreflight(newPV)
+}
+
+func (v *PreflightValidationValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	return nil, NotImplemented
+}
+
+func validatePreflight(pv *kmmv1beta2.PreflightValidation) (admission.Warnings, error) {
+	if pv.Spec.KernelVersion == "" {
+		return nil, fmt.Errorf("kernelVersion cannot be empty")
+	}
+
+	fields := utils.KernelRegexp.Split(pv.Spec.KernelVersion, -1)
+	if len(fields) < 3 {
+		return nil, fmt.Errorf("invalid kernelVersion %s", pv.Spec.KernelVersion)
+	}
+
+	return nil, nil
+}

internal/webhook/preflight_test.go

@@ -0,0 +1,41 @@
+package webhook
+
+import (
+	"context"
+
+	kmmv1beta2 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta2"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("validatePreflight", func() {
+	It("should fail with invalid kernel version", func() {
+		pv := &kmmv1beta2.PreflightValidation{
+			Spec: kmmv1beta2.PreflightValidationSpec{
+				KernelVersion: "test",
+			},
+		}
+		_, err := validatePreflight(pv)
+		Expect(err).To(HaveOccurred())
+	})
+
+	It("should pass with valid kernel version", func() {
+		pv := &kmmv1beta2.PreflightValidation{
+			Spec: kmmv1beta2.PreflightValidationSpec{
+				KernelVersion: "6.0.15-300.fc37.x86_64",
+			},
+		}
+		_, err := validatePreflight(pv)
+		Expect(err).NotTo(HaveOccurred())
+	})
+})
+
+var _ = Describe("PreflightValidationValidator", func() {
+	v := NewPreflightValidationValidator(GinkgoLogr)
+	ctx := context.TODO()
+
+	It("ValidateDelete should return not implemented", func() {
+		_, err := v.ValidateDelete(ctx, nil)
+		Expect(err).To(Equal(NotImplemented))
+	})
+})