Merge pull request #96681 from tkashem/request-timout-e2e

Use default value when the specified timeout for a request is 0s

staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline.go

@@ -69,7 +69,10 @@ func WithRequestDeadline(handler http.Handler, longRunning request.LongRunningRe
 				return
 			}
 
-			timeout = userSpecifiedTimeout
+			// if the user has specified a timeout of 0s, this means no timeout, so we should use the maximum timeout allowed.
+			if userSpecifiedTimeout != 0 {
+				timeout = userSpecifiedTimeout
+			}
 		}
 
 		ctx, cancel := context.WithTimeout(ctx, timeout)

staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline_test.go

@@ -94,6 +94,15 @@ func TestWithRequestDeadline(t *testing.T) {
 			deadlineExpected:         14 * time.Second, // to account for the delay in verification
 			statusCodeExpected:       http.StatusOK,
 		},
+		{
+			name:                     "the specified timeout is 0s, default deadline is expected to be set",
+			requestURL:               "/api/v1/namespaces?timeout=0s",
+			longRunning:              false,
+			handlerCallCountExpected: 1,
+			hasDeadlineExpected:      true,
+			deadlineExpected:         requestTimeoutMaximum - time.Second, // to account for the delay in verification
+			statusCodeExpected:       http.StatusOK,
+		},
 		{
 			name:                     "the user does not specify any request timeout, default deadline is expected to be set",
 			requestURL:               "/api/v1/namespaces?timeout=",

test/e2e/apimachinery/BUILD

@@ -25,6 +25,7 @@ go_library(
         "health_handlers.go",
         "namespace.go",
         "protocol.go",
+        "request_timeout.go",
         "resource_quota.go",
         "server_version.go",
         "table_conversion.go",

test/e2e/apimachinery/request_timeout.go

@@ -0,0 +1,113 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apimachinery
+
+import (
+	"io/ioutil"
+	"net/http"
+	"strings"
+
+	"github.com/onsi/ginkgo"
+	"k8s.io/client-go/rest"
+	"k8s.io/kubernetes/test/e2e/framework"
+)
+
+const (
+	invalidTimeoutMessageExpected = "invalid timeout specified in the request URL"
+	timeoutExceedsMaximumExpected = "timeout specified in the request URL exceeds the maximum timeout allowed by the server"
+)
+
+var _ = SIGDescribe("Server request timeout", func() {
+	f := framework.NewDefaultFramework("request-timeout")
+
+	ginkgo.It("should return HTTP status code 400 if the user specifies an invalid timeout in the request URL", func() {
+		rt := getRoundTripper(f)
+		req := newRequest(f, "invalid")
+
+		response, err := rt.RoundTrip(req)
+		framework.ExpectNoError(err)
+		defer response.Body.Close()
+
+		if response.StatusCode != http.StatusBadRequest {
+			framework.Failf("expected HTTP status code: %d, but got: %d", http.StatusBadRequest, response.StatusCode)
+		}
+
+		messageGot := readBody(response)
+		if !strings.Contains(messageGot, invalidTimeoutMessageExpected) {
+			framework.Failf("expected HTTP status message to contain: %s, but got: %s", invalidTimeoutMessageExpected, messageGot)
+		}
+	})
+
+	ginkgo.It("should return HTTP status code 400 if the specified timeout in the request URL exceeds maximum allowed", func() {
+		rt := getRoundTripper(f)
+		// Choose a timeout that exceeds the default timeout (60s) enforced by the apiserver
+		req := newRequest(f, "3m")
+
+		response, err := rt.RoundTrip(req)
+		framework.ExpectNoError(err)
+		defer response.Body.Close()
+
+		if response.StatusCode != http.StatusBadRequest {
+			framework.Failf("expected HTTP status code: %d, but got: %d", http.StatusBadRequest, response.StatusCode)
+		}
+
+		messageGot := readBody(response)
+		if !strings.Contains(messageGot, timeoutExceedsMaximumExpected) {
+			framework.Failf("expected HTTP status message to contain: %s, but got: %s", timeoutExceedsMaximumExpected, messageGot)
+		}
+	})
+
+	ginkgo.It("default timeout should be used if the specified timeout in the request URL is 0s", func() {
+		rt := getRoundTripper(f)
+		req := newRequest(f, "0s")
+
+		response, err := rt.RoundTrip(req)
+		framework.ExpectNoError(err)
+		defer response.Body.Close()
+
+		if response.StatusCode != http.StatusOK {
+			framework.Failf("expected HTTP status code: %d, but got: %d", http.StatusOK, response.StatusCode)
+		}
+	})
+})
+
+func getRoundTripper(f *framework.Framework) http.RoundTripper {
+	config := rest.CopyConfig(f.ClientConfig())
+
+	// ensure we don't enforce any transport timeout from the client side.
+	config.Timeout = 0
+
+	roundTripper, err := rest.TransportFor(config)
+	framework.ExpectNoError(err)
+
+	return roundTripper
+}
+
+func newRequest(f *framework.Framework, timeout string) *http.Request {
+	req, err := http.NewRequest(http.MethodGet, f.ClientSet.CoreV1().RESTClient().Get().
+		Param("timeout", timeout).AbsPath("version").URL().String(), nil)
+	framework.ExpectNoError(err)
+
+	return req
+}
+
+func readBody(response *http.Response) string {
+	raw, err := ioutil.ReadAll(response.Body)
+	framework.ExpectNoError(err)
+
+	return string(raw)
+}