Skip to content

Commit

Permalink
implement segment match for httproute and add integration tests
Browse files Browse the repository at this point in the history
  • Loading branch information
randmonkey committed Mar 29, 2024
1 parent 73f44e9 commit adc86f6
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 49 deletions.
36 changes: 36 additions & 0 deletions internal/dataplane/translator/subtranslator/atc_utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package subtranslator

import (
"fmt"
"strings"

"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator/atc"
Expand Down Expand Up @@ -62,3 +63,38 @@ func hostMatcherFromHosts(hosts []string) atc.Matcher {
}
return atc.Or(matchers...)
}

// pathSegmentMatcher returns matcher to match path segment rules.
// a '*' represents any non-empty string in the segment.
// a '**' which can only appear in the last segment represents any path, including empty.
func pathSegmentMatcherFromPath(path string) (atc.Matcher, error) {
path = strings.Trim(path, "/")
segments := strings.Split(path, "/")
predicates := make([]atc.Matcher, 0)

numSegments := len(segments)
var segmentLenPredicate atc.Predicate
if segments[numSegments-1] == "**" {
segmentLenPredicate = atc.NewPredicateHTTPPathSegmentLength(atc.OpGreaterEqual, numSegments-1)
} else {
segmentLenPredicate = atc.NewPredicateHTTPPathSegmentLength(atc.OpEqual, numSegments)
}
predicates = append(predicates, segmentLenPredicate)

for index, segment := range segments {
if segment == "**" {
if index != numSegments-1 {
return nil, fmt.Errorf("'**' can only appear on the last segment")
}
continue
}
if segment == "*" {
continue
}

segment = strings.ReplaceAll(segment, "\\*", "*")
predicates = append(predicates, atc.NewPredicateHTTPPathSingleSegment(index, atc.OpEqual, segment))
}

return atc.And(predicates...), nil
}
44 changes: 29 additions & 15 deletions internal/dataplane/translator/subtranslator/httproute_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches(
}

