fix(translate) use Ingress-specific regex prefix

Translate between an Ingress-compatible regex prefix and the Kong regex prefix. Allow users to override the default regex prefix. Add tests for regex Ingress paths using the prefix.
Kong · Sep 21, 2022 · babc24a · babc24a
1 parent 42b4b32
commit babc24a
Show file tree

Hide file tree

Showing 8 changed files with 355 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,34 @@
  - [0.0.5](#005)
  - [0.0.4 and prior](#004-and-prior)
 
+## [2.7.0]
+
+> Release date: TBD
+
+### 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)
+
 ## [2.6.0]
 
 > Release date: 2022-09-14
@@ -1934,6 +1962,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

diff --git a/internal/annotations/annotations.go b/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

diff --git a/internal/dataplane/parser/translate_ingress.go b/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,7 +69,10 @@ func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules {
 					log.Errorf("rule skipped: invalid path: '%v'", path)
 					continue
 				}
-				if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
+				if strings.HasPrefix(path, regexPrefix) {
+					path = strings.Replace(path, regexPrefix,
+						translators.KongPathRegexPrefix, 1)
+				} else if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
 					path = maybePrependRegexPrefix(path)
 				}
 				if path == "" {
@@ -200,6 +208,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 +228,21 @@ 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 := *path
+						if strings.HasPrefix(*path, regexPrefix) {
+							newPath = strings.Replace(*path, regexPrefix,
+								translators.KongPathRegexPrefix, 1)
+						} else if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
+							newPath = maybePrependRegexPrefix(*path)
 						}
+						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 {
@@ -251,7 +267,10 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 
 					for i, path := range paths {
 						newPath := *path
-						if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
+						if strings.HasPrefix(*path, regexPrefix) {
+							newPath = strings.Replace(*path, regexPrefix,
+								translators.KongPathRegexPrefix, 1)
+						} else if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
 							newPath = maybePrependRegexPrefix(*path)
 						}
 						paths[i] = &newPath

diff --git a/internal/dataplane/parser/translate_ingress_test.go b/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) {

diff --git a/internal/dataplane/parser/translate_knative.go b/internal/dataplane/parser/translate_knative.go
@@ -4,11 +4,14 @@ import (
 	"errors"
 	"fmt"
 	"sort"
+	"strings"
 
 	"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 +48,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)
@@ -61,7 +68,10 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules {
 				if path == "" {
 					path = "/"
 				}
-				if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
+				if strings.HasPrefix(path, regexPrefix) {
+					path = strings.Replace(path, regexPrefix,
+						translators.KongPathRegexPrefix, 1)
+				} else if icp.EnableLegacyRegexDetection && p.flagEnabledRegexPathPrefix {
 					path = maybePrependRegexPrefix(path)
 				}
 				r := kongstate.Route{

diff --git a/internal/dataplane/parser/translate_knative_test.go b/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)
 	})
 }
diff --git a/internal/dataplane/parser/translators/translator_vars.go b/internal/dataplane/parser/translators/translator_vars.go
@@ -3,4 +3,8 @@ package translators
 const (
 	// KongPathRegexPrefix is the reserved prefix string that instructs Kong 3.0+ to interpret a path as a regex.
 	KongPathRegexPrefix = "~"
+
+	// ControllerPathRegePrefix is the prefix string used to indicate that the controller should treat a path as a
+	// regular expression. The controller replaces this prefix with KongPathRegexPrefix when sending routes to Kong.
+	ControllerPathRegexPrefix = "/~"
 )