diff --git a/pkg/hub/test/helper.go b/pkg/hub/test/helper.go index 4a4b3c5b4..b487a455a 100644 --- a/pkg/hub/test/helper.go +++ b/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, }, diff --git a/pkg/webhook/v1/manifestwork_validating.go b/pkg/webhook/v1/manifestwork_validating.go index bcd47dbe9..f0187c0ae 100644 --- a/pkg/webhook/v1/manifestwork_validating.go +++ b/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 diff --git a/pkg/webhook/v1alpha1/placemanifestwork_validating.go b/pkg/webhook/v1alpha1/placemanifestwork_validating.go index c879a7379..27584b153 100644 --- a/pkg/webhook/v1alpha1/placemanifestwork_validating.go +++ b/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) } diff --git a/pkg/webhook/v1alpha1/placemanifestwork_validating_test.go b/pkg/webhook/v1alpha1/placemanifestwork_validating_test.go new file mode 100644 index 000000000..8dfe6beca --- /dev/null +++ b/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) + } +}