diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 027db9407..8958ccfee 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -31,6 +31,9 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string| |[nginx.ingress.kubernetes.io/auth-cache-key](#external-authentication)|string| |[nginx.ingress.kubernetes.io/auth-cache-duration](#external-authentication)|string| +|[nginx.ingress.kubernetes.io/auth-keepalive](#external-authentication)|number| +|[nginx.ingress.kubernetes.io/auth-keepalive-requests](#external-authentication)|number| +|[nginx.ingress.kubernetes.io/auth-keepalive-timeout](#external-authentication)|number| |[nginx.ingress.kubernetes.io/auth-proxy-set-headers](#external-authentication)|string| |[nginx.ingress.kubernetes.io/auth-snippet](#external-authentication)|string| |[nginx.ingress.kubernetes.io/enable-global-auth](#external-authentication)|"true" or "false"| @@ -453,6 +456,19 @@ nginx.ingress.kubernetes.io/auth-url: "URL to the authentication service" Additionally it is possible to set: +* `nginx.ingress.kubernetes.io/auth-keepalive`: + `` to specify the maximum number of keepalive connections to `auth-url`. Only takes effect + when no variables are used in the host part of the URL. Defaults to `0` (keepalive disabled). + +> Note: does not work with HTTP/2 listener because of a limitation in Lua [subrequests](https://github.com/openresty/lua-nginx-module#spdy-mode-not-fully-supported). +> [UseHTTP2](./configmap.md#use-http2) configuration should be disabled! + +* `nginx.ingress.kubernetes.io/auth-keepalive-requests`: + `` to specify the maximum number of requests that can be served through one keepalive connection. + Defaults to `1000` and only applied if `auth-keepalive` is set to higher than `0`. +* `nginx.ingress.kubernetes.io/auth-keepalive-timeout`: + `` to specify a duration in seconds which an idle keepalive connection to an upstream server will stay open. + Defaults to `60` and only applied if `auth-keepalive` is set to higher than `0`. * `nginx.ingress.kubernetes.io/auth-method`: `` to specify the HTTP method to use. * `nginx.ingress.kubernetes.io/auth-signin`: diff --git a/internal/ingress/annotations/authreq/main.go b/internal/ingress/annotations/authreq/main.go index 36af0e208..5dd1126fa 100644 --- a/internal/ingress/annotations/authreq/main.go +++ b/internal/ingress/annotations/authreq/main.go @@ -44,12 +44,22 @@ type Config struct { AuthSnippet string `json:"authSnippet"` AuthCacheKey string `json:"authCacheKey"` AuthCacheDuration []string `json:"authCacheDuration"` + KeepaliveConnections int `json:"keepaliveConnections"` + KeepaliveRequests int `json:"keepaliveRequests"` + KeepaliveTimeout int `json:"keepaliveTimeout"` ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"` } // DefaultCacheDuration is the fallback value if no cache duration is provided const DefaultCacheDuration = "200 202 401 5m" +// fallback values when no keepalive parameters are set +const ( + defaultKeepaliveConnections = 0 + defaultKeepaliveRequests = 1000 + defaultKeepaliveTimeout = 60 +) + // Equal tests for equality between two Config types func (e1 *Config) Equal(e2 *Config) bool { if e1 == e2 { @@ -90,6 +100,18 @@ func (e1 *Config) Equal(e2 *Config) bool { return false } + if e1.KeepaliveConnections != e2.KeepaliveConnections { + return false + } + + if e1.KeepaliveRequests != e2.KeepaliveRequests { + return false + } + + if e1.KeepaliveTimeout != e2.KeepaliveTimeout { + return false + } + return sets.StringElementsMatch(e1.AuthCacheDuration, e2.AuthCacheDuration) } @@ -193,6 +215,43 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) { klog.V(3).InfoS("auth-cache-key annotation is undefined and will not be set") } + keepaliveConnections, err := parser.GetIntAnnotation("auth-keepalive", ing) + if err != nil { + klog.V(3).InfoS("auth-keepalive annotation is undefined and will be set to its default value") + keepaliveConnections = defaultKeepaliveConnections + } + switch { + case keepaliveConnections < 0: + klog.Warningf("auth-keepalive annotation (%s) contains a negative value, setting auth-keepalive to 0", authURL.Host) + keepaliveConnections = 0 + case keepaliveConnections > 0: + // NOTE: upstream block cannot reference a variable in the server directive + if strings.IndexByte(authURL.Host, '$') != -1 { + klog.Warningf("auth-url annotation (%s) contains $ in the host:port part, setting auth-keepalive to 0", authURL.Host) + keepaliveConnections = 0 + } + } + + keepaliveRequests, err := parser.GetIntAnnotation("auth-keepalive-requests", ing) + if err != nil { + klog.V(3).InfoS("auth-keepalive-requests annotation is undefined and will be set to its default value") + keepaliveRequests = defaultKeepaliveRequests + } + if keepaliveRequests <= 0 { + klog.Warningf("auth-keepalive-requests annotation (%s) should be greater than zero, setting auth-keepalive to 0", authURL.Host) + keepaliveConnections = 0 + } + + keepaliveTimeout, err := parser.GetIntAnnotation("auth-keepalive-timeout", ing) + if err != nil { + klog.V(3).InfoS("auth-keepalive-timeout annotation is undefined and will be set to its default value") + keepaliveTimeout = defaultKeepaliveTimeout + } + if keepaliveTimeout <= 0 { + klog.Warningf("auth-keepalive-timeout annotation (%s) should be greater than zero, setting auth-keepalive 0", authURL.Host) + keepaliveConnections = 0 + } + durstr, _ := parser.GetStringAnnotation("auth-cache-duration", ing) authCacheDuration, err := ParseStringToCacheDurations(durstr) if err != nil { @@ -249,6 +308,9 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) { AuthSnippet: authSnippet, AuthCacheKey: authCacheKey, AuthCacheDuration: authCacheDuration, + KeepaliveConnections: keepaliveConnections, + KeepaliveRequests: keepaliveRequests, + KeepaliveTimeout: keepaliveTimeout, ProxySetHeaders: proxySetHeaders, }, nil } diff --git a/internal/ingress/annotations/authreq/main_test.go b/internal/ingress/annotations/authreq/main_test.go index da903fe30..eac1870e5 100644 --- a/internal/ingress/annotations/authreq/main_test.go +++ b/internal/ingress/annotations/authreq/main_test.go @@ -243,6 +243,71 @@ func TestCacheDurationAnnotations(t *testing.T) { } } +func TestKeepaliveAnnotations(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + ing.SetAnnotations(data) + + tests := []struct { + title string + url string + keepaliveConnections string + keepaliveRequests string + keepaliveTimeout string + expectedConnections int + expectedRequests int + expectedTimeout int + }{ + {"all set", "http://goog.url", "5", "500", "50", 5, 500, 50}, + {"no annotation", "http://goog.url", "", "", "", defaultKeepaliveConnections, defaultKeepaliveRequests, defaultKeepaliveTimeout}, + {"default for connections", "http://goog.url", "x", "500", "50", defaultKeepaliveConnections, 500, 50}, + {"default for requests", "http://goog.url", "5", "x", "50", 5, defaultKeepaliveRequests, 50}, + {"default for invalid timeout", "http://goog.url", "5", "500", "x", 5, 500, defaultKeepaliveTimeout}, + {"variable in host", "http://$host:5000/a/b", "5", "", "", 0, defaultKeepaliveRequests, defaultKeepaliveTimeout}, + {"variable in path", "http://goog.url:5000/$path", "5", "", "", 5, defaultKeepaliveRequests, defaultKeepaliveTimeout}, + {"negative connections", "http://goog.url", "-2", "", "", 0, defaultKeepaliveRequests, defaultKeepaliveTimeout}, + {"negative requests", "http://goog.url", "5", "-1", "", 0, -1, defaultKeepaliveTimeout}, + {"negative timeout", "http://goog.url", "5", "", "-1", 0, defaultKeepaliveRequests, -1}, + {"negative request and timeout", "http://goog.url", "5", "-2", "-3", 0, -2, -3}, + } + + for _, test := range tests { + data[parser.GetAnnotationWithPrefix("auth-url")] = test.url + data[parser.GetAnnotationWithPrefix("auth-keepalive")] = test.keepaliveConnections + data[parser.GetAnnotationWithPrefix("auth-keepalive-timeout")] = test.keepaliveTimeout + data[parser.GetAnnotationWithPrefix("auth-keepalive-requests")] = test.keepaliveRequests + + i, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("%v: unexpected error: %v", test.title, err) + continue + } + + u, ok := i.(*Config) + if !ok { + t.Errorf("%v: expected an External type", test.title) + continue + } + + if u.URL != test.url { + t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.url, u.URL) + } + + if u.KeepaliveConnections != test.expectedConnections { + t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.expectedConnections, u.KeepaliveConnections) + } + + if u.KeepaliveRequests != test.expectedRequests { + t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.expectedRequests, u.KeepaliveRequests) + } + + if u.KeepaliveTimeout != test.expectedTimeout { + t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.expectedTimeout, u.KeepaliveTimeout) + } + } +} + func TestParseStringToCacheDurations(t *testing.T) { tests := []struct { diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 9b61d059a..f24aea0c4 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -42,6 +42,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/influxdb" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/annotations/ratelimit" "k8s.io/ingress-nginx/internal/ingress/controller/config" ing_net "k8s.io/ingress-nginx/internal/net" @@ -227,7 +228,12 @@ var ( "buildAuthLocation": buildAuthLocation, "shouldApplyGlobalAuth": shouldApplyGlobalAuth, "buildAuthResponseHeaders": buildAuthResponseHeaders, + "buildAuthUpstreamLuaHeaders": buildAuthUpstreamLuaHeaders, "buildAuthProxySetHeaders": buildAuthProxySetHeaders, + "buildAuthUpstreamName": buildAuthUpstreamName, + "shouldApplyAuthUpstream": shouldApplyAuthUpstream, + "extractHostPort": extractHostPort, + "changeHostPort": changeHostPort, "buildProxyPass": buildProxyPass, "filterRateLimits": filterRateLimits, "buildRateLimitZones": buildRateLimitZones, @@ -572,7 +578,14 @@ func shouldApplyGlobalAuth(input interface{}, globalExternalAuthURL string) bool return false } -func buildAuthResponseHeaders(proxySetHeader string, headers []string) []string { +// buildAuthResponseHeaders sets HTTP response headers when `auth-url` is used. +// Based on `auth-keepalive` value we use auth_request_set Nginx directives, or +// we use Lua and Nginx variables instead. +// +// NOTE: Unfortunately auth_request module ignores the keepalive directive (see: +// https://trac.nginx.org/nginx/ticket/1579), that is why we mimic the same +// functionality with access_by_lua_block. +func buildAuthResponseHeaders(proxySetHeader string, headers []string, lua bool) []string { res := []string{} if len(headers) == 0 { @@ -580,14 +593,31 @@ func buildAuthResponseHeaders(proxySetHeader string, headers []string) []string } for i, h := range headers { - hvar := strings.ToLower(h) - hvar = strings.NewReplacer("-", "_").Replace(hvar) - res = append(res, fmt.Sprintf("auth_request_set $authHeader%v $upstream_http_%v;", i, hvar)) + if lua { + res = append(res, fmt.Sprintf("set $authHeader%d '';", i)) + } else { + hvar := strings.ToLower(h) + hvar = strings.NewReplacer("-", "_").Replace(hvar) + res = append(res, fmt.Sprintf("auth_request_set $authHeader%v $upstream_http_%v;", i, hvar)) + } res = append(res, fmt.Sprintf("%s '%v' $authHeader%v;", proxySetHeader, h, i)) } return res } +func buildAuthUpstreamLuaHeaders(headers []string) []string { + res := []string{} + + if len(headers) == 0 { + return res + } + + for i, h := range headers { + res = append(res, fmt.Sprintf("ngx.var.authHeader%d = res.header['%s']", i, h)) + } + return res +} + func buildAuthProxySetHeaders(headers map[string]string) []string { res := []string{} @@ -602,6 +632,76 @@ func buildAuthProxySetHeaders(headers map[string]string) []string { return res } +func buildAuthUpstreamName(input interface{}, host string) string { + authPath := buildAuthLocation(input, "") + if authPath == "" || host == "" { + return "" + } + + return fmt.Sprintf("%s-%s", host, authPath[2:]) +} + +// shouldApplyAuthUpstream returns true only in case when ExternalAuth.URL and +// ExternalAuth.KeepaliveConnections are all set +func shouldApplyAuthUpstream(l interface{}, c interface{}) bool { + location, ok := l.(*ingress.Location) + if !ok { + klog.Errorf("expected an '*ingress.Location' type but %T was returned", l) + return false + } + + cfg, ok := c.(config.Configuration) + if !ok { + klog.Errorf("expected a 'config.Configuration' type but %T was returned", c) + return false + } + + if location.ExternalAuth.URL == "" || location.ExternalAuth.KeepaliveConnections == 0 { + return false + } + + // Unfortunately, `auth_request` module ignores keepalive in upstream block: https://trac.nginx.org/nginx/ticket/1579 + // The workaround is to use `ngx.location.capture` Lua subrequests but it is not supported with HTTP/2 + if cfg.UseHTTP2 { + klog.Warning("Upstream keepalive is not supported with HTTP/2") + return false + } + + return true +} + +// extractHostPort will extract the host:port part from the URL specified by url +func extractHostPort(url string) string { + if url == "" { + return "" + } + + authURL, err := parser.StringToURL(url) + if err != nil { + klog.Errorf("expected a valid URL but %s was returned", url) + return "" + } + + return authURL.Host +} + +// changeHostPort will change the host:port part of the url to value +func changeHostPort(url string, value string) string { + if url == "" { + return "" + } + + authURL, err := parser.StringToURL(url) + if err != nil { + klog.Errorf("expected a valid URL but %s was returned", url) + return "" + } + + authURL.Host = value + + return authURL.String() +} + // buildProxyPass produces the proxy pass string, if the ingress has redirects // (specified through the nginx.ingress.kubernetes.io/rewrite-target annotation) // If the annotation nginx.ingress.kubernetes.io/add-base-url:"true" is specified it will diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index b65e33c32..a741919ed 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -502,14 +502,49 @@ func TestShouldApplyGlobalAuth(t *testing.T) { func TestBuildAuthResponseHeaders(t *testing.T) { externalAuthResponseHeaders := []string{"h1", "H-With-Caps-And-Dashes"} - expected := []string{ - "auth_request_set $authHeader0 $upstream_http_h1;", - "proxy_set_header 'h1' $authHeader0;", - "auth_request_set $authHeader1 $upstream_http_h_with_caps_and_dashes;", - "proxy_set_header 'H-With-Caps-And-Dashes' $authHeader1;", + tests := []struct { + headers []string + expected []string + lua bool + }{ + { + headers: externalAuthResponseHeaders, + lua: false, + expected: []string{ + "auth_request_set $authHeader0 $upstream_http_h1;", + "proxy_set_header 'h1' $authHeader0;", + "auth_request_set $authHeader1 $upstream_http_h_with_caps_and_dashes;", + "proxy_set_header 'H-With-Caps-And-Dashes' $authHeader1;", + }, + }, + { + headers: externalAuthResponseHeaders, + lua: true, + expected: []string{ + "set $authHeader0 '';", + "proxy_set_header 'h1' $authHeader0;", + "set $authHeader1 '';", + "proxy_set_header 'H-With-Caps-And-Dashes' $authHeader1;", + }, + }, } - headers := buildAuthResponseHeaders(proxySetHeader(nil), externalAuthResponseHeaders) + for _, test := range tests { + got := buildAuthResponseHeaders(proxySetHeader(nil), test.headers, test.lua) + if !reflect.DeepEqual(test.expected, got) { + t.Errorf("Expected \n'%v'\nbut returned \n'%v'", test.expected, got) + } + } +} + +func TestBuildAuthResponseLua(t *testing.T) { + externalAuthResponseHeaders := []string{"h1", "H-With-Caps-And-Dashes"} + expected := []string{ + "ngx.var.authHeader0 = res.header['h1']", + "ngx.var.authHeader1 = res.header['H-With-Caps-And-Dashes']", + } + + headers := buildAuthUpstreamLuaHeaders(externalAuthResponseHeaders) if !reflect.DeepEqual(expected, headers) { t.Errorf("Expected \n'%v'\nbut returned \n'%v'", expected, headers) @@ -533,6 +568,139 @@ func TestBuildAuthProxySetHeaders(t *testing.T) { } } +func TestBuildAuthUpstreamName(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildAuthUpstreamName(invalidType, "") + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + loc := &ingress.Location{ + ExternalAuth: authreq.Config{ + URL: "foo.com/auth", + }, + Path: "/cat", + } + + encodedAuthURL := strings.Replace(base64.URLEncoding.EncodeToString([]byte(loc.Path)), "=", "", -1) + externalAuthPath := fmt.Sprintf("external-auth-%v-default", encodedAuthURL) + + testCases := []struct { + title string + host string + expected string + }{ + {"valid host", "auth.my.site", fmt.Sprintf("%s-%s", "auth.my.site", externalAuthPath)}, + {"valid host", "your.auth.site", fmt.Sprintf("%s-%s", "your.auth.site", externalAuthPath)}, + {"empty host", "", ""}, + } + + for _, testCase := range testCases { + str := buildAuthUpstreamName(loc, testCase.host) + if str != testCase.expected { + t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, str) + } + } +} + +func TestShouldApplyAuthUpstream(t *testing.T) { + authURL := "foo.com/auth" + + loc := &ingress.Location{ + ExternalAuth: authreq.Config{ + URL: authURL, + KeepaliveConnections: 0, + }, + Path: "/cat", + } + + cfg := config.Configuration{ + UseHTTP2: false, + } + + testCases := []struct { + title string + authURL string + keepaliveConnections int + expected bool + }{ + {"authURL, no keepalive", authURL, 0, false}, + {"authURL, keepalive", authURL, 10, true}, + {"empty, no keepalive", "", 0, false}, + {"empty, keepalive", "", 10, false}, + } + + for _, testCase := range testCases { + loc.ExternalAuth.URL = testCase.authURL + loc.ExternalAuth.KeepaliveConnections = testCase.keepaliveConnections + + result := shouldApplyAuthUpstream(loc, cfg) + if result != testCase.expected { + t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, result) + } + } + + // keepalive is not supported with UseHTTP2 + cfg.UseHTTP2 = true + for _, testCase := range testCases { + loc.ExternalAuth.URL = testCase.authURL + loc.ExternalAuth.KeepaliveConnections = testCase.keepaliveConnections + + result := shouldApplyAuthUpstream(loc, cfg) + if result != false { + t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, false, result) + } + } +} + +func TestExtractHostPort(t *testing.T) { + testCases := []struct { + title string + url string + expected string + }{ + {"full URL", "https://my.auth.site:5000/path", "my.auth.site:5000"}, + {"URL with no port", "http://my.auth.site/path", "my.auth.site"}, + {"URL with no path", "https://my.auth.site:5000", "my.auth.site:5000"}, + {"URL no port and path", "http://my.auth.site", "my.auth.site"}, + {"missing method", "my.auth.site/path", ""}, + {"all empty", "", ""}, + } + + for _, testCase := range testCases { + result := extractHostPort(testCase.url) + if result != testCase.expected { + t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, result) + } + } +} + +func TestChangeHostPort(t *testing.T) { + testCases := []struct { + title string + url string + host string + expected string + }{ + {"full URL", "https://my.auth.site:5000/path", "your.domain", "https://your.domain/path"}, + {"URL with no port", "http://my.auth.site/path", "your.domain", "http://your.domain/path"}, + {"URL with no path", "http://my.auth.site:5000", "your.domain:8888", "http://your.domain:8888"}, + {"URL with no port and path", "https://my.auth.site", "your.domain:8888", "https://your.domain:8888"}, + {"invalid host", "my.auth.site/path", "", ""}, + {"missing method", "my.auth.site/path", "your.domain", ""}, + {"all empty", "", "", ""}, + } + + for _, testCase := range testCases { + result := changeHostPort(testCase.url, testCase.host) + if result != testCase.expected { + t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, result) + } + } +} + func TestTemplateWithData(t *testing.T) { pwd, _ := os.Getwd() f, err := os.Open(path.Join(pwd, "../../../../test/data/config.json")) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 2ee76831c..f6495eb4a 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -610,7 +610,25 @@ http { {{ end }} {{ range $server := $servers }} + {{ range $location := $server.Locations }} + {{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL }} + {{ $applyAuthUpstream := shouldApplyAuthUpstream $location $all.Cfg }} + {{ if and (eq $applyAuthUpstream true) (eq $applyGlobalAuth false) }} + ## start auth upstream {{ $server.Hostname }}{{ $location.Path }} + upstream {{ buildAuthUpstreamName $location $server.Hostname }} { + {{- $externalAuth := $location.ExternalAuth }} + server {{ extractHostPort $externalAuth.URL }}; + keepalive {{ $externalAuth.KeepaliveConnections }}; + keepalive_requests {{ $externalAuth.KeepaliveRequests }}; + keepalive_timeout {{ $externalAuth.KeepaliveTimeout }}s; + } + ## end auth upstream {{ $server.Hostname }}{{ $location.Path }} + {{ end }} + {{ end }} + {{ end }} + + {{ range $server := $servers }} ## start server {{ $server.Hostname }} server { server_name {{ buildServerName $server.Hostname }} {{range $server.Aliases }}{{ . }} {{ end }}; @@ -995,6 +1013,7 @@ stream { {{ $proxySetHeader := proxySetHeader $location }} {{ $authPath := buildAuthLocation $location $all.Cfg.GlobalExternalAuth.URL }} {{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL }} + {{ $applyAuthUpstream := shouldApplyAuthUpstream $location $all.Cfg }} {{ $externalAuth := $location.ExternalAuth }} {{ if eq $applyGlobalAuth true }} @@ -1074,7 +1093,6 @@ stream { proxy_buffer_size {{ $location.Proxy.BufferSize }}; proxy_buffers {{ $location.Proxy.BuffersNumber }} {{ $location.Proxy.BufferSize }}; proxy_request_buffering {{ $location.Proxy.RequestBuffering }}; - proxy_http_version {{ $location.Proxy.ProxyHTTPVersion }}; proxy_ssl_server_name on; proxy_pass_request_headers on; @@ -1103,7 +1121,19 @@ stream { {{ $externalAuth.AuthSnippet }} {{ end }} + {{ if and (eq $applyAuthUpstream true) (eq $applyGlobalAuth false) }} + {{ $authUpstreamName := buildAuthUpstreamName $location $server.Hostname }} + # The target is an upstream with HTTP keepalive, that is why the + # Connection header is cleared and the HTTP version is set to 1.1 as + # the Nginx documentation suggests: + # http://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive + proxy_http_version 1.1; + proxy_set_header Connection ""; + set $target {{ changeHostPort $externalAuth.URL $authUpstreamName }}; + {{ else }} + proxy_http_version {{ $location.Proxy.ProxyHTTPVersion }}; set $target {{ $externalAuth.URL }}; + {{ end }} proxy_pass $target; } {{ end }} @@ -1208,13 +1238,37 @@ stream { {{ if not (isLocationInLocationList $location $all.Cfg.NoAuthLocations) }} {{ if $authPath }} # this location requires authentication + {{ if and (eq $applyAuthUpstream true) (eq $applyGlobalAuth false) }} + set $auth_cookie ''; + add_header Set-Cookie $auth_cookie; + {{- range $line := buildAuthResponseHeaders $proxySetHeader $externalAuth.ResponseHeaders true }} + {{ $line }} + {{- end }} + # `auth_request` module does not support HTTP keepalives in upstream block: + # https://trac.nginx.org/nginx/ticket/1579 + access_by_lua_block { + local res = ngx.location.capture('{{ $authPath }}', { method = ngx.HTTP_GET, body = '' }) + if res.status == ngx.HTTP_OK then + ngx.var.auth_cookie = res.header['Set-Cookie'] + {{- range $line := buildAuthUpstreamLuaHeaders $externalAuth.ResponseHeaders }} + {{ $line }} + {{- end }} + return + end + if res.status == ngx.HTTP_FORBIDDEN then + ngx.exit(res.status) + end + ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR) + } + {{ else }} auth_request {{ $authPath }}; auth_request_set $auth_cookie $upstream_http_set_cookie; add_header Set-Cookie $auth_cookie; - {{- range $line := buildAuthResponseHeaders $proxySetHeader $externalAuth.ResponseHeaders }} + {{- range $line := buildAuthResponseHeaders $proxySetHeader $externalAuth.ResponseHeaders false }} {{ $line }} {{- end }} {{ end }} + {{ end }} {{ if $externalAuth.SigninURL }} set_escape_uri $escaped_request_uri $request_uri; diff --git a/test/e2e/annotations/auth.go b/test/e2e/annotations/auth.go index 7ca2fff45..0ea97d3ef 100644 --- a/test/e2e/annotations/auth.go +++ b/test/e2e/annotations/auth.go @@ -476,6 +476,99 @@ http { Body(). NotContainsFold(fmt.Sprintf("%s=%s", rewriteHeader, rewriteVal)) }) + + ginkgo.It(`should not create additional upstream block when auth-keepalive is not set`, func() { + f.UpdateNginxConfigMapData("use-http2", "false") + defer func() { + f.UpdateNginxConfigMapData("use-http2", "true") + }() + // Sleep a while just to guarantee that the configmap is applied + framework.Sleep() + + annotations["nginx.ingress.kubernetes.io/auth-url"] = "http://foo.bar.baz:5000/path" + f.UpdateIngress(ing) + + f.WaitForNginxServer("", + func(server string) bool { + return strings.Contains(server, "http://foo.bar.baz:5000/path") && + !strings.Contains(server, `upstream auth-external-auth`) + }) + }) + + ginkgo.It(`should not create additional upstream block when host part of auth-url contains a variable`, func() { + f.UpdateNginxConfigMapData("use-http2", "false") + defer func() { + f.UpdateNginxConfigMapData("use-http2", "true") + }() + // Sleep a while just to guarantee that the configmap is applied + framework.Sleep() + + annotations["nginx.ingress.kubernetes.io/auth-url"] = "http://$host/path" + annotations["nginx.ingress.kubernetes.io/auth-keepalive"] = "123" + f.UpdateIngress(ing) + + f.WaitForNginxServer("", + func(server string) bool { + return strings.Contains(server, "http://$host/path") && + !strings.Contains(server, `upstream auth-external-auth`) && + !strings.Contains(server, `keepalive 123;`) + }) + }) + + ginkgo.It(`should not create additional upstream block when auth-keepalive is negative`, func() { + f.UpdateNginxConfigMapData("use-http2", "false") + defer func() { + f.UpdateNginxConfigMapData("use-http2", "true") + }() + // Sleep a while just to guarantee that the configmap is applied + framework.Sleep() + + annotations["nginx.ingress.kubernetes.io/auth-url"] = "http://foo.bar.baz:5000/path" + annotations["nginx.ingress.kubernetes.io/auth-keepalive"] = "-1" + f.UpdateIngress(ing) + + f.WaitForNginxServer("", + func(server string) bool { + return strings.Contains(server, "http://foo.bar.baz:5000/path") && + !strings.Contains(server, `upstream auth-external-auth`) + }) + }) + + ginkgo.It(`should not create additional upstream block when auth-keepalive is set with HTTP/2`, func() { + annotations["nginx.ingress.kubernetes.io/auth-url"] = "http://foo.bar.baz:5000/path" + annotations["nginx.ingress.kubernetes.io/auth-keepalive"] = "123" + annotations["nginx.ingress.kubernetes.io/auth-keepalive-requests"] = "456" + annotations["nginx.ingress.kubernetes.io/auth-keepalive-timeout"] = "789" + f.UpdateIngress(ing) + + f.WaitForNginxServer("", + func(server string) bool { + return strings.Contains(server, "http://foo.bar.baz:5000/path") && + !strings.Contains(server, `upstream auth-external-auth`) + }) + }) + + ginkgo.It(`should create additional upstream block when auth-keepalive is set with HTTP/1.x`, func() { + f.UpdateNginxConfigMapData("use-http2", "false") + defer func() { + f.UpdateNginxConfigMapData("use-http2", "true") + }() + // Sleep a while just to guarantee that the configmap is applied + framework.Sleep() + + annotations["nginx.ingress.kubernetes.io/auth-keepalive"] = "123" + annotations["nginx.ingress.kubernetes.io/auth-keepalive-requests"] = "456" + annotations["nginx.ingress.kubernetes.io/auth-keepalive-timeout"] = "789" + f.UpdateIngress(ing) + + f.WaitForNginxServer("", + func(server string) bool { + return strings.Contains(server, `upstream auth-external-auth`) && + strings.Contains(server, `keepalive 123;`) && + strings.Contains(server, `keepalive_requests 456;`) && + strings.Contains(server, `keepalive_timeout 789s;`) + }) + }) }) ginkgo.Context("when external authentication is configured with a custom redirect param", func() {