From 72a2aa171a575a34b3d686f79b3f84f657a565bd Mon Sep 17 00:00:00 2001 From: dongqi1990 Date: Wed, 18 Jul 2018 22:08:37 +0800 Subject: [PATCH] fix the bug #2799, add prefix (?i) in rewrite statement and add new e2e test. --- .../ingress/controller/template/template.go | 6 +- .../controller/template/template_test.go | 30 ++--- test/e2e/annotations/affinity.go | 3 +- test/e2e/annotations/rewrite.go | 120 ++++++++++++++++++ test/e2e/annotations/rewrite_log.go | 100 --------------- 5 files changed, 139 insertions(+), 120 deletions(-) create mode 100644 test/e2e/annotations/rewrite.go delete mode 100644 test/e2e/annotations/rewrite_log.go diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 1e69a0077f..6e300af061 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -446,14 +446,14 @@ subs_filter '%v' '$1' ro; // special case redirect to / // ie /something to / return fmt.Sprintf(` -rewrite %s(.*) /$1 break; -rewrite %s / break; +rewrite (?i)%s(.*) /$1 break; +rewrite (?i)%s / break; %v%v %s://%s; %v`, path, location.Path, xForwardedPrefix, proxyPass, proto, upstreamName, abu) } return fmt.Sprintf(` -rewrite %s(.*) %s/$1 break; +rewrite (?i)%s(.*) %s/$1 break; %v%v %s://%s; %v`, path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName, abu) } diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index f42514c9a3..69807ed7b4 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -122,7 +122,7 @@ var ( "/jenkins", "~* /", ` -rewrite /(.*) /jenkins/$1 break; +rewrite (?i)/(.*) /jenkins/$1 break; proxy_pass http://upstream-name; `, false, @@ -136,8 +136,8 @@ proxy_pass http://upstream-name; "/", `~* ^/something\/?(?.*)`, ` -rewrite /something/(.*) /$1 break; -rewrite /something / break; +rewrite (?i)/something/(.*) /$1 break; +rewrite (?i)/something / break; proxy_pass http://upstream-name; `, false, @@ -151,7 +151,7 @@ proxy_pass http://upstream-name; "/not-root", "~* ^/end-with-slash/(?.*)", ` -rewrite /end-with-slash/(.*) /not-root/$1 break; +rewrite (?i)/end-with-slash/(.*) /not-root/$1 break; proxy_pass http://upstream-name; `, false, @@ -165,7 +165,7 @@ proxy_pass http://upstream-name; "/not-root", `~* ^/something-complex\/?(?.*)`, ` -rewrite /something-complex/(.*) /not-root/$1 break; +rewrite (?i)/something-complex/(.*) /not-root/$1 break; proxy_pass http://upstream-name; `, false, @@ -179,7 +179,7 @@ proxy_pass http://upstream-name; "/jenkins", "~* /", ` -rewrite /(.*) /jenkins/$1 break; +rewrite (?i)/(.*) /jenkins/$1 break; proxy_pass http://upstream-name; set_escape_uri $escaped_base_uri $baseuri; @@ -196,8 +196,8 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1.*)`, ` -rewrite /something/(.*) /$1 break; -rewrite /something / break; +rewrite (?i)/something/(.*) /$1 break; +rewrite (?i)/something / break; proxy_pass http://upstream-name; set_escape_uri $escaped_base_uri $baseuri; @@ -214,7 +214,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1.*)`, ` -rewrite /end-with-slash/(.*) /not-root/$1 break; +rewrite (?i)/end-with-slash/(.*) /not-root/$1 break; proxy_pass http://upstream-name; set_escape_uri $escaped_base_uri $baseuri; @@ -231,7 +231,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1.*)`, ` -rewrite /something-complex/(.*) /not-root/$1 break; +rewrite (?i)/something-complex/(.*) /not-root/$1 break; proxy_pass http://upstream-name; set_escape_uri $escaped_base_uri $baseuri; @@ -248,8 +248,8 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1.*)`, ` -rewrite /something/(.*) /$1 break; -rewrite /something / break; +rewrite (?i)/something/(.*) /$1 break; +rewrite (?i)/something / break; proxy_pass http://upstream-name; set_escape_uri $escaped_base_uri $baseuri; @@ -266,7 +266,7 @@ subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1.*)`, ` -rewrite /there/(.*) /something/$1 break; +rewrite (?i)/there/(.*) /something/$1 break; proxy_set_header X-Forwarded-Prefix "/there/"; proxy_pass http://sticky-upstream-name; `, diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index 83ad03ecf5..bfe8f24f0e 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -105,7 +105,6 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity", func() { Annotations: map[string]string{ "nginx.ingress.kubernetes.io/affinity": "cookie", "nginx.ingress.kubernetes.io/session-cookie-name": "SERVERID", - "nginx.ingress.kubernetes.io/rewrite-target": "/something", }, }, Spec: v1beta1.IngressSpec{ @@ -146,7 +145,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Affinity", func() { Expect(len(errs)).Should(BeNumerically("==", 0)) Expect(resp.StatusCode).Should(Equal(http.StatusOK)) - Expect(body).Should(ContainSubstring(fmt.Sprintf("request_uri=http://%v:8080/something/", host))) + Expect(body).Should(ContainSubstring(fmt.Sprintf("request_uri=http://%v:8080/", host))) Expect(resp.Header.Get("Set-Cookie")).Should(ContainSubstring("SERVERID=")) }) diff --git a/test/e2e/annotations/rewrite.go b/test/e2e/annotations/rewrite.go new file mode 100644 index 0000000000..11956748d0 --- /dev/null +++ b/test/e2e/annotations/rewrite.go @@ -0,0 +1,120 @@ +/* +Copyright 2017 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 ( + "fmt" + "net/http" + "strings" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/parnurzeal/gorequest" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Annotations - Rewrite", func() { + f := framework.NewDefaultFramework("rewrite") + + BeforeEach(func() { + err := f.NewEchoDeploymentWithReplicas(1) + Expect(err).NotTo(HaveOccurred()) + }) + + AfterEach(func() { + }) + + It("should rewrite request URL", func() { + By("setting rewrite-target annotation") + + host := "rewrite.foo.com" + annotations := map[string]string{"nginx.ingress.kubernetes.io/rewrite-target": "/"} + expectBodyRequestURI := fmt.Sprintf("request_uri=http://%v:8080/", host) + + ing := framework.NewSingleIngress(host, "/something", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + _, err := f.EnsureIngress(ing) + + Expect(err).NotTo(HaveOccurred()) + Expect(ing).NotTo(BeNil()) + + err = f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "rewrite (?i)/something/(.*) /$1 break;") && + strings.Contains(server, "rewrite (?i)/something / break;") + }) + Expect(err).NotTo(HaveOccurred()) + + By("sending request to Ingress rule path (lowercase)") + + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL+"/something"). + Set("Host", host). + End() + + Expect(len(errs)).Should(Equal(0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring(expectBodyRequestURI)) + + By("sending request to Ingress rule path (mixed case)") + + resp, body, errs = gorequest.New(). + Get(f.IngressController.HTTPURL+"/SomeThing"). + Set("Host", host). + End() + + Expect(len(errs)).Should(Equal(0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + Expect(body).Should(ContainSubstring(expectBodyRequestURI)) + }) + + It("should write rewrite logs", func() { + By("setting enable-rewrite-log annotation") + + host := "rewrite.bar.com" + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/rewrite-target": "/", + "nginx.ingress.kubernetes.io/enable-rewrite-log": "true", + } + + ing := framework.NewSingleIngress(host, "/something", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + _, err := f.EnsureIngress(ing) + + Expect(err).NotTo(HaveOccurred()) + Expect(ing).NotTo(BeNil()) + + err = f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "rewrite_log on;") + }) + Expect(err).NotTo(HaveOccurred()) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL+"/something"). + Set("Host", host). + End() + + Expect(len(errs)).Should(Equal(0)) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + logs, err := f.NginxLogs() + Expect(err).ToNot(HaveOccurred()) + Expect(logs).To(ContainSubstring(`"(?i)/something" matches "/something", client:`)) + Expect(logs).To(ContainSubstring(`rewritten data: "/", args: "",`)) + }) +}) diff --git a/test/e2e/annotations/rewrite_log.go b/test/e2e/annotations/rewrite_log.go deleted file mode 100644 index 104c7f3756..0000000000 --- a/test/e2e/annotations/rewrite_log.go +++ /dev/null @@ -1,100 +0,0 @@ -/* -Copyright 2017 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 ( - "net/http" - "strings" - "time" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "github.com/parnurzeal/gorequest" - "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 - Rewrite Log", func() { - f := framework.NewDefaultFramework("rewrite-log") - - BeforeEach(func() { - err := f.NewEchoDeploymentWithReplicas(2) - Expect(err).NotTo(HaveOccurred()) - }) - - AfterEach(func() { - }) - - It("should write rewrite logs", func() { - host := "rewrite.bar.com" - - ing, err := f.EnsureIngress(&v1beta1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: host, - Namespace: f.IngressController.Namespace, - Annotations: map[string]string{ - "nginx.ingress.kubernetes.io/rewrite-target": "/", - "nginx.ingress.kubernetes.io/enable-rewrite-log": "true", - }, - }, - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{ - { - Host: host, - IngressRuleValue: v1beta1.IngressRuleValue{ - HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ - { - Path: "/something", - 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 strings.Contains(server, "rewrite_log on;") - }) - Expect(err).NotTo(HaveOccurred()) - - resp, _, errs := gorequest.New(). - Get(f.IngressController.HTTPURL+"/something"). - Set("Host", host). - End() - - Expect(len(errs)).Should(Equal(0)) - Expect(resp.StatusCode).Should(Equal(http.StatusOK)) - - time.Sleep(5 * time.Second) - logs, err := f.NginxLogs() - Expect(err).ToNot(HaveOccurred()) - Expect(logs).To(ContainSubstring(`"/something" matches "/something", client:`)) - Expect(logs).To(ContainSubstring(`rewritten data: "/", args: "",`)) - }) -})