add missing crashhandler

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
kubernetes-sigs · Dec 3, 2023 · b2c9381 · b2c9381
1 parent 8d817e7
commit b2c9381
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 0 deletions.
diff --git a/pkg/webhook/admission/http.go b/pkg/webhook/admission/http.go
@@ -42,6 +42,11 @@ func init() {
 var _ http.Handler = &Webhook{}
 
 func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	defer utilruntime.HandleCrash(func(_ interface{}) {
+		// Assume the crash happened before the response was written.
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+	})
+
 	ctx := r.Context()
 	if wh.WithContextFunc != nil {
 		ctx = wh.WithContextFunc(ctx, r)

diff --git a/pkg/webhook/admission/http_test.go b/pkg/webhook/admission/http_test.go
@@ -198,6 +198,33 @@ var _ = Describe("Admission Webhooks", func() {
 				return respRecorder.Body.Len()
 			}, time.Second*3).Should(Equal(0))
 		})
+
+		It("should handle crashes", func() {
+			req := &http.Request{
+				Header: http.Header{"Content-Type": []string{"application/json"}},
+				Method: http.MethodPost,
+				Body:   nopCloser{Reader: bytes.NewBufferString(`{"spec":{"token":"foobar"}}`)},
+			}
+			webhook := &Webhook{
+				Handler: &fakeHandler{
+					fn: func(ctx context.Context, req Request) Response {
+						panic("boom")
+					},
+				},
+			}
+
+			expected := `internal server error
+`
+			(func() {
+				defer func() {
+					if r := recover(); r != nil {
+						Expect(r).To(Equal("boom"))
+					}
+				}()
+				webhook.ServeHTTP(respRecorder, req)
+			})()
+			Expect(respRecorder.Body.String()).To(Equal(expected))
+		})
 	})
 })
 

diff --git a/pkg/webhook/authentication/http.go b/pkg/webhook/authentication/http.go
@@ -42,6 +42,11 @@ func init() {
 var _ http.Handler = &Webhook{}
 
 func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	defer utilruntime.HandleCrash(func(_ interface{}) {
+		// Assume the crash happened before the response was written.
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+	})
+
 	ctx := r.Context()
 	if wh.WithContextFunc != nil {
 		ctx = wh.WithContextFunc(ctx, r)

diff --git a/pkg/webhook/authentication/http_test.go b/pkg/webhook/authentication/http_test.go
@@ -181,6 +181,33 @@ var _ = Describe("Authentication Webhooks", func() {
 			webhook.ServeHTTP(respRecorder, req.WithContext(ctx))
 			Expect(respRecorder.Body.String()).To(Equal(expected))
 		})
+
+		It("should handle crashes", func() {
+			req := &http.Request{
+				Header: http.Header{"Content-Type": []string{"application/json"}},
+				Method: http.MethodPost,
+				Body:   nopCloser{Reader: bytes.NewBufferString(`{"spec":{"token":"foobar"}}`)},
+			}
+			webhook := &Webhook{
+				Handler: &fakeHandler{
+					fn: func(ctx context.Context, req Request) Response {
+						panic("boom")
+					},
+				},
+			}
+
+			expected := `internal server error
+`
+			(func() {
+				defer func() {
+					if r := recover(); r != nil {
+						Expect(r).To(Equal("boom"))
+					}
+				}()
+				webhook.ServeHTTP(respRecorder, req)
+			})()
+			Expect(respRecorder.Body.String()).To(Equal(expected))
+		})
 	})
 })