Skip to content

Commit 5f8342e

Browse files
k8s-ci-robotk8s-ci-robot
authored
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.
2 parents 4852a8d + 01ff68b commit 5f8342e

37 files changed

+27
-421
lines changed

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@ resources:
88
patches:
99
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
1010
# patches here are for enabling the conversion webhook for each CRD
11-
- path: patches/webhook_in_cronjobs.yaml
1211
# +kubebuilder:scaffold:crdkustomizewebhookpatch
1312

1413
# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
1514
# patches here are for enabling the CA injection for each CRD
16-
- path: patches/cainjection_in_cronjobs.yaml
1715
# +kubebuilder:scaffold:crdkustomizecainjectionpatch
1816

1917
# [WEBHOOK] To enable webhook, uncomment the following section
2018
# the following config is for teaching kustomize how to do kustomization for CRDs.
21-
22-
configurations:
23-
- kustomizeconfig.yaml
19+
#configurations:
20+
#- kustomizeconfig.yaml

‎docs/book/src/cronjob-tutorial/testdata/project/config/crd/patches/cainjection_in_cronjobs.yaml

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎docs/book/src/cronjob-tutorial/testdata/project/config/crd/patches/webhook_in_cronjobs.yaml

Lines changed: 0 additions & 16 deletions
This file was deleted.

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,6 @@ metadata:
1414
controller-gen.kubebuilder.io/version: v0.16.4
1515
name: cronjobs.batch.tutorial.kubebuilder.io
1616
spec:
17-
conversion:
18-
strategy: Webhook
19-
webhook:
20-
clientConfig:
21-
service:
22-
name: project-webhook-service
23-
namespace: project-system
24-
path: /convert
25-
conversionReviewVersions:
26-
- v1
2717
group: batch.tutorial.kubebuilder.io
2818
names:
2919
kind: CronJob

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ patches:
1212

1313
# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
1414
# patches here are for enabling the CA injection for each CRD
15-
#- path: patches/cainjection_in_memcacheds.yaml
1615
# +kubebuilder:scaffold:crdkustomizecainjectionpatch
1716

1817
# [WEBHOOK] To enable webhook, uncomment the following section
1918
# the following config is for teaching kustomize how to do kustomization for CRDs.
20-
2119
#configurations:
2220
#- kustomizeconfig.yaml

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,14 @@ patches:
99
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
1010
# patches here are for enabling the conversion webhook for each CRD
1111
- path: patches/webhook_in_cronjobs.yaml
12-
- path: patches/webhook_in_cronjobs.yaml
1312
# +kubebuilder:scaffold:crdkustomizewebhookpatch
1413

1514
# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
1615
# patches here are for enabling the CA injection for each CRD
17-
- path: patches/cainjection_in_cronjobs.yaml
1816
#- path: patches/cainjection_in_cronjobs.yaml
1917
# +kubebuilder:scaffold:crdkustomizecainjectionpatch
2018

2119
# [WEBHOOK] To enable webhook, uncomment the following section
2220
# the following config is for teaching kustomize how to do kustomization for CRDs.
23-
2421
configurations:
2522
- kustomizeconfig.yaml

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,11 +595,6 @@ func (sp *Sample) updateKustomization() {
595595
filepath.Join(sp.ctx.Dir, "config/default/kustomization.yaml"),
596596
certmanagerForWebhooks, `#`)
597597
hackutils.CheckError("fixing default/kustomization", err)
598-
599-
err = pluginutil.UncommentCode(
600-
filepath.Join(sp.ctx.Dir, "config/crd/kustomization.yaml"),
601-
`#- path: patches/cainjection_in_cronjobs.yaml`, `#`)
602-
hackutils.CheckError("fixing crd/kustomization", err)
603598
}
604599

