From c2ddddf486a395595c98491f41176f5e7c18686d Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 26 Nov 2019 04:27:10 +0900 Subject: [PATCH] Use prefix instead of regex for authority match in virtualservice (#6088) * Use prefix instead of regex for authority match in virtualservice This patch changes to use prefix instead of regex for authority match in virtualservice. As described in https://github.com/knative/serving/issues/6058, Istio 1.4 introduced 100 bytes limitation for the regex. So, Knative service which has long service name or domain name, it hits the limit easily. To fix it, this patch uses `prefix` and stop using `regex`. Current regex in VirtualService should be able to replaced with Prefix. CURRENT: ``` regex: ^hello-example\.default\.example\.com(?::\d{1,5})?$ ``` AFTER: ``` prefix: hello-example.default.example.com ``` * Trim cluster local domain to match local --- .../ingress/resources/virtual_service.go | 27 +++++-------------- .../ingress/resources/virtual_service_test.go | 14 +++++----- 2 files changed, 14 insertions(+), 27 deletions(-) diff --git a/pkg/reconciler/ingress/resources/virtual_service.go b/pkg/reconciler/ingress/resources/virtual_service.go index 19cd89e658..6f6c9d1829 100644 --- a/pkg/reconciler/ingress/resources/virtual_service.go +++ b/pkg/reconciler/ingress/resources/virtual_service.go @@ -20,7 +20,6 @@ import ( "crypto/md5" "encoding/json" "fmt" - "regexp" "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -282,7 +281,8 @@ func makeMatch(host string, pathRegExp string, gateways sets.String) v1alpha3.HT match := v1alpha3.HTTPMatchRequest{ Gateways: gateways.List(), Authority: &istiov1alpha1.StringMatch{ - Regex: hostRegExp(host), + // Do not use Regex as Istio 1.4 or later has 100 bytes limitation. + Prefix: hostPrefix(host), }, } // Empty pathRegExp is considered match all path. We only need to @@ -295,29 +295,16 @@ func makeMatch(host string, pathRegExp string, gateways sets.String) v1alpha3.HT return match } -// Should only match 1..65535, but for simplicity it matches 0-99999. -const portMatch = `(?::\d{1,5})?` - -// hostRegExp returns an ECMAScript regular expression to match either host or host: -// for clusterLocalHost, we will also match the prefixes. -func hostRegExp(host string) string { +// hostPrefix returns an host to match either host or host:. +// For clusterLocalHost, it trims .svc. from the host to match short host. +func hostPrefix(host string) string { localDomainSuffix := ".svc." + network.GetClusterDomainName() if !strings.HasSuffix(host, localDomainSuffix) { - return exact(regexp.QuoteMeta(host) + portMatch) + return host } - prefix := regexp.QuoteMeta(strings.TrimSuffix(host, localDomainSuffix)) - clusterSuffix := regexp.QuoteMeta("." + network.GetClusterDomainName()) - svcSuffix := regexp.QuoteMeta(".svc") - return exact(prefix + optional(svcSuffix+optional(clusterSuffix)) + portMatch) -} - -func exact(regexp string) string { - return "^" + regexp + "$" + return strings.TrimSuffix(host, localDomainSuffix) } -func optional(regexp string) string { - return "(" + regexp + ")?" -} func getHosts(ia *v1alpha1.Ingress) sets.String { hosts := sets.NewString() for _, rule := range ia.Spec.Rules { diff --git a/pkg/reconciler/ingress/resources/virtual_service_test.go b/pkg/reconciler/ingress/resources/virtual_service_test.go index 42ae5a981d..77a5eb1e4a 100644 --- a/pkg/reconciler/ingress/resources/virtual_service_test.go +++ b/pkg/reconciler/ingress/resources/virtual_service_test.go @@ -225,7 +225,7 @@ func TestMakeMeshVirtualServiceSpec_CorrectRoutes(t *testing.T) { expected := []v1alpha3.HTTPRoute{{ Match: []v1alpha3.HTTPMatchRequest{{ URI: &istiov1alpha1.StringMatch{Regex: "^/pets/(.*?)?"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^test-route\.test-ns(\.svc(\.cluster\.local)?)?(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `test-route.test-ns`}, Gateways: []string{"mesh"}, }}, Route: []v1alpha3.HTTPRouteDestination{{ @@ -351,11 +351,11 @@ func TestMakeIngressVirtualServiceSpec_CorrectRoutes(t *testing.T) { expected := []v1alpha3.HTTPRoute{{ Match: []v1alpha3.HTTPMatchRequest{{ URI: &istiov1alpha1.StringMatch{Regex: "^/pets/(.*?)?"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^domain\.com(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `domain.com`}, Gateways: []string{"gateway.public"}, }, { URI: &istiov1alpha1.StringMatch{Regex: "^/pets/(.*?)?"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^test-route\.test-ns(\.svc(\.cluster\.local)?)?(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `test-route.test-ns`}, Gateways: []string{"gateway.private"}, }}, Route: []v1alpha3.HTTPRouteDestination{{ @@ -388,7 +388,7 @@ func TestMakeIngressVirtualServiceSpec_CorrectRoutes(t *testing.T) { }, { Match: []v1alpha3.HTTPMatchRequest{{ URI: &istiov1alpha1.StringMatch{Regex: "^/pets/(.*?)?"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^v1\.domain\.com(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `v1.domain.com`}, Gateways: []string{}, }}, Route: []v1alpha3.HTTPRouteDestination{{ @@ -441,10 +441,10 @@ func TestMakeVirtualServiceRoute_Vanilla(t *testing.T) { expected := v1alpha3.HTTPRoute{ Match: []v1alpha3.HTTPMatchRequest{{ Gateways: []string{"gateway-1"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^a\.com(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `a.com`}, }, { Gateways: []string{"gateway-1"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^b\.org(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `b.org`}, }}, Route: []v1alpha3.HTTPRouteDestination{{ Destination: v1alpha3.Destination{ @@ -493,7 +493,7 @@ func TestMakeVirtualServiceRoute_TwoTargets(t *testing.T) { expected := v1alpha3.HTTPRoute{ Match: []v1alpha3.HTTPMatchRequest{{ Gateways: []string{"knative-testing/gateway-1"}, - Authority: &istiov1alpha1.StringMatch{Regex: `^test\.org(?::\d{1,5})?$`}, + Authority: &istiov1alpha1.StringMatch{Prefix: `test.org`}, }}, Route: []v1alpha3.HTTPRouteDestination{{ Destination: v1alpha3.Destination{