Skip to content

Commit 48a1c08

Browse files
alvaroalemanalvaroaleman
committed
Update existing WebhookManagedBy
1 parent fc15d4a commit 48a1c08

File tree

6 files changed

+61
-107
lines changed

6 files changed

+61
-107
lines changed

‎alias.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ var (
105105
// NewControllerManagedBy returns a new controller builder that will be started by the provided Manager.
106106
NewControllerManagedBy = builder.ControllerManagedBy
107107

108-
// NewWebhookManagedBy returns a new webhook builder that will be started by the provided Manager.
109-
// Deprecated: Use NewWebhookFor instead.
110-
NewWebhookManagedBy = builder.WebhookManagedBy
111-
112108
// NewManager returns a new Manager for creating Controllers.
113109
// Note that if ContentType in the given config is not set, "application/vnd.kubernetes.protobuf"
114110
// will be used for all built-in resources of Kubernetes, and "application/json" is for other types
@@ -158,7 +154,7 @@ var (
158154
SetLogger = log.SetLogger
159155
)
160156

161-
// NewWebhookFor returns a new webhook builder for the provided type T.
162-
func NewWebhookFor[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] {
163-
return builder.WebhookFor(mgr, obj)
157+
// WebhookManagedBy returns a new webhook builder for the provided type T.
158+
func WebhookManagedBy[T runtime.Object](mgr manager.Manager, obj T) *builder.WebhookBuilder[T] {
159+
return builder.WebhookManagedBy(mgr, obj)
164160
}

‎examples/builtins/main.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ func main() {
6060
os.Exit(1)
6161
}
6262

63-
if err := ctrl.NewWebhookFor(mgr, &corev1.Pod{}).
64-
WithDefaulter(&podAnnotator{}).
65-
WithValidator(&podValidator{}).
63+
if err := ctrl.WebhookManagedBy(mgr, &corev1.Pod{}).
64+
WithAdmissionDefaulter(&podAnnotator{}).
65+
WithAdmissionValidator(&podValidator{}).
6666
Complete(); err != nil {
6767
entryLog.Error(err, "unable to create webhook", "webhook", "Pod")
6868
os.Exit(1)

‎examples/crd/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func main() {
129129
os.Exit(1)
130130
}
131131

132-
err = ctrl.NewWebhookFor(mgr, &api.ChaosPod{}).
132+
err = ctrl.WebhookManagedBy(mgr, &api.ChaosPod{}).
133133
Complete()
134134
if err != nil {
135135
setupLog.Error(err, "unable to create webhook")

‎pkg/builder/example_webhook_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package builder_test
1919
import (
2020
"os"
2121

22-
"k8s.io/apimachinery/pkg/runtime"
2322
"sigs.k8s.io/controller-runtime/pkg/builder"
2423
"sigs.k8s.io/controller-runtime/pkg/client/config"
2524
logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -41,7 +40,7 @@ func ExampleWebhookBuilder() {
4140
}
4241

4342
err = builder.
44-
WebhookFor[runtime.Object](mgr, &examplegroup.ChaosPod{}). // Create the WebhookManagedBy
43+
WebhookManagedBy(mgr, &examplegroup.ChaosPod{}). // Create the WebhookManagedBy
4544
Complete()
4645
if err != nil {
4746
log.Error(err, "could not create webhook")

‎pkg/builder/webhook.go‎

Lines changed: 41 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ import (
3939
// WebhookBuilder builds a Webhook.
4040
type WebhookBuilder[T runtime.Object] struct {
4141
apiType runtime.Object
42-
customDefaulter admission.Defaulter[T]
42+
customDefaulter admission.CustomDefaulter
43+
defaulter admission.Defaulter[T]
4344
customDefaulterOpts []admission.DefaulterOption
44-
customValidator admission.Validator[T]
45+
customValidator admission.CustomValidator
46+
validator admission.Validator[T]
4547
customPath string
4648
customValidatorCustomPath string
4749
customDefaulterCustomPath string
@@ -56,43 +58,42 @@ type WebhookBuilder[T runtime.Object] struct {
5658
}
5759

5860
// WebhookManagedBy returns a new webhook builder.
59-
// Deprecated: Use WebhookFor instead.
60-
func WebhookManagedBy(m manager.Manager) *WebhookBuilder[runtime.Object] {
61-
return &WebhookBuilder[runtime.Object]{mgr: m}
62-
}
63-
64-
// WebhookFor return a new webhook builder for the provided type T.
65-
func WebhookFor[T runtime.Object](m manager.Manager, object T) *WebhookBuilder[T] {
66-
return &WebhookBuilder[T]{mgr: m}
67-
}
68-
69-
// For takes a runtime.Object which should be a CR. It is only required
70-
// if the webhooks generic type is runtime.Object
71-
func (blder *WebhookBuilder[T]) For(apiType runtime.Object) *WebhookBuilder[T] {
72-
if blder.apiType != nil {
73-
blder.err = errors.New("For(...) should only be called once, could not assign multiple objects for webhook registration")
74-
}
75-
blder.apiType = apiType
76-
return blder
61+
func WebhookManagedBy[T runtime.Object](m manager.Manager, object T) *WebhookBuilder[T] {
62+
return &WebhookBuilder[T]{mgr: m, apiType: object}
7763
}
7864

7965
// WithDefaulter takes an admission.CustomDefaulter interface, a MutatingWebhook with the provided opts (admission.DefaulterOption)
8066
// will be wired for this type.
81-
func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] {
67+
// deprecated: Use WithAdmissionDefaulter instead.
68+
func (blder *WebhookBuilder[T]) WithDefaulter(defaulter admission.CustomDefaulter, opts ...admission.DefaulterOption) *WebhookBuilder[T] {
8269
blder.customDefaulter = defaulter
8370
blder.customDefaulterOpts = opts
8471
return blder
8572
}
8673

74+
// WithAdmissionDefaulter sets up the provided admission.Defaulter in a defaulting webhook.
75+
func (blder *WebhookBuilder[T]) WithAdmissionDefaulter(defaulter admission.Defaulter[T], opts ...admission.DefaulterOption) *WebhookBuilder[T] {
76+
blder.defaulter = defaulter
77+
blder.customDefaulterOpts = opts
78+
return blder
79+
}
80+
8781
// WithValidator takes a admission.CustomValidator interface, a ValidatingWebhook will be wired for this type.
88-
func (blder *WebhookBuilder[T]) WithValidator(validator admission.Validator[T]) *WebhookBuilder[T] {
82+
// deprecated: Use WithAdmissionValidator instead.
83+
func (blder *WebhookBuilder[T]) WithValidator(validator admission.CustomValidator) *WebhookBuilder[T] {
8984
blder.customValidator = validator
9085
return blder
9186
}
9287

88+
// WithAdmissionValidator sets up the provided admission.Validator in a validating webhook.
89+
func (blder *WebhookBuilder[T]) WithAdmissionValidator(validator admission.Validator[T]) *WebhookBuilder[T] {
90+
blder.validator = validator
91+
return blder
92+
}
93+
9394
// WithConverter takes a func that constructs a converter.Converter.
9495
// The Converter will then be used by the conversion endpoint for the type passed into For().
95-
func (blder *WebhookBuilder) WithConverter(converterConstructor func(*runtime.Scheme) (conversion.Converter, error)) *WebhookBuilder {
96+
func (blder *WebhookBuilder[T]) WithConverter(converterConstructor func(*runtime.Scheme) (conversion.Converter, error)) *WebhookBuilder[T] {
9697
blder.converterConstructor = converterConstructor
9798
return blder
9899
}
@@ -248,28 +249,19 @@ func (blder *WebhookBuilder[T]) registerDefaultingWebhook() error {
248249
}
249250

250251
func (blder *WebhookBuilder[T]) getDefaultingWebhook() *admission.Webhook {
251-
if defaulter := blder.customDefaulter; defaulter != nil {
252-
var w *admission.Webhook
253-
switch any(*new(T)).(type) {
254-
case runtime.Object:
255-
w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, &defaulterWrapper[T]{defaulter: defaulter}, blder.customDefaulterOpts...)
256-
default:
257-
w = admission.WithDefaulter(blder.mgr.GetScheme(), defaulter, blder.customDefaulterOpts...)
258-
}
252+
var w *admission.Webhook
253+
if defaulter := blder.defaulter; defaulter != nil {
254+
w = admission.WithDefaulter(blder.mgr.GetScheme(), blder.defaulter, blder.customDefaulterOpts...)
255+
} else if customDefaulter := blder.customDefaulter; customDefaulter != nil {
256+
w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, customDefaulter, blder.customDefaulterOpts...)
259257
if blder.recoverPanic != nil {
260-
w = w.WithRecoverPanic(*blder.recoverPanic)
261258
}
262259
return w
263260
}
264-
return nil
265-
}
266-
267-
type defaulterWrapper[T runtime.Object] struct {
268-
defaulter admission.Defaulter[T]
269-
}
270-
271-
func (d *defaulterWrapper[T]) Default(ctx context.Context, obj runtime.Object) error {
272-
return d.defaulter.Default(ctx, obj.(T))
261+
if w != nil && blder.recoverPanic != nil {
262+
w = w.WithRecoverPanic(*blder.recoverPanic)
263+
}
264+
return w
273265
}
274266

275267
// registerValidatingWebhook registers a validating webhook if necessary.
@@ -301,19 +293,16 @@ func (blder *WebhookBuilder[T]) registerValidatingWebhook() error {
301293
}
302294

303295
func (blder *WebhookBuilder[T]) getValidatingWebhook() *admission.Webhook {
304-
if validator := blder.customValidator; validator != nil {
305-
var w *admission.Webhook
306-
switch any(*new(T)).(type) {
307-
case runtime.Object:
308-
w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, &validatorWrapper[T]{validator: validator})
309-
default:
310-
w = admission.WithValidator(blder.mgr.GetScheme(), validator)
311-
}
312-
if blder.recoverPanic != nil {
313-
w = w.WithRecoverPanic(*blder.recoverPanic)
314-
}
296+
var w *admission.Webhook
297+
if validator := blder.validator; validator != nil {
298+
w = admission.WithValidator(blder.mgr.GetScheme(), validator)
299+
} else if customValidator := blder.customValidator; customValidator != nil {
300+
w = admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, customValidator)
315301
return w
316302
}
303+
if w != nil && blder.recoverPanic != nil {
304+
w = w.WithRecoverPanic(*blder.recoverPanic)
305+
}
317306
return nil
318307
}
319308

‎pkg/builder/webhook_test.go‎

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ func runTests(admissionReviewVersion string) {
9696
err = builder.AddToScheme(m.GetScheme())
9797
ExpectWithOffset(1, err).NotTo(HaveOccurred())
9898

99-
err = WebhookManagedBy(m).
100-
For(&TestDefaulter{}).
99+
err = WebhookManagedBy(m, &TestDefaulter{}).
101100
WithDefaulter(&TestCustomDefaulter{}).
102101
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
103102
return admission.DefaultLogConstructor(testingLogger, req)
@@ -173,8 +172,7 @@ func runTests(admissionReviewVersion string) {
173172
ExpectWithOffset(1, err).NotTo(HaveOccurred())
174173

175174
customPath := "/custom-defaulting-path"
176-
err = WebhookManagedBy(m).
177-
For(&TestDefaulter{}).
175+
err = WebhookManagedBy(m, &TestDefaulter{}).
178176
WithDefaulter(&TestCustomDefaulter{}).
179177
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
180178
return admission.DefaultLogConstructor(testingLogger, req)
@@ -251,8 +249,7 @@ func runTests(admissionReviewVersion string) {
251249
err = builder.AddToScheme(m.GetScheme())
252250
ExpectWithOffset(1, err).NotTo(HaveOccurred())
253251

254-
err = WebhookManagedBy(m).
255-
For(&TestDefaulter{}).
252+
err = WebhookManagedBy(m, &TestDefaulter{}).
256253
WithDefaulter(&TestCustomDefaulter{}).
257254
RecoverPanic(true).
258255
// RecoverPanic defaults to true.
@@ -315,8 +312,7 @@ func runTests(admissionReviewVersion string) {
315312
err = builder.AddToScheme(m.GetScheme())
316313
ExpectWithOffset(1, err).NotTo(HaveOccurred())
317314

318-
err = WebhookManagedBy(m).
319-
For(&TestValidator{}).
315+
err = WebhookManagedBy(m, &TestValidator{}).
320316
WithValidator(&TestCustomValidator{}).
321317
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
322318
return admission.DefaultLogConstructor(testingLogger, req)
@@ -434,8 +430,7 @@ func runTests(admissionReviewVersion string) {
434430
ExpectWithOffset(1, err).NotTo(HaveOccurred())
435431

436432
customPath := "/custom-validating-path"
437-
err = WebhookManagedBy(m).
438-
For(&TestValidator{}).
433+
err = WebhookManagedBy(m, &TestValidator{}).
439434
WithValidator(&TestCustomValidator{}).
440435
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
441436
return admission.DefaultLogConstructor(testingLogger, req)
@@ -513,8 +508,7 @@ func runTests(admissionReviewVersion string) {
513508
err = builder.AddToScheme(m.GetScheme())
514509
ExpectWithOffset(1, err).NotTo(HaveOccurred())
515510

516-
err = WebhookManagedBy(m).
517-
For(&TestValidator{}).
511+
err = WebhookManagedBy(m, &TestValidator{}).
518512
WithValidator(&TestCustomValidator{}).
519513
RecoverPanic(true).
520514
Complete()
@@ -579,8 +573,7 @@ func runTests(admissionReviewVersion string) {
579573
err = builder.AddToScheme(m.GetScheme())
580574
ExpectWithOffset(1, err).NotTo(HaveOccurred())
581575

582-
err = WebhookManagedBy(m).
583-
For(&TestValidator{}).
576+
err = WebhookManagedBy(m, &TestValidator{}).
584577
WithValidator(&TestCustomValidator{}).
585578
Complete()
586579
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -659,24 +652,6 @@ func runTests(admissionReviewVersion string) {
659652
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
660653
})
661654

662-
It("should send an error when trying to register a webhook with more than one For", func() {
663-
By("creating a controller manager")
664-
m, err := manager.New(cfg, manager.Options{})
665-
ExpectWithOffset(1, err).NotTo(HaveOccurred())
666-
667-
By("registering the type in the Scheme")
668-
builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()}
669-
builder.Register(&TestDefaulter{}, &TestDefaulterList{})
670-
err = builder.AddToScheme(m.GetScheme())
671-
ExpectWithOffset(1, err).NotTo(HaveOccurred())
672-
673-
err = WebhookManagedBy(m).
674-
For(&TestDefaulter{}).
675-
For(&TestDefaulter{}).
676-
Complete()
677-
Expect(err).To(HaveOccurred())
678-
})
679-
680655
It("should scaffold a custom defaulting and validating webhook", func(specCtx SpecContext) {
681656
By("creating a controller manager")
682657
m, err := manager.New(cfg, manager.Options{})
@@ -688,8 +663,7 @@ func runTests(admissionReviewVersion string) {
688663
err = builder.AddToScheme(m.GetScheme())
689664
ExpectWithOffset(1, err).NotTo(HaveOccurred())
690665

691-
err = WebhookManagedBy(m).
692-
For(&TestDefaultValidator{}).
666+
err = WebhookManagedBy(m, &TestDefaultValidator{}).
693667
WithDefaulter(&TestCustomDefaultValidator{}).
694668
WithValidator(&TestCustomDefaultValidator{}).
695669
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
@@ -773,8 +747,7 @@ func runTests(admissionReviewVersion string) {
773747

774748
validatingCustomPath := "/custom-validating-path"
775749
defaultingCustomPath := "/custom-defaulting-path"
776-
err = WebhookManagedBy(m).
777-
For(&TestDefaultValidator{}).
750+
err = WebhookManagedBy(m, &TestDefaultValidator{}).
778751
WithDefaulter(&TestCustomDefaultValidator{}).
779752
WithValidator(&TestCustomDefaultValidator{}).
780753
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
@@ -878,8 +851,7 @@ func runTests(admissionReviewVersion string) {
878851
err = builder.AddToScheme(m.GetScheme())
879852
ExpectWithOffset(1, err).NotTo(HaveOccurred())
880853

881-
err = WebhookManagedBy(m).
882-
For(&TestDefaultValidator{}).
854+
err = WebhookManagedBy(m, &TestDefaultValidator{}).
883855
WithDefaulter(&TestCustomDefaultValidator{}).
884856
WithValidator(&TestCustomDefaultValidator{}).
885857
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
@@ -901,8 +873,7 @@ func runTests(admissionReviewVersion string) {
901873
err = builder.AddToScheme(m.GetScheme())
902874
ExpectWithOffset(1, err).NotTo(HaveOccurred())
903875

904-
err = WebhookManagedBy(m).
905-
For(&TestDefaulter{}).
876+
err = WebhookManagedBy(m, &TestDefaulter{}).
906877
WithDefaulter(&TestCustomDefaulter{}).
907878
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
908879
return admission.DefaultLogConstructor(testingLogger, req)
@@ -924,8 +895,7 @@ func runTests(admissionReviewVersion string) {
924895
err = builder.AddToScheme(m.GetScheme())
925896
ExpectWithOffset(1, err).NotTo(HaveOccurred())
926897

927-
err = WebhookManagedBy(m).
928-
For(&TestValidator{}).
898+
err = WebhookManagedBy(m, &TestValidator{}).
929899
WithValidator(&TestCustomValidator{}).
930900
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
931901
return admission.DefaultLogConstructor(testingLogger, req)

0 commit comments

Comments
 (0)