// if we do not need to generate a kong route for each match, we OR matchers from all matches together.
routeMatcher := atc.And(atc.Or(generateMatchersFromHTTPRouteMatches(translation.Matches)...))
matchers, _ := generateMatchersFromHTTPRouteMatches(translation.Matches, ingressObjectInfo.Annotations)
routeMatcher := atc.And(atc.Or(matchers...))
// Add matcher from parent httproute (hostnames, SNIs) to be ANDed with the matcher from match.
matchersFromParent := matchersFromParentHTTPRoute(hostnames, ingressObjectInfo.Annotations)
for _, matcher := range matchersFromParent {
Expand Down Expand Up @@ -91,7 +92,8 @@ func generateKongExpressionRoutesWithRequestRedirectFilter(
ExpressionRoutes: true,
}
// generate matcher for this HTTPRoute Match.
matcher := atc.And(generateMatcherFromHTTPRouteMatch(match))
matcherFromMatch, _ := generateMatcherFromHTTPRouteMatch(match, ingressObjectInfo.Annotations)
matcher := atc.And(matcherFromMatch)

// add matcher from parent httproute (hostnames, protocols, SNIs) to be ANDed with the matcher from match.
matchersFromParent := matchersFromParentHTTPRoute(hostnames, ingressObjectInfo.Annotations)
Expand All @@ -113,20 +115,28 @@ func generateKongExpressionRoutesWithRequestRedirectFilter(
return routes, nil
}

func generateMatchersFromHTTPRouteMatches(matches []gatewayapi.HTTPRouteMatch) []atc.Matcher {
func generateMatchersFromHTTPRouteMatches(matches []gatewayapi.HTTPRouteMatch, routeAnnotations map[string]string) ([]atc.Matcher, []error) {
ret := make([]atc.Matcher, 0, len(matches))
errs := make([]error, 0)
for _, match := range matches {
matcher := generateMatcherFromHTTPRouteMatch(match)
matcher, err := generateMatcherFromHTTPRouteMatch(match, routeAnnotations)
if err != nil {
errs = append(errs, err)
}
ret = append(ret, matcher)
}
return ret
return ret, errs
}

func generateMatcherFromHTTPRouteMatch(match gatewayapi.HTTPRouteMatch) atc.Matcher {
func generateMatcherFromHTTPRouteMatch(match gatewayapi.HTTPRouteMatch, routeAnnotations map[string]string) (atc.Matcher, error) {
matcher := atc.And()

if match.Path != nil {
pathMatcher := pathMatcherFromHTTPPathMatch(match.Path)
segmentPrefix := annotations.ExtractSegmentPrefix(routeAnnotations)
pathMatcher, err := pathMatcherFromHTTPPathMatch(match.Path, segmentPrefix)
if err != nil {
return nil, err
}
matcher.And(pathMatcher)
}

Expand All @@ -145,7 +155,7 @@ func generateMatcherFromHTTPRouteMatch(match gatewayapi.HTTPRouteMatch) atc.Matc
methodMatcher := methodMatcherFromMethods([]string{string(method)})
matcher.And(methodMatcher)
}
return matcher
return matcher, nil
}

func appendRegexBeginIfNotExist(regex string) string {
Expand All @@ -155,33 +165,36 @@ func appendRegexBeginIfNotExist(regex string) string {
return regex
}

func pathMatcherFromHTTPPathMatch(pathMatch *gatewayapi.HTTPPathMatch) atc.Matcher {
func pathMatcherFromHTTPPathMatch(pathMatch *gatewayapi.HTTPPathMatch, segmentPrefix string) (atc.Matcher, error) {
path := ""
if pathMatch.Value != nil {
path = *pathMatch.Value
}
switch *pathMatch.Type {
case gatewayapi.PathMatchExact:
return atc.NewPredicateHTTPPath(atc.OpEqual, path)
return atc.NewPredicateHTTPPath(atc.OpEqual, path), nil
case gatewayapi.PathMatchPathPrefix:
if path == "" || path == "/" {
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/")
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/"), nil
}
// if path ends with /, we should remove the trailing / because it should be ignored:
// https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.PathMatchType
path = strings.TrimSuffix(path, "/")
return atc.Or(
atc.NewPredicateHTTPPath(atc.OpEqual, path),
atc.NewPredicateHTTPPath(atc.OpPrefixMatch, path+"/"),
)
), nil
case gatewayapi.PathMatchRegularExpression:
if segmentPrefix != "" && strings.HasPrefix(path, segmentPrefix) {
return pathSegmentMatcherFromPath(strings.TrimPrefix(path, segmentPrefix))
}
// TODO: for compatibility with kong traditional routes, here we append the ^ prefix to match the path from beginning.
// Could we allow the regex to match any part of the path?
// https://github.com/Kong/kubernetes-ingress-controller/issues/3983
return atc.NewPredicateHTTPPath(atc.OpRegexMatch, appendRegexBeginIfNotExist(path))
return atc.NewPredicateHTTPPath(atc.OpRegexMatch, appendRegexBeginIfNotExist(path)), nil
}

return nil // should be unreachable
return nil, fmt.Errorf("path match type %s not supported", *pathMatch.Type) // should be unreachable
}

func headerMatcherFromHTTPHeaderMatch(headerMatch gatewayapi.HTTPHeaderMatch) atc.Matcher {
Expand Down Expand Up @@ -575,7 +588,8 @@ func KongExpressionRouteFromHTTPRouteMatchWithPriority(
hostnames := []string{match.Hostname}
matchers := matchersFromParentHTTPRoute(hostnames, httproute.Annotations)
// generate ATC matcher from split HTTPRouteMatch itself.
matchers = append(matchers, generateMatcherFromHTTPRouteMatch(match.Match))
matcherFromMatch, _ := generateMatcherFromHTTPRouteMatch(match.Match, httproute.Annotations)
matchers = append(matchers, matcherFromMatch)

atc.ApplyExpression(&r.Route, atc.And(matchers...), httpRouteMatchWithPriority.Priority)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,9 @@ func TestGenerateMatcherFromHTTPRouteMatch(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.expression, generateMatcherFromHTTPRouteMatch(tc.match).Expression())
matcher, err := generateMatcherFromHTTPRouteMatch(tc.match, map[string]string{})
require.NoError(t, err)
require.Equal(t, tc.expression, matcher.Expression())
})
}
}
Expand Down
34 changes: 1 addition & 33 deletions internal/dataplane/translator/subtranslator/ingress_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,46 +158,14 @@ func pathMatcherFromIngressPath(httpIngressPath netv1.HTTPIngressPath, regexPath
// segment match path.
if segmentPathPrefix != "" && strings.HasPrefix(httpIngressPath.Path, segmentPathPrefix) {
segmentMatchingPath := strings.TrimPrefix(httpIngressPath.Path, segmentPathPrefix)
return pathSegmentMatcherFromIngressMatch(segmentMatchingPath)
return pathSegmentMatcherFromPath(segmentMatchingPath)
}
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, httpIngressPath.Path), nil
}
// Should not reach here.
return nil, nil
}

