Skip to content

Commit

Permalink
Adapt response writing in webhook handlers
Browse files Browse the repository at this point in the history
  • Loading branch information
@rfranzke
rfranzke committed May 25, 2021
1 parent 2767220 commit 32afe3f
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 91 deletions.
38 changes: 13 additions & 25 deletions pkg/webhook/admission/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
err = errors.New("request body is empty")
wh.log.Error(err, "bad request")
reviewResponse = Errored(http.StatusBadRequest, err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, nil, reviewResponse)
return
}

defer r.Body.Close()
if body, err = ioutil.ReadAll(r.Body); err != nil {
wh.log.Error(err, "unable to read the body from the incoming request")
reviewResponse = Errored(http.StatusBadRequest, err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, nil, reviewResponse)
return
}

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

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

reviewResponse = wh.Handle(ctx, req)
wh.writeResponseTyped(w, reviewResponse, actualAdmRevGVK)
wh.writeResponse(w, actualAdmRevGVK, reviewResponse)
}

// writeResponse writes response to w generically, i.e. without encoding GVK information.
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
wh.writeAdmissionResponse(w, v1.AdmissionReview{Response: &response.AdmissionResponse})
}

// writeResponseTyped writes response to w with GVK set to admRevGVK, which is necessary
// if multiple AdmissionReview versions are permitted by the webhook.
func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, admRevGVK *schema.GroupVersionKind) {
func (wh *Webhook) writeResponse(w io.Writer, gvk *schema.GroupVersionKind, response Response) {
ar := v1.AdmissionReview{
Response: &response.AdmissionResponse,
}
// Default to a v1 AdmissionReview, otherwise the API server may not recognize the request
// if multiple AdmissionReview versions are permitted by the webhook config.
// TODO(estroz): this should be configurable since older API servers won't know about v1.
if admRevGVK == nil || *admRevGVK == (schema.GroupVersionKind{}) {
if gvk == nil || *gvk == (schema.GroupVersionKind{}) {
ar.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("AdmissionReview"))
} else {
ar.SetGroupVersionKind(*admRevGVK)
ar.SetGroupVersionKind(*gvk)
}
wh.writeAdmissionResponse(w, ar)
}

// writeAdmissionResponse writes ar to w.
func (wh *Webhook) writeAdmissionResponse(w io.Writer, ar v1.AdmissionReview) {
err := json.NewEncoder(w).Encode(ar)
if err != nil {
if err := json.NewEncoder(w).Encode(ar); err != nil {
wh.log.Error(err, "unable to encode the response")
wh.writeResponse(w, Errored(http.StatusInternalServerError, err))
wh.writeResponse(w, gvk, Errored(http.StatusInternalServerError, err))
}

log := wh.log.V(1)
logger := wh.log.V(1)
if result := ar.Response.Result; result != nil {
log = log.WithValues("code", result.Code, "reason", result.Reason)
logger = logger.WithValues("code", result.Code, "reason", result.Reason)
}
log.V(1).Info("wrote response", "UID", ar.Response.UID, "allowed", ar.Response.Allowed)
logger.V(1).Info("wrote response", "UID", ar.Response.UID, "allowed", ar.Response.Allowed)
}

// unversionedAdmissionReview is used to decode both v1 and v1beta1 AdmissionReview types.
Expand Down
19 changes: 11 additions & 8 deletions pkg/webhook/admission/http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ var _ = Describe("Admission Webhooks", func() {
It("should return bad-request when given an empty body", func() {
req := &http.Request{Body: nil}

expected := `{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}}
`
expected := fmt.Sprintf(`{%s,"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request body is empty","code":400}}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -68,9 +69,10 @@ var _ = Describe("Admission Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
}

expected :=
`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"contentType=application/foo, expected application/json","code":400}}}
`
expected := fmt.Sprintf(`{%s,"response":{"uid":"","allowed":false,"status":{"metadata":{},`+
`"message":"contentType=application/foo, expected application/json","code":400}}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -81,9 +83,10 @@ var _ = Describe("Admission Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBufferString("{")},
}

expected :=
`{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}}
`
expected := fmt.Sprintf(`{%s,"response":{"uid":"","allowed":false,"status":{"metadata":{},`+
`"message":"couldn't get version/kind; json parse error: unexpected end of JSON input","code":400}}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand Down
42 changes: 19 additions & 23 deletions pkg/webhook/authentication/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
err = errors.New("request body is empty")
wh.log.Error(err, "bad request")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, nil, reviewResponse)
return
}

defer r.Body.Close()
if body, err = ioutil.ReadAll(r.Body); err != nil {
wh.log.Error(err, "unable to read the body from the incoming request")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, nil, reviewResponse)
return
}

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

Expand All @@ -93,7 +93,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if err != nil {
wh.log.Error(err, "unable to decode the request")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
return
}
wh.log.V(1).Info("received request", "UID", req.UID, "kind", req.Kind)
Expand All @@ -102,42 +102,38 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
err = errors.New("token is empty")
wh.log.Error(err, "bad request")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
return
}

reviewResponse = wh.Handle(ctx, req)
wh.writeResponseTyped(w, reviewResponse, actualTokRevGVK)
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
}

// writeResponse writes response to w generically, i.e. without encoding GVK information.
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
wh.writeTokenResponse(w, response.TokenReview)
}

// writeResponseTyped writes response to w with GVK set to tokRevGVK, which is necessary
// if multiple TokenReview versions are permitted by the webhook.
func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, tokRevGVK *schema.GroupVersionKind) {
func (wh *Webhook) writeResponse(w io.Writer, gvk *schema.GroupVersionKind, response Response) {
ar := response.TokenReview

// Default to a v1 TokenReview, otherwise the API server may not recognize the request
// if multiple TokenReview versions are permitted by the webhook config.
if tokRevGVK == nil || *tokRevGVK == (schema.GroupVersionKind{}) {
if gvk == nil || *gvk == (schema.GroupVersionKind{}) {
ar.SetGroupVersionKind(authenticationv1.SchemeGroupVersion.WithKind("TokenReview"))
} else {
ar.SetGroupVersionKind(*tokRevGVK)
ar.SetGroupVersionKind(*gvk)
}
wh.writeTokenResponse(w, ar)
}

// writeTokenResponse writes ar to w.
func (wh *Webhook) writeTokenResponse(w io.Writer, ar authenticationv1.TokenReview) {
if err := json.NewEncoder(w).Encode(ar); err != nil {
wh.log.Error(err, "unable to encode the response")
wh.writeResponse(w, Errored(err))
wh.writeResponse(w, gvk, Errored(err))
}
log.V(1).Info("wrote response", "UID", ar.UID, "authenticated", ar.Status.Authenticated)
return

wh.log.
V(1).
WithValues(
"uid", ar.UID,
"authenticated", ar.Status.Authenticated,
"error", ar.Status.Error,
).
Info("wrote response")
}

// unversionedTokenReview is used to decode both v1 and v1beta1 TokenReview types.
Expand Down
23 changes: 15 additions & 8 deletions pkg/webhook/authentication/http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ var _ = Describe("Authentication Webhooks", func() {
It("should return bad-request when given an empty body", func() {
req := &http.Request{Body: nil}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"request body is empty"}}
`
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},`+
`"error":"request body is empty"}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -68,8 +70,10 @@ var _ = Describe("Authentication Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"contentType=application/foo, expected application/json"}}
`
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},`+
`"error":"contentType=application/foo, expected application/json"}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -81,8 +85,10 @@ var _ = Describe("Authentication Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBufferString("{")},
}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"couldn't get version/kind; json parse error: unexpected end of JSON input"}}
`
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},`+
`"error":"couldn't get version/kind; json parse error: unexpected end of JSON input"}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -94,8 +100,9 @@ var _ = Describe("Authentication Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBufferString(`{"spec":{"token":""}}`)},
}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"token is empty"}}
`
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"token is empty"}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand Down
42 changes: 21 additions & 21 deletions pkg/webhook/authorization/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
err = errors.New("request body is empty")
wh.log.Error(err, "bad request")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, nil, reviewResponse)
return
}

defer r.Body.Close()
if body, err = ioutil.ReadAll(r.Body); err != nil {
wh.log.Error(err, "unable to read the body from the incoming request")
reviewResponse = Errored(err)
wh.writeResponse(w, reviewResponse)
wh.writeResponse(w, nil, reviewResponse)
return
}

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

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

reviewResponse = wh.Handle(ctx, Request{sar.SubjectAccessReview})
wh.writeResponseTyped(w, reviewResponse, actualTokRevGVK)
wh.writeResponse(w, actualTokRevGVK, reviewResponse)
}

// writeResponse writes response to w generically, i.e. without encoding GVK information.
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
wh.writeSubjectAccessReviewResponse(w, response.SubjectAccessReview)
}

// writeResponseTyped writes response to w with GVK set to subjRevGVK, which is necessary
// if multiple SubjectAccessReview versions are permitted by the webhook.
func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, subjRevGVK *schema.GroupVersionKind) {
func (wh *Webhook) writeResponse(w io.Writer, gvk *schema.GroupVersionKind, response Response) {
ar := response.SubjectAccessReview

// Default to a v1 SubjectAccessReview, otherwise the API server may not recognize the request
// if multiple SubjectAccessReview versions are permitted by the webhook config.
if subjRevGVK == nil || *subjRevGVK == (schema.GroupVersionKind{}) {
if gvk == nil || *gvk == (schema.GroupVersionKind{}) {
ar.SetGroupVersionKind(authorizationv1.SchemeGroupVersion.WithKind("SubjectAccessReview"))
} else {
ar.SetGroupVersionKind(*subjRevGVK)
ar.SetGroupVersionKind(*gvk)
}
wh.writeSubjectAccessReviewResponse(w, ar)
}

// writeSubjectAccessReviewResponse writes ar to w.
func (wh *Webhook) writeSubjectAccessReviewResponse(w io.Writer, ar authorizationv1.SubjectAccessReview) {
if err := json.NewEncoder(w).Encode(ar); err != nil {
wh.log.Error(err, "unable to encode the response")
wh.writeResponse(w, Errored(err))
wh.writeResponse(w, gvk, Errored(err))
return
}
log.V(1).Info("wrote response", "UID", ar.UID, "authorized", ar.Status.Allowed)

wh.log.
V(1).
WithValues(
"uid", ar.UID,
"allowed", ar.Status.Allowed,
"denied", ar.Status.Denied,
"reason", ar.Status.Reason,
"error", ar.Status.EvaluationError,
).
Info("wrote response")
}

func (wh *Webhook) decodeRequestBody(body []byte) (unversionedSubjectAccessReview, *schema.GroupVersionKind, error) {
Expand Down
16 changes: 10 additions & 6 deletions pkg/webhook/authorization/http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ var _ = Describe("Authentication Webhooks", func() {
It("should return bad-request when given an empty body", func() {
req := &http.Request{Body: nil}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,"evaluationError":"request body is empty"}}
`
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,"evaluationError":"request body is empty"}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -68,8 +69,10 @@ var _ = Describe("Authentication Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBuffer(nil)},
}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,"evaluationError":"contentType=application/foo, expected application/json"}}
`
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,`+
`"evaluationError":"contentType=application/foo, expected application/json"}}
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand All @@ -81,9 +84,10 @@ var _ = Describe("Authentication Webhooks", func() {
Body: nopCloser{Reader: bytes.NewBufferString("{")},
}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,` +
expected := fmt.Sprintf(`{%s,"metadata":{"creationTimestamp":null},"spec":{},"status":{"allowed":false,`+
`"evaluationError":"couldn't get version/kind; json parse error: unexpected end of JSON input"}}
`
`, gvkJSONv1)

webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})
Expand Down

0 comments on commit 32afe3f

Please sign in to comment.