From 188295550c5481a8cfddf831078a547052fea84a Mon Sep 17 00:00:00 2001 From: Alex Kursell Date: Mon, 11 Mar 2019 12:23:14 -0400 Subject: [PATCH] Simplify x-forwarded-prefix annotation --- internal/ingress/annotations/annotations.go | 2 +- .../annotations/xforwardedprefix/main.go | 2 +- .../annotations/xforwardedprefix/main_test.go | 12 +-- .../ingress/controller/template/template.go | 4 +- .../controller/template/template_test.go | 22 ++--- internal/ingress/types.go | 2 +- test/e2e/annotations/xforwardedprefix.go | 84 +++++++++++++++++++ 7 files changed, 106 insertions(+), 22 deletions(-) create mode 100644 test/e2e/annotations/xforwardedprefix.go diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index 45d68c0eb..b803ebedc 100644 --- a/internal/ingress/annotations/annotations.go +++ b/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 diff --git a/internal/ingress/annotations/xforwardedprefix/main.go b/internal/ingress/annotations/xforwardedprefix/main.go index e25beebe3..33458773a 100644 --- a/internal/ingress/annotations/xforwardedprefix/main.go +++ b/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) } diff --git a/internal/ingress/annotations/xforwardedprefix/main_test.go b/internal/ingress/annotations/xforwardedprefix/main_test.go index 95a4d1f7c..97cc2253c 100644 --- a/internal/ingress/annotations/xforwardedprefix/main_test.go +++ b/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{ diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index cf586f08f..8833da8e3 100644 --- a/internal/ingress/controller/template/template.go +++ b/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(` diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index b09ccb6b6..e5fa6c5c1 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/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, }, diff --git a/internal/ingress/types.go b/internal/ingress/types.go index ec84381d3..5483914f2 100644 --- a/internal/ingress/types.go +++ b/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"` diff --git a/test/e2e/annotations/xforwardedprefix.go b/test/e2e/annotations/xforwardedprefix.go new file mode 100644 index 000000000..908630bfc --- /dev/null +++ b/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.IngressController.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.IngressController.HTTPURL+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.IngressController.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.IngressController.HTTPURL+uri). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).To(Not(ContainSubstring("x-forwarded-prefix"))) + }) +})