func pathSegmentMatcherFromIngressMatch(path string) (atc.Matcher, error) {
path = strings.Trim(path, "/")
segments := strings.Split(path, "/")
predicates := make([]atc.Matcher, 0)

numSegments := len(segments)
var segmentLenPredicate atc.Predicate
if segments[numSegments-1] == "**" {
segmentLenPredicate = atc.NewPredicateHTTPPathSegmentLength(atc.OpGreaterEqual, numSegments-1)
} else {
segmentLenPredicate = atc.NewPredicateHTTPPathSegmentLength(atc.OpEqual, numSegments)
}
predicates = append(predicates, segmentLenPredicate)

for index, segment := range segments {
if segment == "**" {
if index != numSegments-1 {
return nil, fmt.Errorf("'**' can only appear on the last segment")
}
continue
}
if segment == "*" {
continue
}

segment = strings.ReplaceAll(segment, "\\*", "*")
predicates = append(predicates, atc.NewPredicateHTTPPathSingleSegment(index, atc.OpEqual, segment))
}

return atc.And(predicates...), nil
}

// headerMatcherFromHeaders generates matcher to match headers in HTTP requests.
func headerMatcherFromHeaders(headers map[string][]string) atc.Matcher {
matchers := make([]atc.Matcher, 0, len(headers))
Expand Down
85 changes: 85 additions & 0 deletions test/integration/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,3 +675,88 @@ func TestHTTPRouteFilterHosts(t *testing.T) {
t.Logf("test host matched in httproute, but not in listeners")
require.False(t, testGetByHost(t, "another.specific.io"))
}

func TestHTTPRoutePathSegmentMatch(t *testing.T) {
RunWhenKongVersion(t, ">=3.6.0", "Path segment match is only supported in Kong 3.6.0+ with expression router")
RunWhenKongExpressionRouter(t)

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-traditional

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-gateway-alpha

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-traditional-compatible

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-traditional

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / enterprise-dbless

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / enterprise-postgres

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-gateway-alpha

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-rewrite-uris

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / postgres-rewrite-uris

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless-invalid-config

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / integration-tests / dbless

not enough arguments in call to RunWhenKongExpressionRouter

Check failure on line 681 in test/integration/httproute_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

not enough arguments in call to RunWhenKongExpressionRouter

ctx := context.Background()

ns, cleaner := helpers.Setup(ctx, t, env)

t.Log("getting a gateway client")
gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config())
require.NoError(t, err)

t.Log("deploying a new gatewayClass")
gatewayClassName := uuid.NewString()
gwc, err := helpers.DeployGatewayClass(ctx, gatewayClient, gatewayClassName)
require.NoError(t, err)
cleaner.Add(gwc)

t.Log("deploying a new gateway")
gatewayName := uuid.NewString()
gateway, err := helpers.DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayapi.Gateway) {
gw.Name = gatewayName
})
require.NoError(t, err)
cleaner.Add(gateway)

t.Log("deploying a minimal HTTP container deployment to test Ingress routes")
container := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort)
deployment := generators.NewDeploymentForContainer(container)
deployment, err = env.Cluster().Client().AppsV1().Deployments(ns.Name).Create(ctx, deployment, metav1.CreateOptions{})
require.NoError(t, err)

t.Logf("exposing deployment %s via service", deployment.Name)
service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer)
_, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{})
require.NoError(t, err)

