Support for URL Rewrite filter (#1396)

Problem: As a user, I want to be able to configure URL rewrites for hostname and/or path rewrites on the server side.

Solution: Using the HTTPRoute filter API, a user can now configure a hostname and/or path-based rewrite. Hostname rewrite will update the Host header, while a path rewrite utilizes nginx's `rewrite` directive.

Enabled conformance tests for these features and added how-to guides.

Author: sjberman

conformance/Makefile

@@ -3,7 +3,7 @@ PREFIX = nginx-gateway-fabric
 NGINX_PREFIX = $(PREFIX)/nginx
 GW_API_VERSION ?= 1.0.0
 GATEWAY_CLASS = nginx
-SUPPORTED_FEATURES = HTTPRoute,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,GatewayClassObservedGenerationBump
+SUPPORTED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080
 KIND_IMAGE ?= $(shell grep -m1 'FROM kindest/node' <tests/Dockerfile | awk -F'[ ]' '{print $$2}')
 KIND_KUBE_CONFIG=$${HOME}/.kube/kind/config
 CONFORMANCE_TAG = latest

internal/mode/static/nginx/config/http/config.go

@@ -13,6 +13,7 @@ type Server struct {
 // Location holds all configuration for an HTTP location.
 type Location struct {
 	Return          *Return
+	Rewrites        []string
 	Path            string
 	ProxyPass       string
 	HTTPMatchVar    string

internal/mode/static/nginx/config/servers.go

@@ -18,6 +18,26 @@ const (
 	rootPath             = "/"
 )
 
+// baseHeaders contains the constant headers set in each server block
+var baseHeaders = []http.Header{
+	{
+		Name:  "Host",
+		Value: "$gw_api_compliant_host",
+	},
+	{
+		Name:  "X-Forwarded-For",
+		Value: "$proxy_add_x_forwarded_for",
+	},
+	{
+		Name:  "Upgrade",
+		Value: "$http_upgrade",
+	},
+	{
+		Name:  "Connection",
+		Value: "$connection_upgrade",
+	},
+}
+
 func executeServers(conf dataplane.Configuration) []byte {
 	servers := createServers(conf.HTTPServers, conf.SSLServers)
 
@@ -72,6 +92,15 @@ func createServer(virtualServer dataplane.VirtualServer) http.Server {
 	}
 }
 
+// rewriteConfig contains the configuration for a location to rewrite paths,
+// as specified in a URLRewrite filter
+type rewriteConfig struct {
+	// InternalRewrite rewrites an internal URI to the original URI (ex: /coffee_prefix_route0 -> /coffee)
+	InternalRewrite string
+	// MainRewrite rewrites the original URI to the new URI (ex: /coffee -> /beans)
+	MainRewrite string
+}
+
 func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.Location {
 	maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(pathRules)
 	locs := make([]http.Location, 0, maxLocs)
@@ -94,42 +123,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.
 				matches = append(matches, match)
 			}
 
-			if r.Filters.InvalidFilter != nil {
-				for i := range buildLocations {
-					buildLocations[i].Return = &http.Return{Code: http.StatusInternalServerError}
-				}
-				locs = append(locs, buildLocations...)
-				continue
-			}
-
-			// There could be a case when the filter has the type set but not the corresponding field.
-			// For example, type is v1.HTTPRouteFilterRequestRedirect, but RequestRedirect field is nil.
-			// The imported Webhook validation webhook catches that.
-
-			// FIXME(pleshakov): Ensure dataplane.Configuration -related types don't include v1 types, so that
-			// we don't need to make any assumptions like above here. After fixing this, ensure that there is a test
-			// for checking the imported Webhook validation catches the case above.
-			// https://github.com/nginxinc/nginx-gateway-fabric/issues/660
-
-			// RequestRedirect and proxying are mutually exclusive.
-			if r.Filters.RequestRedirect != nil {
-				ret := createReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort)
-				for i := range buildLocations {
-					buildLocations[i].Return = ret
-				}
-				locs = append(locs, buildLocations...)
-				continue
-			}
-
-			proxySetHeaders := generateProxySetHeaders(r.Filters.RequestHeaderModifiers)
-			for i := range buildLocations {
-				buildLocations[i].ProxySetHeaders = proxySetHeaders
-			}
-
-			proxyPass := createProxyPass(r.BackendGroup)
-			for i := range buildLocations {
-				buildLocations[i].ProxyPass = proxyPass
-			}
+			buildLocations = updateLocationsForFilters(r.Filters, buildLocations, r, listenerPort, rule.Path)
 			locs = append(locs, buildLocations...)
 		}
 
@@ -230,6 +224,48 @@ func initializeInternalLocation(
 	return createMatchLocation(path), createHTTPMatch(match, path)
 }
 
+// updateLocationsForFilters updates the existing locations with any relevant filters.
+func updateLocationsForFilters(
+	filters dataplane.HTTPFilters,
+	buildLocations []http.Location,
+	matchRule dataplane.MatchRule,
+	listenerPort int32,
+	path string,
+) []http.Location {
+	if filters.InvalidFilter != nil {
+		for i := range buildLocations {
+			buildLocations[i].Return = &http.Return{Code: http.StatusInternalServerError}
+		}
+		return buildLocations
+	}
+
+	if filters.RequestRedirect != nil {
+		ret := createReturnValForRedirectFilter(filters.RequestRedirect, listenerPort)
+		for i := range buildLocations {
+			buildLocations[i].Return = ret
+		}
+		return buildLocations
+	}
+
+	rewrites := createRewritesValForRewriteFilter(filters.RequestURLRewrite, path)
+	proxySetHeaders := generateProxySetHeaders(&matchRule.Filters)
+	proxyPass := createProxyPass(matchRule.BackendGroup, matchRule.Filters.RequestURLRewrite)
+	for i := range buildLocations {
+		if rewrites != nil {
+			if buildLocations[i].Internal && rewrites.InternalRewrite != "" {
+				buildLocations[i].Rewrites = append(buildLocations[i].Rewrites, rewrites.InternalRewrite)
+			}
+			if rewrites.MainRewrite != "" {
+				buildLocations[i].Rewrites = append(buildLocations[i].Rewrites, rewrites.MainRewrite)
+			}
+		}
+		buildLocations[i].ProxySetHeaders = proxySetHeaders
+		buildLocations[i].ProxyPass = proxyPass
+	}
+
+	return buildLocations
+}
+
 func createReturnValForRedirectFilter(filter *dataplane.HTTPRequestRedirectFilter, listenerPort int32) *http.Return {
 	if filter == nil {
 		return nil
@@ -275,6 +311,49 @@ func createReturnValForRedirectFilter(filter *dataplane.HTTPRequestRedirectFilte
 	}
 }
 
+func createRewritesValForRewriteFilter(filter *dataplane.HTTPURLRewriteFilter, path string) *rewriteConfig {
+	if filter == nil {
+		return nil
+	}
+
+	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 after the configured prefix
+			regex := fmt.Sprintf("^%s(.*)$", path)
+			// replace the configured prefix with the filter prefix and append what was captured
+			replacement := fmt.Sprintf("%s$1", 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", filterPrefix)
+			}
+
+			rewrites.MainRewrite = fmt.Sprintf("%s %s break", regex, replacement)
+		}
+	}
+
+	return rewrites
+}
+
 // httpMatch is an internal representation of an HTTPRouteMatch.
 // This struct is marshaled into a string and stored as a variable in the nginx location block for the route's path.
 // The NJS httpmatches module will look up this variable on the request object and compare the request against the
