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.

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

🐛 (go/v4): Fix path configuration for webhook markers generated for core types with non-"core" group values #4301

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 and .Resource.Core (eq .Resource.QualifiedGroup "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 and .Resource.Core (eq .Resource.QualifiedGroup "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
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:
- apps
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:
- apps
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:
- apps
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:
- apps
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=apps,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=apps,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
Loading