Skip to content

Commit c80ac38

Browse files
timebertttimebertt
committed
Allow configuring LogConstructor in webhook builder
1 parent 4dddfe9 commit c80ac38

File tree

2 files changed

+73
-14
lines changed

2 files changed

+73
-14
lines changed

‎pkg/builder/webhook.go

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@ import (
2222
"net/url"
2323
"strings"
2424

25+
"github.com/go-logr/logr"
2526
"k8s.io/apimachinery/pkg/runtime"
2627
"k8s.io/apimachinery/pkg/runtime/schema"
2728
"k8s.io/client-go/rest"
29+
"k8s.io/klog/v2"
30+
2831
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
2932
"sigs.k8s.io/controller-runtime/pkg/manager"
3033
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -33,13 +36,14 @@ import (
3336

3437
// WebhookBuilder builds a Webhook.
3538
type WebhookBuilder struct {
36-
apiType runtime.Object
37-
withDefaulter admission.CustomDefaulter
38-
withValidator admission.CustomValidator
39-
gvk schema.GroupVersionKind
40-
mgr manager.Manager
41-
config *rest.Config
42-
recoverPanic bool
39+
apiType runtime.Object
40+
withDefaulter admission.CustomDefaulter
41+
withValidator admission.CustomValidator
42+
gvk schema.GroupVersionKind
43+
mgr manager.Manager
44+
config *rest.Config
45+
recoverPanic bool
46+
logConstructor func(base logr.Logger, req *admission.Request) logr.Logger
4347
}
4448

4549
// WebhookManagedBy allows inform its manager.Manager.
@@ -69,6 +73,12 @@ func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator)
6973
return blder
7074
}
7175

76+
// WithLogConstructor overrides the webhook's LogConstructor.
77+
func (blder *WebhookBuilder) WithLogConstructor(logConstructor func(base logr.Logger, req *admission.Request) logr.Logger) *WebhookBuilder {
78+
blder.logConstructor = logConstructor
79+
return blder
80+
}
81+
7282
// RecoverPanic indicates whether the panic caused by webhook should be recovered.
7383
func (blder *WebhookBuilder) RecoverPanic() *WebhookBuilder {
7484
blder.recoverPanic = true
@@ -80,6 +90,9 @@ func (blder *WebhookBuilder) Complete() error {
8090
// Set the Config
8191
blder.loadRestConfig()
8292

93+
// Configure the default LogConstructor
94+
blder.setLogConstructor()
95+
8396
// Set the Webhook if needed
8497
return blder.registerWebhooks()
8598
}
@@ -90,6 +103,25 @@ func (blder *WebhookBuilder) loadRestConfig() {
90103
}
91104
}
92105

