Skip to content

Commit

Permalink
OCPBUGS-14914: Set tunnel and server timeouts only at the backend level
Browse files Browse the repository at this point in the history 
The middle backends (be_sni, be_no_sni) are updated with the server timeout set to the maximum value among all routes from the configuration. This prevents a warning message during config reload.
  • Loading branch information
@alebedev87
alebedev87 committed Jan 29, 2024
1 parent fb70ac4 commit e165421
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 9 deletions.
19 changes: 12 additions & 7 deletions images/router/haproxy/conf/haproxy-config.template
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" }}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }}

Expand Down Expand Up @@ -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 }}

Expand Down
27 changes: 25 additions & 2 deletions pkg/router/template/template_helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"sync"
"text/template"
"time"

routev1 "github.com/openshift/api/route/v1"
"github.com/openshift/router/pkg/router/routeapihelpers"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
102 changes: 102 additions & 0 deletions pkg/router/template/template_helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}

0 comments on commit e165421

Please sign in to comment.