Fix x-forwarded-prefix annotation

cmd/plugin/lints/ingress.go

@@ -78,9 +78,24 @@ func GetIngressLints() []IngressLint {
 			message: "Contains an annotation with the prefix 'nginx.com'. This is a prefix for https://github.com/nginxinc/kubernetes-ingress",
 			f:       annotationPrefixIsNginxCom,
 		},
+		{
+			message: "The x-forwarded-prefix annotation value is a boolean instead of a string",
+			issue:   3786,
+			version: "0.24.0",
+			f:       xForwardedPrefixIsBool,
+		},
 	}
 }
 
+func xForwardedPrefixIsBool(ing v1beta1.Ingress) bool {
+	for name, val := range ing.Annotations {
+		if strings.HasSuffix(name, "/x-forwarded-prefix") && (val == "true" || val == "false") {
+			return true
+		}
+	}
+	return false
+}
+
 func annotationPrefixIsNginxCom(ing v1beta1.Ingress) bool {
 	for name := range ing.Annotations {
 		if strings.HasPrefix(name, "nginx.com/") {

internal/ingress/annotations/annotations.go

@@ -99,7 +99,7 @@ type Ingress struct {
 	LoadBalancing      string
 	UpstreamVhost      string
 	Whitelist          ipwhitelist.SourceRange
-	XForwardedPrefix   bool
+	XForwardedPrefix   string
 	SSLCiphers         string
 	Logs               log.Config
 	LuaRestyWAF        luarestywaf.Config

internal/ingress/annotations/xforwardedprefix/main.go

@@ -35,5 +35,5 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
 // Parse parses the annotations contained in the ingress rule
 // used to add an x-forwarded-prefix header to the request
 func (cbbs xforwardedprefix) Parse(ing *extensions.Ingress) (interface{}, error) {
-	return parser.GetBoolAnnotation("x-forwarded-prefix", ing)
+	return parser.GetStringAnnotation("x-forwarded-prefix", ing)
 }

internal/ingress/annotations/xforwardedprefix/main_test.go

@@ -35,13 +35,13 @@ func TestParse(t *testing.T) {
 
 	testCases := []struct {
 		annotations map[string]string
-		expected    bool
+		expected    string
 	}{
-		{map[string]string{annotation: "true"}, true},
-		{map[string]string{annotation: "1"}, true},
-		{map[string]string{annotation: ""}, false},
-		{map[string]string{}, false},
-		{nil, false},
+		{map[string]string{annotation: "true"}, "true"},
+		{map[string]string{annotation: "1"}, "1"},
+		{map[string]string{annotation: ""}, ""},
+		{map[string]string{}, ""},
+		{nil, ""},
 	}
 
 	ing := &extensions.Ingress{

internal/ingress/controller/template/template.go

@@ -456,8 +456,8 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
 	if len(location.Rewrite.Target) > 0 {
 		var xForwardedPrefix string
 
-		if location.XForwardedPrefix {
-			xForwardedPrefix = fmt.Sprintf("proxy_set_header X-Forwarded-Prefix \"%s\";\n", path)
+		if len(location.XForwardedPrefix) > 0 {
+			xForwardedPrefix = fmt.Sprintf("proxy_set_header X-Forwarded-Prefix \"%s\";\n", location.XForwardedPrefix)
 		}
 
 		return fmt.Sprintf(`

internal/ingress/controller/template/template_test.go

@@ -48,7 +48,7 @@ var (
 		Location         string
 		ProxyPass        string
 		Sticky           bool
-		XForwardedPrefix bool
+		XForwardedPrefix string
 		SecureBackend    bool
 		enforceRegex     bool
 	}{
@@ -58,7 +58,7 @@ var (
 			"/",
 			"proxy_pass https://upstream_balancer;",
 			false,
-			false,
+			"",
 			true,
 			false,
 		},
@@ -68,7 +68,7 @@ var (
 			"/",
 			"proxy_pass https://upstream_balancer;",
 			false,
-			false,
+			"",
 			true,
 			false,
 		},
@@ -78,7 +78,7 @@ var (
 			"/",
 			"proxy_pass https://upstream_balancer;",
 			true,
-			false,
+			"",
 			true,
 			false,
 		},
@@ -88,7 +88,7 @@ var (
 			"/",
 			"proxy_pass http://upstream_balancer;",
 			false,
-			false,
+			"",
 			false,
 			false,
 		},
@@ -98,7 +98,7 @@ var (
 			"/",
 			"proxy_pass http://upstream_balancer;",
 			false,
-			false,
+			"",
 			false,
 			false,
 		},
@@ -110,7 +110,7 @@ var (
 rewrite "(?i)/" /jenkins break;
 proxy_pass http://upstream_balancer;`,
 			false,
-			false,
+			"",
 			false,
 			true,
 		},
@@ -122,7 +122,7 @@ proxy_pass http://upstream_balancer;`,
 rewrite "(?i)/" /something break;
 proxy_pass http://upstream_balancer;`,
 			true,
-			false,
+			"",
 			false,
 			true,
 		},
@@ -134,7 +134,7 @@ proxy_pass http://upstream_balancer;`,
 rewrite "(?i)/" /something break;
 proxy_pass http://upstream_balancer;`,
 			true,
-			false,
+			"",
 			false,
 			true,
 		},
@@ -147,7 +147,7 @@ rewrite "(?i)/there" /something break;
 proxy_set_header X-Forwarded-Prefix "/there";
 proxy_pass http://upstream_balancer;`,
 			true,
-			true,
+			"/there",
 			false,
 			true,
 		},
@@ -157,7 +157,7 @@ proxy_pass http://upstream_balancer;`,
 			`~* "^/something"`,
 			"proxy_pass http://upstream_balancer;",
 			false,
-			false,
+			"",
 			false,
 			true,
 		},

internal/ingress/types.go

@@ -292,7 +292,7 @@ type Location struct {
 	// XForwardedPrefix allows to add a header X-Forwarded-Prefix to the request with the
 	// original location.
 	// +optional
-	XForwardedPrefix bool `json:"xForwardedPrefix,omitempty"`
+	XForwardedPrefix string `json:"xForwardedPrefix,omitempty"`
 	// Logs allows to enable or disable the nginx logs
 	// By default access logs are enabled and rewrite logs are disabled
 	Logs log.Config `json:"logs,omitempty"`

test/e2e/annotations/xforwardedprefix.go

@@ -0,0 +1,84 @@
+/*
+Copyright 2019 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"
+	"github.com/parnurzeal/gorequest"
+	"k8s.io/ingress-nginx/test/e2e/framework"
+	"net/http"
+)
+
+var _ = framework.IngressNginxDescribe("Annotations - X-Forwarded-Prefix", func() {
+	f := framework.NewDefaultFramework("xforwardedprefix")
+
+	BeforeEach(func() {
+		f.NewEchoDeployment()
+	})
+
+	AfterEach(func() {
+	})
+
+	It("should set the X-Forwarded-Prefix to the annotation value", func() {
+		host := "xfp.baz.com"
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/x-forwarded-prefix": "/test/value",
+			"nginx.ingress.kubernetes.io/rewrite-target":     "/foo",
+		}
+
+		f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, &annotations))
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring("proxy_set_header X-Forwarded-Prefix \"/test/value\";"))
+			})
+
+		uri := "/"
+		resp, body, errs := gorequest.New().
+			Get(f.GetURL(framework.HTTP)+uri).
+			Set("Host", host).
+			End()
+
+		Expect(errs).Should(BeEmpty())
+		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
+		Expect(body).To(ContainSubstring("x-forwarded-prefix=/test/value"))
+	})
+
+	It("should not add X-Forwarded-Prefix if the annotation value is empty", func() {
+		host := "noxfp.baz.com"
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/x-forwarded-prefix": "",
+			"nginx.ingress.kubernetes.io/rewrite-target":     "/foo",
+		}
+
+		f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, &annotations))
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(And(ContainSubstring(host), Not(ContainSubstring("proxy_set_header X-Forwarded-Prefix"))))
+			})
+
+		uri := "/"
+		resp, body, errs := gorequest.New().
+			Get(f.GetURL(framework.HTTP)+uri).
+			Set("Host", host).
+			End()
+
+		Expect(errs).Should(BeEmpty())
+		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
+		Expect(body).To(Not(ContainSubstring("x-forwarded-prefix")))
+	})
+})