feat(translator): implement httproutefilter host rewrite (#4446)

* implement httproutefilter host rewrite Signed-off-by: Guy Daich <guy.daich@sap.com> * review fixes Signed-off-by: Guy Daich <guy.daich@sap.com> * fix gen Signed-off-by: Guy Daich <guy.daich@sap.com> * fix comment Signed-off-by: Guy Daich <guy.daich@sap.com> --------- Signed-off-by: Guy Daich <guy.daich@sap.com> Co-authored-by: zirain <zirain2009@gmail.com>
envoyproxy · Oct 23, 2024 · 8b8884d · 8b8884d
1 parent d9dd4e6
commit 8b8884d
Show file tree

Hide file tree

Showing 25 changed files with 1,736 additions and 138 deletions.
diff --git a/api/v1alpha1/httproutefilter_types.go b/api/v1alpha1/httproutefilter_types.go
@@ -43,7 +43,6 @@ type HTTPURLRewriteFilter struct {
 	// forwarding.
 	//
 	// +optional
-	// +notImplementedHide
 	Hostname *HTTPHostnameModifier `json:"hostname,omitempty"`
 	// Path defines a path rewrite.
 	//
@@ -83,12 +82,12 @@ const (
 type HTTPHostnameModifierType string
 
 const (
-	// HeaderHTTPHostnameModifier indicates that the Host header value would be replaced with the value of the header specified in setFromHeader.
+	// HeaderHTTPHostnameModifier indicates that the Host header value would be replaced with the value of the header specified in header.
 	// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header
-	HeaderHTTPHostnameModifier HTTPHostnameModifierType = "SetFromHeader"
+	HeaderHTTPHostnameModifier HTTPHostnameModifierType = "Header"
 	// BackendHTTPHostnameModifier indicates that the Host header value would be replaced by the DNS name of the backend if it exists.
 	// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-auto-host-rewrite
-	BackendHTTPHostnameModifier HTTPHostnameModifierType = "SetFromBackend"
+	BackendHTTPHostnameModifier HTTPHostnameModifierType = "Backend"
 )
 
 type ReplaceRegexMatch struct {
@@ -129,16 +128,16 @@ type HTTPPathModifier struct {
 	ReplaceRegexMatch *ReplaceRegexMatch `json:"replaceRegexMatch,omitempty"`
 }
 
-// +kubebuilder:validation:XValidation:message="setFromHeader must be nil if the type is not SetFromHeader",rule="!(has(self.setFromHeader) && self.type != 'SetFromHeader')"
-// +kubebuilder:validation:XValidation:message="setFromHeader must be specified for SetFromHeader type",rule="!(!has(self.setFromHeader) && self.type == 'SetFromHeader')"
+// +kubebuilder:validation:XValidation:message="header must be nil if the type is not Header",rule="!(has(self.header) && self.type != 'Header')"
+// +kubebuilder:validation:XValidation:message="header must be specified for Header type",rule="!(!has(self.header) && self.type == 'Header')"
 type HTTPHostnameModifier struct {
-	// +kubebuilder:validation:Enum=SetFromHeader;SetFromBackend
+	// +kubebuilder:validation:Enum=Header;Backend
 	// +kubebuilder:validation:Required
 	Type HTTPHostnameModifierType `json:"type"`
 
-	// SetFromHeader is the name of the header whose value would be used to rewrite the Host header
+	// Header is the name of the header whose value would be used to rewrite the Host header
 	// +optional
-	SetFromHeader *string `json:"setFromHeader,omitempty"`
+	Header *string `json:"header,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_httproutefilters.yaml
@@ -137,25 +137,25 @@ spec:
                       Hostname is the value to be used to replace the Host header value during
                       forwarding.
                     properties:
-                      setFromHeader:
-                        description: SetFromHeader is the name of the header whose
-                          value would be used to rewrite the Host header
+                      header:
+                        description: Header is the name of the header whose value
+                          would be used to rewrite the Host header
                         type: string
                       type:
                         description: HTTPPathModifierType defines the type of Hostname
                           rewrite.
                         enum:
-                        - SetFromHeader
-                        - SetFromBackend
+                        - Header
+                        - Backend
                         type: string
                     required:
                     - type
                     type: object
                     x-kubernetes-validations:
-                    - message: setFromHeader must be nil if the type is not SetFromHeader
-                      rule: '!(has(self.setFromHeader) && self.type != ''SetFromHeader'')'
-                    - message: setFromHeader must be specified for SetFromHeader type
-                      rule: '!(!has(self.setFromHeader) && self.type == ''SetFromHeader'')'
+                    - message: header must be nil if the type is not Header
+                      rule: '!(has(self.header) && self.type != ''Header'')'
+                    - message: header must be specified for Header type
+                      rule: '!(!has(self.header) && self.type == ''Header'')'
                   path:
                     description: Path defines a path rewrite.
                     properties:

diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go
@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
@@ -147,14 +148,46 @@ func (t *Translator) ProcessGRPCFilters(parentRef *RouteParentContext,
 	return httpFiltersContext
 }
 
+// Checks if the context and the rewrite both contain a core gw-api HTTP URL rewrite
+func hasMultipleCoreRewrites(rewrite *gwapiv1.HTTPURLRewriteFilter, contextRewrite *ir.URLRewrite) bool {
+	contextHasCoreRewrites := contextRewrite.Path != nil && (contextRewrite.Path.FullReplace != nil ||
+		contextRewrite.Path.PrefixMatchReplace != nil) || (contextRewrite.Host != nil && contextRewrite.Host.Name != nil)
+	rewriteHasCoreRewrites := rewrite.Hostname != nil || rewrite.Path != nil
+	return contextHasCoreRewrites && rewriteHasCoreRewrites
+}
+
+// Checks if the context and the rewrite both contain a envoy-gateway extended HTTP URL rewrite
+func hasMultipleExtensionRewrites(rewrite *egv1a1.HTTPURLRewriteFilter, contextRewrite *ir.URLRewrite) bool {
+	contextHasExtensionRewrites := (contextRewrite.Path != nil && contextRewrite.Path.RegexMatchReplace != nil) ||
+		(contextRewrite.Host != nil && (contextRewrite.Host.Header != nil || contextRewrite.Host.Backend != nil))
+
+	return contextHasExtensionRewrites && (rewrite.Hostname != nil || rewrite.Path != nil)
+}
+
+// Checks if the context and the gw-api core rewrite both contain an HTTP URL rewrite that creates a conflict (e.g. both rewrite path)
+func hasConflictingCoreAndExtensionRewrites(rewrite *gwapiv1.HTTPURLRewriteFilter, contextRewrite *ir.URLRewrite) bool {
+	contextHasExtensionPathRewrites := contextRewrite.Path != nil && contextRewrite.Path.RegexMatchReplace != nil
+	contextHasExtensionHostRewrites := contextRewrite.Host != nil && (contextRewrite.Host.Header != nil ||
+		contextRewrite.Host.Backend != nil)
+	return (rewrite.Hostname != nil && contextHasExtensionHostRewrites) || (rewrite.Path != nil && contextHasExtensionPathRewrites)
+}
+
+// Checks if the context and the envoy-gateway extended rewrite both contain an HTTP URL rewrite that creates a conflict (e.g. both rewrite path)
+func hasConflictingExtensionAndCoreRewrites(rewrite *egv1a1.HTTPURLRewriteFilter, contextRewrite *ir.URLRewrite) bool {
+	contextHasCorePathRewrites := contextRewrite.Path != nil && (contextRewrite.Path.FullReplace != nil ||
+		contextRewrite.Path.PrefixMatchReplace != nil)
+	contextHasCoreHostnameRewrites := contextRewrite.Host != nil && contextRewrite.Host.Name != nil
+
+	return (rewrite.Hostname != nil && contextHasCoreHostnameRewrites) || (rewrite.Path != nil && contextHasCorePathRewrites)
+}
+
 func (t *Translator) processURLRewriteFilter(
 	rewrite *gwapiv1.HTTPURLRewriteFilter,
 	filterContext *HTTPFiltersContext,
 ) {
 	if filterContext.URLRewrite != nil {
-		if filterContext.URLRewrite.Hostname != nil ||
-			filterContext.URLRewrite.Path.FullReplace != nil ||
-			filterContext.URLRewrite.Path.PrefixMatchReplace != nil {
+		if hasMultipleCoreRewrites(rewrite, filterContext.URLRewrite) ||
+			hasConflictingCoreAndExtensionRewrites(rewrite, filterContext.URLRewrite) {
 			routeStatus := GetRouteStatus(filterContext.Route)
 			status.SetRouteStatusCondition(routeStatus,
 				filterContext.ParentRef.routeParentStatusIdx,
@@ -188,7 +221,9 @@ func (t *Translator) processURLRewriteFilter(
 			return
 		}
 		redirectHost := string(*rewrite.Hostname)
-		newURLRewrite.Hostname = &redirectHost
+		newURLRewrite.Host = &ir.HTTPHostModifier{
+			Name: &redirectHost,
+		}
 	}
 
 	if rewrite.Path != nil {
@@ -751,48 +786,12 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec
 
 	if string(extFilter.Kind) == egv1a1.KindHTTPRouteFilter {
 		for _, hrf := range resources.HTTPRouteFilters {
-			if hrf.Namespace == filterNs && hrf.Name == string(extFilter.Name) &&
-				hrf.Spec.URLRewrite.Path.Type == egv1a1.RegexHTTPPathModifier {
-
-				if hrf.Spec.URLRewrite.Path.ReplaceRegexMatch == nil ||
-					hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern == "" {
-					errMsg := "ReplaceRegexMatch Pattern must be set when rewrite path type is \"ReplaceRegexMatch\""
-					routeStatus := GetRouteStatus(filterContext.Route)
-					status.SetRouteStatusCondition(routeStatus,
-						filterContext.ParentRef.routeParentStatusIdx,
-						filterContext.Route.GetGeneration(),
-						gwapiv1.RouteConditionAccepted,
-						metav1.ConditionFalse,
-						gwapiv1.RouteReasonUnsupportedValue,
-						errMsg,
-					)
-					return
-				} else if _, err := regexp.Compile(hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern); err != nil {
-					// Avoid envoy NACKs due to invalid regex.
-					// Golang's regexp is almost identical to RE2: https://pkg.go.dev/regexp/syntax
-					errMsg := "ReplaceRegexMatch must be a valid RE2 regular expression"
-					routeStatus := GetRouteStatus(filterContext.Route)
-					status.SetRouteStatusCondition(routeStatus,
-						filterContext.ParentRef.routeParentStatusIdx,
-						filterContext.Route.GetGeneration(),
-						gwapiv1.RouteConditionAccepted,
-						metav1.ConditionFalse,
-						gwapiv1.RouteReasonUnsupportedValue,
-						errMsg,
-					)
-					return
-				}
-
-				rmr := &ir.RegexMatchReplace{
-					Pattern:      hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern,
-					Substitution: hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Substitution,
-				}
+			if hrf.Namespace == filterNs && hrf.Name == string(extFilter.Name) {
+				if hrf.Spec.URLRewrite != nil {
 
-				if filterContext.HTTPFilterIR.URLRewrite != nil {
-					// If path IR is already set - check for a conflict
-					if filterContext.HTTPFilterIR.URLRewrite.Path != nil {
-						path := filterContext.HTTPFilterIR.URLRewrite.Path
-						if path.RegexMatchReplace != nil || path.PrefixMatchReplace != nil || path.FullReplace != nil {
+					if filterContext.URLRewrite != nil {
+						if hasMultipleExtensionRewrites(hrf.Spec.URLRewrite, filterContext.URLRewrite) ||
+							hasConflictingExtensionAndCoreRewrites(hrf.Spec.URLRewrite, filterContext.URLRewrite) {
 							routeStatus := GetRouteStatus(filterContext.Route)
 							status.SetRouteStatusCondition(routeStatus,
 								filterContext.ParentRef.routeParentStatusIdx,
@@ -804,19 +803,100 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec
 							)
 							return
 						}
-					} else { // no path
-						filterContext.HTTPFilterIR.URLRewrite.Path = &ir.ExtendedHTTPPathModifier{
-							RegexMatchReplace: rmr,
+					}
+
+					if hrf.Spec.URLRewrite.Path != nil {
+						if hrf.Spec.URLRewrite.Path.Type == egv1a1.RegexHTTPPathModifier {
+							if hrf.Spec.URLRewrite.Path.ReplaceRegexMatch == nil ||
+								hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern == "" {
+								errMsg := "ReplaceRegexMatch Pattern must be set when rewrite path type is \"ReplaceRegexMatch\""
+								routeStatus := GetRouteStatus(filterContext.Route)
+								status.SetRouteStatusCondition(routeStatus,
+									filterContext.ParentRef.routeParentStatusIdx,
+									filterContext.Route.GetGeneration(),
+									gwapiv1.RouteConditionAccepted,
+									metav1.ConditionFalse,
+									gwapiv1.RouteReasonUnsupportedValue,
+									errMsg,
+								)
+								return
+							} else if _, err := regexp.Compile(hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern); err != nil {
+								// Avoid envoy NACKs due to invalid regex.
+								// Golang's regexp is almost identical to RE2: https://pkg.go.dev/regexp/syntax
+								errMsg := "ReplaceRegexMatch must be a valid RE2 regular expression"
+								routeStatus := GetRouteStatus(filterContext.Route)
+								status.SetRouteStatusCondition(routeStatus,
+									filterContext.ParentRef.routeParentStatusIdx,
+									filterContext.Route.GetGeneration(),
+									gwapiv1.RouteConditionAccepted,
+									metav1.ConditionFalse,
+									gwapiv1.RouteReasonUnsupportedValue,
+									errMsg,
+								)
+								return
+							}
+
+							rmr := &ir.RegexMatchReplace{
+								Pattern:      hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern,
+								Substitution: hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Substitution,
+							}
+
+							if filterContext.HTTPFilterIR.URLRewrite != nil {
+								if filterContext.HTTPFilterIR.URLRewrite.Path == nil {
+									filterContext.HTTPFilterIR.URLRewrite.Path = &ir.ExtendedHTTPPathModifier{
+										RegexMatchReplace: rmr,
+									}
+									return
+								}
+							} else { // no url rewrite
+								filterContext.HTTPFilterIR.URLRewrite = &ir.URLRewrite{
+									Path: &ir.ExtendedHTTPPathModifier{
+										RegexMatchReplace: rmr,
+									},
+								}
+								return
+							}
 						}
-						return
 					}
-				} else { // no url rewrite
-					filterContext.HTTPFilterIR.URLRewrite = &ir.URLRewrite{
-						Path: &ir.ExtendedHTTPPathModifier{
-							RegexMatchReplace: rmr,
-						},
+
+					if hrf.Spec.URLRewrite.Hostname != nil {
+						var hm *ir.HTTPHostModifier
+						if hrf.Spec.URLRewrite.Hostname.Type == egv1a1.HeaderHTTPHostnameModifier {
+							if hrf.Spec.URLRewrite.Hostname.Header == nil {
+								errMsg := "Header must be set when rewrite path type is \"Header\""
+								routeStatus := GetRouteStatus(filterContext.Route)
+								status.SetRouteStatusCondition(routeStatus,
+									filterContext.ParentRef.routeParentStatusIdx,
+									filterContext.Route.GetGeneration(),
+									gwapiv1.RouteConditionAccepted,
+									metav1.ConditionFalse,
+									gwapiv1.RouteReasonUnsupportedValue,
+									errMsg,
+								)
+								return
+							}
+							hm = &ir.HTTPHostModifier{
+								Header: hrf.Spec.URLRewrite.Hostname.Header,
+							}
+						} else if hrf.Spec.URLRewrite.Hostname.Type == egv1a1.BackendHTTPHostnameModifier {
+							hm = &ir.HTTPHostModifier{
+								Backend: ptr.To(true),
+							}
+						}
+
+						if filterContext.HTTPFilterIR.URLRewrite != nil {
+							if filterContext.HTTPFilterIR.URLRewrite.Host == nil {
+								filterContext.HTTPFilterIR.URLRewrite.Host = hm
+								return
+							}
+						} else { // no url rewrite
+							filterContext.HTTPFilterIR.URLRewrite = &ir.URLRewrite{
+								Host: hm,
+							}
+							return
+						}
 					}
-					return
+
 				}
 			}
 		}

diff --git a/...nal/gatewayapi/testdata/httproute-with-urlrewrite-filter-hostname-prefix-replace.out.yaml b/...nal/gatewayapi/testdata/httproute-with-urlrewrite-filter-hostname-prefix-replace.out.yaml
@@ -144,7 +144,8 @@ xdsIR:
           name: ""
           prefix: /
         urlRewrite:
-          hostname: rewrite.com
+          host:
+            name: rewrite.com
           path:
             fullReplace: null
             prefixMatchReplace: /rewrite
diff --git a/internal/gatewayapi/testdata/httproute-with-urlrewrite-filter-hostname.out.yaml b/internal/gatewayapi/testdata/httproute-with-urlrewrite-filter-hostname.out.yaml
@@ -141,4 +141,5 @@ xdsIR:
           name: ""
           prefix: /
         urlRewrite:
-          hostname: rewrite.com
+          host:
+            name: rewrite.com
diff --git a/internal/gatewayapi/testdata/httproute-with-urlrewrite-filter-invalid-filter-type.out.yaml b/internal/gatewayapi/testdata/httproute-with-urlrewrite-filter-invalid-filter-type.out.yaml
@@ -141,4 +141,5 @@ xdsIR:
           name: ""
           prefix: /
         urlRewrite:
-          hostname: urlrewrite.envoyproxy.io
+          host:
+            name: urlrewrite.envoyproxy.io
diff --git a/...al/gatewayapi/testdata/httproute-with-urlrewrite-filter-regex-match-replace-http.out.yaml b/...al/gatewayapi/testdata/httproute-with-urlrewrite-filter-regex-match-replace-http.out.yaml
@@ -308,7 +308,8 @@ xdsIR:
           name: ""
           prefix: /host-and-regex-path
         urlRewrite:
-          hostname: rewrite.com
+          host:
+            name: rewrite.com
           path:
             fullReplace: null
             prefixMatchReplace: null
@@ -336,7 +337,8 @@ xdsIR:
           name: ""
           prefix: /regex-path-and-host
         urlRewrite:
-          hostname: rewrite.com
+          host:
+            name: rewrite.com
       - destination:
           name: httproute/default/httproute-1/rule/0
           settings: