From 1b59a9bbeedeace8a1d3f800ec26681493b2fa6d Mon Sep 17 00:00:00 2001 From: Tao Yi Date: Wed, 28 Jun 2023 19:54:21 +0800 Subject: [PATCH] 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 --- CHANGELOG.md | 6 ++ .../grpcroute-example/default_golden.yaml | 42 +++++++++ .../testdata/golden/grpcroute-example/in.yaml | 33 +++++++ .../regex-path-prefix-off_golden.yaml | 42 +++++++++ .../regex-path-prefix-off_settings.yaml | 2 + .../dataplane/parser/translate_grpcroute.go | 2 +- .../dataplane/parser/translators/grpcroute.go | 14 ++- .../parser/translators/grpcroute_test.go | 86 +++++++++++++++++-- 8 files changed, 217 insertions(+), 10 deletions(-) create mode 100644 internal/dataplane/parser/testdata/golden/grpcroute-example/default_golden.yaml create mode 100644 internal/dataplane/parser/testdata/golden/grpcroute-example/in.yaml create mode 100644 internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_golden.yaml create mode 100644 internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_settings.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 6464735e8f..55c0f47104 100644 --- 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 new file mode 100644 index 0000000000..b6958b4e5a --- /dev/null +++ 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 new file mode 100644 index 0000000000..45461deb99 --- /dev/null +++ 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 \ No newline at end of file diff --git a/internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_golden.yaml b/internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_golden.yaml new file mode 100644 index 0000000000..590b0ccb73 --- /dev/null +++ b/internal/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/internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_settings.yaml b/internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_settings.yaml new file mode 100644 index 0000000000..af2e3f86e3 --- /dev/null +++ b/internal/dataplane/parser/testdata/golden/grpcroute-example/regex-path-prefix-off_settings.yaml @@ -0,0 +1,2 @@ +feature_flags: + RegexPathPrefix: false \ No newline at end of file diff --git a/internal/dataplane/parser/translate_grpcroute.go b/internal/dataplane/parser/translate_grpcroute.go index c74d58fdcc..0272f688a0 100644 --- 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 index 32adb2d67c..813c9f39bb 100644 --- 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 index 29f1cabe06..c84cafd967 100644 --- 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) }) }