Merge pull request #4280 from camilamacedo86/fix-conversion-webhooks

🐛 (kustomize/v2, go/v4): Fix incorrect generation of manifests under config/crd/patches. Previously, the /convert service patch was being generated for all webhooks instead of only for those with --conversion enabled.

docs/book/src/cronjob-tutorial/testdata/project/config/crd/kustomization.yaml

@@ -8,16 +8,13 @@ resources:
 patches:
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
 # patches here are for enabling the conversion webhook for each CRD
-- path: patches/webhook_in_cronjobs.yaml
 # +kubebuilder:scaffold:crdkustomizewebhookpatch
 
 # [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
 # patches here are for enabling the CA injection for each CRD
-- path: patches/cainjection_in_cronjobs.yaml
 # +kubebuilder:scaffold:crdkustomizecainjectionpatch
 
 # [WEBHOOK] To enable webhook, uncomment the following section
 # the following config is for teaching kustomize how to do kustomization for CRDs.
-
-configurations:
-- kustomizeconfig.yaml
+#configurations:
+#- kustomizeconfig.yaml

docs/book/src/cronjob-tutorial/testdata/project/dist/install.yaml

@@ -14,16 +14,6 @@ metadata:
     controller-gen.kubebuilder.io/version: v0.16.4
   name: cronjobs.batch.tutorial.kubebuilder.io
 spec:
-  conversion:
-    strategy: Webhook
-    webhook:
-      clientConfig:
-        service:
-          name: project-webhook-service
-          namespace: project-system
-          path: /convert
-      conversionReviewVersions:
-      - v1
   group: batch.tutorial.kubebuilder.io
   names:
     kind: CronJob

docs/book/src/getting-started/testdata/project/config/crd/kustomization.yaml

@@ -12,11 +12,9 @@ patches:
 
 # [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
 # patches here are for enabling the CA injection for each CRD
-#- path: patches/cainjection_in_memcacheds.yaml
 # +kubebuilder:scaffold:crdkustomizecainjectionpatch
 
 # [WEBHOOK] To enable webhook, uncomment the following section
 # the following config is for teaching kustomize how to do kustomization for CRDs.
-
 #configurations:
 #- kustomizeconfig.yaml

docs/book/src/multiversion-tutorial/testdata/project/config/crd/kustomization.yaml

@@ -9,17 +9,14 @@ patches:
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
 # patches here are for enabling the conversion webhook for each CRD
 - path: patches/webhook_in_cronjobs.yaml
-- path: patches/webhook_in_cronjobs.yaml
 # +kubebuilder:scaffold:crdkustomizewebhookpatch
 
 # [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
 # patches here are for enabling the CA injection for each CRD
-- path: patches/cainjection_in_cronjobs.yaml
 #- path: patches/cainjection_in_cronjobs.yaml
 # +kubebuilder:scaffold:crdkustomizecainjectionpatch
 
 # [WEBHOOK] To enable webhook, uncomment the following section
 # the following config is for teaching kustomize how to do kustomization for CRDs.
-
 configurations:
 - kustomizeconfig.yaml

hack/docs/internal/cronjob-tutorial/generate_cronjob.go

@@ -595,11 +595,6 @@ func (sp *Sample) updateKustomization() {
 		filepath.Join(sp.ctx.Dir, "config/default/kustomization.yaml"),
 		certmanagerForWebhooks, `#`)
 	hackutils.CheckError("fixing default/kustomization", err)
