feat: Multiple RequestMirrors Filters per HTTPRoute Rule (#1819)

* initial code change draft coding Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> * update test cases Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> * update mirror to mirrors Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> * update test file Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> * fix conformance test Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> * update gatewayapi testdata Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> * address review comments Signed-off-by: Ronnie-personal <76408835+Ronnie-personal@users.noreply.github.com> * update mirror to mirrors Signed-off-by: Ronnie-personal <76408835+Ronnie-personal@users.noreply.github.com> * Mirror to Mirrors Signed-off-by: Ronnie-personal <76408835+Ronnie-personal@users.noreply.github.com> * add conformance test back Signed-off-by: Ronnie-personal <76408835+Ronnie-personal@users.noreply.github.com> --------- Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com> Signed-off-by: Ronnie-personal <76408835+Ronnie-personal@users.noreply.github.com> Co-authored-by: zirain <zirain2009@gmail.com>
envoyproxy · Sep 17, 2023 · 14a395a · 14a395a
1 parent 1f08e46
commit 14a395a
Show file tree

Hide file tree

Showing 22 changed files with 252 additions and 103 deletions.
diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go
@@ -29,7 +29,7 @@ type HTTPFiltersTranslator interface {
 	processRedirectFilter(redirect *v1beta1.HTTPRequestRedirectFilter, filterContext *HTTPFiltersContext)
 	processRequestHeaderModifierFilter(headerModifier *v1beta1.HTTPHeaderFilter, filterContext *HTTPFiltersContext)
 	processResponseHeaderModifierFilter(headerModifier *v1beta1.HTTPHeaderFilter, filterContext *HTTPFiltersContext)
-	processRequestMirrorFilter(mirror *v1beta1.HTTPRequestMirrorFilter, filterContext *HTTPFiltersContext, resources *Resources)
+	processRequestMirrorFilter(filterIdx int, mirror *v1beta1.HTTPRequestMirrorFilter, filterContext *HTTPFiltersContext, resources *Resources)
 	processExtensionRefHTTPFilter(extRef *v1beta1.LocalObjectReference, filterContext *HTTPFiltersContext, resources *Resources)
 	processUnsupportedHTTPFilter(filterType string, filterContext *HTTPFiltersContext)
 }
@@ -56,7 +56,7 @@ type HTTPFilterIR struct {
 	AddResponseHeaders    []ir.AddHeader
 	RemoveResponseHeaders []string
 
-	Mirror *ir.RouteDestination
+	Mirrors []*ir.RouteDestination
 
 	RequestAuthentication *ir.RequestAuthentication
 	RateLimit             *ir.RateLimit
@@ -76,7 +76,6 @@ func (t *Translator) ProcessHTTPFilters(parentRef *RouteParentContext,
 		RuleIdx:      ruleIdx,
 		HTTPFilterIR: &HTTPFilterIR{},
 	}
-
 	for i := range filters {
 		filter := filters[i]
 		// If an invalid filter type has been configured then skip processing any more filters
@@ -98,7 +97,7 @@ func (t *Translator) ProcessHTTPFilters(parentRef *RouteParentContext,
 		case v1beta1.HTTPRouteFilterResponseHeaderModifier:
 			t.processResponseHeaderModifierFilter(filter.ResponseHeaderModifier, httpFiltersContext)
 		case v1beta1.HTTPRouteFilterRequestMirror:
-			t.processRequestMirrorFilter(filter.RequestMirror, httpFiltersContext, resources)
+			t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources)
 		case v1beta1.HTTPRouteFilterExtensionRef:
 			t.processExtensionRefHTTPFilter(filter.ExtensionRef, httpFiltersContext, resources)
 		default:
@@ -120,7 +119,6 @@ func (t *Translator) ProcessGRPCFilters(parentRef *RouteParentContext,
 
 		HTTPFilterIR: &HTTPFilterIR{},
 	}
-
 	for i := range filters {
 		filter := filters[i]
 		// If an invalid filter type has been configured then skip processing any more filters
@@ -138,7 +136,7 @@ func (t *Translator) ProcessGRPCFilters(parentRef *RouteParentContext,
 		case v1alpha2.GRPCRouteFilterResponseHeaderModifier:
 			t.processResponseHeaderModifierFilter(filter.ResponseHeaderModifier, httpFiltersContext)
 		case v1alpha2.GRPCRouteFilterRequestMirror:
-			t.processRequestMirrorFilter(filter.RequestMirror, httpFiltersContext, resources)
+			t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources)
 		case v1alpha2.GRPCRouteFilterExtensionRef:
 			t.processExtensionRefHTTPFilter(filter.ExtensionRef, httpFiltersContext, resources)
 		default:
@@ -808,6 +806,7 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *v1beta1.LocalObjec
 }
 
 func (t *Translator) processRequestMirrorFilter(
+	filterIdx int,
 	mirrorFilter *v1beta1.HTTPRequestMirrorFilter,
 	filterContext *HTTPFiltersContext,
 	resources *Resources) {
@@ -836,27 +835,11 @@ func (t *Translator) processRequestMirrorFilter(
 
 	mirrorEndpoints, _ := t.processDestEndpoints(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources)
 
-	// Only add missing mirror destinations
-	for _, mirrorEp := range mirrorEndpoints {
-		var found bool
-		if filterContext.Mirror != nil {
-			for _, mirror := range filterContext.Mirror.Endpoints {
-				if mirror != nil {
-					if mirror.Host == mirrorEp.Host && mirror.Port == mirrorEp.Port {
-						found = true
-					}
-				}
-			}
-		}
-		if !found {
-			if filterContext.Mirror == nil {
-				filterContext.Mirror = &ir.RouteDestination{
-					Name: fmt.Sprintf("%s-mirror", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx)),
-				}
-			}
-			filterContext.Mirror.Endpoints = append(filterContext.Mirror.Endpoints, mirrorEp)
-		}
+	newMirror := &ir.RouteDestination{
+		Name:      fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx),
+		Endpoints: mirrorEndpoints,
 	}
+	filterContext.Mirrors = append(filterContext.Mirrors, newMirror)
 }
 
 func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *HTTPFiltersContext) {

diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -286,8 +286,8 @@ func applyHTTPFiltersContextToIRRoute(httpFiltersContext *HTTPFiltersContext, ir
 	if len(httpFiltersContext.RemoveResponseHeaders) > 0 {
 		irRoute.RemoveResponseHeaders = httpFiltersContext.RemoveResponseHeaders
 	}
-	if httpFiltersContext.Mirror != nil {
-		irRoute.Mirror = httpFiltersContext.Mirror
+	if httpFiltersContext.Mirrors != nil {
+		irRoute.Mirrors = httpFiltersContext.Mirrors
 	}
 	if httpFiltersContext.RequestAuthentication != nil {
 		irRoute.RequestAuthentication = httpFiltersContext.RequestAuthentication
@@ -533,7 +533,7 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route
 					Redirect:              routeRoute.Redirect,
 					DirectResponse:        routeRoute.DirectResponse,
 					URLRewrite:            routeRoute.URLRewrite,
-					Mirror:                routeRoute.Mirror,
+					Mirrors:               routeRoute.Mirrors,
 					RequestAuthentication: routeRoute.RequestAuthentication,
 					RateLimit:             routeRoute.RateLimit,
 					ExtensionRefs:         routeRoute.ExtensionRefs,

diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter-duplicates.out.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter-duplicates.out.yaml
@@ -125,12 +125,17 @@ xdsIR:
             weight: 1
           name: httproute/default/httproute-1/rule/0
         hostname: gateway.envoyproxy.io
-        mirror:
-          endpoints:
+        mirrors:
+        - endpoints:
+          - host: 7.7.7.7
+            port: 8080
+            weight: 1
+          name: httproute/default/httproute-1/rule/0-mirror-0
+        - endpoints:
           - host: 7.7.7.7
             port: 8080
             weight: 1
-          name: httproute/default/httproute-1/rule/0-mirror
+          name: httproute/default/httproute-1/rule/0-mirror-1
         name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
         pathMatch:
           distinct: false

diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter-multiple.in.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter-multiple.in.yaml
@@ -35,6 +35,18 @@ httpRoutes:
       - name: service-1
         port: 8080
       filters:
+      - type: RequestHeaderModifier
+        requestHeaderModifier:
+          set:
+          - name: X-Header-Set
+            value: set-overwrites-values
+          add:
+          - name: X-Header-Add
+            value: header-val-1
+          - name: X-Header-Add-Append
+            value: header-val-2
+          remove:
+          - X-Header-Remove
       - type: RequestMirror
         requestMirror:
           backendRef:

diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter-multiple.out.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter-multiple.out.yaml
@@ -54,6 +54,18 @@ httpRoutes:
       - name: service-1
         port: 8080
       filters:
+      - requestHeaderModifier:
+          add:
+          - name: X-Header-Add
+            value: header-val-1
+          - name: X-Header-Add-Append
+            value: header-val-2
+          remove:
+          - X-Header-Remove
+          set:
+          - name: X-Header-Set
+            value: set-overwrites-values
+        type: RequestHeaderModifier
       - requestMirror:
           backendRef:
             kind: Service
@@ -115,7 +127,17 @@ xdsIR:
       name: envoy-gateway/gateway-1/http
       port: 10080
       routes:
-      - backendWeights:
+      - addRequestHeaders:
+        - append: true
+          name: X-Header-Add
+          value: header-val-1
+        - append: true
+          name: X-Header-Add-Append
+          value: header-val-2
+        - append: false
+          name: X-Header-Set
+          value: set-overwrites-values
+        backendWeights:
           invalid: 0
           valid: 0
         destination:
@@ -125,17 +147,21 @@ xdsIR:
             weight: 1
           name: httproute/default/httproute-1/rule/0
         hostname: gateway.envoyproxy.io
-        mirror:
-          endpoints:
+        mirrors:
+        - endpoints:
           - host: 7.7.7.7
             port: 8080
             weight: 1
+          name: httproute/default/httproute-1/rule/0-mirror-1
+        - endpoints:
           - host: 7.6.5.4
             port: 8080
             weight: 1
-          name: httproute/default/httproute-1/rule/0-mirror
+          name: httproute/default/httproute-1/rule/0-mirror-2
         name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
         pathMatch:
           distinct: false
           name: ""
           prefix: /
+        removeRequestHeaders:
+        - X-Header-Remove
diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter.out.yaml
@@ -119,12 +119,12 @@ xdsIR:
             weight: 1
           name: httproute/default/httproute-1/rule/0
         hostname: gateway.envoyproxy.io
-        mirror:
-          endpoints:
+        mirrors:
+        - endpoints:
           - host: 7.7.7.7
             port: 8080
             weight: 1
-          name: httproute/default/httproute-1/rule/0-mirror
+          name: httproute/default/httproute-1/rule/0-mirror-0
         name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
         pathMatch:
           distinct: false

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -263,7 +263,7 @@ type HTTPRoute struct {
 	// Redirections to be returned for this route. Takes precedence over Destinations.
 	Redirect *Redirect `json:"redirect,omitempty" yaml:"redirect,omitempty"`
 	// Destination that requests to this HTTPRoute will be mirrored to
-	Mirror *RouteDestination `json:"mirror,omitempty" yaml:"mirror,omitempty"`
+	Mirrors []*RouteDestination `json:"mirrors,omitempty" yaml:"mirrors,omitempty"`
 	// Destination associated with this matched route.
 	Destination *RouteDestination `json:"destination,omitempty" yaml:"destination,omitempty"`
 	// Rewrite to be changed for this route.
@@ -353,9 +353,11 @@ func (h HTTPRoute) Validate() error {
 			errs = multierror.Append(errs, err)
 		}
 	}
-	if h.Mirror != nil {
-		if err := h.Mirror.Validate(); err != nil {
-			errs = multierror.Append(errs, err)
+	if h.Mirrors != nil {
+		for _, mirror := range h.Mirrors {
+			if err := mirror.Validate(); err != nil {
+				errs = multierror.Append(errs, err)
+			}
 		}
 	}
 	if len(h.AddRequestHeaders) > 0 {

diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go
@@ -456,7 +456,7 @@ var (
 		PathMatch: &StringMatch{
 			Exact: ptrTo("mirrorfilter"),
 		},
-		Mirror: &happyRouteDestination,
+		Mirrors: []*RouteDestination{&happyRouteDestination},
 	}
 
 	// RouteDestination

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go
@@ -43,23 +43,23 @@ func buildXdsRoute(httpRoute *ir.HTTPRoute) *routev3.Route {
 		router.Action = &routev3.Route_Redirect{Redirect: buildXdsRedirectAction(httpRoute.Redirect)}
 	case httpRoute.URLRewrite != nil:
 		routeAction := buildXdsURLRewriteAction(httpRoute.Destination.Name, httpRoute.URLRewrite)
-		if httpRoute.Mirror != nil {
-			routeAction.RequestMirrorPolicies = buildXdsRequestMirrorPolicies(httpRoute.Mirror.Name)
+		if httpRoute.Mirrors != nil {
+			routeAction.RequestMirrorPolicies = buildXdsRequestMirrorPolicies(httpRoute.Mirrors)
 		}
 
 		router.Action = &routev3.Route_Route{Route: routeAction}
 	default:
 		if httpRoute.BackendWeights.Invalid != 0 {
 			// If there are invalid backends then a weighted cluster is required for the route
 			routeAction := buildXdsWeightedRouteAction(httpRoute)
-			if httpRoute.Mirror != nil {
-				routeAction.RequestMirrorPolicies = buildXdsRequestMirrorPolicies(httpRoute.Mirror.Name)
+			if httpRoute.Mirrors != nil {
+				routeAction.RequestMirrorPolicies = buildXdsRequestMirrorPolicies(httpRoute.Mirrors)
 			}
 			router.Action = &routev3.Route_Route{Route: routeAction}
 		} else {
 			routeAction := buildXdsRouteAction(httpRoute.Destination.Name)
-			if httpRoute.Mirror != nil {
-				routeAction.RequestMirrorPolicies = buildXdsRequestMirrorPolicies(httpRoute.Mirror.Name)
+			if httpRoute.Mirrors != nil {
+				routeAction.RequestMirrorPolicies = buildXdsRequestMirrorPolicies(httpRoute.Mirrors)
 			}
 			router.Action = &routev3.Route_Route{Route: routeAction}
 		}
@@ -292,11 +292,13 @@ func buildXdsDirectResponseAction(res *ir.DirectResponse) *routev3.DirectRespons
 	return routeAction
 }
 
-func buildXdsRequestMirrorPolicies(destName string) []*routev3.RouteAction_RequestMirrorPolicy {
-	mirrorPolicies := []*routev3.RouteAction_RequestMirrorPolicy{
-		{
-			Cluster: destName,
-		},
+func buildXdsRequestMirrorPolicies(mirrorDestinations []*ir.RouteDestination) []*routev3.RouteAction_RequestMirrorPolicy {
+	var mirrorPolicies []*routev3.RouteAction_RequestMirrorPolicy
+
+	for _, mirrorDest := range mirrorDestinations {
+		mirrorPolicies = append(mirrorPolicies, &routev3.RouteAction_RequestMirrorPolicy{
+			Cluster: mirrorDest.Name,
+		})
 	}
 
 	return mirrorPolicies

diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-multiple-mirrors.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-multiple-mirrors.yaml
@@ -0,0 +1,22 @@
+name: "http-route"
+http:
+- name: "first-listener"
+  address: "0.0.0.0"
+  port: 10080
+  hostnames:
+  - "*"
+  routes:
+  - name: "mirror-route"
+    hostname: "*"
+    destination:
+      name: "route-dest"
+      endpoints:
+      - host: "1.2.3.4"
+        port: 50000
+    mirrors:
+    - name: "mirror-route-dest"
+      endpoints:
+      - host: "2.3.4.5"
+    - name: "mirror-route-dest1"
+      endpoints:
+      - host: "3.4.5.6"
diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-mirror.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-mirror.clusters.yaml
@@ -11,16 +11,3 @@
   outlierDetection: {}
   perConnectionBufferLimitBytes: 32768
   type: EDS
-- commonLbConfig:
-    localityWeightedLbConfig: {}
-  connectTimeout: 10s
-  dnsLookupFamily: V4_ONLY
-  edsClusterConfig:
-    edsConfig:
-      ads: {}
-      resourceApiVersion: V3
-    serviceName: mirror-route-dest
-  name: mirror-route-dest
-  outlierDetection: {}
-  perConnectionBufferLimitBytes: 32768
-  type: EDS
diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-mirror.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-mirror.endpoints.yaml
@@ -8,13 +8,3 @@
             portValue: 50000
     loadBalancingWeight: 1
     locality: {}
-- clusterName: mirror-route-dest
-  endpoints:
-  - lbEndpoints:
-    - endpoint:
-        address:
-          socketAddress:
-            address: 2.3.4.5
-            portValue: 0
-    loadBalancingWeight: 1
-    locality: {}
diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-mirror.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-mirror.routes.yaml
@@ -10,5 +10,3 @@
       name: mirror-route
       route:
         cluster: route-dest
-        requestMirrorPolicies:
-        - cluster: mirror-route-dest