add checks and validations for path segment match

internal/admission/validation/ingress/ingress.go

@@ -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"
@@ -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
@@ -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
+}

internal/dataplane/translator/subtranslator/ingress.go

@@ -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()

internal/dataplane/translator/subtranslator/ingress_atc.go

@@ -1,6 +1,7 @@
 package subtranslator
 
 import (
+	"fmt"
 	"regexp"
 	"sort"
 	"strings"
@@ -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{
@@ -77,9 +78,16 @@ func (m *ingressTranslationMeta) translateIntoKongExpressionRoute() *kongstate.R
 		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, pathSegmentPrefix))
+		pathMatcher, err := pathMatcherFromIngressPath(path, pathRegexPrefix, pathSegmentPrefix)
+		if err != nil {
+			return nil, err
+		}
+		pathMatchers = append(pathMatchers, pathMatcher)
 	}
 	routeMatcher.And(atc.Or(pathMatchers...))
 
@@ -106,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, segmentPathPrefix 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) {
@@ -145,20 +153,20 @@ 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
 		}
 		// segment match path.
 		if segmentPathPrefix != "" && strings.HasPrefix(httpIngressPath.Path, segmentPathPrefix) {
 			segmentMatchingPath := strings.TrimPrefix(httpIngressPath.Path, segmentPathPrefix)
 			return pathSegmentMatcherFromIngressMatch(segmentMatchingPath)
 		}
-		return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, httpIngressPath.Path)
+		return atc.NewPredicateHTTPPath(atc.OpPrefixMatch, httpIngressPath.Path), nil
 	}
-
-	return nil
+	// Should not reach here.
+	return nil, nil
 }
 
-func pathSegmentMatcherFromIngressMatch(path string) atc.Matcher {
+func pathSegmentMatcherFromIngressMatch(path string) (atc.Matcher, error) {
 	path = strings.Trim(path, "/")
 	segments := strings.Split(path, "/")
 	predicates := make([]atc.Matcher, 0)
@@ -173,14 +181,21 @@ func pathSegmentMatcherFromIngressMatch(path string) atc.Matcher {
 	predicates = append(predicates, segmentLenPredicate)
 
 	for index, segment := range segments {
-		if segment == "*" || segment == "**" {
+		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...)
+	return atc.And(predicates...), nil
 }
 
 // headerMatcherFromHeaders generates matcher to match headers in HTTP requests.

internal/dataplane/translator/subtranslator/ingress_atc_test.go

@@ -623,6 +623,8 @@ func TestPathMatcherFromIngressPath(t *testing.T) {
 		regexPrefix   string
 		segmentPrefix string
 		expression    string
+		expectError   bool
+		errorContains string
 	}{
 		{
 			name: "simple prefix match",
@@ -725,6 +727,16 @@ func TestPathMatcherFromIngressPath(t *testing.T) {
 			segmentPrefix: "/!",
 			expression:    `http.path.segments.len >= 0`,
 		},
+		{
+			name: "segment match with ** in the middle segments should be invalid",
+			path: netv1.HTTPIngressPath{
+				Path:     "/!/a/**/b",
+				PathType: &pathTypeImplementationSpecific,
+			},
+			segmentPrefix: "/!",
+			expectError:   true,
+			errorContains: "'**' can only appear on the last segment",
+		},
 		{
 			name: "empty implementation specific (non-regex) match",
 			path: netv1.HTTPIngressPath{
@@ -742,8 +754,14 @@ func TestPathMatcherFromIngressPath(t *testing.T) {
 			if regexPrefix == "" {
 				regexPrefix = ControllerPathRegexPrefix
 			}
-			matcher := pathMatcherFromIngressPath(tc.path, regexPrefix, tc.segmentPrefix)
-			require.Equal(t, tc.expression, matcher.Expression())
+			matcher, err := pathMatcherFromIngressPath(tc.path, regexPrefix, tc.segmentPrefix)
+			if !tc.expectError {
+				require.NoError(t, err)
+				require.Equal(t, tc.expression, matcher.Expression())
+			} else {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.errorContains)
+			}
 		})
 	}
 }

internal/versions/versions.go

@@ -9,3 +9,9 @@ var KICv3VersionCutoff = semver.Version{Major: 3, Minor: 4, Patch: 1}
 
 // DeckFileFormatVersion is the version of the decK file format used by KIC everywhere.
 const DeckFileFormatVersion = "3.0"
+
+var (
+	HTTPPathSegmentMatchVersionCutoff      = semver.Version{Major: 3, Minor: 6, Patch: 0}
+	HTTPPathSegmentMatchMinPatchVersion3_5 = semver.Version{Major: 3, Minor: 5, Patch: 1}
+	HTTPPathSegmentMatchMinPatchVersion3_4 = semver.Version{Major: 3, Minor: 4, Patch: 3}
+)