rds: allow case_insensitive path matching (#3997)
- in xds_client, accept (not NACK) RDS resp with case_insensitive=true
- pass case_insensitive to xds resolver and routing balancer
- Note that after the config selector change, the routing balancer will be removed, and
this will be handled in the resolver config selectordiff --git a/xds/internal/balancer/xdsrouting/matcher_path.go b/xds/internal/balancer/xdsrouting/matcher_path.go
index 9c783ac..f6f4b7d 100644
--- a/xds/internal/balancer/xdsrouting/matcher_path.go
+++ b/xds/internal/balancer/xdsrouting/matcher_path.go
@@ -30,14 +30,26 @@
}
type pathExactMatcher struct {
- fullPath string
+ // fullPath is all upper case if caseInsensitive is true.
+ fullPath string
+ caseInsensitive bool
}
-func newPathExactMatcher(p string) *pathExactMatcher {
- return &pathExactMatcher{fullPath: p}
+func newPathExactMatcher(p string, caseInsensitive bool) *pathExactMatcher {
+ ret := &pathExactMatcher{
+ fullPath: p,
+ caseInsensitive: caseInsensitive,
+ }
+ if caseInsensitive {
+ ret.fullPath = strings.ToUpper(p)
+ }
+ return ret
}
func (pem *pathExactMatcher) match(path string) bool {
+ if pem.caseInsensitive {
+ return pem.fullPath == strings.ToUpper(path)
+ }
return pem.fullPath == path
}
@@ -46,7 +58,7 @@
if !ok {
return false
}
- return pem.fullPath == mm.fullPath
+ return pem.fullPath == mm.fullPath && pem.caseInsensitive == mm.caseInsensitive
}
func (pem *pathExactMatcher) String() string {
@@ -54,14 +66,26 @@
}
type pathPrefixMatcher struct {
- prefix string
+ // prefix is all upper case if caseInsensitive is true.
+ prefix string
+ caseInsensitive bool
}
-func newPathPrefixMatcher(p string) *pathPrefixMatcher {
- return &pathPrefixMatcher{prefix: p}
+func newPathPrefixMatcher(p string, caseInsensitive bool) *pathPrefixMatcher {
+ ret := &pathPrefixMatcher{
+ prefix: p,
+ caseInsensitive: caseInsensitive,
+ }
+ if caseInsensitive {
+ ret.prefix = strings.ToUpper(p)
+ }
+ return ret
}
func (ppm *pathPrefixMatcher) match(path string) bool {
+ if ppm.caseInsensitive {
+ return strings.HasPrefix(strings.ToUpper(path), ppm.prefix)
+ }
return strings.HasPrefix(path, ppm.prefix)
}
@@ -70,7 +94,7 @@
if !ok {
return false
}
- return ppm.prefix == mm.prefix
+ return ppm.prefix == mm.prefix && ppm.caseInsensitive == mm.caseInsensitive
}
func (ppm *pathPrefixMatcher) String() string {
diff --git a/xds/internal/balancer/xdsrouting/matcher_path_test.go b/xds/internal/balancer/xdsrouting/matcher_path_test.go
index ad7db74..b50d5d8 100644
--- a/xds/internal/balancer/xdsrouting/matcher_path_test.go
+++ b/xds/internal/balancer/xdsrouting/matcher_path_test.go
@@ -25,17 +25,21 @@
func TestPathFullMatcherMatch(t *testing.T) {
tests := []struct {
- name string
- fullPath string
- path string
- want bool
+ name string
+ fullPath string
+ caseInsensitive bool
+ path string
+ want bool
}{
{name: "match", fullPath: "/s/m", path: "/s/m", want: true},
+ {name: "case insensitive match", fullPath: "/s/m", caseInsensitive: true, path: "/S/m", want: true},
+ {name: "case insensitive match 2", fullPath: "/s/M", caseInsensitive: true, path: "/S/m", want: true},
{name: "not match", fullPath: "/s/m", path: "/a/b", want: false},
+ {name: "case insensitive not match", fullPath: "/s/m", caseInsensitive: true, path: "/a/b", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- fpm := newPathExactMatcher(tt.fullPath)
+ fpm := newPathExactMatcher(tt.fullPath, tt.caseInsensitive)
if got := fpm.match(tt.path); got != tt.want {
t.Errorf("{%q}.match(%q) = %v, want %v", tt.fullPath, tt.path, got, tt.want)
}
@@ -45,17 +49,21 @@
func TestPathPrefixMatcherMatch(t *testing.T) {
tests := []struct {
- name string
- prefix string
- path string
- want bool
+ name string
+ prefix string
+ caseInsensitive bool
+ path string
+ want bool
}{
{name: "match", prefix: "/s/", path: "/s/m", want: true},
+ {name: "case insensitive match", prefix: "/s/", caseInsensitive: true, path: "/S/m", want: true},
+ {name: "case insensitive match 2", prefix: "/S/", caseInsensitive: true, path: "/s/m", want: true},
{name: "not match", prefix: "/s/", path: "/a/b", want: false},
+ {name: "case insensitive not match", prefix: "/s/", caseInsensitive: true, path: "/a/b", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- fpm := newPathPrefixMatcher(tt.prefix)
+ fpm := newPathPrefixMatcher(tt.prefix, tt.caseInsensitive)
if got := fpm.match(tt.path); got != tt.want {
t.Errorf("{%q}.match(%q) = %v, want %v", tt.prefix, tt.path, got, tt.want)
}
diff --git a/xds/internal/balancer/xdsrouting/matcher_test.go b/xds/internal/balancer/xdsrouting/matcher_test.go
index e7d76e2..74c570a 100644
--- a/xds/internal/balancer/xdsrouting/matcher_test.go
+++ b/xds/internal/balancer/xdsrouting/matcher_test.go
@@ -38,7 +38,17 @@
}{
{
name: "both match",
- pm: newPathExactMatcher("/a/b"),
+ pm: newPathExactMatcher("/a/b", false),
+ hm: newHeaderExactMatcher("th", "tv"),
+ info: balancer.PickInfo{
+ FullMethodName: "/a/b",
+ Ctx: metadata.NewOutgoingContext(context.Background(), metadata.Pairs("th", "tv")),
+ },
+ want: true,
+ },
+ {
+ name: "both match with path case insensitive",
+ pm: newPathExactMatcher("/A/B", true),
hm: newHeaderExactMatcher("th", "tv"),
info: balancer.PickInfo{
FullMethodName: "/a/b",
@@ -48,7 +58,7 @@
},
{
name: "only one match",
- pm: newPathExactMatcher("/a/b"),
+ pm: newPathExactMatcher("/a/b", false),
hm: newHeaderExactMatcher("th", "tv"),
info: balancer.PickInfo{
FullMethodName: "/z/y",
@@ -58,7 +68,7 @@
},
{
name: "both not match",
- pm: newPathExactMatcher("/z/y"),
+ pm: newPathExactMatcher("/z/y", false),
hm: newHeaderExactMatcher("th", "abc"),
info: balancer.PickInfo{
FullMethodName: "/a/b",
@@ -68,7 +78,7 @@
},
{
name: "fake header",
- pm: newPathPrefixMatcher("/"),
+ pm: newPathPrefixMatcher("/", false),
hm: newHeaderExactMatcher("content-type", "fake"),
info: balancer.PickInfo{
FullMethodName: "/a/b",
@@ -80,7 +90,7 @@
},
{
name: "binary header",
- pm: newPathPrefixMatcher("/"),
+ pm: newPathPrefixMatcher("/", false),
hm: newHeaderPresentMatcher("t-bin", true),
info: balancer.PickInfo{
FullMethodName: "/a/b",
@@ -146,8 +156,8 @@
}{
{
name: "equal",
- pm: newPathExactMatcher("/a/b"),
- mm: newCompositeMatcher(newPathExactMatcher("/a/b"), nil, nil),
+ pm: newPathExactMatcher("/a/b", false),
+ mm: newCompositeMatcher(newPathExactMatcher("/a/b", false), nil, nil),
want: true,
},
{
@@ -158,9 +168,9 @@
},
{
name: "not equal",
- pm: newPathExactMatcher("/a/b"),
+ pm: newPathExactMatcher("/a/b", false),
fm: newFractionMatcher(123),
- mm: newCompositeMatcher(newPathExactMatcher("/a/b"), nil, nil),
+ mm: newCompositeMatcher(newPathExactMatcher("/a/b", false), nil, nil),
want: false,
},
}
diff --git a/xds/internal/balancer/xdsrouting/routing.go b/xds/internal/balancer/xdsrouting/routing.go
index ad5373a..fd92fd9 100644
--- a/xds/internal/balancer/xdsrouting/routing.go
+++ b/xds/internal/balancer/xdsrouting/routing.go
@@ -136,9 +136,9 @@
}
pathMatcher = newPathRegexMatcher(re)
case r.path != "":
- pathMatcher = newPathExactMatcher(r.path)
+ pathMatcher = newPathExactMatcher(r.path, r.caseInsensitive)
default:
- pathMatcher = newPathPrefixMatcher(r.prefix)
+ pathMatcher = newPathPrefixMatcher(r.prefix, r.caseInsensitive)
}
var headerMatchers []headerMatcherInterface
diff --git a/xds/internal/balancer/xdsrouting/routing_config.go b/xds/internal/balancer/xdsrouting/routing_config.go
index 78716a0..526fe27 100644
--- a/xds/internal/balancer/xdsrouting/routing_config.go
+++ b/xds/internal/balancer/xdsrouting/routing_config.go
@@ -52,6 +52,9 @@
// Path, Prefix and Regex can have at most one set. This is guaranteed by
// config parsing.
path, prefix, regex string
+ // Indicates if prefix/path matching should be case insensitive. The default
+ // is false (case sensitive).
+ caseInsensitive bool
headers []headerMatcher
fraction *uint32
@@ -74,6 +77,7 @@
type routeJSON struct {
// Path, Prefix and Regex can have at most one non-nil.
Path, Prefix, Regex *string
+ CaseInsensitive bool
// Zero or more header matchers.
Headers []*xdsclient.HeaderMatcher
MatchFraction *wrapperspb.UInt32Value
@@ -99,6 +103,7 @@
case r.Regex != nil:
tempR.regex = *r.Regex
}
+ tempR.caseInsensitive = r.CaseInsensitive
for _, h := range r.Headers {
var tempHeader headerMatcher
switch {
diff --git a/xds/internal/balancer/xdsrouting/routing_picker_test.go b/xds/internal/balancer/xdsrouting/routing_picker_test.go
index 83e494a..fd03c3b 100644
--- a/xds/internal/balancer/xdsrouting/routing_picker_test.go
+++ b/xds/internal/balancer/xdsrouting/routing_picker_test.go
@@ -52,7 +52,7 @@
{
name: "one route no match",
routes: []route{
- {m: newCompositeMatcher(newPathPrefixMatcher("/a/"), nil, nil), action: "action-0"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/a/", false), nil, nil), action: "action-0"},
},
pickers: map[string]*subBalancerState{
"action-0": {state: balancer.State{
@@ -66,7 +66,7 @@
{
name: "one route one match",
routes: []route{
- {m: newCompositeMatcher(newPathPrefixMatcher("/a/"), nil, nil), action: "action-0"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/a/", false), nil, nil), action: "action-0"},
},
pickers: map[string]*subBalancerState{
"action-0": {state: balancer.State{
@@ -80,8 +80,8 @@
{
name: "two routes first match",
routes: []route{
- {m: newCompositeMatcher(newPathPrefixMatcher("/a/"), nil, nil), action: "action-0"},
- {m: newCompositeMatcher(newPathPrefixMatcher("/z/"), nil, nil), action: "action-1"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/a/", false), nil, nil), action: "action-0"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/z/", false), nil, nil), action: "action-1"},
},
pickers: map[string]*subBalancerState{
"action-0": {state: balancer.State{
@@ -99,8 +99,8 @@
{
name: "two routes second match",
routes: []route{
- {m: newCompositeMatcher(newPathPrefixMatcher("/a/"), nil, nil), action: "action-0"},
- {m: newCompositeMatcher(newPathPrefixMatcher("/z/"), nil, nil), action: "action-1"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/a/", false), nil, nil), action: "action-0"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/z/", false), nil, nil), action: "action-1"},
},
pickers: map[string]*subBalancerState{
"action-0": {state: balancer.State{
@@ -118,8 +118,8 @@
{
name: "two routes both match former more specific",
routes: []route{
- {m: newCompositeMatcher(newPathExactMatcher("/a/b"), nil, nil), action: "action-0"},
- {m: newCompositeMatcher(newPathPrefixMatcher("/a/"), nil, nil), action: "action-1"},
+ {m: newCompositeMatcher(newPathExactMatcher("/a/b", false), nil, nil), action: "action-0"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/a/", false), nil, nil), action: "action-1"},
},
pickers: map[string]*subBalancerState{
"action-0": {state: balancer.State{
@@ -138,8 +138,8 @@
{
name: "tow routes both match latter more specific",
routes: []route{
- {m: newCompositeMatcher(newPathPrefixMatcher("/a/"), nil, nil), action: "action-0"},
- {m: newCompositeMatcher(newPathExactMatcher("/a/b"), nil, nil), action: "action-1"},
+ {m: newCompositeMatcher(newPathPrefixMatcher("/a/", false), nil, nil), action: "action-0"},
+ {m: newCompositeMatcher(newPathExactMatcher("/a/b", false), nil, nil), action: "action-1"},
},
pickers: map[string]*subBalancerState{
"action-0": {state: balancer.State{
diff --git a/xds/internal/client/client.go b/xds/internal/client/client.go
index c2bbeaf..583f09b 100644
--- a/xds/internal/client/client.go
+++ b/xds/internal/client/client.go
@@ -167,9 +167,12 @@
// indication of the action to take upon match.
type Route struct {
Path, Prefix, Regex *string
- Headers []*HeaderMatcher
- Fraction *uint32
- Action map[string]uint32 // action is weighted clusters.
+ // Indicates if prefix/path matching should be case insensitive. The default
+ // is false (case sensitive).
+ CaseInsensitive bool
+ Headers []*HeaderMatcher
+ Fraction *uint32
+ Action map[string]uint32 // action is weighted clusters.
}
// HeaderMatcher represents header matchers.
diff --git a/xds/internal/client/client_rds_test.go b/xds/internal/client/client_rds_test.go
index ee09211..7b39158 100644
--- a/xds/internal/client/client_rds_test.go
+++ b/xds/internal/client/client_rds_test.go
@@ -280,7 +280,14 @@
Route: &v3routepb.RouteAction{
ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName},
}}}}}}},
- wantError: true,
+ wantUpdate: RouteConfigUpdate{
+ VirtualHosts: []*VirtualHost{
+ {
+ Domains: []string{ldsTarget},
+ Routes: []*Route{{Prefix: newStringP("/"), CaseInsensitive: true, Action: map[string]uint32{clusterName: 1}}},
+ },
+ },
+ },
},
{
name: "good-route-config-with-empty-string-route",
@@ -434,7 +441,7 @@
t.Run(test.name, func(t *testing.T) {
gotUpdate, gotError := generateRDSUpdateFromRouteConfiguration(test.rc, nil)
if (gotError != nil) != test.wantError || !cmp.Equal(gotUpdate, test.wantUpdate, cmpopts.EquateEmpty()) {
- t.Errorf("generateRDSUpdateFromRouteConfiguration(%+v, %v) = %v, want %v", test.rc, ldsTarget, gotUpdate, test.wantUpdate)
+ t.Errorf("generateRDSUpdateFromRouteConfiguration(%+v, %v) returned unexpected, diff (-want +got):\\n%s", test.rc, ldsTarget, cmp.Diff(test.wantUpdate, gotUpdate, cmpopts.EquateEmpty()))
}
})
}
@@ -654,8 +661,22 @@
PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/"},
CaseSensitive: &wrapperspb.BoolValue{Value: false},
},
+ Action: &v3routepb.Route_Route{
+ Route: &v3routepb.RouteAction{
+ ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{
+ WeightedClusters: &v3routepb.WeightedCluster{
+ Clusters: []*v3routepb.WeightedCluster_ClusterWeight{
+ {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 60}},
+ {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 40}},
+ },
+ TotalWeight: &wrapperspb.UInt32Value{Value: 100},
+ }}}},
}},
- wantErr: true,
+ wantRoutes: []*Route{{
+ Prefix: newStringP("/"),
+ CaseInsensitive: true,
+ Action: map[string]uint32{"A": 40, "B": 60},
+ }},
},
{
name: "good",
diff --git a/xds/internal/client/client_xds.go b/xds/internal/client/client_xds.go
index b8598c0..af25ce2 100644
--- a/xds/internal/client/client_xds.go
+++ b/xds/internal/client/client_xds.go
@@ -171,10 +171,6 @@
continue
}
- if caseSensitive := match.GetCaseSensitive(); caseSensitive != nil && !caseSensitive.Value {
- return nil, fmt.Errorf("route %+v has case-sensitive false", r)
- }
-
pathSp := match.GetPathSpecifier()
if pathSp == nil {
return nil, fmt.Errorf("route %+v doesn't have a path specifier", r)
@@ -193,6 +189,10 @@
continue
}
+ if caseSensitive := match.GetCaseSensitive(); caseSensitive != nil {
+ route.CaseInsensitive = !caseSensitive.Value
+ }
+
for _, h := range match.GetHeaders() {
var header HeaderMatcher
switch ht := h.GetHeaderMatchSpecifier().(type) {
diff --git a/xds/internal/resolver/serviceconfig.go b/xds/internal/resolver/serviceconfig.go
index 965817c..f585e43 100644
--- a/xds/internal/resolver/serviceconfig.go
+++ b/xds/internal/resolver/serviceconfig.go
@@ -56,12 +56,13 @@
}
type route struct {
- Path *string `json:"path,omitempty"`
- Prefix *string `json:"prefix,omitempty"`
- Regex *string `json:"regex,omitempty"`
- Headers []*xdsclient.HeaderMatcher `json:"headers,omitempty"`
- Fraction *wrapperspb.UInt32Value `json:"matchFraction,omitempty"`
- Action string `json:"action"`
+ Path *string `json:"path,omitempty"`
+ Prefix *string `json:"prefix,omitempty"`
+ Regex *string `json:"regex,omitempty"`
+ CaseInsensitive bool `json:"caseInsensitive"`
+ Headers []*xdsclient.HeaderMatcher `json:"headers,omitempty"`
+ Fraction *wrapperspb.UInt32Value `json:"matchFraction,omitempty"`
+ Action string `json:"action"`
}
type xdsActionConfig struct {
@@ -80,10 +81,11 @@
var rts []*route
for _, rt := range routes {
t := &route{
- Path: rt.Path,
- Prefix: rt.Prefix,
- Regex: rt.Regex,
- Headers: rt.Headers,
+ Path: rt.Path,
+ Prefix: rt.Prefix,
+ Regex: rt.Regex,
+ Headers: rt.Headers,
+ CaseInsensitive: rt.CaseInsensitive,
}
if f := rt.Fraction; f != nil {
diff --git a/xds/internal/resolver/serviceconfig_test.go b/xds/internal/resolver/serviceconfig_test.go
index 5969bd8..cce30c1 100644
--- a/xds/internal/resolver/serviceconfig_test.go
+++ b/xds/internal/resolver/serviceconfig_test.go
@@ -144,6 +144,7 @@
},
{
"prefix":"/service_2/method_1",
+ "caseInsensitive":true,
"action":"cluster_1_0"
},
{
@@ -214,8 +215,9 @@
Action: map[string]uint32{"cluster_1": 1},
},
{
- Prefix: newStringP("/service_2/method_1"),
- Action: map[string]uint32{"cluster_1": 1},
+ Prefix: newStringP("/service_2/method_1"),
+ CaseInsensitive: true,
+ Action: map[string]uint32{"cluster_1": 1},
},
{
Regex: newStringP("^/service_2/method_3$"),