@@ -354,13 +433,18 @@ func isPathOnlyMatch(match dataplane.Match) bool {
 	return match.Method == nil && len(match.Headers) == 0 && len(match.QueryParams) == 0
 }
 
-func createProxyPass(backendGroup dataplane.BackendGroup) string {
+func createProxyPass(backendGroup dataplane.BackendGroup, filter *dataplane.HTTPURLRewriteFilter) string {
+	var requestURI string
+	if filter == nil || filter.Path == nil {
+		requestURI = "$request_uri"
+	}
+
 	backendName := backendGroupName(backendGroup)
 	if backendGroupNeedsSplit(backendGroup) {
-		return "http://$" + convertStringToSafeVariableName(backendName)
+		return "http://$" + convertStringToSafeVariableName(backendName) + requestURI
 	}
 
-	return "http://" + backendName
+	return "http://" + backendName + requestURI
 }
 
 func createMatchLocation(path string) http.Location {
@@ -370,27 +454,44 @@ func createMatchLocation(path string) http.Location {
 	}
 }
 
-func generateProxySetHeaders(filters *dataplane.HTTPHeaderFilter) []http.Header {
-	if filters == nil {
-		return nil
+func generateProxySetHeaders(filters *dataplane.HTTPFilters) []http.Header {
+	headers := make([]http.Header, len(baseHeaders))
+	copy(headers, baseHeaders)
+
+	if filters != nil && filters.RequestURLRewrite != nil && filters.RequestURLRewrite.Hostname != nil {
+		for i, header := range headers {
+			if header.Name == "Host" {
+				headers[i].Value = *filters.RequestURLRewrite.Hostname
+				break
+			}
+		}
+	}
+
+	if filters == nil || filters.RequestHeaderModifiers == nil {
+		return headers
 	}
-	proxySetHeaders := make([]http.Header, 0, len(filters.Add)+len(filters.Set)+len(filters.Remove))
-	if len(filters.Add) > 0 {
-		addHeaders := convertAddHeaders(filters.Add)
+
+	headerFilter := filters.RequestHeaderModifiers
+
+	headerLen := len(headerFilter.Add) + len(headerFilter.Set) + len(headerFilter.Remove) + len(headers)
+	proxySetHeaders := make([]http.Header, 0, headerLen)
+	if len(headerFilter.Add) > 0 {
+		addHeaders := convertAddHeaders(headerFilter.Add)
 		proxySetHeaders = append(proxySetHeaders, addHeaders...)
 	}
-	if len(filters.Set) > 0 {
-		setHeaders := convertSetHeaders(filters.Set)
+	if len(headerFilter.Set) > 0 {
+		setHeaders := convertSetHeaders(headerFilter.Set)
 		proxySetHeaders = append(proxySetHeaders, setHeaders...)
 	}
 	// If the value of a header field is an empty string then this field will not be passed to a proxied server
-	for _, h := range filters.Remove {
+	for _, h := range headerFilter.Remove {
 		proxySetHeaders = append(proxySetHeaders, http.Header{
 			Name:  h,
 			Value: "",
 		})
 	}
-	return proxySetHeaders
+
+	return append(proxySetHeaders, headers...)
 }
 
 func convertAddHeaders(headers []dataplane.HTTPHeader) []http.Header {

internal/mode/static/nginx/config/servers_template.go

@@ -37,6 +37,10 @@ server {
         internal;
         {{ end }}
 
+        {{- range $r := $l.Rewrites }}
+        rewrite {{ $r }};
+        {{- end }}
+
         {{- if $l.Return -}}
         return {{ $l.Return.Code }} "{{ $l.Return.Body }}";
         {{ end }}
@@ -50,12 +54,8 @@ server {
             {{ range $h := $l.ProxySetHeaders }}
         proxy_set_header {{ $h.Name }} "{{ $h.Value }}";
             {{- end }}
-        proxy_set_header Host $gw_api_compliant_host;
-        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
         proxy_http_version 1.1;
-        proxy_set_header Upgrade $http_upgrade;
-        proxy_set_header Connection $connection_upgrade;
-        proxy_pass {{ $l.ProxyPass }}$request_uri;
+        proxy_pass {{ $l.ProxyPass }};
         {{- end }}
     }
         {{ end }}