Allow preservation of trailing slashes on TLS redirects via annotation. (#7144)

* allow retaining a trailing slash in a TLS redirect via annotation.

Signed-off-by: mamiller <mamiller@rosettastone.com>

* requested changes

* gofmt

internal/ingress/annotations/rewrite/main.go

@@ -35,6 +35,8 @@ type Config struct {
 	SSLRedirect bool `json:"sslRedirect"`
 	// ForceSSLRedirect indicates if the location section is accessible SSL only
 	ForceSSLRedirect bool `json:"forceSSLRedirect"`
+	// PreserveTrailingSlash indicates if the trailing slash should be kept during a tls redirect
+	PreserveTrailingSlash bool `json:"preserveTrailingSlash"`
 	// AppRoot defines the Application Root that the Controller must redirect if it's in '/' context
 	AppRoot string `json:"appRoot"`
 	// UseRegex indicates whether or not the locations use regex paths
@@ -88,6 +90,10 @@ func (a rewrite) Parse(ing *networking.Ingress) (interface{}, error) {
 	if err != nil {
 		config.SSLRedirect = a.r.GetDefaultBackend().SSLRedirect
 	}
+	config.PreserveTrailingSlash, err = parser.GetBoolAnnotation("preserve-trailing-slash", ing)
+	if err != nil {
+		config.PreserveTrailingSlash = a.r.GetDefaultBackend().PreserveTrailingSlash
+	}
 
 	config.ForceSSLRedirect, err = parser.GetBoolAnnotation("force-ssl-redirect", ing)
 	if err != nil {

internal/ingress/controller/config/config.go

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

internal/ingress/controller/template/template.go

@@ -342,12 +342,14 @@ func locationConfigForLua(l interface{}, a interface{}) string {
 		force_ssl_redirect = %t,
 		ssl_redirect = %t,
 		force_no_ssl_redirect = %t,
+		preserve_trailing_slash = %t,
 		use_port_in_redirects = %t,
 		global_throttle = { namespace = "%v", limit = %d, window_size = %d, key = %v, ignored_cidrs = %v },
 	}`,
 		location.Rewrite.ForceSSLRedirect,
 		location.Rewrite.SSLRedirect,
 		isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations),
+		location.Rewrite.PreserveTrailingSlash,
 		location.UsePortInRedirects,
 		location.GlobalRateLimit.Namespace,
 		location.GlobalRateLimit.Limit,

internal/ingress/defaults/main.go

@@ -31,6 +31,9 @@ type Backend struct {
 	// By default this is disabled
 	CustomHTTPErrors []int `json:"custom-http-errors"`
 
+	// toggles whether or not to remove trailing slashes during TLS redirects
+	PreserveTrailingSlash bool `json:"preserve-trailing-slash"`
+
 	// http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size
 	// Sets the maximum allowed size of the client request body
 	ProxyBodySize string `json:"proxy-body-size"`

rootfs/etc/nginx/lua/lua_ingress.lua

@@ -147,9 +147,11 @@ function _M.rewrite(location_config)
 
   if redirect_to_https(location_config) then
     local request_uri = ngx.var.request_uri
-    -- do not append a trailing slash on redirects
-    if string.sub(request_uri, -1) == "/" then
-      request_uri = string.sub(request_uri, 1, -2)
+    -- do not append a trailing slash on redirects unless enabled by annotations
+    if location_config.preserve_trailing_slash == false then
+      if string.byte(request_uri, -1, -1) == string.byte('/') then
+        request_uri = string.sub(request_uri, 1, -2)
+      end
     end
 
     local uri = string_format("https://%s%s", redirect_host(), request_uri)

test/e2e/annotations/preservetrailingslash.go

@@ -0,0 +1,53 @@
+/*
+Copyright 2021 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"
+
+	"github.com/onsi/ginkgo"
+
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+var _ = framework.DescribeAnnotation("preserve-trailing-slash", func() {
+	f := framework.NewDefaultFramework("preservetrailingslash")
+
+	ginkgo.BeforeEach(func() {
+		f.NewEchoDeployment()
+	})
+
+	ginkgo.It("should allow preservation of trailing slashes", func() {
+		host := "forcesslredirect.bar.com"
+		tlsHosts := []string{host}
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/ssl-redirect":            "true",
+			"nginx.ingress.kubernetes.io/preserve-trailing-slash": "true",
+		}
+
+		ing := framework.NewSingleIngressWithTLS(host, "/", host, tlsHosts, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			Expect().
+			Status(http.StatusPermanentRedirect).
+			Header("Location").Equal("https://forcesslredirect.bar.com/")
+	})
+})