Skip to content

Commit

Permalink
Use prefix instead of regex for authority match in virtualservice (#6…
Browse files Browse the repository at this point in the history
…088)

* 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 knative/serving#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
  • Loading branch information
nak3 authored and mattmoor committed Mar 5, 2020
1 parent 295c1f7 commit c2ddddf
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 27 deletions.
27 changes: 7 additions & 20 deletions pkg/reconciler/ingress/resources/virtual_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"crypto/md5"
"encoding/json"
"fmt"
"regexp"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -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
Expand All @@ -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:<any port>
// for clusterLocalHost, we will also match the prefixes.
func hostRegExp(host string) string {
// hostPrefix returns an host to match either host or host:<any port>.
// For clusterLocalHost, it trims .svc.<local domain> 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 {
Expand Down
14 changes: 7 additions & 7 deletions pkg/reconciler/ingress/resources/virtual_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{{
Expand Down Expand Up @@ -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{{
Expand Down Expand Up @@ -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{{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down

0 comments on commit c2ddddf

Please sign in to comment.