Skip to content

Commit 32afe3f

Browse files
rfranzkerfranzke
committed
Adapt response writing in webhook handlers
1 parent 2767220 commit 32afe3f

File tree

6 files changed

+89
-91
lines changed

6 files changed

+89
-91
lines changed

‎pkg/webhook/admission/http.go

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
5555
err = errors.New("request body is empty")
5656
wh.log.Error(err, "bad request")
5757
reviewResponse = Errored(http.StatusBadRequest, err)
58-
wh.writeResponse(w, reviewResponse)
58+
wh.writeResponse(w, nil, reviewResponse)
5959
return
6060
}
6161

6262
defer r.Body.Close()
6363
if body, err = ioutil.ReadAll(r.Body); err != nil {
6464
wh.log.Error(err, "unable to read the body from the incoming request")
6565
reviewResponse = Errored(http.StatusBadRequest, err)
66-
wh.writeResponse(w, reviewResponse)
66+
wh.writeResponse(w, nil, reviewResponse)
6767
return
6868
}
6969

@@ -73,7 +73,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7373
err = fmt.Errorf("contentType=%s, expected application/json", contentType)
7474
wh.log.Error(err, "unable to process a request with an unknown content type", "content type", contentType)
7575
reviewResponse = Errored(http.StatusBadRequest, err)
76-
wh.writeResponse(w, reviewResponse)
76+
wh.writeResponse(w, nil, reviewResponse)
7777
return
7878
}
7979

@@ -92,50 +92,38 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
9292
if err != nil {
9393
wh.log.Error(err, "unable to decode the request")
9494
reviewResponse = Errored(http.StatusBadRequest, err)
95-
wh.writeResponse(w, reviewResponse)
95+
wh.writeResponse(w, actualAdmRevGVK, reviewResponse)
9696
return
9797
}
9898
wh.log.V(1).Info("received request", "UID", req.UID, "kind", req.Kind, "resource", req.Resource)
9999

100100
reviewResponse = wh.Handle(ctx, req)
101-
wh.writeResponseTyped(w, reviewResponse, actualAdmRevGVK)
101+
wh.writeResponse(w, actualAdmRevGVK, reviewResponse)
102102
}
103103

