Skip to content

Commit 3452b25

Browse files
committed
fix review findings
1 parent 36acd6b commit 3452b25

File tree

3 files changed

+26
-11
lines changed

3 files changed

+26
-11
lines changed

pkg/builder/webhook.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ func (blder *WebhookBuilder) WithLogConstructor(logConstructor func(base logr.Lo
8585

8686
// RecoverPanic indicates whether panics caused by the webhook should be recovered.
8787
// Defaults to true.
88-
func (blder *WebhookBuilder) RecoverPanic(recoverPanic *bool) *WebhookBuilder {
89-
blder.recoverPanic = recoverPanic
88+
func (blder *WebhookBuilder) RecoverPanic(recoverPanic bool) *WebhookBuilder {
89+
blder.recoverPanic = &recoverPanic
9090
return blder
9191
}
9292

@@ -170,10 +170,18 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
170170

171171
func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
172172
if defaulter := blder.customDefaulter; defaulter != nil {
173-
return admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter).WithRecoverPanic(blder.recoverPanic)
173+
w := admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter)
174+
if blder.recoverPanic != nil {
175+
w = w.WithRecoverPanic(*blder.recoverPanic)
176+
}
177+
return w
174178
}
175179
if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
176-
return admission.DefaultingWebhookFor(blder.mgr.GetScheme(), defaulter).WithRecoverPanic(blder.recoverPanic)
180+
w := admission.DefaultingWebhookFor(blder.mgr.GetScheme(), defaulter)
181+
if blder.recoverPanic != nil {
182+
w = w.WithRecoverPanic(*blder.recoverPanic)
183+
}
184+
return w
177185
}
178186
log.Info(
179187
"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called",
@@ -201,10 +209,18 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
201209

202210
func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
203211
if validator := blder.customValidator; validator != nil {
204-
return admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator).WithRecoverPanic(blder.recoverPanic)
212+
w := admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator)
213+
if blder.recoverPanic != nil {
214+
w = w.WithRecoverPanic(*blder.recoverPanic)
215+
}
216+
return w
205217
}
206218
if validator, ok := blder.apiType.(admission.Validator); ok {
207-
return admission.ValidatingWebhookFor(blder.mgr.GetScheme(), validator).WithRecoverPanic(blder.recoverPanic)
219+
w := admission.ValidatingWebhookFor(blder.mgr.GetScheme(), validator)
220+
if blder.recoverPanic != nil {
221+
w = w.WithRecoverPanic(*blder.recoverPanic)
222+
}
223+
return w
208224
}
209225
log.Info(
210226
"skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called",

pkg/builder/webhook_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3434
"k8s.io/apimachinery/pkg/runtime"
3535
"k8s.io/apimachinery/pkg/runtime/schema"
36-
"k8s.io/utils/ptr"
3736

3837
logf "sigs.k8s.io/controller-runtime/pkg/log"
3938
"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -161,7 +160,7 @@ func runTests(admissionReviewVersion string) {
161160

162161
err = WebhookManagedBy(m).
163162
For(&TestDefaulter{Panic: true}).
164-
RecoverPanic(nil). // Defaults to true.
163+
// RecoverPanic defaults to true.
165164
Complete()
166165
ExpectWithOffset(1, err).NotTo(HaveOccurred())
167166
svr := m.GetWebhookServer()
@@ -370,7 +369,7 @@ func runTests(admissionReviewVersion string) {
370369

371370
err = WebhookManagedBy(m).
372371
For(&TestValidator{Panic: true}).
373-
RecoverPanic(ptr.To[bool](true)).
372+
RecoverPanic(true).
374373
Complete()
375374
ExpectWithOffset(1, err).NotTo(HaveOccurred())
376375
svr := m.GetWebhookServer()

pkg/webhook/admission/webhook.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,8 @@ type Webhook struct {
144144

145145
// WithRecoverPanic takes a bool flag which indicates whether the panic caused by webhook should be recovered.
146146
// Defaults to true.
147-
func (wh *Webhook) WithRecoverPanic(recoverPanic *bool) *Webhook {
148-
wh.RecoverPanic = recoverPanic
147+
func (wh *Webhook) WithRecoverPanic(recoverPanic bool) *Webhook {
148+
wh.RecoverPanic = &recoverPanic
149149
return wh
150150
}
151151

0 commit comments

Comments
 (0)