From ce7e0bb4850604f57e350e24a62db3ba56a804ed Mon Sep 17 00:00:00 2001 From: dpasiukevich Date: Wed, 3 Jan 2024 18:46:43 +0000 Subject: [PATCH] Istio provider: translate istio rewrite clause to HTTPRouteFilterURLRewrite. --- pkg/i2gw/providers/istio/README.md | 14 ++ pkg/i2gw/providers/istio/converter.go | 168 +++++++++++++----- pkg/i2gw/providers/istio/converter_test.go | 4 +- .../fixtures/input/2-virtualservice-http.yaml | 9 +- .../fixtures/input/4-virtualservice-tcp.yaml | 2 +- .../input/6-virtualservice-http-rewrite.yaml | 58 ++++++ .../output/2-virtualservice-http.yaml | 5 - .../fixtures/output/3-virtualservice-tls.yaml | 1 - .../output/6-virtualservice-http-rewrite.yaml | 75 ++++++++ 9 files changed, 278 insertions(+), 58 deletions(-) create mode 100644 pkg/i2gw/providers/istio/fixtures/input/6-virtualservice-http-rewrite.yaml create mode 100644 pkg/i2gw/providers/istio/fixtures/output/6-virtualservice-http-rewrite.yaml diff --git a/pkg/i2gw/providers/istio/README.md b/pkg/i2gw/providers/istio/README.md index f8036921..780b64e8 100644 --- a/pkg/i2gw/providers/istio/README.md +++ b/pkg/i2gw/providers/istio/README.md @@ -68,6 +68,20 @@ The list of fields showing how istio.VirtualService.Http fields are converted to * headers.request -> requestHeaderModifier gw.HTTPHeaderFilter * headers.response -> responseHeaderModifier gw.HTTPHeaderFilter +##### rewrite HTTPRewrite translation + +In istio, the rewrite logic depends on the match URI parameters: + * for prefix match, istio rewrites matched prefix to the given value. + * for exact match and for regex match, istio rewrites full URI path to the given value. + +Also, in K8S Gateway API only 1 HTTPRouteFilterURLRewrite is allowed per HTTPRouteRule +https://github.com/kubernetes-sigs/gateway-api/blob/0ad0daffe8d47f97a293b2a947bb3b2ee658e967/apis/v1/httproute_types.go#L228 + +To take this all into consideration, translator aggregates prefix matches vs non-prefix matches for the istio virtualservice.HTTPRoute. +And generates max 2 HTTPRoutes (one with prefix matches and ReplacePrefixMatch filter and the other if non-prefix matches and ReplaceFullPath filter). +If any of the match group is empty, the corresponding HTTPRoute won't be generated. +If all URI matches are empty, there would be HTTPRoute with HTTPRouteFilterURLRewrite of ReplaceFullPath type. + #### TLS The list of fields showing how istio.VirtualService.Tls fields are converted to the TLSRoute equivalents diff --git a/pkg/i2gw/providers/istio/converter.go b/pkg/i2gw/providers/istio/converter.go index 259eee76..85e5f892 100644 --- a/pkg/i2gw/providers/istio/converter.go +++ b/pkg/i2gw/providers/istio/converter.go @@ -510,27 +510,6 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH klog.Infof("ignoring field: %v", httpRouteFieldPath.Child("CorsPolicy")) } - if rewrite := httpRoute.GetRewrite(); rewrite != nil { - rewriteFieldPath := httpRouteFieldPath.Child("HTTPRewrite") - - if rewrite.GetAuthority() != "" { - klog.Infof("ignoring field: %v", rewriteFieldPath.Child("Authority")) - } - if rewrite.GetUriRegexRewrite() != nil { - klog.Infof("ignoring field: %v", rewriteFieldPath.Child("UriRegexRewrite")) - } - - gwHTTPRouteFilters = append(gwHTTPRouteFilters, gatewayv1.HTTPRouteFilter{ - Type: gatewayv1.HTTPRouteFilterURLRewrite, - URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ - Path: &gatewayv1.HTTPPathModifier{ - Type: gatewayv1.PrefixMatchHTTPPathModifier, - ReplacePrefixMatch: &httpRoute.Rewrite.Uri, - }, - }, - }) - } - if httpRoute.GetMirror() != nil && len(httpRoute.GetMirrors()) > 0 { errList = append(errList, field.Invalid(httpRouteFieldPath, httpRoute, "HTTP route cannot contain both mirror and mirrors")) continue @@ -606,19 +585,13 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH hostnames = append(hostnames, gatewayv1.Hostname(host)) } - apiVersion, kind := common.HTTPRouteGVK.ToAPIVersionAndKind() - routeName := fmt.Sprintf("%v-idx-%v", virtualService.Name, i) if httpRoute.GetName() != "" { routeName = fmt.Sprintf("%v-%v", virtualService.Name, httpRoute.GetName()) } - resHTTPRoutes = append(resHTTPRoutes, &gatewayv1.HTTPRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: apiVersion, - Kind: kind, - }, - ObjectMeta: metav1.ObjectMeta{ + createHTTPRouteParams := createHTTPRouteParams{ + objectMeta: metav1.ObjectMeta{ Namespace: virtualService.Namespace, Name: routeName, Labels: virtualService.Labels, @@ -626,18 +599,19 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH OwnerReferences: virtualService.OwnerReferences, Finalizers: virtualService.Finalizers, }, - Spec: gatewayv1.HTTPRouteSpec{ - Hostnames: hostnames, - Rules: []gatewayv1.HTTPRouteRule{ - { - Matches: gwHTTPRouteMatches, - Filters: gwHTTPRouteFilters, - BackendRefs: backendRefs, - Timeouts: httpRouteTimeouts, - }, - }, - }, - }) + hostnames: hostnames, + matches: gwHTTPRouteMatches, + filters: gwHTTPRouteFilters, + backendRefs: backendRefs, + timeouts: httpRouteTimeouts, + } + + if httpRoute.GetRewrite() != nil { + resHTTPRoutes = append(resHTTPRoutes, c.createHTTPRoutesWithRewrite(createHTTPRouteParams, httpRoute.GetRewrite(), httpRouteFieldPath.Child("HTTPRewrite"))...) + continue + } + + resHTTPRoutes = append(resHTTPRoutes, c.createHTTPRoute(createHTTPRouteParams)) } if len(errList) > 0 { @@ -647,6 +621,118 @@ func (c *converter) convertVsHTTPRoutes(virtualService metav1.ObjectMeta, istioH return resHTTPRoutes, nil } +type createHTTPRouteParams struct { + objectMeta metav1.ObjectMeta + hostnames []gatewayv1.Hostname + matches []gatewayv1.HTTPRouteMatch + filters []gatewayv1.HTTPRouteFilter + backendRefs []gatewayv1.HTTPBackendRef + timeouts *gatewayv1.HTTPRouteTimeouts +} + +func (c *converter) createHTTPRoute(params createHTTPRouteParams) *gatewayv1.HTTPRoute { + apiVersion, kind := common.HTTPRouteGVK.ToAPIVersionAndKind() + + return &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + ObjectMeta: params.objectMeta, + Spec: gatewayv1.HTTPRouteSpec{ + Hostnames: params.hostnames, + Rules: []gatewayv1.HTTPRouteRule{ + { + Matches: params.matches, + Filters: params.filters, + BackendRefs: params.backendRefs, + Timeouts: params.timeouts, + }, + }, + }, + } +} + +// createHTTPRoutesWithRewrite generates k8sgw.HTTRoutes taking into consideration "rewrite" option in istio.HTTPRewrite +// In istio, the rewrite logic depends on the match URI parameters: +// 1. for prefix match, istio rewrites matched prefix to the given value. +// 2. for exact match and for regex match, istio rewrites full URI path to the given value. +// +// Also, in K8S Gateway API only 1 HTTPRouteFilterURLRewrite is allowed per HTTPRouteRule +// https://github.com/kubernetes-sigs/gateway-api/blob/0ad0daffe8d47f97a293b2a947bb3b2ee658e967/apis/v1/httproute_types.go#L228 +// +// To take this all into consideration, translator aggregates prefix matches vs non-prefix matches +// And generates max 2 HTTPRoutes (one with prefix matches and ReplacePrefixMatch filter and the other if non-prefix matches and ReplaceFullPath filter). +// If any of the match group is empty, the corresponding HTTPRoute won't be generated. +// If all URI matches are empty, there would be HTTPRoute with HTTPRouteFilterURLRewrite of ReplaceFullPath type. +func (c *converter) createHTTPRoutesWithRewrite(params createHTTPRouteParams, rewrite *istiov1beta1.HTTPRewrite, fieldPath *field.Path) []*gatewayv1.HTTPRoute { + if rewrite == nil { + return nil + } + + if rewrite.GetAuthority() != "" { + klog.Infof("ignoring field: %v", fieldPath.Child("Authority")) + } + if rewrite.GetUriRegexRewrite() != nil { + klog.Infof("ignoring field: %v", fieldPath.Child("UriRegexRewrite")) + } + + origFilters := params.filters + + var prefixRouteMatches, nonPrefixRouteMatches []gatewayv1.HTTPRouteMatch + for _, match := range params.matches { + // if it's a non-path match, then prefixMatch rewrite is generated + if match.Path == nil { + prefixRouteMatches = append(prefixRouteMatches, match) + continue + } + + // if type == nil, prefixMatch is the default + if match.Path.Type == nil || *match.Path.Type == gatewayv1.PathMatchPathPrefix { + prefixRouteMatches = append(prefixRouteMatches, match) + } else { + nonPrefixRouteMatches = append(nonPrefixRouteMatches, match) + } + } + + var resHTTPRoutes []*gatewayv1.HTTPRoute + + // these matches contain Exact and Regex matches, istio does FullPath rewrite for both + // if there are no matches at all -- use FullPath rewrite as well + if len(params.matches) == 0 || len(nonPrefixRouteMatches) > 0 { + params.filters = append(origFilters, gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterURLRewrite, + URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.FullPathHTTPPathModifier, + ReplaceFullPath: &rewrite.Uri, + }, + }, + }) + params.matches = nonPrefixRouteMatches + + resHTTPRoutes = append(resHTTPRoutes, c.createHTTPRoute(params)) + } + + if len(prefixRouteMatches) > 0 { + params.filters = append(origFilters, gatewayv1.HTTPRouteFilter{ + Type: gatewayv1.HTTPRouteFilterURLRewrite, + URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ + Path: &gatewayv1.HTTPPathModifier{ + Type: gatewayv1.PrefixMatchHTTPPathModifier, + ReplacePrefixMatch: &rewrite.Uri, + }, + }, + }) + params.matches = prefixRouteMatches + params.objectMeta.Name += "-prefix-match" + + resHTTPRoutes = append(resHTTPRoutes, c.createHTTPRoute(params)) + } + + return resHTTPRoutes +} + func (c *converter) convertVsTLSRoutes(virtualService metav1.ObjectMeta, istioTLSRoutes []*istiov1beta1.TLSRoute, fieldPath *field.Path) []*gatewayv1alpha2.TLSRoute { var resTLSRoutes []*gatewayv1alpha2.TLSRoute diff --git a/pkg/i2gw/providers/istio/converter_test.go b/pkg/i2gw/providers/istio/converter_test.go index 4bcfc03b..4418664b 100644 --- a/pkg/i2gw/providers/istio/converter_test.go +++ b/pkg/i2gw/providers/istio/converter_test.go @@ -859,8 +859,8 @@ func Test_converter_convertVsHTTPRoutes(t *testing.T) { Type: gatewayv1.HTTPRouteFilterURLRewrite, URLRewrite: &gatewayv1.HTTPURLRewriteFilter{ Path: &gatewayv1.HTTPPathModifier{ - Type: gatewayv1.PrefixMatchHTTPPathModifier, - ReplacePrefixMatch: common.PtrTo[string]("redirect-uri"), + Type: gatewayv1.FullPathHTTPPathModifier, + ReplaceFullPath: common.PtrTo[string]("redirect-uri"), }, }, }, diff --git a/pkg/i2gw/providers/istio/fixtures/input/2-virtualservice-http.yaml b/pkg/i2gw/providers/istio/fixtures/input/2-virtualservice-http.yaml index 500b2008..66d862ab 100644 --- a/pkg/i2gw/providers/istio/fixtures/input/2-virtualservice-http.yaml +++ b/pkg/i2gw/providers/istio/fixtures/input/2-virtualservice-http.yaml @@ -54,13 +54,6 @@ spec: port: 8080 # authority & derivePort are ignored authority: newratings.default.svc.cluster.local - rewrite: - uri: "/newcatalog" - # authority & uriRegexRewrite are ignored - authority: test - uriRegexRewrite: - match: pattern - rewrite: new-pattern mirror: host: reviews # interpreted as reviews.test.svc.cluster.local subset: v1 @@ -111,4 +104,4 @@ spec: - destination: host: reviewsB.prod.svc.cluster.local # interpreted as reviews.test.svc.cluster.local percentage: - value: 50 \ No newline at end of file + value: 50 diff --git a/pkg/i2gw/providers/istio/fixtures/input/4-virtualservice-tcp.yaml b/pkg/i2gw/providers/istio/fixtures/input/4-virtualservice-tcp.yaml index 2fbf5c00..d1a37f3e 100644 --- a/pkg/i2gw/providers/istio/fixtures/input/4-virtualservice-tcp.yaml +++ b/pkg/i2gw/providers/istio/fixtures/input/4-virtualservice-tcp.yaml @@ -13,4 +13,4 @@ spec: - destination: host: mongo.backup.svc.cluster.local port: - number: 5555 \ No newline at end of file + number: 5555 diff --git a/pkg/i2gw/providers/istio/fixtures/input/6-virtualservice-http-rewrite.yaml b/pkg/i2gw/providers/istio/fixtures/input/6-virtualservice-http-rewrite.yaml new file mode 100644 index 00000000..cefef8c6 --- /dev/null +++ b/pkg/i2gw/providers/istio/fixtures/input/6-virtualservice-http-rewrite.yaml @@ -0,0 +1,58 @@ +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + namespace: test + name: virtualservice +spec: + gateways: + - "my-gateway" + hosts: + - reviews.prod.svc.cluster.local + http: + - name: "test" + match: + - uri: + prefix: "/wpcatalog" + - uri: + exact: "/consumercatalog" + - uri: + regex: "/catalog[0-9]+" + rewrite: + uri: "/newcatalog" + # authority & uriRegexRewrite are ignored + authority: test + uriRegexRewrite: + match: pattern + rewrite: new-pattern + route: + - destination: + host: reviews.prod.svc.cluster.local + subset: v2 +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + namespace: test + name: no-uri-matches +spec: + gateways: + - "my-gateway" + hosts: + - reviews.test.svc.cluster.local + http: + - name: "route" + match: + - headers: + h1: + exact: v1 + rewrite: + uri: "/newcatalog" + # authority & uriRegexRewrite are ignored + authority: test + uriRegexRewrite: + match: pattern + rewrite: new-pattern + route: + - destination: + host: reviews.test.svc.cluster.local + subset: v2 diff --git a/pkg/i2gw/providers/istio/fixtures/output/2-virtualservice-http.yaml b/pkg/i2gw/providers/istio/fixtures/output/2-virtualservice-http.yaml index 56932dc8..47107410 100644 --- a/pkg/i2gw/providers/istio/fixtures/output/2-virtualservice-http.yaml +++ b/pkg/i2gw/providers/istio/fixtures/output/2-virtualservice-http.yaml @@ -48,11 +48,6 @@ spec: replaceFullPath: /v1/bookRatings statusCode: 302 port: 8080 - - type: URLRewrite - urlRewrite: - path: - replacePrefixMatch: /newcatalog - type: ReplacePrefixMatch - type: RequestMirror requestMirror: backendRef: diff --git a/pkg/i2gw/providers/istio/fixtures/output/3-virtualservice-tls.yaml b/pkg/i2gw/providers/istio/fixtures/output/3-virtualservice-tls.yaml index b242932d..21ed3097 100644 --- a/pkg/i2gw/providers/istio/fixtures/output/3-virtualservice-tls.yaml +++ b/pkg/i2gw/providers/istio/fixtures/output/3-virtualservice-tls.yaml @@ -25,4 +25,3 @@ spec: - name: reviews namespace: test weight: 0 - diff --git a/pkg/i2gw/providers/istio/fixtures/output/6-virtualservice-http-rewrite.yaml b/pkg/i2gw/providers/istio/fixtures/output/6-virtualservice-http-rewrite.yaml new file mode 100644 index 00000000..a17f0552 --- /dev/null +++ b/pkg/i2gw/providers/istio/fixtures/output/6-virtualservice-http-rewrite.yaml @@ -0,0 +1,75 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: virtualservice-test + namespace: test +spec: + hostnames: + - reviews.prod.svc.cluster.local + rules: + - matches: + - path: + type: Exact + value: /consumercatalog + - path: + type: RegularExpression + value: "/catalog[0-9]+" + backendRefs: + - name: reviews + namespace: prod + weight: 0 + filters: + - type: URLRewrite + urlRewrite: + path: + replaceFullPath: /newcatalog + type: ReplaceFullPath +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: virtualservice-test-prefix-match + namespace: test +spec: + hostnames: + - reviews.prod.svc.cluster.local + rules: + - matches: + - path: + type: PathPrefix + value: /wpcatalog + backendRefs: + - name: reviews + namespace: prod + weight: 0 + filters: + - type: URLRewrite + urlRewrite: + path: + replacePrefixMatch: /newcatalog + type: ReplacePrefixMatch +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: no-uri-matches-route-prefix-match + namespace: test +spec: + hostnames: + - reviews.test.svc.cluster.local + rules: + - matches: + - headers: + - type: Exact + name: h1 + value: v1 + backendRefs: + - name: reviews + namespace: test + weight: 0 + filters: + - type: URLRewrite + urlRewrite: + path: + replacePrefixMatch: /newcatalog + type: ReplacePrefixMatch \ No newline at end of file