Skip to content

Commit

Permalink
Fix webhook core path scaffold bug
Browse files Browse the repository at this point in the history
  • Loading branch information
damsien committed Nov 7, 2024
1 parent 5c8befa commit 6fa2024
Show file tree
Hide file tree
Showing 17 changed files with 771 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Kubebuilder scaffolded a `internal/controller/suite_test.go` file that does the
First, it will contain the necessary imports.
*/


package controller

import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 .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 }}
{{ if .IsLegacyPath -}}
// +kubebuilder:object:generate=false
Expand Down Expand Up @@ -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 .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 }}
{{ if .IsLegacyPath -}}
// +kubebuilder:object:generate=false
Expand Down
4 changes: 4 additions & 0 deletions test/testdata/generate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 --make=false
fi

if [[ $project =~ multigroup ]]; then
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions testdata/project-v4-multigroup/PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions testdata/project-v4-multigroup/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand Down
40 changes: 40 additions & 0 deletions testdata/project-v4-multigroup/config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
- ""
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- deployments
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
Expand Down Expand Up @@ -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:
- ""
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- deployments
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
Expand Down
40 changes: 40 additions & 0 deletions testdata/project-v4-multigroup/dist/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
- ""
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- deployments
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
Expand Down Expand Up @@ -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:
- ""
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- deployments
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
Expand Down
Original file line number Diff line number Diff line change
@@ -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="",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="",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
}
Original file line number Diff line number Diff line change
@@ -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())
// })
})

})
Loading

0 comments on commit 6fa2024

Please sign in to comment.