⚠️ decouple webhooks from APIs

Author: camilamacedo86

docs/book/src/cronjob-tutorial/testdata/project/api/v1/zz_generated.deepcopy.go

@@ -38,6 +38,7 @@ import (
 
 	batchv1 "tutorial.kubebuilder.io/project/api/v1"
 	"tutorial.kubebuilder.io/project/internal/controller"
+	webhooksv1 "tutorial.kubebuilder.io/project/internal/webhooks/v1"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -183,7 +184,7 @@ func main() {
 	*/
 	// nolint:goconst
 	if os.Getenv("ENABLE_WEBHOOKS") != "false" {
-		if err = (&batchv1.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
+		if err = webhooksv1.SetupCronJobWebhookWithManager(mgr); err != nil {
 			setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
 			os.Exit(1)
 		}

docs/book/src/cronjob-tutorial/testdata/project/cmd/main.go

@@ -32,7 +32,7 @@ webhooks:
       namespace: system
       path: /mutate-batch-tutorial-kubebuilder-io-v1-cronjob
   failurePolicy: Fail
-  name: mcronjob.kb.io
+  name: mcronjob-v1.kb.io
   rules:
   - apiGroups:
     - batch.tutorial.kubebuilder.io
@@ -67,6 +67,7 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
+    - DELETE
     resources:
     - cronjobs
   sideEffects: None
@@ -78,7 +79,7 @@ webhooks:
       namespace: system
       path: /validate-batch-tutorial-kubebuilder-io-v1-cronjob
   failurePolicy: Fail
-  name: vcronjob.kb.io
+  name: vcronjob-v1.kb.io
   rules:
   - apiGroups:
     - batch.tutorial.kubebuilder.io
@@ -87,7 +88,6 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
-    - DELETE
     resources:
     - cronjobs
   sideEffects: None

docs/book/src/cronjob-tutorial/testdata/project/config/webhook/manifests.yaml

@@ -31,6 +31,8 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	batchv1 "tutorial.kubebuilder.io/project/api/v1"
 )
 
 // +kubebuilder:docs-gen:collapse=Go imports
@@ -45,13 +47,12 @@ var cronjoblog = logf.Log.WithName("cronjob-resource")
 Then, we set up the webhook with the manager.
 */
 
-// SetupWebhookWithManager will setup the manager to manage the webhooks.
-func (r *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(r).
+// SetupCronJobWebhookWithManager registers the webhook for CronJob in the manager.
+func SetupCronJobWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).For(&batchv1.CronJob{}).
 		WithValidator(&CronJobCustomValidator{}).
 		WithDefaulter(&CronJobCustomDefaulter{
-			DefaultConcurrencyPolicy:          AllowConcurrent,
+			DefaultConcurrencyPolicy:          batchv1.AllowConcurrent,
 			DefaultSuspend:                    false,
 			DefaultSuccessfulJobsHistoryLimit: 3,
 			DefaultFailedJobsHistoryLimit:     1,
@@ -66,7 +67,7 @@ This marker is responsible for generating a mutating webhook manifest.
 The meaning of each marker can be found [here](/reference/markers/webhook.md).
 */
 
-// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
+// +kubebuilder:webhook:path=/mutate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,verbs=create;update,versions=v1,name=mcronjob-v1.kb.io,sideEffects=None,admissionReviewVersions=v1
 
 /*
 We use the `webhook.CustomDefaulter` interface to set defaults to our CRD.
@@ -86,7 +87,7 @@ The `Default` method is expected to mutate the receiver, setting the defaults.
 type CronJobCustomDefaulter struct {
 
 	// Default values for various CronJob fields
-	DefaultConcurrencyPolicy          ConcurrencyPolicy
+	DefaultConcurrencyPolicy          batchv1.ConcurrencyPolicy
 	DefaultSuspend                    bool
 	DefaultSuccessfulJobsHistoryLimit int32
 	DefaultFailedJobsHistoryLimit     int32
@@ -96,40 +97,42 @@ var _ webhook.CustomDefaulter = &CronJobCustomDefaulter{}
 
 // Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind CronJob.
 func (d *CronJobCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
-	cronjob, ok := obj.(*CronJob)
+	cronjob, ok := obj.(*batchv1.CronJob)
+
 	if !ok {
 		return fmt.Errorf("expected an CronJob object but got %T", obj)
 	}
 	cronjoblog.Info("Defaulting for CronJob", "name", cronjob.GetName())
 
 	// Set default values
-	cronjob.Default()
-
+	d.applyDefaults(cronjob)
 	return nil
 }
 
-func (r *CronJob) Default() {
-	if r.Spec.ConcurrencyPolicy == "" {
-		r.Spec.ConcurrencyPolicy = AllowConcurrent
+// applyDefaults applies default values to CronJob fields.
+func (d *CronJobCustomDefaulter) applyDefaults(cronJob *batchv1.CronJob) {
+	if cronJob.Spec.ConcurrencyPolicy == "" {
+		cronJob.Spec.ConcurrencyPolicy = d.DefaultConcurrencyPolicy
 	}
-	if r.Spec.Suspend == nil {
-		r.Spec.Suspend = new(bool)
+	if cronJob.Spec.Suspend == nil {
+		cronJob.Spec.Suspend = new(bool)
+		*cronJob.Spec.Suspend = d.DefaultSuspend
 	}
-	if r.Spec.SuccessfulJobsHistoryLimit == nil {
-		r.Spec.SuccessfulJobsHistoryLimit = new(int32)
-		*r.Spec.SuccessfulJobsHistoryLimit = 3
+	if cronJob.Spec.SuccessfulJobsHistoryLimit == nil {
+		cronJob.Spec.SuccessfulJobsHistoryLimit = new(int32)
+		*cronJob.Spec.SuccessfulJobsHistoryLimit = d.DefaultSuccessfulJobsHistoryLimit
 	}
-	if r.Spec.FailedJobsHistoryLimit == nil {
-		r.Spec.FailedJobsHistoryLimit = new(int32)
-		*r.Spec.FailedJobsHistoryLimit = 1
+	if cronJob.Spec.FailedJobsHistoryLimit == nil {
+		cronJob.Spec.FailedJobsHistoryLimit = new(int32)
+		*cronJob.Spec.FailedJobsHistoryLimit = d.DefaultFailedJobsHistoryLimit
 	}
 }
 
 /*
 This marker is responsible for generating a validating webhook manifest.
 */
 
-// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,versions=v1,name=vcronjob.kb.io,sideEffects=None,admissionReviewVersions=v1
+// +kubebuilder:webhook:verbs=create;update;delete,path=/validate-batch-tutorial-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,groups=batch.tutorial.kubebuilder.io,resources=cronjobs,versions=v1,name=vcronjob-v1.kb.io,sideEffects=None,admissionReviewVersions=v1
 
 /*
 We can validate our CRD beyond what's possible with declarative
@@ -171,29 +174,29 @@ var _ webhook.CustomValidator = &CronJobCustomValidator{}
 
 // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
 func (v *CronJobCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
-	cronjob, ok := obj.(*CronJob)
+	cronjob, ok := obj.(*batchv1.CronJob)
 	if !ok {
 		return nil, fmt.Errorf("expected a CronJob object but got %T", obj)
 	}
 	cronjoblog.Info("Validation for CronJob upon creation", "name", cronjob.GetName())
 
-	return nil, cronjob.validateCronJob()
+	return nil, validateCronJob(cronjob)
 }
 
 // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
 func (v *CronJobCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
-	cronjob, ok := newObj.(*CronJob)
+	cronjob, ok := newObj.(*batchv1.CronJob)
 	if !ok {
-		return nil, fmt.Errorf("expected a CronJob object but got %T", newObj)
+		return nil, fmt.Errorf("expected a CronJob object for the newObj but got got %T", newObj)
 	}
 	cronjoblog.Info("Validation for CronJob upon update", "name", cronjob.GetName())
 
-	return nil, cronjob.validateCronJob()
+	return nil, validateCronJob(cronjob)
 }
 
 // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type CronJob.
 func (v *CronJobCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
-	cronjob, ok := obj.(*CronJob)
+	cronjob, ok := obj.(*batchv1.CronJob)
 	if !ok {
 		return nil, fmt.Errorf("expected a CronJob object but got %T", obj)
 	}
@@ -208,21 +211,19 @@ func (v *CronJobCustomValidator) ValidateDelete(ctx context.Context, obj runtime
 We validate the name and the spec of the CronJob.
 */
 
-func (r *CronJob) validateCronJob() error {
+// validateCronJob validates the fields of a CronJob object.
+func validateCronJob(cronjob *batchv1.CronJob) error {
 	var allErrs field.ErrorList
-	if err := r.validateCronJobName(); err != nil {
+	if err := validateCronJobName(cronjob); err != nil {
 		allErrs = append(allErrs, err)
 	}
-	if err := r.validateCronJobSpec(); err != nil {
+	if err := validateCronJobSpec(cronjob); err != nil {
 		allErrs = append(allErrs, err)
 	}
 	if len(allErrs) == 0 {
 		return nil
 	}
-
-	return apierrors.NewInvalid(
-		schema.GroupKind{Group: "batch.tutorial.kubebuilder.io", Kind: "CronJob"},
-		r.Name, allErrs)
+	return apierrors.NewInvalid(schema.GroupKind{Group: "batch.tutorial.kubebuilder.io", Kind: "CronJob"}, cronjob.Name, allErrs)
 }
 
 /*
@@ -235,19 +236,17 @@ declaring validation by running `controller-gen crd -w`,
 or [here](/reference/markers/crd-validation.md).
 */
 
-func (r *CronJob) validateCronJobSpec() *field.Error {
-	// The field helpers from the kubernetes API machinery help us return nicely
-	// structured validation errors.
-	return validateScheduleFormat(
-		r.Spec.Schedule,
-		field.NewPath("spec").Child("schedule"))
+// validateCronJobSpec validates the schedule field of the CronJob spec.
+func validateCronJobSpec(cronjob *batchv1.CronJob) *field.Error {
+	return validateScheduleFormat(cronjob.Spec.Schedule, field.NewPath("spec").Child("schedule"))
 }
 
 /*
 We'll need to validate the [cron](https://en.wikipedia.org/wiki/Cron) schedule
 is well-formatted.
 */
 
+// validateScheduleFormat validates that the schedule field follows the cron format.
 func validateScheduleFormat(schedule string, fldPath *field.Path) *field.Error {
 	if _, err := cron.ParseStandard(schedule); err != nil {
 		return field.Invalid(fldPath, schedule, err.Error())
@@ -264,15 +263,10 @@ the apimachinery repo, so we can't declaratively validate it using
 the validation schema.
 */
 
-func (r *CronJob) validateCronJobName() *field.Error {
-	if len(r.ObjectMeta.Name) > validationutils.DNS1035LabelMaxLength-11 {
-		// The job name length is 63 characters like all Kubernetes objects
-		// (which must fit in a DNS subdomain). The cronjob controller appends
-		// a 11-character suffix to the cronjob (`-$TIMESTAMP`) when creating
-		// a job. The job name length limit is 63 characters. Therefore cronjob
-		// names must have length <= 63-11=52. If we don't validate this here,
-		// then job creation will fail later.
-		return field.Invalid(field.NewPath("metadata").Child("name"), r.ObjectMeta.Name, "must be no more than 52 characters")
+// validateCronJobName validates the name of the CronJob object.
+func validateCronJobName(cronjob *batchv1.CronJob) *field.Error {
+	if len(cronjob.ObjectMeta.Name) > validationutils.DNS1035LabelMaxLength-11 {
+		return field.Invalid(field.NewPath("metadata").Child("name"), cronjob.ObjectMeta.Name, "must be no more than 52 characters")
 	}
 	return nil
 }

docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook.go renamed to docs/book/src/cronjob-tutorial/testdata/project/internal/webhooks/v1/cronjob_webhook.go

@@ -19,32 +19,35 @@ package v1
 import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+
+	batchv1 "tutorial.kubebuilder.io/project/api/v1"
 	// TODO (user): Add any additional imports if needed
 )
 
 var _ = Describe("CronJob Webhook", func() {
 	var (
-		obj       *CronJob
-		oldObj    *CronJob
+		obj       = &batchv1.CronJob{}
+		oldObj    = &batchv1.CronJob{}
 		validator CronJobCustomValidator
+		defaulter CronJobCustomDefaulter
 	)
 
 	BeforeEach(func() {
-		obj = &CronJob{
-			Spec: CronJobSpec{
+		obj = &batchv1.CronJob{
+			Spec: batchv1.CronJobSpec{
 				Schedule:                   "*/5 * * * *",
-				ConcurrencyPolicy:          AllowConcurrent,
+				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
 			},
 		}
 		*obj.Spec.SuccessfulJobsHistoryLimit = 3
 		*obj.Spec.FailedJobsHistoryLimit = 1
 
-		oldObj = &CronJob{
-			Spec: CronJobSpec{
+		oldObj = &batchv1.CronJob{
+			Spec: batchv1.CronJobSpec{
 				Schedule:                   "*/5 * * * *",
-				ConcurrencyPolicy:          AllowConcurrent,
+				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
 			},
@@ -53,6 +56,12 @@ var _ = Describe("CronJob Webhook", func() {
 		*oldObj.Spec.FailedJobsHistoryLimit = 1
 
 		validator = CronJobCustomValidator{}
+		defaulter = CronJobCustomDefaulter{
+			DefaultConcurrencyPolicy:          batchv1.AllowConcurrent,
+			DefaultSuspend:                    false,
+			DefaultSuccessfulJobsHistoryLimit: 3,
+			DefaultFailedJobsHistoryLimit:     1,
+		}
 
 		Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
 		Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized")
@@ -71,18 +80,18 @@ var _ = Describe("CronJob Webhook", func() {
 			obj.Spec.FailedJobsHistoryLimit = nil     // This should default to 1
 
 			By("calling the Default method to apply defaults")
-			obj.Default()
+			defaulter.Default(ctx, obj)
 
 			By("checking that the default values are set")
-			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
+			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
 			Expect(*obj.Spec.Suspend).To(BeFalse(), "Expected Suspend to default to false")
 			Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(3)), "Expected SuccessfulJobsHistoryLimit to default to 3")
 			Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(1)), "Expected FailedJobsHistoryLimit to default to 1")
 		})
 
 		It("Should not overwrite fields that are already set", func() {
 			By("setting fields that would normally get a default")
-			obj.Spec.ConcurrencyPolicy = ForbidConcurrent
+			obj.Spec.ConcurrencyPolicy = batchv1.ForbidConcurrent
 			obj.Spec.Suspend = new(bool)
 			*obj.Spec.Suspend = true
 			obj.Spec.SuccessfulJobsHistoryLimit = new(int32)
@@ -91,10 +100,10 @@ var _ = Describe("CronJob Webhook", func() {
 			*obj.Spec.FailedJobsHistoryLimit = 2
 
 			By("calling the Default method to apply defaults")
-			obj.Default()
+			defaulter.Default(ctx, obj)
 
 			By("checking that the fields were not overwritten")
-			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
+			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
 			Expect(*obj.Spec.Suspend).To(BeTrue(), "Expected Suspend to retain its set value")
 			Expect(*obj.Spec.SuccessfulJobsHistoryLimit).To(Equal(int32(5)), "Expected SuccessfulJobsHistoryLimit to retain its set value")
 			Expect(*obj.Spec.FailedJobsHistoryLimit).To(Equal(int32(2)), "Expected FailedJobsHistoryLimit to retain its set value")

docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook_test.go renamed to docs/book/src/cronjob-tutorial/testdata/project/internal/webhooks/v1/cronjob_webhook_test.go

@@ -89,7 +89,7 @@ var _ = BeforeSuite(func() {
 	Expect(cfg).NotTo(BeNil())
 
 	scheme := apimachineryruntime.NewScheme()
-	err = AddToScheme(scheme)
+	err = admissionv1.AddToScheme(scheme)
 	Expect(err).NotTo(HaveOccurred())
 
 	err = admissionv1.AddToScheme(scheme)
@@ -115,7 +115,7 @@ var _ = BeforeSuite(func() {
 	})
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&CronJob{}).SetupWebhookWithManager(mgr)
+	err = SetupCronJobWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
 	// +kubebuilder:scaffold:webhook