Resolves issue with proxy-redirect nginx configuration

Resolves an issue where the proxy-redirect annotations were not generating the
correct configuration possibly because of user error. This is done by only
setting the proxy_redirect if both proxy-redirect-from and proxy-redirect-to
have valid values. Also adds the e2e tests.

Fixes #2074

docs/user-guide/nginx-configuration/annotations.md

@@ -443,11 +443,13 @@ In some scenarios is required to have different values. To allow this we provide
 
 ### Proxy redirect
 
-With the annotations `nginx.ingress.kubernetes.io/proxy-redirect-from` and `nginx.ingress.kubernetes.io/proxy-redirect-to` it is possible to set the text that should be changed in the `Location` and `Refresh` header fields of a proxied server response (http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_redirect)
+With the annotations `nginx.ingress.kubernetes.io/proxy-redirect-from` and `nginx.ingress.kubernetes.io/proxy-redirect-to` it is possible to
+set the text that should be changed in the `Location` and `Refresh` header fields of a proxied server response (http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_redirect)
 
-Setting "off" or "default" in the annotation `nginx.ingress.kubernetes.io/proxy-redirect-from` disables `nginx.ingress.kubernetes.io/proxy-redirect-to`.
+Setting "off" or "default" in the annotation `nginx.ingress.kubernetes.io/proxy-redirect-from` disables `nginx.ingress.kubernetes.io/proxy-redirect-to`,
+otherwise, both annotations must be used in unison. Note that each annotation must be a string without spaces.
 
-Both annotations will be used in any other case. By default the value is "off".
+By default the value of each annotation is "off".
 
 ### Custom max body size
 

internal/ingress/controller/config/config.go

@@ -604,6 +604,7 @@ func NewDefault() Configuration {
 			ProxyNextUpstreamTries: 0,
 			ProxyRequestBuffering:  "on",
 			ProxyRedirectFrom:      "off",
+			ProxyRedirectTo:        "off",
 			SSLRedirect:            true,
 			CustomHTTPErrors:       []int{},
 			WhitelistSourceRange:   []string{},

rootfs/etc/nginx/template/nginx.tmpl

@@ -1087,7 +1087,7 @@ stream {
             {{ buildProxyPass $server.Hostname $all.Backends $location $all.DynamicConfigurationEnabled }}
             {{ if (or (eq $location.Proxy.ProxyRedirectFrom "default") (eq $location.Proxy.ProxyRedirectFrom "off")) }}
             proxy_redirect                          {{ $location.Proxy.ProxyRedirectFrom }};
-            {{ else }}
+            {{ else if not (eq $location.Proxy.ProxyRedirectTo "off") }}
             proxy_redirect                          {{ $location.Proxy.ProxyRedirectFrom }} {{ $location.Proxy.ProxyRedirectTo }};
             {{ end }}
             {{ else }}

test/e2e/annotations/auth.go

@@ -31,8 +31,8 @@ import (
 	"k8s.io/ingress-nginx/test/e2e/framework"
 )
 
-var _ = framework.IngressNginxDescribe("Annotations - Alias", func() {
+var _ = framework.IngressNginxDescribe("Annotations - Auth", func() {
-	f := framework.NewDefaultFramework("alias")
+	f := framework.NewDefaultFramework("auth")
 
 	BeforeEach(func() {
 		err := f.NewEchoDeployment()

test/e2e/annotations/proxy.go

@@ -0,0 +1,169 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package annotations
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	v1beta1 "k8s.io/api/extensions/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+var _ = framework.IngressNginxDescribe("Annotations - Proxy", func() {
+	f := framework.NewDefaultFramework("proxy")
+
+	BeforeEach(func() {
+		err := f.NewEchoDeploymentWithReplicas(2)
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+	})
+
+	It("should set proxy_redirect to off", func() {
+		host := "proxy.foo.com"
+
+		ing, err := f.EnsureIngress(&v1beta1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      host,
+				Namespace: f.IngressController.Namespace,
+				Annotations: map[string]string{
+					"nginx.ingress.kubernetes.io/proxy-redirect-from": "off",
+					"nginx.ingress.kubernetes.io/proxy-redirect-to":   "goodbye.com",
+				},
+			},
+			Spec: v1beta1.IngressSpec{
+				Rules: []v1beta1.IngressRule{
+					{
+						Host: host,
+						IngressRuleValue: v1beta1.IngressRuleValue{
+							HTTP: &v1beta1.HTTPIngressRuleValue{
+								Paths: []v1beta1.HTTPIngressPath{
+									{
+										Path: "/",
+										Backend: v1beta1.IngressBackend{
+											ServiceName: "http-svc",
+											ServicePort: intstr.FromInt(80),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ing).NotTo(BeNil())
+
+		err = f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring("proxy_redirect off;"))
+			})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should set proxy_redirect to default", func() {
+		host := "proxy.foo.com"
+
+		ing, err := f.EnsureIngress(&v1beta1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      host,
+				Namespace: f.IngressController.Namespace,
+				Annotations: map[string]string{
+					"nginx.ingress.kubernetes.io/proxy-redirect-from": "default",
+					"nginx.ingress.kubernetes.io/proxy-redirect-to":   "goodbye.com",
+				},
+			},
+			Spec: v1beta1.IngressSpec{
+				Rules: []v1beta1.IngressRule{
+					{
+						Host: host,
+						IngressRuleValue: v1beta1.IngressRuleValue{
+							HTTP: &v1beta1.HTTPIngressRuleValue{
+								Paths: []v1beta1.HTTPIngressPath{
+									{
+										Path: "/",
+										Backend: v1beta1.IngressBackend{
+											ServiceName: "http-svc",
+											ServicePort: intstr.FromInt(80),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ing).NotTo(BeNil())
+
+		err = f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring("proxy_redirect default;"))
+			})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should set proxy_redirect to hello.com goodbye.com", func() {
+		host := "proxy.foo.com"
+
+		ing, err := f.EnsureIngress(&v1beta1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      host,
+				Namespace: f.IngressController.Namespace,
+				Annotations: map[string]string{
+					"nginx.ingress.kubernetes.io/proxy-redirect-from": "hello.com",
+					"nginx.ingress.kubernetes.io/proxy-redirect-to":   "goodbye.com",
+				},
+			},
+			Spec: v1beta1.IngressSpec{
+				Rules: []v1beta1.IngressRule{
+					{
+						Host: host,
+						IngressRuleValue: v1beta1.IngressRuleValue{
+							HTTP: &v1beta1.HTTPIngressRuleValue{
+								Paths: []v1beta1.HTTPIngressPath{
+									{
+										Path: "/",
+										Backend: v1beta1.IngressBackend{
+											ServiceName: "http-svc",
+											ServicePort: intstr.FromInt(80),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(ing).NotTo(BeNil())
+
+		err = f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring("proxy_redirect hello.com goodbye.com;"))
+			})
+		Expect(err).NotTo(HaveOccurred())
+	})
+})

test/e2e/framework/framework.go

@@ -262,7 +262,7 @@ func (f *Framework) matchNginxConditions(name string, matcher func(cfg string) b
 				glog.Infof("nginx.conf:\n%v", o)
 			}
 
-			if matcher(o) {
+			if matcher(strings.Join(strings.Fields(o), " ")) {
 				match = true
 			}
 		})