Skip to content

Commit

Permalink
Merge pull request #1931 from k8s-infra-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history
…1930-to-release-0.12

🐛 Fix webhook write response error for broken HTTP connection
  • Loading branch information
k8s-ci-robot authored Jun 14, 2022
2 parents d15de97 + 0d4500b commit 697e66d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
12 changes: 10 additions & 2 deletions pkg/webhook/admission/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,16 @@ func (wh *Webhook) writeResponseTyped(w io.Writer, response Response, admRevGVK
// writeAdmissionResponse writes ar to w.
func (wh *Webhook) writeAdmissionResponse(w io.Writer, ar v1.AdmissionReview) {
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.log.Error(err, "unable to encode and write the response")
// Since the `ar v1.AdmissionReview` is a clear and legal object,
// it should not have problem to be marshalled into bytes.
// The error here is probably caused by the abnormal HTTP connection,
// e.g., broken pipe, so we can only write the error response once,
// to avoid endless circular calling.
serverError := Errored(http.StatusInternalServerError, err)
if err = json.NewEncoder(w).Encode(v1.AdmissionReview{Response: &serverError.AdmissionResponse}); err != nil {
wh.log.Error(err, "still unable to encode and write the InternalServerError response")
}
} else {
res := ar.Response
if log := wh.log; log.V(1).Enabled() {
Expand Down
27 changes: 27 additions & 0 deletions pkg/webhook/admission/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -190,6 +191,24 @@ var _ = Describe("Admission Webhooks", func() {
webhook.ServeHTTP(respRecorder, req.WithContext(ctx))
Expect(respRecorder.Body.String()).To(Equal(expected))
})

It("should never run into circular calling if the writer has broken", func() {
req := &http.Request{
Header: http.Header{"Content-Type": []string{"application/json"}},
Body: nopCloser{Reader: bytes.NewBufferString(fmt.Sprintf(`{%s,"request":{}}`, gvkJSONv1))},
}
webhook := &Webhook{
Handler: &fakeHandler{},
log: logf.RuntimeLog.WithName("webhook"),
}

bw := &brokenWriter{ResponseWriter: respRecorder}
Eventually(func() int {
// This should not be blocked by the circular calling of writeResponse and writeAdmissionResponse
webhook.ServeHTTP(bw, req)
return respRecorder.Body.Len()
}, time.Second*3).Should(Equal(0))
})
})
})

Expand Down Expand Up @@ -225,3 +244,11 @@ func (h *fakeHandler) Handle(ctx context.Context, req Request) Response {
Allowed: true,
}}
}

type brokenWriter struct {
http.ResponseWriter
}

func (bw *brokenWriter) Write(buf []byte) (int, error) {
return 0, fmt.Errorf("mock: write: broken pipe")
}

0 comments on commit 697e66d

Please sign in to comment.