Merge pull request konflux-ci#930 from FilipNikolovski/RELEASE-1894

Author: FilipNikolovski

api/v1alpha1/webhooks/release/webhook.go

@@ -80,9 +80,15 @@ func (w *Webhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (w *Webhook) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
 	release := obj.(*v1alpha1.Release)
+
+	// Validate that resource names used as labels don't exceed Kubernetes label value limit (63 characters)
 	if len(release.Name) > 63 {
 		return nil, fmt.Errorf("release name must be no more than 63 characters, got %d characters", len(release.Name))
 	}
+	if len(release.Spec.Snapshot) > 63 {
+		return nil, fmt.Errorf("snapshot name must be no more than 63 characters, got %d characters", len(release.Spec.Snapshot))
+	}
+
 	return nil, nil
 }
 

api/v1alpha1/webhooks/release/webhook_test.go

@@ -123,6 +123,39 @@ var _ = Describe("Release validation webhook", func() {
 			Expect(err.Error()).To(ContainSubstring("release name must be no more than 63 characters"))
 			Expect(warnings).To(BeEmpty())
 		})
+
+		It("should return an error when snapshot name is longer than 63 characters", func() {
+			release := &v1alpha1.Release{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-release",
+					Namespace: "default",
+				},
+				Spec: v1alpha1.ReleaseSpec{
+					Snapshot:    "this-is-a-very-long-snapshot-name-that-exceeds-sixty-three-chars",
+					ReleasePlan: "test-releaseplan",
+				},
+			}
+			warnings, err := webhook.ValidateCreate(context.TODO(), release)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("snapshot name must be no more than 63 characters"))
+			Expect(warnings).To(BeEmpty())
+		})
+
+		It("should not return an error when both release name and snapshot name are within 63 characters", func() {
+			release := &v1alpha1.Release{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-release",
+					Namespace: "default",
+				},
+				Spec: v1alpha1.ReleaseSpec{
+					Snapshot:    "test-snapshot",
+					ReleasePlan: "test-releaseplan",
+				},
+			}
+			warnings, err := webhook.ValidateCreate(context.TODO(), release)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(warnings).To(BeEmpty())
+		})
 	})
 
 	createResources = func() {

api/v1alpha1/webhooks/releaseplan/suite_test.go

@@ -59,6 +59,7 @@ var (
 	k8sClient client.Client
 	mgr       manager.Manager
 	testEnv   *envtest.Environment
+	webhook   *Webhook
 )
 
 func TestReleasePlanWebhook(t *testing.T) {
@@ -107,7 +108,8 @@ var _ = BeforeSuite(func() {
 	})
 	Expect(err).NotTo(HaveOccurred())
 
-	err = toolkit.SetupWebhooks(mgr, &Webhook{}, &author.Webhook{})
+	webhook = &Webhook{}
+	err = toolkit.SetupWebhooks(mgr, webhook, &author.Webhook{})
 	Expect(err).NotTo(HaveOccurred())
 
 	//+kubebuilder:scaffold:webhook

api/v1alpha1/webhooks/releaseplan/webhook.go

@@ -18,13 +18,15 @@ package releaseplan
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/go-logr/logr"
 	"github.com/konflux-ci/release-service/api/v1alpha1"
 	"github.com/konflux-ci/release-service/metadata"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 // Webhook describes the data structure for the bar webhook
@@ -49,6 +51,7 @@ func (w *Webhook) Default(ctx context.Context, obj runtime.Object) error {
 }
 
 // +kubebuilder:webhook:path=/mutate-appstudio-redhat-com-v1alpha1-releaseplan,mutating=true,failurePolicy=fail,sideEffects=None,groups=appstudio.redhat.com,resources=releaseplans,verbs=create,versions=v1alpha1,name=mreleaseplan.kb.io,admissionReviewVersions=v1
+// +kubebuilder:webhook:path=/validate-appstudio-redhat-com-v1alpha1-releaseplan,mutating=false,failurePolicy=fail,sideEffects=None,groups=appstudio.redhat.com,resources=releaseplans,verbs=create;update,versions=v1alpha1,name=vreleaseplan.kb.io,admissionReviewVersions=v1
 
 // Register registers the webhook with the passed manager and log.
 func (w *Webhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
@@ -58,5 +61,35 @@ func (w *Webhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&v1alpha1.ReleasePlan{}).
 		WithDefaulter(w).
+		WithValidator(w).
 		Complete()
 }
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
+func (w *Webhook) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
+	releasePlan := obj.(*v1alpha1.ReleasePlan)
+
+	// Validate that the Application field doesn't exceed Kubernetes label value limit (63 characters)
+	if len(releasePlan.Spec.Application) > 63 {
+		return nil, fmt.Errorf("application name must be no more than 63 characters, got %d characters", len(releasePlan.Spec.Application))
+	}
+
+	return nil, nil
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
+func (w *Webhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
+	releasePlan := newObj.(*v1alpha1.ReleasePlan)
+
+	// Validate that the Application field doesn't exceed Kubernetes label value limit (63 characters)
+	if len(releasePlan.Spec.Application) > 63 {
+		return nil, fmt.Errorf("application name must be no more than 63 characters, got %d characters", len(releasePlan.Spec.Application))
+	}
+
+	return nil, nil
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
+func (w *Webhook) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
+	return nil, nil
+}

api/v1alpha1/webhooks/releaseplan/webhook_test.go

@@ -105,4 +105,30 @@ var _ = Describe("ReleasePlan webhook", func() {
 			}, timeout).Should(BeTrue())
 		})
 	})
