From 6bcab3dbd29d140ae614e63d3effbfd77e2ef5ea Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 5 Sep 2018 10:33:15 +0200 Subject: [PATCH] UPSTREAM: 68008: apiserver: forward panic in WithTimeout filter --- .../k8s.io/apiserver/pkg/server/filters/BUILD | 1 + .../apiserver/pkg/server/filters/timeout.go | 11 +++++-- .../pkg/server/filters/timeout_test.go | 32 ++++++++++++++++--- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/BUILD b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/BUILD index 6822469d4519..b2b0328b02ec 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/BUILD +++ b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/BUILD @@ -20,6 +20,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/filters:go_default_library", diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go index eaf767f4ff20..7d37ed05a3de 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go +++ b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go @@ -91,14 +91,19 @@ func (t *timeoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - done := make(chan struct{}) + result := make(chan interface{}) tw := newTimeoutWriter(w) go func() { + defer func() { + result <- recover() + }() t.handler.ServeHTTP(tw, r) - close(done) }() select { - case <-done: + case err := <-result: + if err != nil { + panic(err) + } return case <-after: postTimeoutFn() diff --git a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go index 441ca3efd381..28403f93509f 100644 --- a/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go +++ b/vendor/k8s.io/kubernetes/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/runtime" ) type recorder struct { @@ -50,22 +51,33 @@ func (r *recorder) Count() int { } func TestTimeout(t *testing.T) { + origReallyCrash := runtime.ReallyCrash + runtime.ReallyCrash = false + defer func() { + runtime.ReallyCrash = origReallyCrash + }() + sendResponse := make(chan struct{}, 1) + doPanic := make(chan struct{}, 1) writeErrors := make(chan error, 1) timeout := make(chan time.Time, 1) resp := "test response" timeoutErr := apierrors.NewServerTimeout(schema.GroupResource{Group: "foo", Resource: "bar"}, "get", 0) record := &recorder{} - ts := httptest.NewServer(WithTimeout(http.HandlerFunc( + ts := httptest.NewServer(WithPanicRecovery(WithTimeout(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { - <-sendResponse - _, err := w.Write([]byte(resp)) - writeErrors <- err + select { + case <-sendResponse: + _, err := w.Write([]byte(resp)) + writeErrors <- err + case <-doPanic: + panic("inner handler panics") + } }), func(req *http.Request) (*http.Request, <-chan time.Time, func(), *apierrors.StatusError) { return req, timeout, record.Record, timeoutErr - })) + }))) defer ts.Close() // No timeouts @@ -114,4 +126,14 @@ func TestTimeout(t *testing.T) { if err := <-writeErrors; err != http.ErrHandlerTimeout { t.Errorf("got Write error of %v; expected %v", err, http.ErrHandlerTimeout) } + + // Panics + doPanic <- struct{}{} + res, err = http.Get(ts.URL) + if err != nil { + t.Fatal(err) + } + if res.StatusCode != http.StatusInternalServerError { + t.Errorf("got res.StatusCode %d; expected %d due to panic", res.StatusCode, http.StatusInternalServerError) + } }