t.Logf("creating an httproute to access deployment %s via kong", deployment.Name)
httpPort := gatewayapi.PortNumber(80)
httpRoute := &gatewayapi.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: uuid.NewString(),
Annotations: map[string]string{
annotations.AnnotationPrefix + annotations.SegmentPrefixKey: "/#",
},
},
Spec: gatewayapi.HTTPRouteSpec{
CommonRouteSpec: gatewayapi.CommonRouteSpec{
ParentRefs: []gatewayapi.ParentReference{{
Name: gatewayapi.ObjectName(gateway.Name),
}},
},
Rules: []gatewayapi.HTTPRouteRule{{
Matches: []gatewayapi.HTTPRouteMatch{
{
Path: &gatewayapi.HTTPPathMatch{
Type: lo.ToPtr(gatewayapi.PathMatchRegularExpression),
Value: lo.ToPtr("/#/status/*"),
},
},
},
BackendRefs: []gatewayapi.HTTPBackendRef{{
BackendRef: gatewayapi.BackendRef{
BackendObjectReference: gatewayapi.BackendObjectReference{
Name: gatewayapi.ObjectName(service.Name),
Port: &httpPort,
Kind: util.StringToGatewayAPIKindPtr("Service"),
},
},
}},
}},
},
}
_, err = gatewayClient.GatewayV1().HTTPRoutes(ns.Name).Create(ctx, httpRoute, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(httpRoute)

// paths that should match /status/*
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/200", http.StatusOK, "", emptyHeaderSet, ingressWait, waitTick)
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/204", http.StatusNoContent, "", emptyHeaderSet, ingressWait, waitTick)
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/404", http.StatusNotFound, "", emptyHeaderSet, ingressWait, waitTick)
// paths that should not match
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/200/aaa", http.StatusNotFound, "no Route matched", emptyHeaderSet, ingressWait, waitTick)
}
47 changes: 47 additions & 0 deletions test/integration/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,53 @@ func TestIngressRewriteURI(t *testing.T) {
require.NoError(t, <-backgroundTestError, "for Ingress without rewrite run in background")
}

func TestIngressPathSegmentMatch(t *testing.T) {
ctx := context.Background()

ns, cleaner := helpers.Setup(ctx, t, env)

t.Log("deploying a minimal HTTP container deployment to test Ingress routes")
container := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort)
deployment := generators.NewDeploymentForContainer(container)
deployment, err := env.Cluster().Client().AppsV1().Deployments(ns.Name).Create(ctx, deployment, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(deployment)

t.Logf("exposing deployment %s via service", deployment.Name)
service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer)
service, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(service)

t.Logf("creating an ingress for service %s with fixed host", service.Name)
ingress := generators.NewIngressForService("/#/status/*", map[string]string{
annotations.AnnotationPrefix + annotations.SegmentPrefixKey: "/#",
}, service)
ingress.Spec.IngressClassName = kong.String(consts.IngressClass)
for i, rule := range ingress.Spec.Rules {
for j := range rule.HTTP.Paths {
ingress.Spec.Rules[i].HTTP.Paths[j].PathType = lo.ToPtr(netv1.PathTypeImplementationSpecific)
}
}
require.NoError(t, clusters.DeployIngress(ctx, env.Cluster(), ns.Name, ingress))
cleaner.Add(ingress)

require.Eventually(t, func() bool {
lbstatus, err := clusters.GetIngressLoadbalancerStatus(ctx, env.Cluster(), ns.Name, ingress)
if err != nil {
return false
}
return len(lbstatus.Ingress) > 0
}, statusWait, waitTick)

// paths that should match /status/*
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/200", http.StatusOK, "", emptyHeaderSet, ingressWait, waitTick)
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/204", http.StatusNoContent, "", emptyHeaderSet, ingressWait, waitTick)
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/404", http.StatusNotFound, "", emptyHeaderSet, ingressWait, waitTick)
// paths that should not match
helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "/status/200/aaa", http.StatusNotFound, "no Route matched", emptyHeaderSet, ingressWait, waitTick)
}

// setIngressClassNameWithRetry changes Ingress.Spec.IngressClassName to specified value
// and retries if update conflict happens.
func setIngressClassNameWithRetry(ctx context.Context, namespace string, ingress *netv1.Ingress, ingressClassName *string) error {
Expand Down

0 comments on commit adc86f6

Please sign in to comment.