From 955d723bc67b9e9417e6f3fcc4b3b9a04e59b995 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sat, 18 Oct 2025 15:31:38 -0400 Subject: [PATCH 01/15] Type defaulter and validator --- pkg/webhook/admission/defaulter_custom.go | 49 +++++++++++++++-------- pkg/webhook/admission/validator_custom.go | 46 +++++++++++++-------- 2 files changed, 62 insertions(+), 33 deletions(-) diff --git a/pkg/webhook/admission/defaulter_custom.go b/pkg/webhook/admission/defaulter_custom.go index a703cbd2c5..d303c17f41 100644 --- a/pkg/webhook/admission/defaulter_custom.go +++ b/pkg/webhook/admission/defaulter_custom.go @@ -31,11 +31,14 @@ import ( "k8s.io/apimachinery/pkg/util/sets" ) -// CustomDefaulter defines functions for setting defaults on resources. -type CustomDefaulter interface { - Default(ctx context.Context, obj runtime.Object) error +// Defaulter defines functions for setting defaults on resources. +type Defaulter[T runtime.Object] interface { + Default(ctx context.Context, obj T) error } +// Deprecated: CustomDefaulter is deprecated, use Defaulter instead +type CustomDefaulter = Defaulter[runtime.Object] + type defaulterOptions struct { removeUnknownOrOmitableFields bool } @@ -50,40 +53,52 @@ func DefaulterRemoveUnknownOrOmitableFields(o *defaulterOptions) { o.removeUnknownOrOmitableFields = true } -// WithCustomDefaulter creates a new Webhook for a CustomDefaulter interface. +// WithDefaulter creates a new Webhook for a CustomDefaulter interface. +func WithDefaulter[T runtime.Object](scheme *runtime.Scheme, defaulter Defaulter[T], opts ...DefaulterOption) *Webhook { + options := &defaulterOptions{} + for _, o := range opts { + o(options) + } + return &Webhook{ + Handler: &defaulterForType[T]{ + defaulter: defaulter, + decoder: NewDecoder(scheme), + removeUnknownOrOmitableFields: options.removeUnknownOrOmitableFields, + new: func() T { return *new(T) }, + }, + } +} + func WithCustomDefaulter(scheme *runtime.Scheme, obj runtime.Object, defaulter CustomDefaulter, opts ...DefaulterOption) *Webhook { options := &defaulterOptions{} for _, o := range opts { o(options) } return &Webhook{ - Handler: &defaulterForType{ - object: obj, + Handler: &defaulterForType[runtime.Object]{ defaulter: defaulter, decoder: NewDecoder(scheme), removeUnknownOrOmitableFields: options.removeUnknownOrOmitableFields, + new: func() runtime.Object { return obj.DeepCopyObject() }, }, } } -type defaulterForType struct { - defaulter CustomDefaulter - object runtime.Object +type defaulterForType[T runtime.Object] struct { + defaulter Defaulter[T] decoder Decoder removeUnknownOrOmitableFields bool + new func() T } // Handle handles admission requests. -func (h *defaulterForType) Handle(ctx context.Context, req Request) Response { +func (h *defaulterForType[T]) Handle(ctx context.Context, req Request) Response { if h.decoder == nil { panic("decoder should never be nil") } if h.defaulter == nil { panic("defaulter should never be nil") } - if h.object == nil { - panic("object should never be nil") - } // Always skip when a DELETE operation received in custom mutation handler. if req.Operation == admissionv1.Delete { @@ -98,15 +113,15 @@ func (h *defaulterForType) Handle(ctx context.Context, req Request) Response { ctx = NewContextWithRequest(ctx, req) // Get the object in the request - obj := h.object.DeepCopyObject() + obj := h.new() if err := h.decoder.Decode(req, obj); err != nil { return Errored(http.StatusBadRequest, err) } // Keep a copy of the object if needed - var originalObj runtime.Object + var originalObj T if !h.removeUnknownOrOmitableFields { - originalObj = obj.DeepCopyObject() + originalObj = obj.DeepCopyObject().(T) } // Default the object @@ -131,7 +146,7 @@ func (h *defaulterForType) Handle(ctx context.Context, req Request) Response { return handlerResponse } -func (h *defaulterForType) dropSchemeRemovals(r Response, original runtime.Object, raw []byte) Response { +func (h *defaulterForType[T]) dropSchemeRemovals(r Response, original T, raw []byte) Response { const opRemove = "remove" if !r.Allowed || r.PatchType == nil { return r diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index ef1be52a8f..5b8e7460c5 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -32,52 +32,66 @@ type Warnings []string // CustomValidator defines functions for validating an operation. // The object to be validated is passed into methods as a parameter. -type CustomValidator interface { +type Validator[T runtime.Object] interface { // ValidateCreate validates the object on creation. // The optional warnings will be added to the response as warning messages. // Return an error if the object is invalid. - ValidateCreate(ctx context.Context, obj runtime.Object) (warnings Warnings, err error) + ValidateCreate(ctx context.Context, obj T) (warnings Warnings, err error) // ValidateUpdate validates the object on update. // The optional warnings will be added to the response as warning messages. // Return an error if the object is invalid. - ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings Warnings, err error) + ValidateUpdate(ctx context.Context, oldObj, newObj T) (warnings Warnings, err error) // ValidateDelete validates the object on deletion. // The optional warnings will be added to the response as warning messages. // Return an error if the object is invalid. - ValidateDelete(ctx context.Context, obj runtime.Object) (warnings Warnings, err error) + ValidateDelete(ctx context.Context, obj T) (warnings Warnings, err error) } -// WithCustomValidator creates a new Webhook for validating the provided type. +// deprecated: CustomValidator is deprecated, use Validator instead +type CustomValidator = Validator[runtime.Object] + +// WithValidator creates a new Webhook for validating the provided type. +func WithValidator[T runtime.Object](scheme *runtime.Scheme, validator Validator[T]) *Webhook { + return &Webhook{ + Handler: &validatorForType[T]{ + validator: validator, + decoder: NewDecoder(scheme), + new: func() runtime.Object { return *new(T) }, + }, + } +} + +// deprecated: WithCustomValidator is deprecated, use WithValidator instead func WithCustomValidator(scheme *runtime.Scheme, obj runtime.Object, validator CustomValidator) *Webhook { return &Webhook{ - Handler: &validatorForType{object: obj, validator: validator, decoder: NewDecoder(scheme)}, + Handler: &validatorForType[runtime.Object]{ + validator: validator, + decoder: NewDecoder(scheme), + new: func() runtime.Object { return obj.DeepCopyObject() }, + }, } } -type validatorForType struct { - validator CustomValidator - object runtime.Object +type validatorForType[T runtime.Object] struct { + validator Validator[T] decoder Decoder + new func() runtime.Object } // Handle handles admission requests. -func (h *validatorForType) Handle(ctx context.Context, req Request) Response { +func (h *validatorForType[T]) Handle(ctx context.Context, req Request) Response { if h.decoder == nil { panic("decoder should never be nil") } if h.validator == nil { panic("validator should never be nil") } - if h.object == nil { - panic("object should never be nil") - } ctx = NewContextWithRequest(ctx, req) - // Get the object in the request - obj := h.object.DeepCopyObject() + obj := h.new().(T) var err error var warnings []string @@ -93,7 +107,7 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { warnings, err = h.validator.ValidateCreate(ctx, obj) case v1.Update: - oldObj := obj.DeepCopyObject() + oldObj := h.new().(T) if err := h.decoder.DecodeRaw(req.Object, obj); err != nil { return Errored(http.StatusBadRequest, err) } From 8196239b7b12e05ae969013fa46d326879854305 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sat, 18 Oct 2025 16:06:07 -0400 Subject: [PATCH 02/15] Builder --- alias.go | 9 ++- examples/builtins/main.go | 3 +- examples/builtins/mutatingwebhook.go | 8 +-- examples/builtins/validatingwebhook.go | 13 ++-- examples/crd/main.go | 3 +- pkg/builder/example_webhook_test.go | 5 +- pkg/builder/webhook.go | 83 ++++++++++++++++---------- pkg/builder/webhook_test.go | 26 ++++---- 8 files changed, 79 insertions(+), 71 deletions(-) diff --git a/alias.go b/alias.go index 01ba012dcc..1adc97a1f3 100644 --- a/alias.go +++ b/alias.go @@ -18,6 +18,7 @@ package controllerruntime import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client/config" @@ -104,9 +105,6 @@ var ( // NewControllerManagedBy returns a new controller builder that will be started by the provided Manager. NewControllerManagedBy = builder.ControllerManagedBy - // NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager. - NewWebhookManagedBy = builder.WebhookManagedBy - // NewManager returns a new Manager for creating Controllers. // Note that if ContentType in the given config is not set, "application/vnd.kubernetes.protobuf" // will be used for all built-in resources of Kubernetes, and "application/json" is for other types @@ -155,3 +153,8 @@ var ( // SetLogger sets a concrete logging implementation for all deferred Loggers. SetLogger = log.SetLogger ) + +// NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager. +func NewWebhookManagedBy[T runtime.Object](m manager.Manager) *builder.WebhookBuilder[T] { + return builder.WebhookManagedBy[T](m) +} diff --git a/examples/builtins/main.go b/examples/builtins/main.go index 3a47814d8c..afd41e1d86 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -60,8 +60,7 @@ func main() { os.Exit(1) } - if err := ctrl.NewWebhookManagedBy(mgr). - For(&corev1.Pod{}). + if err := ctrl.NewWebhookManagedBy[*corev1.Pod](mgr). WithDefaulter(&podAnnotator{}). WithValidator(&podValidator{}). Complete(); err != nil { diff --git a/examples/builtins/mutatingwebhook.go b/examples/builtins/mutatingwebhook.go index a588eba8f9..0f150c9b6c 100644 --- a/examples/builtins/mutatingwebhook.go +++ b/examples/builtins/mutatingwebhook.go @@ -18,10 +18,8 @@ package main import ( "context" - "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -31,12 +29,8 @@ import ( // podAnnotator annotates Pods type podAnnotator struct{} -func (a *podAnnotator) Default(ctx context.Context, obj runtime.Object) error { +func (a *podAnnotator) Default(ctx context.Context, pod *corev1.Pod) error { log := logf.FromContext(ctx) - pod, ok := obj.(*corev1.Pod) - if !ok { - return fmt.Errorf("expected a Pod but got a %T", obj) - } if pod.Annotations == nil { pod.Annotations = map[string]string{} diff --git a/examples/builtins/validatingwebhook.go b/examples/builtins/validatingwebhook.go index 1bee7f7c84..eb08159688 100644 --- a/examples/builtins/validatingwebhook.go +++ b/examples/builtins/validatingwebhook.go @@ -21,7 +21,6 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -33,12 +32,8 @@ import ( type podValidator struct{} // validate admits a pod if a specific annotation exists. -func (v *podValidator) validate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (v *podValidator) validate(ctx context.Context, pod *corev1.Pod) (admission.Warnings, error) { log := logf.FromContext(ctx) - pod, ok := obj.(*corev1.Pod) - if !ok { - return nil, fmt.Errorf("expected a Pod but got a %T", obj) - } log.Info("Validating Pod") key := "example-mutating-admission-webhook" @@ -53,14 +48,14 @@ func (v *podValidator) validate(ctx context.Context, obj runtime.Object) (admiss return nil, nil } -func (v *podValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (v *podValidator) ValidateCreate(ctx context.Context, obj *corev1.Pod) (admission.Warnings, error) { return v.validate(ctx, obj) } -func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { +func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *corev1.Pod) (admission.Warnings, error) { return v.validate(ctx, newObj) } -func (v *podValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (v *podValidator) ValidateDelete(ctx context.Context, obj *corev1.Pod) (admission.Warnings, error) { return v.validate(ctx, obj) } diff --git a/examples/crd/main.go b/examples/crd/main.go index 0bf65c9890..f92f238d23 100644 --- a/examples/crd/main.go +++ b/examples/crd/main.go @@ -129,8 +129,7 @@ func main() { os.Exit(1) } - err = ctrl.NewWebhookManagedBy(mgr). - For(&api.ChaosPod{}). + err = ctrl.NewWebhookManagedBy[*api.ChaosPod](mgr). Complete() if err != nil { setupLog.Error(err, "unable to create webhook") diff --git a/pkg/builder/example_webhook_test.go b/pkg/builder/example_webhook_test.go index c26eba8a13..33d2a499be 100644 --- a/pkg/builder/example_webhook_test.go +++ b/pkg/builder/example_webhook_test.go @@ -19,6 +19,7 @@ package builder_test import ( "os" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client/config" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -40,8 +41,8 @@ func ExampleWebhookBuilder() { } err = builder. - WebhookManagedBy(mgr). // Create the WebhookManagedBy - For(&examplegroup.ChaosPod{}). // ChaosPod is a CRD. + WebhookManagedBy[runtime.Object](mgr). // Create the WebhookManagedBy + For(&examplegroup.ChaosPod{}). // ChaosPod is a CRD. Complete() if err != nil { log.Error(err, "could not create webhook") diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index bb5b6deb56..6647f2e448 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -37,11 +37,11 @@ import ( ) // WebhookBuilder builds a Webhook. -type WebhookBuilder struct { +type WebhookBuilder[T runtime.Object] struct { apiType runtime.Object - customDefaulter admission.CustomDefaulter + customDefaulter admission.Defaulter[T] customDefaulterOpts []admission.DefaulterOption - customValidator admission.CustomValidator + customValidator admission.Validator[T] customPath string customValidatorCustomPath string customDefaulterCustomPath string @@ -56,16 +56,13 @@ type WebhookBuilder struct { } // WebhookManagedBy returns a new webhook builder. -func WebhookManagedBy(m manager.Manager) *WebhookBuilder { - return &WebhookBuilder{mgr: m} +func WebhookManagedBy[T runtime.Object](m manager.Manager) *WebhookBuilder[T] { + return &WebhookBuilder[T]{mgr: m} } -// TODO(droot): update the GoDoc for conversion. - -// For takes a runtime.Object which should be a CR. -// If the given object implements the admission.Defaulter interface, a MutatingWebhook will be wired for this type. -// If the given object implements the admission.Validator interface, a ValidatingWebhook will be wired for this type. -func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder { +// For takes a runtime.Object which should be a CR. It is only required +// if the webhooks generic type is runtime.Object +func (blder *WebhookBuilder[T]) For(apiType runtime.Object) *WebhookBuilder[T] { if blder.apiType != nil { blder.err = errors.New("For(...) should only be called once, could not assign multiple objects for webhook registration") } @@ -75,14 +72,14 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder { // WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption) // will be wired for this type. -func (blder *WebhookBuilder) WithDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] { blder.customDefaulter = defaulter blder.customDefaulterOpts = opts return blder } // WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. -func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithValidator(validator admission.Validator[T]) *WebhookBuilder[T] { blder.customValidator = validator return blder } @@ -95,20 +92,20 @@ func (blder *WebhookBuilder) WithConverter(converterConstructor func(*runtime.Sc } // WithLogConstructor overrides the webhook's LogConstructor. -func (blder *WebhookBuilder) WithLogConstructor(logConstructor func(base logr.Logger, req *admission.Request) logr.Logger) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithLogConstructor(logConstructor func(base logr.Logger, req *admission.Request) logr.Logger) *WebhookBuilder[T] { blder.logConstructor = logConstructor return blder } // WithContextFunc overrides the webhook's WithContextFunc. -func (blder *WebhookBuilder) WithContextFunc(contextFunc func(context.Context, *http.Request) context.Context) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithContextFunc(contextFunc func(context.Context, *http.Request) context.Context) *WebhookBuilder[T] { blder.contextFunc = contextFunc return blder } // RecoverPanic indicates whether panics caused by the webhook should be recovered. // Defaults to true. -func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder { +func (blder *WebhookBuilder[T]) RecoverPanic(recoverPanic bool) *WebhookBuilder[T] { blder.recoverPanic = &recoverPanic return blder } @@ -117,25 +114,25 @@ func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder { // // Deprecated: WithCustomPath should not be used anymore. // Please use WithValidatorCustomPath or WithDefaulterCustomPath instead. -func (blder *WebhookBuilder) WithCustomPath(customPath string) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithCustomPath(customPath string) *WebhookBuilder[T] { blder.customPath = customPath return blder } // WithValidatorCustomPath overrides the path of the Validator. -func (blder *WebhookBuilder) WithValidatorCustomPath(customPath string) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithValidatorCustomPath(customPath string) *WebhookBuilder[T] { blder.customValidatorCustomPath = customPath return blder } // WithDefaulterCustomPath overrides the path of the Defaulter. -func (blder *WebhookBuilder) WithDefaulterCustomPath(customPath string) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithDefaulterCustomPath(customPath string) *WebhookBuilder[T] { blder.customDefaulterCustomPath = customPath return blder } // Complete builds the webhook. -func (blder *WebhookBuilder) Complete() error { +func (blder *WebhookBuilder[T]) Complete() error { // Set the Config blder.loadRestConfig() @@ -146,13 +143,13 @@ func (blder *WebhookBuilder) Complete() error { return blder.registerWebhooks() } -func (blder *WebhookBuilder) loadRestConfig() { +func (blder *WebhookBuilder[T]) loadRestConfig() { if blder.config == nil { blder.config = blder.mgr.GetConfig() } } -func (blder *WebhookBuilder) setLogConstructor() { +func (blder *WebhookBuilder[T]) setLogConstructor() { if blder.logConstructor == nil { blder.logConstructor = func(base logr.Logger, req *admission.Request) logr.Logger { log := base.WithValues( @@ -172,11 +169,11 @@ func (blder *WebhookBuilder) setLogConstructor() { } } -func (blder *WebhookBuilder) isThereCustomPathConflict() bool { +func (blder *WebhookBuilder[T]) isThereCustomPathConflict() bool { return (blder.customPath != "" && blder.customDefaulter != nil && blder.customValidator != nil) || (blder.customPath != "" && blder.customDefaulterCustomPath != "") || (blder.customPath != "" && blder.customValidatorCustomPath != "") } -func (blder *WebhookBuilder) registerWebhooks() error { +func (blder *WebhookBuilder[T]) registerWebhooks() error { typ, err := blder.getType() if err != nil { return err @@ -217,7 +214,7 @@ func (blder *WebhookBuilder) registerWebhooks() error { } // registerDefaultingWebhook registers a defaulting webhook if necessary. -func (blder *WebhookBuilder) registerDefaultingWebhook() error { +func (blder *WebhookBuilder[T]) registerDefaultingWebhook() error { mwh := blder.getDefaultingWebhook() if mwh != nil { mwh.LogConstructor = blder.logConstructor @@ -244,9 +241,15 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() error { return nil } -func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook { +func (blder *WebhookBuilder[T]) getDefaultingWebhook() *admission.Webhook { if defaulter := blder.customDefaulter; defaulter != nil { - w := admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter, blder.customDefaulterOpts...) + var w *admission.Webhook + switch any(*new(T)).(type) { + case runtime.Object: + w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, &defaulterWrapper[T]{defaulter: defaulter}, blder.customDefaulterOpts...) + default: + w = admission.WithDefaulter(blder.mgr.GetScheme(), defaulter, blder.customDefaulterOpts...) + } if blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) } @@ -255,8 +258,16 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook { return nil } +type defaulterWrapper[T runtime.Object] struct { + defaulter admission.Defaulter[T] +} + +func (d *defaulterWrapper[T]) Default(ctx context.Context, obj runtime.Object) error { + return d.defaulter.Default(ctx, obj.(T)) +} + // registerValidatingWebhook registers a validating webhook if necessary. -func (blder *WebhookBuilder) registerValidatingWebhook() error { +func (blder *WebhookBuilder[T]) registerValidatingWebhook() error { vwh := blder.getValidatingWebhook() if vwh != nil { vwh.LogConstructor = blder.logConstructor @@ -283,9 +294,15 @@ func (blder *WebhookBuilder) registerValidatingWebhook() error { return nil } -func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook { +func (blder *WebhookBuilder[T]) getValidatingWebhook() *admission.Webhook { if validator := blder.customValidator; validator != nil { - w := admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator) + var w *admission.Webhook + switch any(*new(T)).(type) { + case runtime.Object: + w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, &validatorWrapper[T]{validator: validator}) + default: + w = admission.WithValidator(blder.mgr.GetScheme(), validator) + } if blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) } @@ -294,7 +311,7 @@ func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook { return nil } -func (blder *WebhookBuilder) registerConversionWebhook() error { +func (blder *WebhookBuilder[T]) registerConversionWebhook() error { if blder.converterConstructor != nil { converter, err := blder.converterConstructor(blder.mgr.GetScheme()) if err != nil { @@ -323,14 +340,14 @@ func (blder *WebhookBuilder) registerConversionWebhook() error { return nil } -func (blder *WebhookBuilder) getType() (runtime.Object, error) { +func (blder *WebhookBuilder[T]) getType() (runtime.Object, error) { if blder.apiType != nil { return blder.apiType, nil } return nil, errors.New("For() must be called with a valid object") } -func (blder *WebhookBuilder) isAlreadyHandled(path string) bool { +func (blder *WebhookBuilder[T]) isAlreadyHandled(path string) bool { if blder.mgr.GetWebhookServer().WebhookMux() == nil { return false } diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 72538ef7bf..8ba76452f7 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -96,7 +96,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -173,7 +173,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) customPath := "/custom-defaulting-path" - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -251,7 +251,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). RecoverPanic(true). @@ -315,7 +315,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -434,7 +434,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) customPath := "/custom-validating-path" - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -513,7 +513,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). RecoverPanic(true). @@ -579,7 +579,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). Complete() @@ -670,7 +670,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaulter{}). For(&TestDefaulter{}). Complete() @@ -688,7 +688,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). @@ -773,7 +773,7 @@ func runTests(admissionReviewVersion string) { validatingCustomPath := "/custom-validating-path" defaultingCustomPath := "/custom-defaulting-path" - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). @@ -878,7 +878,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). @@ -901,7 +901,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -924,7 +924,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). + err = WebhookManagedBy[runtime.Object](m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { From fc15d4aca7ed24e9534fd744205e0c6a5a1abd4c Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sat, 18 Oct 2025 22:55:08 -0400 Subject: [PATCH 03/15] Introduce WebhookFor --- alias.go | 10 +++++++--- examples/builtins/main.go | 2 +- examples/crd/main.go | 2 +- pkg/builder/example_webhook_test.go | 3 +-- pkg/builder/webhook.go | 8 +++++++- pkg/builder/webhook_test.go | 26 +++++++++++++------------- 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/alias.go b/alias.go index 1adc97a1f3..883ca7d816 100644 --- a/alias.go +++ b/alias.go @@ -105,6 +105,10 @@ var ( // NewControllerManagedBy returns a new controller builder that will be started by the provided Manager. NewControllerManagedBy = builder.ControllerManagedBy + // NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager. + // Deprecated: Use NewWebhookFor instead. + NewWebhookManagedBy = builder.WebhookManagedBy + // NewManager returns a new Manager for creating Controllers. // Note that if ContentType in the given config is not set, "application/vnd.kubernetes.protobuf" // will be used for all built-in resources of Kubernetes, and "application/json" is for other types @@ -154,7 +158,7 @@ var ( SetLogger = log.SetLogger ) -// NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager. -func NewWebhookManagedBy[T runtime.Object](m manager.Manager) *builder.WebhookBuilder[T] { - return builder.WebhookManagedBy[T](m) +// NewWebhookFor returns a new webhook builder for the provided type T. +func NewWebhookFor[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] { + return builder.WebhookFor(mgr, obj) } diff --git a/examples/builtins/main.go b/examples/builtins/main.go index afd41e1d86..d70b1fc089 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -60,7 +60,7 @@ func main() { os.Exit(1) } - if err := ctrl.NewWebhookManagedBy[*corev1.Pod](mgr). + if err := ctrl.NewWebhookFor(mgr, &corev1.Pod{}). WithDefaulter(&podAnnotator{}). WithValidator(&podValidator{}). Complete(); err != nil { diff --git a/examples/crd/main.go b/examples/crd/main.go index f92f238d23..77e59e148d 100644 --- a/examples/crd/main.go +++ b/examples/crd/main.go @@ -129,7 +129,7 @@ func main() { os.Exit(1) } - err = ctrl.NewWebhookManagedBy[*api.ChaosPod](mgr). + err = ctrl.NewWebhookFor(mgr, &api.ChaosPod{}). Complete() if err != nil { setupLog.Error(err, "unable to create webhook") diff --git a/pkg/builder/example_webhook_test.go b/pkg/builder/example_webhook_test.go index 33d2a499be..03ecff9772 100644 --- a/pkg/builder/example_webhook_test.go +++ b/pkg/builder/example_webhook_test.go @@ -41,8 +41,7 @@ func ExampleWebhookBuilder() { } err = builder. - WebhookManagedBy[runtime.Object](mgr). // Create the WebhookManagedBy - For(&examplegroup.ChaosPod{}). // ChaosPod is a CRD. + WebhookFor[runtime.Object](mgr, &examplegroup.ChaosPod{}). // Create the WebhookManagedBy Complete() if err != nil { log.Error(err, "could not create webhook") diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 6647f2e448..49ed669707 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -56,7 +56,13 @@ type WebhookBuilder[T runtime.Object] struct { } // WebhookManagedBy returns a new webhook builder. -func WebhookManagedBy[T runtime.Object](m manager.Manager) *WebhookBuilder[T] { +// Deprecated: Use WebhookFor instead. +func WebhookManagedBy(m manager.Manager) *WebhookBuilder[runtime.Object] { + return &WebhookBuilder[runtime.Object]{mgr: m} +} + +// WebhookFor return a new webhook builder for the provided type T. +func WebhookFor[T runtime.Object](m manager.Manager, object T) *WebhookBuilder[T] { return &WebhookBuilder[T]{mgr: m} } diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 8ba76452f7..72538ef7bf 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -96,7 +96,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -173,7 +173,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) customPath := "/custom-defaulting-path" - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -251,7 +251,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). RecoverPanic(true). @@ -315,7 +315,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -434,7 +434,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) customPath := "/custom-validating-path" - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -513,7 +513,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). RecoverPanic(true). @@ -579,7 +579,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). Complete() @@ -670,7 +670,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaulter{}). For(&TestDefaulter{}). Complete() @@ -688,7 +688,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). @@ -773,7 +773,7 @@ func runTests(admissionReviewVersion string) { validatingCustomPath := "/custom-validating-path" defaultingCustomPath := "/custom-defaulting-path" - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). @@ -878,7 +878,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). @@ -901,7 +901,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -924,7 +924,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy[runtime.Object](m). + err = WebhookManagedBy(m). For(&TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { From 48a1c0856d4eb1ffbd66ec82ea11c1a545f93f78 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Mon, 20 Oct 2025 19:20:19 -0400 Subject: [PATCH 04/15] Update existing WebhookManagedBy --- alias.go | 10 +--- examples/builtins/main.go | 6 +- examples/crd/main.go | 2 +- pkg/builder/example_webhook_test.go | 3 +- pkg/builder/webhook.go | 93 +++++++++++++---------------- pkg/builder/webhook_test.go | 54 ++++------------- 6 files changed, 61 insertions(+), 107 deletions(-) diff --git a/alias.go b/alias.go index 883ca7d816..a294caeda2 100644 --- a/alias.go +++ b/alias.go @@ -105,10 +105,6 @@ var ( // NewControllerManagedBy returns a new controller builder that will be started by the provided Manager. NewControllerManagedBy = builder.ControllerManagedBy - // NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager. - // Deprecated: Use NewWebhookFor instead. - NewWebhookManagedBy = builder.WebhookManagedBy - // NewManager returns a new Manager for creating Controllers. // Note that if ContentType in the given config is not set, "application/vnd.kubernetes.protobuf" // will be used for all built-in resources of Kubernetes, and "application/json" is for other types @@ -158,7 +154,7 @@ var ( SetLogger = log.SetLogger ) -// NewWebhookFor returns a new webhook builder for the provided type T. -func NewWebhookFor[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] { - return builder.WebhookFor(mgr, obj) +// WebhookManagedBy returns a new webhook builder for the provided type T. +func WebhookManagedBy[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] { + return builder.WebhookManagedBy(mgr, obj) } diff --git a/examples/builtins/main.go b/examples/builtins/main.go index d70b1fc089..650c921a4a 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -60,9 +60,9 @@ func main() { os.Exit(1) } - if err := ctrl.NewWebhookFor(mgr, &corev1.Pod{}). - WithDefaulter(&podAnnotator{}). - WithValidator(&podValidator{}). + if err := ctrl.WebhookManagedBy(mgr, &corev1.Pod{}). + WithAdmissionDefaulter(&podAnnotator{}). + WithAdmissionValidator(&podValidator{}). Complete(); err != nil { entryLog.Error(err, "unable to create webhook", "webhook", "Pod") os.Exit(1) diff --git a/examples/crd/main.go b/examples/crd/main.go index 77e59e148d..a910e7fc74 100644 --- a/examples/crd/main.go +++ b/examples/crd/main.go @@ -129,7 +129,7 @@ func main() { os.Exit(1) } - err = ctrl.NewWebhookFor(mgr, &api.ChaosPod{}). + err = ctrl.WebhookManagedBy(mgr, &api.ChaosPod{}). Complete() if err != nil { setupLog.Error(err, "unable to create webhook") diff --git a/pkg/builder/example_webhook_test.go b/pkg/builder/example_webhook_test.go index 03ecff9772..133da47272 100644 --- a/pkg/builder/example_webhook_test.go +++ b/pkg/builder/example_webhook_test.go @@ -19,7 +19,6 @@ package builder_test import ( "os" - "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client/config" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -41,7 +40,7 @@ func ExampleWebhookBuilder() { } err = builder. - WebhookFor[runtime.Object](mgr, &examplegroup.ChaosPod{}). // Create the WebhookManagedBy + WebhookManagedBy(mgr, &examplegroup.ChaosPod{}). // Create the WebhookManagedBy Complete() if err != nil { log.Error(err, "could not create webhook") diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 49ed669707..373e0d2e4d 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -39,9 +39,11 @@ import ( // WebhookBuilder builds a Webhook. type WebhookBuilder[T runtime.Object] struct { apiType runtime.Object - customDefaulter admission.Defaulter[T] + customDefaulter admission.CustomDefaulter + defaulter admission.Defaulter[T] customDefaulterOpts []admission.DefaulterOption - customValidator admission.Validator[T] + customValidator admission.CustomValidator + validator admission.Validator[T] customPath string customValidatorCustomPath string customDefaulterCustomPath string @@ -56,43 +58,42 @@ type WebhookBuilder[T runtime.Object] struct { } // WebhookManagedBy returns a new webhook builder. -// Deprecated: Use WebhookFor instead. -func WebhookManagedBy(m manager.Manager) *WebhookBuilder[runtime.Object] { - return &WebhookBuilder[runtime.Object]{mgr: m} -} - -// WebhookFor return a new webhook builder for the provided type T. -func WebhookFor[T runtime.Object](m manager.Manager, object T) *WebhookBuilder[T] { - return &WebhookBuilder[T]{mgr: m} -} - -// For takes a runtime.Object which should be a CR. It is only required -// if the webhooks generic type is runtime.Object -func (blder *WebhookBuilder[T]) For(apiType runtime.Object) *WebhookBuilder[T] { - if blder.apiType != nil { - blder.err = errors.New("For(...) should only be called once, could not assign multiple objects for webhook registration") - } - blder.apiType = apiType - return blder +func WebhookManagedBy[T runtime.Object](m manager.Manager, object T) *WebhookBuilder[T] { + return &WebhookBuilder[T]{mgr: m, apiType: object} } // WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption) // will be wired for this type. -func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] { +// deprecated: Use WithAdmissionDefaulter instead. +func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder[T] { blder.customDefaulter = defaulter blder.customDefaulterOpts = opts return blder } +// WithAdmissionDefaulter sets up the provided admission.Defaulter in a defaulting webhook. +func (blder *WebhookBuilder[T]) WithAdmissionDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] { + blder.defaulter = defaulter + blder.customDefaulterOpts = opts + return blder +} + // WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. -func (blder *WebhookBuilder[T]) WithValidator(validator admission.Validator[T]) *WebhookBuilder[T] { +// deprecated: Use WithAdmissionValidator instead. +func (blder *WebhookBuilder[T]) WithValidator(validator admission.CustomValidator) *WebhookBuilder[T] { blder.customValidator = validator return blder } +// WithAdmissionValidator sets up the provided admission.Validator in a validating webhook. +func (blder *WebhookBuilder[T]) WithAdmissionValidator(validator admission.Validator[T]) *WebhookBuilder[T] { + blder.validator = validator + return blder +} + // WithConverter takes a func that constructs a converter.Converter. // The Converter will then be used by the conversion endpoint for the type passed into For(). -func (blder *WebhookBuilder) WithConverter(converterConstructor func(*runtime.Scheme) (conversion.Converter, error)) *WebhookBuilder { +func (blder *WebhookBuilder[T]) WithConverter(converterConstructor func(*runtime.Scheme) (conversion.Converter, error)) *WebhookBuilder[T] { blder.converterConstructor = converterConstructor return blder } @@ -248,28 +249,19 @@ func (blder *WebhookBuilder[T]) registerDefaultingWebhook() error { } func (blder *WebhookBuilder[T]) getDefaultingWebhook() *admission.Webhook { - if defaulter := blder.customDefaulter; defaulter != nil { - var w *admission.Webhook - switch any(*new(T)).(type) { - case runtime.Object: - w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, &defaulterWrapper[T]{defaulter: defaulter}, blder.customDefaulterOpts...) - default: - w = admission.WithDefaulter(blder.mgr.GetScheme(), defaulter, blder.customDefaulterOpts...) - } + var w *admission.Webhook + if defaulter := blder.defaulter; defaulter != nil { + w = admission.WithDefaulter(blder.mgr.GetScheme(), blder.defaulter, blder.customDefaulterOpts...) + } else if customDefaulter := blder.customDefaulter; customDefaulter != nil { + w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, customDefaulter, blder.customDefaulterOpts...) if blder.recoverPanic != nil { - w = w.WithRecoverPanic(*blder.recoverPanic) } return w } - return nil -} - -type defaulterWrapper[T runtime.Object] struct { - defaulter admission.Defaulter[T] -} - -func (d *defaulterWrapper[T]) Default(ctx context.Context, obj runtime.Object) error { - return d.defaulter.Default(ctx, obj.(T)) + if w != nil && blder.recoverPanic != nil { + w = w.WithRecoverPanic(*blder.recoverPanic) + } + return w } // registerValidatingWebhook registers a validating webhook if necessary. @@ -301,19 +293,16 @@ func (blder *WebhookBuilder[T]) registerValidatingWebhook() error { } func (blder *WebhookBuilder[T]) getValidatingWebhook() *admission.Webhook { - if validator := blder.customValidator; validator != nil { - var w *admission.Webhook - switch any(*new(T)).(type) { - case runtime.Object: - w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, &validatorWrapper[T]{validator: validator}) - default: - w = admission.WithValidator(blder.mgr.GetScheme(), validator) - } - if blder.recoverPanic != nil { - w = w.WithRecoverPanic(*blder.recoverPanic) - } + var w *admission.Webhook + if validator := blder.validator; validator != nil { + w = admission.WithValidator(blder.mgr.GetScheme(), validator) + } else if customValidator := blder.customValidator; customValidator != nil { + w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, customValidator) return w } + if w != nil && blder.recoverPanic != nil { + w = w.WithRecoverPanic(*blder.recoverPanic) + } return nil } diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 72538ef7bf..a93f187a95 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -96,8 +96,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestDefaulter{}). + err = WebhookManagedBy(m, &TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -173,8 +172,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) customPath := "/custom-defaulting-path" - err = WebhookManagedBy(m). - For(&TestDefaulter{}). + err = WebhookManagedBy(m, &TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -251,8 +249,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestDefaulter{}). + err = WebhookManagedBy(m, &TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). RecoverPanic(true). // RecoverPanic defaults to true. @@ -315,8 +312,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestValidator{}). + err = WebhookManagedBy(m, &TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -434,8 +430,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) customPath := "/custom-validating-path" - err = WebhookManagedBy(m). - For(&TestValidator{}). + err = WebhookManagedBy(m, &TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -513,8 +508,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestValidator{}). + err = WebhookManagedBy(m, &TestValidator{}). WithValidator(&TestCustomValidator{}). RecoverPanic(true). Complete() @@ -579,8 +573,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestValidator{}). + err = WebhookManagedBy(m, &TestValidator{}). WithValidator(&TestCustomValidator{}). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -659,24 +652,6 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) }) - It("should send an error when trying to register a webhook with more than one For", func() { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()} - builder.Register(&TestDefaulter{}, &TestDefaulterList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - err = WebhookManagedBy(m). - For(&TestDefaulter{}). - For(&TestDefaulter{}). - Complete() - Expect(err).To(HaveOccurred()) - }) - It("should scaffold a custom defaulting and validating webhook", func(specCtx SpecContext) { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) @@ -688,8 +663,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestDefaultValidator{}). + err = WebhookManagedBy(m, &TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -773,8 +747,7 @@ func runTests(admissionReviewVersion string) { validatingCustomPath := "/custom-validating-path" defaultingCustomPath := "/custom-defaulting-path" - err = WebhookManagedBy(m). - For(&TestDefaultValidator{}). + err = WebhookManagedBy(m, &TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -878,8 +851,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestDefaultValidator{}). + err = WebhookManagedBy(m, &TestDefaultValidator{}). WithDefaulter(&TestCustomDefaultValidator{}). WithValidator(&TestCustomDefaultValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { @@ -901,8 +873,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestDefaulter{}). + err = WebhookManagedBy(m, &TestDefaulter{}). WithDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -924,8 +895,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m). - For(&TestValidator{}). + err = WebhookManagedBy(m, &TestValidator{}). WithValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) From 3cc191067ec1531c80418b3612663c56b43e4552 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Mon, 20 Oct 2025 19:48:12 -0400 Subject: [PATCH 05/15] Linting --- pkg/builder/webhook.go | 10 +++------- pkg/webhook/admission/defaulter_custom.go | 4 +++- pkg/webhook/admission/validator_custom.go | 8 +++++--- pkg/webhook/alias.go | 6 ------ 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 373e0d2e4d..a8e72cc993 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -64,7 +64,7 @@ func WebhookManagedBy[T runtime.Object](m manager.Manager, object T) *WebhookBui // WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption) // will be wired for this type. -// deprecated: Use WithAdmissionDefaulter instead. +// Deprecated: Use WithAdmissionDefaulter instead. func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder[T] { blder.customDefaulter = defaulter blder.customDefaulterOpts = opts @@ -79,7 +79,7 @@ func (blder *WebhookBuilder[T]) WithAdmissionDefaulter(defaulter admission.Defau } // WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. -// deprecated: Use WithAdmissionValidator instead. +// Deprecated: Use WithAdmissionValidator instead. func (blder *WebhookBuilder[T]) WithValidator(validator admission.CustomValidator) *WebhookBuilder[T] { blder.customValidator = validator return blder @@ -254,9 +254,6 @@ func (blder *WebhookBuilder[T]) getDefaultingWebhook() *admission.Webhook { w = admission.WithDefaulter(blder.mgr.GetScheme(), blder.defaulter, blder.customDefaulterOpts...) } else if customDefaulter := blder.customDefaulter; customDefaulter != nil { w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, customDefaulter, blder.customDefaulterOpts...) - if blder.recoverPanic != nil { - } - return w } if w != nil && blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) @@ -298,12 +295,11 @@ func (blder *WebhookBuilder[T]) getValidatingWebhook() *admission.Webhook { w = admission.WithValidator(blder.mgr.GetScheme(), validator) } else if customValidator := blder.customValidator; customValidator != nil { w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, customValidator) - return w } if w != nil && blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) } - return nil + return w } func (blder *WebhookBuilder[T]) registerConversionWebhook() error { diff --git a/pkg/webhook/admission/defaulter_custom.go b/pkg/webhook/admission/defaulter_custom.go index d303c17f41..a159e2ceaf 100644 --- a/pkg/webhook/admission/defaulter_custom.go +++ b/pkg/webhook/admission/defaulter_custom.go @@ -36,6 +36,7 @@ type Defaulter[T runtime.Object] interface { Default(ctx context.Context, obj T) error } +// CustomDefaulter defines functions for setting defaults on resources. // Deprecated: CustomDefaulter is deprecated, use Defaulter instead type CustomDefaulter = Defaulter[runtime.Object] @@ -53,7 +54,7 @@ func DefaulterRemoveUnknownOrOmitableFields(o *defaulterOptions) { o.removeUnknownOrOmitableFields = true } -// WithDefaulter creates a new Webhook for a CustomDefaulter interface. +// WithDefaulter creates a new Webhook for a Defaulter interface. func WithDefaulter[T runtime.Object](scheme *runtime.Scheme, defaulter Defaulter[T], opts ...DefaulterOption) *Webhook { options := &defaulterOptions{} for _, o := range opts { @@ -69,6 +70,7 @@ func WithDefaulter[T runtime.Object](scheme *runtime.Scheme, defaulter Defaulter } } +// WithCustomDefaulter creates a new Webhook for a CustomDefaulter interface. func WithCustomDefaulter(scheme *runtime.Scheme, obj runtime.Object, defaulter CustomDefaulter, opts ...DefaulterOption) *Webhook { options := &defaulterOptions{} for _, o := range opts { diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index 5b8e7460c5..b37a6916bd 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -30,7 +30,7 @@ import ( // Warnings represents warning messages. type Warnings []string -// CustomValidator defines functions for validating an operation. +// Validator defines functions for validating an operation. // The object to be validated is passed into methods as a parameter. type Validator[T runtime.Object] interface { // ValidateCreate validates the object on creation. @@ -49,7 +49,8 @@ type Validator[T runtime.Object] interface { ValidateDelete(ctx context.Context, obj T) (warnings Warnings, err error) } -// deprecated: CustomValidator is deprecated, use Validator instead +// CustomValidator defines functions for validating an operation. +// Deprecated: CustomValidator is deprecated, use Validator instead type CustomValidator = Validator[runtime.Object] // WithValidator creates a new Webhook for validating the provided type. @@ -63,7 +64,8 @@ func WithValidator[T runtime.Object](scheme *runtime.Scheme, validator Validator } } -// deprecated: WithCustomValidator is deprecated, use WithValidator instead +// WithCustomValidator creates a new Webhook for a CustomValidator. +// Deprecated: WithCustomValidator is deprecated, use WithValidator instead func WithCustomValidator(scheme *runtime.Scheme, obj runtime.Object, validator CustomValidator) *Webhook { return &Webhook{ Handler: &validatorForType[runtime.Object]{ diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index 2882e7bab3..8338074c98 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -23,12 +23,6 @@ import ( // define some aliases for common bits of the webhook functionality -// CustomDefaulter defines functions for setting defaults on resources. -type CustomDefaulter = admission.CustomDefaulter - -// CustomValidator defines functions for validating an operation. -type CustomValidator = admission.CustomValidator - // AdmissionRequest defines the input for an admission handler. // It contains information to identify the object in // question (group, version, kind, resource, subresource, From 7a149e8219156a9379fdd9c83bb6633fa5a3e1f2 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Mon, 20 Oct 2025 19:55:40 -0400 Subject: [PATCH 06/15] Preserve alias for NewWebhookManagedBy --- alias.go | 4 ++-- examples/builtins/main.go | 2 +- examples/crd/main.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/alias.go b/alias.go index a294caeda2..dde2c5b930 100644 --- a/alias.go +++ b/alias.go @@ -154,7 +154,7 @@ var ( SetLogger = log.SetLogger ) -// WebhookManagedBy returns a new webhook builder for the provided type T. -func WebhookManagedBy[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] { +// NewWebhookManagedBy returns a new webhook builder for the provided type T. +func NewWebhookManagedBy[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] { return builder.WebhookManagedBy(mgr, obj) } diff --git a/examples/builtins/main.go b/examples/builtins/main.go index 650c921a4a..3ab1c50c1e 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -60,7 +60,7 @@ func main() { os.Exit(1) } - if err := ctrl.WebhookManagedBy(mgr, &corev1.Pod{}). + if err := ctrl.NewWebhookManagedBy(mgr, &corev1.Pod{}). WithAdmissionDefaulter(&podAnnotator{}). WithAdmissionValidator(&podValidator{}). Complete(); err != nil { diff --git a/examples/crd/main.go b/examples/crd/main.go index a910e7fc74..eec58abc01 100644 --- a/examples/crd/main.go +++ b/examples/crd/main.go @@ -129,7 +129,7 @@ func main() { os.Exit(1) } - err = ctrl.WebhookManagedBy(mgr, &api.ChaosPod{}). + err = ctrl.NewWebhookManagedBy(mgr, &api.ChaosPod{}). Complete() if err != nil { setupLog.Error(err, "unable to create webhook") From 2b33f300e81078f5d7abeaee4a150be5880877b9 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 29 Oct 2025 20:39:42 -0400 Subject: [PATCH 07/15] Use WithDefaulter/WithValidator going forward --- examples/builtins/main.go | 4 ++-- pkg/builder/webhook.go | 20 ++++++++++---------- pkg/builder/webhook_test.go | 30 +++++++++++++++--------------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/examples/builtins/main.go b/examples/builtins/main.go index 3ab1c50c1e..f30c652583 100644 --- a/examples/builtins/main.go +++ b/examples/builtins/main.go @@ -61,8 +61,8 @@ func main() { } if err := ctrl.NewWebhookManagedBy(mgr, &corev1.Pod{}). - WithAdmissionDefaulter(&podAnnotator{}). - WithAdmissionValidator(&podValidator{}). + WithDefaulter(&podAnnotator{}). + WithValidator(&podValidator{}). Complete(); err != nil { entryLog.Error(err, "unable to create webhook", "webhook", "Pod") os.Exit(1) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index a8e72cc993..f63226c9ae 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -62,37 +62,37 @@ func WebhookManagedBy[T runtime.Object](m manager.Manager, object T) *WebhookBui return &WebhookBuilder[T]{mgr: m, apiType: object} } -// WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption) +// WithCustomDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption) // will be wired for this type. // Deprecated: Use WithAdmissionDefaulter instead. -func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder[T] { +func (blder *WebhookBuilder[T]) WithCustomDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder[T] { blder.customDefaulter = defaulter blder.customDefaulterOpts = opts return blder } -// WithAdmissionDefaulter sets up the provided admission.Defaulter in a defaulting webhook. -func (blder *WebhookBuilder[T]) WithAdmissionDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] { +// WithDefaulter sets up the provided admission.Defaulter in a defaulting webhook. +func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] { blder.defaulter = defaulter blder.customDefaulterOpts = opts return blder } -// WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. +// WithCustomValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. // Deprecated: Use WithAdmissionValidator instead. -func (blder *WebhookBuilder[T]) WithValidator(validator admission.CustomValidator) *WebhookBuilder[T] { +func (blder *WebhookBuilder[T]) WithCustomValidator(validator admission.CustomValidator) *WebhookBuilder[T] { blder.customValidator = validator return blder } -// WithAdmissionValidator sets up the provided admission.Validator in a validating webhook. -func (blder *WebhookBuilder[T]) WithAdmissionValidator(validator admission.Validator[T]) *WebhookBuilder[T] { +// WithValidator sets up the provided admission.Validator in a validating webhook. +func (blder *WebhookBuilder[T]) WithValidator(validator admission.Validator[T]) *WebhookBuilder[T] { blder.validator = validator return blder } // WithConverter takes a func that constructs a converter.Converter. -// The Converter will then be used by the conversion endpoint for the type passed into For(). +// The Converter will then be used by the conversion endpoint for the type passed into NewWebhookManagedBy() func (blder *WebhookBuilder[T]) WithConverter(converterConstructor func(*runtime.Scheme) (conversion.Converter, error)) *WebhookBuilder[T] { blder.converterConstructor = converterConstructor return blder @@ -335,7 +335,7 @@ func (blder *WebhookBuilder[T]) getType() (runtime.Object, error) { if blder.apiType != nil { return blder.apiType, nil } - return nil, errors.New("For() must be called with a valid object") + return nil, errors.New("NewWebhookManagedBy() must be called with a valid object") } func (blder *WebhookBuilder[T]) isAlreadyHandled(path string) bool { diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index a93f187a95..03192e2baa 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -97,7 +97,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestDefaulter{}). - WithDefaulter(&TestCustomDefaulter{}). + WithCustomDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -173,7 +173,7 @@ func runTests(admissionReviewVersion string) { customPath := "/custom-defaulting-path" err = WebhookManagedBy(m, &TestDefaulter{}). - WithDefaulter(&TestCustomDefaulter{}). + WithCustomDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -250,7 +250,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestDefaulter{}). - WithDefaulter(&TestCustomDefaulter{}). + WithCustomDefaulter(&TestCustomDefaulter{}). RecoverPanic(true). // RecoverPanic defaults to true. Complete() @@ -313,7 +313,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestValidator{}). - WithValidator(&TestCustomValidator{}). + WithCustomValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -431,7 +431,7 @@ func runTests(admissionReviewVersion string) { customPath := "/custom-validating-path" err = WebhookManagedBy(m, &TestValidator{}). - WithValidator(&TestCustomValidator{}). + WithCustomValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -509,7 +509,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestValidator{}). - WithValidator(&TestCustomValidator{}). + WithCustomValidator(&TestCustomValidator{}). RecoverPanic(true). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -574,7 +574,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestValidator{}). - WithValidator(&TestCustomValidator{}). + WithCustomValidator(&TestCustomValidator{}). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) svr := m.GetWebhookServer() @@ -664,8 +664,8 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestDefaultValidator{}). - WithDefaulter(&TestCustomDefaultValidator{}). - WithValidator(&TestCustomDefaultValidator{}). + WithCustomDefaulter(&TestCustomDefaultValidator{}). + WithCustomValidator(&TestCustomDefaultValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -748,8 +748,8 @@ func runTests(admissionReviewVersion string) { validatingCustomPath := "/custom-validating-path" defaultingCustomPath := "/custom-defaulting-path" err = WebhookManagedBy(m, &TestDefaultValidator{}). - WithDefaulter(&TestCustomDefaultValidator{}). - WithValidator(&TestCustomDefaultValidator{}). + WithCustomDefaulter(&TestCustomDefaultValidator{}). + WithCustomValidator(&TestCustomDefaultValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -852,8 +852,8 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestDefaultValidator{}). - WithDefaulter(&TestCustomDefaultValidator{}). - WithValidator(&TestCustomDefaultValidator{}). + WithCustomDefaulter(&TestCustomDefaultValidator{}). + WithCustomValidator(&TestCustomDefaultValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -874,7 +874,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestDefaulter{}). - WithDefaulter(&TestCustomDefaulter{}). + WithCustomDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). @@ -896,7 +896,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m, &TestValidator{}). - WithValidator(&TestCustomValidator{}). + WithCustomValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). From 6fc8cb1368d51c5c4a4d13c5c6edce601d70eebc Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 29 Oct 2025 21:28:04 -0400 Subject: [PATCH 08/15] Test defaulter --- pkg/builder/webhook_test.go | 337 ++++++++++++---------- pkg/webhook/admission/defaulter_custom.go | 10 +- 2 files changed, 193 insertions(+), 154 deletions(-) diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 03192e2baa..cc36c32393 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -85,34 +85,36 @@ func runTests(admissionReviewVersion string) { close(stop) }) - It("should scaffold a custom defaulting webhook", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("scaffold a defaulting webhook", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestDefaulterObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} - builder.Register(&TestDefaulter{}, &TestDefaulterList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulterObject{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestDefaulter{}). - WithCustomDefaulter(&TestCustomDefaulter{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { - return admission.DefaultLogConstructor(testingLogger, req) - }). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + webhookBuilder := WebhookManagedBy(m, &TestDefaulterObject{}) + build(webhookBuilder) + err = webhookBuilder. + WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + return admission.DefaultLogConstructor(testingLogger, req) + }). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"foo.test.org", "version":"v1", - "kind":"TestDefaulter" + "kind":"TestDefaulterObject" }, "resource":{ "group":"foo.test.org", @@ -129,67 +131,76 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path") + path := generateMutatePath(testDefaulterGVK) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"patch":`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"code":200`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + + By("sending a request to a validating webhook path that doesn't exist") + path = generateValidatePath(testDefaulterGVK) + _, err = reader.Seek(0, 0) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } - - By("sending a request to a mutating webhook path") - path := generateMutatePath(testDefaulterGVK) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable fields") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - - By("sending a request to a validating webhook path that doesn't exist") - path = generateValidatePath(testDefaulterGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) - }) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + }, + Entry("Custom Defaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithCustomDefaulter(&TestCustomDefaulter{}) + }), + Entry("Defaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithDefaulter(&testDefaulter{}) + }), + ) - It("should scaffold a custom defaulting webhook with a custom path", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom defaulting webhook with a custom path", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestDefaulterObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} - builder.Register(&TestDefaulter{}, &TestDefaulterList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulterObject{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - customPath := "/custom-defaulting-path" - err = WebhookManagedBy(m, &TestDefaulter{}). - WithCustomDefaulter(&TestCustomDefaulter{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { - return admission.DefaultLogConstructor(testingLogger, req) - }). - WithDefaulterCustomPath(customPath). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + customPath := "/custom-defaulting-path" + webhookBuilder := WebhookManagedBy(m, &TestDefaulterObject{}) + build(webhookBuilder) + err = webhookBuilder. + WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + return admission.DefaultLogConstructor(testingLogger, req) + }). + WithDefaulterCustomPath(customPath). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"foo.test.org", "version":"v1", - "kind":"TestDefaulter" + "kind":"TestDefaulterObject" }, "resource":{ "group":"foo.test.org", @@ -206,65 +217,73 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } - By("sending a request to a mutating webhook path that have been overriten by a custom path") - path, err := generateCustomPath(customPath) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable fields") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - - By("sending a request to a mutating webhook path") - path = generateMutatePath(testDefaulterGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) - }) + By("sending a request to a mutating webhook path that have been overriten by a custom path") + path, err := generateCustomPath(customPath) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"patch":`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"code":200`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + + By("sending a request to a mutating webhook path") + path = generateMutatePath(testDefaulterGVK) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + }, + Entry("Custom Defaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithCustomDefaulter(&TestCustomDefaulter{}) + }), + Entry("Defaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithDefaulter(&testDefaulter{}) + }), + ) - It("should scaffold a custom defaulting webhook which recovers from panics", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom defaulting webhook which recovers from panics", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestDefaulterObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} - builder.Register(&TestDefaulter{}, &TestDefaulterList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulterObject{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestDefaulter{}). - WithCustomDefaulter(&TestCustomDefaulter{}). - RecoverPanic(true). - // RecoverPanic defaults to true. - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + webhookBuilder := WebhookManagedBy(m, &TestDefaulterObject{}) + build(webhookBuilder) + err = webhookBuilder. + RecoverPanic(true). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"", "version":"v1", - "kind":"TestDefaulter" + "kind":"TestDefaulterObject" }, "resource":{ "group":"", @@ -281,25 +300,32 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } - - By("sending a request to a mutating webhook path") - path := generateMutatePath(testDefaulterGVK) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable fields") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`)) - }) + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path") + path := generateMutatePath(testDefaulterGVK) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`)) + }, + Entry("CustomDefaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithCustomDefaulter(&TestCustomDefaulter{}) + }), + Entry("Defaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithDefaulter(&testDefaulter{}) + }), + ) It("should scaffold a custom validating webhook", func(specCtx SpecContext) { By("creating a controller manager") @@ -873,7 +899,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestDefaulter{}). + err = WebhookManagedBy(m, &TestDefaulterObject{}). WithCustomDefaulter(&TestCustomDefaulter{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -908,29 +934,29 @@ func runTests(admissionReviewVersion string) { } // TestDefaulter. -var _ runtime.Object = &TestDefaulter{} +var _ runtime.Object = &TestDefaulterObject{} -const testDefaulterKind = "TestDefaulter" +const testDefaulterKind = "TestDefaulterObject" -type TestDefaulter struct { +type TestDefaulterObject struct { Replica int `json:"replica,omitempty"` Panic bool `json:"panic,omitempty"` } var testDefaulterGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: testDefaulterKind} -func (d *TestDefaulter) GetObjectKind() schema.ObjectKind { return d } -func (d *TestDefaulter) DeepCopyObject() runtime.Object { - return &TestDefaulter{ +func (d *TestDefaulterObject) GetObjectKind() schema.ObjectKind { return d } +func (d *TestDefaulterObject) DeepCopyObject() runtime.Object { + return &TestDefaulterObject{ Replica: d.Replica, } } -func (d *TestDefaulter) GroupVersionKind() schema.GroupVersionKind { +func (d *TestDefaulterObject) GroupVersionKind() schema.GroupVersionKind { return testDefaulterGVK } -func (d *TestDefaulter) SetGroupVersionKind(gvk schema.GroupVersionKind) {} +func (d *TestDefaulterObject) SetGroupVersionKind(gvk schema.GroupVersionKind) {} var _ runtime.Object = &TestDefaulterList{} @@ -1005,10 +1031,16 @@ type TestDefaultValidatorList struct{} func (*TestDefaultValidatorList) GetObjectKind() schema.ObjectKind { return nil } func (*TestDefaultValidatorList) DeepCopyObject() runtime.Object { return nil } -// TestCustomDefaulter. type TestCustomDefaulter struct{} func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { + d := obj.(*TestDefaulterObject) //nolint:ifshort + return (&testDefaulter{}).Default(ctx, d) +} + +type testDefaulter struct{} + +func (*testDefaulter) Default(ctx context.Context, obj *TestDefaulterObject) error { logf.FromContext(ctx).Info("Defaulting object") req, err := admission.RequestFromContext(ctx) if err != nil { @@ -1018,13 +1050,12 @@ func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) err return fmt.Errorf("expected Kind TestDefaulter got %q", req.Kind.Kind) } - d := obj.(*TestDefaulter) //nolint:ifshort - if d.Panic { + if obj.Panic { panic("fake panic test") } - if d.Replica < 2 { - d.Replica = 2 + if obj.Replica < 2 { + obj.Replica = 2 } return nil diff --git a/pkg/webhook/admission/defaulter_custom.go b/pkg/webhook/admission/defaulter_custom.go index a159e2ceaf..1dc8af10ee 100644 --- a/pkg/webhook/admission/defaulter_custom.go +++ b/pkg/webhook/admission/defaulter_custom.go @@ -21,6 +21,7 @@ import ( "encoding/json" "errors" "net/http" + "reflect" "slices" "gomodules.xyz/jsonpatch/v2" @@ -65,7 +66,14 @@ func WithDefaulter[T runtime.Object](scheme *runtime.Scheme, defaulter Defaulter defaulter: defaulter, decoder: NewDecoder(scheme), removeUnknownOrOmitableFields: options.removeUnknownOrOmitableFields, - new: func() T { return *new(T) }, + new: func() T { + var zero T + typ := reflect.TypeOf(zero) + if typ.Kind() == reflect.Ptr { + return reflect.New(typ.Elem()).Interface().(T) + } + return zero + }, }, } } From cad07365bcac659ed18b580a3954ccf4fee1904f Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 29 Oct 2025 21:55:05 -0400 Subject: [PATCH 09/15] TestValidatorBuilder --- pkg/builder/webhook_test.go | 482 ++++++++++++---------- pkg/webhook/admission/validator_custom.go | 10 +- 2 files changed, 272 insertions(+), 220 deletions(-) diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index cc36c32393..c8b55737c5 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -327,37 +327,38 @@ func runTests(admissionReviewVersion string) { }), ) - It("should scaffold a custom validating webhook", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom validating webhook", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestValidatorObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestValidator{}, &TestValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidatorObject{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestValidator{}). - WithCustomValidator(&TestCustomValidator{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + webhook := WebhookManagedBy(m, &TestValidatorObject{}) + build(webhook) + err = webhook.WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). - WithContextFunc(func(ctx context.Context, request *http.Request) context.Context { - return context.WithValue(ctx, userAgentCtxKey, request.Header.Get(userAgentHeader)) - }). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + WithContextFunc(func(ctx context.Context, request *http.Request) context.Context { + return context.WithValue(ctx, userAgentCtxKey, request.Header.Get(userAgentHeader)) + }). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"foo.test.org", "version":"v1", - "kind":"TestValidator" + "kind":"TestValidatorObject" }, "resource":{ "group":"foo.test.org", @@ -375,13 +376,13 @@ func runTests(admissionReviewVersion string) { } } }`) - readerWithCxt := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + readerWithCxt := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"foo.test.org", "version":"v1", - "kind":"TestValidator" + "kind":"TestValidatorObject" }, "resource":{ "group":"foo.test.org", @@ -400,80 +401,88 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } - - By("sending a request to a mutating webhook path that doesn't exist") - path := generateMutatePath(testValidatorGVK) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } - By("sending a request to a validating webhook path") - path = generateValidatePath(testValidatorGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + By("sending a request to a mutating webhook path that doesn't exist") + path := generateMutatePath(testValidatorGVK) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) - By("sending a request to a validating webhook with context header validation") - path = generateValidatePath(testValidatorGVK) - _, err = readerWithCxt.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, readerWithCxt) - req.Header.Add("Content-Type", "application/json") - req.Header.Add(userAgentHeader, userAgentValue) - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - }) + By("sending a request to a validating webhook path") + path = generateValidatePath(testValidatorGVK) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"code":403`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + + By("sending a request to a validating webhook with context header validation") + path = generateValidatePath(testValidatorGVK) + _, err = readerWithCxt.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", svcBaseAddr+path, readerWithCxt) + req.Header.Add("Content-Type", "application/json") + req.Header.Add(userAgentHeader, userAgentValue) + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"code":200`)) + }, + Entry("CustomValidator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithCustomValidator(&TestCustomValidator{}) + }), + Entry("Validator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithValidator(&testValidator{}) + }), + ) - It("should scaffold a custom validating webhook with a custom path", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom validating webhook with a custom path", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestValidatorObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestValidator{}, &TestValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidatorObject{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - customPath := "/custom-validating-path" - err = WebhookManagedBy(m, &TestValidator{}). - WithCustomValidator(&TestCustomValidator{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + customPath := "/custom-validating-path" + webhookBuilder := WebhookManagedBy(m, &TestValidatorObject{}) + build(webhookBuilder) + err = webhookBuilder.WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) }). - WithValidatorCustomPath(customPath). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + WithValidatorCustomPath(customPath). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"foo.test.org", "version":"v1", - "kind":"TestValidator" + "kind":"TestValidatorObject" }, "resource":{ "group":"foo.test.org", @@ -492,63 +501,71 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } - By("sending a request to a valiting webhook path that have been overriten by a custom path") - path, err := generateCustomPath(customPath) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + By("sending a request to a valiting webhook path that have been overriten by a custom path") + path, err := generateCustomPath(customPath) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body.String()).To(ContainSubstring(`"code":403`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - By("sending a request to a validating webhook path") - path = generateValidatePath(testValidatorGVK) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) - }) + By("sending a request to a validating webhook path") + path = generateValidatePath(testValidatorGVK) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + }, + Entry("CustomValidator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithCustomValidator(&TestCustomValidator{}) + }), + Entry("Validator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithValidator(&testValidator{}) + }), + ) - It("should scaffold a custom validating webhook which recovers from panics", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom validating webhook which recovers from panics", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestValidatorObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestValidator{}, &TestValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidatorObject{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestValidator{}). - WithCustomValidator(&TestCustomValidator{}). - RecoverPanic(true). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + webhookBuilder := WebhookManagedBy(m, &TestValidatorObject{}) + build(webhookBuilder) + err = webhookBuilder.RecoverPanic(true). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"", "version":"v1", - "kind":"TestValidator" + "kind":"TestValidatorObject" }, "resource":{ "group":"", @@ -564,55 +581,63 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } - By("sending a request to a validating webhook path") - path := generateValidatePath(testValidatorGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`)) - }) + By("sending a request to a validating webhook path") + path := generateValidatePath(testValidatorGVK) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`)) + }, + Entry("CustomValidator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithCustomValidator(&TestCustomValidator{}) + }), + Entry("Validator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithValidator(&testValidator{}) + }), + ) - It("should scaffold a custom validating webhook to validate deletes", func(specCtx SpecContext) { - By("creating a controller manager") - ctx, cancel := context.WithCancel(specCtx) + DescribeTable("should scaffold a custom validating webhook to validate deletes", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestValidatorObject])) { + By("creating a controller manager") + ctx, cancel := context.WithCancel(specCtx) - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestValidator{}, &TestValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidatorObject{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestValidator{}). - WithCustomValidator(&TestCustomValidator{}). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + webhookBuilder := WebhookManagedBy(m, &TestValidatorObject{}) + build(webhookBuilder) + err = webhookBuilder.Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"", "version":"v1", - "kind":"TestValidator" + "kind":"TestValidatorObject" }, "resource":{ "group":"", @@ -628,30 +653,30 @@ func runTests(admissionReviewVersion string) { } }`) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } - By("sending a request to a validating webhook path to check for failed delete") - path := generateValidatePath(testValidatorGVK) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) + By("sending a request to a validating webhook path to check for failed delete") + path := generateValidatePath(testValidatorGVK) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) - reader = strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader = strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ "group":"", "version":"v1", - "kind":"TestValidator" + "kind":"TestValidatorObject" }, "resource":{ "group":"", @@ -666,17 +691,24 @@ func runTests(admissionReviewVersion string) { } } }`) - By("sending a request to a validating webhook path with correct request") - path = generateValidatePath(testValidatorGVK) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - }) + By("sending a request to a validating webhook path with correct request") + path = generateValidatePath(testValidatorGVK) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) + }, + Entry("CustomValidator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithCustomValidator(&TestCustomValidator{}) + }), + Entry("Validator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithValidator(&testValidator{}) + }), + ) It("should scaffold a custom defaulting and validating webhook", func(specCtx SpecContext) { By("creating a controller manager") @@ -921,7 +953,7 @@ func runTests(admissionReviewVersion string) { err = builder.AddToScheme(m.GetScheme()) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestValidator{}). + err = WebhookManagedBy(m, &TestValidatorObject{}). WithCustomValidator(&TestCustomValidator{}). WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { return admission.DefaultLogConstructor(testingLogger, req) @@ -966,29 +998,29 @@ func (*TestDefaulterList) GetObjectKind() schema.ObjectKind { return nil } func (*TestDefaulterList) DeepCopyObject() runtime.Object { return nil } // TestValidator. -var _ runtime.Object = &TestValidator{} +var _ runtime.Object = &TestValidatorObject{} -const testValidatorKind = "TestValidator" +const testValidatorKind = "TestValidatorObject" -type TestValidator struct { +type TestValidatorObject struct { Replica int `json:"replica,omitempty"` Panic bool `json:"panic,omitempty"` } var testValidatorGVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: testValidatorKind} -func (v *TestValidator) GetObjectKind() schema.ObjectKind { return v } -func (v *TestValidator) DeepCopyObject() runtime.Object { - return &TestValidator{ +func (v *TestValidatorObject) GetObjectKind() schema.ObjectKind { return v } +func (v *TestValidatorObject) DeepCopyObject() runtime.Object { + return &TestValidatorObject{ Replica: v.Replica, } } -func (v *TestValidator) GroupVersionKind() schema.GroupVersionKind { +func (v *TestValidatorObject) GroupVersionKind() schema.GroupVersionKind { return testValidatorGVK } -func (v *TestValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {} +func (v *TestValidatorObject) SetGroupVersionKind(gvk schema.GroupVersionKind) {} var _ runtime.Object = &TestValidatorList{} @@ -1063,11 +1095,27 @@ func (*testDefaulter) Default(ctx context.Context, obj *TestDefaulterObject) err var _ admission.CustomDefaulter = &TestCustomDefaulter{} -// TestCustomValidator. - type TestCustomValidator struct{} func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + v := obj.(*TestValidatorObject) //nolint:ifshort + return (&testValidator{}).ValidateCreate(ctx, v) +} + +func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + v := newObj.(*TestValidatorObject) + old := oldObj.(*TestValidatorObject) + return (&testValidator{}).ValidateUpdate(ctx, old, v) +} + +func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + v := obj.(*TestValidatorObject) //nolint:ifshort + return (&testValidator{}).ValidateDelete(ctx, v) +} + +type testValidator struct{} + +func (*testValidator) ValidateCreate(ctx context.Context, obj *TestValidatorObject) (admission.Warnings, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { @@ -1077,18 +1125,17 @@ func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Obje return nil, fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) } - v := obj.(*TestValidator) //nolint:ifshort - if v.Panic { + if obj.Panic { panic("fake panic test") } - if v.Replica < 0 { + if obj.Replica < 0 { return nil, errors.New("number of replica should be greater than or equal to 0") } return nil, nil } -func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { +func (*testValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *TestValidatorObject) (admission.Warnings, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { @@ -1098,13 +1145,11 @@ func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj r return nil, fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) } - v := newObj.(*TestValidator) - old := oldObj.(*TestValidator) - if v.Replica < 0 { + if newObj.Replica < 0 { return nil, errors.New("number of replica should be greater than or equal to 0") } - if v.Replica < old.Replica { - return nil, fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, old.Replica) + if newObj.Replica < oldObj.Replica { + return nil, fmt.Errorf("new replica %v should not be fewer than old replica %v", oldObj.Replica, newObj.Replica) } userAgent, ok := ctx.Value(userAgentCtxKey).(string) @@ -1115,7 +1160,7 @@ func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj r return nil, nil } -func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { +func (*testValidator) ValidateDelete(ctx context.Context, obj *TestValidatorObject) (admission.Warnings, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { @@ -1125,8 +1170,7 @@ func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Obje return nil, fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) } - v := obj.(*TestValidator) //nolint:ifshort - if v.Replica > 0 { + if obj.Replica > 0 { return nil, errors.New("number of replica should be less than or equal to 0 to delete") } return nil, nil diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index b37a6916bd..26c0828c5e 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "net/http" + "reflect" v1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -59,7 +60,14 @@ func WithValidator[T runtime.Object](scheme *runtime.Scheme, validator Validator Handler: &validatorForType[T]{ validator: validator, decoder: NewDecoder(scheme), - new: func() runtime.Object { return *new(T) }, + new: func() runtime.Object { + var zero T + typ := reflect.TypeOf(zero) + if typ.Kind() == reflect.Ptr { + return reflect.New(typ.Elem()).Interface().(T) + } + return zero + }, }, } } From 4344a72fbb07226d8df8be4db601085857c52367 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 29 Oct 2025 22:11:57 -0400 Subject: [PATCH 10/15] Mixed --- pkg/builder/webhook_test.go | 408 +++++++++++++++++++++--------------- 1 file changed, 241 insertions(+), 167 deletions(-) diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index c8b55737c5..cbfd6ba6be 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -710,29 +710,30 @@ func runTests(admissionReviewVersion string) { }), ) - It("should scaffold a custom defaulting and validating webhook", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom defaulting and validating webhook", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestDefaultValidator])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestDefaultValidator{}). - WithCustomDefaulter(&TestCustomDefaultValidator{}). - WithCustomValidator(&TestCustomDefaultValidator{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { - return admission.DefaultLogConstructor(testingLogger, req) - }). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + webhookBuilder := WebhookManagedBy(m, &TestDefaultValidator{}) + build(webhookBuilder) + err = webhookBuilder. + WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + return admission.DefaultLogConstructor(testingLogger, req) + }). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ @@ -757,68 +758,86 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path") + path := generateMutatePath(testDefaultValidatorGVK) + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + + By("sending a request to a validating webhook path") + path = generateValidatePath(testDefaultValidatorGVK) + _, err = reader.Seek(0, 0) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } - - By("sending a request to a mutating webhook path") - path := generateMutatePath(testDefaultValidatorGVK) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable fields") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - - By("sending a request to a validating webhook path") - path = generateValidatePath(testDefaultValidatorGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - }) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + }, + Entry("CustomDefaulter + CustomValidator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithCustomDefaulter(&TestCustomDefaultValidator{}) + b.WithCustomValidator(&TestCustomDefaultValidator{}) + }), + Entry("CustomDefaulter + Validator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithCustomDefaulter(&TestCustomDefaultValidator{}) + b.WithValidator(&testDefaultValidatorValidator{}) + }), + Entry("Defaulter + CustomValidator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithDefaulter(&testValidatorDefaulter{}) + b.WithCustomValidator(&TestCustomDefaultValidator{}) + }), + Entry("Defaulter + Validator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithDefaulter(&testValidatorDefaulter{}) + b.WithValidator(&testDefaultValidatorValidator{}) + }), + ) - It("should scaffold a custom defaulting and validating webhook with a custom path for each of them", func(specCtx SpecContext) { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should scaffold a custom defaulting and validating webhook with a custom path for each of them", + func(specCtx SpecContext, build func(*WebhookBuilder[*TestDefaultValidator])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - validatingCustomPath := "/custom-validating-path" - defaultingCustomPath := "/custom-defaulting-path" - err = WebhookManagedBy(m, &TestDefaultValidator{}). - WithCustomDefaulter(&TestCustomDefaultValidator{}). - WithCustomValidator(&TestCustomDefaultValidator{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { - return admission.DefaultLogConstructor(testingLogger, req) - }). - WithValidatorCustomPath(validatingCustomPath). - WithDefaulterCustomPath(defaultingCustomPath). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) + validatingCustomPath := "/custom-validating-path" + defaultingCustomPath := "/custom-defaulting-path" + webhookBuilder := WebhookManagedBy(m, &TestDefaultValidator{}) + build(webhookBuilder) + err = webhookBuilder. + WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + return admission.DefaultLogConstructor(testingLogger, req) + }). + WithValidatorCustomPath(validatingCustomPath). + WithDefaulterCustomPath(defaultingCustomPath). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", + reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", "request":{ "uid":"07e52e8d-4513-11e9-a716-42010a800270", "kind":{ @@ -843,60 +862,77 @@ func runTests(admissionReviewVersion string) { } }`) - ctx, cancel := context.WithCancel(specCtx) - cancel() - err = svr.Start(ctx) - if err != nil && !os.IsNotExist(err) { + ctx, cancel := context.WithCancel(specCtx) + cancel() + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path that have been overriten by the custom path") + path, err := generateCustomPath(defaultingCustomPath) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - } + req := httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - By("sending a request to a mutating webhook path that have been overriten by the custom path") - path, err := generateCustomPath(defaultingCustomPath) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req := httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w := httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable fields") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Defaulting object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - - By("sending a request to a mutating webhook path") - path = generateMutatePath(testDefaultValidatorGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) - - By("sending a request to a valiting webhook path that have been overriten by a custom path") - path, err = generateCustomPath(validatingCustomPath) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) - By("sanity checking the response contains reasonable field") - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) - EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) - - By("sending a request to a validating webhook path") - path = generateValidatePath(testValidatorGVK) - req = httptest.NewRequest("POST", svcBaseAddr+path, reader) - req.Header.Add("Content-Type", "application/json") - w = httptest.NewRecorder() - svr.WebhookMux().ServeHTTP(w, req) - ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) - }) + By("sending a request to a mutating webhook path") + path = generateMutatePath(testDefaultValidatorGVK) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + + By("sending a request to a valiting webhook path that have been overriten by a custom path") + path, err = generateCustomPath(validatingCustomPath) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) + EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) + + By("sending a request to a validating webhook path") + path = generateValidatePath(testValidatorGVK) + req = httptest.NewRequest("POST", svcBaseAddr+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux().ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + }, + Entry("CustomDefaulter + CustomValidator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithCustomDefaulter(&TestCustomDefaultValidator{}) + b.WithCustomValidator(&TestCustomDefaultValidator{}) + }), + Entry("CustomDefaulter + Validator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithCustomDefaulter(&TestCustomDefaultValidator{}) + b.WithValidator(&testDefaultValidatorValidator{}) + }), + Entry("Defaulter + CustomValidator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithDefaulter(&testValidatorDefaulter{}) + b.WithCustomValidator(&TestCustomDefaultValidator{}) + }), + Entry("Defaulter + Validator", func(b *WebhookBuilder[*TestDefaultValidator]) { + b.WithDefaulter(&testValidatorDefaulter{}) + b.WithValidator(&testDefaultValidatorValidator{}) + }), + ) It("should not scaffold a custom defaulting and a custom validating webhook with the same custom path", func() { By("creating a controller manager") @@ -920,49 +956,67 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).To(HaveOccurred()) }) - It("should not scaffold a custom defaulting when setting a custom path and a defaulting custom path", func() { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should not scaffold a custom defaulting when setting a custom path and a defaulting custom path", + func(build func(*WebhookBuilder[*TestDefaulterObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulterObject{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestDefaulterObject{}). - WithCustomDefaulter(&TestCustomDefaulter{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { - return admission.DefaultLogConstructor(testingLogger, req) - }). - WithDefaulterCustomPath(customPath). - WithCustomPath(customPath). - Complete() - ExpectWithOffset(1, err).To(HaveOccurred()) - }) + webhookBuilder := WebhookManagedBy(m, &TestDefaulterObject{}) + build(webhookBuilder) + err = webhookBuilder. + WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + return admission.DefaultLogConstructor(testingLogger, req) + }). + WithDefaulterCustomPath(customPath). + WithCustomPath(customPath). + Complete() + ExpectWithOffset(1, err).To(HaveOccurred()) + }, + Entry("CustomDefaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithCustomDefaulter(&TestCustomDefaulter{}) + }), + Entry("Defaulter", func(b *WebhookBuilder[*TestDefaulterObject]) { + b.WithDefaulter(&testDefaulter{}) + }), + ) - It("should not scaffold a custom defaulting when setting a custom path and a validating custom path", func() { - By("creating a controller manager") - m, err := manager.New(cfg, manager.Options{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + DescribeTable("should not scaffold a custom validating when setting a custom path and a validating custom path", + func(build func(*WebhookBuilder[*TestValidatorObject])) { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} - builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidatorObject{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) - err = WebhookManagedBy(m, &TestValidatorObject{}). - WithCustomValidator(&TestCustomValidator{}). - WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { - return admission.DefaultLogConstructor(testingLogger, req) - }). - WithDefaulterCustomPath(customPath). - WithCustomPath(customPath). - Complete() - ExpectWithOffset(1, err).To(HaveOccurred()) - }) + webhookBuilder := WebhookManagedBy(m, &TestValidatorObject{}) + build(webhookBuilder) + err = webhookBuilder. + WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger { + return admission.DefaultLogConstructor(testingLogger, req) + }). + WithValidatorCustomPath(customPath). + WithCustomPath(customPath). + Complete() + ExpectWithOffset(1, err).To(HaveOccurred()) + }, + Entry("CustomValidator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithCustomValidator(&TestCustomValidator{}) + }), + Entry("Validator", func(b *WebhookBuilder[*TestValidatorObject]) { + b.WithValidator(&testValidator{}) + }), + ) } // TestDefaulter. @@ -1259,3 +1313,23 @@ func (*TestCustomDefaultValidator) ValidateDelete(ctx context.Context, obj runti } var _ admission.CustomValidator = &TestCustomValidator{} + +type testValidatorDefaulter struct{} + +func (*testValidatorDefaulter) Default(ctx context.Context, obj *TestDefaultValidator) error { + return (&TestCustomDefaultValidator{}).Default(ctx, obj) +} + +type testDefaultValidatorValidator struct{} + +func (*testDefaultValidatorValidator) ValidateCreate(ctx context.Context, obj *TestDefaultValidator) (admission.Warnings, error) { + return (&TestCustomDefaultValidator{}).ValidateCreate(ctx, obj) +} + +func (*testDefaultValidatorValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *TestDefaultValidator) (admission.Warnings, error) { + return (&TestCustomDefaultValidator{}).ValidateUpdate(ctx, oldObj, newObj) +} + +func (*testDefaultValidatorValidator) ValidateDelete(ctx context.Context, obj *TestDefaultValidator) (admission.Warnings, error) { + return (&TestCustomDefaultValidator{}).ValidateDelete(ctx, obj) +} From 3c11c9833a8abf8123587f04c2c716c9b33e0879 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Wed, 29 Oct 2025 23:20:13 -0400 Subject: [PATCH 11/15] Custom and Typed validator/defaulter are mutually exclusive --- pkg/builder/webhook.go | 24 ++++++++++++++++++------ pkg/builder/webhook_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index f63226c9ae..6c66a6d46b 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -222,7 +222,10 @@ func (blder *WebhookBuilder[T]) registerWebhooks() error { // registerDefaultingWebhook registers a defaulting webhook if necessary. func (blder *WebhookBuilder[T]) registerDefaultingWebhook() error { - mwh := blder.getDefaultingWebhook() + mwh, err := blder.getDefaultingWebhook() + if err != nil { + return err + } if mwh != nil { mwh.LogConstructor = blder.logConstructor mwh.WithContextFunc = blder.contextFunc @@ -248,9 +251,12 @@ func (blder *WebhookBuilder[T]) registerDefaultingWebhook() error { return nil } -func (blder *WebhookBuilder[T]) getDefaultingWebhook() *admission.Webhook { +func (blder *WebhookBuilder[T]) getDefaultingWebhook() (*admission.Webhook, error) { var w *admission.Webhook if defaulter := blder.defaulter; defaulter != nil { + if blder.customDefaulter != nil { + return nil, errors.New("only one of Defaulter or CustomDefaulter can be set") + } w = admission.WithDefaulter(blder.mgr.GetScheme(), blder.defaulter, blder.customDefaulterOpts...) } else if customDefaulter := blder.customDefaulter; customDefaulter != nil { w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, customDefaulter, blder.customDefaulterOpts...) @@ -258,12 +264,15 @@ func (blder *WebhookBuilder[T]) getDefaultingWebhook() *admission.Webhook { if w != nil && blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) } - return w + return w, nil } // registerValidatingWebhook registers a validating webhook if necessary. func (blder *WebhookBuilder[T]) registerValidatingWebhook() error { - vwh := blder.getValidatingWebhook() + vwh, err := blder.getValidatingWebhook() + if err != nil { + return err + } if vwh != nil { vwh.LogConstructor = blder.logConstructor vwh.WithContextFunc = blder.contextFunc @@ -289,9 +298,12 @@ func (blder *WebhookBuilder[T]) registerValidatingWebhook() error { return nil } -func (blder *WebhookBuilder[T]) getValidatingWebhook() *admission.Webhook { +func (blder *WebhookBuilder[T]) getValidatingWebhook() (*admission.Webhook, error) { var w *admission.Webhook if validator := blder.validator; validator != nil { + if blder.customValidator != nil { + return nil, errors.New("only one of Validator or CustomValidator can be set") + } w = admission.WithValidator(blder.mgr.GetScheme(), validator) } else if customValidator := blder.customValidator; customValidator != nil { w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, customValidator) @@ -299,7 +311,7 @@ func (blder *WebhookBuilder[T]) getValidatingWebhook() *admission.Webhook { if w != nil && blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) } - return w + return w, nil } func (blder *WebhookBuilder[T]) registerConversionWebhook() error { diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index cbfd6ba6be..a8bfd24654 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -1017,6 +1017,39 @@ func runTests(admissionReviewVersion string) { b.WithValidator(&testValidator{}) }), ) + + It("should error if both a defaulter and a custom defaulter are set", func() { + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulterObject{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + err = WebhookManagedBy(m, &TestDefaulterObject{}). + WithDefaulter(&testDefaulter{}). + WithCustomDefaulter(&TestCustomDefaulter{}). + Complete() + ExpectWithOffset(1, err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("only one of Defaulter or CustomDefaulter can be set")) + }) + It("should error if both a validator and a custom validator are set", func() { + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidatorObject{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + err = WebhookManagedBy(m, &TestValidatorObject{}). + WithValidator(&testValidator{}). + WithCustomValidator(&TestCustomValidator{}). + Complete() + ExpectWithOffset(1, err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("only one of Validator or CustomValidator can be set")) + }) } // TestDefaulter. From 480d040a680e8a5f24e6120e218e0de8cfe8e6c6 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Thu, 30 Oct 2025 18:56:54 -0400 Subject: [PATCH 12/15] Type new in validator --- pkg/webhook/admission/validator_custom.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index 26c0828c5e..7f59119367 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -60,7 +60,7 @@ func WithValidator[T runtime.Object](scheme *runtime.Scheme, validator Validator Handler: &validatorForType[T]{ validator: validator, decoder: NewDecoder(scheme), - new: func() runtime.Object { + new: func() T { var zero T typ := reflect.TypeOf(zero) if typ.Kind() == reflect.Ptr { @@ -87,7 +87,7 @@ func WithCustomValidator(scheme *runtime.Scheme, obj runtime.Object, validator C type validatorForType[T runtime.Object] struct { validator Validator[T] decoder Decoder - new func() runtime.Object + new func() T } // Handle handles admission requests. @@ -101,7 +101,7 @@ func (h *validatorForType[T]) Handle(ctx context.Context, req Request) Response ctx = NewContextWithRequest(ctx, req) - obj := h.new().(T) + obj := h.new() var err error var warnings []string From 2e246e9dcc0c848dcc39f53f1c66544453f4e577 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Thu, 30 Oct 2025 18:59:50 -0400 Subject: [PATCH 13/15] Simplify builder --- pkg/builder/webhook.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 6c66a6d46b..2db83d5eea 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -253,13 +253,13 @@ func (blder *WebhookBuilder[T]) registerDefaultingWebhook() error { func (blder *WebhookBuilder[T]) getDefaultingWebhook() (*admission.Webhook, error) { var w *admission.Webhook - if defaulter := blder.defaulter; defaulter != nil { + if blder.defaulter != nil { if blder.customDefaulter != nil { return nil, errors.New("only one of Defaulter or CustomDefaulter can be set") } w = admission.WithDefaulter(blder.mgr.GetScheme(), blder.defaulter, blder.customDefaulterOpts...) - } else if customDefaulter := blder.customDefaulter; customDefaulter != nil { - w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, customDefaulter, blder.customDefaulterOpts...) + } else if blder.customDefaulter != nil { + w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, blder.customDefaulter, blder.customDefaulterOpts...) } if w != nil && blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) @@ -300,13 +300,13 @@ func (blder *WebhookBuilder[T]) registerValidatingWebhook() error { func (blder *WebhookBuilder[T]) getValidatingWebhook() (*admission.Webhook, error) { var w *admission.Webhook - if validator := blder.validator; validator != nil { + if blder.validator != nil { if blder.customValidator != nil { return nil, errors.New("only one of Validator or CustomValidator can be set") } - w = admission.WithValidator(blder.mgr.GetScheme(), validator) - } else if customValidator := blder.customValidator; customValidator != nil { - w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, customValidator) + w = admission.WithValidator(blder.mgr.GetScheme(), blder.validator) + } else if blder.customValidator != nil { + w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, blder.customValidator) } if w != nil && blder.recoverPanic != nil { w = w.WithRecoverPanic(*blder.recoverPanic) From 8f658345733ebdbd7eacca23d824074ce81fa0db Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Thu, 30 Oct 2025 19:01:56 -0400 Subject: [PATCH 14/15] Re-add aliases --- pkg/webhook/admission/validator_custom.go | 2 +- pkg/webhook/alias.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index 7f59119367..abd68e88bf 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -117,7 +117,7 @@ func (h *validatorForType[T]) Handle(ctx context.Context, req Request) Response warnings, err = h.validator.ValidateCreate(ctx, obj) case v1.Update: - oldObj := h.new().(T) + oldObj := h.new() if err := h.decoder.DecodeRaw(req.Object, obj); err != nil { return Errored(http.StatusBadRequest, err) } diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index 8338074c98..b4f16a3f5f 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -23,6 +23,14 @@ import ( // define some aliases for common bits of the webhook functionality +// CustomDefaulter defines functions for setting defaults on resources. +// Deprecated: Use admission.Defaulter instead. +type CustomDefaulter = admission.CustomDefaulter + +// CustomValidator defines functions for validating an operation. +// Deprecated: Use admission.Validator instead. +type CustomValidator = admission.CustomValidator + // AdmissionRequest defines the input for an admission handler. // It contains information to identify the object in // question (group, version, kind, resource, subresource, From 31c6c998ef1449b57de6920e57c9bb07d2845d57 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Thu, 30 Oct 2025 19:15:48 -0400 Subject: [PATCH 15/15] feedback --- pkg/builder/webhook.go | 4 ++-- pkg/builder/webhook_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 2db83d5eea..428100a66c 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -64,7 +64,7 @@ func WebhookManagedBy[T runtime.Object](m manager.Manager, object T) *WebhookBui // WithCustomDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption) // will be wired for this type. -// Deprecated: Use WithAdmissionDefaulter instead. +// Deprecated: Use WithDefaulter instead. func (blder *WebhookBuilder[T]) WithCustomDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder[T] { blder.customDefaulter = defaulter blder.customDefaulterOpts = opts @@ -79,7 +79,7 @@ func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.Defaulter[T], } // WithCustomValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type. -// Deprecated: Use WithAdmissionValidator instead. +// Deprecated: Use WithValidator instead. func (blder *WebhookBuilder[T]) WithCustomValidator(validator admission.CustomValidator) *WebhookBuilder[T] { blder.customValidator = validator return blder diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index a8bfd24654..e10e693ab8 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -1236,7 +1236,7 @@ func (*testValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *TestVa return nil, errors.New("number of replica should be greater than or equal to 0") } if newObj.Replica < oldObj.Replica { - return nil, fmt.Errorf("new replica %v should not be fewer than old replica %v", oldObj.Replica, newObj.Replica) + return nil, fmt.Errorf("new replica %v should not be fewer than old replica %v", newObj.Replica, oldObj.Replica) } userAgent, ok := ctx.Value(userAgentCtxKey).(string)