From 8b9bd1dbb0a926b911d24f2e677e44487ffbdce9 Mon Sep 17 00:00:00 2001 From: Andrej Krejcir Date: Mon, 4 Dec 2023 13:28:17 +0100 Subject: [PATCH 1/2] chore: webhook: Extract common functionality to a function Extracted common functionality from ValidateCreate and ValidateUpdate functions. Signed-off-by: Andrej Krejcir --- webhooks/ssp_webhook.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/webhooks/ssp_webhook.go b/webhooks/ssp_webhook.go index df970372e..03a366252 100644 --- a/webhooks/ssp_webhook.go +++ b/webhooks/ssp_webhook.go @@ -87,19 +87,7 @@ func (s *sspValidator) ValidateCreate(ctx context.Context, obj runtime.Object) e 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 +98,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 From f08e4a92b5315b5601d8cf4fa17e92fa13b58e14 Mon Sep 17 00:00:00 2001 From: Andrej Krejcir Date: Mon, 4 Dec 2023 13:35:48 +0100 Subject: [PATCH 2/2] fix: webhook: Don't check common template namespace The namespace of common templates is not checked in webhook. Even if the webhook passes, user can still remove the namespace later. Signed-off-by: Andrej Krejcir --- tests/webhook_test.go | 14 ------------- webhooks/ssp_webhook.go | 8 -------- webhooks/ssp_webhook_test.go | 38 ------------------------------------ 3 files changed, 60 deletions(-) diff --git a/tests/webhook_test.go b/tests/webhook_test.go index 33458a288..be7fd7b74 100644 --- 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 index 03a366252..bc4322fc4 100644 --- a/webhooks/ssp_webhook.go +++ b/webhooks/ssp_webhook.go @@ -79,14 +79,6 @@ 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) - } - return s.validateSspObject(ctx, sspObj) } diff --git a/webhooks/ssp_webhook_test.go b/webhooks/ssp_webhook_test.go index e5c3289a5..420fc2a2e 100644 --- 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"