From b511333130526d34afa3f50874bc3e5eea3c7929 Mon Sep 17 00:00:00 2001 From: Adnan Baruni Date: Mon, 29 Oct 2018 16:34:44 -0500 Subject: [PATCH] add support for auth-snippet annotation add test for new auth-snippet annotation document auth-snippet annotation add e2e test for auth-snippet annotation add log warning and update documentation --- .../nginx-configuration/annotations.md | 10 ++++++ internal/ingress/annotations/authreq/main.go | 17 ++++++++- .../ingress/annotations/authreq/main_test.go | 24 ++++++++----- rootfs/etc/nginx/template/nginx.tmpl | 4 +++ test/e2e/annotations/auth.go | 36 ++++++++++++++++++- 5 files changed, 80 insertions(+), 11 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index a9e4f9f4e2..366ac9b2b7 100644 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -27,6 +27,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/auth-tls-error-page](#client-certificate-authentication)|string| |[nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream](#client-certificate-authentication)|"true" or "false"| |[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string| +|[nginx.ingress.kubernetes.io/auth-snippet](#external-authentication)|string| |[nginx.ingress.kubernetes.io/backend-protocol](#backend-protocol)|string|HTTP,HTTPS,GRPC,GRPCS,AJP| |[nginx.ingress.kubernetes.io/base-url-scheme](#rewrite)|string| |[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string| @@ -326,6 +327,15 @@ Additionally it is possible to set: `` to specify headers to pass to backend once authentication request completes. * `nginx.ingress.kubernetes.io/auth-request-redirect`: `` to specify the X-Auth-Request-Redirect header value. +* `nginx.ingress.kubernetes.io/auth-snippet`: + `` to specify a custom snippet to use with external authentication, e.g. + +```yaml +nginx.ingress.kubernetes.io/auth-url: http://foo.com/external-auth +nginx.ingress.kubernetes.io/auth-snippet: | + proxy_set_header Foo-Header 42; +``` +> Note: `nginx.ingress.kubernetes.io/auth-snippet` is an optional annotation. However, it may only be used in conjunction with `nginx.ingress.kubernetes.io/auth-url` and will be ignored if `nginx.ingress.kubernetes.io/auth-url` is not set !!! example Please check the [external-auth](../../examples/auth/external-auth/README.md) example. diff --git a/internal/ingress/annotations/authreq/main.go b/internal/ingress/annotations/authreq/main.go index 3f33762260..7cca4cad6c 100644 --- a/internal/ingress/annotations/authreq/main.go +++ b/internal/ingress/annotations/authreq/main.go @@ -21,6 +21,8 @@ import ( "regexp" "strings" + "github.com/golang/glog" + extensions "k8s.io/api/extensions/v1beta1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" @@ -37,6 +39,7 @@ type Config struct { Method string `json:"method"` ResponseHeaders []string `json:"responseHeaders,omitempty"` RequestRedirect string `json:"requestRedirect"` + AuthSnippet string `json:"authSnippet"` } // Equal tests for equality between two Config types @@ -74,6 +77,9 @@ func (e1 *Config) Equal(e2 *Config) bool { if e1.RequestRedirect != e2.RequestRedirect { return false } + if e1.AuthSnippet != e2.AuthSnippet { + return false + } return true } @@ -141,7 +147,15 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { } // Optional Parameters - signIn, _ := parser.GetStringAnnotation("auth-signin", ing) + signIn, err := parser.GetStringAnnotation("auth-signin", ing) + if err != nil { + glog.Warning("auth-signin annotation is undefined and will not be set") + } + + authSnippet, err := parser.GetStringAnnotation("auth-snippet", ing) + if err != nil { + glog.Warning("auth-snippet annotation is undefined and will not be set") + } responseHeaders := []string{} hstr, _ := parser.GetStringAnnotation("auth-response-headers", ing) @@ -167,5 +181,6 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { Method: authMethod, ResponseHeaders: responseHeaders, RequestRedirect: requestRedirect, + AuthSnippet: authSnippet, }, nil } diff --git a/internal/ingress/annotations/authreq/main_test.go b/internal/ingress/annotations/authreq/main_test.go index 4ddd956011..6af03dae8e 100644 --- a/internal/ingress/annotations/authreq/main_test.go +++ b/internal/ingress/annotations/authreq/main_test.go @@ -77,16 +77,18 @@ func TestAnnotations(t *testing.T) { signinURL string method string requestRedirect string + authSnippet string expErr bool }{ - {"empty", "", "", "", "", true}, - {"no scheme", "bar", "bar", "", "", true}, - {"invalid host", "http://", "http://", "", "", true}, - {"invalid host (multiple dots)", "http://foo..bar.com", "http://foo..bar.com", "", "", true}, - {"valid URL", "http://bar.foo.com/external-auth", "http://bar.foo.com/external-auth", "", "", false}, - {"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "POST", "", false}, - {"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "", false}, - {"valid URL - request redirect", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "http://foo.com/redirect-me", false}, + {"empty", "", "", "", "", "", true}, + {"no scheme", "bar", "bar", "", "", "", true}, + {"invalid host", "http://", "http://", "", "", "", true}, + {"invalid host (multiple dots)", "http://foo..bar.com", "http://foo..bar.com", "", "", "", true}, + {"valid URL", "http://bar.foo.com/external-auth", "http://bar.foo.com/external-auth", "", "", "", false}, + {"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "POST", "", "", false}, + {"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "", "", false}, + {"valid URL - request redirect", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "http://foo.com/redirect-me", "", false}, + {"auth snippet", "http://foo.com/external-auth", "http://foo.com/external-auth", "", "", "proxy_set_header My-Custom-Header 42;", false}, } for _, test := range tests { @@ -94,11 +96,12 @@ func TestAnnotations(t *testing.T) { data[parser.GetAnnotationWithPrefix("auth-signin")] = test.signinURL data[parser.GetAnnotationWithPrefix("auth-method")] = fmt.Sprintf("%v", test.method) data[parser.GetAnnotationWithPrefix("auth-request-redirect")] = test.requestRedirect + data[parser.GetAnnotationWithPrefix("auth-snippet")] = test.authSnippet i, err := NewParser(&resolver.Mock{}).Parse(ing) if test.expErr { if err == nil { - t.Errorf("%v: expected error but retuned nil", test.title) + t.Errorf("%v: expected error but returned nil", test.title) } continue } @@ -118,6 +121,9 @@ func TestAnnotations(t *testing.T) { if u.RequestRedirect != test.requestRedirect { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.requestRedirect, u.RequestRedirect) } + if u.AuthSnippet != test.authSnippet { + t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.authSnippet, u.AuthSnippet) + } } } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 82370bba76..9790cb0e0b 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -888,6 +888,10 @@ stream { proxy_set_header ssl-client-issuer-dn $ssl_client_i_dn; {{ end }} + {{ if not (empty $location.ExternalAuth.AuthSnippet) }} + {{ $location.ExternalAuth.AuthSnippet }} + {{ end }} + set $target {{ $location.ExternalAuth.URL }}; proxy_pass $target; } diff --git a/test/e2e/annotations/auth.go b/test/e2e/annotations/auth.go index 20939be241..4e1286373c 100644 --- a/test/e2e/annotations/auth.go +++ b/test/e2e/annotations/auth.go @@ -25,7 +25,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/parnurzeal/gorequest" corev1 "k8s.io/api/core/v1" @@ -234,6 +233,41 @@ var _ = framework.IngressNginxDescribe("Annotations - Auth", func() { Expect(resp.StatusCode).Should(Equal(http.StatusInternalServerError)) }) + It(`should set snippet "proxy_set_header My-Custom-Header 42;" when external auth is configured`, func() { + host := "auth" + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/auth-url": "http://foo.bar/basic-auth/user/password", + "nginx.ingress.kubernetes.io/auth-snippet": ` + proxy_set_header My-Custom-Header 42;`, + } + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring(`proxy_set_header My-Custom-Header 42;`)) + }) + }) + + It(`should not set snippet "proxy_set_header My-Custom-Header 42;" when external auth is not configured`, func() { + host := "auth" + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/auth-snippet": ` + proxy_set_header My-Custom-Header 42;`, + } + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).ShouldNot(ContainSubstring(`proxy_set_header My-Custom-Header 42;`)) + }) + }) + Context("when external authentication is configured", func() { host := "auth"