fix(translate) use Ingress-specific regex prefix (#2956)

Translate between an Ingress-compatible regex prefix and the Kong regex
prefix.

Allow users to override the default regex prefix.

Move non-legacy logic inside maybePrependPrefix.

Add tests for regex Ingress paths using the prefix.

CHANGELOG.md

@@ -51,6 +51,48 @@
  - [0.0.5](#005)
  - [0.0.4 and prior](#004-and-prior)
 
+## [2.7.0]
+
+> Release date: TBD
+
+2.7 patches several bugs in 2.6.0. One of these required a breaking change. The
+breaking change is not expected to affect most configurations, but does require
+a minor version bump to comply with semver. If you have not already upgraded to
+2.6, you should upgrade directly from 2.5 to 2.7, and follow the 2.6 upgrade
+instructions and the [revised Kong 3.x upgrade instructions](https://docs.konghq.com/kubernetes-ingress-controller/2.7.x/guides/upgrade-kong-3x).
+
+### Breaking changes
+
+- Ingress paths that begin with `/~` are now treated as regular expressions,
+  and are translated into a Kong route path that begins with `~` instead of
+  `/~`. To preserve the existing translation, set `konghq.com/regex-prefix` to
+  some value. For example, if you set `konghq.com/regex-prefix: /@`, paths
+  beginning with `/~` will result in route paths beginning in `/~`, whereas
+  paths beginning in `/@` will result in route paths beginning in `~`.
+  [#2956](https://github.com/Kong/kubernetes-ingress-controller/pull/2956)
+
+### Added
+
+- The controller-specific `/~` prefix translates to the Kong `~` prefix, as
+  Ingress does not allow paths that do not begin in `/`. The prefix can be
+  overriden by setting a `konghq.com/regex-prefix` annotation, for routes that
+  need their paths to actually begin with `/~`
+  [#2956](https://github.com/Kong/kubernetes-ingress-controller/pull/2956)
+
+### Fixed
+
+- The legacy regex heuristic toggle on IngressClassParameters now works when
+  the combined routes feature flag is enabled.
+  [#2942](https://github.com/Kong/kubernetes-ingress-controller/pull/2942)
+- Handles Kubernetes versions that do not support namespaced
+  IngressClassParameters without panicking. Although the controller will run on
+  clusters without the `IngressClassNamespacedParams` feature gate enabled
+  (1.21) or without it available (<1.21), these clusters do not support the
+  legacy regular expression heuristic IngressClassParameters option. These
+  versions are EOL, and we advise users to upgrade to Kubernetes 1.22 or later
+  before upgrading to KIC 2.6+ or Kong 3.0+.
+  [#2970](https://github.com/Kong/kubernetes-ingress-controller/pull/2970)
+
 ## [2.6.0]
 
 > Release date: 2022-09-14
@@ -1934,6 +1976,7 @@ Please read the changelog and test in your environment.
  - The initial versions  were rapildy iterated to deliver
    a working ingress controller.
 
+[2.7.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.6.0...v2.7.0
 [2.6.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.5.0...v2.6.0
 [2.5.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.4.2...v2.5.0
 [2.4.2]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.4.1...v2.4.2

internal/annotations/annotations.go

@@ -55,6 +55,7 @@ const (
 	RequestBuffering     = "/request-buffering"
 	ResponseBuffering    = "/response-buffering"
 	HostAliasesKey       = "/host-aliases"
+	RegexPrefixKey       = "/regex-prefix"
 
 	// GatewayClassUnmanagedAnnotationSuffix is an annotation used on a Gateway resource to
 	// indicate that the GatewayClass should be reconciled according to unmanaged

internal/dataplane/parser/translate_ingress.go

@@ -11,6 +11,7 @@ import (
 	netv1 "k8s.io/api/networking/v1"
 	netv1beta1 "k8s.io/api/networking/v1beta1"
 
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
@@ -39,6 +40,10 @@ func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules {
 	})
 
 	for _, ingress := range ingressList {
+		regexPrefix := translators.ControllerPathRegexPrefix
+		if prefix, ok := ingress.ObjectMeta.Annotations[annotations.AnnotationPrefix+annotations.RegexPrefixKey]; ok {
+			regexPrefix = prefix
+		}
 		ingressSpec := ingress.Spec
 		log := p.logger.WithFields(logrus.Fields{
 			"ingress_namespace": ingress.Namespace,
@@ -64,9 +69,7 @@ func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules {
 					log.Errorf("rule skipped: invalid path: '%v'", path)
 					continue
 				}
-				if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
-					path = maybePrependRegexPrefix(path)
-				}
+				path = maybePrependRegexPrefix(path, regexPrefix, icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix)
 				if path == "" {
 					path = "/"
 				}
@@ -200,6 +203,10 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 	})
 
 	for _, ingress := range ingressList {
+		regexPrefix := translators.ControllerPathRegexPrefix
+		if prefix, ok := ingress.ObjectMeta.Annotations[annotations.AnnotationPrefix+annotations.RegexPrefixKey]; ok {
+			regexPrefix = prefix
+		}
 		ingressSpec := ingress.Spec
 		log := p.logger.WithFields(logrus.Fields{
 			"ingress_namespace": ingress.Namespace,
@@ -216,17 +223,15 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 
 		if p.featureEnabledCombinedServiceRoutes {
 			for _, kongStateService := range translators.TranslateIngress(ingress, p.flagEnabledRegexPathPrefix) {
-				if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
-					for _, route := range kongStateService.Routes {
-						for i, path := range route.Paths {
-							newPath := maybePrependRegexPrefix(*path)
-							route.Paths[i] = &newPath
-						}
+				for _, route := range kongStateService.Routes {
+					for i, path := range route.Paths {
+						newPath := maybePrependRegexPrefix(*path, regexPrefix, icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix)
+						route.Paths[i] = &newPath
 					}
 				}
 				result.ServiceNameToServices[*kongStateService.Service.Name] = *kongStateService
+				objectSuccessfullyParsed = true
 			}
-			objectSuccessfullyParsed = true
 		} else {
 			for i, rule := range ingressSpec.Rules {
 				if rule.HTTP == nil {
@@ -250,10 +255,7 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 					}
 
 					for i, path := range paths {
-						newPath := *path
-						if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
-							newPath = maybePrependRegexPrefix(*path)
-						}
+						newPath := maybePrependRegexPrefix(*path, regexPrefix, icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix)
 						paths[i] = &newPath
 					}
 

internal/dataplane/parser/translate_ingress_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
 )
 
@@ -262,6 +263,36 @@ func TestFromIngressV1beta1(t *testing.T) {
 				},
 			},
 		},
+		// 8
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "regex-prefix",
+				Namespace: "foo-namespace",
+				Annotations: map[string]string{
+					annotations.IngressClassKey: annotations.DefaultIngressClass,
+				},
+			},
+			Spec: netv1beta1.IngressSpec{
+				Rules: []netv1beta1.IngressRule{
+					{
+						Host: "example.com",
+						IngressRuleValue: netv1beta1.IngressRuleValue{
+							HTTP: &netv1beta1.HTTPIngressRuleValue{
+								Paths: []netv1beta1.HTTPIngressPath{
+									{
+										Path: translators.ControllerPathRegexPrefix + "/foo/\\d{3}",
+										Backend: netv1beta1.IngressBackend{
+											ServiceName: "foo-svc",
+											ServicePort: intstr.FromInt(80),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	t.Run("no ingress returns empty info", func(t *testing.T) {
@@ -399,6 +430,18 @@ func TestFromIngressV1beta1(t *testing.T) {
 		parsedInfo := p.ingressRulesFromIngressV1beta1()
 		assert.Empty(parsedInfo.ServiceNameToServices)
 	})
+	t.Run("Ingress rule with regex prefixed path creates route with Kong regex prefix", func(t *testing.T) {
+		store, err := store.NewFakeStore(store.FakeObjects{
+			IngressesV1beta1: []*netv1beta1.Ingress{
+				ingressList[8],
+			},
+		})
+		require.NoError(t, err)
+		p := NewParser(logrus.New(), store)
+
+		parsedInfo := p.ingressRulesFromIngressV1beta1()
+		assert.Equal(translators.KongPathRegexPrefix+"/foo/\\d{3}", *parsedInfo.ServiceNameToServices["foo-namespace.foo-svc.80"].Routes[0].Paths[0])
+	})
 }
 
 func TestFromIngressV1(t *testing.T) {
@@ -704,6 +747,38 @@ func TestFromIngressV1(t *testing.T) {
 				},
 			},
 		},
+		// 9
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "regex-prefix",
+				Namespace: "foo-namespace",
+				Annotations: map[string]string{
+					annotations.IngressClassKey: annotations.DefaultIngressClass,
+				},
+			},
+			Spec: netv1.IngressSpec{
+				Rules: []netv1.IngressRule{
+					{
+						Host: "example.com",
+						IngressRuleValue: netv1.IngressRuleValue{
+							HTTP: &netv1.HTTPIngressRuleValue{
+								Paths: []netv1.HTTPIngressPath{
+									{
+										Path: translators.ControllerPathRegexPrefix + "/foo/\\d{3}",
+										Backend: netv1.IngressBackend{
+											Service: &netv1.IngressServiceBackend{
+												Name: "foo-svc",
+												Port: netv1.ServiceBackendPort{Number: 80},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	t.Run("no ingress returns empty info", func(t *testing.T) {
@@ -849,18 +924,28 @@ func TestFromIngressV1(t *testing.T) {
 	t.Run("Ingress rule with ports defined by name", func(t *testing.T) {
 		store, err := store.NewFakeStore(store.FakeObjects{
 			IngressesV1: []*netv1.Ingress{
-				ingressList[8],
+				ingressList[9],
 			},
 		})
 		require.NoError(t, err)
 		p := NewParser(logrus.New(), store)
 
 		parsedInfo := p.ingressRulesFromIngressV1()
-		_, ok := parsedInfo.ServiceNameToServices["foo-namespace.foo-svc.pname-http"]
-		assert.True(ok)
-		_, ok = parsedInfo.ServiceNameToServices["foo-namespace.foo-svc.pname-ws"]
+		_, ok := parsedInfo.ServiceNameToServices["foo-namespace.foo-svc.pnum-80"]
 		assert.True(ok)
 	})
+	t.Run("Ingress rule with regex prefixed path creates route with Kong regex prefix", func(t *testing.T) {
+		store, err := store.NewFakeStore(store.FakeObjects{
+			IngressesV1: []*netv1.Ingress{
+				ingressList[9],
+			},
+		})
+		require.NoError(t, err)
+		p := NewParser(logrus.New(), store)
+
+		parsedInfo := p.ingressRulesFromIngressV1()
+		assert.Equal(translators.KongPathRegexPrefix+"/foo/\\d{3}", *parsedInfo.ServiceNameToServices["foo-namespace.foo-svc.pnum-80"].Routes[0].Paths[0])
+	})
 }
 
 func TestFromIngressV1_RegexPrefix(t *testing.T) {

internal/dataplane/parser/translate_knative.go

@@ -8,7 +8,9 @@ import (
 	"github.com/kong/go-kong/kong"
 	knative "knative.dev/networking/pkg/apis/networking/v1alpha1"
 
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
 )
@@ -45,6 +47,10 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules {
 	secretToSNIs := newSecretNameToSNIs()
 
 	for _, ingress := range ingressList {
+		regexPrefix := translators.ControllerPathRegexPrefix
+		if prefix, ok := ingress.ObjectMeta.Annotations[annotations.AnnotationPrefix+annotations.RegexPrefixKey]; ok {
+			regexPrefix = prefix
+		}
 		ingressSpec := ingress.Spec
 
 		secretToSNIs.addFromIngressV1beta1TLS(knativeIngressToNetworkingTLS(ingress.Spec.TLS), ingress.Namespace)
@@ -58,12 +64,10 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules {
 			for j, rule := range rule.HTTP.Paths {
 				path := rule.Path
 
+				path = maybePrependRegexPrefix(path, regexPrefix, icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix)
 				if path == "" {
 					path = "/"
 				}
-				if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
-					path = maybePrependRegexPrefix(path)
-				}
 				r := kongstate.Route{
 					Ingress: util.FromK8sObject(ingress),
 					Route: kong.Route{

internal/dataplane/parser/translate_knative_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
+	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
 )
 
@@ -176,6 +177,43 @@ func TestFromKnativeIngress(t *testing.T) {
 				},
 			},
 		},
+		// 4
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "regex-prefix",
+				Namespace: "foo-namespace",
+				Annotations: map[string]string{
+					annotations.KnativeIngressClassKey: annotations.DefaultIngressClass,
+				},
+			},
+			Spec: knative.IngressSpec{
+				Rules: []knative.IngressRule{
+					{
+						Hosts: []string{"my-func.example.com"},
+						HTTP: &knative.HTTPIngressRuleValue{
+							Paths: []knative.HTTPIngressPath{
+								{
+									Path: translators.ControllerPathRegexPrefix + "/foo/\\d{3}",
+									AppendHeaders: map[string]string{
+										"foo": "bar",
+									},
+									Splits: []knative.IngressBackendSplit{
+										{
+											IngressBackend: knative.IngressBackend{
+												ServiceNamespace: "foo-ns",
+												ServiceName:      "foo-svc",
+												ServicePort:      intstr.FromInt(42),
+											},
+											Percent: 100,
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	t.Run("no ingress returns empty info", func(t *testing.T) {
 		store, err := store.NewFakeStore(store.FakeObjects{
@@ -304,6 +342,22 @@ func TestFromKnativeIngress(t *testing.T) {
 			},
 		}, svc.Plugins[0])
 
+		assert.Equal(newSecretNameToSNIs(), parsedInfo.SecretNameToSNIs)
+	})
+	t.Run("regex prefix translated to Kong form", func(t *testing.T) {
+		store, err := store.NewFakeStore(store.FakeObjects{
+			KnativeIngresses: []*knative.Ingress{
+				ingressList[4],
+			},
+		})
+		assert.NoError(err)
+		p := NewParser(logrus.New(), store)
+
+		parsedInfo := p.ingressRulesFromKnativeIngress()
+		assert.Equal(1, len(parsedInfo.ServiceNameToServices))
+		svc := parsedInfo.ServiceNameToServices["foo-ns.foo-svc.42"]
+		assert.Equal(translators.KongPathRegexPrefix+"/foo/\\d{3}", *svc.Routes[0].Route.Paths[0])
+
 		assert.Equal(newSecretNameToSNIs(), parsedInfo.SecretNameToSNIs)
 	})
 }