104-
// writeResponse writes response to w generically, i.e. without encoding GVK information.
105-
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
106-
wh.writeAdmissionResponse(w, v1.AdmissionReview{Response: &response.AdmissionResponse})
107-
}
108-
109-
// writeResponseTyped writes response to w with GVK set to admRevGVK, which is necessary
110-
// if multiple AdmissionReview versions are permitted by the webhook.
111-
func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, admRevGVK *schema.GroupVersionKind) {
104+
func (wh *Webhook) writeResponse(w io.Writer, gvk *schema.GroupVersionKind, response Response) {
112105
ar := v1.AdmissionReview{
113106
Response: &response.AdmissionResponse,
114107
}
115108
// Default to a v1 AdmissionReview, otherwise the API server may not recognize the request
116109
// if multiple AdmissionReview versions are permitted by the webhook config.
117110
// TODO(estroz): this should be configurable since older API servers won't know about v1.
118-
if admRevGVK == nil || *admRevGVK == (schema.GroupVersionKind{}) {
111+
if gvk == nil || *gvk == (schema.GroupVersionKind{}) {
119112
ar.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("AdmissionReview"))
120113
} else {
121-
ar.SetGroupVersionKind(*admRevGVK)
114+
ar.SetGroupVersionKind(*gvk)
122115
}
123-
wh.writeAdmissionResponse(w, ar)
124-
}
125116

126-
// writeAdmissionResponse writes ar to w.
127-
func (wh *Webhook) writeAdmissionResponse(w io.Writer, ar v1.AdmissionReview) {
128-
err := json.NewEncoder(w).Encode(ar)
129-
if err != nil {
117+
if err := json.NewEncoder(w).Encode(ar); err != nil {
130118
wh.log.Error(err, "unable to encode the response")
131-
wh.writeResponse(w, Errored(http.StatusInternalServerError, err))
119+
wh.writeResponse(w, gvk, Errored(http.StatusInternalServerError, err))
132120
}
133121

134-
log := wh.log.V(1)
122+
logger := wh.log.V(1)
135123
if result := ar.Response.Result; result != nil {
136-
log = log.WithValues("code", result.Code, "reason", result.Reason)
124+
logger = logger.WithValues("code", result.Code, "reason", result.Reason)
137125
}
138-
log.V(1).Info("wrote response", "UID", ar.Response.UID, "allowed", ar.Response.Allowed)
126+
logger.V(1).Info("wrote response", "UID", ar.Response.UID, "allowed", ar.Response.Allowed)
139127
}
140128

141129
// unversionedAdmissionReview is used to decode both v1 and v1beta1 AdmissionReview types.

‎pkg/webhook/admission/http_test.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ var _ = Describe("Admission Webhooks", func() {
5656
It("should return bad-request when given an empty body", func() {
5757
req := &http.Request{Body: nil}
5858

59-
expected := `{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}}
60-
`
59+
expected := fmt.Sprintf(`{%s,"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}}
60+
`, gvkJSONv1)
61+
6162
webhook.ServeHTTP(respRecorder, req)
6263
Expect(respRecorder.Body.String()).To(Equal(expected))
6364
})
@@ -68,9 +69,10 @@ var _ = Describe("Admission Webhooks", func() {
6869
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
6970
}
7071

71-
expected :=
72-
`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=application/foo, expected application/json","code":400}}}
73-
`
72+
expected := fmt.Sprintf(`{%s,"response":{"uid":"","allowed":false,"status":{"metadata":{},`+
73+
`"message":"contentType=application/foo, expected application/json","code":400}}}
74+
`, gvkJSONv1)
75+
7476
webhook.ServeHTTP(respRecorder, req)
7577
Expect(respRecorder.Body.String()).To(Equal(expected))
7678
})
@@ -81,9 +83,10 @@ var _ = Describe("Admission Webhooks", func() {
8183
Body: nopCloser{Reader: bytes.NewBufferString("{")},
8284
}
8385

84-
expected :=
85-
`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}}
86-
`
86+
expected := fmt.Sprintf(`{%s,"response":{"uid":"","allowed":false,"status":{"metadata":{},`+
87+
`"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}}
88+
`, gvkJSONv1)
89+
8790
webhook.ServeHTTP(respRecorder, req)
8891
Expect(respRecorder.Body.String()).To(Equal(expected))
8992
})

‎pkg/webhook/authentication/http.go

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
5555
err = errors.New("request body is empty")
5656
wh.log.Error(err, "bad request")
5757
reviewResponse = Errored(err)
58-
wh.writeResponse(w, reviewResponse)
58+
wh.writeResponse(w, nil, reviewResponse)
5959
return
6060
}
6161

6262
defer r.Body.Close()
6363
if body, err = ioutil.ReadAll(r.Body); err != nil {
6464
wh.log.Error(err, "unable to read the body from the incoming request")
6565
reviewResponse = Errored(err)
66-
wh.writeResponse(w, reviewResponse)
66+
wh.writeResponse(w, nil, reviewResponse)
6767
return
6868
}
6969

@@ -73,7 +73,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7373
err = fmt.Errorf("contentType=%s, expected application/json", contentType)
7474
wh.log.Error(err, "unable to process a request with an unknown content type", "content type", contentType)
7575
reviewResponse = Errored(err)
76-
wh.writeResponse(w, reviewResponse)
76+
wh.writeResponse(w, nil, reviewResponse)
7777
return
7878
}
7979

@@ -93,7 +93,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
9393
if err != nil {
9494
wh.log.Error(err, "unable to decode the request")
9595
reviewResponse = Errored(err)
96-
wh.writeResponse(w, reviewResponse)
96+
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
9797
return
9898
}
9999
wh.log.V(1).Info("received request", "UID", req.UID, "kind", req.Kind)
@@ -102,42 +102,38 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
102102
err = errors.New("token is empty")
103103
wh.log.Error(err, "bad request")
104104
reviewResponse = Errored(err)
105-
wh.writeResponse(w, reviewResponse)
105+
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
106106
return
107107
}
108108

109109
reviewResponse = wh.Handle(ctx, req)
110-
wh.writeResponseTyped(w, reviewResponse, actualTokRevGVK)
110+
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
111111
}
112112

