Add validate webhook (#182)

Signed-off-by: melserngawy <melserng@redhat.com>

pkg/hub/test/helper.go

@@ -3,11 +3,14 @@ package test
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1"
-	workapiv1 "open-cluster-management.io/api/work/v1"
 	workapiv1alpha1 "open-cluster-management.io/api/work/v1alpha1"
+	"open-cluster-management.io/work/pkg/spoke/spoketesting"
 )
 
 func CreateTestPlaceManifestWork(name string, ns string, placementName string) *workapiv1alpha1.PlaceManifestWork {
+	obj := spoketesting.NewUnstructured("v1", "kind", "test-ns", "test-name")
+	mw, _ := spoketesting.NewManifestWork(0, obj)
+
 	pmw := &workapiv1alpha1.PlaceManifestWork{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            name,
@@ -16,11 +19,7 @@ func CreateTestPlaceManifestWork(name string, ns string, placementName string) *
 			UID:             "0b1441ec-717f-4877-a165-27e5b59245f5",
 		},
 		Spec: workapiv1alpha1.PlaceManifestWorkSpec{
-			ManifestWorkTemplate: workapiv1.ManifestWorkSpec{
-				Workload: workapiv1.ManifestsTemplate{
-					Manifests: []workapiv1.Manifest{},
-				},
-			},
+			ManifestWorkTemplate: mw.Spec,
 			PlacementRef: workapiv1alpha1.LocalPlacementReference{
 				Name: placementName,
 			},

pkg/webhook/v1/manifestwork_validating.go

@@ -59,7 +59,7 @@ func (r *ManifestWorkWebhook) validateRequest(newWork, oldWork *workv1.ManifestW
 		return apierrors.NewBadRequest("manifests should not be empty")
 	}
 
-	if err := validateManifests(newWork); err != nil {
+	if err := ValidateManifests(newWork.Spec.Workload.Manifests); err != nil {
 		return apierrors.NewBadRequest(err.Error())
 	}
 
@@ -75,9 +75,13 @@ func (r *ManifestWorkWebhook) validateRequest(newWork, oldWork *workv1.ManifestW
 	return validateExecutor(r.kubeClient, newWork, req.UserInfo)
 }
 
-func validateManifests(work *workv1.ManifestWork) error {
+func ValidateManifests(manifests []workv1.Manifest) error {
+	if len(manifests) == 0 {
+		return apierrors.NewBadRequest("Workload manifests should not be empty")
+	}
+
 	totalSize := 0
-	for _, manifest := range work.Spec.Workload.Manifests {
+	for _, manifest := range manifests {
 		totalSize = totalSize + manifest.Size()
 	}
 
@@ -85,7 +89,7 @@ func validateManifests(work *workv1.ManifestWork) error {
 		return apierrors.NewBadRequest(fmt.Sprintf("the size of manifests is %v bytes which exceeds the 50k limit", totalSize))
 	}
 
-	for _, manifest := range work.Spec.Workload.Manifests {
+	for _, manifest := range manifests {
 		err := validateManifest(manifest.Raw)
 		if err != nil {
 			return err

pkg/webhook/v1alpha1/placemanifestwork_validating.go

@@ -6,6 +6,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	workv1alpha1 "open-cluster-management.io/api/work/v1alpha1"
+	webhookv1 "open-cluster-management.io/work/pkg/webhook/v1"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
@@ -55,6 +56,5 @@ func (r *PlaceManifestWorkWebhook) validateRequest(newPlaceWork *workv1alpha1.Pl
 }
 
 func validatePlaceManifests(placeWork *workv1alpha1.PlaceManifestWork) error {
-
-	return nil
+	return webhookv1.ValidateManifests(placeWork.Spec.ManifestWorkTemplate.Workload.Manifests)
 }

pkg/webhook/v1alpha1/placemanifestwork_validating_test.go

@@ -0,0 +1,110 @@
+package v1alpha1
+
+import (
+	"context"
+	admissionv1 "k8s.io/api/admission/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	workv1alpha1 "open-cluster-management.io/api/work/v1alpha1"
+	helpertest "open-cluster-management.io/work/pkg/hub/test"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	"testing"
+)
+
+var placeManifestWorkSchema = metav1.GroupVersionResource{
+	Group:    "work.open-cluster-management.io",
+	Version:  "v1alpha1",
+	Resource: "placemanifestworks",
+}
+
+func TestWebHookValidateRequest(t *testing.T) {
+	request := admission.Request{
+		AdmissionRequest: admissionv1.AdmissionRequest{
+			Resource:  placeManifestWorkSchema,
+			Operation: admissionv1.Connect,
+		},
+	}
+	ctx := admission.NewContextWithRequest(context.Background(), request)
+	webHook := PlaceManifestWorkWebhook{}
+	pmw := &workv1alpha1.PlaceManifestWork{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "name",
+			Namespace: "ns",
+		},
+		Spec: workv1alpha1.PlaceManifestWorkSpec{
+			PlacementRef: workv1alpha1.LocalPlacementReference{
+				Name: "place",
+			},
+		},
+	}
+
+	err := webHook.validateRequest(pmw, nil, ctx)
+	if err == nil {
+		t.Fatal("Expecting error for empty ManifestWorkTemplate")
+	}
+	if !apierrors.IsBadRequest(err) {
+		t.Fatal("Expecting bad request error type")
+	}
+
+	pmw = helpertest.CreateTestPlaceManifestWork("pmw-test", "default", "place-test")
+	err = webHook.validateRequest(pmw, nil, ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestWebHookCreateRequest(t *testing.T) {
+	webHook := PlaceManifestWorkWebhook{}
+	pmw := &workv1alpha1.PlaceManifestWork{}
+	err := webHook.ValidateCreate(context.Background(), pmw)
+	if err == nil {
+		t.Fatal("Expecting error for empty pmw")
+	}
+	if !apierrors.IsBadRequest(err) {
+		t.Fatal("Expecting bad request error type")
+	}
+	request := admission.Request{
+		AdmissionRequest: admissionv1.AdmissionRequest{
+			Resource:  placeManifestWorkSchema,
+			Operation: admissionv1.Create,
+		},
+	}
+	ctx := admission.NewContextWithRequest(context.Background(), request)
+	pmw = helpertest.CreateTestPlaceManifestWork("pmw-test", "default", "place-test")
+	err = webHook.ValidateCreate(ctx, pmw)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestWebHookUpdateRequest(t *testing.T) {
+	webHook := PlaceManifestWorkWebhook{}
+	pmw := helpertest.CreateTestPlaceManifestWork("pmw-test", "default", "place-test")
+	err := webHook.ValidateUpdate(context.Background(), nil, pmw)
+	if err == nil {
+		t.Fatal("Expecting error for nil oldPlaceMW")
+	}
+	if !apierrors.IsBadRequest(err) {
+		t.Fatal("Expecting bad request error type")
+	}
+
+	err = webHook.ValidateUpdate(context.Background(), pmw, nil)
+	if err == nil {
+		t.Fatal("Expecting error for nil newPlaceMW")
+	}
+	if !apierrors.IsBadRequest(err) {
+		t.Fatal("Expecting bad request error type")
+	}
+
+	request := admission.Request{
+		AdmissionRequest: admissionv1.AdmissionRequest{
+			Resource:  placeManifestWorkSchema,
+			Operation: admissionv1.Update,
+		},
+	}
+	ctx := admission.NewContextWithRequest(context.Background(), request)
+	err = webHook.ValidateUpdate(ctx, pmw, pmw)
+	if err != nil {
+		t.Fatal(err)
+	}
+}