-
-	err = pluginutil.UncommentCode(
-		filepath.Join(sp.ctx.Dir, "config/crd/kustomization.yaml"),
-		`#- path: patches/cainjection_in_cronjobs.yaml`, `#`)
-	hackutils.CheckError("fixing crd/kustomization", err)
 }
 
 func (sp *Sample) updateExample() {

pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/kustomization.go

@@ -70,7 +70,7 @@ func (f *Kustomization) GetMarkers() []machinery.Marker {
 const (
 	resourceCodeFragment = `- bases/%s_%s.yaml
 `
-	webhookPatchCodeFragment = `#- path: patches/webhook_in_%s.yaml
+	webhookPatchCodeFragment = `- path: patches/webhook_in_%s.yaml
 `
 	caInjectionPatchCodeFragment = `#- path: patches/cainjection_in_%s.yaml
 `
@@ -89,7 +89,7 @@ func (f *Kustomization) GetCodeFragments() machinery.CodeFragmentsMap {
 		suffix = f.Resource.Group + "_" + f.Resource.Plural
 	}
 
-	if !f.Resource.Webhooks.IsEmpty() {
+	if !f.Resource.Webhooks.IsEmpty() && f.Resource.Webhooks.Conversion {
 		webhookPatch := fmt.Sprintf(webhookPatchCodeFragment, suffix)
 
 		marker := machinery.NewMarkerFor(f.Path, webhookPatchMarker)
@@ -100,7 +100,9 @@ func (f *Kustomization) GetCodeFragments() machinery.CodeFragmentsMap {
 
 	// Generate resource code fragments
 	caInjectionPatch := make([]string, 0)
-	caInjectionPatch = append(caInjectionPatch, fmt.Sprintf(caInjectionPatchCodeFragment, suffix))
+	if !f.Resource.Webhooks.IsEmpty() && f.Resource.Webhooks.Conversion {
+		caInjectionPatch = append(caInjectionPatch, fmt.Sprintf(caInjectionPatchCodeFragment, suffix))
+	}
 
 	// Only store code fragments in the map if the slices are non-empty
 	if len(res) != 0 {
@@ -131,7 +133,6 @@ patches:
 
 # [WEBHOOK] To enable webhook, uncomment the following section
 # the following config is for teaching kustomize how to do kustomization for CRDs.
-
 #configurations:
 #- kustomizeconfig.yaml
 `

pkg/plugins/common/kustomize/v2/scaffolds/webhook.go

@@ -19,16 +19,16 @@ package scaffolds
 import (
 	"fmt"
 
-	log "github.com/sirupsen/logrus"
-	pluginutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util"
-	"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"
 	"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/patches"
 
+	log "github.com/sirupsen/logrus"
 	"sigs.k8s.io/kubebuilder/v4/pkg/config"
 	"sigs.k8s.io/kubebuilder/v4/pkg/machinery"
 	"sigs.k8s.io/kubebuilder/v4/pkg/model/resource"
+	pluginutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util"
 	"sigs.k8s.io/kubebuilder/v4/pkg/plugins"
 	"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/certmanager"
+	"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"
 	"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault"
 	network_policy "sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/network-policy"
 	"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/webhook"
@@ -81,11 +81,15 @@ func (s *webhookScaffolder) Scaffold() error {
 		&certmanager.Certificate{},
 		&certmanager.Kustomization{},
 		&certmanager.KustomizeConfig{},
-		&patches.EnableWebhookPatch{},
-		&patches.EnableCAInjectionPatch{},
 		&network_policy.NetworkPolicyAllowWebhooks{},
 	}
 
+	// Only scaffold the following patches if is a conversion webhook
+	if s.resource.Webhooks.Conversion {
+		buildScaffold = append(buildScaffold, &patches.EnableWebhookPatch{})
+		buildScaffold = append(buildScaffold, &patches.EnableCAInjectionPatch{})
+	}
+
 	if !s.resource.External && !s.resource.Core {
 		buildScaffold = append(buildScaffold, &crd.Kustomization{})
 	}
@@ -130,22 +134,17 @@ func (s *webhookScaffolder) Scaffold() error {
 		}
 	}
 
-	crdKustomizationsFilePath := "config/crd/kustomization.yaml"
-	err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#- path: patches/webhook", `#`)
-	if err != nil {
-		hasWebHookUncommented, err := pluginutil.HasFileContentWith(crdKustomizationsFilePath, "- path: patches/webhook")
-		if !hasWebHookUncommented || err != nil {
-			log.Errorf("Unable to find the target(s) #- path: patches/webhook/* to uncomment in the file "+
-				"%s.", crdKustomizationsFilePath)
-		}
-	}
-
-	err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#configurations:\n#- kustomizeconfig.yaml", `#`)
-	if err != nil {
-		hasWebHookUncommented, err := pluginutil.HasFileContentWith(crdKustomizationsFilePath, "- kustomizeconfig.yaml")
-		if !hasWebHookUncommented || err != nil {
-			log.Errorf("Unable to find the target(s) #configurations:\n#- kustomizeconfig.yaml to uncomment in the file "+
-				"%s.", crdKustomizationsFilePath)
+	if s.resource.Webhooks.Conversion {
+		crdKustomizationsFilePath := "config/crd/kustomization.yaml"
+		err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#configurations:\n#- kustomizeconfig.yaml", `#`)
+		if err != nil {
+			hasWebHookUncommented, err := pluginutil.HasFileContentWith(crdKustomizationsFilePath,
+				"configurations:\n- kustomizeconfig.yaml")
+			if !hasWebHookUncommented || err != nil {
+				log.Warningf("Unable to find the target(s) configurations.kustomizeconfig.yaml "+
+					"to uncomment in the file "+
+					"%s.", crdKustomizationsFilePath)
+			}
 		}
 	}
 

testdata/project-v4-multigroup/config/crd/kustomization.yaml

@@ -19,31 +19,15 @@ resources:
 patches:
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
 # patches here are for enabling the conversion webhook for each CRD
-- path: patches/webhook_in_crew_captains.yaml
-- path: patches/webhook_in_ship_destroyers.yaml
-- path: patches/webhook_in_ship_cruisers.yaml
-- path: patches/webhook_in_example.com_memcacheds.yaml
 - path: patches/webhook_in_example.com_wordpresses.yaml
 # +kubebuilder:scaffold:crdkustomizewebhookpatch
 
 # [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
 # patches here are for enabling the CA injection for each CRD
-#- path: patches/cainjection_in_crew_captains.yaml
-#- path: patches/cainjection_in_ship_frigates.yaml
-#- path: patches/cainjection_in_ship_destroyers.yaml
-#- path: patches/cainjection_in_ship_cruisers.yaml
-#- path: patches/cainjection_in_sea-creatures_krakens.yaml
-#- path: patches/cainjection_in_sea-creatures_leviathans.yaml
-#- path: patches/cainjection_in_foo.policy_healthcheckpolicies.yaml
-#- path: patches/cainjection_in_foo_bars.yaml
-#- path: patches/cainjection_in_fiz_bars.yaml
-#- path: patches/cainjection_in_example.com_memcacheds.yaml
-#- path: patches/cainjection_in_example.com_busyboxes.yaml
 #- path: patches/cainjection_in_example.com_wordpresses.yaml
 # +kubebuilder:scaffold:crdkustomizecainjectionpatch
 
 # [WEBHOOK] To enable webhook, uncomment the following section
 # the following config is for teaching kustomize how to do kustomization for CRDs.
-
 configurations:
 - kustomizeconfig.yaml