diff --git a/xds/internal/balancer/xdsrouting/matcher_path.go b/xds/internal/balancer/xdsrouting/matcher_path.go index 9c783acbb577..f6f4b7ddd9e1 100644 --- a/xds/internal/balancer/xdsrouting/matcher_path.go +++ b/xds/internal/balancer/xdsrouting/matcher_path.go @@ -30,14 +30,26 @@ type pathMatcherInterface interface { } 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 @@ func (pem *pathExactMatcher) equal(m pathMatcherInterface) bool { 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 @@ func (pem *pathExactMatcher) String() string { } 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 @@ func (ppm *pathPrefixMatcher) equal(m pathMatcherInterface) bool { 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 ad7db7481060..b50d5d84d8bc 100644 --- a/xds/internal/balancer/xdsrouting/matcher_path_test.go +++ b/xds/internal/balancer/xdsrouting/matcher_path_test.go @@ -25,17 +25,21 @@ import ( 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 TestPathFullMatcherMatch(t *testing.T) { 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 e7d76e27469f..74c570a1a5b7 100644 --- a/xds/internal/balancer/xdsrouting/matcher_test.go +++ b/xds/internal/balancer/xdsrouting/matcher_test.go @@ -38,7 +38,17 @@ func TestAndMatcherMatch(t *testing.T) { }{ { 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 @@ func TestAndMatcherMatch(t *testing.T) { }, { 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 @@ func TestAndMatcherMatch(t *testing.T) { }, { 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 @@ func TestAndMatcherMatch(t *testing.T) { }, { name: "fake header", - pm: newPathPrefixMatcher("/"), + pm: newPathPrefixMatcher("/", false), hm: newHeaderExactMatcher("content-type", "fake"), info: balancer.PickInfo{ FullMethodName: "/a/b", @@ -80,7 +90,7 @@ func TestAndMatcherMatch(t *testing.T) { }, { name: "binary header", - pm: newPathPrefixMatcher("/"), + pm: newPathPrefixMatcher("/", false), hm: newHeaderPresentMatcher("t-bin", true), info: balancer.PickInfo{ FullMethodName: "/a/b", @@ -146,8 +156,8 @@ func TestCompositeMatcherEqual(t *testing.T) { }{ { 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 @@ func TestCompositeMatcherEqual(t *testing.T) { }, { 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 ad5373a2f773..fd92fd91b8a8 100644 --- a/xds/internal/balancer/xdsrouting/routing.go +++ b/xds/internal/balancer/xdsrouting/routing.go @@ -136,9 +136,9 @@ func routeToMatcher(r routeConfig) (*compositeMatcher, error) { } 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 78716a098136..526fe27692f0 100644 --- a/xds/internal/balancer/xdsrouting/routing_config.go +++ b/xds/internal/balancer/xdsrouting/routing_config.go @@ -52,6 +52,9 @@ type routeConfig struct { // 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 lbConfig struct { 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 @@ func (jc lbConfigJSON) toLBConfig() *lbConfig { 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 83e494a4bf8d..fd03c3b975cb 100644 --- a/xds/internal/balancer/xdsrouting/routing_picker_test.go +++ b/xds/internal/balancer/xdsrouting/routing_picker_test.go @@ -52,7 +52,7 @@ func (s) TestRoutingPickerGroupPick(t *testing.T) { { 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 @@ func (s) TestRoutingPickerGroupPick(t *testing.T) { { 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 @@ func (s) TestRoutingPickerGroupPick(t *testing.T) { { 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 @@ func (s) TestRoutingPickerGroupPick(t *testing.T) { { 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 @@ func (s) TestRoutingPickerGroupPick(t *testing.T) { { 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 @@ func (s) TestRoutingPickerGroupPick(t *testing.T) { { 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 c2bbeafbdd8d..583f09baf1fd 100644 --- a/xds/internal/client/client.go +++ b/xds/internal/client/client.go @@ -167,9 +167,12 @@ type VirtualHost struct { // 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 ee092117cac5..7b39158b8e36 100644 --- a/xds/internal/client/client_rds_test.go +++ b/xds/internal/client/client_rds_test.go @@ -280,7 +280,14 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) { 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 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) { 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 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { 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}, + }}}}, + }}, + wantRoutes: []*Route{{ + Prefix: newStringP("/"), + CaseInsensitive: true, + Action: map[string]uint32{"A": 40, "B": 60}, }}, - wantErr: true, }, { name: "good", diff --git a/xds/internal/client/client_xds.go b/xds/internal/client/client_xds.go index b8598c0247d9..af25ce226bd4 100644 --- a/xds/internal/client/client_xds.go +++ b/xds/internal/client/client_xds.go @@ -171,10 +171,6 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger) 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 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger) 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 965817c6c8ba..f585e4379678 100644 --- a/xds/internal/resolver/serviceconfig.go +++ b/xds/internal/resolver/serviceconfig.go @@ -56,12 +56,13 @@ type cdsBalancerConfig struct { } 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 @@ func (r *xdsResolver) routesToJSON(routes []*xdsclient.Route) (string, error) { 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 5969bd83caed..cce30c183910 100644 --- a/xds/internal/resolver/serviceconfig_test.go +++ b/xds/internal/resolver/serviceconfig_test.go @@ -144,6 +144,7 @@ const ( }, { "prefix":"/service_2/method_1", + "caseInsensitive":true, "action":"cluster_1_0" }, { @@ -214,8 +215,9 @@ func TestRoutesToJSON(t *testing.T) { 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$"),