From e6f1cc90e2f75058390fa928b07df07e37b62a30 Mon Sep 17 00:00:00 2001 From: erni27 Date: Sat, 8 Oct 2022 00:43:01 +0200 Subject: [PATCH 1/2] Add sum of weights check --- .../xdsclient/xdsresource/unmarshal_eds.go | 16 ++++++++---- .../xdsresource/unmarshal_eds_test.go | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_eds.go b/xds/internal/xdsclient/xdsresource/unmarshal_eds.go index 7d4b89dc9dc..15b0d88f9f1 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_eds.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_eds.go @@ -19,6 +19,7 @@ package xdsresource import ( "fmt" + "math" "net" "strconv" @@ -118,6 +119,7 @@ func parseEDSRespProto(m *v3endpointpb.ClusterLoadAssignment, logger *grpclog.Pr ret.Drops = append(ret.Drops, parseDropPolicy(dropPolicy)) } priorities := make(map[uint32]map[string]bool) + sumOfWeights := make(map[uint32]uint64) for _, locality := range m.Endpoints { l := locality.GetLocality() if l == nil { @@ -128,17 +130,21 @@ func parseEDSRespProto(m *v3endpointpb.ClusterLoadAssignment, logger *grpclog.Pr logger.Warningf("Ignoring locality %s with weight 0", pretty.ToJSON(l)) continue } - lid := internal.LocalityID{ - Region: l.Region, - Zone: l.Zone, - SubZone: l.SubZone, - } priority := locality.GetPriority() + sumOfWeights[priority] += uint64(weight) + if sumOfWeights[priority] > math.MaxUint32 { + return EndpointsUpdate{}, fmt.Errorf("sum of weights of localities at the same priority %d exceeded maximal value", priority) + } localitiesWithPriority := priorities[priority] if localitiesWithPriority == nil { localitiesWithPriority = make(map[string]bool) priorities[priority] = localitiesWithPriority } + lid := internal.LocalityID{ + Region: l.Region, + Zone: l.Zone, + SubZone: l.SubZone, + } lidStr, _ := lid.ToString() if localitiesWithPriority[lidStr] { return EndpointsUpdate{}, fmt.Errorf("duplicate locality %s with the same priority %v", lidStr, priority) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go index 28ceb11c6e1..ea7b6425f01 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go @@ -99,6 +99,31 @@ func (s) TestEDSParseRespProto(t *testing.T) { }(), want: EndpointsUpdate{}, }, + { + name: "max sum of weights at the same priority exceeded", + m: func() *v3endpointpb.ClusterLoadAssignment { + clab0 := newClaBuilder("test", nil) + clab0.addLocality("locality-1", 1, 0, []string{"addr1:314"}, &addLocalityOptions{ + Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_UNHEALTHY}, + Weight: []uint32{271}, + }) + clab0.addLocality("locality-2", 1431655765, 1, []string{"addr2:159"}, &addLocalityOptions{ + Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_DRAINING}, + Weight: []uint32{828}, + }) + clab0.addLocality("locality-3", 1431655765, 1, []string{"addr2:73"}, &addLocalityOptions{ + Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_HEALTHY}, + Weight: []uint32{997}, + }) + clab0.addLocality("locality-4", 1431655766, 1, []string{"addr2:44"}, &addLocalityOptions{ + Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_UNHEALTHY}, + Weight: []uint32{1143}, + }) + return clab0.Build() + }(), + want: EndpointsUpdate{}, + wantErr: true, + }, { name: "good", m: func() *v3endpointpb.ClusterLoadAssignment { From d8e7f9e5efce6c9cfa44cfde0842a28386344b1c Mon Sep 17 00:00:00 2001 From: erni27 Date: Mon, 10 Oct 2022 18:13:46 +0200 Subject: [PATCH 2/2] Simplify test case --- .../xdsresource/unmarshal_eds_test.go | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go index ea7b6425f01..02718e09dda 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go @@ -103,22 +103,9 @@ func (s) TestEDSParseRespProto(t *testing.T) { name: "max sum of weights at the same priority exceeded", m: func() *v3endpointpb.ClusterLoadAssignment { clab0 := newClaBuilder("test", nil) - clab0.addLocality("locality-1", 1, 0, []string{"addr1:314"}, &addLocalityOptions{ - Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_UNHEALTHY}, - Weight: []uint32{271}, - }) - clab0.addLocality("locality-2", 1431655765, 1, []string{"addr2:159"}, &addLocalityOptions{ - Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_DRAINING}, - Weight: []uint32{828}, - }) - clab0.addLocality("locality-3", 1431655765, 1, []string{"addr2:73"}, &addLocalityOptions{ - Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_HEALTHY}, - Weight: []uint32{997}, - }) - clab0.addLocality("locality-4", 1431655766, 1, []string{"addr2:44"}, &addLocalityOptions{ - Health: []v3corepb.HealthStatus{v3corepb.HealthStatus_UNHEALTHY}, - Weight: []uint32{1143}, - }) + clab0.addLocality("locality-1", 1, 0, []string{"addr1:314"}, nil) + clab0.addLocality("locality-2", 4294967295, 1, []string{"addr2:159"}, nil) + clab0.addLocality("locality-3", 1, 1, []string{"addr2:88"}, nil) return clab0.Build() }(), want: EndpointsUpdate{},