From 573d175aeb90fb84b777ca4a5493eb35ce9092cc Mon Sep 17 00:00:00 2001 From: salonichf5 <146118978+salonichf5@users.noreply.github.com> Date: Sun, 5 Jan 2025 19:48:37 -0700 Subject: [PATCH] add path field for RequestRedirect Filter --- internal/mode/static/nginx/config/servers.go | 93 +++-- .../mode/static/nginx/config/servers_test.go | 256 ++++++++++--- .../static/nginx/config/validation/common.go | 17 + .../nginx/config/validation/common_test.go | 21 ++ .../nginx/config/validation/http_filters.go | 25 +- .../config/validation/http_filters_test.go | 22 ++ .../mode/static/state/dataplane/convert.go | 1 + .../static/state/dataplane/convert_test.go | 46 +++ internal/mode/static/state/dataplane/types.go | 2 + internal/mode/static/state/graph/httproute.go | 18 +- .../mode/static/state/graph/httproute_test.go | 4 + .../fake_httpfields_validator.go | 74 ++++ .../mode/static/state/validation/validator.go | 1 + .../redirects-and-rewrites.md | 343 ++++++++++++++++-- .../overview/gateway-api-compatibility.md | 2 +- tests/Makefile | 2 +- 16 files changed, 788 insertions(+), 139 deletions(-) diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index d387404f51..49c15bcf3f 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -435,7 +435,10 @@ func updateLocation( location.Includes = append(location.Includes, createIncludesFromLocationSnippetsFilters(filters.SnippetsFilters)...) if filters.RequestRedirect != nil { - ret := createReturnValForRedirectFilter(filters.RequestRedirect, listenerPort) + ret, rewrite := createReturnValForRedirectFilter(filters.RequestRedirect, listenerPort, path) + if rewrite.MainRewrite != "" { + location.Rewrites = append(location.Rewrites, rewrite.MainRewrite) + } location.Return = ret return location } @@ -543,9 +546,13 @@ func createProxySSLVerify(v *dataplane.VerifyTLS) *http.ProxySSLVerify { } } -func createReturnValForRedirectFilter(filter *dataplane.HTTPRequestRedirectFilter, listenerPort int32) *http.Return { +func createReturnValForRedirectFilter( + filter *dataplane.HTTPRequestRedirectFilter, + listenerPort int32, + path string, +) (*http.Return, *rewriteConfig) { if filter == nil { - return nil + return nil, nil } hostname := "$host" @@ -582,10 +589,55 @@ func createReturnValForRedirectFilter(filter *dataplane.HTTPRequestRedirectFilte } } + body := fmt.Sprintf("%s://%s$request_uri", scheme, hostnamePort) + + rewrites := &rewriteConfig{} + if filter.Path != nil { + rewrites.MainRewrite = createMainRewriteForFilters(filter.Path, path) + body = fmt.Sprintf("%s://%s$uri$is_args$args", scheme, hostnamePort) + } + return &http.Return{ Code: code, - Body: fmt.Sprintf("%s://%s$request_uri", scheme, hostnamePort), + Body: body, + }, rewrites +} + +func createMainRewriteForFilters(pathModifier *dataplane.HTTPPathModifier, path string) string { + var mainRewrite string + switch pathModifier.Type { + case dataplane.ReplaceFullPath: + mainRewrite = fmt.Sprintf("^ %s", pathModifier.Replacement) + case dataplane.ReplacePrefixMatch: + filterPrefix := pathModifier.Replacement + if filterPrefix == "" { + filterPrefix = "/" + } + + // capture everything following the configured prefix up to the first ?, if present. + regex := fmt.Sprintf("^%s([^?]*)?", path) + // replace the configured prefix with the filter prefix, append the captured segment, + // and include the request arguments stored in nginx variable $args. + // https://nginx.org/en/docs/http/ngx_http_core_module.html#var_args + replacement := fmt.Sprintf("%s$1?$args?", filterPrefix) + + // if configured prefix does not end in /, but replacement prefix does end in /, + // then make sure that we *require* but *don't capture* a trailing slash in the request, + // otherwise we'll get duplicate slashes in the full replacement + if strings.HasSuffix(filterPrefix, "/") && !strings.HasSuffix(path, "/") { + regex = fmt.Sprintf("^%s(?:/([^?]*))?", path) + } + + // if configured prefix ends in / we won't capture it for a request (since it's not in the regex), + // so append it to the replacement prefix if the replacement prefix doesn't already end in / + if strings.HasSuffix(path, "/") && !strings.HasSuffix(filterPrefix, "/") { + replacement = fmt.Sprintf("%s/$1?$args?", filterPrefix) + } + + mainRewrite = fmt.Sprintf("%s %s", regex, replacement) } + + return mainRewrite } func createRewritesValForRewriteFilter(filter *dataplane.HTTPURLRewriteFilter, path string) *rewriteConfig { @@ -594,40 +646,11 @@ func createRewritesValForRewriteFilter(filter *dataplane.HTTPURLRewriteFilter, p } rewrites := &rewriteConfig{} - if filter.Path != nil { rewrites.InternalRewrite = "^ $request_uri" - switch filter.Path.Type { - case dataplane.ReplaceFullPath: - rewrites.MainRewrite = fmt.Sprintf("^ %s break", filter.Path.Replacement) - case dataplane.ReplacePrefixMatch: - filterPrefix := filter.Path.Replacement - if filterPrefix == "" { - filterPrefix = "/" - } - // capture everything following the configured prefix up to the first ?, if present. - regex := fmt.Sprintf("^%s([^?]*)?", path) - // replace the configured prefix with the filter prefix, append the captured segment, - // and include the request arguments stored in nginx variable $args. - // https://nginx.org/en/docs/http/ngx_http_core_module.html#var_args - replacement := fmt.Sprintf("%s$1?$args?", filterPrefix) - - // if configured prefix does not end in /, but replacement prefix does end in /, - // then make sure that we *require* but *don't capture* a trailing slash in the request, - // otherwise we'll get duplicate slashes in the full replacement - if strings.HasSuffix(filterPrefix, "/") && !strings.HasSuffix(path, "/") { - regex = fmt.Sprintf("^%s(?:/([^?]*))?", path) - } - - // if configured prefix ends in / we won't capture it for a request (since it's not in the regex), - // so append it to the replacement prefix if the replacement prefix doesn't already end in / - if strings.HasSuffix(path, "/") && !strings.HasSuffix(filterPrefix, "/") { - replacement = fmt.Sprintf("%s/$1?$args?", filterPrefix) - } - - rewrites.MainRewrite = fmt.Sprintf("%s %s break", regex, replacement) - } + // for URLRewriteFilter, we add a break to the rewrite to prevent further processing of the request. + rewrites.MainRewrite = fmt.Sprintf("%s break", createMainRewriteForFilters(filter.Path, path)) } return rewrites diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index 1a92090a26..803f2206ac 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -2259,53 +2259,82 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { const listenerPortHTTP = 80 const listenerPortHTTPS = 443 + createBasicHTTPRequestRedirectFilter := func() *dataplane.HTTPRequestRedirectFilter { + return &dataplane.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), + Hostname: helpers.GetPointer("foo.example.com"), + StatusCode: helpers.GetPointer(301), + } + } + + modifiedHTTPRequestRedirectFilter := func( + mod func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + return mod(createBasicHTTPRequestRedirectFilter()) + } + tests := []struct { - filter *dataplane.HTTPRequestRedirectFilter - expected *http.Return - msg string - listenerPort int32 + filter *dataplane.HTTPRequestRedirectFilter + expectedReturn *http.Return + expectedRewrite *rewriteConfig + msg string + path string + listenerPort int32 }{ { - filter: nil, - expected: nil, - listenerPort: listenerPortCustom, - msg: "filter is nil", + filter: nil, + expectedReturn: nil, + listenerPort: listenerPortCustom, + msg: "filter is nil", }, { filter: &dataplane.HTTPRequestRedirectFilter{}, listenerPort: listenerPortCustom, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: http.StatusFound, Body: "$scheme://$host:123$request_uri", }, - msg: "all fields are empty", + expectedRewrite: &rewriteConfig{}, + msg: "all fields are empty", }, { - filter: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("https"), - Hostname: helpers.GetPointer("foo.example.com"), - Port: helpers.GetPointer[int32](2022), - StatusCode: helpers.GetPointer(301), - }, + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Scheme = helpers.GetPointer("https") + filter.Port = helpers.GetPointer[int32](2022) + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplaceFullPath, + Replacement: "/full-path", + } + return filter + }), listenerPort: listenerPortCustom, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, - Body: "https://foo.example.com:2022$request_uri", + Body: "https://foo.example.com:2022$uri$is_args$args", + }, + expectedRewrite: &rewriteConfig{ + MainRewrite: "^ /full-path", }, msg: "all fields are set", }, { - filter: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("https"), - Hostname: helpers.GetPointer("foo.example.com"), - StatusCode: helpers.GetPointer(301), - }, + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Scheme = helpers.GetPointer("https") + return filter + }), listenerPort: listenerPortCustom, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, Body: "https://foo.example.com$request_uri", }, - msg: "listenerPort is custom, scheme is set, no port", + expectedRewrite: &rewriteConfig{}, + msg: "listenerPort is custom, scheme is set, no port", }, { filter: &dataplane.HTTPRequestRedirectFilter{ @@ -2313,65 +2342,173 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { StatusCode: helpers.GetPointer(301), }, listenerPort: listenerPortHTTPS, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, Body: "$scheme://foo.example.com:443$request_uri", }, - msg: "no scheme, listenerPort https, no port is set", + expectedRewrite: &rewriteConfig{}, + msg: "no scheme, listenerPort https, no port is set", }, { - filter: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("https"), - Hostname: helpers.GetPointer("foo.example.com"), - StatusCode: helpers.GetPointer(301), - }, + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Scheme = helpers.GetPointer("https") + return filter + }), listenerPort: listenerPortHTTPS, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, Body: "https://foo.example.com$request_uri", }, - msg: "scheme is https, listenerPort https, no port is set", + expectedRewrite: &rewriteConfig{}, + msg: "scheme is https, listenerPort https, no port is set", }, { - filter: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("http"), - Hostname: helpers.GetPointer("foo.example.com"), - StatusCode: helpers.GetPointer(301), - }, + filter: createBasicHTTPRequestRedirectFilter(), listenerPort: listenerPortHTTP, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, Body: "http://foo.example.com$request_uri", }, - msg: "scheme is http, listenerPort http, no port is set", + expectedRewrite: &rewriteConfig{}, + msg: "scheme is http, listenerPort http, no port is set", }, { - filter: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("http"), - Hostname: helpers.GetPointer("foo.example.com"), - Port: helpers.GetPointer[int32](80), - StatusCode: helpers.GetPointer(301), + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Port = helpers.GetPointer[int32](80) + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplacePrefixMatch, + Replacement: "", + } + return filter + }), + path: "/original", + listenerPort: listenerPortCustom, + expectedReturn: &http.Return{ + Code: 301, + Body: "http://foo.example.com$uri$is_args$args", }, + expectedRewrite: &rewriteConfig{ + MainRewrite: "^/original(?:/([^?]*))? /$1?$args?", + }, + msg: "scheme is http, port http, prefix path is empty", + }, + { + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Port = helpers.GetPointer[int32](443) + filter.Scheme = helpers.GetPointer("https") + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplacePrefixMatch, + Replacement: "/", + } + return filter + }), + path: "/original", listenerPort: listenerPortCustom, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, - Body: "http://foo.example.com$request_uri", + Body: "https://foo.example.com$uri$is_args$args", + }, + expectedRewrite: &rewriteConfig{ + MainRewrite: "^/original(?:/([^?]*))? /$1?$args?", }, - msg: "scheme is http, port http", + msg: "scheme is https, port https and prefix path is /", }, { - filter: &dataplane.HTTPRequestRedirectFilter{ - Scheme: helpers.GetPointer("https"), - Hostname: helpers.GetPointer("foo.example.com"), - Port: helpers.GetPointer[int32](443), - StatusCode: helpers.GetPointer(301), + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Port = helpers.GetPointer[int32](80) + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplacePrefixMatch, + Replacement: "/prefix-path", + } + return filter + }), + path: "/original", + listenerPort: listenerPortCustom, + expectedReturn: &http.Return{ + Code: 301, + Body: "http://foo.example.com$uri$is_args$args", + }, + expectedRewrite: &rewriteConfig{ + MainRewrite: "^/original([^?]*)? /prefix-path$1?$args?", + }, + msg: "scheme is http, port http, prefix path with no trailing slashes", + }, + { + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Port = helpers.GetPointer[int32](80) + filter.StatusCode = helpers.GetPointer(302) + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplacePrefixMatch, + Replacement: "/trailing/", + } + return filter + }), + path: "/original", + listenerPort: listenerPortCustom, + expectedReturn: &http.Return{ + Code: 302, + Body: "http://foo.example.com$uri$is_args$args", + }, + expectedRewrite: &rewriteConfig{ + MainRewrite: "^/original(?:/([^?]*))? /trailing/$1?$args?", }, + msg: "scheme is http, port http, prefix path replacement with trailing /", + }, + { + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Port = helpers.GetPointer[int32](80) + filter.StatusCode = helpers.GetPointer(301) + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplacePrefixMatch, + Replacement: "/trailing", + } + return filter + }), + path: "/original/", listenerPort: listenerPortCustom, - expected: &http.Return{ + expectedReturn: &http.Return{ Code: 301, - Body: "https://foo.example.com$request_uri", + Body: "http://foo.example.com$uri$is_args$args", }, - msg: "scheme is https, port https", + expectedRewrite: &rewriteConfig{ + MainRewrite: "^/original/([^?]*)? /trailing/$1?$args?", + }, + msg: "scheme is http, port http, prefix path original with trailing /", + }, + { + filter: modifiedHTTPRequestRedirectFilter(func( + filter *dataplane.HTTPRequestRedirectFilter, + ) *dataplane.HTTPRequestRedirectFilter { + filter.Port = helpers.GetPointer[int32](80) + filter.StatusCode = helpers.GetPointer(301) + filter.Path = &dataplane.HTTPPathModifier{ + Type: dataplane.ReplacePrefixMatch, + Replacement: "/trailing/", + } + return filter + }), + path: "/original/", + listenerPort: listenerPortCustom, + expectedReturn: &http.Return{ + Code: 301, + Body: "http://foo.example.com$uri$is_args$args", + }, + expectedRewrite: &rewriteConfig{ + MainRewrite: "^/original/([^?]*)? /trailing/$1?$args?", + }, + msg: "scheme is http, port http, prefix path both with trailing slashes", }, } @@ -2380,8 +2517,9 @@ func TestCreateReturnValForRedirectFilter(t *testing.T) { t.Parallel() g := NewWithT(t) - result := createReturnValForRedirectFilter(test.filter, test.listenerPort) - g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + result, rewriteConfig := createReturnValForRedirectFilter(test.filter, test.listenerPort, test.path) + g.Expect(helpers.Diff(test.expectedReturn, result)).To(BeEmpty()) + g.Expect(helpers.Diff(test.expectedRewrite, rewriteConfig)).To(BeEmpty()) }) } } diff --git a/internal/mode/static/nginx/config/validation/common.go b/internal/mode/static/nginx/config/validation/common.go index bca6a97095..ec2eb2d1e2 100644 --- a/internal/mode/static/nginx/config/validation/common.go +++ b/internal/mode/static/nginx/config/validation/common.go @@ -84,3 +84,20 @@ func validateHeaderName(name string) error { } return nil } + +func validatePath(path string) error { + if path == "" { + return nil + } + + if !pathRegexp.MatchString(path) { + msg := k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...) + return errors.New(msg) + } + + if strings.Contains(path, "$") { + return errors.New("cannot contain $") + } + + return nil +} diff --git a/internal/mode/static/nginx/config/validation/common_test.go b/internal/mode/static/nginx/config/validation/common_test.go index 33cdd0ee5e..a9c31febf8 100644 --- a/internal/mode/static/nginx/config/validation/common_test.go +++ b/internal/mode/static/nginx/config/validation/common_test.go @@ -74,3 +74,24 @@ func TestValidateValidHeaderName(t *testing.T) { strings.Repeat("very-long-header", 16)+"1", ) } + +func TestValidatePath(t *testing.T) { + t.Parallel() + validator := validatePath + + testValidValuesForSimpleValidator( + t, + validator, + `/path`, + `/longer/path`, + `/trailing/`, + ) + + testInvalidValuesForSimpleValidator( + t, + validator, + `path`, + `$path`, + "/path$", + ) +} diff --git a/internal/mode/static/nginx/config/validation/http_filters.go b/internal/mode/static/nginx/config/validation/http_filters.go index 0a8de17957..19bcd530ab 100644 --- a/internal/mode/static/nginx/config/validation/http_filters.go +++ b/internal/mode/static/nginx/config/validation/http_filters.go @@ -1,12 +1,5 @@ package validation -import ( - "errors" - "strings" - - k8svalidation "k8s.io/apimachinery/pkg/util/validation" -) - // HTTPRedirectValidator validates values for a redirect, which in NGINX is done with the return directive. // For example, return 302 "https://example.com:8080"; type HTTPRedirectValidator struct{} @@ -56,20 +49,12 @@ func (HTTPRedirectValidator) ValidateHostname(hostname string) error { // ValidateRewritePath validates a path used in a URL Rewrite filter. func (HTTPURLRewriteValidator) ValidateRewritePath(path string) error { - if path == "" { - return nil - } - - if !pathRegexp.MatchString(path) { - msg := k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...) - return errors.New(msg) - } - - if strings.Contains(path, "$") { - return errors.New("cannot contain $") - } + return validatePath(path) +} - return nil +// ValidateRedirectPath validates a path used in a Request Redirect filter. +func (HTTPRedirectValidator) ValidateRedirectPath(path string) error { + return validatePath(path) } func (HTTPHeaderValidator) ValidateFilterHeaderName(name string) error { diff --git a/internal/mode/static/nginx/config/validation/http_filters_test.go b/internal/mode/static/nginx/config/validation/http_filters_test.go index bd06ed2b41..03127cbb7c 100644 --- a/internal/mode/static/nginx/config/validation/http_filters_test.go +++ b/internal/mode/static/nginx/config/validation/http_filters_test.go @@ -93,6 +93,28 @@ func TestValidateRewritePath(t *testing.T) { ) } +func TestValidateRedirectPath(t *testing.T) { + t.Parallel() + validator := HTTPRedirectValidator{} + + testValidValuesForSimpleValidator( + t, + validator.ValidateRedirectPath, + "", + "/path", + "/longer/path", + "/trailing/", + ) + + testInvalidValuesForSimpleValidator( + t, + validator.ValidateRedirectPath, + "path", + "$path", + "/path$", + ) +} + func TestValidateFilterHeaderName(t *testing.T) { t.Parallel() validator := HTTPHeaderValidator{} diff --git a/internal/mode/static/state/dataplane/convert.go b/internal/mode/static/state/dataplane/convert.go index 626e5c9030..52b558dde4 100644 --- a/internal/mode/static/state/dataplane/convert.go +++ b/internal/mode/static/state/dataplane/convert.go @@ -47,6 +47,7 @@ func convertHTTPRequestRedirectFilter(filter *v1.HTTPRequestRedirectFilter) *HTT Hostname: (*string)(filter.Hostname), Port: (*int32)(filter.Port), StatusCode: filter.StatusCode, + Path: convertPathModifier(filter.Path), } } diff --git a/internal/mode/static/state/dataplane/convert_test.go b/internal/mode/static/state/dataplane/convert_test.go index 3ab1e083f1..417c50898d 100644 --- a/internal/mode/static/state/dataplane/convert_test.go +++ b/internal/mode/static/state/dataplane/convert_test.go @@ -137,6 +137,52 @@ func TestConvertHTTPRequestRedirectFilter(t *testing.T) { expected: &HTTPRequestRedirectFilter{}, name: "empty", }, + { + filter: &v1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), + Hostname: helpers.GetPointer[v1.PreciseHostname]("example.com"), + Port: helpers.GetPointer[v1.PortNumber](8080), + StatusCode: helpers.GetPointer(302), + Path: &v1.HTTPPathModifier{ + Type: v1.FullPathHTTPPathModifier, + ReplaceFullPath: helpers.GetPointer("/path"), + }, + }, + expected: &HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("http"), + Hostname: helpers.GetPointer("example.com"), + Port: helpers.GetPointer[int32](8080), + StatusCode: helpers.GetPointer(302), + Path: &HTTPPathModifier{ + Type: ReplaceFullPath, + Replacement: "/path", + }, + }, + name: "request redirect with ReplaceFullPath modifier", + }, + { + filter: &v1.HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("https"), + Hostname: helpers.GetPointer[v1.PreciseHostname]("example.com"), + Port: helpers.GetPointer[v1.PortNumber](8443), + StatusCode: helpers.GetPointer(302), + Path: &v1.HTTPPathModifier{ + Type: v1.PrefixMatchHTTPPathModifier, + ReplacePrefixMatch: helpers.GetPointer("/prefix"), + }, + }, + expected: &HTTPRequestRedirectFilter{ + Scheme: helpers.GetPointer("https"), + Hostname: helpers.GetPointer("example.com"), + Port: helpers.GetPointer[int32](8443), + StatusCode: helpers.GetPointer(302), + Path: &HTTPPathModifier{ + Type: ReplacePrefixMatch, + Replacement: "/prefix", + }, + }, + name: "request redirect with ReplacePrefixMatch modifier", + }, { filter: &v1.HTTPRequestRedirectFilter{ Scheme: helpers.GetPointer("https"), diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 128990617b..49f61fbe1c 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -193,6 +193,8 @@ type HTTPRequestRedirectFilter struct { Port *int32 // StatusCode is the HTTP status code of the redirect. StatusCode *int + // Path is the path of the redirect. + Path *HTTPPathModifier } // HTTPURLRewriteFilter rewrites HTTP requests. diff --git a/internal/mode/static/state/graph/httproute.go b/internal/mode/static/state/graph/httproute.go index 8cc3b66ea1..f25768701e 100644 --- a/internal/mode/static/state/graph/httproute.go +++ b/internal/mode/static/state/graph/httproute.go @@ -328,8 +328,22 @@ func validateFilterRedirect( } if redirect.Path != nil { - valErr := field.Forbidden(redirectPath.Child("path"), "path is not supported") - allErrs = append(allErrs, valErr) + var path string + switch redirect.Path.Type { + case v1.FullPathHTTPPathModifier: + path = *redirect.Path.ReplaceFullPath + case v1.PrefixMatchHTTPPathModifier: + path = *redirect.Path.ReplacePrefixMatch + default: + msg := fmt.Sprintf("requestRedirect path type %s not supported", redirect.Path.Type) + valErr := field.Invalid(redirectPath.Child("path"), *redirect.Path, msg) + return append(allErrs, valErr) + } + + if err := validator.ValidateRewritePath(path); err != nil { + valErr := field.Invalid(redirectPath.Child("path"), *redirect.Path, err.Error()) + allErrs = append(allErrs, valErr) + } } if redirect.StatusCode != nil { diff --git a/internal/mode/static/state/graph/httproute_test.go b/internal/mode/static/state/graph/httproute_test.go index a655364e60..119a264e23 100644 --- a/internal/mode/static/state/graph/httproute_test.go +++ b/internal/mode/static/state/graph/httproute_test.go @@ -1207,6 +1207,10 @@ func TestValidateFilterRedirect(t *testing.T) { Hostname: helpers.GetPointer[gatewayv1.PreciseHostname]("example.com"), Port: helpers.GetPointer[gatewayv1.PortNumber](80), StatusCode: helpers.GetPointer(301), + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.FullPathHTTPPathModifier, + ReplaceFullPath: helpers.GetPointer("/path"), + }, }, expectErrCount: 0, name: "valid redirect filter", diff --git a/internal/mode/static/state/validation/validationfakes/fake_httpfields_validator.go b/internal/mode/static/state/validation/validationfakes/fake_httpfields_validator.go index 64d9b09349..3f52d1f877 100644 --- a/internal/mode/static/state/validation/validationfakes/fake_httpfields_validator.go +++ b/internal/mode/static/state/validation/validationfakes/fake_httpfields_validator.go @@ -109,6 +109,17 @@ type FakeHTTPFieldsValidator struct { validateQueryParamValueInMatchReturnsOnCall map[int]struct { result1 error } + ValidateRedirectPathStub func(string) error + validateRedirectPathMutex sync.RWMutex + validateRedirectPathArgsForCall []struct { + arg1 string + } + validateRedirectPathReturns struct { + result1 error + } + validateRedirectPathReturnsOnCall map[int]struct { + result1 error + } ValidateRedirectPortStub func(int32) error validateRedirectPortMutex sync.RWMutex validateRedirectPortArgsForCall []struct { @@ -713,6 +724,67 @@ func (fake *FakeHTTPFieldsValidator) ValidateQueryParamValueInMatchReturnsOnCall }{result1} } +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPath(arg1 string) error { + fake.validateRedirectPathMutex.Lock() + ret, specificReturn := fake.validateRedirectPathReturnsOnCall[len(fake.validateRedirectPathArgsForCall)] + fake.validateRedirectPathArgsForCall = append(fake.validateRedirectPathArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.ValidateRedirectPathStub + fakeReturns := fake.validateRedirectPathReturns + fake.recordInvocation("ValidateRedirectPath", []interface{}{arg1}) + fake.validateRedirectPathMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPathCallCount() int { + fake.validateRedirectPathMutex.RLock() + defer fake.validateRedirectPathMutex.RUnlock() + return len(fake.validateRedirectPathArgsForCall) +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPathCalls(stub func(string) error) { + fake.validateRedirectPathMutex.Lock() + defer fake.validateRedirectPathMutex.Unlock() + fake.ValidateRedirectPathStub = stub +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPathArgsForCall(i int) string { + fake.validateRedirectPathMutex.RLock() + defer fake.validateRedirectPathMutex.RUnlock() + argsForCall := fake.validateRedirectPathArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPathReturns(result1 error) { + fake.validateRedirectPathMutex.Lock() + defer fake.validateRedirectPathMutex.Unlock() + fake.ValidateRedirectPathStub = nil + fake.validateRedirectPathReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeHTTPFieldsValidator) ValidateRedirectPathReturnsOnCall(i int, result1 error) { + fake.validateRedirectPathMutex.Lock() + defer fake.validateRedirectPathMutex.Unlock() + fake.ValidateRedirectPathStub = nil + if fake.validateRedirectPathReturnsOnCall == nil { + fake.validateRedirectPathReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.validateRedirectPathReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeHTTPFieldsValidator) ValidateRedirectPort(arg1 int32) error { fake.validateRedirectPortMutex.Lock() ret, specificReturn := fake.validateRedirectPortReturnsOnCall[len(fake.validateRedirectPortArgsForCall)] @@ -984,6 +1056,8 @@ func (fake *FakeHTTPFieldsValidator) Invocations() map[string][][]interface{} { defer fake.validateQueryParamNameInMatchMutex.RUnlock() fake.validateQueryParamValueInMatchMutex.RLock() defer fake.validateQueryParamValueInMatchMutex.RUnlock() + fake.validateRedirectPathMutex.RLock() + defer fake.validateRedirectPathMutex.RUnlock() fake.validateRedirectPortMutex.RLock() defer fake.validateRedirectPortMutex.RUnlock() fake.validateRedirectSchemeMutex.RLock() diff --git a/internal/mode/static/state/validation/validator.go b/internal/mode/static/state/validation/validator.go index c941ed1700..622ae69dfb 100644 --- a/internal/mode/static/state/validation/validator.go +++ b/internal/mode/static/state/validation/validator.go @@ -31,6 +31,7 @@ type HTTPFieldsValidator interface { ValidateRedirectScheme(scheme string) (valid bool, supportedValues []string) ValidateRedirectPort(port int32) error ValidateRedirectStatusCode(statusCode int) (valid bool, supportedValues []string) + ValidateRedirectPath(path string) error ValidateHostname(hostname string) error ValidateRewritePath(path string) error ValidateFilterHeaderName(name string) error diff --git a/site/content/how-to/traffic-management/redirects-and-rewrites.md b/site/content/how-to/traffic-management/redirects-and-rewrites.md index 8785da32aa..01925664e3 100644 --- a/site/content/how-to/traffic-management/redirects-and-rewrites.md +++ b/site/content/how-to/traffic-management/redirects-and-rewrites.md @@ -11,11 +11,11 @@ Learn how to redirect or rewrite your HTTP traffic using NGINX Gateway Fabric. [HTTPRoute](https://gateway-api.sigs.k8s.io/api-types/httproute/) filters can be used to configure HTTP redirects or rewrites. Redirects return HTTP 3XX responses to a client, instructing it to retrieve a different resource. Rewrites modify components of a client request (such as hostname and/or path) before proxying it upstream. -{{< note >}}NGINX Gateway Fabric currently does not support path-based redirects.{{< /note >}} +In this guide, we will set up the coffee application to demonstrate path URL rewriting and the tea and soda applications to showcase path-based request redirection.. For an introduction to exposing your application, we recommend that you follow the [basic guide]({{< relref "/how-to/traffic-management/routing-traffic-to-your-app.md" >}}) first. To see an example of a redirect using scheme and port, see the [HTTPS Termination]({{< relref "/how-to/traffic-management/https-termination.md" >}}) guide. -In this guide, we will be configuring a path URL rewrite. +--- ## Before you begin @@ -29,7 +29,42 @@ In this guide, we will be configuring a path URL rewrite. {{< note >}}In a production environment, you should have a DNS record for the external IP address that is exposed, and it should refer to the hostname that the gateway will forward for.{{< /note >}} -## Set up +--- + +## HTTP rewrites and redirects examples + +We will configure a common gateway for the `URLRewrite` and `RequestRedirect` filter examples mentioned below. + +--- + +### Deploy the Gateway API resources for the applications + +The [Gateway](https://gateway-api.sigs.k8s.io/api-types/gateway/) resource is typically deployed by the [Cluster Operator](https://gateway-api.sigs.k8s.io/concepts/roles-and-personas/#roles-and-personas_1). This Gateway defines a single listener on port 80. Since no hostname is specified, this listener matches on all hostnames. To deploy the Gateway: + +```yaml +kubectl apply -f - < 80/TCP 40s ``` -## Configure a path rewrite - -To create the **cafe** gateway, copy and paste the following into your terminal: +--- -```yaml -kubectl apply -f - < 80/TCP 89m +service/tea ClusterIP 10.96.151.194 80/TCP 120m +``` + +--- + +### Configure a path redirect + +We will define two HTTPRoutes for **tea** application: `tea`, which specifies the destination location block to handle redirected requests, and `tea-redirect` that redirect requests as follows: + +- `http://cafe.example.com/tea` to `http://cafe.example.com/organic` +- `http://cafe.example.com/tea/origin` to `http://cafe.example.com/organic/origin` + +The two HTTPRoutes defined for **soda** application: `soda`, which specifies the destination location block to handle redirected requests, and `soda-redirect` that redirect requests as follows: + +- `http://cafe.example.com/soda` to `http://cafe.example.com/flavors` +- `http://cafe.example.com/soda/pepsi` to `http://cafe.example.com/flavors` + +To create the httproute resource, copy and paste the following into your terminal: + +```yaml +kubectl apply -f - <}}If you have a DNS record allocated for `cafe.example.com`, you can send the request directly to that hostname, without needing to resolve.{{< /note >}} + +```shell +curl -L --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/tea --include +``` + +Notice in the output that the URI has been redirected to the new location: + +```text +HTTP/1.1 302 Moved Temporarily +.. +Location: http://cafe.example.com:8080/organic +``` + +Other examples: + +```shell +curl -L --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/tea/type --include +``` + +```text +HTTP/1.1 302 Moved Temporarily +.. +Location: http://cafe.example.com:8080/organic/type +``` + +```shell +curl -L --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/tea/type\?test\=v1 --includ +``` + +```text +HTTP/1.1 302 Moved Temporarily +.. +Location: http://cafe.example.com:8080/organic/type?test=v1 +``` + +```shell +curl -L --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/soda --include +``` + +```text +HTTP/1.1 302 Moved Temporarily +.. +Location: http://cafe.example.com:8080/flavors +``` + +```shell +curl -L --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/soda/pepsi --include +``` + +```text +HTTP/1.1 302 Moved Temporarily +.. +Location: http://cafe.example.com:8080/flavors +``` + +```shell +curl -L --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/soda/pepsi\?test\=v1 --include +``` + +```text +HTTP/1.1 302 Moved Temporarily +.. +Location: http://cafe.example.com:8080/flavors?test=v1 +``` + +--- + ## Further reading To learn more about redirects and rewrites using the Gateway API, see the following resource: diff --git a/site/content/overview/gateway-api-compatibility.md b/site/content/overview/gateway-api-compatibility.md index b0a344e0a6..7573597983 100644 --- a/site/content/overview/gateway-api-compatibility.md +++ b/site/content/overview/gateway-api-compatibility.md @@ -166,7 +166,7 @@ See the [static-mode]({{< relref "/reference/cli-help.md#static-mode">}}) comman - `method`: Supported. - `filters` - `type`: Supported. - - `requestRedirect`: Supported except for the experimental `path` field. If multiple filters are configured, NGINX Gateway Fabric will choose the first and ignore the rest. Incompatible with `urlRewrite`. + - `requestRedirect`: Supported. If multiple filters are configured, NGINX Gateway Fabric will choose the first and ignore the rest. Incompatible with `urlRewrite`. - `requestHeaderModifier`: Supported. If multiple filters are configured, NGINX Gateway Fabric will choose the first and ignore the rest. - `urlRewrite`: Supported. If multiple filters are configured, NGINX Gateway Fabric will choose the first and ignore the rest. Incompatible with `requestRedirect`. - `responseHeaderModifier`: Supported. If multiple filters are configured, NGINX Gateway Fabric will choose the first and ignore the rest. diff --git a/tests/Makefile b/tests/Makefile index a68ab7e120..004c073a09 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -14,7 +14,7 @@ NGF_VERSION ?= edge## NGF version to be tested PULL_POLICY = Never## Pull policy for the images NGINX_CONF_DIR = internal/mode/static/nginx/conf PROVISIONER_MANIFEST = conformance/provisioner/provisioner.yaml -SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification +SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles.