From 0ea4b52e202293e113e83cb1a051c29e700d36f2 Mon Sep 17 00:00:00 2001 From: Troy Connor Date: Fri, 16 May 2025 14:30:55 -0400 Subject: [PATCH] return warnings on webhooks Signed-off-by: Troy Connor --- pkg/webhook/admission/multi.go | 6 +++ pkg/webhook/admission/multi_test.go | 57 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/pkg/webhook/admission/multi.go b/pkg/webhook/admission/multi.go index 2f7820d04b..ef9c456248 100644 --- a/pkg/webhook/admission/multi.go +++ b/pkg/webhook/admission/multi.go @@ -31,6 +31,7 @@ type multiMutating []Handler func (hs multiMutating) Handle(ctx context.Context, req Request) Response { patches := []jsonpatch.JsonPatchOperation{} + warnings := []string{} for _, handler := range hs { resp := handler.Handle(ctx, req) if !resp.Allowed { @@ -42,6 +43,7 @@ func (hs multiMutating) Handle(ctx context.Context, req Request) Response { resp.PatchType, admissionv1.PatchTypeJSONPatch)) } patches = append(patches, resp.Patches...) + warnings = append(warnings, resp.Warnings...) } var err error marshaledPatch, err := json.Marshal(patches) @@ -55,6 +57,7 @@ func (hs multiMutating) Handle(ctx context.Context, req Request) Response { Code: http.StatusOK, }, Patch: marshaledPatch, + Warnings: warnings, PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(), }, } @@ -71,11 +74,13 @@ func MultiMutatingHandler(handlers ...Handler) Handler { type multiValidating []Handler func (hs multiValidating) Handle(ctx context.Context, req Request) Response { + warnings := []string{} for _, handler := range hs { resp := handler.Handle(ctx, req) if !resp.Allowed { return resp } + warnings = append(warnings, resp.Warnings...) } return Response{ AdmissionResponse: admissionv1.AdmissionResponse{ @@ -83,6 +88,7 @@ func (hs multiValidating) Handle(ctx context.Context, req Request) Response { Result: &metav1.Status{ Code: http.StatusOK, }, + Warnings: warnings, }, } } diff --git a/pkg/webhook/admission/multi_test.go b/pkg/webhook/admission/multi_test.go index da85a52e42..d41675ab30 100644 --- a/pkg/webhook/admission/multi_test.go +++ b/pkg/webhook/admission/multi_test.go @@ -46,6 +46,17 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() { }, } + withWarnings := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + AdmissionResponse: admissionv1.AdmissionResponse{ + Allowed: true, + Warnings: []string{"handler-warning"}, + }, + } + }, + } + Context("with validating handlers", func() { It("should deny the request if any handler denies the request", func() { By("setting up a handler with accept and deny") @@ -54,6 +65,7 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() { By("checking that the handler denies the request") resp := handler.Handle(context.Background(), Request{}) Expect(resp.Allowed).To(BeFalse()) + Expect(resp.Warnings).To(BeEmpty()) }) It("should allow the request if all handlers allow the request", func() { @@ -63,6 +75,17 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() { By("checking that the handler allows the request") resp := handler.Handle(context.Background(), Request{}) Expect(resp.Allowed).To(BeTrue()) + Expect(resp.Warnings).To(BeEmpty()) + }) + + It("should show the warnings if all handlers allow the request", func() { + By("setting up a handler with only accept") + handler := MultiValidatingHandler(alwaysAllow, withWarnings) + + By("checking that the handler allows the request") + resp := handler.Handle(context.Background(), Request{}) + Expect(resp.Allowed).To(BeTrue()) + Expect(resp.Warnings).To(HaveLen(1)) }) }) @@ -107,6 +130,25 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() { }, } + patcher3 := &fakeHandler{ + fn: func(ctx context.Context, req Request) Response { + return Response{ + Patches: []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: "/metadata/annotation/newest-key", + Value: "value", + }, + }, + AdmissionResponse: admissionv1.AdmissionResponse{ + Allowed: true, + Warnings: []string{"annotation-warning"}, + PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(), + }, + } + }, + } + It("should not return any patches if the request is denied", func() { By("setting up a webhook with some patches and a deny") handler := MultiMutatingHandler(patcher1, patcher2, alwaysDeny) @@ -128,5 +170,20 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() { `[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},` + `{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"}]`))) }) + + It("should produce all patches if the requests are all allowed and show warnings", func() { + By("setting up a webhook with some patches") + handler := MultiMutatingHandler(patcher1, patcher2, alwaysAllow, patcher3) + + By("checking that the handler accepts the request and returns all patches") + resp := handler.Handle(context.Background(), Request{}) + Expect(resp.Allowed).To(BeTrue()) + Expect(resp.Patch).To(Equal([]byte( + `[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},` + + `{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"},` + + `{"op":"add","path":"/metadata/annotation/newest-key","value":"value"}]`))) + Expect(resp.Warnings).To(HaveLen(1)) + }) + }) })