605600
func (sp *Sample) updateExample() {

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (f *Kustomization) GetMarkers() []machinery.Marker {
7070
const (
7171
resourceCodeFragment = `- bases/%s_%s.yaml
7272
`
73-
webhookPatchCodeFragment = `#- path: patches/webhook_in_%s.yaml
73+
webhookPatchCodeFragment = `- path: patches/webhook_in_%s.yaml
7474
`
7575
caInjectionPatchCodeFragment = `#- path: patches/cainjection_in_%s.yaml
7676
`
@@ -89,7 +89,7 @@ func (f *Kustomization) GetCodeFragments() machinery.CodeFragmentsMap {
8989
suffix = f.Resource.Group + "_" + f.Resource.Plural
9090
}
9191

92-
if !f.Resource.Webhooks.IsEmpty() {
92+
if !f.Resource.Webhooks.IsEmpty() && f.Resource.Webhooks.Conversion {
9393
webhookPatch := fmt.Sprintf(webhookPatchCodeFragment, suffix)
9494

9595
marker := machinery.NewMarkerFor(f.Path, webhookPatchMarker)
@@ -100,7 +100,9 @@ func (f *Kustomization) GetCodeFragments() machinery.CodeFragmentsMap {
100100

101101
// Generate resource code fragments
102102
caInjectionPatch := make([]string, 0)
103-
caInjectionPatch = append(caInjectionPatch, fmt.Sprintf(caInjectionPatchCodeFragment, suffix))
103+
if !f.Resource.Webhooks.IsEmpty() && f.Resource.Webhooks.Conversion {
104+
caInjectionPatch = append(caInjectionPatch, fmt.Sprintf(caInjectionPatchCodeFragment, suffix))
105+
}
104106

105107
// Only store code fragments in the map if the slices are non-empty
106108
if len(res) != 0 {
@@ -131,7 +133,6 @@ patches:
131133
132134
# [WEBHOOK] To enable webhook, uncomment the following section
133135
# the following config is for teaching kustomize how to do kustomization for CRDs.
134-
135136
#configurations:
136137
#- kustomizeconfig.yaml
137138
`

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

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,16 @@ package scaffolds
1919
import (
2020
"fmt"
2121

22-
log "github.com/sirupsen/logrus"
23-
pluginutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util"
24-
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"
2522
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/patches"
2623

24+
log "github.com/sirupsen/logrus"
2725
"sigs.k8s.io/kubebuilder/v4/pkg/config"
2826
"sigs.k8s.io/kubebuilder/v4/pkg/machinery"
2927
"sigs.k8s.io/kubebuilder/v4/pkg/model/resource"
28+
pluginutil "sigs.k8s.io/kubebuilder/v4/pkg/plugin/util"
3029
"sigs.k8s.io/kubebuilder/v4/pkg/plugins"
3130
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/certmanager"
31+
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"
3232
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault"
3333
network_policy "sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/network-policy"
3434
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/webhook"
@@ -81,11 +81,15 @@ func (s *webhookScaffolder) Scaffold() error {
8181
&certmanager.Certificate{},
8282
&certmanager.Kustomization{},
8383
&certmanager.KustomizeConfig{},
84-
&patches.EnableWebhookPatch{},
85-
&patches.EnableCAInjectionPatch{},
8684
&network_policy.NetworkPolicyAllowWebhooks{},
8785
}
8886

87+
// Only scaffold the following patches if is a conversion webhook
88+
if s.resource.Webhooks.Conversion {
89+
buildScaffold = append(buildScaffold, &patches.EnableWebhookPatch{})
90+
buildScaffold = append(buildScaffold, &patches.EnableCAInjectionPatch{})
91+
}
92+
8993
if !s.resource.External && !s.resource.Core {
9094
buildScaffold = append(buildScaffold, &crd.Kustomization{})
9195
}
@@ -130,22 +134,17 @@ func (s *webhookScaffolder) Scaffold() error {
130134
}
131135
}
132136

133-
crdKustomizationsFilePath := "config/crd/kustomization.yaml"
134-
err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#- path: patches/webhook", `#`)
135-
if err != nil {
136-
hasWebHookUncommented, err := pluginutil.HasFileContentWith(crdKustomizationsFilePath, "- path: patches/webhook")
137-
if !hasWebHookUncommented || err != nil {
138-
log.Errorf("Unable to find the target(s) #- path: patches/webhook/* to uncomment in the file "+
139-
"%s.", crdKustomizationsFilePath)
140-
}
141-
}
142-
143-
err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#configurations:\n#- kustomizeconfig.yaml", `#`)
144-
if err != nil {
145-
hasWebHookUncommented, err := pluginutil.HasFileContentWith(crdKustomizationsFilePath, "- kustomizeconfig.yaml")
146-
if !hasWebHookUncommented || err != nil {
147-
log.Errorf("Unable to find the target(s) #configurations:\n#- kustomizeconfig.yaml to uncomment in the file "+
148-
"%s.", crdKustomizationsFilePath)
137+
if s.resource.Webhooks.Conversion {
138+
crdKustomizationsFilePath := "config/crd/kustomization.yaml"
139+
err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#configurations:\n#- kustomizeconfig.yaml", `#`)
140+
if err != nil {
141+
hasWebHookUncommented, err := pluginutil.HasFileContentWith(crdKustomizationsFilePath,
142+
"configurations:\n- kustomizeconfig.yaml")
143+
if !hasWebHookUncommented || err != nil {
144+
log.Warningf("Unable to find the target(s) configurations.kustomizeconfig.yaml "+
145+
"to uncomment in the file "+
146+
"%s.", crdKustomizationsFilePath)
147+
}
149148
}
150149
}
151150

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,31 +19,15 @@ resources:
1919
patches:
2020
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
2121
# patches here are for enabling the conversion webhook for each CRD
22-
- path: patches/webhook_in_crew_captains.yaml
23-
- path: patches/webhook_in_ship_destroyers.yaml
24-
- path: patches/webhook_in_ship_cruisers.yaml
25-
- path: patches/webhook_in_example.com_memcacheds.yaml
2622
- path: patches/webhook_in_example.com_wordpresses.yaml
2723
# +kubebuilder:scaffold:crdkustomizewebhookpatch
2824

2925
# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
3026
# patches here are for enabling the CA injection for each CRD
31-
#- path: patches/cainjection_in_crew_captains.yaml
32-
#- path: patches/cainjection_in_ship_frigates.yaml
33-
#- path: patches/cainjection_in_ship_destroyers.yaml
34-
#- path: patches/cainjection_in_ship_cruisers.yaml
35-
#- path: patches/cainjection_in_sea-creatures_krakens.yaml
36-
#- path: patches/cainjection_in_sea-creatures_leviathans.yaml
37-
#- path: patches/cainjection_in_foo.policy_healthcheckpolicies.yaml
38-
#- path: patches/cainjection_in_foo_bars.yaml
39-
#- path: patches/cainjection_in_fiz_bars.yaml
40-
#- path: patches/cainjection_in_example.com_memcacheds.yaml
41-
#- path: patches/cainjection_in_example.com_busyboxes.yaml
4227
#- path: patches/cainjection_in_example.com_wordpresses.yaml
4328
# +kubebuilder:scaffold:crdkustomizecainjectionpatch
4429

4530
# [WEBHOOK] To enable webhook, uncomment the following section
4631
# the following config is for teaching kustomize how to do kustomization for CRDs.
47-
4832
configurations:
4933
- kustomizeconfig.yaml

0 commit comments

Comments
 (0)