diff --git a/changelogs/unreleased/6188-lubronzhan-minor.md b/changelogs/unreleased/6188-lubronzhan-minor.md new file mode 100644 index 00000000000..73240450ad5 --- /dev/null +++ b/changelogs/unreleased/6188-lubronzhan-minor.md @@ -0,0 +1,10 @@ +## Gateway API: handle Route conflicts with HTTPRoute.Matches + +It's possible that multiple HTTPRoutes will define the same Match conditions. In this case the following logic is applied to resolve the conflict: + +- The oldest Route based on creation timestamp. For example, a Route with a creation timestamp of “2020-09-08 01:02:03” is given precedence over a Route with a creation timestamp of “2020-09-08 01:02:04”. +- The Route appearing first in alphabetical order (namespace/name) for example, foo/bar is given precedence over foo/baz. + +With above ordering, any HTTPRoute that ranks lower, will be marked with below conditions accordionly +1. If only partial rules under this HTTPRoute are conflicted, it's marked with `Accepted: True` and `PartiallyInvalid: true` Conditions and Reason: `RuleMatchPartiallyConflict`. +2. If all the rules under this HTTPRoute are conflicted, it's marked with `Accepted: False` Condition and Reason `RuleMatchConflict`. diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 85fb54a6a34..9c67318e0c8 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -751,13 +751,25 @@ type VirtualHost struct { Routes map[string]*Route } +// Add route to VirtualHosts.Routes map. func (v *VirtualHost) AddRoute(route *Route) { if v.Routes == nil { v.Routes = make(map[string]*Route) } + v.Routes[conditionsToString(route)] = route } +// HasConflictRoute returns true if there is existing Path + Headers +// + QueryParams combination match this route candidate and also they are same kind of Route. +func (v *VirtualHost) HasConflictRoute(route *Route) bool { + // If match exist and kind is the same kind, return true. + if r, ok := v.Routes[conditionsToString(route)]; ok && r.Kind == route.Kind { + return true + } + return false +} + func conditionsToString(r *Route) string { s := []string{r.PathMatchCondition.String()} for _, cond := range r.HeaderMatchConditions { diff --git a/internal/dag/dag_test.go b/internal/dag/dag_test.go index 3849ce71e50..3e161d26274 100644 --- a/internal/dag/dag_test.go +++ b/internal/dag/dag_test.go @@ -265,3 +265,200 @@ func TestServiceClusterRebalance(t *testing.T) { }) } } + +func TestHasConflictRouteForVirtualHost(t *testing.T) { + cases := map[string]struct { + vHost VirtualHost + rs []Route + expectConflict bool + }{ + "2 different routes with same path and header, with same kind, expect conflict": { + vHost: VirtualHost{ + Routes: map[string]*Route{}, + }, + rs: []Route{ + { + Kind: KindHTTPRoute, + Name: "a", + Namespace: "b", + PathMatchCondition: prefixSegment("/path1"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: ":authority", MatchType: HeaderMatchTypeRegex, Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?"}, + }, + }, + { + Kind: KindHTTPRoute, + Name: "b", + Namespace: "c", + PathMatchCondition: prefixSegment("/path1"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: ":authority", MatchType: HeaderMatchTypeRegex, Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?"}, + }, + }, + }, + expectConflict: true, + }, + "2 different routes with same path and header and query params, with same kind, expect conflict": { + vHost: VirtualHost{ + Routes: map[string]*Route{}, + }, + rs: []Route{ + { + Kind: KindHTTPRoute, + Name: "a", + Namespace: "b", + 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: KindHTTPRoute, + Name: "c", + Namespace: "d", + 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}, + }, + }, + }, + expectConflict: true, + }, + "2 different routes with same path and header, but different kind, expect no conflict": { + vHost: VirtualHost{ + Routes: map[string]*Route{}, + }, + rs: []Route{ + { + Kind: KindGRPCRoute, + Name: "a", + Namespace: "b", + PathMatchCondition: prefixSegment("/path1"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: ":authority", MatchType: HeaderMatchTypeRegex, Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?"}, + }, + }, + { + Kind: KindHTTPRoute, + Name: "b", + Namespace: "c", + PathMatchCondition: prefixSegment("/path1"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: ":authority", MatchType: HeaderMatchTypeRegex, Value: "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.example\\.com(:[0-9]+)?"}, + }, + }, + }, + expectConflict: false, + }, + "2 different routes with same path and header and query params, but different kind, expect no conflict": { + vHost: VirtualHost{ + Routes: map[string]*Route{}, + }, + rs: []Route{ + { + Kind: KindHTTPRoute, + Name: "a", + Namespace: "b", + 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: "c", + Namespace: "d", + 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}, + }, + }, + }, + expectConflict: false, + }, + "2 different routes with same path and header, but different query params, with same kind, don't expect conflict": { + vHost: VirtualHost{ + Routes: map[string]*Route{}, + }, + rs: []Route{ + { + Kind: KindHTTPRoute, + Name: "a", + Namespace: "b", + 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: KindHTTPRoute, + Name: "c", + Namespace: "d", + 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-2different", Value: "value-2different", MatchType: QueryParamMatchTypePrefix}, + }, + }, + }, + expectConflict: false, + }, + "2 different routes with different path, with same kind, don't expect conflict": { + vHost: VirtualHost{ + Routes: map[string]*Route{}, + }, + rs: []Route{ + { + Kind: KindHTTPRoute, + Name: "a", + Namespace: "b", + PathMatchCondition: prefixSegment("/path2"), + 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: KindHTTPRoute, + Name: "c", + Namespace: "d", + 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}, + }, + }, + }, + expectConflict: false, + }, + } + + for n, c := range cases { + t.Run(n, func(t *testing.T) { + c.vHost.AddRoute(&c.rs[0]) + + assert.Equal(t, c.vHost.HasConflictRoute(&c.rs[1]), c.expectConflict) + }) + } +} diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 83759524dd1..662d8ed1ffc 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -17,6 +17,7 @@ import ( "fmt" "net/http" "regexp" + "sort" "strings" "time" @@ -153,8 +154,10 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { // to each Listener so we can set status properly. listenerAttachedRoutes := map[string]int{} + // sort httproutes based on age/name first + sortedHTTPRoutes := sortRoutes(p.source.httproutes) // Process HTTPRoutes. - for _, httpRoute := range p.source.httproutes { + for _, httpRoute := range sortedHTTPRoutes { p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1.HTTPRoute{}) } @@ -1162,6 +1165,8 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener( listener *listenerInfo, hosts sets.Set[string], ) { + // Count number of rules under this Route that are invalid. + invalidRuleCnt := 0 for ruleIndex, rule := range route.Spec.Rules { // Get match conditions for the rule. var matchconditions []*matchConditions @@ -1438,21 +1443,66 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener( timeoutPolicy) } - // Add each route to the relevant vhost(s)/svhosts(s). - for host := range hosts { - for _, route := range routes { - switch { - case listener.tlsSecret != nil: - svhost := p.dag.EnsureSecureVirtualHost(listener.dagListenerName, host) - svhost.Secret = listener.tlsSecret - svhost.AddRoute(route) - default: - vhost := p.dag.EnsureVirtualHost(listener.dagListenerName, host) - vhost.AddRoute(route) + // 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 each route to the relevant vhost(s)/svhosts(s). + for host := range hosts { + for _, route := range routes { + switch { + case listener.tlsSecret != nil: + svhost := p.dag.EnsureSecureVirtualHost(listener.dagListenerName, host) + svhost.Secret = listener.tlsSecret + svhost.AddRoute(route) + default: + vhost := p.dag.EnsureVirtualHost(listener.dagListenerName, host) + vhost.AddRoute(route) + } } } + } else { + // Skip adding the routes under this rule. + invalidRuleCnt++ } } + + if invalidRuleCnt == len(route.Spec.Rules) { + // No rules under the route is valid, mark it as not accepted. + routeAccessor.AddCondition( + gatewayapi_v1.RouteConditionAccepted, + meta_v1.ConditionFalse, + status.ReasonRouteRuleMatchConflict, + status.MessageRouteRuleMatchConflict, + ) + } else if invalidRuleCnt > 0 { + routeAccessor.AddCondition( + gatewayapi_v1.RouteConditionPartiallyInvalid, + meta_v1.ConditionTrue, + status.ReasonRouteRuleMatchPartiallyConflict, + status.MessageRouteRuleMatchPartiallyConflict, + ) + } +} + +func (p *GatewayAPIProcessor) hasConflictRoute(listener *listenerInfo, hosts sets.Set[string], routes []*Route) bool { + // check if there is conflict match first + for host := range hosts { + for _, route := range routes { + switch { + case listener.tlsSecret != nil: + svhost := p.dag.EnsureSecureVirtualHost(listener.dagListenerName, host) + if svhost.HasConflictRoute(route) { + return true + } + default: + vhost := p.dag.EnsureVirtualHost(listener.dagListenerName, host) + if vhost.HasConflictRoute(route) { + return true + } + } + } + } + return false } func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1alpha2.GRPCRoute, routeAccessor *status.RouteParentStatusUpdate, listener *listenerInfo, hosts sets.Set[string]) bool { @@ -2410,3 +2460,22 @@ func handlePathRewritePrefixRemoval(p *PathRewritePolicy, mc *matchConditions) * return p } + +// sort routes based on creationTimestamp in ascending order +// if creationTimestamps are the same, sort based on namespaced name ("/") in alphetical ascending order +func sortRoutes(m map[types.NamespacedName]*gatewayapi_v1.HTTPRoute) []*gatewayapi_v1.HTTPRoute { + routes := []*gatewayapi_v1.HTTPRoute{} + for _, r := range m { + routes = append(routes, r) + } + sort.SliceStable(routes, func(i, j int) bool { + // if the creation time is the same, compare the route name + if routes[i].CreationTimestamp.Equal(&routes[j].CreationTimestamp) { + return k8s.NamespacedNameOf(routes[i]).String() < + k8s.NamespacedNameOf(routes[j]).String() + } + return routes[i].CreationTimestamp.Before(&routes[j].CreationTimestamp) + }) + + return routes +} diff --git a/internal/dag/gatewayapi_processor_test.go b/internal/dag/gatewayapi_processor_test.go index bf7e5cf71f3..c78ef4142c6 100644 --- a/internal/dag/gatewayapi_processor_test.go +++ b/internal/dag/gatewayapi_processor_test.go @@ -16,12 +16,14 @@ package dag import ( "fmt" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" core_v1 "k8s.io/api/core/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" gatewayapi_v1 "sigs.k8s.io/gateway-api/apis/v1" @@ -768,3 +770,534 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }) } } + +func TestSortRoutes(t *testing.T) { + time1 := time.Date(2021, time.Month(2), 21, 1, 10, 30, 0, time.UTC) + time2 := time.Date(2022, time.Month(2), 21, 1, 10, 30, 0, time.UTC) + time3 := time.Date(2023, time.Month(2), 21, 1, 10, 30, 0, time.UTC) + tests := []struct { + name string + m map[types.NamespacedName]*gatewayapi_v1.HTTPRoute + expected []*gatewayapi_v1.HTTPRoute + }{ + { + name: "3 httproutes, with different timestamp, earlier one should be first ", + m: map[types.NamespacedName]*gatewayapi_v1.HTTPRoute{ + { + Namespace: "ns", Name: "name1", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time3), + }, + }, + { + Namespace: "ns", Name: "name2", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time2), + }, + }, + { + Namespace: "ns", Name: "name3", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name1", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + expected: []*gatewayapi_v1.HTTPRoute{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name1", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time2), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time3), + }, + }, + }, + }, + { + name: "3 httproutes with same creation timestamps, same namespaces, smaller name comes first", + m: map[types.NamespacedName]*gatewayapi_v1.HTTPRoute{ + { + Namespace: "ns", Name: "name3", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns", Name: "name2", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns", Name: "name1", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name1", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + expected: []*gatewayapi_v1.HTTPRoute{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name1", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + }, + { + name: "3 httproutes with same creation timestamp, smaller namespaces comes first", + m: map[types.NamespacedName]*gatewayapi_v1.HTTPRoute{ + { + Namespace: "ns3", Name: "name1", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns3", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns2", Name: "name2", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns2", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns1", Name: "name3", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + expected: []*gatewayapi_v1.HTTPRoute{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns2", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns3", + Name: "name3", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + }, + { + name: "mixed order, two with same creation timestamp, two with same name", + m: map[types.NamespacedName]*gatewayapi_v1.HTTPRoute{ + { + Namespace: "ns1", Name: "name2", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time2), + }, + }, + { + Namespace: "ns2", Name: "name2", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns2", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns1", Name: "name1", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name1", + CreationTimestamp: meta_v1.NewTime(time2), + }, + }, + }, + expected: []*gatewayapi_v1.HTTPRoute{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns2", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name1", + CreationTimestamp: meta_v1.NewTime(time2), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name2", + CreationTimestamp: meta_v1.NewTime(time2), + }, + }, + }, + }, + { + name: "same name, same timestamp, different namespace", + m: map[types.NamespacedName]*gatewayapi_v1.HTTPRoute{ + { + Namespace: "ns3", Name: "name", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns3", + Name: "name", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns2", Name: "name", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns2", + Name: "name", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + Namespace: "ns1", Name: "name", + }: { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + expected: []*gatewayapi_v1.HTTPRoute{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns1", + Name: "name", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns2", + Name: "name", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "ns3", + Name: "name", + CreationTimestamp: meta_v1.NewTime(time1), + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + res := sortRoutes(tc.m) + assert.Equal(t, tc.expected, res) + }) + } +} + +func TestHasConflictRoute(t *testing.T) { + host1 := "test.projectcontour.io" + host2 := "test1.projectcontour.io" + hosts := sets.New( + host1, + host2, + ) + listener := &listenerInfo{ + listener: gatewayapi_v1.Listener{ + Name: "l1", + AllowedRoutes: &gatewayapi_v1.AllowedRoutes{ + Namespaces: &gatewayapi_v1.RouteNamespaces{ + From: ptr.To(gatewayapi_v1.NamespacesFromSame), + }, + }, + }, + dagListenerName: "l1", + allowedKinds: []gatewayapi_v1.Kind{KindHTTPRoute}, + ready: true, + } + tlsListener := &listenerInfo{ + listener: gatewayapi_v1.Listener{ + Name: "ltls", + AllowedRoutes: &gatewayapi_v1.AllowedRoutes{ + Namespaces: &gatewayapi_v1.RouteNamespaces{ + From: ptr.To(gatewayapi_v1.NamespacesFromSame), + }, + }, + }, + allowedKinds: []gatewayapi_v1.Kind{KindHTTPRoute}, + ready: true, + dagListenerName: "ltls", + tlsSecret: &Secret{}, + } + tests := []struct { + name string + existingRoutes []*Route + routes []*Route + listener *listenerInfo + 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", + 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: KindHTTPRoute, + Name: "route2", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + }, + }, + routes: []*Route{ + { + Kind: KindHTTPRoute, + 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{ + { + 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: KindHTTPRoute, + Name: "route2", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + }, + }, + routes: []*Route{ + { + Kind: KindHTTPRoute, + Name: "route3", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "e-tag", Value: "abc", MatchType: "contains", Invert: true}, + }, + }, + }, + listener: tlsListener, + }, + { + name: "There are 2 existing route, the 3rd route to add is conflict with 2nd route, listen doesn't have tls, expect conflicts", + 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]+)?"}, + }, + }, + { + Kind: KindHTTPRoute, + Name: "route2", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + }, + }, + routes: []*Route{ + { + Kind: KindHTTPRoute, + Name: "route3", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + }, + }, + listener: listener, + expectedConflict: true, + }, + { + name: "There are 1 existing route, the 2nd route to add is conflict with 1st route, listen has tls, expect conflicts", + existingRoutes: []*Route{ + { + Kind: KindHTTPRoute, + Name: "route1", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + }, + }, + routes: []*Route{ + { + Kind: KindHTTPRoute, + Name: "route2", + Namespace: "default", + PathMatchCondition: prefixSegment("/path2"), + HeaderMatchConditions: []HeaderMatchCondition{ + {Name: "version", Value: "2", MatchType: "exact", Invert: false}, + }, + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + }, + }, + listener: tlsListener, + expectedConflict: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + processor := &GatewayAPIProcessor{ + FieldLogger: fixture.NewTestLogger(t), + dag: &DAG{ + Listeners: map[string]*Listener{ + tlsListener.dagListenerName: { + svhostsByName: map[string]*SecureVirtualHost{ + host1: { + VirtualHost: VirtualHost{ + Name: host1, + Routes: make(map[string]*Route), + }, + }, + }, + }, + listener.dagListenerName: { + vhostsByName: map[string]*VirtualHost{ + host2: { + Routes: make(map[string]*Route), + }, + }, + }, + }, + }, + } + for host := range hosts { + for _, route := range tc.existingRoutes { + switch { + case tc.listener.tlsSecret != nil: + svhost := processor.dag.EnsureSecureVirtualHost(tc.listener.dagListenerName, host) + svhost.Secret = listener.tlsSecret + svhost.AddRoute(route) + default: + vhost := processor.dag.EnsureVirtualHost(tc.listener.dagListenerName, host) + vhost.AddRoute(route) + } + } + } + + res := processor.hasConflictRoute(tc.listener, hosts, tc.routes) + assert.Equal(t, tc.expectedConflict, res) + }) + } +} diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index 7ed92689c56..c6307b46d2d 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -16,6 +16,7 @@ package dag import ( "fmt" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -5510,7 +5511,8 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { "test.projectcontour.io", }, Rules: []gatewayapi_v1.HTTPRouteRule{{ - Matches: gatewayapi.HTTPRouteMatch(gatewayapi_v1.PathMatchPathPrefix, "/"), + // changed to different matches in case there is conflict with above HTTPRoute + Matches: gatewayapi.HTTPRouteMatch(gatewayapi_v1.PathMatchExact, "/"), BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), }}, }, @@ -5545,6 +5547,193 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 2), }) + run(t, "3 httproutes, but duplicate match condition between these 3. The 2 rank lower gets Conflict condition ", testcase{ + objs: []any{ + kuardService, + // first HTTPRoute with oldest creationTimestamp + &gatewayapi_v1.HTTPRoute{ + TypeMeta: meta_v1.TypeMeta{ + Kind: KindHTTPRoute, + }, + 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.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + 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", + }, + }, + QueryParams: []gatewayapi_v1.HTTPQueryParamMatch{ + { + Type: ptr.To(gatewayapi_v1.QueryParamMatchRegularExpression), + Name: "param-1", + Value: "valid-[a-z]?-[A-Za-z]+-[0=9]+-\\d+", + }, + }, + }, + { + 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("a"), + Value: "b", + }, + }, + }, + }, + + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }, + // second HTTPRoute with 2nd oldest creationTimestamp, conflict with 1st HTTPRoute + &gatewayapi_v1.HTTPRoute{ + TypeMeta: meta_v1.TypeMeta{ + Kind: KindHTTPRoute, + }, + 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.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + 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("a"), + Value: "b", + }, + }, + }, + }, + + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }, + // 3rd HTTPRoute with newest creationTimestamp, conflict with 1st HTTPRoute + &gatewayapi_v1.HTTPRoute{ + TypeMeta: meta_v1.TypeMeta{ + Kind: KindHTTPRoute, + }, + 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.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + 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", + }, + }, + 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, "HTTPRoute's Match has conflict with other HTTPRoute's Match"), + 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, "HTTPRoute's Match has conflict with other HTTPRoute's Match"), + 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), + }) + run(t, "prefix path match not starting with '/' for httproute", testcase{ objs: []any{ kuardService, diff --git a/internal/status/routeconditions.go b/internal/status/routeconditions.go index b90d212d9fb..fa9bb809366 100644 --- a/internal/status/routeconditions.go +++ b/internal/status/routeconditions.go @@ -32,11 +32,16 @@ const ( ) const ( - ReasonDegraded gatewayapi_v1.RouteConditionReason = "Degraded" - ReasonAllBackendRefsHaveZeroWeights gatewayapi_v1.RouteConditionReason = "AllBackendRefsHaveZeroWeights" - ReasonInvalidPathMatch gatewayapi_v1.RouteConditionReason = "InvalidPathMatch" - ReasonInvalidMethodMatch gatewayapi_v1.RouteConditionReason = "InvalidMethodMatch" - ReasonInvalidGateway gatewayapi_v1.RouteConditionReason = "InvalidGateway" + ReasonDegraded gatewayapi_v1.RouteConditionReason = "Degraded" + ReasonAllBackendRefsHaveZeroWeights gatewayapi_v1.RouteConditionReason = "AllBackendRefsHaveZeroWeights" + ReasonInvalidPathMatch gatewayapi_v1.RouteConditionReason = "InvalidPathMatch" + ReasonInvalidMethodMatch gatewayapi_v1.RouteConditionReason = "InvalidMethodMatch" + ReasonInvalidGateway gatewayapi_v1.RouteConditionReason = "InvalidGateway" + ReasonRouteRuleMatchConflict gatewayapi_v1.RouteConditionReason = "RuleMatchConflict" + ReasonRouteRuleMatchPartiallyConflict gatewayapi_v1.RouteConditionReason = "RuleMatchPartiallyConflict" + + MessageRouteRuleMatchConflict string = "HTTPRoute's Match has conflict with other HTTPRoute's Match" + MessageRouteRuleMatchPartiallyConflict string = "Dropped Rule: HTTPRoute's rule(s) has(ve) been dropped because of conflict against other HTTPRoute's rule(s)" ) // RouteStatusUpdate represents an atomic update to a diff --git a/test/e2e/gateway/gateway_test.go b/test/e2e/gateway/gateway_test.go index c0287e53279..506a414ed1a 100644 --- a/test/e2e/gateway/gateway_test.go +++ b/test/e2e/gateway/gateway_test.go @@ -181,6 +181,10 @@ var _ = Describe("Gateway API", func() { f.NamespacedTest("gateway-request-redirect-rule", testWithHTTPGateway(testRequestRedirectRule)) f.NamespacedTest("gateway-backend-tls-policy", testWithHTTPGateway(testBackendTLSPolicy)) + + f.NamespacedTest("gateway-httproute-conflict-match", testWithHTTPGateway(testHTTPRouteConflictMatch)) + + f.NamespacedTest("gateway-httproute-partially-conflict-match", testWithHTTPGateway(testHTTPRoutePartiallyConflictMatch)) }) Describe("Gateway with one HTTP listener and one HTTPS listener", func() { diff --git a/test/e2e/gateway/http_route_conflict_match_test.go b/test/e2e/gateway/http_route_conflict_match_test.go new file mode 100644 index 00000000000..a06458e19b4 --- /dev/null +++ b/test/e2e/gateway/http_route_conflict_match_test.go @@ -0,0 +1,89 @@ +// Copyright Project Contour Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build e2e + +package gateway + +import ( + . "github.com/onsi/ginkgo/v2" + "github.com/stretchr/testify/require" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gatewayapi_v1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/projectcontour/contour/internal/gatewayapi" + "github.com/projectcontour/contour/test/e2e" +) + +func testHTTPRouteConflictMatch(namespace string, gateway types.NamespacedName) { + Specify("Creates two http routes, second one has conflict match against the first one, report Accepted: false", func() { + By("create httproute-1 first") + route1 := &gatewayapi_v1.HTTPRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: namespace, + Name: "httproute-1", + }, + Spec: gatewayapi_v1.HTTPRouteSpec{ + Hostnames: []gatewayapi_v1.Hostname{"queryparams.gateway.projectcontour.io"}, + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{ + gatewayapi.GatewayParentRef(gateway.Namespace, gateway.Name), + }, + }, + Rules: []gatewayapi_v1.HTTPRouteRule{ + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "whale"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-1", 80, 1), + }, + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "dolphin"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-2", 80, 1), + }, + }, + }, + } + _, ok := f.CreateHTTPRouteAndWaitFor(route1, e2e.HTTPRouteAccepted) + require.True(f.T(), ok) + + By("create httproute-2 with conflicted matches") + route2 := &gatewayapi_v1.HTTPRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: namespace, + Name: "httproute-2", + }, + Spec: gatewayapi_v1.HTTPRouteSpec{ + Hostnames: []gatewayapi_v1.Hostname{"queryparams.gateway.projectcontour.io"}, + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{ + gatewayapi.GatewayParentRef(gateway.Namespace, gateway.Name), + }, + }, + Rules: []gatewayapi_v1.HTTPRouteRule{ + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "whale"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-1", 80, 1), + }, + }, + }, + } + _, ok = f.CreateHTTPRouteAndWaitFor(route2, e2e.HTTPRouteNotAcceptedDueToConflict) + require.True(f.T(), ok) + }) +} diff --git a/test/e2e/gateway/http_route_partially_conflict_match_test.go b/test/e2e/gateway/http_route_partially_conflict_match_test.go new file mode 100644 index 00000000000..4e3ae38d71a --- /dev/null +++ b/test/e2e/gateway/http_route_partially_conflict_match_test.go @@ -0,0 +1,97 @@ +// Copyright Project Contour Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build e2e + +package gateway + +import ( + . "github.com/onsi/ginkgo/v2" + "github.com/stretchr/testify/require" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gatewayapi_v1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/projectcontour/contour/internal/gatewayapi" + "github.com/projectcontour/contour/test/e2e" +) + +func testHTTPRoutePartiallyConflictMatch(namespace string, gateway types.NamespacedName) { + Specify("Creates two http routes, second one has partial conflict match against the first one, has partially match condition", func() { + By("create httproute-1 first") + route1 := &gatewayapi_v1.HTTPRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: namespace, + Name: "httproute-1", + }, + Spec: gatewayapi_v1.HTTPRouteSpec{ + Hostnames: []gatewayapi_v1.Hostname{"queryparams.gateway.projectcontour.io"}, + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{ + gatewayapi.GatewayParentRef(gateway.Namespace, gateway.Name), + }, + }, + Rules: []gatewayapi_v1.HTTPRouteRule{ + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "whale"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-1", 80, 1), + }, + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "dolphin"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-2", 80, 1), + }, + }, + }, + } + _, ok := f.CreateHTTPRouteAndWaitFor(route1, e2e.HTTPRouteAccepted) + require.True(f.T(), ok) + + By("create httproute-2 with only partially conflicted matches") + route2 := &gatewayapi_v1.HTTPRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: namespace, + Name: "httproute-2", + }, + Spec: gatewayapi_v1.HTTPRouteSpec{ + Hostnames: []gatewayapi_v1.Hostname{"queryparams.gateway.projectcontour.io"}, + CommonRouteSpec: gatewayapi_v1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1.ParentReference{ + gatewayapi.GatewayParentRef(gateway.Namespace, gateway.Name), + }, + }, + Rules: []gatewayapi_v1.HTTPRouteRule{ + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "whale"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-1", 80, 1), + }, + { + Matches: []gatewayapi_v1.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"no-conflict": "no-joke", "no-conflict-again": "no-joke"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-2", 80, 1), + }, + }, + }, + } + // Partially accepted + f.CreateHTTPRouteAndWaitFor(route2, func(*gatewayapi_v1.HTTPRoute) bool { + return e2e.HTTPRoutePartiallyInvalid(route2) && e2e.HTTPRouteAccepted(route2) + }) + }) +} diff --git a/test/e2e/gatewayapi_predicates.go b/test/e2e/gatewayapi_predicates.go index a9ac70d2ee6..22194a67e85 100644 --- a/test/e2e/gatewayapi_predicates.go +++ b/test/e2e/gatewayapi_predicates.go @@ -19,6 +19,8 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayapi_v1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapi_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/projectcontour/contour/internal/status" ) // GatewayClassAccepted returns true if the gateway has a .status.conditions @@ -119,6 +121,40 @@ func HTTPRouteAccepted(route *gatewayapi_v1.HTTPRoute) bool { return false } +// HTTPRouteNotAcceptedDueToConflict returns true if the route has a .status.conditions +// entry of "Accepted: false" && "Reason: RouteMatchConflict" && "Message: HTTPRoute's Match has +// conflict with other HTTPRoute's Match". +func HTTPRouteNotAcceptedDueToConflict(route *gatewayapi_v1.HTTPRoute) bool { + if route == nil { + return false + } + + for _, gw := range route.Status.Parents { + if conditionExistsWithAllKeys(gw.Conditions, string(gatewayapi_v1.RouteConditionAccepted), meta_v1.ConditionFalse, string(status.ReasonRouteRuleMatchConflict), status.MessageRouteRuleMatchConflict) { + return true + } + } + + return false +} + +// HTTPRoutePartiallyInvalid returns true if the route has a .status.conditions +// entry of "PartiallyInvalid: true" && "Reason: RuleMatchPartiallyConflict" && "Message: +// HTTPRoute's Match has partial conflict with other HTTPRoute's Match". +func HTTPRoutePartiallyInvalid(route *gatewayapi_v1.HTTPRoute) bool { + if route == nil { + return false + } + + for _, gw := range route.Status.Parents { + if conditionExistsWithAllKeys(gw.Conditions, string(gatewayapi_v1.RouteConditionPartiallyInvalid), meta_v1.ConditionTrue, string(status.ReasonRouteRuleMatchPartiallyConflict), status.MessageRouteRuleMatchPartiallyConflict) { + return true + } + } + + return false +} + // HTTPRouteIgnoredByContour returns true if the route has an empty .status.parents.conditions list func HTTPRouteIgnoredByContour(route *gatewayapi_v1.HTTPRoute) bool { if route == nil { @@ -194,3 +230,13 @@ func conditionExists(conditions []meta_v1.Condition, conditionType string, condi return false } + +func conditionExistsWithAllKeys(conditions []meta_v1.Condition, conditionType string, conditionStatus meta_v1.ConditionStatus, reason, message string) bool { + for _, cond := range conditions { + if cond.Type == conditionType && cond.Status == conditionStatus && cond.Reason == reason && cond.Message == message { + return true + } + } + + return false +}