Skip to content

Commit

Permalink
feat(expression router): implement query parameter match of HTTPRoute (
Browse files Browse the repository at this point in the history
…#4780)

* feat: implement query parameter match of HTTPRoute

* update CHANGELOG

* update helm chart version

* update httproute validation message

* remove explanations about limit of Kong 3.4.1 and above

* remove version check in validation

* remove version cutoff

* update tests

* update comments

* add Query param match to experimental conformance tests
  • Loading branch information
randmonkey authored Oct 18, 2023
1 parent 2089f87 commit 1022266
Show file tree
Hide file tree
Showing 13 changed files with 114 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ Adding a new version? You'll need three changes:
- `--publish-service-udp` to `--ingress-service-udp`
- `--publish-status-address-udp` to `--ingress-address-udp`
[#4765](https://github.com/Kong/kubernetes-ingress-controller/pull/4765)
- Support Query Parameter matching of `HTTPRoute` when expression router enabled.
[#4780](https://github.com/Kong/kubernetes-ingress-controller/pull/4780)

[KIC Annotations reference]: https://docs.konghq.com/kubernetes-ingress-controller/latest/references/annotations/

Expand Down
11 changes: 6 additions & 5 deletions internal/admission/validation/gateway/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func ValidateHTTPRoute(
attachedGateways ...*gatewayapi.Gateway,
) (bool, string, error) {
// validate that no unsupported features are in use
if err := validateHTTPRouteFeatures(httproute); err != nil {
if err := validateHTTPRouteFeatures(httproute, parserFeatures); err != nil {
return false, "httproute spec did not pass validation", err
}

Expand Down Expand Up @@ -95,13 +95,14 @@ func validateHTTPRouteListener(listener *gatewayapi.Listener) error {
// validateHTTPRouteFeatures checks for features that are not supported by this
// HTTPRoute implementation and validates that the provided object is not using
// any of those unsupported features.
func validateHTTPRouteFeatures(httproute *gatewayapi.HTTPRoute) error {
func validateHTTPRouteFeatures(httproute *gatewayapi.HTTPRoute, parserFeatures parser.FeatureFlags) error {
for _, rule := range httproute.Spec.Rules {
for _, match := range rule.Matches {
// We don't support query parameters matching rules yet
// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/3679
// We support query parameters matching rules only with expression router.
if len(match.QueryParams) != 0 {
return fmt.Errorf("queryparam matching is not yet supported for httproute")
if !parserFeatures.ExpressionRoutes {
return fmt.Errorf("queryparam matching is supported with expression router only")
}
}
}
// We don't support any backendRef types except Kubernetes Services.
Expand Down
4 changes: 2 additions & 2 deletions internal/admission/validation/gateway/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func TestValidateHTTPRoute(t *testing.T) {
err: fmt.Errorf("HTTPRoute not supported by listener http-alternate"),
},
{
msg: "if an HTTPRoute is using queryparams matching it fails validation due to lack of support",
msg: "if an HTTPRoute is using queryparams matching it fails validation due to only supporting expression router",
route: &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: corev1.NamespaceDefault,
Expand Down Expand Up @@ -254,7 +254,7 @@ func TestValidateHTTPRoute(t *testing.T) {
}},
valid: false,
validationMsg: "httproute spec did not pass validation",
err: fmt.Errorf("queryparam matching is not yet supported for httproute"),
err: fmt.Errorf("queryparam matching is supported with expression router only"),
},
{
msg: "we don't support any group except core kubernetes for backendRefs",
Expand Down
13 changes: 13 additions & 0 deletions internal/dataplane/parser/atc/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,16 @@ func (f HTTPHeaderField) FieldType() FieldType {
func (f HTTPHeaderField) String() string {
return "http.headers." + strings.ToLower(strings.ReplaceAll(f.HeaderName, "-", "_"))
}

// HTTPQueryField extracts the value of an HTTP query parameter from the query string of the request.
type HTTPQueryField struct {
QueryParamName string
}

func (f HTTPQueryField) FieldType() FieldType {
return FieldTypeString
}

func (f HTTPQueryField) String() string {
return "http.queries." + f.QueryParamName
}
10 changes: 10 additions & 0 deletions internal/dataplane/parser/atc/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,13 @@ func NewPredicateTLSSNI(op BinaryOperator, value string) Predicate {
value: StringLiteral(value),
}
}

func NewPredicateHTTPQuery(key string, op BinaryOperator, value string) Predicate {
return Predicate{
field: HTTPQueryField{
QueryParamName: key,
},
op: op,
value: StringLiteral(value),
}
}
6 changes: 4 additions & 2 deletions internal/dataplane/parser/translate_httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func GenerateKongRouteFromTranslation(
)
}

// generateKongRoutesFromHTTPRouteMatches converts an HTTPRouteMatches to a slice of Kong Route objects.
// generateKongRoutesFromHTTPRouteMatches converts an HTTPRouteMatches to a slice of Kong Route objects with traditional routes.
// This function assumes that the HTTPRouteMatches share the query params, headers and methods.
func generateKongRoutesFromHTTPRouteMatches(
routeName string,
Expand Down Expand Up @@ -232,7 +232,9 @@ func generateKongRoutesFromHTTPRouteMatches(
return []kongstate.Route{r}, nil
}

// TODO: implement query param matches (https://github.com/Kong/kubernetes-ingress-controller/issues/2778)
// Kong supports query parameter match only with expression router.
// This function is only called for Kong with traditional/traditional compatible router,
// so we do not support query parameter match here.
if len(matches[0].QueryParams) > 0 {
return []kongstate.Route{}, translators.ErrRouteValidationQueryParamMatchesUnsupported
}
Expand Down
32 changes: 32 additions & 0 deletions internal/dataplane/parser/translators/httproute_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ func generateMatcherFromHTTPRouteMatch(match gatewayapi.HTTPRouteMatch) atc.Matc
matcher.And(headerMatcher)
}

if len(match.QueryParams) > 0 {
queryMatcher := queryParamMatcherFromHTTPQueryParamMatches(match.QueryParams)
matcher.And(queryMatcher)
}

if match.Method != nil {
method := *match.Method
methodMatcher := methodMatcherFromMethods([]string{string(method)})
Expand Down Expand Up @@ -206,6 +211,33 @@ func headerMatcherFromHTTPHeaderMatches(headerMatches []gatewayapi.HTTPHeaderMat
return atc.And(matchers...)
}

func queryParamMatcherFromHTTPQueryParamMatch(queryParamMatch gatewayapi.HTTPQueryParamMatch) atc.Matcher {
matchType := gatewayapi.QueryParamMatchExact
if queryParamMatch.Type != nil {
matchType = *queryParamMatch.Type
}
switch matchType {
case gatewayapi.QueryParamMatchExact:
return atc.NewPredicateHTTPQuery(string(queryParamMatch.Name), atc.OpEqual, queryParamMatch.Value)
case gatewayapi.QueryParamMatchRegularExpression:
return atc.NewPredicateHTTPQuery(string(queryParamMatch.Name), atc.OpRegexMatch, queryParamMatch.Value)
}
return nil // should be unreachable
}

func queryParamMatcherFromHTTPQueryParamMatches(queryParamMatches []gatewayapi.HTTPQueryParamMatch) atc.Matcher {
// Sort queryParamMatches by names to generate a stable output.
sort.Slice(queryParamMatches, func(i, j int) bool {
return string(queryParamMatches[i].Name) < string(queryParamMatches[j].Name)
})

matchers := make([]atc.Matcher, 0, len(queryParamMatches))
for _, queryParamMatch := range queryParamMatches {
matchers = append(matchers, queryParamMatcherFromHTTPQueryParamMatch(queryParamMatch))
}
return atc.And(matchers...)
}

func matchersFromParentHTTPRoute(hostnames []string, metaAnnotations map[string]string) []atc.Matcher {
// translate hostnames.
ret := []atc.Matcher{}
Expand Down
8 changes: 8 additions & 0 deletions internal/dataplane/parser/translators/httproute_atc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,14 @@ func TestGenerateMatcherFromHTTPRouteMatch(t *testing.T) {
Build(),
expression: `((http.path == "/prefix/0") || (http.path ^= "/prefix/0/")) && ((http.headers.hash ~ "[0-9A-Fa-f]{32}") && (http.headers.x_foo == "Bar"))`,
},
{
name: "prefix path match and multiple query params",
match: builder.NewHTTPRouteMatch().WithPathPrefix("/prefix/0").
WithQueryParam("foo", "bar").
WithQueryParamRegex("id", "[0-9a-z-]+").
Build(),
expression: `((http.path == "/prefix/0") || (http.path ^= "/prefix/0/")) && ((http.queries.foo == "bar") && (http.queries.id ~ "[0-9a-z-]+"))`,
},
}

for _, tc := range testCases {
Expand Down
2 changes: 2 additions & 0 deletions internal/gatewayapi/aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ const (
PathMatchExact = gatewayv1beta1.PathMatchExact
PathMatchPathPrefix = gatewayv1beta1.PathMatchPathPrefix
PathMatchRegularExpression = gatewayv1beta1.PathMatchRegularExpression
QueryParamMatchExact = gatewayv1beta1.QueryParamMatchExact
QueryParamMatchRegularExpression = gatewayv1beta1.QueryParamMatchRegularExpression
RouteConditionAccepted = gatewayv1beta1.RouteConditionAccepted
RouteConditionResolvedRefs = gatewayv1beta1.RouteConditionResolvedRefs
RouteReasonAccepted = gatewayv1beta1.RouteReasonAccepted
Expand Down
9 changes: 9 additions & 0 deletions internal/util/builder/httproutematch.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ func (b *HTTPRouteMatchBuilder) WithQueryParam(name, value string) *HTTPRouteMat
return b
}

func (b *HTTPRouteMatchBuilder) WithQueryParamRegex(name, value string) *HTTPRouteMatchBuilder {
b.httpRouteMatch.QueryParams = append(b.httpRouteMatch.QueryParams, gatewayapi.HTTPQueryParamMatch{
Type: lo.ToPtr(gatewayapi.QueryParamMatchRegularExpression),
Name: gatewayapi.HTTPHeaderName(name),
Value: value,
})
return b
}

func (b *HTTPRouteMatchBuilder) WithMethod(method gatewayapi.HTTPMethod) *HTTPRouteMatchBuilder {
b.httpRouteMatch.Method = &method
return b
Expand Down
4 changes: 2 additions & 2 deletions test/conformance/gateway_conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ var commonSkippedTests = []string{
// https://github.com/Kong/kubernetes-ingress-controller/issues/4563
tests.GatewayWithAttachedRoutesWithPort8080.ShortName,
tests.HTTPRouteRedirectPortAndScheme.ShortName,
// https://github.com/Kong/kubernetes-ingress-controller/issues/3679
tests.HTTPRouteQueryParamMatching.ShortName,
// https://github.com/Kong/kubernetes-ingress-controller/issues/3681
tests.HTTPRouteRedirectPort.ShortName,
// https://github.com/Kong/kubernetes-ingress-controller/issues/3682
Expand Down Expand Up @@ -62,6 +60,8 @@ var (
// cannot support the path > method > header precedence,
// but no way to omit individual cases.
tests.HTTPRouteMethodMatching.ShortName,
// only expression router supports query param matches
tests.HTTPRouteQueryParamMatching.ShortName,
)
)

Expand Down
1 change: 1 addition & 0 deletions test/conformance/gateway_expermimental_conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestGatewayExperimentalConformance(t *testing.T) {
SupportedFeatures: sets.New(
suite.SupportHTTPRouteMethodMatching,
suite.SupportHTTPResponseHeaderModification,
suite.SupportHTTPRouteQueryParamMatching,
),
},
ConformanceProfiles: sets.New(
Expand Down
23 changes: 23 additions & 0 deletions test/integration/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/kong/go-kong/kong"
ktfkong "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong"
"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -196,6 +197,28 @@ func TestHTTPRouteEssentials(t *testing.T) {
helpers.EventuallyGETPath(t, proxyURL, proxyURL.Host, "/", http.StatusOK, "<title>httpbin.org</title>", map[string]string{"Content-Type": "audio/mp3"}, ingressWait, waitTick)
})

t.Run("HTTPRoute query param match", func(t *testing.T) {
RunWhenKongExpressionRouter(t) //nolint:contextcheck

httpRoute, err = gatewayClient.GatewayV1beta1().HTTPRoutes(ns.Name).Get(ctx, httpRoute.Name, metav1.GetOptions{})
require.NoError(t, err)

httpRoute.Spec.Rules[0].Matches = append(httpRoute.Spec.Rules[0].Matches, gatewayapi.HTTPRouteMatch{
QueryParams: []gatewayapi.HTTPQueryParamMatch{
{
Type: lo.ToPtr(gatewayapi.QueryParamMatchExact),
Name: "foo",
Value: "bar",
},
},
})
httpRoute, err = gatewayClient.GatewayV1beta1().HTTPRoutes(ns.Name).Update(ctx, httpRoute, metav1.UpdateOptions{})
require.NoError(t, err)

t.Log("verifying HTTPRoute query param match")
helpers.EventuallyGETPath(t, proxyURL, proxyURL.Host, "/?foo=bar", http.StatusOK, "<title>httpbin.org</title>", nil, ingressWait, waitTick)
})

t.Log("verifying that the HTTPRoute has the Condition 'Accepted' set to 'True'")
require.Eventually(t, HTTPRouteMatchesAcceptedCallback(t, gatewayClient, httpRoute, true, gatewayapi.RouteReasonAccepted), statusWait, waitTick)

Expand Down

0 comments on commit 1022266

Please sign in to comment.