Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 stop to generate crd webhooks patches and cainjetions for any CRD/API and projects without webhooks #3647

Merged
merged 1 commit into from
Oct 12, 2023
Merged

🐛 stop to generate crd webhooks patches and cainjetions for any CRD/API and projects without webhooks #3647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/book/src/component-config-tutorial/testdata/project/config/crd/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ patches:
#- path: patches/cainjection_in_projectconfigs.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
7 changes: 0 additions & 7 deletions ...nt-config-tutorial/testdata/project/config/crd/patches/cainjection_in_projectconfigs.yaml
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions ...ponent-config-tutorial/testdata/project/config/crd/patches/webhook_in_projectconfigs.yaml
View file
Open in desktop

This file was deleted.

2 changes: 2 additions & 0 deletions docs/book/src/cronjob-tutorial/testdata/project/config/crd/kustomization.yaml
View file
Open in desktop
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
6 changes: 1 addition & 5 deletions pkg/plugins/common/kustomize/v2/scaffolds/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ import (
"fmt"

pluginutil "sigs.k8s.io/kubebuilder/v3/pkg/plugin/util"
"sigs.k8s.io/kubebuilder/v3/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd"

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 @@ -78,8 +76,6 @@ func (s *apiScaffolder) Scaffold() error {
&samples.CRDSample{Force: s.force},
&rbac.CRDEditorRole{},
&rbac.CRDViewerRole{},
&patches.EnableWebhookPatch{},
&patches.EnableCAInjectionPatch{},
Copy link
Member Author

@camilamacedo86 camilamacedo86 Oct 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should only be generated with the scaffolding of a webhook. Hence, it's been moved to the webhook subcommand.

&crd.Kustomization{},
&crd.KustomizeConfig{},
); err != nil {
Expand Down
6 changes: 4 additions & 2 deletions pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/kustomization.go
View file
Open in desktop
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

#configurations:
#- kustomizeconfig.yaml
`
12 changes: 2 additions & 10 deletions pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/crd/kustomizeconfig.go
View file
Open in desktop
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 }}
Copy link
Member Author

@camilamacedo86 camilamacedo86 Oct 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRD v1beta1 is now unsupported. It's being removed as the subCommand webhook lacks the necessary implementation to retrieve it. Given that the related code has been unusable since k8s 1.22, there's no rationale to maintain it and modify the subCommand to extract values from resources in the PROJECT config.

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
5 changes: 1 addition & 4 deletions ...n/kustomize/v2/scaffolds/internal/templates/config/crd/patches/enablecainjection_patch.go
View file
Open in desktop
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
15 changes: 2 additions & 13 deletions ...ommon/kustomize/v2/scaffolds/internal/templates/config/crd/patches/enablewebhook_patch.go
View file
Open in desktop
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" }}
Copy link
Member Author

@camilamacedo86 camilamacedo86 Oct 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRD v1beat1 is no longer supported
We are removing because the subCommend webhook has not the implementation to return it. Due the fact of that the code under the condition cannot be used since k8s 1.22 make no sense keep it and change the subCommand to get the value from the resources tracked in the PROJECT config.

# 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 }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same of: https://github.com/kubernetes-sigs/kubebuilder/pull/3647/files#r1342193166

webhook:
clientConfig:
service:
namespace: system
name: webhook-service
path: /convert
conversionReviewVersions:
- {{ .Resource.API.CRDVersion }}
{{- end }}
- v1
`
37 changes: 24 additions & 13 deletions pkg/plugins/common/kustomize/v2/scaffolds/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ 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/patches"

"sigs.k8s.io/kubebuilder/v3/pkg/config"
"sigs.k8s.io/kubebuilder/v3/pkg/machinery"
Expand Down Expand Up @@ -71,6 +71,21 @@ 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{},
); err != nil {
return fmt.Errorf("error scaffolding kustomize webhook manifests: %v", err)
}
Copy link
Member Author

@camilamacedo86 camilamacedo86 Oct 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are just putting the execution here to occur before we start uncomment the pacthes and etc. We have not the scaffold to uncomment before the execution.

However the only chnage here is to add the following generations that should be made ONLY when we scaffold a webhook for the API and not for each API:

&patches.EnableWebhookPatch{},
&patches.EnableCAInjectionPatch{},


kustomizeFilePath := "config/default/kustomization.yaml"
err := pluginutil.UncommentCode(kustomizeFilePath, "#- ../webhook", `#`)
if err != nil {
Expand Down Expand Up @@ -100,17 +115,13 @@ 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)
err = pluginutil.UncommentCode(crdKustomizationsFilePath, "#configurations:\n#- kustomizeconfig.yaml", `#`)
if err != nil {
hasWebHookUncommented, err := pluginutil.HasFragment(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)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will uncomment the
configurations:

  • kustomizeconfig

When webhooks are scaffold
The kustomizeconfig has the patches under config/crd/patches which are relevant only when we scaffold webhooks for the CRD

}

return nil
Expand Down
101 changes: 62 additions & 39 deletions test/e2e/v4/generate_test.go
View file
Open in desktop
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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the e2e tests, we've made the following modifications:

  • Refactored the code for initializing and creating an API into a function, enabling reuse in both the existing and new tests.
  • Moved the code segment for uncommenting when webhooks/certmanager is disabled into a constant.
  • Introduced a new test that replicates all the standard checks (excluding webhook-specific ones) to validate that a project without enabled webhooks deploys and operates as intended.


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
Loading