Config/Annotations: Add relative-redirects. (#12161)

Author: chriss-de

docs/user-guide/nginx-configuration/annotations-risk.md

@@ -103,6 +103,7 @@
 | Redirect | from-to-www-redirect | Low | location |
 | Redirect | permanent-redirect | Medium | location |
 | Redirect | permanent-redirect-code | Low | location |
+| Redirect | relative-redirects | Low | location |
 | Redirect | temporal-redirect | Medium | location |
 | Redirect | temporal-redirect-code | Low | location |
 | Rewrite | app-root | Medium | location |

docs/user-guide/nginx-configuration/configmap.md

@@ -223,6 +223,7 @@ The following table shows a configuration option's name, type, and the default v
 | [debug-connections](#debug-connections)                                         | []string     | "127.0.0.1,1.1.1.1/24"                                                                                                                                                                                                                                                                                                                                       |                                                                                     |
 | [strict-validate-path-type](#strict-validate-path-type)                         | bool         | "true"                                                                                                                                                                                                                                                                                                                                                       |                                                                                     |
 | [grpc-buffer-size-kb](#grpc-buffer-size-kb)                                     | int          | 0                                                                                                                                                                                                                                                                                                                                                            |                                                                                     |
+| [relative-redirects](#relative-redirects)                                       | bool         | false                                                                                                                                                                                                                                                                                                                                                        |                                                                                     |
 
 ## add-headers
 
@@ -1382,3 +1383,14 @@ Sets the configuration for the GRPC Buffer Size parameter. If not set it will us
 
 _References:_
 [https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_buffer_size](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_buffer_size)
+
+## relative-redirects
+
+Use relative redirects instead of absolute redirects. Absolute redirects are the default in nginx. RFC7231 allows relative redirects since 2014.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/relative-redirects`.
+
+_**default:**_ "false"
+
+_References:_
+- [https://nginx.org/en/docs/http/ngx_http_core_module.html#absolute_redirect](https://nginx.org/en/docs/http/ngx_http_core_module.html#absolute_redirect)
+- [https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2](https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2)

internal/ingress/annotations/redirect/redirect.go

@@ -38,6 +38,7 @@ type Config struct {
 	URL       string `json:"url"`
 	Code      int    `json:"code"`
 	FromToWWW bool   `json:"fromToWWW"`
+	Relative  bool   `json:"relative"`
 }
 
 const (
@@ -46,6 +47,7 @@ const (
 	temporalRedirectAnnotationCode  = "temporal-redirect-code"
 	permanentRedirectAnnotation     = "permanent-redirect"
 	permanentRedirectAnnotationCode = "permanent-redirect-code"
+	relativeRedirectsAnnotation     = "relative-redirects"
 )
 
 var redirectAnnotations = parser.Annotation{
@@ -83,6 +85,12 @@ var redirectAnnotations = parser.Annotation{
 			Risk:          parser.AnnotationRiskLow, // Low, as it allows just a set of options
 			Documentation: `This annotation allows you to modify the status code used for permanent redirects.`,
 		},
+		relativeRedirectsAnnotation: {
+			Validator:     parser.ValidateBool,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskLow,
+			Documentation: `If enabled, redirects issued by nginx will be relative. See https://nginx.org/en/docs/http/ngx_http_core_module.html#absolute_redirect`,
+		},
 	},
 }
 
@@ -109,6 +117,11 @@ func (r redirect) Parse(ing *networking.Ingress) (interface{}, error) {
 		return nil, err
 	}
 
+	rr, err := parser.GetBoolAnnotation(relativeRedirectsAnnotation, ing, r.annotationConfig.Annotations)
+	if err != nil && !errors.IsMissingAnnotations(err) {
+		return nil, err
+	}
+
 	tr, err := parser.GetStringAnnotation(temporalRedirectAnnotation, ing, r.annotationConfig.Annotations)
 	if err != nil && !errors.IsMissingAnnotations(err) {
 		return nil, err
@@ -132,6 +145,7 @@ func (r redirect) Parse(ing *networking.Ingress) (interface{}, error) {
 			URL:       tr,
 			Code:      trc,
 			FromToWWW: r3w,
+			Relative:  rr,
 		}, nil
 	}
 
@@ -154,6 +168,13 @@ func (r redirect) Parse(ing *networking.Ingress) (interface{}, error) {
 			URL:       pr,
 			Code:      prc,
 			FromToWWW: r3w,
+			Relative:  rr,
+		}, nil
+	}
+
+	if rr {
+		return &Config{
+			Relative: rr,
 		}, nil
 	}
 
@@ -177,6 +198,9 @@ func (r1 *Config) Equal(r2 *Config) bool {
 	if r1.FromToWWW != r2.FromToWWW {
 		return false
 	}
+	if r1.Relative != r2.Relative {
+		return false
+	}
 	return true
 }
 

internal/ingress/annotations/redirect/redirect_test.go

@@ -193,3 +193,22 @@ func TestIsValidURL(t *testing.T) {
 		t.Errorf("expected nil but got %v", err)
 	}
 }