113-
// writeResponse writes response to w generically, i.e. without encoding GVK information.
114-
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
115-
wh.writeTokenResponse(w, response.TokenReview)
116-
}
117-
118-
// writeResponseTyped writes response to w with GVK set to tokRevGVK, which is necessary
119-
// if multiple TokenReview versions are permitted by the webhook.
120-
func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, tokRevGVK *schema.GroupVersionKind) {
113+
func (wh *Webhook) writeResponse(w io.Writer, gvk *schema.GroupVersionKind, response Response) {
121114
ar := response.TokenReview
122115

123116
// Default to a v1 TokenReview, otherwise the API server may not recognize the request
124117
// if multiple TokenReview versions are permitted by the webhook config.
125-
if tokRevGVK == nil || *tokRevGVK == (schema.GroupVersionKind{}) {
118+
if gvk == nil || *gvk == (schema.GroupVersionKind{}) {
126119
ar.SetGroupVersionKind(authenticationv1.SchemeGroupVersion.WithKind("TokenReview"))
127120
} else {
128-
ar.SetGroupVersionKind(*tokRevGVK)
121+
ar.SetGroupVersionKind(*gvk)
129122
}
130-
wh.writeTokenResponse(w, ar)
131-
}
132123

133-
// writeTokenResponse writes ar to w.
134-
func (wh *Webhook) writeTokenResponse(w io.Writer, ar authenticationv1.TokenReview) {
135124
if err := json.NewEncoder(w).Encode(ar); err != nil {
136125
wh.log.Error(err, "unable to encode the response")
137-
wh.writeResponse(w, Errored(err))
126+
wh.writeResponse(w, gvk, Errored(err))
138127
}
139-
log.V(1).Info("wrote response", "UID", ar.UID, "authenticated", ar.Status.Authenticated)
140-
return
128+
129+
wh.log.
130+
V(1).
131+
WithValues(
132+
"uid", ar.UID,
133+
"authenticated", ar.Status.Authenticated,
134+
"error", ar.Status.Error,
135+
).
136+
Info("wrote response")
141137
}
142138

143139
// unversionedTokenReview is used to decode both v1 and v1beta1 TokenReview types.

‎pkg/webhook/authentication/http_test.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,10 @@ var _ = Describe("Authentication Webhooks", func() {
5555
It("should return bad-request when given an empty body", func() {
5656
req := &http.Request{Body: nil}
5757

58-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"request body is empty"}}
59-
`
58+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},`+
59+
`"error":"request body is empty"}}
60+
`, gvkJSONv1)
61+
6062
webhook.ServeHTTP(respRecorder, req)
6163
Expect(respRecorder.Body.String()).To(Equal(expected))
6264
})
@@ -68,8 +70,10 @@ var _ = Describe("Authentication Webhooks", func() {
6870
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
6971
}
7072

71-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"contentType=application/foo, expected application/json"}}
72-
`
73+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},`+
74+
`"error":"contentType=application/foo, expected application/json"}}
75+
`, gvkJSONv1)
76+
7377
webhook.ServeHTTP(respRecorder, req)
7478
Expect(respRecorder.Body.String()).To(Equal(expected))
7579
})
@@ -81,8 +85,10 @@ var _ = Describe("Authentication Webhooks", func() {
8185
Body: nopCloser{Reader: bytes.NewBufferString("{")},
8286
}
8387

84-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"couldn't get version/kind; json parse error: unexpected end of JSON input"}}
85-
`
88+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},`+
89+
`"error":"couldn't get version/kind; json parse error: unexpected end of JSON input"}}
90+
`, gvkJSONv1)
91+
8692
webhook.ServeHTTP(respRecorder, req)
8793
Expect(respRecorder.Body.String()).To(Equal(expected))
8894
})
@@ -94,8 +100,9 @@ var _ = Describe("Authentication Webhooks", func() {
94100
Body: nopCloser{Reader: bytes.NewBufferString(`{"spec":{"token":""}}`)},
95101
}
96102

97-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"token is empty"}}
98-
`
103+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"token is empty"}}
104+
`, gvkJSONv1)
105+
99106
webhook.ServeHTTP(respRecorder, req)
100107
Expect(respRecorder.Body.String()).To(Equal(expected))
101108
})

‎pkg/webhook/authorization/http.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
5555
err = errors.New("request body is empty")
5656
wh.log.Error(err, "bad request")
5757
reviewResponse = Errored(err)
58-
wh.writeResponse(w, reviewResponse)
58+
wh.writeResponse(w, nil, reviewResponse)
5959
return
6060
}
6161

6262
defer r.Body.Close()
6363
if body, err = ioutil.ReadAll(r.Body); err != nil {
6464
wh.log.Error(err, "unable to read the body from the incoming request")
6565
reviewResponse = Errored(err)
66-
wh.writeResponse(w, reviewResponse)
66+
wh.writeResponse(w, nil, reviewResponse)
6767
return
6868
}
6969

@@ -73,7 +73,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7373
err = fmt.Errorf("contentType=%s, expected application/json", contentType)
7474
wh.log.Error(err, "unable to process a request with an unknown content type", "content type", contentType)
7575
reviewResponse = Errored(err)
76-
wh.writeResponse(w, reviewResponse)
76+
wh.writeResponse(w, nil, reviewResponse)
7777
return
7878
}
7979

