fix: do not prepend ~ before regex paths for Kong < 3.0 (#4238)

* fix: do not prepend ~ before regex paths for Kong < 3.0 * add tests for translating grpcroute * add CHANGELOG
Kong · Jun 28, 2023 · 1b59a9b · 1b59a9b
1 parent d73df77
commit 1b59a9b
Show file tree

Hide file tree

Showing 8 changed files with 217 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -104,6 +104,12 @@ Adding a new version? You'll need three changes:
   time but be aware that your mileage may vary).
   [#4222](https://github.com/Kong/kubernetes-ingress-controller/pull/4222)
 
+### Fixed
+
+- Translator of `GRPCRoute` generates paths without leading `~` when running
+  with Kong gateway with version below 3.0.
+  [#4238](https://github.com/Kong/kubernetes-ingress-controller/pull/4238)
+
 [gojson]: https://github.com/goccy/go-json
 
 ## [2.10.1]

diff --git a/internal/dataplane/parser/testdata/golden/grpcroute-example/default_golden.yaml b/internal/dataplane/parser/testdata/golden/grpcroute-example/default_golden.yaml
@@ -0,0 +1,42 @@
+_format_version: "3.0"
+services:
+- connect_timeout: 60000
+  host: grpcroute.default.grpcbin.0
+  name: grpcroute.default.grpcbin.0
+  plugins:
+  - config:
+      message: no existing backendRef provided
+      status_code: 500
+    name: request-termination
+  protocol: grpcs
+  read_timeout: 60000
+  retries: 5
+  routes:
+  - hosts:
+    - example.com
+    https_redirect_status_code: 426
+    name: grpcroute.default.grpcbin.0.0
+    path_handling: v0
+    paths:
+    - ~/grpcbin.GRPCBin/DummyUnary
+    protocols:
+    - grpc
+    - grpcs
+  tags:
+  - k8s-name:UNKNOWN
+  - k8s-namespace:UNKNOWN
+  - k8s-kind:Service
+  - k8s-uid:00000000-0000-0000-0000-000000000000
+  - k8s-group:core
+  - k8s-version:v1
+  write_timeout: 60000
+upstreams:
+- algorithm: round-robin
+  name: grpcroute.default.grpcbin.0
+  tags:
+  - k8s-name:UNKNOWN
+  - k8s-namespace:UNKNOWN
+  - k8s-kind:Service
+  - k8s-uid:00000000-0000-0000-0000-000000000000
+  - k8s-group:core
+  - k8s-version:v1
diff --git a/internal/dataplane/parser/testdata/golden/grpcroute-example/in.yaml b/internal/dataplane/parser/testdata/golden/grpcroute-example/in.yaml
@@ -0,0 +1,33 @@
+apiVersion: gateway.networking.k8s.io/v1alpha2
+kind: GRPCRoute
+metadata:
+  name: grpcbin
+  namespace: default
+spec:
+  parentRefs:
+  - name: kong
+  hostnames:
+  - "example.com"
+  rules:
+  - backendRefs:
+    - name: grpcbin
+      port: 443
+    matches:
+    - method:
+        service: "grpcbin.GRPCBin"
+        method: "DummyUnary"
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: grpcbin
+  namespace: default
+  labels:
+    app: grpcbin
+spec:
+  ports:
+  - name: grpc
+    port: 443
+    targetPort: 9001
+  selector:
+    app: grpcbin
diff --git a/...rnal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_golden.yaml b/...rnal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_golden.yaml
@@ -0,0 +1,42 @@
+_format_version: "3.0"
+services:
+- connect_timeout: 60000
+  host: grpcroute.default.grpcbin.0
+  name: grpcroute.default.grpcbin.0
+  plugins:
+  - config:
+      message: no existing backendRef provided
+      status_code: 500
+    name: request-termination
+  protocol: grpcs
+  read_timeout: 60000
+  retries: 5
+  routes:
+  - hosts:
+    - example.com
+    https_redirect_status_code: 426
+    name: grpcroute.default.grpcbin.0.0
+    path_handling: v0
+    paths:
+    - /grpcbin.GRPCBin/DummyUnary
+    protocols:
+    - grpc
+    - grpcs
+  tags:
+  - k8s-name:UNKNOWN
+  - k8s-namespace:UNKNOWN
+  - k8s-kind:Service
+  - k8s-uid:00000000-0000-0000-0000-000000000000
+  - k8s-group:core
+  - k8s-version:v1
+  write_timeout: 60000
+upstreams:
+- algorithm: round-robin
+  name: grpcroute.default.grpcbin.0
+  tags:
+  - k8s-name:UNKNOWN
+  - k8s-namespace:UNKNOWN
+  - k8s-kind:Service
+  - k8s-uid:00000000-0000-0000-0000-000000000000
+  - k8s-group:core
+  - k8s-version:v1
diff --git a/...al/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_settings.yaml b/...al/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_settings.yaml
@@ -0,0 +1,2 @@
+feature_flags:
+  RegexPathPrefix: false
diff --git a/internal/dataplane/parser/translate_grpcroute.go b/internal/dataplane/parser/translate_grpcroute.go
@@ -62,7 +62,7 @@ func (p *Parser) ingressRulesFromGRPCRoute(result *ingressRules, grpcroute *gate
 		if p.featureFlags.ExpressionRoutes {
 			routes = translators.GenerateKongExpressionRoutesFromGRPCRouteRule(grpcroute, ruleNumber)
 		} else {
-			routes = translators.GenerateKongRoutesFromGRPCRouteRule(grpcroute, ruleNumber)
+			routes = translators.GenerateKongRoutesFromGRPCRouteRule(grpcroute, ruleNumber, p.featureFlags.RegexPathPrefix)
 		}
 
 		// create a service and attach the routes to it

diff --git a/internal/dataplane/parser/translators/grpcroute.go b/internal/dataplane/parser/translators/grpcroute.go
@@ -34,7 +34,11 @@ func getGRPCMatchDefaults() (
 		}
 }
 
-func GenerateKongRoutesFromGRPCRouteRule(grpcroute *gatewayv1alpha2.GRPCRoute, ruleNumber int) []kongstate.Route {
+func GenerateKongRoutesFromGRPCRouteRule(
+	grpcroute *gatewayv1alpha2.GRPCRoute,
+	ruleNumber int,
+	prependRegexPrefix bool,
+) []kongstate.Route {
 	if ruleNumber >= len(grpcroute.Spec.Rules) {
 		return nil
 	}
@@ -82,7 +86,13 @@ func GenerateKongRoutesFromGRPCRouteRule(grpcroute *gatewayv1alpha2.GRPCRoute, r
 			} else {
 				service = *matchService
 			}
-			r.Paths = append(r.Paths, kong.String(fmt.Sprintf("~/%s/%s", service, method)))
+			// Kong prior to 3.0 does not accept paths starting with ~,
+			// so we should only add the path regex prefix (~) only for Kong 3.0+.
+			path := fmt.Sprintf("/%s/%s", service, method)
+			if prependRegexPrefix {
+				path = KongPathRegexPrefix + path
+			}
+			r.Paths = append(r.Paths, kong.String(path))
 		}
 
 		if len(grpcroute.Spec.Hostnames) > 0 {

diff --git a/internal/dataplane/parser/translators/grpcroute_test.go b/internal/dataplane/parser/translators/grpcroute_test.go
@@ -47,12 +47,13 @@ func makeTestGRPCRoute(
 
 func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 	testCases := []struct {
-		name           string
-		objectName     string
-		annotations    map[string]string
-		hostnames      []string
-		rule           gatewayv1alpha2.GRPCRouteRule
-		expectedRoutes []kongstate.Route
+		name               string
+		objectName         string
+		annotations        map[string]string
+		hostnames          []string
+		rule               gatewayv1alpha2.GRPCRouteRule
+		prependRegexPrefix bool
+		expectedRoutes     []kongstate.Route
 	}{
 		{
 			name:        "single match without hostname",
@@ -74,6 +75,7 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 					},
 				},
 			},
+			prependRegexPrefix: true,
 			expectedRoutes: []kongstate.Route{
 				{
 					Ingress: util.K8sObjectInfo{
@@ -108,6 +110,7 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 					},
 				},
 			},
+			prependRegexPrefix: true,
 			expectedRoutes: []kongstate.Route{
 				{
 					Ingress: util.K8sObjectInfo{
@@ -156,6 +159,7 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 					},
 				},
 			},
+			prependRegexPrefix: true,
 			expectedRoutes: []kongstate.Route{
 				{
 					Ingress: util.K8sObjectInfo{
@@ -190,13 +194,81 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:        "multiple matches with hostname, prependRegexPrefix off",
+			objectName:  "multiple-matches-with-hostname",
+			annotations: map[string]string{},
+			hostnames:   []string{"foo.com", "*.foo.com"},
+			rule: gatewayv1alpha2.GRPCRouteRule{
+				Matches: []gatewayv1alpha2.GRPCRouteMatch{
+					{
+						Method: &gatewayv1alpha2.GRPCMethodMatch{
+							Service: nil,
+							Method:  lo.ToPtr("method0"),
+						},
+						Headers: []gatewayv1alpha2.GRPCHeaderMatch{
+							{
+								Name:  "Version",
+								Value: "2",
+							},
+							{
+								Name:  "Client",
+								Value: "kong-test",
+							},
+						},
+					},
+					{
+						Method: &gatewayv1alpha2.GRPCMethodMatch{
+							Type:    lo.ToPtr(gatewayv1alpha2.GRPCMethodMatchRegularExpression),
+							Service: lo.ToPtr("v[012]"),
+						},
+					},
+				},
+			},
+			prependRegexPrefix: false,
+			expectedRoutes: []kongstate.Route{
+				{
+					Ingress: util.K8sObjectInfo{
+						Name:             "multiple-matches-with-hostname",
+						Namespace:        "default",
+						Annotations:      map[string]string{},
+						GroupVersionKind: grpcRouteGVK,
+					},
+					Route: kong.Route{
+						Name:      kong.String("grpcroute.default.multiple-matches-with-hostname.0.0"),
+						Protocols: kong.StringSlice("grpc", "grpcs"),
+						Paths:     kong.StringSlice("/.+/method0"),
+						Hosts:     kong.StringSlice("foo.com", "*.foo.com"),
+						Headers: map[string][]string{
+							"Version": {"2"},
+							"Client":  {"kong-test"},
+						},
+					},
+				},
+				{
+					Ingress: util.K8sObjectInfo{
+						Name:             "multiple-matches-with-hostname",
+						Namespace:        "default",
+						Annotations:      map[string]string{},
+						GroupVersionKind: grpcRouteGVK,
+					},
+					Route: kong.Route{
+						Name:      kong.String("grpcroute.default.multiple-matches-with-hostname.0.1"),
+						Protocols: kong.StringSlice("grpc", "grpcs"),
+						Paths:     kong.StringSlice("/v[012]/.+"),
+						Hosts:     kong.StringSlice("foo.com", "*.foo.com"),
+						Headers:   map[string][]string{},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			grpcroute := makeTestGRPCRoute(tc.objectName, "default", tc.annotations, tc.hostnames, []gatewayv1alpha2.GRPCRouteRule{tc.rule})
-			routes := GenerateKongRoutesFromGRPCRouteRule(grpcroute, 0)
+			routes := GenerateKongRoutesFromGRPCRouteRule(grpcroute, 0, tc.prependRegexPrefix)
 			require.Equal(t, tc.expectedRoutes, routes)
 		})
 	}