Skip to content

Commit c7969dc

Browse files
DirectXMan12DirectXMan12
committed
Extract out multi-handler support, remove builder
This extracts out the multi-handler support into separate interfaces. That simplifies the code, makes it easier to test directly, and allows us to drop the now-extraneous Type field. This simultaneously removes the builder (for now) since it doesn't actually make anything simpler. Along the way, we also refactor some common functionality out into a helper on reponse itself as opposed to being run by different bits of the webhook.
1 parent 6dbde68 commit c7969dc

File tree

12 files changed

+505
-634
lines changed

12 files changed

+505
-634
lines changed

‎example/main.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
3333
"sigs.k8s.io/controller-runtime/pkg/source"
3434
"sigs.k8s.io/controller-runtime/pkg/webhook"
35-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/builder"
35+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
3636
)
3737

3838
var log = logf.Log.WithName("example-controller")
@@ -79,17 +79,15 @@ func main() {
7979

8080
// Setup webhooks
8181
entryLog.Info("setting up webhooks")
82-
mutatingWebhook := builder.NewWebhookBuilder().
83-
Path("/mutate-pods").
84-
Mutating().
85-
Handlers(&podAnnotator{}).
86-
Build()
87-
88-
validatingWebhook := builder.NewWebhookBuilder().
89-
Path("/validate-pods").
90-
Validating().
91-
Handlers(&podValidator{}).
92-
Build()
82+
mutatingWebhook := &admission.Webhook{
83+
Path: "/mutate-pods",
84+
Handler: &podAnnotator{},
85+
}
86+
87+
validatingWebhook := &admission.Webhook{
88+
Path: "/validate-pods",
89+
Handler: &podValidator{},
90+
}
9391

9492
entryLog.Info("setting up webhook server")
9593
as, err := webhook.NewServer(webhook.ServerOptions{

‎pkg/webhook/admission/builder/builder.go

Lines changed: 0 additions & 100 deletions
This file was deleted.

‎pkg/webhook/admission/builder/doc.go

Lines changed: 0 additions & 58 deletions
This file was deleted.

‎pkg/webhook/admission/http.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7373
// verify the content type is accurate
7474
contentType := r.Header.Get("Content-Type")
7575
if contentType != "application/json" {
76-
err = fmt.Errorf("contentType=%s, expect application/json", contentType)
76+
err = fmt.Errorf("contentType=%s, expected application/json", contentType)
7777
log.Error(err, "unable to process a request with an unknown content type", "content type", contentType)
7878
reviewResponse = ErrorResponse(http.StatusBadRequest, err)
7979
wh.writeResponse(w, reviewResponse)

‎pkg/webhook/admission/http_test.go

Lines changed: 46 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -29,98 +29,66 @@ import (
2929
admissionv1beta1 "k8s.io/api/admission/v1beta1"
3030
)
3131

32-
var _ = Describe("admission webhook http handler", func() {
33-
var w *httptest.ResponseRecorder
34-
BeforeEach(func(done Done) {
35-
w = &httptest.ResponseRecorder{
36-
Body: bytes.NewBuffer(nil),
32+
var _ = Describe("Admission Webhooks", func() {
33+
34+
Describe("HTTP Handler", func() {
35+
var respRecorder *httptest.ResponseRecorder
36+
BeforeEach(func() {
37+
respRecorder = &httptest.ResponseRecorder{
38+
Body: bytes.NewBuffer(nil),
39+
}
40+
})
41+
webhook := &Webhook{
42+
Handler: nil,
3743
}
38-
close(done)
39-
})
4044

41-
Describe("empty request body", func() {
42-
req := &http.Request{Body: nil}
43-
wh := &Webhook{
44-
Handlers: []Handler{},
45-
}
45+
It("should return bad-request when given an empty body", func() {
46+
req := &http.Request{Body: nil}
4647

47-
expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}}
48+
expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}}
4849
`)
49-
It("should return an error with bad-request status code", func() {
50-
wh.ServeHTTP(w, req)
51-
Expect(w.Body.Bytes()).To(Equal(expected))
50+
webhook.ServeHTTP(respRecorder, req)
51+
Expect(respRecorder.Body.Bytes()).To(Equal(expected))
5252
})
53-
})
5453

55-
Describe("wrong content type", func() {
56-
req := &http.Request{
57-
Header: http.Header{"Content-Type": []string{"application/foo"}},
58-
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
59-
}
60-
wh := &Webhook{
61-
Handlers: []Handler{},
62-
}
63-
expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=application/foo, expect application/json","code":400}}}
64-
`)
65-
It("should return an error with bad-request status code", func() {
66-
wh.ServeHTTP(w, req)
67-
Expect(w.Body.Bytes()).To(Equal(expected))
54+
It("should return bad-request when given the wrong content-type", func() {
55+
req := &http.Request{
56+
Header: http.Header{"Content-Type": []string{"application/foo"}},
57+
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
58+
}
6859

69-
})
70-
})
71-
72-
Describe("can't decode body", func() {
73-
req := &http.Request{
74-
Header: http.Header{"Content-Type": []string{"application/json"}},
75-
Body: nopCloser{Reader: bytes.NewBufferString("{")},
76-
}
77-
wh := &Webhook{
78-
Type: MutatingWebhook,
79-
Handlers: []Handler{},
80-
}
81-
expected := []byte(
82-
`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}}
60+
expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=application/foo, expected application/json","code":400}}}
8361
`)
84-
It("should return an error with bad-request status code", func() {
85-
wh.ServeHTTP(w, req)
86-
Expect(w.Body.Bytes()).To(Equal(expected))
87-
62+
webhook.ServeHTTP(respRecorder, req)
63+
Expect(respRecorder.Body.Bytes()).To(Equal(expected))
8864
})
89-
})
9065

91-
Describe("no webhook type", func() {
92-
req := &http.Request{
93-
Header: http.Header{"Content-Type": []string{"application/json"}},
94-
Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)},
95-
}
96-
wh := &Webhook{
97-
Handlers: []Handler{},
98-
}
99-
expected := []byte(`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"you must specify your webhook type","code":500}}}
100-
`)
101-
It("should return an error with internal-error status code", func() {
102-
wh.ServeHTTP(w, req)
103-
Expect(w.Body.Bytes()).To(Equal(expected))
66+
It("should return bad-request when given an undecodable body", func() {
67+
req := &http.Request{
68+
Header: http.Header{"Content-Type": []string{"application/json"}},
69+
Body: nopCloser{Reader: bytes.NewBufferString("{")},
70+
}
10471

72+
expected := []byte(
73+
`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}}
74+
`)
75+
webhook.ServeHTTP(respRecorder, req)
76+
Expect(respRecorder.Body.Bytes()).To(Equal(expected))
10577
})
106-
})
10778

108-
Describe("handler can be invoked", func() {
109-
req := &http.Request{
110-
Header: http.Header{"Content-Type": []string{"application/json"}},
111-
Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)},
112-
}
113-
h := &fakeHandler{}
114-
wh := &Webhook{
115-
Type: ValidatingWebhook,
116-
Handlers: []Handler{h},
117-
}
118-
expected := []byte(`{"response":{"uid":"","allowed":true,"status":{"metadata":{},"code":200}}}
79+
It("should return the response given by the handler", func() {
80+
req := &http.Request{
81+
Header: http.Header{"Content-Type": []string{"application/json"}},
82+
Body: nopCloser{Reader: bytes.NewBufferString(`{"request":{}}`)},
83+
}
84+
webhook := &Webhook{
85+
Handler: &fakeHandler{},
86+
}
87+
88+
expected := []byte(`{"response":{"uid":"","allowed":true,"status":{"metadata":{},"code":200}}}
11989
`)
120-
It("should return a response successfully", func() {
121-
wh.ServeHTTP(w, req)
122-
Expect(w.Body.Bytes()).To(Equal(expected))
123-
Expect(h.invoked).To(BeTrue())
90+
webhook.ServeHTTP(respRecorder, req)
91+
Expect(respRecorder.Body.Bytes()).To(Equal(expected))
12492
})
12593
})
12694
})

0 commit comments

Comments
 (0)