@@ -82,42 +82,42 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
8282
if err != nil {
8383
wh.log.Error(err, "unable to decode the request")
8484
reviewResponse = Errored(err)
85-
wh.writeResponse(w, reviewResponse)
85+
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
8686
return
8787
}
8888
wh.log.V(1).Info("received request", "UID", sar.UID, "kind", sar.Kind)
8989

9090
reviewResponse = wh.Handle(ctx, Request{sar.SubjectAccessReview})
91-
wh.writeResponseTyped(w, reviewResponse, actualTokRevGVK)
91+
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
9292
}
9393

94-
// writeResponse writes response to w generically, i.e. without encoding GVK information.
95-
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
96-
wh.writeSubjectAccessReviewResponse(w, response.SubjectAccessReview)
97-
}
98-
99-
// writeResponseTyped writes response to w with GVK set to subjRevGVK, which is necessary
100-
// if multiple SubjectAccessReview versions are permitted by the webhook.
101-
func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, subjRevGVK *schema.GroupVersionKind) {
94+
func (wh *Webhook) writeResponse(w io.Writer, gvk *schema.GroupVersionKind, response Response) {
10295
ar := response.SubjectAccessReview
10396

10497
// Default to a v1 SubjectAccessReview, otherwise the API server may not recognize the request
10598
// if multiple SubjectAccessReview versions are permitted by the webhook config.
106-
if subjRevGVK == nil || *subjRevGVK == (schema.GroupVersionKind{}) {
99+
if gvk == nil || *gvk == (schema.GroupVersionKind{}) {
107100
ar.SetGroupVersionKind(authorizationv1.SchemeGroupVersion.WithKind("SubjectAccessReview"))
108101
} else {
109-
ar.SetGroupVersionKind(*subjRevGVK)
102+
ar.SetGroupVersionKind(*gvk)
110103
}
111-
wh.writeSubjectAccessReviewResponse(w, ar)
112-
}
113104

114-
// writeSubjectAccessReviewResponse writes ar to w.
115-
func (wh *Webhook) writeSubjectAccessReviewResponse(w io.Writer, ar authorizationv1.SubjectAccessReview) {
116105
if err := json.NewEncoder(w).Encode(ar); err != nil {
117106
wh.log.Error(err, "unable to encode the response")
118-
wh.writeResponse(w, Errored(err))
107+
wh.writeResponse(w, gvk, Errored(err))
108+
return
119109
}
120-
log.V(1).Info("wrote response", "UID", ar.UID, "authorized", ar.Status.Allowed)
110+
111+
wh.log.
112+
V(1).
113+
WithValues(
114+
"uid", ar.UID,
115+
"allowed", ar.Status.Allowed,
116+
"denied", ar.Status.Denied,
117+
"reason", ar.Status.Reason,
118+
"error", ar.Status.EvaluationError,
119+
).
120+
Info("wrote response")
121121
}
122122

123123
func (wh *Webhook) decodeRequestBody(body []byte) (unversionedSubjectAccessReview, *schema.GroupVersionKind, error) {

‎pkg/webhook/authorization/http_test.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ var _ = Describe("Authentication Webhooks", func() {
5555
It("should return bad-request when given an empty body", func() {
5656
req := &http.Request{Body: nil}
5757

58-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,"evaluationError":"request body is empty"}}
59-
`
58+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,"evaluationError":"request body is empty"}}
59+
`, gvkJSONv1)
60+
6061
webhook.ServeHTTP(respRecorder, req)
6162
Expect(respRecorder.Body.String()).To(Equal(expected))
6263
})
@@ -68,8 +69,10 @@ var _ = Describe("Authentication Webhooks", func() {
6869
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
6970
}
7071

71-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,"evaluationError":"contentType=application/foo, expected application/json"}}
72-
`
72+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,`+
73+
`"evaluationError":"contentType=application/foo, expected application/json"}}
74+
`, gvkJSONv1)
75+
7376
webhook.ServeHTTP(respRecorder, req)
7477
Expect(respRecorder.Body.String()).To(Equal(expected))
7578
})
@@ -81,9 +84,10 @@ var _ = Describe("Authentication Webhooks", func() {
8184
Body: nopCloser{Reader: bytes.NewBufferString("{")},
8285
}
8386

84-
expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,` +
87+
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,`+
8588
`"evaluationError":"couldn't get version/kind; json parse error: unexpected end of JSON input"}}
86-
`
89+
`, gvkJSONv1)
90+
8791
webhook.ServeHTTP(respRecorder, req)
8892
Expect(respRecorder.Body.String()).To(Equal(expected))
8993
})

0 commit comments

Comments
 (0)