diff --git a/images/router/haproxy/conf/haproxy-config.template b/images/router/haproxy/conf/haproxy-config.template index 797db3226..3c7e9e245 100644 --- a/images/router/haproxy/conf/haproxy-config.template +++ b/images/router/haproxy/conf/haproxy-config.template @@ -158,14 +158,10 @@ defaults timeout connect {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_CONNECT_TIMEOUT") "5s" }} timeout client {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_CLIENT_TIMEOUT") "30s" }} timeout client-fin {{ firstMatch $timeSpecPattern (env "ROUTER_CLIENT_FIN_TIMEOUT") "1s" }} - timeout server {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_SERVER_TIMEOUT") "30s" }} timeout server-fin {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_SERVER_FIN_TIMEOUT") "1s" }} timeout http-request {{ firstMatch $timeSpecPattern (env "ROUTER_SLOWLORIS_TIMEOUT") "10s" }} timeout http-keep-alive {{ firstMatch $timeSpecPattern (env "ROUTER_SLOWLORIS_HTTP_KEEPALIVE") "300s" }} - # Long timeout for WebSocket connections. - timeout tunnel {{ firstMatch $timeSpecPattern (env "ROUTER_DEFAULT_TUNNEL_TIMEOUT") "1h" }} - {{- if isTrue (env "ROUTER_ENABLE_COMPRESSION") }} compression algo gzip compression type {{ env "ROUTER_COMPRESSION_MIME" "text/html text/plain text/css" }} @@ -312,6 +308,9 @@ frontend public_ssl # traffic ########################################################################## backend be_sni + {{- with $value := maxTimeoutFirstMatchedAndClipped .State "haproxy.router.openshift.io/timeout" $timeSpecPattern (env "ROUTER_DEFAULT_SERVER_TIMEOUT") "30s" }} + timeout server {{ $value }} + {{- end }} server fe_sni unix@/var/lib/haproxy/run/haproxy-sni.sock weight 1 send-proxy frontend fe_sni @@ -425,6 +424,9 @@ frontend fe_sni ########################################################################## # backend for when sni does not exist, or ssl term needs to happen on the edge backend be_no_sni + {{- with $value := maxTimeoutFirstMatchedAndClipped .State "haproxy.router.openshift.io/timeout" $timeSpecPattern (env "ROUTER_DEFAULT_SERVER_TIMEOUT") "30s" }} + timeout server {{ $value }} + {{- end }} server fe_no_sni unix@/var/lib/haproxy/run/haproxy-no-sni.sock weight 1 send-proxy frontend fe_no_sni @@ -581,10 +583,10 @@ backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} {{- end }} tcp-request content reject if !whitelist {{- end }} - {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout")) }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout") (env "ROUTER_DEFAULT_SERVER_TIMEOUT") "30s") }} timeout server {{ $value }} {{- end }} - {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout-tunnel")) }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout-tunnel") (env "ROUTER_DEFAULT_TUNNEL_TIMEOUT") "1h") }} timeout tunnel {{ $value }} {{- end }} @@ -785,7 +787,10 @@ backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }} {{- end }} tcp-request content reject if !whitelist {{- end }} - {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout-tunnel") (index $cfg.Annotations "haproxy.router.openshift.io/timeout")) }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout") (env "ROUTER_DEFAULT_SERVER_TIMEOUT") "30s") }} + timeout server {{ $value }} + {{- end }} + {{- with $value := clipHAProxyTimeoutValue (firstMatch $timeSpecPattern (index $cfg.Annotations "haproxy.router.openshift.io/timeout-tunnel") (index $cfg.Annotations "haproxy.router.openshift.io/timeout") (env "ROUTER_DEFAULT_TUNNEL_TIMEOUT") "1h") }} timeout tunnel {{ $value }} {{- end }} diff --git a/pkg/router/template/template_helper.go b/pkg/router/template/template_helper.go index 7ce99c22c..7ede52d26 100644 --- a/pkg/router/template/template_helper.go +++ b/pkg/router/template/template_helper.go @@ -13,6 +13,7 @@ import ( "strings" "sync" "text/template" + "time" routev1 "github.com/openshift/api/route/v1" "github.com/openshift/router/pkg/router/routeapihelpers" @@ -386,6 +387,27 @@ func parseIPList(list string) string { return list } +// maxTimeoutFirstMatchedAndClipped finds the maximum timeout managed by a given annotation among all the routes, matches it against a given pattern, and clips. +func maxTimeoutFirstMatchedAndClipped(aliases map[ServiceAliasConfigKey]ServiceAliasConfig, annotation, pattern string, values ...string) string { + var ( + max string + maxDuration time.Duration + ) + for _, cfg := range aliases { + timeout := clipHAProxyTimeoutValue(firstMatch(pattern, append([]string{cfg.Annotations[annotation]}, values...)...)) + if timeout != "" { + // No error handling because clipHAProxyTimeoutValue returns + // a valid timeout or an empty string. The latter is already handled. + timeoutDuration, _ := time.ParseDuration(timeout) + if timeoutDuration > maxDuration { + max = timeout + maxDuration = timeoutDuration + } + } + } + return max +} + var helperFunctions = template.FuncMap{ "endpointsForAlias": endpointsForAlias, //returns the list of valid endpoints "processEndpointsForAlias": processEndpointsForAlias, //returns the list of valid endpoints after processing them @@ -409,6 +431,7 @@ var helperFunctions = template.FuncMap{ "validateHAProxyWhiteList": validateHAProxyWhiteList, //validates a haproxy whitelist (acl) content "generateHAProxyWhiteListFile": generateHAProxyWhiteListFile, //generates a haproxy whitelist file for use in an acl - "clipHAProxyTimeoutValue": clipHAProxyTimeoutValue, //clips extrodinarily high timeout values to be below the maximum allowed timeout value - "parseIPList": parseIPList, //parses the list of IPs/CIDRs (IPv4/IPv6) + "clipHAProxyTimeoutValue": clipHAProxyTimeoutValue, //clips extraordinarily high timeout values to be below the maximum allowed timeout value + "parseIPList": parseIPList, //parses the list of IPs/CIDRs (IPv4/IPv6) + "maxTimeoutFirstMatchedAndClipped": maxTimeoutFirstMatchedAndClipped, //finds the maximum timeout managed by a given annotation among all the routes, matches it against a given pattern, and clips } diff --git a/pkg/router/template/template_helper_test.go b/pkg/router/template/template_helper_test.go index 03b1177bc..300ee3eb9 100644 --- a/pkg/router/template/template_helper_test.go +++ b/pkg/router/template/template_helper_test.go @@ -1067,3 +1067,105 @@ func TestParseIPList(t *testing.T) { }) } } + +func TestMaxTimeoutFirstMatchedAndClipped(t *testing.T) { + testCases := []struct { + name string + state map[ServiceAliasConfigKey]ServiceAliasConfig + annotation string + pattern string + values []string + expected string + }{ + { + name: "Route timeout is maximum", + state: map[ServiceAliasConfigKey]ServiceAliasConfig{ + "test:route1": { + Annotations: map[string]string{ + "haproxy.router.openshift.io/timeout": "5m", + }, + }, + "test:route2": { + Annotations: map[string]string{ + "haproxy.router.openshift.io/timeout": "35s", + }, + }, + "test:route3": { + Annotations: map[string]string{ + "haproxy.router.openshift.io/timeout-tunnel": "10m", + }, + }, + "test:route4": {}, + }, + annotation: "haproxy.router.openshift.io/timeout", + pattern: `[1-9][0-9]*(us|ms|s|m|h|d)?`, + values: []string{"30s"}, + expected: "5m", + }, + { + name: "Default timeout is maximum", + state: map[ServiceAliasConfigKey]ServiceAliasConfig{ + "test:route1": {}, + "test:route2": {}, + "test:route3": {}, + }, + annotation: "haproxy.router.openshift.io/timeout", + pattern: `[1-9][0-9]*(us|ms|s|m|h|d)?`, + values: []string{"30s"}, + expected: "30s", + }, + { + name: "Maximum default timeout is choosen", + state: map[ServiceAliasConfigKey]ServiceAliasConfig{ + "test:route1": {}, + "test:route2": {}, + "test:route3": {}, + }, + annotation: "haproxy.router.openshift.io/timeout", + pattern: `[1-9][0-9]*(us|ms|s|m|h|d)?`, + values: []string{"40s", "30s"}, + expected: "40s", + }, + { + name: "Route timeout doesn't match", + state: map[ServiceAliasConfigKey]ServiceAliasConfig{ + "test:route1": { + Annotations: map[string]string{ + "haproxy.router.openshift.io/timeout": "5minutes", + }, + }, + "test:route2": { + Annotations: map[string]string{ + "haproxy.router.openshift.io/timeout": "35s", + }, + }, + }, + annotation: "haproxy.router.openshift.io/timeout", + pattern: `[1-9][0-9]*(us|ms|s|m|h|d)?`, + values: []string{"30s"}, + expected: "35s", + }, + { + name: "Route timeout clipped", + state: map[ServiceAliasConfigKey]ServiceAliasConfig{ + "test:route1": { + Annotations: map[string]string{ + "haproxy.router.openshift.io/timeout": "999999999s", + }, + }, + }, + annotation: "haproxy.router.openshift.io/timeout", + pattern: `[1-9][0-9]*(us|ms|s|m|h|d)?`, + values: []string{"30s"}, + expected: "2147483647ms", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := maxTimeoutFirstMatchedAndClipped(tc.state, tc.annotation, tc.pattern, tc.values...) + if got != tc.expected { + t.Errorf("Failure: expected %q, got %q", tc.expected, got) + } + }) + } +}