gw-api: Support multiple RequestMirror filters per rule in GW API (#5652

) Multiple mirror filters can be configured in HTTP and GRPCRoute Signed-off-by: Lior Lieberman <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]>
projectcontour · Aug 11, 2023 · 3231bf7 · 3231bf7
1 parent 135c8a9
commit 3231bf7
Show file tree

Hide file tree

Showing 11 changed files with 287 additions and 68 deletions.
diff --git a/changelogs/unreleased/5652-LiorLieberman-minor.md b/changelogs/unreleased/5652-LiorLieberman-minor.md
@@ -0,0 +1,6 @@
+## Adding support for multiple gateway-api RequestMirror filters within the same HTTP or GRPC rule 
+
+Currently, Contour supports a single RequestMirror filter per rule in HTTPRoute or GRPCRoute.
+Envoy however, supports more than one mirror backend using [request_mirror_policies](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-routeaction)
+
+This PR adds support for multiple gateway-api RequestMirror filters within the same HTTP or GRPC rule.
diff --git a/internal/dag/accessors.go b/internal/dag/accessors.go
@@ -202,8 +202,10 @@ func (d *DAG) GetClusters() []*Cluster {
 			for _, route := range vhost.Routes {
 				res = append(res, route.Clusters...)
 
-				if route.MirrorPolicy != nil && route.MirrorPolicy.Cluster != nil {
-					res = append(res, route.MirrorPolicy.Cluster)
+				for _, mp := range route.MirrorPolicies {
+					if mp.Cluster != nil {
+						res = append(res, mp.Cluster)
+					}
 				}
 			}
 		}
@@ -212,8 +214,10 @@ func (d *DAG) GetClusters() []*Cluster {
 			for _, route := range vhost.Routes {
 				res = append(res, route.Clusters...)
 
-				if route.MirrorPolicy != nil && route.MirrorPolicy.Cluster != nil {
-					res = append(res, route.MirrorPolicy.Cluster)
+				for _, mp := range route.MirrorPolicies {
+					if mp.Cluster != nil {
+						res = append(res, mp.Cluster)
+					}
 				}
 			}
 

diff --git a/internal/dag/accessors_test.go b/internal/dag/accessors_test.go
@@ -268,4 +268,31 @@ func TestGetServiceClusters(t *testing.T) {
 	// We should only get one cluster since the other is for an ExternalName
 	// service.
 	assert.Len(t, d.GetServiceClusters(), 1)
+
+	dagWithMirror := &DAG{
+		Listeners: map[string]*Listener{
+			"http-1": {
+				VirtualHosts: []*VirtualHost{
+					{
+						Routes: map[string]*Route{
+							"foo": {
+								Clusters: []*Cluster{
+									{Upstream: &Service{}},
+								},
+								MirrorPolicies: []*MirrorPolicy{
+									{
+										Cluster: &Cluster{
+											Upstream: &Service{},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	// We should get two clusters since we have mirrorPolicies.
+	assert.Len(t, dagWithMirror.GetServiceClusters(), 2)
 }
diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go
@@ -3815,7 +3815,55 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
 				&Listener{
 					Name: "http-80",
 					VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io",
-						withMirror(prefixrouteHTTPRoute("/", service(kuardService)), service(kuardService2), 100))),
+						withMirror(prefixrouteHTTPRoute("/", service(kuardService)), []*Service{service(kuardService2)}, 100))),
+				},
+			),
+		},
+		"HTTPRoute rule with two request mirror filters": {
+			gatewayclass: validClass,
+			gateway:      gatewayHTTPAllNamespaces,
+			objs: []any{
+				kuardService,
+				kuardService2,
+				kuardService3,
+				&gatewayapi_v1beta1.HTTPRoute{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "basic",
+						Namespace: "projectcontour",
+					},
+					Spec: gatewayapi_v1beta1.HTTPRouteSpec{
+						CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{
+							ParentRefs: []gatewayapi_v1beta1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")},
+						},
+						Hostnames: []gatewayapi_v1beta1.Hostname{
+							"test.projectcontour.io",
+						},
+						Rules: []gatewayapi_v1beta1.HTTPRouteRule{{
+							Matches:     gatewayapi.HTTPRouteMatch(gatewayapi_v1beta1.PathMatchPathPrefix, "/"),
+							BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1),
+							Filters: []gatewayapi_v1beta1.HTTPRouteFilter{
+								{
+									Type: gatewayapi_v1beta1.HTTPRouteFilterRequestMirror,
+									RequestMirror: &gatewayapi_v1beta1.HTTPRequestMirrorFilter{
+										BackendRef: gatewayapi.ServiceBackendObjectRef("kuard2", 8080),
+									},
+								},
+								{
+									Type: gatewayapi_v1beta1.HTTPRouteFilterRequestMirror,
+									RequestMirror: &gatewayapi_v1beta1.HTTPRequestMirrorFilter{
+										BackendRef: gatewayapi.ServiceBackendObjectRef("kuard3", 8080),
+									},
+								},
+							},
+						}},
+					},
+				},
+			},
+			want: listeners(
+				&Listener{
+					Name: "http-80",
+					VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io",
+						withMirror(prefixrouteHTTPRoute("/", service(kuardService)), []*Service{service(kuardService2), service(kuardService3)}, 100))),
 				},
 			),
 		},
@@ -3857,8 +3905,8 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
 				&Listener{
 					Name: "http-80",
 					VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io",
-						withMirror(prefixrouteHTTPRoute("/", service(kuardService)), service(kuardService2), 100),
-						withMirror(segmentPrefixHTTPRoute("/another-match", service(kuardService)), service(kuardService2), 100),
+						withMirror(prefixrouteHTTPRoute("/", service(kuardService)), []*Service{service(kuardService2)}, 100),
+						withMirror(segmentPrefixHTTPRoute("/another-match", service(kuardService)), []*Service{service(kuardService2)}, 100),
 					)),
 				},
 			),
@@ -5677,7 +5725,57 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
 				&Listener{
 					Name: "http-80",
 					VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io",
-						withMirror(exactrouteGRPCRoute("/io.projectcontour/Login", grpcService(kuardService, "h2c")), grpcService(kuardService2, "h2c"), 100))),
+						withMirror(exactrouteGRPCRoute("/io.projectcontour/Login", grpcService(kuardService, "h2c")), []*Service{grpcService(kuardService2, "h2c")}, 100))),
+				},
+			),
+		},
+		"GRPCRoute: rule with two request mirror filters": {
+			gatewayclass: validClass,
+			gateway:      gatewayHTTPAllNamespaces,
+			objs: []any{
+				kuardService,
+				kuardService2,
+				kuardService3,
+				&gatewayapi_v1alpha2.GRPCRoute{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "basic",
+						Namespace: "projectcontour",
+					},
+					Spec: gatewayapi_v1alpha2.GRPCRouteSpec{
+						CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{
+							ParentRefs: []gatewayapi_v1alpha2.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")},
+						},
+						Hostnames: []gatewayapi_v1alpha2.Hostname{
+							"test.projectcontour.io",
+						},
+						Rules: []gatewayapi_v1alpha2.GRPCRouteRule{{
+							Matches: []gatewayapi_v1alpha2.GRPCRouteMatch{{
+								Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1alpha2.GRPCMethodMatchExact, "io.projectcontour", "Login"),
+							}},
+							BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1),
+							Filters: []gatewayapi_v1alpha2.GRPCRouteFilter{
+								{
+									Type: gatewayapi_v1alpha2.GRPCRouteFilterRequestMirror,
+									RequestMirror: &gatewayapi_v1alpha2.HTTPRequestMirrorFilter{
+										BackendRef: gatewayapi.ServiceBackendObjectRef("kuard2", 8080),
+									},
+								},
+								{
+									Type: gatewayapi_v1alpha2.GRPCRouteFilterRequestMirror,
+									RequestMirror: &gatewayapi_v1alpha2.HTTPRequestMirrorFilter{
+										BackendRef: gatewayapi.ServiceBackendObjectRef("kuard3", 8080),
+									},
+								},
+							},
+						}},
+					},
+				},
+			},
+			want: listeners(
+				&Listener{
+					Name: "http-80",
+					VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io",
+						withMirror(exactrouteGRPCRoute("/io.projectcontour/Login", grpcService(kuardService, "h2c")), []*Service{grpcService(kuardService2, "h2c"), grpcService(kuardService3, "h2c")}, 100))),
 				},
 			),
 		},
@@ -11087,7 +11185,7 @@ func TestDAGInsert(t *testing.T) {
 					Port: 8080,
 					VirtualHosts: virtualhosts(
 						virtualhost("example.com",
-							withMirror(prefixroute("/", service(s1)), service(s2), 100),
+							withMirror(prefixroute("/", service(s1)), []*Service{service(s2)}, 100),
 						),
 					),
 				},
@@ -16146,12 +16244,14 @@ func prefixSegment(prefix string) MatchCondition {
 func exact(path string) MatchCondition  { return &ExactMatchCondition{Path: path} }
 func regex(regex string) MatchCondition { return &RegexMatchCondition{Regex: regex} }
 
-func withMirror(r *Route, mirror *Service, weight int64) *Route {
-	r.MirrorPolicy = &MirrorPolicy{
-		Cluster: &Cluster{
-			Upstream: mirror,
-		},
-		Weight: weight,
+func withMirror(r *Route, mirrors []*Service, weight int64) *Route {
+	for _, mirror := range mirrors {
+		r.MirrorPolicies = append(r.MirrorPolicies, &MirrorPolicy{
+			Cluster: &Cluster{
+				Upstream: mirror,
+			},
+			Weight: weight,
+		})
 	}
 	return r
 }
diff --git a/internal/dag/dag.go b/internal/dag/dag.go
@@ -319,8 +319,8 @@ type Route struct {
 	// Indicates that during forwarding, the matched prefix (or path) should be swapped with this value
 	PathRewritePolicy *PathRewritePolicy
 
-	// Mirror Policy defines the mirroring policy for this Route.
-	MirrorPolicy *MirrorPolicy
+	// MirrorPolicies is a list defining the mirroring policies for this Route.
+	MirrorPolicies []*MirrorPolicy
 
 	// RequestHeadersPolicy defines how headers are managed during forwarding
 	RequestHeadersPolicy *HeadersPolicy

diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go
@@ -1124,13 +1124,14 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be
 			requestHeaderPolicy  *HeadersPolicy
 			responseHeaderPolicy *HeadersPolicy
 			redirect             *Redirect
-			mirrorPolicy         *MirrorPolicy
+			mirrorPolicies       []*MirrorPolicy
 			pathRewritePolicy    *PathRewritePolicy
 			urlRewriteHostname   string
 		)
 
-		// Per Gateway API docs: "Specifying a core filter multiple times
-		// has unspecified or implementation-specific conformance." Contour
+		// Per Gateway API docs: "Specifying the same filter multiple times is
+		// not supported unless explicitly indicated in the filter." For filters
+		// that can't be used multiple times within the same rule, Contour
 		// chooses to use the first instance of each filter type and ignore
 		// subsequent instances.
 		for _, filter := range rule.Filters {
@@ -1222,7 +1223,7 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be
 					PathRewritePolicy: pathRewritePolicy,
 				}
 			case gatewayapi_v1beta1.HTTPRouteFilterRequestMirror:
-				if filter.RequestMirror == nil || mirrorPolicy != nil {
+				if filter.RequestMirror == nil {
 					continue
 				}
 
@@ -1231,12 +1232,12 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be
 					routeAccessor.AddCondition(gatewayapi_v1beta1.RouteConditionType(cond.Type), cond.Status, gatewayapi_v1beta1.RouteConditionReason(cond.Reason), cond.Message)
 					continue
 				}
-				mirrorPolicy = &MirrorPolicy{
+				mirrorPolicies = append(mirrorPolicies, &MirrorPolicy{
 					Cluster: &Cluster{
 						Upstream: mirrorService,
 					},
 					Weight: 100,
-				}
+				})
 			case gatewayapi_v1beta1.HTTPRouteFilterURLRewrite:
 				if filter.URLRewrite == nil || pathRewritePolicy != nil {
 					continue
@@ -1337,7 +1338,7 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be
 				matchconditions,
 				requestHeaderPolicy,
 				responseHeaderPolicy,
-				mirrorPolicy,
+				mirrorPolicies,
 				clusters,
 				totalWeight,
 				priority,
@@ -1403,11 +1404,12 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1al
 		// Process rule-level filters.
 		var (
 			requestHeaderPolicy, responseHeaderPolicy *HeadersPolicy
-			mirrorPolicy                              *MirrorPolicy
+			mirrorPolicies                            []*MirrorPolicy
 		)
 
-		// Per Gateway API docs: "Specifying a core filter multiple times
-		// has unspecified or implementation-specific conformance." Contour
+		// Per Gateway API docs: "Specifying the same filter multiple times is
+		// not supported unless explicitly indicated in the filter." For filters
+		// that can't be used multiple times within the same rule, Contour
 		// chooses to use the first instance of each filter type and ignore
 		// subsequent instances.
 		for _, filter := range rule.Filters {
@@ -1433,7 +1435,7 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1al
 					routeAccessor.AddCondition(gatewayapi_v1beta1.RouteConditionResolvedRefs, metav1.ConditionFalse, status.ReasonDegraded, fmt.Sprintf("%s on response headers", err))
 				}
 			case gatewayapi_v1alpha2.GRPCRouteFilterRequestMirror:
-				if filter.RequestMirror == nil || mirrorPolicy != nil {
+				if filter.RequestMirror == nil {
 					continue
 				}
 
@@ -1444,12 +1446,12 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1al
 				}
 				// If protocol is not set on the service, need to set a default one based on listener's protocol type.
 				setDefaultServiceProtocol(mirrorService, listener.listener.Protocol)
-				mirrorPolicy = &MirrorPolicy{
+				mirrorPolicies = append(mirrorPolicies, &MirrorPolicy{
 					Cluster: &Cluster{
 						Upstream: mirrorService,
 					},
 					Weight: 100,
-				}
+				})
 			default:
 				routeAccessor.AddCondition(
 					gatewayapi_v1beta1.RouteConditionAccepted,
@@ -1481,7 +1483,7 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1al
 			matchconditions,
 			requestHeaderPolicy,
 			responseHeaderPolicy,
-			mirrorPolicy,
+			mirrorPolicies,
 			clusters,
 			totalWeight,
 			priority,
@@ -1890,8 +1892,9 @@ func (p *GatewayAPIProcessor) httpClusters(routeNamespace string, backendRefs []
 		var clusterRequestHeaderPolicy *HeadersPolicy
 		var clusterResponseHeaderPolicy *HeadersPolicy
 
-		// Per Gateway API docs: "Specifying a core filter multiple times
-		// has unspecified or implementation-specific conformance." Contour
+		// Per Gateway API docs: "Specifying the same filter multiple times is
+		// not supported unless explicitly indicated in the filter." For filters
+		// that can't be used multiple times within the same rule, Contour
 		// chooses to use the first instance of each filter type and ignore
 		// subsequent instances.
 		for _, filter := range backendRef.Filters {
@@ -1973,8 +1976,9 @@ func (p *GatewayAPIProcessor) grpcClusters(routeNamespace string, backendRefs []
 
 		var clusterRequestHeaderPolicy, clusterResponseHeaderPolicy *HeadersPolicy
 
-		// Per Gateway API docs: "Specifying a core filter multiple times
-		// has unspecified or implementation-specific conformance." Contour
+		// Per Gateway API docs: "Specifying the same filter multiple times is
+		// not supported unless explicitly indicated in the filter." For filters
+		// that can't be used multiple times within the same rule, Contour
 		// chooses to use the first instance of each filter type and ignore
 		// subsequent instances.
 		for _, filter := range backendRef.Filters {
@@ -2043,7 +2047,7 @@ func (p *GatewayAPIProcessor) clusterRoutes(
 	matchConditions []*matchConditions,
 	requestHeaderPolicy *HeadersPolicy,
 	responseHeaderPolicy *HeadersPolicy,
-	mirrorPolicy *MirrorPolicy,
+	mirrorPolicies []*MirrorPolicy,
 	clusters []*Cluster,
 	totalWeight uint32,
 	priority uint8,
@@ -2071,7 +2075,7 @@ func (p *GatewayAPIProcessor) clusterRoutes(
 			QueryParamMatchConditions: mc.queryParams,
 			RequestHeadersPolicy:      requestHeaderPolicy,
 			ResponseHeadersPolicy:     responseHeaderPolicy,
-			MirrorPolicy:              mirrorPolicy,
+			MirrorPolicies:            mirrorPolicies,
 			Priority:                  priority,
 			PathRewritePolicy:         pathRewritePolicy,
 		}