Fix webhook write response error for broken HTTP connection

Signed-off-by: FillZpp <[email protected]>

Author: FillZpp

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() {

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")
+}