⚠️ (go/v4) decouple webhooks from APIs - Move Webhooks from `api/<ver…

…sion>` or `api/<group>/<version>` to `internal/webhook/<version>` or `internal/webhook/<group>/<version>`

This PR decouples the webhooks from the API, aligning with the recent breaking changes introduced in controller-runtime to ensure that kubebuilder still compatbile with its next release.  Webhooks are now scaffolded under `internal/webhook` to comply with the latest standards.

**Context:**

Controller-runtime deprecated and removed the webhook methods in favor of CustomInterfaces (see [controller-runtime#2641](kubernetes-sigs/controller-runtime#2641)). The motivation for this change is outlined in [controller-runtime#2596](kubernetes-sigs/controller-runtime#2596).

See that the current master branch already reflects these changes, using the CustomInterfaces: [kubebuilder#4060](kubernetes-sigs#4060).

**Changes:**

- Webhooks are now scaffolded in `internal/webhook/<version>` or `internal/webhook/<group>/<version>`.
- However, to ensure backwards compatibility, a new `--legacy` flag is introduced. Running `kubebuilder create webhook [options] --legacy` will scaffold webhooks in the legacy location for projects that need to retain the old structure. However, users will still to address the breaking changes in the source code by replacing the old methods by the new CustomInterfaces.

docs/book/src/cronjob-tutorial/testdata/project/Dockerfile

@@ -14,7 +14,7 @@ RUN go mod download
 # Copy the go source
 COPY cmd/main.go cmd/main.go
 COPY api/ api/
-COPY internal/controller/ internal/controller/
+COPY internal/ internal/
 
 # Build
 # the GOARCH has not a default value to allow the binary be built according to the host where the command

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

@@ -38,6 +38,7 @@ import (
 
 	batchv1 "tutorial.kubebuilder.io/project/api/v1"
 	"tutorial.kubebuilder.io/project/internal/controller"
+	webhookbatchv1 "tutorial.kubebuilder.io/project/internal/webhook/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 = webhookbatchv1.SetupCronJobWebhookWithManager(mgr); err != nil {
 			setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
 			os.Exit(1)
 		}

docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook.go → docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go

@@ -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,
@@ -81,7 +82,7 @@ This marker is responsible for generating a mutation webhook manifest.
 type CronJobCustomDefaulter struct {
 
 	// Default values for various CronJob fields
-	DefaultConcurrencyPolicy          ConcurrencyPolicy
+	DefaultConcurrencyPolicy          batchv1.ConcurrencyPolicy
 	DefaultSuspend                    bool
 	DefaultSuccessfulJobsHistoryLimit int32
 	DefaultFailedJobsHistoryLimit     int32
@@ -98,32 +99,34 @@ The `Default`method is expected to mutate the receiver, setting the defaults.
 
 // 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
 	}
 }
 
@@ -168,29 +171,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)
 	}
@@ -205,12 +208,13 @@ 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 {
@@ -219,7 +223,7 @@ func (r *CronJob) validateCronJob() error {
 
 	return apierrors.NewInvalid(
 		schema.GroupKind{Group: "batch.tutorial.kubebuilder.io", Kind: "CronJob"},
-		r.Name, allErrs)
+		cronjob.Name, allErrs)
 }
 
 /*
@@ -232,11 +236,11 @@ declaring validation by running `controller-gen crd -w`,
 or [here](/reference/markers/crd-validation.md).
 */
 
-func (r *CronJob) validateCronJobSpec() *field.Error {
+func validateCronJobSpec(cronjob *batchv1.CronJob) *field.Error {
 	// The field helpers from the kubernetes API machinery help us return nicely
 	// structured validation errors.
 	return validateScheduleFormat(
-		r.Spec.Schedule,
+		cronjob.Spec.Schedule,
 		field.NewPath("spec").Child("schedule"))
 }
 
@@ -261,15 +265,15 @@ 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 {
+func validateCronJobName(cronjob *batchv1.CronJob) *field.Error {
+	if len(cronjob.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")
+		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_test.go → docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.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/webhook_suite_test.go → docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/webhook_suite_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

docs/book/src/cronjob-tutorial/webhook-implementation.md

@@ -19,4 +19,4 @@ kubebuilder create webhook --group batch --version v1 --kind CronJob --defaultin
 
 This will scaffold the webhook functions and register your webhook with the manager in your `main.go` for you.
 
-{{#literatego ./testdata/project/api/v1/cronjob_webhook.go}}
+{{#literatego ./testdata/project/internal/webhook/v1/cronjob_webhook.go}}

docs/book/src/getting-started/testdata/project/Dockerfile

@@ -14,7 +14,7 @@ RUN go mod download
 # Copy the go source
 COPY cmd/main.go cmd/main.go
 COPY api/ api/
-COPY internal/controller/ internal/controller/
+COPY internal/ internal/
 
 # Build
 # the GOARCH has not a default value to allow the binary be built according to the host where the command

docs/book/src/multiversion-tutorial/testdata/project/Dockerfile

@@ -14,7 +14,7 @@ RUN go mod download
 # Copy the go source
 COPY cmd/main.go cmd/main.go
 COPY api/ api/
-COPY internal/controller/ internal/controller/
+COPY internal/ internal/
 
 # Build
 # the GOARCH has not a default value to allow the binary be built according to the host where the command

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

@@ -40,6 +40,8 @@ import (
 	batchv1 "tutorial.kubebuilder.io/project/api/v1"
 	batchv2 "tutorial.kubebuilder.io/project/api/v2"
 	"tutorial.kubebuilder.io/project/internal/controller"
+	webhookbatchv1 "tutorial.kubebuilder.io/project/internal/webhook/v1"
+	webhookbatchv2 "tutorial.kubebuilder.io/project/internal/webhook/v2"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -175,14 +177,14 @@ func main() {
 	*/
 	// nolint:goconst
 	if os.Getenv("ENABLE_WEBHOOKS") != "false" {
-		if err = (&batchv1.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
+		if err = webhookbatchv1.SetupCronJobWebhookWithManager(mgr); err != nil {
 			setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
 			os.Exit(1)
 		}
 	}
 	// nolint:goconst
 	if os.Getenv("ENABLE_WEBHOOKS") != "false" {
-		if err = (&batchv2.CronJob{}).SetupWebhookWithManager(mgr); err != nil {
+		if err = webhookbatchv2.SetupCronJobWebhookWithManager(mgr); err != nil {
 			setupLog.Error(err, "unable to create webhook", "webhook", "CronJob")
 			os.Exit(1)
 		}