Merge pull request #1638 from jacksgt/configure-upstream-timeout

Configure upstream timeout

CHANGELOG.md

@@ -39,6 +39,7 @@ If you are using an architecture specific tag (ex: v7.2.1-arm64) you should move
 - [#1286](https://github.com/oauth2-proxy/oauth2-proxy/pull/1286) Add the `allowed_email_domains` and the `allowed_groups` on the `auth_request` + support standard wildcard char for validation with sub-domain and email-domain. (@w3st3ry @armandpicard)
 - [#1361](https://github.com/oauth2-proxy/oauth2-proxy/pull/1541) PKCE Code Challenge Support - RFC-7636 (@braunsonm)
 - [#1594](https://github.com/oauth2-proxy/oauth2-proxy/pull/1594) Release ARMv8 docker images (@braunsonm)
+- [#1638](https://github.com/oauth2-proxy/oauth2-proxy/pull/1638) Implement configurable upstream timeout (@jacksgt)
 
 # V7.2.1
 

docs/docs/configuration/alpha_config.md

@@ -512,6 +512,7 @@ Requests will be proxied to this upstream if the path matches the request path.
 | `flushInterval` | _[Duration](#duration)_ | FlushInterval is the period between flushing the response buffer when<br/>streaming response from the upstream.<br/>Defaults to 1 second. |
 | `passHostHeader` | _bool_ | PassHostHeader determines whether the request host header should be proxied<br/>to the upstream server.<br/>Defaults to true. |
 | `proxyWebSockets` | _bool_ | ProxyWebSockets enables proxying of websockets to upstream servers<br/>Defaults to true. |
+| `timeout` | _[Duration](#duration)_ | Timeout is the maximum duration the server will wait for a response from the upstream server.<br/>Defaults to 30 seconds. |
 
 ### UpstreamConfig
 

docs/docs/configuration/overview.md

@@ -196,6 +196,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/
 | `--tls-key-file` | string | path to private key file | |
 | `--tls-min-version` | string | minimum TLS version that is acceptable, either `"TLS1.2"` or `"TLS1.3"` | `"TLS1.2"` |
 | `--upstream` | string \| list | the http url(s) of the upstream endpoint, file:// paths for static files or `static://<status_code>` for static response. Routing is based on the path | |
+| `--upstream-timeout` | duration | maximum amount of time the server will wait for a response from the upstream | 30s |
 | `--allowed-group` | string \| list | restrict logins to members of this group (may be given multiple times) | |
 | `--allowed-role` | string \| list | restrict logins to users with this role (may be given multiple times). Only works with the keycloak-oidc provider. | |
 | `--validate-url` | string | Access token validation endpoint | |

main_test.go

@@ -35,6 +35,7 @@ upstreamConfig:
     flushInterval: 1s
     passHostHeader: true
     proxyWebSockets: true
+    timeout: 30s
 injectRequestHeaders:
 - name: Authorization
   values:
@@ -118,6 +119,7 @@ redirect_url="http://localhost:4180/oauth2/callback"
 					FlushInterval:   durationPtr(options.DefaultUpstreamFlushInterval),
 					PassHostHeader:  boolPtr(true),
 					ProxyWebSockets: boolPtr(true),
+					Timeout:         durationPtr(options.DefaultUpstreamTimeout),
 				},
 			},
 		}

pkg/apis/options/legacy_options.go

@@ -33,6 +33,7 @@ func NewLegacyOptions() *LegacyOptions {
 			PassHostHeader:  true,
 			ProxyWebSockets: true,
 			FlushInterval:   DefaultUpstreamFlushInterval,
+			Timeout:         DefaultUpstreamTimeout,
 		},
 
 		LegacyHeaders: LegacyHeaders{
@@ -101,6 +102,7 @@ type LegacyUpstreams struct {
 	ProxyWebSockets               bool          `flag:"proxy-websockets" cfg:"proxy_websockets"`
 	SSLUpstreamInsecureSkipVerify bool          `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"`
 	Upstreams                     []string      `flag:"upstream" cfg:"upstreams"`
+	Timeout                       time.Duration `flag:"upstream-timeout" cfg:"upstream_timeout"`
 }
 
 func legacyUpstreamsFlagSet() *pflag.FlagSet {
@@ -111,6 +113,7 @@ func legacyUpstreamsFlagSet() *pflag.FlagSet {
 	flagSet.Bool("proxy-websockets", true, "enables WebSocket proxying")
 	flagSet.Bool("ssl-upstream-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS upstreams")
 	flagSet.StringSlice("upstream", []string{}, "the http url(s) of the upstream endpoint, file:// paths for static files or static://<status_code> for static response. Routing is based on the path")
+	flagSet.Duration("upstream-timeout", DefaultUpstreamTimeout, "maximum amount of time the server will wait for a response from the upstream")
 
 	return flagSet
 }
@@ -129,6 +132,7 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) {
 		}
 
 		flushInterval := Duration(l.FlushInterval)
+		timeout := Duration(l.Timeout)
 		upstream := Upstream{
 			ID:                    u.Path,
 			Path:                  u.Path,
@@ -137,6 +141,7 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) {
 			PassHostHeader:        &l.PassHostHeader,
 			ProxyWebSockets:       &l.ProxyWebSockets,
 			FlushInterval:         &flushInterval,
+			Timeout:               &timeout,
 		}
 
 		switch u.Scheme {
@@ -168,6 +173,7 @@ func (l *LegacyUpstreams) convert() (UpstreamConfig, error) {
 			upstream.PassHostHeader = nil
 			upstream.ProxyWebSockets = nil
 			upstream.FlushInterval = nil
+			upstream.Timeout = nil
 		}
 
 		upstreams.Upstreams = append(upstreams.Upstreams, upstream)

pkg/apis/options/legacy_options_test.go

@@ -17,7 +17,9 @@ var _ = Describe("Legacy Options", func() {
 
 			// Set upstreams and related options to test their conversion
 			flushInterval := Duration(5 * time.Second)
+			timeout := Duration(5 * time.Second)
 			legacyOpts.LegacyUpstreams.FlushInterval = time.Duration(flushInterval)
+			legacyOpts.LegacyUpstreams.Timeout = time.Duration(timeout)
 			legacyOpts.LegacyUpstreams.PassHostHeader = true
 			legacyOpts.LegacyUpstreams.ProxyWebSockets = true
 			legacyOpts.LegacyUpstreams.SSLUpstreamInsecureSkipVerify = true
@@ -36,6 +38,7 @@ var _ = Describe("Legacy Options", func() {
 						InsecureSkipTLSVerify: true,
 						PassHostHeader:        &truth,
 						ProxyWebSockets:       &truth,
+						Timeout:               &timeout,
 					},
 					{
 						ID:                    "/bar",
@@ -45,6 +48,7 @@ var _ = Describe("Legacy Options", func() {
 						InsecureSkipTLSVerify: true,
 						PassHostHeader:        &truth,
 						ProxyWebSockets:       &truth,
+						Timeout:               &timeout,
 					},
 					{
 						ID:                    "static://204",
@@ -56,6 +60,7 @@ var _ = Describe("Legacy Options", func() {
 						InsecureSkipTLSVerify: false,
 						PassHostHeader:        nil,
 						ProxyWebSockets:       nil,
+						Timeout:               nil,
 					},
 				},
 			}
@@ -140,6 +145,7 @@ var _ = Describe("Legacy Options", func() {
 		passHostHeader := false
 		proxyWebSockets := true
 		flushInterval := Duration(5 * time.Second)
+		timeout := Duration(5 * time.Second)
 
 		// Test cases and expected outcomes
 		validHTTP := "http://foo.bar/baz"
@@ -151,6 +157,7 @@ var _ = Describe("Legacy Options", func() {
 			PassHostHeader:        &passHostHeader,
 			ProxyWebSockets:       &proxyWebSockets,
 			FlushInterval:         &flushInterval,
+			Timeout:               &timeout,
 		}
 
 		// Test cases and expected outcomes
@@ -163,6 +170,7 @@ var _ = Describe("Legacy Options", func() {
 			PassHostHeader:        &passHostHeader,
 			ProxyWebSockets:       &proxyWebSockets,
 			FlushInterval:         &flushInterval,
+			Timeout:               &timeout,
 		}
 
 		validFileWithFragment := "file:///var/lib/website#/bar"
@@ -174,6 +182,7 @@ var _ = Describe("Legacy Options", func() {
 			PassHostHeader:        &passHostHeader,
 			ProxyWebSockets:       &proxyWebSockets,
 			FlushInterval:         &flushInterval,
+			Timeout:               &timeout,
 		}
 
 		validStatic := "static://204"
@@ -188,6 +197,7 @@ var _ = Describe("Legacy Options", func() {
 			PassHostHeader:        nil,
 			ProxyWebSockets:       nil,
 			FlushInterval:         nil,
+			Timeout:               nil,
 		}
 
 		invalidStatic := "static://abc"
@@ -202,6 +212,7 @@ var _ = Describe("Legacy Options", func() {
 			PassHostHeader:        nil,
 			ProxyWebSockets:       nil,
 			FlushInterval:         nil,
+			Timeout:               nil,
 		}
 
 		invalidHTTP := ":foo"
@@ -215,6 +226,7 @@ var _ = Describe("Legacy Options", func() {
 					PassHostHeader:                passHostHeader,
 					ProxyWebSockets:               proxyWebSockets,
 					FlushInterval:                 time.Duration(flushInterval),
+					Timeout:                       time.Duration(timeout),
 				}
 
 				upstreams, err := legacyUpstreams.convert()

pkg/apis/options/load_test.go

@@ -22,6 +22,7 @@ var _ = Describe("Load", func() {
 			PassHostHeader:  true,
 			ProxyWebSockets: true,
 			FlushInterval:   DefaultUpstreamFlushInterval,
+			Timeout:         DefaultUpstreamTimeout,
 		},
 
 		LegacyHeaders: LegacyHeaders{

pkg/apis/options/upstreams.go

@@ -5,6 +5,9 @@ import "time"
 const (
 	// DefaultUpstreamFlushInterval is the default value for the Upstream FlushInterval.
 	DefaultUpstreamFlushInterval = 1 * time.Second
+
+	// DefaultUpstreamTimeout is the maximum duration a network dial to a upstream server for a response.
+	DefaultUpstreamTimeout = 30 * time.Second
 )
 
 // UpstreamConfig is a collection of definitions for upstream servers.
@@ -84,4 +87,8 @@ type Upstream struct {
 	// ProxyWebSockets enables proxying of websockets to upstream servers
 	// Defaults to true.
 	ProxyWebSockets *bool `json:"proxyWebSockets,omitempty"`
+
+	// Timeout is the maximum duration the server will wait for a response from the upstream server.
+	// Defaults to 30 seconds.
+	Timeout *Duration `json:"timeout,omitempty"`
 }

pkg/upstream/http.go

@@ -1,7 +1,6 @@
 package upstream
 
 import (
-	"crypto/tls"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -100,6 +99,14 @@ func (h *httpUpstreamProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request)
 func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler ProxyErrorHandler) http.Handler {
 	proxy := httputil.NewSingleHostReverseProxy(target)
 
+	// Inherit default transport options from Go's stdlib
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+
+	// Change default duration for waiting for an upstream response
+	if upstream.Timeout != nil {
+		transport.ResponseHeaderTimeout = upstream.Timeout.Duration()
+	}
+
 	// Configure options on the SingleHostReverseProxy
 	if upstream.FlushInterval != nil {
 		proxy.FlushInterval = upstream.FlushInterval.Duration()
@@ -110,9 +117,7 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr
 	// InsecureSkipVerify is a configurable option we allow
 	/* #nosec G402 */
 	if upstream.InsecureSkipTLSVerify {
-		proxy.Transport = &http.Transport{
+		transport.TLSClientConfig.InsecureSkipVerify = true
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		}
 	}
 
 	// Ensure we always pass the original request path
@@ -127,6 +132,10 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr
 	if errorHandler != nil {
 		proxy.ErrorHandler = errorHandler
 	}
+
+	// Apply the customized transport to our proxy before returning it
+	proxy.Transport = transport
+
 	return proxy
 }
 
@@ -156,11 +165,17 @@ func setProxyDirector(proxy *httputil.ReverseProxy) {
 // newWebSocketReverseProxy creates a new reverse proxy for proxying websocket connections.
 func newWebSocketReverseProxy(u *url.URL, skipTLSVerify bool) http.Handler {
 	wsProxy := httputil.NewSingleHostReverseProxy(u)
+
+	// Inherit default transport options from Go's stdlib
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+
 	/* #nosec G402 */
 	if skipTLSVerify {
-		wsProxy.Transport = &http.Transport{
+		transport.TLSClientConfig.InsecureSkipVerify = true
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		}
 	}
+
+	// Apply the customized transport to our proxy before returning it
+	wsProxy.Transport = transport
+
 	return wsProxy
 }

pkg/upstream/http_test.go

@@ -3,7 +3,6 @@ package upstream
 import (
 	"bytes"
 	"crypto"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -23,9 +22,8 @@ import (
 )
 
 var _ = Describe("HTTP Upstream Suite", func() {
-
+	defaultFlushInterval := options.Duration(options.DefaultUpstreamFlushInterval)
-	const flushInterval5s = options.Duration(5 * time.Second)
+	defaultTimeout := options.Duration(options.DefaultUpstreamTimeout)
-	const flushInterval1s = options.Duration(1 * time.Second)
 	truth := true
 	falsum := false
 
@@ -62,12 +60,15 @@ var _ = Describe("HTTP Upstream Suite", func() {
 
 			flush := options.Duration(1 * time.Second)
 
+			timeout := options.Duration(options.DefaultUpstreamTimeout)
+
 			upstream := options.Upstream{
 				ID:                    in.id,
 				PassHostHeader:        &in.passUpstreamHostHeader,
 				ProxyWebSockets:       &falsum,
 				InsecureSkipTLSVerify: false,
 				FlushInterval:         &flush,
+				Timeout:               &timeout,
 			}
 
 			Expect(in.serverAddr).ToNot(BeNil())
@@ -318,13 +319,13 @@ var _ = Describe("HTTP Upstream Suite", func() {
 		req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
 		rw := httptest.NewRecorder()
 
-		flush := options.Duration(1 * time.Second)
 		upstream := options.Upstream{
 			ID:                    "noPassHost",
 			PassHostHeader:        &falsum,
 			ProxyWebSockets:       &falsum,
 			InsecureSkipTLSVerify: false,
-			FlushInterval:         &flush,
+			FlushInterval:         &defaultFlushInterval,
+			Timeout:               &defaultTimeout,
 		}
 
 		u, err := url.Parse(serverAddr)
@@ -354,6 +355,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
 		skipVerify      bool
 		sigData         *options.SignatureData
 		errorHandler    func(http.ResponseWriter, *http.Request, error)
+		timeout         options.Duration
 	}
 
 	DescribeTable("newHTTPUpstreamProxy",
@@ -366,6 +368,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
 				FlushInterval:         &in.flushInterval,
 				InsecureSkipTLSVerify: in.skipVerify,
 				ProxyWebSockets:       &in.proxyWebSockets,
+				Timeout:               &in.timeout,
 			}
 
 			handler := newHTTPUpstreamProxy(upstream, u, in.sigData, in.errorHandler)
@@ -380,49 +383,63 @@ var _ = Describe("HTTP Upstream Suite", func() {
 			proxy, ok := upstreamProxy.handler.(*httputil.ReverseProxy)
 			Expect(ok).To(BeTrue())
 			Expect(proxy.FlushInterval).To(Equal(in.flushInterval.Duration()))
+			transport, ok := proxy.Transport.(*http.Transport)
+			Expect(ok).To(BeTrue())
+			Expect(transport.ResponseHeaderTimeout).To(Equal(in.timeout.Duration()))
 			Expect(proxy.ErrorHandler != nil).To(Equal(in.errorHandler != nil))
 			if in.skipVerify {
-				Expect(proxy.Transport).To(Equal(&http.Transport{
+				Expect(transport.TLSClientConfig.InsecureSkipVerify).To(Equal(true))
-					TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-				}))
 			}
 		},
 		Entry("with proxy websockets", &newUpstreamTableInput{
 			proxyWebSockets: true,
-			flushInterval:   flushInterval1s,
+			flushInterval:   defaultFlushInterval,
 			skipVerify:      false,
 			sigData:         nil,
 			errorHandler:    nil,
+			timeout:         defaultTimeout,
 		}),
 		Entry("with a non standard flush interval", &newUpstreamTableInput{
 			proxyWebSockets: false,
-			flushInterval:   flushInterval5s,
+			flushInterval:   options.Duration(5 * time.Second),
 			skipVerify:      false,
 			sigData:         nil,
 			errorHandler:    nil,
+			timeout:         defaultTimeout,
 		}),
 		Entry("with a InsecureSkipTLSVerify", &newUpstreamTableInput{
 			proxyWebSockets: false,
-			flushInterval:   flushInterval1s,
+			flushInterval:   defaultFlushInterval,
 			skipVerify:      true,
 			sigData:         nil,
 			errorHandler:    nil,
+			timeout:         defaultTimeout,
 		}),
 		Entry("with a SignatureData", &newUpstreamTableInput{
 			proxyWebSockets: false,
-			flushInterval:   flushInterval1s,
+			flushInterval:   defaultFlushInterval,
 			skipVerify:      false,
 			sigData:         &options.SignatureData{Hash: crypto.SHA256, Key: "secret"},
 			errorHandler:    nil,
+			timeout:         defaultTimeout,
 		}),
 		Entry("with an error handler", &newUpstreamTableInput{
 			proxyWebSockets: false,
-			flushInterval:   flushInterval1s,
+			flushInterval:   defaultFlushInterval,
 			skipVerify:      false,
 			sigData:         nil,
 			errorHandler: func(rw http.ResponseWriter, req *http.Request, arg3 error) {
 				rw.WriteHeader(502)
 			},
+			timeout: defaultTimeout,
+		}),
+		Entry("with a non-default timeout", &newUpstreamTableInput{
+			proxyWebSockets: false,
+			flushInterval:   defaultFlushInterval,
+			skipVerify:      false,
+			sigData:         nil,
+			errorHandler:    nil,
+			timeout:         options.Duration(5 * time.Second),
 		}),
 	)
 
@@ -431,12 +448,14 @@ var _ = Describe("HTTP Upstream Suite", func() {
 
 		BeforeEach(func() {
 			flush := options.Duration(1 * time.Second)
+			timeout := options.Duration(options.DefaultUpstreamTimeout)
 			upstream := options.Upstream{
 				ID:                    "websocketProxy",
 				PassHostHeader:        &truth,
 				ProxyWebSockets:       &truth,
 				InsecureSkipTLSVerify: false,
 				FlushInterval:         &flush,
+				Timeout:               &timeout,
 			}
 
 			u, err := url.Parse(serverAddr)