Merge pull request #747 from akrejcir/webhook-templates-namespace

fix: webhook: Don't check common template namespace
kubevirt · Dec 6, 2023 · 5428531 · 5428531
2 parents d90afe7 + f08e4a9
commit 5428531
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 80 deletions.
diff --git a/tests/webhook_test.go b/tests/webhook_test.go
@@ -114,20 +114,6 @@ var _ = Describe("Validation webhook", func() {
 				strategy.RevertToOriginalSspCr()
 			})
 
-			It("[test_id:6056] should fail to create SSP CR with invalid commonTemplates.namespace", func() {
-				newSsp.Spec.CommonTemplates.Namespace = "nonexisting-templates-namespace"
-
-				err := apiClient.Create(ctx, newSsp, client.DryRunAll)
-				if err == nil {
-					Fail("SSP CR with invalid commonTemplates.namespace created.")
-					return
-				}
-				Expect(err.Error()).To(ContainSubstring(fmt.Sprintf(
-					"creation failed, the configured namespace for common templates does not exist: %v",
-					newSsp.Spec.CommonTemplates.Namespace,
-				)))
-			})
-
 			Context("Placement API validation", func() {
 				It("[test_id:5988]should succeed with valid template-validator placement fields", func() {
 					newSsp.Spec.TemplateValidator = &sspv1beta2.TemplateValidator{

diff --git a/webhooks/ssp_webhook.go b/webhooks/ssp_webhook.go
@@ -79,27 +79,7 @@ func (s *sspValidator) ValidateCreate(ctx context.Context, obj runtime.Object) e
 		return fmt.Errorf("creation failed, an SSP CR already exists in namespace %v: %v", ssps.Items[0].ObjectMeta.Namespace, ssps.Items[0].ObjectMeta.Name)
 	}
 
-	// Check if the common templates namespace exists
-	namespaceName := sspObj.Spec.CommonTemplates.Namespace
-	var namespace v1.Namespace
-	err = s.apiClient.Get(ctx, client.ObjectKey{Name: namespaceName}, &namespace)
-	if err != nil {
-		return fmt.Errorf("creation failed, the configured namespace for common templates does not exist: %v", namespaceName)
-	}
-
-	if err = s.validatePlacement(ctx, sspObj); err != nil {
-		return fmt.Errorf("placement api validation error: %w", err)
-	}
-
-	if err := validateDataImportCronTemplates(sspObj); err != nil {
-		return fmt.Errorf("dataImportCronTemplates validation error: %w", err)
-	}
-
-	if err := validateCommonInstancetypes(sspObj); err != nil {
-		return fmt.Errorf("commonInstancetypes validation error: %w", err)
-	}
-
-	return nil
+	return s.validateSspObject(ctx, sspObj)
 }
 
 func (s *sspValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) error {
@@ -110,25 +90,29 @@ func (s *sspValidator) ValidateUpdate(ctx context.Context, _, newObj runtime.Obj
 
 	ssplog.Info("validate update", "name", newSsp.Name)
 
-	if err := s.validatePlacement(ctx, newSsp); err != nil {
+	return s.validateSspObject(ctx, newSsp)
+}
+
+func (s *sspValidator) ValidateDelete(_ context.Context, _ runtime.Object) error {
+	return nil
+}
+
+func (s *sspValidator) validateSspObject(ctx context.Context, ssp *sspv1beta2.SSP) error {
+	if err := s.validatePlacement(ctx, ssp); err != nil {
 		return fmt.Errorf("placement api validation error: %w", err)
 	}
 
-	if err := validateDataImportCronTemplates(newSsp); err != nil {
+	if err := validateDataImportCronTemplates(ssp); err != nil {
 		return fmt.Errorf("dataImportCronTemplates validation error: %w", err)
 	}
 
-	if err := validateCommonInstancetypes(newSsp); err != nil {
+	if err := validateCommonInstancetypes(ssp); err != nil {
 		return fmt.Errorf("commonInstancetypes validation error: %w", err)
 	}
 
 	return nil
 }
 
-func (s *sspValidator) ValidateDelete(_ context.Context, _ runtime.Object) error {
-	return nil
-}
-
 func (s *sspValidator) validatePlacement(ctx context.Context, ssp *sspv1beta2.SSP) error {
 	if ssp.Spec.TemplateValidator == nil {
 		return nil

diff --git a/webhooks/ssp_webhook_test.go b/webhooks/ssp_webhook_test.go
@@ -115,24 +115,6 @@ var _ = Describe("SSP Validation", func() {
 			})
 		})
 
-		It("should fail if template namespace does not exist", func() {
-			const nonexistingNamespace = "nonexisting-namespace"
-			ssp := &sspv1beta2.SSP{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-ssp",
-					Namespace: "test-ns",
-				},
-				Spec: sspv1beta2.SSPSpec{
-					CommonTemplates: sspv1beta2.CommonTemplates{
-						Namespace: nonexistingNamespace,
-					},
-				},
-			}
-			err := validator.ValidateCreate(ctx, toUnstructured(ssp))
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("creation failed, the configured namespace for common templates does not exist: " + nonexistingNamespace))
-		})
-
 		It("should accept old v1beta1 SSP CR", func() {
 			ssp := &sspv1beta1.SSP{
 				ObjectMeta: metav1.ObjectMeta{
@@ -166,26 +148,6 @@ var _ = Describe("SSP Validation", func() {
 		})
 	})
 
-	It("should allow update of commonTemplates.namespace", func() {
-		oldSsp := &sspv1beta2.SSP{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-ssp",
-				Namespace: "test-ns",
-			},
-			Spec: sspv1beta2.SSPSpec{
-				CommonTemplates: sspv1beta2.CommonTemplates{
-					Namespace: "old-ns",
-				},
-			},
-		}
-
-		newSsp := oldSsp.DeepCopy()
-		newSsp.Spec.CommonTemplates.Namespace = "new-ns"
-
-		err := validator.ValidateUpdate(ctx, toUnstructured(oldSsp), toUnstructured(newSsp))
-		Expect(err).ToNot(HaveOccurred())
-	})
-
 	Context("DataImportCronTemplates", func() {
 		const (
 			templatesNamespace = "test-templates-ns"