From 8b3b4211ba72e4b6ac426da00f4409b179459d18 Mon Sep 17 00:00:00 2001 From: d-honeybadger Date: Sat, 1 May 2021 09:09:06 -0400 Subject: [PATCH 1/2] multiValidating and multiMutating handlers implement DecoderInjector --- pkg/webhook/admission/multi.go | 20 +++++++++++++ pkg/webhook/webhook_integration_test.go | 40 +++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/pkg/webhook/admission/multi.go b/pkg/webhook/admission/multi.go index e6179d3729..26900cf2eb 100644 --- a/pkg/webhook/admission/multi.go +++ b/pkg/webhook/admission/multi.go @@ -77,6 +77,16 @@ func (hs multiMutating) InjectFunc(f inject.Func) error { return nil } +// InjectDecoder injects the decoder into the handlers. +func (hs multiMutating) InjectDecoder(d *Decoder) error { + for _, handler := range hs { + if _, err := InjectDecoderInto(d, handler); err != nil { + return err + } + } + return nil +} + // MultiMutatingHandler combines multiple mutating webhook handlers into a single // mutating webhook handler. Handlers are called in sequential order, and the first // `allowed: false` response may short-circuit the rest. Users must take care to @@ -125,3 +135,13 @@ func (hs multiValidating) InjectFunc(f inject.Func) error { return nil } + +// InjectDecoder injects the decoder into the handlers. +func (hs multiValidating) InjectDecoder(d *Decoder) error { + for _, handler := range hs { + if _, err := InjectDecoderInto(d, handler); err != nil { + return err + } + } + return nil +} diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 3e97cce2d8..f50ee7f7e7 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/pkg/webhook/webhook_integration_test.go @@ -76,7 +76,32 @@ var _ = Describe("Webhook", func() { ctx, cancel := context.WithCancel(context.Background()) go func() { - _ = server.Start(ctx) + err = server.Start(ctx) + Expect(err).NotTo(HaveOccurred()) + }() + + Eventually(func() bool { + err = c.Create(context.TODO(), obj) + return errors.ReasonForError(err) == metav1.StatusReason("Always denied") + }, 1*time.Second).Should(BeTrue()) + + cancel() + close(done) + }) + It("should reject create request for multi-webhook that rejects all requests", func(done Done) { + m, err := manager.New(cfg, manager.Options{ + Port: testenv.WebhookInstallOptions.LocalServingPort, + Host: testenv.WebhookInstallOptions.LocalServingHost, + CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, + }) // we need manager here just to leverage manager.SetFields + Expect(err).NotTo(HaveOccurred()) + server := m.GetWebhookServer() + server.Register("/failing", &webhook.Admission{Handler: admission.MultiValidatingHandler(&rejectingValidator{})}) + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + err = server.Start(ctx) + Expect(err).NotTo(HaveOccurred()) }() Eventually(func() bool { @@ -99,7 +124,8 @@ var _ = Describe("Webhook", func() { ctx, cancel := context.WithCancel(context.Background()) go func() { - _ = server.StartStandalone(ctx, scheme.Scheme) + err := server.StartStandalone(ctx, scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) }() Eventually(func() bool { @@ -170,8 +196,18 @@ var _ = Describe("Webhook", func() { }) type rejectingValidator struct { + d *admission.Decoder +} + +func (v *rejectingValidator) InjectDecoder(d *admission.Decoder) error { + v.d = d + return nil } func (v *rejectingValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + var obj appsv1.Deployment + if err := v.d.Decode(req, &obj); err != nil { + return admission.Denied(err.Error()) + } return admission.Denied(fmt.Sprint("Always denied")) } From 21ac80f1c3ea0cf91475b5b25ac6f41782129346 Mon Sep 17 00:00:00 2001 From: d-honeybadger Date: Sat, 1 May 2021 10:27:51 -0400 Subject: [PATCH 2/2] set fields before injecting in controllerManager --- pkg/manager/internal.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 2dc83c40b9..f8c5619110 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -225,6 +225,9 @@ func (cm *controllerManager) Add(r Runnable) error { // Deprecated: use the equivalent Options field to set a field. This method will be removed in v0.10. func (cm *controllerManager) SetFields(i interface{}) error { + if err := cm.cluster.SetFields(i); err != nil { + return err + } if _, err := inject.InjectorInto(cm.SetFields, i); err != nil { return err } @@ -234,9 +237,6 @@ func (cm *controllerManager) SetFields(i interface{}) error { if _, err := inject.LoggerInto(cm.logger, i); err != nil { return err } - if err := cm.cluster.SetFields(i); err != nil { - return err - } return nil }