+
+func TestParseAnnotations(t *testing.T) {
+	ing := new(networking.Ingress)
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix(relativeRedirectsAnnotation)] = "true"
+	ing.SetAnnotations(data)
+
+	_, err := NewParser(&resolver.Mock{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	// test ingress using the annotation without a TLS section
+	_, err = NewParser(&resolver.Mock{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error parsing ingress with relative-redirects")
+	}
+}

internal/ingress/controller/config/config.go

@@ -549,6 +549,10 @@ type Configuration struct {
 	// https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors
 	DisableProxyInterceptErrors bool `json:"disable-proxy-intercept-errors,omitempty"`
 
+	// Disable absolute redirects and enables relative redirects.
+	// https://nginx.org/en/docs/http/ngx_http_core_module.html#absolute_redirect
+	RelativeRedirects bool `json:"relative-redirects"`
+
 	// Sets the ipv4 addresses on which the server will accept requests.
 	BindAddressIpv4 []string `json:"bind-address-ipv4,omitempty"`
 
@@ -834,6 +838,7 @@ func NewDefault() Configuration {
 		VariablesHashMaxSize:             2048,
 		UseHTTP2:                         true,
 		DisableProxyInterceptErrors:      false,
+		RelativeRedirects:                false,
 		ProxyStreamTimeout:               "600s",
 		ProxyStreamNextUpstream:          true,
 		ProxyStreamNextUpstreamTimeout:   "600s",
@@ -857,6 +862,7 @@ func NewDefault() Configuration {
 			SSLRedirect:                 true,
 			CustomHTTPErrors:            []int{},
 			DisableProxyInterceptErrors: false,
+			RelativeRedirects:           false,
 			DenylistSourceRange:         []string{},
 			WhitelistSourceRange:        []string{},
 			SkipAccessLogURLs:           []string{},

internal/ingress/defaults/main.go

@@ -125,6 +125,11 @@ type Backend struct {
 	// Default: false
 	UsePortInRedirects bool `json:"use-port-in-redirects"`
 
+	// Enables or disables relative redirects. By default nginx uses absolute redirects.
+	// http://nginx.org/en/docs/http/ngx_http_core_module.html#absolute_redirect
+	// Default: false
+	RelativeRedirects bool `json:"relative-redirects"`
+
 	// Enable stickiness by client-server mapping based on a NGINX variable, text or a combination of both.
 	// A consistent hashing method will be used which ensures only a few keys would be remapped to different
 	// servers on upstream group changes

rootfs/etc/nginx/template/nginx.tmpl

@@ -459,6 +459,10 @@ http {
     proxy_intercept_errors on;
     {{ end }}
 
+    {{ if $cfg.RelativeRedirects }}
+    absolute_redirect off;
+    {{ end }}
+
     {{ range $errCode := $cfg.CustomHTTPErrors }}
     error_page {{ $errCode }} = @custom_upstream-default-backend_{{ $errCode }};{{ end }}
 
@@ -1343,6 +1347,10 @@ stream {
             satisfy {{ $location.Satisfy }};
             {{ end }}
 
+            {{ if $location.Redirect.Relative }}
+            absolute_redirect off;
+            {{ end }}
+
             {{/* if a location-specific error override is set, add the proxy_intercept here */}}
             {{ if and $location.CustomHTTPErrors (not $location.DisableProxyInterceptErrors) }}
             # Custom error pages per ingress

test/e2e/annotations/relativeredirects.go

@@ -0,0 +1,107 @@
+/*
+Copyright 2023 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/v2"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+const (
+	relativeRedirectsHostname            = "rr.foo.com"
+	relativeRedirectsRedirectPath        = "/something"
+	relativeRedirectsRelativeRedirectURL = "/new-location"
+)
+
+var _ = framework.DescribeAnnotation("relative-redirects", func() {
+	f := framework.NewDefaultFramework("relative-redirects")
+
+	ginkgo.BeforeEach(func() {
+		f.NewHttpbunDeployment()
+		f.NewEchoDeployment()
+	})
+
+	ginkgo.It("configures Nginx correctly", func() {
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/relative-redirects": "true",
+		}
+
+		ing := framework.NewSingleIngress(relativeRedirectsHostname, "/", relativeRedirectsHostname, f.Namespace, framework.HTTPBunService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		var serverConfig string
+		f.WaitForNginxServer(relativeRedirectsHostname, func(srvCfg string) bool {
+			serverConfig = srvCfg
+			return strings.Contains(serverConfig, fmt.Sprintf("server_name %s", relativeRedirectsHostname))
+		})
+
+		ginkgo.By("turning off absolute_redirect directive")
+		assert.Contains(ginkgo.GinkgoT(), serverConfig, "absolute_redirect off;")
+	})
+
+	ginkgo.It("should respond with absolute URL in Location", func() {
+		absoluteRedirectURL := fmt.Sprintf("http://%s%s", relativeRedirectsHostname, relativeRedirectsRelativeRedirectURL)
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/permanent-redirect": relativeRedirectsRelativeRedirectURL,
+			"nginx.ingress.kubernetes.io/relative-redirects": "false",
+		}
+
+		ginkgo.By("setup ingress")
+		ing := framework.NewSingleIngress(relativeRedirectsHostname, relativeRedirectsRedirectPath, relativeRedirectsHostname, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(relativeRedirectsHostname, func(srvCfg string) bool {
+			return strings.Contains(srvCfg, fmt.Sprintf("server_name %s", relativeRedirectsHostname))
+		})
+
+		ginkgo.By("sending request to redirected URL path")
+		f.HTTPTestClient().
+			GET(relativeRedirectsRedirectPath).
+			WithHeader("Host", relativeRedirectsHostname).
+			Expect().
+			Status(http.StatusMovedPermanently).
+			Header("Location").Equal(absoluteRedirectURL)
+	})
+
+	ginkgo.It("should respond with relative URL in Location", func() {
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/permanent-redirect": relativeRedirectsRelativeRedirectURL,
+			"nginx.ingress.kubernetes.io/relative-redirects": "true",
+		}
+
+		ginkgo.By("setup ingress")
+		ing := framework.NewSingleIngress(relativeRedirectsHostname, relativeRedirectsRedirectPath, relativeRedirectsHostname, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(relativeRedirectsHostname, func(srvCfg string) bool {
+			return strings.Contains(srvCfg, fmt.Sprintf("server_name %s", relativeRedirectsHostname))
+		})
+
+		ginkgo.By("sending request to redirected URL path")
+		f.HTTPTestClient().
+			GET(relativeRedirectsRedirectPath).
+			WithHeader("Host", relativeRedirectsHostname).
+			Expect().
+			Status(http.StatusMovedPermanently).
+			Header("Location").Equal(relativeRedirectsRelativeRedirectURL)
+	})
+})