Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement translator to translate path segment matching #5734

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions internal/admission/validation/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
netv1 "k8s.io/api/networking/v1"

"github.com/kong/kubernetes-ingress-controller/v3/internal/admission/validation"
"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/failures"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator/subtranslator"
Expand Down Expand Up @@ -37,6 +38,9 @@ func ValidateIngress(
if err := validation.ValidateRouteSourceAnnotations(ingress); err != nil {
return false, fmt.Sprintf("Ingress has invalid Kong annotations: %s", err), nil
}
if err := ValidatePathSegmentMatchPrefix(ingress, translatorFeatures); err != nil {
return false, fmt.Sprintf("Ingress has invalid specification of path segment matches: %s", err), nil
}

for _, kg := range ingressToKongRoutesForValidation(translatorFeatures, ingress, failuresCollector, storer) {
kg := kg
Expand Down Expand Up @@ -87,3 +91,17 @@ func ingressToKongRoutesForValidation(
}
return kongRoutes
}

func ValidatePathSegmentMatchPrefix(ingress *netv1.Ingress, translatorFeatures translator.FeatureFlags) error {
segmentPrefix := annotations.ExtractSegmentPrefix(ingress.Annotations)
if segmentPrefix == "" {
return nil
}
if !strings.HasPrefix(segmentPrefix, "/") || len(segmentPrefix) < 2 {
return fmt.Errorf("invalid segment match prefix %s: must be started with '/' and has at least 2 characters", segmentPrefix)
}
if !translatorFeatures.ExpressionRoutes {
return fmt.Errorf("path segment match is only supported when expression routes enabled")
}
return nil
}
5 changes: 5 additions & 0 deletions internal/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const (
ResponseBuffering = "/response-buffering"
HostAliasesKey = "/host-aliases"
RegexPrefixKey = "/regex-prefix"
SegmentPrefixKey = "/segment-prefix"
ConnectTimeoutKey = "/connect-timeout"
WriteTimeoutKey = "/write-timeout"
ReadTimeoutKey = "/read-timeout"
Expand Down Expand Up @@ -212,6 +213,10 @@ func ExtractRegexPrefix(anns map[string]string) string {
return anns[AnnotationPrefix+RegexPrefixKey]
}

func ExtractSegmentPrefix(anns map[string]string) string {
return anns[AnnotationPrefix+SegmentPrefixKey]
}

// HasServiceUpstreamAnnotation returns true if the annotation
// ingress.kubernetes.io/service-upstream is set to "true" in anns.
func HasServiceUpstreamAnnotation(anns map[string]string) bool {
Expand Down
2 changes: 2 additions & 0 deletions internal/dataplane/translator/atc/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ type Predicate struct {
value Literal
}

var _ Matcher = Predicate{}

// Expression returns a string representation of a Predicate.
func (p Predicate) Expression() string {
lhs := p.field.String()
Expand Down
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
6 changes: 5 additions & 1 deletion internal/dataplane/translator/subtranslator/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,11 @@ func (i *ingressTranslationIndex) Translate() map[string]kongstate.Service {
}

if i.featureFlags.ExpressionRoutes {
route := meta.translateIntoKongExpressionRoute()
route, err := meta.translateIntoKongExpressionRoute()
if err != nil {
i.failuresCollector.PushResourceFailure(fmt.Sprintf("failed to translate to expression based route: %s", err), meta.parentIngress)
continue
}
kongStateService.Routes = append(kongStateService.Routes, *route)
} else {
route := meta.translateIntoKongRoute()
Expand Down
39 changes: 27 additions & 12 deletions internal/dataplane/translator/subtranslator/ingress_atc.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package subtranslator

import (
"fmt"
"regexp"
"sort"
"strings"
Expand Down Expand Up @@ -28,7 +29,7 @@ var (

const IngressDefaultBackendPriority RoutePriorityType = 0

func (m *ingressTranslationMeta) translateIntoKongExpressionRoute() *kongstate.Route {
func (m *ingressTranslationMeta) translateIntoKongExpressionRoute() (*kongstate.Route, error) {
// '_' is not allowed in a host, so use '_' to replace a possible occurrence of '*' since '*' is not allowed in Kong.
ingressHost := strings.ReplaceAll(m.ingressHost, "*", "_")
routeName := m.backend.intoKongRouteName(k8stypes.NamespacedName{
Expand Down Expand Up @@ -76,8 +77,17 @@ func (m *ingressTranslationMeta) translateIntoKongExpressionRoute() *kongstate.R
if pathRegexPrefix == "" {
pathRegexPrefix = ControllerPathRegexPrefix
}
pathSegmentPrefix := annotations.ExtractSegmentPrefix(ingressAnnotations)
if pathRegexPrefix == pathSegmentPrefix {
return nil, fmt.Errorf("cannot use the same prefix %s for regex match and segment match", pathRegexPrefix)
}

for _, path := range m.paths {
pathMatchers = append(pathMatchers, pathMatcherFromIngressPath(path, pathRegexPrefix))
pathMatcher, err := pathMatcherFromIngressPath(path, pathRegexPrefix, pathSegmentPrefix)
if err != nil {
return nil, err
}
pathMatchers = append(pathMatchers, pathMatcher)
}
routeMatcher.And(atc.Or(pathMatchers...))

Expand All @@ -104,34 +114,34 @@ func (m *ingressTranslationMeta) translateIntoKongExpressionRoute() *kongstate.R

priority := calculateExpressionRoutePriority(m.paths, pathRegexPrefix, m.ingressHost, ingressAnnotations)
atc.ApplyExpression(&route.Route, routeMatcher, priority)
return route
return route, nil
}

// pathMatcherFromIngressPath translate ingress path into matcher to match the path.
func pathMatcherFromIngressPath(httpIngressPath netv1.HTTPIngressPath, regexPathPrefix string) atc.Matcher {
func pathMatcherFromIngressPath(httpIngressPath netv1.HTTPIngressPath, regexPathPrefix string, segmentPathPrefix string) (atc.Matcher, error) {
switch *httpIngressPath.PathType {
// Prefix paths.
case netv1.PathTypePrefix:
base := strings.Trim(httpIngressPath.Path, "/")
if base == "" {
// empty string in prefix path matches prefix "/".
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/")
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/"), nil
}
return atc.Or(
// otherwise, match /<path>/* or /<path>.
atc.NewPredicateHTTPPath(atc.OpEqual, "/"+base),
atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/"+base+"/"),
)
), nil
// Exact paths.
case netv1.PathTypeExact:
relative := strings.TrimLeft(httpIngressPath.Path, "/")
return atc.NewPredicateHTTPPath(atc.OpEqual, "/"+relative)
return atc.NewPredicateHTTPPath(atc.OpEqual, "/"+relative), nil
// Implementation Specific match. treat it as regex match if it begins with a regex prefix (/~ by default),
// otherwise generate a prefix match.
case netv1.PathTypeImplementationSpecific:
// empty path. matches prefix "/" to match any path.
if httpIngressPath.Path == "" {
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/")
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, "/"), nil
}
// regex match.
if regexPathPrefix != "" && strings.HasPrefix(httpIngressPath.Path, regexPathPrefix) {
Expand All @@ -143,12 +153,17 @@ func pathMatcherFromIngressPath(httpIngressPath netv1.HTTPIngressPath, regexPath
if !strings.HasPrefix(regex, "^") {
regex = "^" + regex
}
return atc.NewPredicateHTTPPath(atc.OpRegexMatch, regex)
return atc.NewPredicateHTTPPath(atc.OpRegexMatch, regex), nil
}
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, httpIngressPath.Path)
// segment match path.
if segmentPathPrefix != "" && strings.HasPrefix(httpIngressPath.Path, segmentPathPrefix) {
segmentMatchingPath := strings.TrimPrefix(httpIngressPath.Path, segmentPathPrefix)
return pathSegmentMatcherFromPath(segmentMatchingPath)
}
return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, httpIngressPath.Path), nil
}

return nil
// Should not reach here.
return nil, nil
}

// headerMatcherFromHeaders generates matcher to match headers in HTTP requests.
Expand Down
Loading
Loading