106+
func (blder *WebhookBuilder) setLogConstructor() {
107+
if blder.logConstructor == nil {
108+
blder.logConstructor = func(base logr.Logger, req *admission.Request) logr.Logger {
109+
log := base.WithValues(
110+
"webhookGroup", blder.gvk.Group,
111+
"webhookKind", blder.gvk.Kind,
112+
)
113+
if req != nil {
114+
return log.WithValues(
115+
blder.gvk.Kind, klog.KRef(req.Namespace, req.Name),
116+
"namespace", req.Namespace, "name", req.Name,
117+
"resource", req.Resource, "user", req.UserInfo.Username,
118+
)
119+
}
120+
return log
121+
}
122+
}
123+
}
124+
93125
func (blder *WebhookBuilder) registerWebhooks() error {
94126
typ, err := blder.getType()
95127
if err != nil {
@@ -116,6 +148,7 @@ func (blder *WebhookBuilder) registerWebhooks() error {
116148
func (blder *WebhookBuilder) registerDefaultingWebhook() {
117149
mwh := blder.getDefaultingWebhook()
118150
if mwh != nil {
151+
mwh.LogConstructor = blder.logConstructor
119152
path := generateMutatePath(blder.gvk)
120153

121154
// Checking if the path is already registered.
@@ -145,6 +178,7 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
145178
func (blder *WebhookBuilder) registerValidatingWebhook() {
146179
vwh := blder.getValidatingWebhook()
147180
if vwh != nil {
181+
vwh.LogConstructor = blder.logConstructor
148182
path := generateValidatePath(blder.gvk)
149183

150184
// Checking if the path is already registered.

‎pkg/builder/webhook_test.go

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,23 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"io"
2324
"net/http"
2425
"net/http/httptest"
2526
"os"
2627
"strings"
2728

29+
"github.com/go-logr/logr"
2830
. "github.com/onsi/ginkgo/v2"
2931
. "github.com/onsi/gomega"
32+
"github.com/onsi/gomega/gbytes"
3033
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3134
"k8s.io/apimachinery/pkg/runtime"
3235
"k8s.io/apimachinery/pkg/runtime/schema"
3336

3437
"sigs.k8s.io/controller-runtime/pkg/controller"
38+
logf "sigs.k8s.io/controller-runtime/pkg/log"
39+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
3540
"sigs.k8s.io/controller-runtime/pkg/manager"
3641
"sigs.k8s.io/controller-runtime/pkg/scheme"
3742
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -49,11 +54,17 @@ var _ = Describe("webhook", func() {
4954
})
5055

5156
func runTests(admissionReviewVersion string) {
52-
var stop chan struct{}
57+
var (
58+
stop chan struct{}
59+
logBuffer *gbytes.Buffer
60+
testingLogger logr.Logger
61+
)
5362

5463
BeforeEach(func() {
5564
stop = make(chan struct{})
5665
newController = controller.New
66+
logBuffer = gbytes.NewBuffer()
67+
testingLogger = zap.New(zap.JSONEncoder(), zap.WriteTo(io.MultiWriter(logBuffer, GinkgoWriter)))
5768
})
5869

5970
AfterEach(func() {
@@ -214,6 +225,9 @@ func runTests(admissionReviewVersion string) {
214225
err = WebhookManagedBy(m).
215226
WithDefaulter(&TestCustomDefaulter{}).
216227
For(&TestDefaulter{}).
228+
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
229+
return admission.DefaultLogConstructor(testingLogger, req)
230+
}).
217231
Complete()
218232
ExpectWithOffset(1, err).NotTo(HaveOccurred())
219233
svr := m.GetWebhookServer()
@@ -225,16 +239,17 @@ func runTests(admissionReviewVersion string) {
225239
"request":{
226240
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
227241
"kind":{
228-
"group":"",
242+
"group":"foo.test.org",
229243
"version":"v1",
230244
"kind":"TestDefaulter"
231245
},
232246
"resource":{
233-
"group":"",
247+
"group":"foo.test.org",
234248
"version":"v1",
235249
"resource":"testdefaulter"
236250
},
237251
"namespace":"default",
252+
"name":"foo",
238253
"operation":"CREATE",
239254
"object":{
240255
"replica":1
@@ -263,6 +278,7 @@ func runTests(admissionReviewVersion string) {
263278
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`))
264279
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`))
265280
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`))
281+
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"`))
266282

267283
By("sending a request to a validating webhook path that doesn't exist")
268284
path = generateValidatePath(testDefaulterGVK)
@@ -431,6 +447,9 @@ func runTests(admissionReviewVersion string) {
431447
err = WebhookManagedBy(m).
432448
WithValidator(&TestCustomValidator{}).
433449
For(&TestValidator{}).
450+
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
451+
return admission.DefaultLogConstructor(testingLogger, req)
452+
}).
434453
Complete()
435454
ExpectWithOffset(1, err).NotTo(HaveOccurred())
436455
svr := m.GetWebhookServer()
@@ -442,16 +461,17 @@ func runTests(admissionReviewVersion string) {
442461
"request":{
443462
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
444463
"kind":{
445-
"group":"",
464+
"group":"foo.test.org",
446465
"version":"v1",
447-
"kind":"TestValidator"
466+
"kind":"TestDefaulter"
448467
},
449468
"resource":{
450-
"group":"",
469+
"group":"foo.test.org",
451470
"version":"v1",
452-
"resource":"testvalidator"
471+
"resource":"testdefaulter"
453472
},
454473
"namespace":"default",
474+
"name":"foo",
455475
"operation":"UPDATE",
456476
"object":{
457477
"replica":1
@@ -491,6 +511,7 @@ func runTests(admissionReviewVersion string) {
491511
By("sanity checking the response contains reasonable field")
492512
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
493513
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
514+
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":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
494515
})
495516

496517
It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {
@@ -845,6 +866,7 @@ func (dv *TestDefaultValidator) ValidateDelete() error {
845866
type TestCustomDefaulter struct{}
846867

847868
func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
869+
logf.FromContext(ctx).Info("Defaulting object")
848870
req, err := admission.RequestFromContext(ctx)
849871
if err != nil {
850872
return fmt.Errorf("expected admission.Request in ctx: %w", err)
@@ -867,6 +889,7 @@ var _ admission.CustomDefaulter = &TestCustomDefaulter{}
867889
type TestCustomValidator struct{}
868890

869891
func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
892+
logf.FromContext(ctx).Info("Validating object")
870893
req, err := admission.RequestFromContext(ctx)
871894
if err != nil {
872895
return fmt.Errorf("expected admission.Request in ctx: %w", err)
@@ -883,6 +906,7 @@ func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Obje
883906
}
884907

885908
func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
909+
logf.FromContext(ctx).Info("Validating object")
886910
req, err := admission.RequestFromContext(ctx)
887911
if err != nil {
888912
return fmt.Errorf("expected admission.Request in ctx: %w", err)
@@ -903,6 +927,7 @@ func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj r
903927
}
904928

905929
func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
930+
logf.FromContext(ctx).Info("Validating object")
906931
req, err := admission.RequestFromContext(ctx)
907932
if err != nil {
908933
return fmt.Errorf("expected admission.Request in ctx: %w", err)

0 commit comments

Comments
 (0)