Merge pull request #68001 from sttts/sttts-timeout-panic-forward

Automatic merge from submit-queue (batch tested with PRs 66577, 67948, 68001, 67982). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

apiserver: forward panic in WithTimeout filter

```release-note
Return apiserver panics as 500 errors instead terminating the apiserver process.
```

Without this PR a panic in a HTTP handler will not be caught in the Go routine started by the timeout filter. Uncaught panics terminate the process.

This is a strong condidate to be backported to 1.11, 1.10 and 1.9.

Kubernetes-commit: ca8f267cc86c183a23668e24a59a0eaa973daf93

Godeps/Godeps.json

@@ -856,131 +856,131 @@
 		},
 		{
 			"ImportPath": "k8s.io/api/admission/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/admissionregistration/v1alpha1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/admissionregistration/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/apps/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/apps/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/apps/v1beta2",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/authentication/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/authentication/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/authorization/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/authorization/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/autoscaling/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/autoscaling/v2beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/autoscaling/v2beta2",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/batch/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/batch/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/batch/v2alpha1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/certificates/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/coordination/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/core/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/events/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/extensions/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/networking/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/policy/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/rbac/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/rbac/v1alpha1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/rbac/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/scheduling/v1alpha1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/scheduling/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/settings/v1alpha1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/storage/v1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/storage/v1alpha1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/api/storage/v1beta1",
-			"Rev": "d150a58332329a1cd3e80959b04f5487a8be7149"
+			"Rev": "966002ba0fc391179b1696ae6ec7862071637fa5"
 		},
 		{
 			"ImportPath": "k8s.io/apimachinery/pkg/api/apitesting",

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

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