From 0d4500b4498e4ac6f441ab247bcdb85b3219879c Mon Sep 17 00:00:00 2001 From: FillZpp Date: Sat, 11 Jun 2022 21:46:14 +0800 Subject: [PATCH] Fix webhook write response error for broken HTTP connection Signed-off-by: FillZpp --- pkg/webhook/admission/http.go | 12 ++++++++++-- pkg/webhook/admission/http_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/webhook/admission/http.go b/pkg/webhook/admission/http.go index f640104786..066cc42256 100644 --- a/pkg/webhook/admission/http.go +++ b/pkg/webhook/admission/http.go @@ -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() { diff --git a/pkg/webhook/admission/http_test.go b/pkg/webhook/admission/http_test.go index 7dd2d5bcfc..af8ff31ee2 100644 --- a/pkg/webhook/admission/http_test.go +++ b/pkg/webhook/admission/http_test.go @@ -23,6 +23,7 @@ import ( "io" "net/http" "net/http/httptest" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -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)) + }) }) }) @@ -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") +}