From 817f85770298ad89f4e4e3c0b515a388ea9eacfc Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Thu, 23 Jun 2022 12:02:56 -0600 Subject: [PATCH] Gateway API: implement HTTP query param matching (#4588) Closes #4089. Signed-off-by: Steve Kriss --- changelogs/unreleased/4588-skriss-minor.md | 30 ++++ internal/dag/builder_test.go | 172 +++++++++++++++++++++ internal/dag/dag.go | 29 ++++ internal/dag/gatewayapi_processor.go | 60 +++++-- internal/dag/status_test.go | 55 ++++++- internal/envoy/v3/route.go | 37 ++++- internal/envoy/v3/route_test.go | 25 +++ internal/gatewayapi/helpers.go | 18 +++ internal/sorter/sorter.go | 39 +++-- internal/sorter/sorter_test.go | 34 ++++ internal/status/routeconditions.go | 1 + test/e2e/gateway/gateway_test.go | 2 + test/e2e/gateway/query_param_match_test.go | 110 +++++++++++++ 13 files changed, 580 insertions(+), 32 deletions(-) create mode 100644 changelogs/unreleased/4588-skriss-minor.md create mode 100644 test/e2e/gateway/query_param_match_test.go diff --git a/changelogs/unreleased/4588-skriss-minor.md b/changelogs/unreleased/4588-skriss-minor.md new file mode 100644 index 00000000000..c50abc92b52 --- /dev/null +++ b/changelogs/unreleased/4588-skriss-minor.md @@ -0,0 +1,30 @@ +## Gateway API: implement HTTP query parameter matching + +Contour now implements Gateway API's [HTTP query parameter matching](https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.HTTPQueryParamMatch). +Only `Exact` matching is supported. +For example, the following HTTPRoute will send a request with a query string of `?animal=whale` to `s1`, and a request with a querystring of `?animal=dolphin` to `s2`. + +```yaml +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: HTTPRoute +metadata: + name: httproute-queryparam-matching +spec: + parentRefs: + - name: contour-gateway + rules: + - matches: + - queryParams: + - type: Exact + name: animal + value: whale + backendRefs: + - name: s1 + - matches: + - queryParams: + - type: Exact + name: animal + value: dolphin + backendRefs: + - name: s2 +``` diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index a9b72f01bb3..b914aa9e198 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -2686,6 +2686,178 @@ func TestDAGInsertGatewayAPI(t *testing.T) { }, ), }, + "insert single route with single query param match without type specified and path match": { + gatewayclass: validClass, + gateway: gatewayHTTPAllNamespaces, + objs: []interface{}{ + kuardService, + &gatewayapi_v1alpha2.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha2.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1alpha2.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1alpha2.HTTPRouteRule{{ + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{{ + Path: &gatewayapi_v1alpha2.HTTPPathMatch{ + Type: gatewayapi.PathMatchTypePtr(gatewayapi_v1alpha2.PathMatchPathPrefix), + Value: pointer.StringPtr("/"), + }, + QueryParams: []gatewayapi_v1alpha2.HTTPQueryParamMatch{ + { + Name: "param-1", + Value: "value-1", + }, + }, + }}, + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }, + }, + want: listeners( + &Listener{ + Name: HTTP_LISTENER_NAME, + Port: 80, + VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io", + &Route{ + PathMatchCondition: prefixString("/"), + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + Clusters: clustersWeight(service(kuardService)), + }), + ), + }, + ), + }, + "insert single route with single query param match with type specified and path match": { + gatewayclass: validClass, + gateway: gatewayHTTPAllNamespaces, + objs: []interface{}{ + kuardService, + &gatewayapi_v1alpha2.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha2.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1alpha2.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1alpha2.HTTPRouteRule{{ + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{{ + Path: &gatewayapi_v1alpha2.HTTPPathMatch{ + Type: gatewayapi.PathMatchTypePtr(gatewayapi_v1alpha2.PathMatchPathPrefix), + Value: pointer.StringPtr("/"), + }, + QueryParams: []gatewayapi_v1alpha2.HTTPQueryParamMatch{ + { + Type: gatewayapi.QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchExact), + Name: "param-1", + Value: "value-1", + }, + }, + }}, + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }, + }, + want: listeners( + &Listener{ + Name: HTTP_LISTENER_NAME, + Port: 80, + VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io", + &Route{ + PathMatchCondition: prefixString("/"), + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + }, + Clusters: clustersWeight(service(kuardService)), + }), + ), + }, + ), + }, + "insert single route with multiple query param matches including multiple for the same key": { + gatewayclass: validClass, + gateway: gatewayHTTPAllNamespaces, + objs: []interface{}{ + kuardService, + &gatewayapi_v1alpha2.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha2.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1alpha2.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1alpha2.HTTPRouteRule{{ + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{{ + Path: &gatewayapi_v1alpha2.HTTPPathMatch{ + Type: gatewayapi.PathMatchTypePtr(gatewayapi_v1alpha2.PathMatchPathPrefix), + Value: pointer.StringPtr("/"), + }, + QueryParams: []gatewayapi_v1alpha2.HTTPQueryParamMatch{ + { + Type: gatewayapi.QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchExact), + Name: "param-1", + Value: "value-1", + }, + { + Type: gatewayapi.QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchExact), + Name: "param-2", + Value: "value-2", + }, + { + Type: gatewayapi.QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchExact), + Name: "param-1", + Value: "value-3", + }, + { + Type: gatewayapi.QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchExact), + Name: "Param-1", + Value: "value-4", + }, + }, + }}, + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }, + }, + want: listeners( + &Listener{ + Name: HTTP_LISTENER_NAME, + Port: 80, + VirtualHosts: virtualhosts(virtualhost("test.projectcontour.io", + &Route{ + PathMatchCondition: prefixString("/"), + QueryParamMatchConditions: []QueryParamMatchCondition{ + {Name: "param-1", Value: "value-1", MatchType: QueryParamMatchTypeExact}, + {Name: "param-2", Value: "value-2", MatchType: QueryParamMatchTypeExact}, + {Name: "Param-1", Value: "value-4", MatchType: QueryParamMatchTypeExact}, + }, + Clusters: clustersWeight(service(kuardService)), + }), + ), + }, + ), + }, "Route rule with request header modifier": { gatewayclass: validClass, gateway: gatewayHTTPAllNamespaces, diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 5ad64b857c4..858c0aafca2 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -153,6 +153,28 @@ func (hc *HeaderMatchCondition) String() string { return "header: " + details } +const ( + // QueryParamMatchTypeExact matches a querystring parameter value exactly. + QueryParamMatchTypeExact = "exact" +) + +// QueryParamMatchCondition matches querystring parameters by MatchType +type QueryParamMatchCondition struct { + Name string + Value string + MatchType string +} + +func (qc *QueryParamMatchCondition) String() string { + details := strings.Join([]string{ + "name=" + qc.Name, + "value=" + qc.Value, + "matchtype=", qc.MatchType, + }, "&") + + return "queryparam: " + details +} + // DirectResponse allows for a specific HTTP status code and body // to be the response to a route request vs routing to // an envoy cluster. @@ -202,6 +224,10 @@ type Route struct { // match on the request headers. HeaderMatchConditions []HeaderMatchCondition + // QueryParamMatchConditions specifies a set of additional Conditions to + // match on the querystring parameters. + QueryParamMatchConditions []QueryParamMatchCondition + Clusters []*Cluster // Should this route generate a 301 upgrade if accessed @@ -522,6 +548,9 @@ func conditionsToString(r *Route) string { for _, cond := range r.HeaderMatchConditions { s = append(s, cond.String()) } + for _, cond := range r.QueryParamMatchConditions { + s = append(s, cond.String()) + } return strings.Join(s, ",") } diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 076f088a43d..5ffa12d19dd 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -58,8 +58,9 @@ type GatewayAPIProcessor struct { // matchConditions holds match rules. type matchConditions struct { - path MatchCondition - headers []HeaderMatchCondition + path MatchCondition + headers []HeaderMatchCondition + queryParams []QueryParamMatchCondition } // Run translates Gateway API types into DAG objects and @@ -994,7 +995,7 @@ func (p *GatewayAPIProcessor) computeHTTPRoute(route *gatewayapi_v1alpha2.HTTPRo headerMatches, err := gatewayHeaderMatchConditions(match.Headers) if err != nil { - routeAccessor.AddCondition(status.ConditionNotImplemented, metav1.ConditionTrue, status.ReasonHeaderMatchType, "HTTPRoute.Spec.Rules.HeaderMatch: Only Exact match type is supported.") + routeAccessor.AddCondition(status.ConditionNotImplemented, metav1.ConditionTrue, status.ReasonHeaderMatchType, err.Error()) continue } @@ -1008,9 +1009,16 @@ func (p *GatewayAPIProcessor) computeHTTPRoute(route *gatewayapi_v1alpha2.HTTPRo }) } + queryParamMatches, err := gatewayQueryParamMatchConditions(match.QueryParams) + if err != nil { + routeAccessor.AddCondition(status.ConditionNotImplemented, metav1.ConditionTrue, status.ReasonQueryParamMatchType, err.Error()) + continue + } + matchconditions = append(matchconditions, &matchConditions{ - path: pathMatch, - headers: headerMatches, + path: pathMatch, + headers: headerMatches, + queryParams: queryParamMatches, }) } @@ -1238,7 +1246,7 @@ func gatewayHeaderMatchConditions(matches []gatewayapi_v1alpha2.HTTPHeaderMatch) case gatewayapi_v1alpha2.HeaderMatchExact: headerMatchType = HeaderMatchTypeExact default: - return nil, fmt.Errorf("HTTPRoute.Spec.Rules.HeaderMatch: Only Exact match type is supported") + return nil, fmt.Errorf("HTTPRoute.Spec.Rules.Matches.Headers: Only Exact match type is supported") } } @@ -1248,6 +1256,35 @@ func gatewayHeaderMatchConditions(matches []gatewayapi_v1alpha2.HTTPHeaderMatch) return headerMatchConditions, nil } +func gatewayQueryParamMatchConditions(matches []gatewayapi_v1alpha2.HTTPQueryParamMatch) ([]QueryParamMatchCondition, error) { + var dagMatchConditions []QueryParamMatchCondition + seenNames := sets.String{} + + for _, match := range matches { + // QueryParamMatchTypeExact is the default if not defined in the object. + queryParamMatchType := QueryParamMatchTypeExact + + if match.Type != nil && *match.Type != gatewayapi_v1alpha2.QueryParamMatchExact { + return nil, fmt.Errorf("HTTPRoute.Spec.Rules.Matches.QueryParams: Only Exact match type is supported") + } + + // If multiple match conditions are found for the same value, + // use the first one and ignore subsequent ones. + if seenNames.Has(match.Name) { + continue + } + seenNames.Insert(match.Name) + + dagMatchConditions = append(dagMatchConditions, QueryParamMatchCondition{ + MatchType: queryParamMatchType, + Name: match.Name, + Value: match.Value, + }) + } + + return dagMatchConditions, nil +} + // clusterRoutes builds a []*dag.Route for the supplied set of matchConditions, headerPolicy and backendRefs. func (p *GatewayAPIProcessor) clusterRoutes(routeNamespace string, matchConditions []*matchConditions, headerPolicy *HeadersPolicy, mirrorPolicy *MirrorPolicy, backendRefs []gatewayapi_v1alpha2.HTTPBackendRef, routeAccessor *status.RouteConditionsUpdate) []*Route { if len(backendRefs) == 0 { @@ -1309,11 +1346,12 @@ func (p *GatewayAPIProcessor) clusterRoutes(routeNamespace string, matchConditio // we create a separate route per match. for _, mc := range matchConditions { routes = append(routes, &Route{ - Clusters: clusters, - PathMatchCondition: mc.path, - HeaderMatchConditions: mc.headers, - RequestHeadersPolicy: headerPolicy, - MirrorPolicy: mirrorPolicy, + Clusters: clusters, + PathMatchCondition: mc.path, + HeaderMatchConditions: mc.headers, + QueryParamMatchConditions: mc.queryParams, + RequestHeadersPolicy: headerPolicy, + MirrorPolicy: mirrorPolicy, }) } diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index fce6d0177ad..22b6379b81c 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -3343,7 +3343,60 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { Type: string(status.ConditionNotImplemented), Status: contour_api_v1.ConditionTrue, Reason: string(status.ReasonHeaderMatchType), - Message: "HTTPRoute.Spec.Rules.HeaderMatch: Only Exact match type is supported.", + Message: "HTTPRoute.Spec.Rules.Matches.Headers: Only Exact match type is supported", + }, + }, + }}, + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", "HTTPRoute", 0), + }) + + run(t, "RegularExpression query param match not supported for httproute", testcase{ + objs: []interface{}{ + kuardService, + &gatewayapi_v1alpha2.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "default", + }, + Spec: gatewayapi_v1alpha2.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{gatewayapi.GatewayParentRef("projectcontour", "contour")}, + }, + Hostnames: []gatewayapi_v1alpha2.Hostname{ + "test.projectcontour.io", + }, + Rules: []gatewayapi_v1alpha2.HTTPRouteRule{{ + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{{ + Path: &gatewayapi_v1alpha2.HTTPPathMatch{ + Type: gatewayapi.PathMatchTypePtr(gatewayapi_v1alpha2.PathMatchPathPrefix), + Value: pointer.StringPtr("/"), + }, + QueryParams: []gatewayapi_v1alpha2.HTTPQueryParamMatch{ + { + Type: gatewayapi.QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchRegularExpression), // <---- RegularExpression type not yet supported + Name: "param-1", + Value: "value-1", + }, + }, + }}, + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }}, + wantRouteConditions: []*status.RouteConditionsUpdate{{ + FullName: types.NamespacedName{Namespace: "default", Name: "basic"}, + Conditions: map[gatewayapi_v1alpha2.RouteConditionType]metav1.Condition{ + gatewayapi_v1alpha2.RouteConditionAccepted: { + Type: string(gatewayapi_v1alpha2.RouteConditionAccepted), + Status: contour_api_v1.ConditionFalse, + Reason: string(status.ReasonErrorsExist), + Message: "Errors found, check other Conditions for details.", + }, + status.ConditionNotImplemented: { + Type: string(status.ConditionNotImplemented), + Status: contour_api_v1.ConditionTrue, + Reason: string(status.ReasonQueryParamMatchType), + Message: "HTTPRoute.Spec.Rules.Matches.QueryParams: Only Exact match type is supported", }, }, }}, diff --git a/internal/envoy/v3/route.go b/internal/envoy/v3/route.go index db344cb0cff..a9d3be60054 100644 --- a/internal/envoy/v3/route.go +++ b/internal/envoy/v3/route.go @@ -159,7 +159,8 @@ func RouteMatch(route *dag.Route) *envoy_route_v3.RouteMatch { // Reduces regex program size so Envoy doesn't reject long prefix matches. SafeRegex: SafeRegexMatch("^" + c.Regex), }, - Headers: headerMatcher(route.HeaderMatchConditions), + Headers: headerMatcher(route.HeaderMatchConditions), + QueryParameters: queryParamMatcher(route.QueryParamMatchConditions), } case *dag.PrefixMatchCondition: switch c.PrefixMatchType { @@ -168,7 +169,8 @@ func RouteMatch(route *dag.Route) *envoy_route_v3.RouteMatch { PathSpecifier: &envoy_route_v3.RouteMatch_PathSeparatedPrefix{ PathSeparatedPrefix: c.Prefix, }, - Headers: headerMatcher(route.HeaderMatchConditions), + Headers: headerMatcher(route.HeaderMatchConditions), + QueryParameters: queryParamMatcher(route.QueryParamMatchConditions), } case dag.PrefixMatchString: fallthrough @@ -177,7 +179,8 @@ func RouteMatch(route *dag.Route) *envoy_route_v3.RouteMatch { PathSpecifier: &envoy_route_v3.RouteMatch_Prefix{ Prefix: c.Prefix, }, - Headers: headerMatcher(route.HeaderMatchConditions), + Headers: headerMatcher(route.HeaderMatchConditions), + QueryParameters: queryParamMatcher(route.QueryParamMatchConditions), } } case *dag.ExactMatchCondition: @@ -185,11 +188,13 @@ func RouteMatch(route *dag.Route) *envoy_route_v3.RouteMatch { PathSpecifier: &envoy_route_v3.RouteMatch_Path{ Path: c.Path, }, - Headers: headerMatcher(route.HeaderMatchConditions), + Headers: headerMatcher(route.HeaderMatchConditions), + QueryParameters: queryParamMatcher(route.QueryParamMatchConditions), } default: return &envoy_route_v3.RouteMatch{ - Headers: headerMatcher(route.HeaderMatchConditions), + Headers: headerMatcher(route.HeaderMatchConditions), + QueryParameters: queryParamMatcher(route.QueryParamMatchConditions), } } } @@ -562,6 +567,28 @@ func headerMatcher(headers []dag.HeaderMatchCondition) []*envoy_route_v3.HeaderM return envoyHeaders } +func queryParamMatcher(queryParams []dag.QueryParamMatchCondition) []*envoy_route_v3.QueryParameterMatcher { + var envoyQueryParamMatchers []*envoy_route_v3.QueryParameterMatcher + + for _, q := range queryParams { + queryParam := &envoy_route_v3.QueryParameterMatcher{ + Name: q.Name, + } + + if q.MatchType == dag.QueryParamMatchTypeExact { + queryParam.QueryParameterMatchSpecifier = &envoy_route_v3.QueryParameterMatcher_StringMatch{ + StringMatch: &matcher.StringMatcher{ + MatchPattern: &matcher.StringMatcher_Exact{Exact: q.Value}, + }, + } + } + + envoyQueryParamMatchers = append(envoyQueryParamMatchers, queryParam) + } + + return envoyQueryParamMatchers +} + // containsMatch returns a HeaderMatchSpecifier which will match the // supplied substring func containsMatch(s string) *envoy_route_v3.HeaderMatcher_StringMatch { diff --git a/internal/envoy/v3/route_test.go b/internal/envoy/v3/route_test.go index 3df52b23517..f619c084f02 100644 --- a/internal/envoy/v3/route_test.go +++ b/internal/envoy/v3/route_test.go @@ -1286,6 +1286,31 @@ func TestRouteMatch(t *testing.T) { }}, }, }, + "query param exact match": { + route: &dag.Route{ + QueryParamMatchConditions: []dag.QueryParamMatchCondition{ + { + Name: "query-param-1", + Value: "query-value-1", + MatchType: "exact", + }, + }, + }, + want: &envoy_route_v3.RouteMatch{ + QueryParameters: []*envoy_route_v3.QueryParameterMatcher{ + { + Name: "query-param-1", + QueryParameterMatchSpecifier: &envoy_route_v3.QueryParameterMatcher_StringMatch{ + StringMatch: &matcher.StringMatcher{ + MatchPattern: &matcher.StringMatcher_Exact{ + Exact: "query-value-1", + }, + }, + }, + }, + }, + }, + }, } for name, tc := range tests { diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go index c895b660b8c..6ba15f5e6fd 100644 --- a/internal/gatewayapi/helpers.go +++ b/internal/gatewayapi/helpers.go @@ -40,6 +40,10 @@ func HeaderMatchTypePtr(val gatewayapi_v1alpha2.HeaderMatchType) *gatewayapi_v1a return &val } +func QueryParamMatchTypePtr(val gatewayapi_v1alpha2.QueryParamMatchType) *gatewayapi_v1alpha2.QueryParamMatchType { + return &val +} + func TLSModeTypePtr(mode gatewayapi_v1alpha2.TLSModeType) *gatewayapi_v1alpha2.TLSModeType { return &mode } @@ -154,6 +158,20 @@ func HTTPHeaderMatch(matchType gatewayapi_v1alpha2.HeaderMatchType, name, value } } +func HTTPQueryParamMatches(namesAndValues map[string]string) []gatewayapi_v1alpha2.HTTPQueryParamMatch { + var matches []gatewayapi_v1alpha2.HTTPQueryParamMatch + + for name, val := range namesAndValues { + matches = append(matches, gatewayapi_v1alpha2.HTTPQueryParamMatch{ + Type: QueryParamMatchTypePtr(gatewayapi_v1alpha2.QueryParamMatchExact), + Name: name, + Value: val, + }) + } + + return matches +} + func HTTPBackendRefs(backendRefs ...[]gatewayapi_v1alpha2.HTTPBackendRef) []gatewayapi_v1alpha2.HTTPBackendRef { var res []gatewayapi_v1alpha2.HTTPBackendRef diff --git a/internal/sorter/sorter.go b/internal/sorter/sorter.go index e7f45bcd6ed..f6f2fa26685 100644 --- a/internal/sorter/sorter.go +++ b/internal/sorter/sorter.go @@ -107,23 +107,32 @@ func (s headerMatchConditionSorter) Less(i, j int) bool { } } -// longestRouteByHeaderConditions compares the HeaderMatchCondition slices for -// lhs and rhs and returns true if lhs is longer. -func longestRouteByHeaderConditions(lhs, rhs *dag.Route) bool { - if len(lhs.HeaderMatchConditions) == len(rhs.HeaderMatchConditions) { - pair := make([]dag.HeaderMatchCondition, 2) +// longestRouteByHeaderAndQueryParamConditions compares the HeaderMatchConditions +// and QueryParamMatchConditions slices for lhs and rhs and returns true if lhs is +// longer. +func longestRouteByHeaderAndQueryParamConditions(lhs, rhs *dag.Route) bool { + // One route has a longer HeaderMatchConditions slice. + if len(lhs.HeaderMatchConditions) != len(rhs.HeaderMatchConditions) { + return len(lhs.HeaderMatchConditions) > len(rhs.HeaderMatchConditions) + } - for i := 0; i < len(lhs.HeaderMatchConditions); i++ { - pair[0] = lhs.HeaderMatchConditions[i] - pair[1] = rhs.HeaderMatchConditions[i] + // HeaderMatchConditions are equal length: compare item by item. + pair := make([]dag.HeaderMatchCondition, 2) - if headerMatchConditionSorter(pair).Less(0, 1) { - return true - } + for i := 0; i < len(lhs.HeaderMatchConditions); i++ { + pair[0] = lhs.HeaderMatchConditions[i] + pair[1] = rhs.HeaderMatchConditions[i] + + if headerMatchConditionSorter(pair).Less(0, 1) { + return true } } - return len(lhs.HeaderMatchConditions) > len(rhs.HeaderMatchConditions) + // If there is no difference in the header match conditions, compare the length + // of the query param match conditions. Note that this is sufficient for implementing + // the Gateway API spec, but this may need to be enhanced if we add query param + // match support to HTTPProxy. + return len(lhs.QueryParamMatchConditions) > len(rhs.QueryParamMatchConditions) } // Sorts the given Route slice in place. Routes are ordered first by @@ -148,7 +157,7 @@ func (s routeSorter) Less(i, j int) bool { return false default: if a.PrefixMatchType == b.PrefixMatchType { - return longestRouteByHeaderConditions(s[i], s[j]) + return longestRouteByHeaderAndQueryParamConditions(s[i], s[j]) } // Segment prefixes sort first as they are more specific. return a.PrefixMatchType == dag.PrefixMatchSegment @@ -165,7 +174,7 @@ func (s routeSorter) Less(i, j int) bool { case -1: return false default: - return longestRouteByHeaderConditions(s[i], s[j]) + return longestRouteByHeaderAndQueryParamConditions(s[i], s[j]) } case *dag.PrefixMatchCondition: return true @@ -181,7 +190,7 @@ func (s routeSorter) Less(i, j int) bool { case -1: return false default: - return longestRouteByHeaderConditions(s[i], s[j]) + return longestRouteByHeaderAndQueryParamConditions(s[i], s[j]) } case *dag.PrefixMatchCondition: return true diff --git a/internal/sorter/sorter_test.go b/internal/sorter/sorter_test.go index 1e8d09d6912..854a865bf2d 100644 --- a/internal/sorter/sorter_test.go +++ b/internal/sorter/sorter_test.go @@ -283,6 +283,40 @@ func TestSortRoutesLongestHeaders(t *testing.T) { assert.Equal(t, want, have) } +func TestSortRoutesMostQueryParams(t *testing.T) { + want := []*dag.Route{ + { + + PathMatchCondition: matchExact("/"), + QueryParamMatchConditions: []dag.QueryParamMatchCondition{ + {Name: "query-param-1", Value: "query-value-1"}, + {Name: "query-param-2", Value: "query-value-2"}, + {Name: "query-param-3", Value: "query-value-3"}, + }, + }, + { + + PathMatchCondition: matchExact("/"), + QueryParamMatchConditions: []dag.QueryParamMatchCondition{ + {Name: "query-param-1", Value: "query-value-1"}, + {Name: "query-param-2", Value: "query-value-2"}, + }, + }, + { + + PathMatchCondition: matchExact("/"), + QueryParamMatchConditions: []dag.QueryParamMatchCondition{ + {Name: "query-param-1", Value: "query-value-1"}, + }, + }, + } + + have := shuffleRoutes(want) + + sort.Stable(For(have)) + assert.Equal(t, want, have) +} + func TestSortSecrets(t *testing.T) { want := []*envoy_tls_v3.Secret{ {Name: "first"}, diff --git a/internal/status/routeconditions.go b/internal/status/routeconditions.go index 581c08351f1..5c843834719 100644 --- a/internal/status/routeconditions.go +++ b/internal/status/routeconditions.go @@ -33,6 +33,7 @@ type RouteReasonType string const ReasonNotImplemented RouteReasonType = "NotImplemented" const ReasonPathMatchType RouteReasonType = "PathMatchType" const ReasonHeaderMatchType RouteReasonType = "HeaderMatchType" +const ReasonQueryParamMatchType RouteReasonType = "QueryParamMatchType" const ReasonHTTPRouteFilterType RouteReasonType = "HTTPRouteFilterType" const ReasonDegraded RouteReasonType = "Degraded" const ReasonValid RouteReasonType = "Valid" diff --git a/test/e2e/gateway/gateway_test.go b/test/e2e/gateway/gateway_test.go index 10b4dca1fa8..0b06b5afc65 100644 --- a/test/e2e/gateway/gateway_test.go +++ b/test/e2e/gateway/gateway_test.go @@ -202,6 +202,8 @@ func runGatewayTests() { f.NamespacedTest("gateway-header-condition-match", testWithHTTPGateway(testGatewayHeaderConditionMatch)) + f.NamespacedTest("gateway-query-param-match", testWithHTTPGateway(testGatewayQueryParamMatch)) + f.NamespacedTest("gateway-invalid-forward-to", testWithHTTPGateway(testInvalidBackendRef)) f.NamespacedTest("gateway-request-header-modifier-forward-to", testWithHTTPGateway(testRequestHeaderModifierForwardTo)) diff --git a/test/e2e/gateway/query_param_match_test.go b/test/e2e/gateway/query_param_match_test.go new file mode 100644 index 00000000000..7a17f25c452 --- /dev/null +++ b/test/e2e/gateway/query_param_match_test.go @@ -0,0 +1,110 @@ +// 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 +// +build e2e + +package gateway + +import ( + . "github.com/onsi/ginkgo/v2" + "github.com/projectcontour/contour/internal/gatewayapi" + "github.com/projectcontour/contour/test/e2e" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayapi_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" +) + +func testGatewayQueryParamMatch(namespace string) { + Specify("query param matching works", func() { + t := f.T() + + f.Fixtures.Echo.Deploy(namespace, "echo-1") + f.Fixtures.Echo.Deploy(namespace, "echo-2") + f.Fixtures.Echo.Deploy(namespace, "echo-3") + f.Fixtures.Echo.Deploy(namespace, "echo-4") + + route := &gatewayapi_v1alpha2.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "httproute-1", + }, + Spec: gatewayapi_v1alpha2.HTTPRouteSpec{ + Hostnames: []gatewayapi_v1alpha2.Hostname{"queryparams.gateway.projectcontour.io"}, + CommonRouteSpec: gatewayapi_v1alpha2.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1alpha2.ParentReference{ + gatewayapi.GatewayParentRef("", "http"), + }, + }, + Rules: []gatewayapi_v1alpha2.HTTPRouteRule{ + { + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "whale"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-1", 80, 1), + }, + { + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "dolphin"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-2", 80, 1), + }, + { + Matches: []gatewayapi_v1alpha2.HTTPRouteMatch{ + {QueryParams: gatewayapi.HTTPQueryParamMatches(map[string]string{"animal": "dolphin", "color": "red"})}, + }, + BackendRefs: gatewayapi.HTTPBackendRef("echo-3", 80, 1), + }, + { + Matches: gatewayapi.HTTPRouteMatch(gatewayapi_v1alpha2.PathMatchPathPrefix, "/"), + BackendRefs: gatewayapi.HTTPBackendRef("echo-4", 80, 1), + }, + }, + }, + } + f.CreateHTTPRouteAndWaitFor(route, httpRouteAccepted) + + cases := map[string]string{ + "/?animal=whale": "echo-1", + "/?animal=whale&foo=bar": "echo-1", // extra irrelevant parameters have no impact + "/?animal=whale&animal=dolphin": "echo-1", // the first occurrence of a given key in a querystring is used for matching + "/?animal=dolphin": "echo-2", + "/?animal=dolphin&animal=whale": "echo-2", // the first occurrence of a given key in a querystring is used for matching + "/?animal=dolphin&color=blue": "echo-2", // all matches must match for a route to be selected + "/?animal=dolphin&color=red": "echo-3", + "/?animal=dolphin&color=red&foo=bar": "echo-3", // extra irrelevant parameters have no impact + "/?animal=horse": "echo-4", // non-matching values do not match + "/?animal=whalesay": "echo-4", // value matching is exact, not prefix + "/?animal=bluedolphin": "echo-4", // value matching is exact, not suffix + "/?color=blue": "echo-4", + "/?nomatch=true": "echo-4", + } + + for path, expectedService := range cases { + t.Logf("Querying %q, expecting service %q", path, expectedService) + + res, ok := f.HTTP.RequestUntil(&e2e.HTTPRequestOpts{ + Host: string(route.Spec.Hostnames[0]), + Path: path, + Condition: e2e.HasStatusCode(200), + }) + if !assert.Truef(t, ok, "expected 200 response code, got %d", res.StatusCode) { + continue + } + + body := f.GetEchoResponseBody(res.Body) + assert.Equal(t, namespace, body.Namespace) + assert.Equal(t, expectedService, body.Service) + } + }) +}