Skip to content

Commit 0ea4b52

Browse files
troy0820k8s-infra-cherrypick-robot
authored and
k8s-infra-cherrypick-robot
committed
return warnings on webhooks
Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
1 parent 0f7927c commit 0ea4b52

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

pkg/webhook/admission/multi.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ type multiMutating []Handler
3131

3232
func (hs multiMutating) Handle(ctx context.Context, req Request) Response {
3333
patches := []jsonpatch.JsonPatchOperation{}
34+
warnings := []string{}
3435
for _, handler := range hs {
3536
resp := handler.Handle(ctx, req)
3637
if !resp.Allowed {
@@ -42,6 +43,7 @@ func (hs multiMutating) Handle(ctx context.Context, req Request) Response {
4243
resp.PatchType, admissionv1.PatchTypeJSONPatch))
4344
}
4445
patches = append(patches, resp.Patches...)
46+
warnings = append(warnings, resp.Warnings...)
4547
}
4648
var err error
4749
marshaledPatch, err := json.Marshal(patches)
@@ -55,6 +57,7 @@ func (hs multiMutating) Handle(ctx context.Context, req Request) Response {
5557
Code: http.StatusOK,
5658
},
5759
Patch: marshaledPatch,
60+
Warnings: warnings,
5861
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
5962
},
6063
}
@@ -71,18 +74,21 @@ func MultiMutatingHandler(handlers ...Handler) Handler {
7174
type multiValidating []Handler
7275

7376
func (hs multiValidating) Handle(ctx context.Context, req Request) Response {
77+
warnings := []string{}
7478
for _, handler := range hs {
7579
resp := handler.Handle(ctx, req)
7680
if !resp.Allowed {
7781
return resp
7882
}
83+
warnings = append(warnings, resp.Warnings...)
7984
}
8085
return Response{
8186
AdmissionResponse: admissionv1.AdmissionResponse{
8287
Allowed: true,
8388
Result: &metav1.Status{
8489
Code: http.StatusOK,
8590
},
91+
Warnings: warnings,
8692
},
8793
}
8894
}

pkg/webhook/admission/multi_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,17 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
4646
},
4747
}
4848

49+
withWarnings := &fakeHandler{
50+
fn: func(ctx context.Context, req Request) Response {
51+
return Response{
52+
AdmissionResponse: admissionv1.AdmissionResponse{
53+
Allowed: true,
54+
Warnings: []string{"handler-warning"},
55+
},
56+
}
57+
},
58+
}
59+
4960
Context("with validating handlers", func() {
5061
It("should deny the request if any handler denies the request", func() {
5162
By("setting up a handler with accept and deny")
@@ -54,6 +65,7 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
5465
By("checking that the handler denies the request")
5566
resp := handler.Handle(context.Background(), Request{})
5667
Expect(resp.Allowed).To(BeFalse())
68+
Expect(resp.Warnings).To(BeEmpty())
5769
})
5870

5971
It("should allow the request if all handlers allow the request", func() {
@@ -63,6 +75,17 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
6375
By("checking that the handler allows the request")
6476
resp := handler.Handle(context.Background(), Request{})
6577
Expect(resp.Allowed).To(BeTrue())
78+
Expect(resp.Warnings).To(BeEmpty())
79+
})
80+
81+
It("should show the warnings if all handlers allow the request", func() {
82+
By("setting up a handler with only accept")
83+
handler := MultiValidatingHandler(alwaysAllow, withWarnings)
84+
85+
By("checking that the handler allows the request")
86+
resp := handler.Handle(context.Background(), Request{})
87+
Expect(resp.Allowed).To(BeTrue())
88+
Expect(resp.Warnings).To(HaveLen(1))
6689
})
6790
})
6891

@@ -107,6 +130,25 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
107130
},
108131
}
109132

133+
patcher3 := &fakeHandler{
134+
fn: func(ctx context.Context, req Request) Response {
135+
return Response{
136+
Patches: []jsonpatch.JsonPatchOperation{
137+
{
138+
Operation: "add",
139+
Path: "/metadata/annotation/newest-key",
140+
Value: "value",
141+
},
142+
},
143+
AdmissionResponse: admissionv1.AdmissionResponse{
144+
Allowed: true,
145+
Warnings: []string{"annotation-warning"},
146+
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
147+
},
148+
}
149+
},
150+
}
151+
110152
It("should not return any patches if the request is denied", func() {
111153
By("setting up a webhook with some patches and a deny")
112154
handler := MultiMutatingHandler(patcher1, patcher2, alwaysDeny)
@@ -128,5 +170,20 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
128170
`[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},` +
129171
`{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"}]`)))
130172
})
173+
174+
It("should produce all patches if the requests are all allowed and show warnings", func() {
175+
By("setting up a webhook with some patches")
176+
handler := MultiMutatingHandler(patcher1, patcher2, alwaysAllow, patcher3)
177+
178+
By("checking that the handler accepts the request and returns all patches")
179+
resp := handler.Handle(context.Background(), Request{})
180+
Expect(resp.Allowed).To(BeTrue())
181+
Expect(resp.Patch).To(Equal([]byte(
182+
`[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},` +
183+
`{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"},` +
184+
`{"op":"add","path":"/metadata/annotation/newest-key","value":"value"}]`)))
185+
Expect(resp.Warnings).To(HaveLen(1))
186+
})
187+
131188
})
132189
})

0 commit comments

Comments
 (0)