Merge pull request #4301 from damsien/fix-webhook-core-path

🐛 (go/v4): Fix path configuration for webhook markers generated for core types with non-"core" group values

Author: k8s-ci-robot

pkg/plugins/golang/v4/scaffolds/internal/templates/webhooks/webhook.go

@@ -158,7 +158,7 @@ func Setup{{ .Resource.Kind }}WebhookWithManager(mgr ctrl.Manager) error {
 
 	//nolint:lll
 	defaultingWebhookTemplate = `
-// +kubebuilder:webhook:{{ if ne .Resource.Webhooks.WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .Resource.Webhooks.WebhookVersion }}{{"}"}},{{ end }}path=/mutate-{{ if .Resource.Core }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }}{{ else }}{{ .QualifiedGroupWithDash }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }}{{ end }},mutating=true,failurePolicy=fail,sideEffects=None,groups={{ if .Resource.Core }}""{{ else }}{{ .Resource.QualifiedGroup }}{{ end }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=m{{ lower .Resource.Kind }}-{{ .Resource.Version }}.kb.io,admissionReviewVersions={{ .AdmissionReviewVersions }}
+// +kubebuilder:webhook:{{ if ne .Resource.Webhooks.WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .Resource.Webhooks.WebhookVersion }}{{"}"}},{{ end }}path=/mutate-{{ if and .Resource.Core (eq .Resource.QualifiedGroup "core") }}-{{ else }}{{ .QualifiedGroupWithDash }}-{{ end }}{{ .Resource.Version }}-{{ lower .Resource.Kind }},mutating=true,failurePolicy=fail,sideEffects=None,groups={{ if and .Resource.Core (eq .Resource.QualifiedGroup "core") }}""{{ else }}{{ .Resource.QualifiedGroup }}{{ end }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=m{{ lower .Resource.Kind }}-{{ .Resource.Version }}.kb.io,admissionReviewVersions={{ .AdmissionReviewVersions }}
 
 {{ if .IsLegacyPath -}}
 // +kubebuilder:object:generate=false
@@ -198,7 +198,7 @@ func (d *{{ .Resource.Kind }}CustomDefaulter) Default(ctx context.Context, obj r
 // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
 // NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
 // Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
-// +kubebuilder:webhook:{{ if ne .Resource.Webhooks.WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .Resource.Webhooks.WebhookVersion }}{{"}"}},{{ end }}path=/validate-{{ if .Resource.Core }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }}{{ else }}{{ .QualifiedGroupWithDash }}-{{ .Resource.Version }}-{{ lower .Resource.Kind }}{{ end }},mutating=false,failurePolicy=fail,sideEffects=None,groups={{ if .Resource.Core }}""{{ else }}{{ .Resource.QualifiedGroup }}{{ end }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=v{{ lower .Resource.Kind }}-{{ .Resource.Version }}.kb.io,admissionReviewVersions={{ .AdmissionReviewVersions }}
+// +kubebuilder:webhook:{{ if ne .Resource.Webhooks.WebhookVersion "v1" }}webhookVersions={{"{"}}{{ .Resource.Webhooks.WebhookVersion }}{{"}"}},{{ end }}path=/validate-{{ if and .Resource.Core (eq .Resource.QualifiedGroup "core") }}-{{ else }}{{ .QualifiedGroupWithDash }}-{{ end }}{{ .Resource.Version }}-{{ lower .Resource.Kind }},mutating=false,failurePolicy=fail,sideEffects=None,groups={{ if and .Resource.Core (eq .Resource.QualifiedGroup "core") }}""{{ else }}{{ .Resource.QualifiedGroup }}{{ end }},resources={{ .Resource.Plural }},verbs=create;update,versions={{ .Resource.Version }},name=v{{ lower .Resource.Kind }}-{{ .Resource.Version }}.kb.io,admissionReviewVersions={{ .AdmissionReviewVersions }}
 
 {{ if .IsLegacyPath -}}
 // +kubebuilder:object:generate=false

test/testdata/generate.sh

@@ -59,6 +59,8 @@ function scaffold_test_project {
     $kb create webhook --group "cert-manager" --version v1 --kind Issuer --defaulting --external-api-path=github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1 --external-api-domain=io
     # Webhook for Core type
     $kb create webhook --group core --version v1 --kind Pod --defaulting
+    # Webhook for kubernetes Core type that is part of an api group
+    $kb create webhook --group apps --version v1 --kind Deployment --defaulting --programmatic-validation
   fi
 
   if [[ $project =~ multigroup ]]; then
@@ -88,6 +90,8 @@ function scaffold_test_project {
     $kb create webhook --group "cert-manager" --version v1 --kind Issuer --defaulting --external-api-path=github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1 --external-api-domain=io
     # Webhook for Core type
     $kb create webhook --group core --version v1 --kind Pod --programmatic-validation --make=false
+    # Webhook for kubernetes Core type that is part of an api group
+    $kb create webhook --group apps --version v1 --kind Deployment --defaulting --programmatic-validation --make=false
   fi
 
   if [[ $project =~ multigroup ]] || [[ $project =~ with-plugins ]] ; then

testdata/project-v4-multigroup/PROJECT

@@ -105,6 +105,10 @@ resources:
   kind: Deployment
   path: k8s.io/api/apps/v1
   version: v1
+  webhooks:
+    defaulting: true
+    validation: true
+    webhookVersion: v1
 - api:
     crdVersion: v1
     namespaced: true

testdata/project-v4-multigroup/cmd/main.go

@@ -58,6 +58,7 @@ import (
 	foopolicycontroller "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/controller/foo.policy"
 	seacreaturescontroller "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/controller/sea-creatures"
 	shipcontroller "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/controller/ship"
+	webhookappsv1 "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/webhook/apps/v1"
 	webhookcertmanagerv1 "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/webhook/cert-manager/v1"
 	webhookcorev1 "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/webhook/core/v1"
 	webhookcrewv1 "sigs.k8s.io/kubebuilder/testdata/project-v4-multigroup/internal/webhook/crew/v1"
@@ -294,6 +295,13 @@ func main() {
 			os.Exit(1)
 		}
 	}
+	// nolint:goconst
+	if os.Getenv("ENABLE_WEBHOOKS") != "false" {
+		if err = webhookappsv1.SetupDeploymentWebhookWithManager(mgr); err != nil {
+			setupLog.Error(err, "unable to create webhook", "webhook", "Deployment")
+			os.Exit(1)
+		}
+	}
 	if err = (&examplecomcontroller.MemcachedReconciler{
 		Client:   mgr.GetClient(),
 		Scheme:   mgr.GetScheme(),

testdata/project-v4-multigroup/config/webhook/manifests.yaml

@@ -4,6 +4,26 @@ kind: MutatingWebhookConfiguration
 metadata:
   name: mutating-webhook-configuration
 webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-apps-v1-deployment
+  failurePolicy: Fail
+  name: mdeployment-v1.kb.io
+  rules:
+  - apiGroups:
+    - apps
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - deployments
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:
@@ -70,6 +90,26 @@ kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration
 webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-apps-v1-deployment
+  failurePolicy: Fail
+  name: vdeployment-v1.kb.io
+  rules:
+  - apiGroups:
+    - apps
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - deployments
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

testdata/project-v4-multigroup/dist/install.yaml

@@ -1919,6 +1919,26 @@ kind: MutatingWebhookConfiguration
 metadata:
   name: project-v4-multigroup-mutating-webhook-configuration
 webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: project-v4-multigroup-webhook-service
+      namespace: project-v4-multigroup-system
+      path: /mutate-apps-v1-deployment
+  failurePolicy: Fail
+  name: mdeployment-v1.kb.io
+  rules:
+  - apiGroups:
+    - apps
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - deployments
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:
@@ -1985,6 +2005,26 @@ kind: ValidatingWebhookConfiguration
 metadata:
   name: project-v4-multigroup-validating-webhook-configuration
 webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: project-v4-multigroup-webhook-service
+      namespace: project-v4-multigroup-system
+      path: /validate-apps-v1-deployment
+  failurePolicy: Fail
+  name: vdeployment-v1.kb.io
+  rules:
+  - apiGroups:
+    - apps
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - deployments
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

testdata/project-v4-multigroup/internal/webhook/apps/v1/deployment_webhook.go

@@ -0,0 +1,125 @@
+/*
+Copyright 2024 The Kubernetes authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"context"
+	"fmt"
+
+	appsv1 "k8s.io/api/apps/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var deploymentlog = logf.Log.WithName("deployment-resource")
+
+// SetupDeploymentWebhookWithManager registers the webhook for Deployment in the manager.
+func SetupDeploymentWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).For(&appsv1.Deployment{}).
+		WithValidator(&DeploymentCustomValidator{}).
+		WithDefaulter(&DeploymentCustomDefaulter{}).
+		Complete()
+}
+
+// TODO(user): EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
+
+// +kubebuilder:webhook:path=/mutate-apps-v1-deployment,mutating=true,failurePolicy=fail,sideEffects=None,groups=apps,resources=deployments,verbs=create;update,versions=v1,name=mdeployment-v1.kb.io,admissionReviewVersions=v1
+
+// DeploymentCustomDefaulter struct is responsible for setting default values on the custom resource of the
+// Kind Deployment when those are created or updated.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as it is used only for temporary operations and does not need to be deeply copied.
+type DeploymentCustomDefaulter struct {
+	// TODO(user): Add more fields as needed for defaulting
+}
+
+var _ webhook.CustomDefaulter = &DeploymentCustomDefaulter{}
+
+// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind Deployment.
+func (d *DeploymentCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+	deployment, ok := obj.(*appsv1.Deployment)
+
+	if !ok {
+		return fmt.Errorf("expected an Deployment object but got %T", obj)
+	}
+	deploymentlog.Info("Defaulting for Deployment", "name", deployment.GetName())
+
+	// TODO(user): fill in your defaulting logic.
+
+	return nil
+}
+
+// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-apps-v1-deployment,mutating=false,failurePolicy=fail,sideEffects=None,groups=apps,resources=deployments,verbs=create;update,versions=v1,name=vdeployment-v1.kb.io,admissionReviewVersions=v1
+
+// DeploymentCustomValidator struct is responsible for validating the Deployment resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type DeploymentCustomValidator struct {
+	//TODO(user): Add more fields as needed for validation
+}
+
+var _ webhook.CustomValidator = &DeploymentCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Deployment.
+func (v *DeploymentCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	deployment, ok := obj.(*appsv1.Deployment)
+	if !ok {
+		return nil, fmt.Errorf("expected a Deployment object but got %T", obj)
+	}
+	deploymentlog.Info("Validation for Deployment upon creation", "name", deployment.GetName())
+
+	// TODO(user): fill in your validation logic upon object creation.
+
+	return nil, nil
+}
+
+// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type Deployment.
+func (v *DeploymentCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+	deployment, ok := newObj.(*appsv1.Deployment)
+	if !ok {
+		return nil, fmt.Errorf("expected a Deployment object for the newObj but got %T", newObj)
+	}
+	deploymentlog.Info("Validation for Deployment upon update", "name", deployment.GetName())
+
+	// TODO(user): fill in your validation logic upon object update.
+
+	return nil, nil
+}
+
+// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type Deployment.
+func (v *DeploymentCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	deployment, ok := obj.(*appsv1.Deployment)
+	if !ok {
+		return nil, fmt.Errorf("expected a Deployment object but got %T", obj)
+	}
+	deploymentlog.Info("Validation for Deployment upon deletion", "name", deployment.GetName())
+
+	// TODO(user): fill in your validation logic upon object deletion.
+
+	return nil, nil
+}

testdata/project-v4-multigroup/internal/webhook/apps/v1/deployment_webhook_test.go

@@ -0,0 +1,87 @@
+/*
+Copyright 2024 The Kubernetes authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	appsv1 "k8s.io/api/apps/v1"
+	// TODO (user): Add any additional imports if needed
+)
+
+var _ = Describe("Deployment Webhook", func() {
+	var (
+		obj       *appsv1.Deployment
+		oldObj    *appsv1.Deployment
+		validator DeploymentCustomValidator
+		defaulter DeploymentCustomDefaulter
+	)
+
+	BeforeEach(func() {
+		obj = &appsv1.Deployment{}
+		oldObj = &appsv1.Deployment{}
+		validator = DeploymentCustomValidator{}
+		Expect(validator).NotTo(BeNil(), "Expected validator to be initialized")
+		defaulter = DeploymentCustomDefaulter{}
+		Expect(defaulter).NotTo(BeNil(), "Expected defaulter to be initialized")
+		Expect(oldObj).NotTo(BeNil(), "Expected oldObj to be initialized")
+		Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
+		// TODO (user): Add any setup logic common to all tests
+	})
+
+	AfterEach(func() {
+		// TODO (user): Add any teardown logic common to all tests
+	})
+
+	Context("When creating Deployment under Defaulting Webhook", func() {
+		// TODO (user): Add logic for defaulting webhooks
+		// Example:
+		// It("Should apply defaults when a required field is empty", func() {
+		//     By("simulating a scenario where defaults should be applied")
+		//     obj.SomeFieldWithDefault = ""
+		//     By("calling the Default method to apply defaults")
+		//     defaulter.Default(ctx, obj)
+		//     By("checking that the default values are set")
+		//     Expect(obj.SomeFieldWithDefault).To(Equal("default_value"))
+		// })
+	})
+
+	Context("When creating or updating Deployment under Validating Webhook", func() {
+		// TODO (user): Add logic for validating webhooks
+		// Example:
+		// It("Should deny creation if a required field is missing", func() {
+		//     By("simulating an invalid creation scenario")
+		//     obj.SomeRequiredField = ""
+		//     Expect(validator.ValidateCreate(ctx, obj)).Error().To(HaveOccurred())
+		// })
+		//
+		// It("Should admit creation if all required fields are present", func() {
+		//     By("simulating an invalid creation scenario")
+		//     obj.SomeRequiredField = "valid_value"
+		//     Expect(validator.ValidateCreate(ctx, obj)).To(BeNil())
+		// })
+		//
+		// It("Should validate updates correctly", func() {
+		//     By("simulating a valid update scenario")
+		//     oldObj.SomeRequiredField = "updated_value"
+		//     obj.SomeRequiredField = "updated_value"
+		//     Expect(validator.ValidateUpdate(ctx, oldObj, obj)).To(BeNil())
+		// })
+	})
+
+})