diff --git a/internal/ingress/annotations/redirect/redirect.go b/internal/ingress/annotations/redirect/redirect.go index cecf88c835..52b540974f 100644 --- a/internal/ingress/annotations/redirect/redirect.go +++ b/internal/ingress/annotations/redirect/redirect.go @@ -50,7 +50,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation { // rule used to create a redirect in the paths defined in the rule. // If the Ingress contains both annotations the execution order is // temporal and then permanent -func (a redirect) Parse(ing *extensions.Ingress) (interface{}, error) { +func (r redirect) Parse(ing *extensions.Ingress) (interface{}, error) { r3w, _ := parser.GetBoolAnnotation("from-to-www-redirect", ing) tr, err := parser.GetStringAnnotation("temporal-redirect", ing) @@ -84,11 +84,7 @@ func (a redirect) Parse(ing *extensions.Ingress) (interface{}, error) { prc = defaultPermanentRedirectCode } - if pr != "" { - if err := isValidURL(pr); err != nil { - return nil, err - } - + if pr != "" || r3w { return &Config{ URL: pr, Code: prc, @@ -96,12 +92,6 @@ func (a redirect) Parse(ing *extensions.Ingress) (interface{}, error) { }, nil } - if r3w { - return &Config{ - FromToWWW: r3w, - }, nil - } - return nil, errors.ErrMissingAnnotations } diff --git a/internal/ingress/annotations/redirect/redirect_test.go b/internal/ingress/annotations/redirect/redirect_test.go index 8692a97fd0..6c60341aa0 100644 --- a/internal/ingress/annotations/redirect/redirect_test.go +++ b/internal/ingress/annotations/redirect/redirect_test.go @@ -1,5 +1,5 @@ /* -Copyright 2015 The Kubernetes Authors. +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. @@ -21,107 +21,81 @@ import ( "strconv" "testing" - api "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" - "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/resolver" ) const ( - defRedirect = "http://some-site.com" - defCode = http.StatusMovedPermanently + defRedirectURL = "http://some-site.com" ) -func buildIngress() *extensions.Ingress { - defaultBackend := extensions.IngressBackend{ - ServiceName: "default-backend", - ServicePort: intstr.FromInt(80), +func TestPermanentRedirectWithDefaultCode(t *testing.T) { + rp := NewParser(resolver.Mock{}) + if rp == nil { + t.Fatalf("Expected a parser.IngressAnnotation but returned nil") } - return &extensions.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "foo", - Namespace: api.NamespaceDefault, - }, - Spec: extensions.IngressSpec{ - Backend: &extensions.IngressBackend{ - ServiceName: "default-backend", - ServicePort: intstr.FromInt(80), - }, - Rules: []extensions.IngressRule{ - { - Host: "foo.bar.com", - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: "/foo", - Backend: defaultBackend, - }, - }, - }, - }, - }, - }, - }, - } -} - -type mockBackend struct { - resolver.Mock - redirect bool -} + ing := new(extensions.Ingress) -func (m mockBackend) GetDefaultBackend() defaults.Backend { - return defaults.Backend{SSLRedirect: m.redirect} -} -func TestPermanentRedirectWithDefaultCode(t *testing.T) { - ing := buildIngress() - - data := map[string]string{} - data[parser.GetAnnotationWithPrefix("permanent-redirect")] = defRedirect + data := make(map[string]string, 1) + data[parser.GetAnnotationWithPrefix("permanent-redirect")] = defRedirectURL ing.SetAnnotations(data) - i, err := NewParser(mockBackend{}).Parse(ing) + i, err := rp.Parse(ing) if err != nil { t.Errorf("Unexpected error with ingress: %v", err) } redirect, ok := i.(*Config) if !ok { - t.Errorf("expected a Redirect type") + t.Errorf("Expected a Redirect type") } - if redirect.URL != defRedirect { - t.Errorf("Expected %v as redirect but returned %s", defRedirect, redirect.URL) + if redirect.URL != defRedirectURL { + t.Errorf("Expected %v as redirect but returned %s", defRedirectURL, redirect.URL) } - if redirect.Code != defCode { - t.Errorf("Expected %v as redirect to have a code %s but had %s", defRedirect, strconv.Itoa(defCode), strconv.Itoa(redirect.Code)) + if redirect.Code != defaultPermanentRedirectCode { + t.Errorf("Expected %v as redirect to have a code %d but had %d", defRedirectURL, defaultPermanentRedirectCode, redirect.Code) } } func TestPermanentRedirectWithCustomCode(t *testing.T) { - ing := buildIngress() - - data := map[string]string{} - data[parser.GetAnnotationWithPrefix("permanent-redirect")] = defRedirect - data[parser.GetAnnotationWithPrefix("permanent-redirect-code")] = "308" - ing.SetAnnotations(data) - - i, err := NewParser(mockBackend{}).Parse(ing) - if err != nil { - t.Errorf("Unexpected error with ingress: %v", err) + rp := NewParser(resolver.Mock{}) + if rp == nil { + t.Fatalf("Expected a parser.IngressAnnotation but returned nil") } - redirect, ok := i.(*Config) - if !ok { - t.Errorf("expected a Redirect type") - } - if redirect.URL != defRedirect { - t.Errorf("Expected %v as redirect but returned %s", defRedirect, redirect.URL) + + testCases := map[string]struct { + input int + expectOutput int + }{ + "valid code": {http.StatusPermanentRedirect, http.StatusPermanentRedirect}, + "invalid code": {http.StatusTeapot, defaultPermanentRedirectCode}, } - if redirect.Code != http.StatusPermanentRedirect { - t.Errorf("Expected %v as redirect to have a code %s but had %s", defRedirect, strconv.Itoa(http.StatusPermanentRedirect), strconv.Itoa(redirect.Code)) + + for n, tc := range testCases { + t.Run(n, func(t *testing.T) { + ing := new(extensions.Ingress) + + data := make(map[string]string, 2) + data[parser.GetAnnotationWithPrefix("permanent-redirect")] = defRedirectURL + data[parser.GetAnnotationWithPrefix("permanent-redirect-code")] = strconv.Itoa(tc.input) + ing.SetAnnotations(data) + + i, err := rp.Parse(ing) + if err != nil { + t.Errorf("Unexpected error with ingress: %v", err) + } + redirect, ok := i.(*Config) + if !ok { + t.Errorf("Expected a redirect Config type") + } + if redirect.URL != defRedirectURL { + t.Errorf("Expected %v as redirect but returned %s", defRedirectURL, redirect.URL) + } + if redirect.Code != tc.expectOutput { + t.Errorf("Expected %v as redirect to have a code %d but had %d", defRedirectURL, tc.expectOutput, redirect.Code) + } + }) } } diff --git a/test/e2e/annotations/redirect.go b/test/e2e/annotations/redirect.go new file mode 100644 index 0000000000..a2ce10d25e --- /dev/null +++ b/test/e2e/annotations/redirect.go @@ -0,0 +1,121 @@ +/* +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 ( + "fmt" + "net/http" + "strconv" + "strings" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/parnurzeal/gorequest" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +func noRedirectPolicyFunc(gorequest.Request, []gorequest.Request) error { + return http.ErrUseLastResponse +} + +var _ = framework.IngressNginxDescribe("Annotations - Redirect", func() { + f := framework.NewDefaultFramework("redirect") + + BeforeEach(func() { + }) + + AfterEach(func() { + }) + + It("should respond with a standard redirect code", func() { + By("setting permanent-redirect annotation") + + host := "redirect" + redirectPath := "/something" + redirectURL := "http://redirect.example.com" + + annotations := map[string]string{"nginx.ingress.kubernetes.io/permanent-redirect": redirectURL} + + ing := framework.NewSingleIngress(host, redirectPath, 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, fmt.Sprintf("if ($uri ~* %s) {", redirectPath)) && + strings.Contains(server, fmt.Sprintf("return 301 %s;", redirectURL)) + }) + Expect(err).NotTo(HaveOccurred()) + + By("sending request to redirected URL path") + + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL+redirectPath). + Set("Host", host). + RedirectPolicy(noRedirectPolicyFunc). + End() + + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(BeNumerically("==", http.StatusMovedPermanently)) + Expect(resp.Header.Get("Location")).Should(Equal(redirectURL)) + Expect(body).Should(ContainSubstring("nginx/")) + }) + + It("should respond with a custom redirect code", func() { + By("setting permanent-redirect-code annotation") + + host := "redirect" + redirectPath := "/something" + redirectURL := "http://redirect.example.com" + redirectCode := http.StatusFound + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/permanent-redirect": redirectURL, + "nginx.ingress.kubernetes.io/permanent-redirect-code": strconv.Itoa(redirectCode), + } + + ing := framework.NewSingleIngress(host, redirectPath, 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, fmt.Sprintf("if ($uri ~* %s) {", redirectPath)) && + strings.Contains(server, fmt.Sprintf("return %d %s;", redirectCode, redirectURL)) + }) + Expect(err).NotTo(HaveOccurred()) + + By("sending request to redirected URL path") + + resp, body, errs := gorequest.New(). + Get(f.IngressController.HTTPURL+redirectPath). + Set("Host", host). + RedirectPolicy(noRedirectPolicyFunc). + End() + + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(BeNumerically("==", redirectCode)) + Expect(resp.Header.Get("Location")).Should(Equal(redirectURL)) + Expect(body).Should(ContainSubstring("nginx/")) + }) +})