diff --git a/balancer/rls/internal/keys/builder.go b/balancer/rls/internal/keys/builder.go index a606155f..abc076ab 100644 --- a/balancer/rls/internal/keys/builder.go +++ b/balancer/rls/internal/keys/builder.go @@ -27,7 +27,6 @@ import ( "sort" "strings" - "github.com/google/go-cmp/cmp" rlspb "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1" "google.golang.org/grpc/metadata" ) @@ -123,9 +122,25 @@ func (bm BuilderMap) RLSKey(md metadata.MD, path string) KeyMap { return b.keys(md) } -// BuilderMapEqual returns true if the provided BuilderMap objects are equal. -func BuilderMapEqual(a, b BuilderMap) bool { - return cmp.Equal(a, b, cmp.AllowUnexported(builder{}, matcher{})) +// Equal reports whether bm and am represent equivalent BuilderMaps. +func (bm BuilderMap) Equal(am BuilderMap) bool { + if (bm == nil) != (am == nil) { + return false + } + if len(bm) != len(am) { + return false + } + + for key, bBuilder := range bm { + aBuilder, ok := am[key] + if !ok { + return false + } + if !bBuilder.Equal(aBuilder) { + return false + } + } + return true } // builder provides the actual functionality of building RLS keys. These are @@ -142,6 +157,28 @@ type builder struct { matchers []matcher } +// Equal reports whether b and a represent equivalent key builders. +func (b builder) Equal(a builder) bool { + if (b.matchers == nil) != (a.matchers == nil) { + return false + } + if len(b.matchers) != len(a.matchers) { + return false + } + // Protobuf serialization maintains the order of repeated fields. Matchers + // are specified as a repeated field inside the KeyBuilder proto. If the + // order changes, it means that the order in the protobuf changed. We report + // this case as not being equal even though the builders could possible be + // functionally equal. + for i, bMatcher := range b.matchers { + aMatcher := a.matchers[i] + if !bMatcher.Equal(aMatcher) { + return false + } + } + return true +} + // matcher helps extract a key from request headers based on a given name. type matcher struct { // The key used in the keyMap sent as part of the RLS request. @@ -150,6 +187,25 @@ type matcher struct { names []string } +// Equal reports if m and are are equivalent matchers. +func (m matcher) Equal(a matcher) bool { + if m.key != a.key { + return false + } + if (m.names == nil) != (a.names == nil) { + return false + } + if len(m.names) != len(a.names) { + return false + } + for i := 0; i < len(m.names); i++ { + if m.names[i] != a.names[i] { + return false + } + } + return true +} + func (b builder) keys(md metadata.MD) KeyMap { kvMap := make(map[string]string) for _, m := range b.matchers { diff --git a/balancer/rls/internal/keys/builder_test.go b/balancer/rls/internal/keys/builder_test.go index 09a392ea..f5e03ac5 100644 --- a/balancer/rls/internal/keys/builder_test.go +++ b/balancer/rls/internal/keys/builder_test.go @@ -85,7 +85,7 @@ func TestMakeBuilderMap(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { builderMap, err := MakeBuilderMap(test.cfg) - if err != nil || !BuilderMapEqual(builderMap, test.wantBuilderMap) { + if err != nil || !builderMap.Equal(test.wantBuilderMap) { t.Errorf("MakeBuilderMap(%+v) returned {%v, %v}, want: {%v, nil}", test.cfg, builderMap, err, test.wantBuilderMap) } }) @@ -330,3 +330,201 @@ func TestMapToString(t *testing.T) { }) } } + +func TestBuilderMapEqual(t *testing.T) { + tests := []struct { + desc string + a BuilderMap + b BuilderMap + wantEqual bool + }{ + { + desc: "nil builder maps", + a: nil, + b: nil, + wantEqual: true, + }, + { + desc: "empty builder maps", + a: make(map[string]builder), + b: make(map[string]builder), + wantEqual: true, + }, + { + desc: "nil and non-nil builder maps", + a: nil, + b: map[string]builder{"/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}}, + wantEqual: false, + }, + { + desc: "empty and non-empty builder maps", + a: make(map[string]builder), + b: map[string]builder{"/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}}, + wantEqual: false, + }, + { + desc: "different number of map keys", + a: map[string]builder{ + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + "/gBar/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + b: map[string]builder{ + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + wantEqual: false, + }, + { + desc: "different map keys", + a: map[string]builder{ + "/gBar/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + b: map[string]builder{ + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + wantEqual: false, + }, + { + desc: "equal keys different values", + a: map[string]builder{ + "/gBar/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1", "n2"}}}}, + }, + b: map[string]builder{ + "/gBar/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + wantEqual: false, + }, + { + desc: "good match", + a: map[string]builder{ + "/gBar/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + b: map[string]builder{ + "/gBar/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + "/gFoo/": {matchers: []matcher{{key: "k1", names: []string{"n1"}}}}, + }, + wantEqual: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + if gotEqual := test.a.Equal(test.b); gotEqual != test.wantEqual { + t.Errorf("BuilderMap.Equal(%v, %v) = %v, want %v", test.a, test.b, gotEqual, test.wantEqual) + } + }) + } +} + +func TestBuilderEqual(t *testing.T) { + tests := []struct { + desc string + a builder + b builder + wantEqual bool + }{ + { + desc: "nil builders", + a: builder{matchers: nil}, + b: builder{matchers: nil}, + wantEqual: true, + }, + { + desc: "empty builders", + a: builder{matchers: []matcher{}}, + b: builder{matchers: []matcher{}}, + wantEqual: true, + }, + { + desc: "nil and non-nil builders", + a: builder{matchers: nil}, + b: builder{matchers: []matcher{}}, + wantEqual: false, + }, + { + desc: "empty and non-empty builders", + a: builder{matchers: []matcher{}}, + b: builder{matchers: []matcher{{key: "foo"}}}, + wantEqual: false, + }, + { + desc: "different number of matchers", + a: builder{matchers: []matcher{{key: "foo"}, {key: "bar"}}}, + b: builder{matchers: []matcher{{key: "foo"}}}, + wantEqual: false, + }, + { + desc: "equal number but differing matchers", + a: builder{matchers: []matcher{{key: "bar"}}}, + b: builder{matchers: []matcher{{key: "foo"}}}, + wantEqual: false, + }, + { + desc: "good match", + a: builder{matchers: []matcher{{key: "foo"}}}, + b: builder{matchers: []matcher{{key: "foo"}}}, + wantEqual: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + t.Run(test.desc, func(t *testing.T) { + if gotEqual := test.a.Equal(test.b); gotEqual != test.wantEqual { + t.Errorf("builder.Equal(%v, %v) = %v, want %v", test.a, test.b, gotEqual, test.wantEqual) + } + }) + }) + } +} + +// matcher helps extract a key from request headers based on a given name. +func TestMatcherEqual(t *testing.T) { + tests := []struct { + desc string + a matcher + b matcher + wantEqual bool + }{ + { + desc: "different keys", + a: matcher{key: "foo"}, + b: matcher{key: "bar"}, + wantEqual: false, + }, + { + desc: "different number of names", + a: matcher{key: "foo", names: []string{"v1", "v2"}}, + b: matcher{key: "foo", names: []string{"v1"}}, + wantEqual: false, + }, + { + desc: "equal number but differing names", + a: matcher{key: "foo", names: []string{"v1", "v2"}}, + b: matcher{key: "foo", names: []string{"v1", "v22"}}, + wantEqual: false, + }, + { + desc: "same names in different order", + a: matcher{key: "foo", names: []string{"v2", "v1"}}, + b: matcher{key: "foo", names: []string{"v1", "v3"}}, + wantEqual: false, + }, + { + desc: "good match", + a: matcher{key: "foo", names: []string{"v1", "v2"}}, + b: matcher{key: "foo", names: []string{"v1", "v2"}}, + wantEqual: true, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + if gotEqual := test.a.Equal(test.b); gotEqual != test.wantEqual { + t.Errorf("matcher.Equal(%v, %v) = %v, want %v", test.a, test.b, gotEqual, test.wantEqual) + } + }) + } +}