Merge pull request oauth2-proxy#1638 from jacksgt/configure-upstream-…

…timeout Configure upstream timeout
atiasn · May 20, 2022 · 086b869 · 086b869
2 parents 27f4bb6 + 7a27cb0
commit 086b869
Show file tree

Hide file tree

Showing 10 changed files with 86 additions and 21 deletions.
diff --git a/CHANGELOG.md b/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
 

diff --git a/docs/docs/configuration/alpha_config.md b/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
 

diff --git a/docs/docs/configuration/overview.md b/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 | |

diff --git a/main_test.go b/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),
 				},
 			},
 		}

diff --git a/pkg/apis/options/legacy_options.go b/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)

diff --git a/pkg/apis/options/legacy_options_test.go b/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()

diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go
@@ -22,6 +22,7 @@ var _ = Describe("Load", func() {
 			PassHostHeader:  true,
 			ProxyWebSockets: true,
 			FlushInterval:   DefaultUpstreamFlushInterval,
+			Timeout:         DefaultUpstreamTimeout,
 		},
 
 		LegacyHeaders: LegacyHeaders{

diff --git a/pkg/apis/options/upstreams.go b/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"`
 }
diff --git a/pkg/upstream/http.go b/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{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		}
+		transport.TLSClientConfig.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{
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-		}
+		transport.TLSClientConfig.InsecureSkipVerify = true
 	}
+
+	// Apply the customized transport to our proxy before returning it
+	wsProxy.Transport = transport
+
 	return wsProxy
 }