Skip to content

Commit

Permalink
stop to generate crd webhooks patches and cainjetions for any CRD/API…
Browse files Browse the repository at this point in the history
… and projects without webhooks

At present, we scaffold config/crd/patches, kustomizations, and CA injections for every CRD, irrespective of whether webhooks are enabled in the project or not. However, these configurations are only relevant and valid if the project has webhooks.

Consequently, for projects without webhooks, this leads to failures as documented in kubebuilder pull request kubernetes-sigs#3585.

To address this, we are now introducing a test to ensure that projects without enabled webhooks function correctly and as anticipated.

Signed-off-by: Camila Macedo <[email protected]>
Co-authored-by: lowang-bh <[email protected]>

fix: just generate crd webhooks patches and ca injestions when webhooks are created for those
  • Loading branch information
lowang-bh authored and camilamacedo86 committed Oct 1, 2023
1 parent 87c2b2b commit fdfab55
Show file tree
Hide file tree
Showing 39 changed files with 153 additions and 479 deletions.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ patches:
- 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
7 changes: 0 additions & 7 deletions pkg/plugins/common/kustomize/v2/scaffolds/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@ import (
"fmt"

log "github.com/sirupsen/logrus"

"sigs.k8s.io/kubebuilder/v3/pkg/config"
"sigs.k8s.io/kubebuilder/v3/pkg/machinery"
"sigs.k8s.io/kubebuilder/v3/pkg/model/resource"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/patches"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/rbac"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/samples"
)
Expand Down Expand Up @@ -76,10 +73,6 @@ func (s *apiScaffolder) Scaffold() error {
&samples.CRDSample{Force: s.force},
&rbac.CRDEditorRole{},
&rbac.CRDViewerRole{},
&patches.EnableWebhookPatch{},
&patches.EnableCAInjectionPatch{},
&crd.Kustomization{},
&crd.KustomizeConfig{},
); err != nil {
return fmt.Errorf("error scaffolding kustomize API manifests: %v", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ patches:
# patches here are for enabling the CA injection for each CRD
%s
# [WEBHOOK] To enable webhook, uncomment the following section
# the following config is for teaching kustomize how to do kustomization for CRDs.
configurations:
- kustomizeconfig.yaml
`
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,15 @@ nameReference:
version: v1
fieldSpecs:
- kind: CustomResourceDefinition
version: {{ .Resource.API.CRDVersion }}
version: v1
group: apiextensions.k8s.io
{{- if ne .Resource.API.CRDVersion "v1" }}
path: spec/conversion/webhookClientConfig/service/name
{{- else }}
path: spec/conversion/webhook/clientConfig/service/name
{{- end }}
namespace:
- kind: CustomResourceDefinition
version: {{ .Resource.API.CRDVersion }}
version: v1
group: apiextensions.k8s.io
{{- if ne .Resource.API.CRDVersion "v1" }}
path: spec/conversion/webhookClientConfig/service/namespace
{{- else }}
path: spec/conversion/webhook/clientConfig/service/namespace
{{- end }}
create: false
varReference:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ func (f *EnableCAInjectionPatch) SetTemplateDefaults() error {

//nolint:lll
const enableCAInjectionPatchTemplate = `# The following patch adds a directive for certmanager to inject CA into the CRD
{{- if ne .Resource.API.CRDVersion "v1" }}
# CRD conversion requires k8s 1.13 or later.
{{- end }}
apiVersion: apiextensions.k8s.io/{{ .Resource.API.CRDVersion }}
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,30 +49,19 @@ func (f *EnableWebhookPatch) SetTemplateDefaults() error {
}

const enableWebhookPatchTemplate = `# The following patch enables a conversion webhook for the CRD
{{- if ne .Resource.API.CRDVersion "v1" }}
# CRD conversion requires k8s 1.13 or later.
{{- end }}
apiVersion: apiextensions.k8s.io/{{ .Resource.API.CRDVersion }}
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: {{ .Resource.Plural }}.{{ .Resource.QualifiedGroup }}
spec:
conversion:
strategy: Webhook
{{- if ne .Resource.API.CRDVersion "v1" }}
webhookClientConfig:
service:
namespace: system
name: webhook-service
path: /convert
{{- else }}
webhook:
clientConfig:
service:
namespace: system
name: webhook-service
path: /convert
conversionReviewVersions:
- {{ .Resource.API.CRDVersion }}
{{- end }}
- v1
`
35 changes: 20 additions & 15 deletions pkg/plugins/common/kustomize/v2/scaffolds/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ package scaffolds
import (
"fmt"

pluginutil "sigs.k8s.io/kubebuilder/v3/pkg/plugin/util"

log "github.com/sirupsen/logrus"
pluginutil "sigs.k8s.io/kubebuilder/v3/pkg/plugin/util"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/patches"

"sigs.k8s.io/kubebuilder/v3/pkg/config"
"sigs.k8s.io/kubebuilder/v3/pkg/machinery"
Expand Down Expand Up @@ -71,6 +72,23 @@ func (s *webhookScaffolder) Scaffold() error {
return fmt.Errorf("error updating resource: %w", err)
}

if err := scaffold.Execute(
&kdefault.WebhookCAInjectionPatch{},
&kdefault.ManagerWebhookPatch{},
&webhook.Kustomization{Force: s.force},
&webhook.KustomizeConfig{},
&webhook.Service{},
&certmanager.Certificate{},
&certmanager.Kustomization{},
&certmanager.KustomizeConfig{},
&patches.EnableWebhookPatch{},
&patches.EnableCAInjectionPatch{},
&crd.Kustomization{},
&crd.KustomizeConfig{},
); err != nil {
return fmt.Errorf("error scaffolding kustomize webhook manifests: %v", err)
}

kustomizeFilePath := "config/default/kustomization.yaml"
err := pluginutil.UncommentCode(kustomizeFilePath, "#- ../webhook", `#`)
if err != nil {
Expand Down Expand Up @@ -100,18 +118,5 @@ func (s *webhookScaffolder) Scaffold() error {
}
}

if err := scaffold.Execute(
&kdefault.WebhookCAInjectionPatch{},
&kdefault.ManagerWebhookPatch{},
&webhook.Kustomization{Force: s.force},
&webhook.KustomizeConfig{},
&webhook.Service{},
&certmanager.Certificate{},
&certmanager.Kustomization{},
&certmanager.KustomizeConfig{},
); err != nil {
return fmt.Errorf("error scaffolding kustomize webhook manifests: %v", err)
}

return nil
}
101 changes: 62 additions & 39 deletions test/e2e/v4/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,63 @@ import (
"sigs.k8s.io/kubebuilder/v3/test/e2e/utils"
)

// GenerateV4 implements a go/v4(-alpha) plugin project defined by a TestContext.
// GenerateV4 implements a go/v4 plugin project defined by a TestContext.
func GenerateV4(kbc *utils.TestContext) {
var err error
initingTheProject(kbc)
creatingAPI(kbc)

By("initializing a project")
err = kbc.Init(
"--plugins", "go/v4",
"--project-version", "3",
"--domain", kbc.Domain,
By("scaffolding mutating and validating webhooks")
err := kbc.CreateWebhook(
"--group", kbc.Group,
"--version", kbc.Version,
"--kind", kbc.Kind,
"--defaulting",
"--programmatic-validation",
)
ExpectWithOffset(1, err).NotTo(HaveOccurred())

By("implementing the mutating and validating webhooks")
err = pluginutil.ImplementWebhooks(filepath.Join(
kbc.Dir, "api", kbc.Version,
fmt.Sprintf("%s_webhook.go", strings.ToLower(kbc.Kind))))
ExpectWithOffset(1, err).NotTo(HaveOccurred())

ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- ../certmanager", "#")).To(Succeed())
ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- ../prometheus", "#")).To(Succeed())
ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- webhookcainjection_patch.yaml", "#")).To(Succeed())
ExpectWithOffset(1, pluginutil.UncommentCode(filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
certManagerTarget, "#")).To(Succeed())

if kbc.IsRestricted {
By("uncomment kustomize files to ensure that pods are restricted")
uncommentPodStandards(kbc)
}
}

// GenerateV4WithoutWebhooks implements a go/v4 plugin with APIs and enable Prometheus and CertManager
func GenerateV4WithoutWebhooks(kbc *utils.TestContext) {
initingTheProject(kbc)
creatingAPI(kbc)

ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- ../prometheus", "#")).To(Succeed())

if kbc.IsRestricted {
By("uncomment kustomize files to ensure that pods are restricted")
uncommentPodStandards(kbc)
}
}

func creatingAPI(kbc *utils.TestContext) {
By("creating API definition")
err = kbc.CreateAPI(
err := kbc.CreateAPI(
"--group", kbc.Group,
"--version", kbc.Version,
"--kind", kbc.Kind,
Expand All @@ -65,34 +108,20 @@ func GenerateV4(kbc *utils.TestContext) {
` // +optional
Count int `+"`"+`json:"count,omitempty"`+"`"+`
`)).Should(Succeed())
}

By("scaffolding mutating and validating webhooks")
err = kbc.CreateWebhook(
"--group", kbc.Group,
"--version", kbc.Version,
"--kind", kbc.Kind,
"--defaulting",
"--programmatic-validation",
func initingTheProject(kbc *utils.TestContext) {
By("initializing a project")
err := kbc.Init(
"--plugins", "go/v4",
"--project-version", "3",
"--domain", kbc.Domain,
)
ExpectWithOffset(1, err).NotTo(HaveOccurred())
}

By("implementing the mutating and validating webhooks")
err = pluginutil.ImplementWebhooks(filepath.Join(
kbc.Dir, "api", kbc.Version,
fmt.Sprintf("%s_webhook.go", strings.ToLower(kbc.Kind))))
ExpectWithOffset(1, err).NotTo(HaveOccurred())

ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- ../certmanager", "#")).To(Succeed())
ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- ../prometheus", "#")).To(Succeed())
ExpectWithOffset(1, pluginutil.UncommentCode(
filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
"#- webhookcainjection_patch.yaml", "#")).To(Succeed())
ExpectWithOffset(1, pluginutil.UncommentCode(filepath.Join(kbc.Dir, "config", "default", "kustomization.yaml"),
`#replacements:
//nolint:lll
const certManagerTarget = `#replacements:
# - source: # Add cert-manager annotation to ValidatingWebhookConfiguration, MutatingWebhookConfiguration and CRDs
# kind: Certificate
# group: cert-manager.io
Expand Down Expand Up @@ -188,13 +217,7 @@ Count int `+"`"+`json:"count,omitempty"`+"`"+`
# options:
# delimiter: '.'
# index: 1
# create: true`, "#")).To(Succeed())

if kbc.IsRestricted {
By("uncomment kustomize files to ensure that pods are restricted")
uncommentPodStandards(kbc)
}
}
# create: true`

func uncommentPodStandards(kbc *utils.TestContext) {
configManager := filepath.Join(kbc.Dir, "config", "manager", "manager.yaml")
Expand Down
Loading

0 comments on commit fdfab55

Please sign in to comment.