+
+	When("ValidateDelete method is called", func() {
+		It("should return nil", func() {
+			releasePlan := &v1alpha1.ReleasePlan{}
+			Expect(webhook.ValidateDelete(ctx, releasePlan)).To(BeNil())
+		})
+	})
+
+	When("a ReleasePlan is created with an application name longer than 63 characters", func() {
+		It("should be rejected", func() {
+			releasePlan.Spec.Application = "this-is-a-very-long-application-name-that-exceeds-sixty-three-chars"
+			err := k8sClient.Create(ctx, releasePlan)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("application name must be no more than 63 characters"))
+		})
+	})
+
+	When("a ReleasePlan is updated with an application name longer than 63 characters", func() {
+		It("should be rejected", func() {
+			Expect(k8sClient.Create(ctx, releasePlan)).Should(Succeed())
+			releasePlan.Spec.Application = "this-is-a-very-long-application-name-that-exceeds-sixty-three-chars"
+			err := k8sClient.Update(ctx, releasePlan)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("application name must be no more than 63 characters"))
+		})
+	})
 })

api/v1alpha1/webhooks/releaseplanadmission/webhook.go

@@ -69,11 +69,29 @@ func (w *Webhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (w *Webhook) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings admission.Warnings, err error) {
+	releasePlanAdmission := obj.(*v1alpha1.ReleasePlanAdmission)
+
+	// Validate that application names don't exceed Kubernetes label value limit (63 characters)
+	for _, app := range releasePlanAdmission.Spec.Applications {
+		if len(app) > 63 {
+			return nil, fmt.Errorf("application name '%s' must be no more than 63 characters, got %d characters", app, len(app))
+		}
+	}
+
 	return w.validateBlockReleasesLabel(obj)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
 func (w *Webhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
+	releasePlanAdmission := newObj.(*v1alpha1.ReleasePlanAdmission)
+
+	// Validate that application names don't exceed Kubernetes label value limit (63 characters)
+	for _, app := range releasePlanAdmission.Spec.Applications {
+		if len(app) > 63 {
+			return nil, fmt.Errorf("application name '%s' must be no more than 63 characters, got %d characters", app, len(app))
+		}
+	}
+
 	return w.validateBlockReleasesLabel(newObj)
 }
 

api/v1alpha1/webhooks/releaseplanadmission/webhook_test.go

@@ -143,4 +143,30 @@ var _ = Describe("ReleasePlanAdmission webhook", func() {
 			Expect(webhook.ValidateDelete(ctx, releasePlanAdmission)).To(BeNil())
 		})
 	})
+
+	When("a ReleasePlanAdmission is created with an application name longer than 63 characters", func() {
+		It("should be rejected", func() {
+			releasePlanAdmission.Spec.Applications = []string{
+				"valid-app",
+				"this-is-a-very-long-application-name-that-exceeds-sixty-three-chars",
+			}
+			err := k8sClient.Create(ctx, releasePlanAdmission)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("application name"))
+			Expect(err.Error()).To(ContainSubstring("must be no more than 63 characters"))
+		})
+	})
+
+	When("a ReleasePlanAdmission is updated with an application name longer than 63 characters", func() {
+		It("should be rejected", func() {
+			Expect(k8sClient.Create(ctx, releasePlanAdmission)).Should(Succeed())
+			releasePlanAdmission.Spec.Applications = []string{
+				"this-is-a-very-long-application-name-that-exceeds-sixty-three-chars",
+			}
+			err := k8sClient.Update(ctx, releasePlanAdmission)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("application name"))
+			Expect(err.Error()).To(ContainSubstring("must be no more than 63 characters"))
+		})
+	})
 })

config/webhook/manifests.yaml

@@ -108,6 +108,26 @@ webhooks:
     resources:
     - releases
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-appstudio-redhat-com-v1alpha1-releaseplan
+  failurePolicy: Fail
+  name: vreleaseplan.kb.io
+  rules:
+  - apiGroups:
+    - appstudio.redhat.com
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - releaseplans
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig: