diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index f5d71fd9eb4..8cfee94484d 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -1480,9 +1480,9 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener( timeoutPolicy) } - // check all the routes whether there is conflict against previous rules + // Check all the routes whether there is conflict against previous rules. if !p.hasConflictRoute(listener, hosts, routes) { - // add the route if there is conflict + // Add the route if there is no conflict at the same rule level. // Add each route to the relevant vhost(s)/svhosts(s). for host := range hosts { for _, route := range routes { @@ -1507,6 +1507,7 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener( // No rules under the route is valid, mark it as not accepted. addRouteNotAcceptedConditionDueToMatchConflict(routeAccessor, KindHTTPRoute) } else if invalidRuleCnt > 0 { + // Some of the rules are conflicted, mark it as partially invalid. addRoutePartiallyInvalidConditionDueToMatchPartiallyConflict(routeAccessor, KindHTTPRoute) } } @@ -1657,9 +1658,9 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1.G nil, ) - // check all the routes whether there is conflict against previous rules + // Check all the routes whether there is conflict against previous rules. if !p.hasConflictRoute(listener, hosts, routes) { - // add the route if there is conflict + // Add the route if there is no conflict at the same rule level. // Add each route to the relevant vhost(s)/svhosts(s). for host := range hosts { for _, route := range routes { @@ -1684,6 +1685,7 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1.G // No rules under the route is valid, mark it as not accepted. addRouteNotAcceptedConditionDueToMatchConflict(routeAccessor, KindGRPCRoute) } else if invalidRuleCnt > 0 { + // Some of the rules are conflicted, mark it as partially invalid. addRoutePartiallyInvalidConditionDueToMatchPartiallyConflict(routeAccessor, KindGRPCRoute) } diff --git a/internal/dag/gatewayapi_processor_test.go b/internal/dag/gatewayapi_processor_test.go index c1c50bcb70b..c4b0a3de608 100644 --- a/internal/dag/gatewayapi_processor_test.go +++ b/internal/dag/gatewayapi_processor_test.go @@ -1475,7 +1475,7 @@ func TestHasConflictRoute(t *testing.T) { expectedConflict bool }{ { - name: "There are 2 existing route, the 3rd route to add doesn't have conflict, listen doesn't have tls, no conflict expected", + name: "There are 2 existing httproute, the 3rd route to add doesn't have conflict, listen doesn't have tls, no conflict expected", existingRoutes: []*Route{ { Name: "route1", @@ -1511,6 +1511,43 @@ func TestHasConflictRoute(t *testing.T) { }, listener: listener, }, + { + name: "There are 2 existing grpcroute, the 3rd route to add doesn't have conflict, listen doesn't have tls, no conflict expected", + existingRoutes: []*Route{ + { + Name: "route1", + Namespace: "default", + PathMatchCondition: prefixSegment("/path1"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: ":authority", MatchType: HeaderMatchTypeRegex, Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?"}, + }, + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + }, + { + Kind: KindGRPCRoute, + Name: "route2", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + }, + }, + routes: []*Route{ + { + Kind: KindGRPCRoute, + Name: "route3", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "e-tag", Value: "abc", MatchType: "contains", Invert: true}, + }, + }, + }, + listener: listener, + }, { name: "There are 2 existing route, the 3rd route to add doesn't have conflict, listen has tls, no conflict expected", existingRoutes: []*Route{ diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index 2c6caae88bb..cefc5a79836 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -5647,7 +5647,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }}, }, }, - // 3rd HTTPRoute with newest creationTimestamp, conflict with 1st HTTPRoute + // 3rd HTTPRoute with newest creationTimestamp, partially conflict with 1st HTTPRoute &gatewayapi_v1.HTTPRoute{ TypeMeta: meta_v1.TypeMeta{ Kind: KindHTTPRoute, @@ -5664,172 +5664,56 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { Hostnames: []gatewayapi_v1.Hostname{ "test.projectcontour.io", }, - Rules: []gatewayapi_v1.HTTPRouteRule{{ - Matches: []gatewayapi_v1.HTTPRouteMatch{ - { - Path: &gatewayapi_v1.HTTPPathMatch{ - Type: ptr.To(gatewayapi_v1.PathMatchPathPrefix), - Value: ptr.To("/"), - }, - Headers: []gatewayapi_v1.HTTPHeaderMatch{ - { - Type: ptr.To(gatewayapi_v1.HeaderMatchExact), - Name: gatewayapi_v1.HTTPHeaderName("foo"), - Value: "bar", + Rules: []gatewayapi_v1.HTTPRouteRule{ + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + { + Path: &gatewayapi_v1.HTTPPathMatch{ + Type: ptr.To(gatewayapi_v1.PathMatchPathPrefix), + Value: ptr.To("/"), }, - }, - QueryParams: []gatewayapi_v1.HTTPQueryParamMatch{ - { - Type: ptr.To(gatewayapi_v1.QueryParamMatchRegularExpression), - Name: "param-1", - Value: "valid-[a-z]?-[A-Za-z]+-[0=9]+-\\d+", + Headers: []gatewayapi_v1.HTTPHeaderMatch{ + { + Type: ptr.To(gatewayapi_v1.HeaderMatchExact), + Name: gatewayapi_v1.HTTPHeaderName("foo"), + Value: "bar", + }, + }, + QueryParams: []gatewayapi_v1.HTTPQueryParamMatch{ + { + Type: ptr.To(gatewayapi_v1.QueryParamMatchRegularExpression), + Name: "param-1", + Value: "valid-[a-z]?-[A-Za-z]+-[0=9]+-\\d+", + }, }, }, }, - }, - BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), - }}, - }, - }, - }, - wantRouteConditions: []*status.RouteStatusUpdate{ - { - FullName: types.NamespacedName{Namespace: "default", Name: "basic-1"}, - RouteParentStatuses: []*gatewayapi_v1.RouteParentStatus{ - { - ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), - Conditions: []meta_v1.Condition{ - routeResolvedRefsCondition(), - routeAcceptedHTTPRouteCondition(), - }, - }, - }, - }, - { - FullName: types.NamespacedName{Namespace: "default", Name: "basic-2"}, - RouteParentStatuses: []*gatewayapi_v1.RouteParentStatus{ - { - ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), - Conditions: []meta_v1.Condition{ - routeAcceptedFalse(status.ReasonRouteRuleMatchConflict, fmt.Sprintf(status.MessageRouteRuleMatchConflict, KindHTTPRoute, KindHTTPRoute)), - routeResolvedRefsCondition(), - }, - }, - }, - }, - { - FullName: types.NamespacedName{Namespace: "default", Name: "basic-3"}, - RouteParentStatuses: []*gatewayapi_v1.RouteParentStatus{ - { - ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), - Conditions: []meta_v1.Condition{ - routeAcceptedFalse(status.ReasonRouteRuleMatchConflict, fmt.Sprintf(status.MessageRouteRuleMatchConflict, KindHTTPRoute, KindHTTPRoute)), - routeResolvedRefsCondition(), + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), }, - }, - }, - }, - }, - // is it ok to show the listeners are attached, just it's not accepted because of the conflict - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 3), - }) - - run(t, "3 grpcroutes, but duplicate match condition between these 3. The 2 rank lower gets Conflict condition ", testcase{ - objs: []any{ - kuardService, - // first GRPCRoute with oldest creationTimestamp - &gatewayapi_v1.GRPCRoute{ - TypeMeta: meta_v1.TypeMeta{ - Kind: KindGRPCRoute, - }, - ObjectMeta: meta_v1.ObjectMeta{ - Name: "basic-1", - Namespace: "default", - CreationTimestamp: meta_v1.Date(2020, time.Month(2), 21, 1, 10, 30, 0, time.UTC), - }, - Spec: gatewayapi_v1.GRPCRouteSpec{ - CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ - ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, - }, - Hostnames: []gatewayapi_v1.Hostname{ - "test.projectcontour.io", - }, - Rules: []gatewayapi_v1.GRPCRouteRule{{ - Matches: []gatewayapi_v1.GRPCRouteMatch{ - { - Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "com.example.service", "Login"), - }, - { - Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "foo.com.example.service", "Login"), + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + { + Path: &gatewayapi_v1.HTTPPathMatch{ + Type: ptr.To(gatewayapi_v1.PathMatchPathPrefix), + Value: ptr.To("/random"), + }, + Headers: []gatewayapi_v1.HTTPHeaderMatch{ + { + Type: ptr.To(gatewayapi_v1.HeaderMatchExact), + Name: gatewayapi_v1.HTTPHeaderName("random"), + Value: "b", + }, + }, + }, }, + + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), }, - BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), - }}, - }, - }, - // second GRPCRoute with 2nd oldest creationTimestamp, conflict with 1st GRPCRoute - &gatewayapi_v1.GRPCRoute{ - TypeMeta: meta_v1.TypeMeta{ - Kind: KindGRPCRoute, - }, - ObjectMeta: meta_v1.ObjectMeta{ - Name: "basic-2", - Namespace: "default", - CreationTimestamp: meta_v1.Date(2021, time.Month(2), 21, 1, 10, 30, 0, time.UTC), - }, - Spec: gatewayapi_v1.GRPCRouteSpec{ - CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ - ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, }, - Hostnames: []gatewayapi_v1.Hostname{ - "test.projectcontour.io", - }, - Rules: []gatewayapi_v1.GRPCRouteRule{{ - Matches: []gatewayapi_v1.GRPCRouteMatch{ - { - Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "com.example.service", "Login"), - }, - { - Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "foo.com.example.service", "Login"), - }, - }, - BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), - }}, - }, - }, - // 3rd GRPCRoute with newest creationTimestamp, conflict with 1st GRPCRoute - &gatewayapi_v1.GRPCRoute{ - TypeMeta: meta_v1.TypeMeta{ - Kind: KindGRPCRoute, - }, - ObjectMeta: meta_v1.ObjectMeta{ - Name: "basic-3", - Namespace: "default", - CreationTimestamp: meta_v1.Date(2022, time.Month(2), 21, 1, 10, 30, 0, time.UTC), - }, - Spec: gatewayapi_v1.GRPCRouteSpec{ - CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ - ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, - }, - Hostnames: []gatewayapi_v1.Hostname{ - "test.projectcontour.io", - }, - Rules: []gatewayapi_v1.GRPCRouteRule{{ - Matches: []gatewayapi_v1.GRPCRouteMatch{ - { - Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "com.example.service", "Login"), - }, - { - Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "foo.com.example.service", "Login"), - }, - }, - BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), - }}, }, }, }, - wantRouteConditions: []*status.RouteStatusUpdate{ { FullName: types.NamespacedName{Namespace: "default", Name: "basic-1"}, @@ -5838,7 +5722,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), Conditions: []meta_v1.Condition{ routeResolvedRefsCondition(), - routeAcceptedGRPCRouteCondition(), + routeAcceptedHTTPRouteCondition(), }, }, }, @@ -5849,7 +5733,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { { ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), Conditions: []meta_v1.Condition{ - routeAcceptedFalse(status.ReasonRouteRuleMatchConflict, fmt.Sprintf(status.MessageRouteRuleMatchConflict, KindGRPCRoute, KindGRPCRoute)), + routeAcceptedFalse(status.ReasonRouteRuleMatchConflict, fmt.Sprintf(status.MessageRouteRuleMatchConflict, KindHTTPRoute, KindHTTPRoute)), routeResolvedRefsCondition(), }, }, @@ -5861,7 +5745,8 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { { ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), Conditions: []meta_v1.Condition{ - routeAcceptedFalse(status.ReasonRouteRuleMatchConflict, fmt.Sprintf(status.MessageRouteRuleMatchConflict, KindGRPCRoute, KindGRPCRoute)), + routeAcceptedHTTPRouteCondition(), + routePartialMatchConflict(status.ReasonRouteRuleMatchPartiallyConflict, fmt.Sprintf(status.MessageRouteRuleMatchPartiallyConflict, KindHTTPRoute, KindHTTPRoute)), routeResolvedRefsCondition(), }, }, @@ -11412,6 +11297,148 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { // Invalid filters still result in an attached route. wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) + + run(t, "grpcroute: 3 grpcroutes, but duplicate match condition between these 3. The 2 out of 3 rank lower gets Conflict condition ", testcase{ + objs: []any{ + kuardService, + // first GRPCRoute with oldest creationTimestamp + &gatewayapi_v1.GRPCRoute{ + TypeMeta: meta_v1.TypeMeta{ + Kind: KindGRPCRoute, + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: "basic-1", + Namespace: "default", + CreationTimestamp: meta_v1.Date(2020, time.Month(2), 21, 1, 10, 30, 0, time.UTC), + }, + Spec: gatewayapi_v1.GRPCRouteSpec{ + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1.GRPCRouteRule{{ + Matches: []gatewayapi_v1.GRPCRouteMatch{ + { + Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "ok.service", "Login"), + }, + { + Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "foo.ok.service", "Login"), + }, + }, + BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), + }}, + }, + }, + // second GRPCRoute with 2nd oldest creationTimestamp, conflict with 1st GRPCRoute + &gatewayapi_v1.GRPCRoute{ + TypeMeta: meta_v1.TypeMeta{ + Kind: KindGRPCRoute, + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: "basic-2", + Namespace: "default", + CreationTimestamp: meta_v1.Date(2021, time.Month(2), 21, 1, 10, 30, 0, time.UTC), + }, + Spec: gatewayapi_v1.GRPCRouteSpec{ + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1.GRPCRouteRule{{ + Matches: []gatewayapi_v1.GRPCRouteMatch{ + { + Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "ok.service", "Login"), + }, + }, + BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), + }}, + }, + }, + // 3rd GRPCRoute with newest creationTimestamp, partially conflict with 1st GRPCRoute + &gatewayapi_v1.GRPCRoute{ + TypeMeta: meta_v1.TypeMeta{ + Kind: KindGRPCRoute, + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: "basic-3", + Namespace: "default", + CreationTimestamp: meta_v1.Date(2022, time.Month(2), 21, 1, 10, 30, 0, time.UTC), + }, + Spec: gatewayapi_v1.GRPCRouteSpec{ + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1.GRPCRouteRule{ + { + Matches: []gatewayapi_v1.GRPCRouteMatch{ + { + Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "foo.ok.service", "Login"), + }, + }, + BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), + }, + { + Matches: []gatewayapi_v1.GRPCRouteMatch{ + { + Method: gatewayapi.GRPCMethodMatch(gatewayapi_v1.GRPCMethodMatchExact, "bar.ok.service", "Logout"), + }, + }, + BackendRefs: gatewayapi.GRPCRouteBackendRef("kuard", 8080, 1), + }, + }, + }, + }, + }, + + wantRouteConditions: []*status.RouteStatusUpdate{ + { + FullName: types.NamespacedName{Namespace: "default", Name: "basic-1"}, + RouteParentStatuses: []*gatewayapi_v1.RouteParentStatus{ + { + ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), + Conditions: []meta_v1.Condition{ + routeResolvedRefsCondition(), + routeAcceptedGRPCRouteCondition(), + }, + }, + }, + }, + { + FullName: types.NamespacedName{Namespace: "default", Name: "basic-2"}, + RouteParentStatuses: []*gatewayapi_v1.RouteParentStatus{ + { + ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), + Conditions: []meta_v1.Condition{ + routeAcceptedFalse(status.ReasonRouteRuleMatchConflict, fmt.Sprintf(status.MessageRouteRuleMatchConflict, KindGRPCRoute, KindGRPCRoute)), + routeResolvedRefsCondition(), + }, + }, + }, + }, + { + FullName: types.NamespacedName{Namespace: "default", Name: "basic-3"}, + RouteParentStatuses: []*gatewayapi_v1.RouteParentStatus{ + { + ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"), + Conditions: []meta_v1.Condition{ + routeAcceptedGRPCRouteCondition(), + routePartialMatchConflict(status.ReasonRouteRuleMatchPartiallyConflict, fmt.Sprintf(status.MessageRouteRuleMatchPartiallyConflict, KindGRPCRoute, KindGRPCRoute)), + routeResolvedRefsCondition(), + }, + }, + }, + }, + }, + // is it ok to show the listeners are attached, just it's not accepted because of the conflict + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 3), + }) } func TestGatewayAPITCPRouteDAGStatus(t *testing.T) { @@ -12330,6 +12357,15 @@ func routeAcceptedHTTPRouteCondition() meta_v1.Condition { } } +func routePartialMatchConflict(reason gatewayapi_v1.RouteConditionReason, message string) meta_v1.Condition { + return meta_v1.Condition{ + Type: string(gatewayapi_v1.RouteConditionPartiallyInvalid), + Status: contour_v1.ConditionTrue, + Reason: string(reason), + Message: message, + } +} + func routeAcceptedFalse(reason gatewayapi_v1.RouteConditionReason, message string) meta_v1.Condition { return meta_v1.Condition{ Type: string(gatewayapi_v1.RouteConditionAccepted), diff --git a/internal/status/routeconditions.go b/internal/status/routeconditions.go index ed26405ca67..95d68557d04 100644 --- a/internal/status/routeconditions.go +++ b/internal/status/routeconditions.go @@ -41,7 +41,7 @@ const ( ReasonRouteRuleMatchPartiallyConflict gatewayapi_v1.RouteConditionReason = "RuleMatchPartiallyConflict" MessageRouteRuleMatchConflict string = "%s's Match has conflict with other %s's Match" - MessageRouteRuleMatchPartiallyConflict string = "Dropped Rule: %s's rule(s) has(ve) been dropped because of conflict against other %s's rule(s)" + MessageRouteRuleMatchPartiallyConflict string = "Dropped Rule: some of %s's rule(s) has(ve) been dropped because of conflict against other %s's rule(s)" ) // RouteStatusUpdate represents an atomic update to a