xds: reject ring_hash cluster with out-of-range or inverted ring size

Author: nvxbug

internal/xds/xdsclient/xdsresource/unmarshal_cds.go

@@ -137,6 +137,15 @@ func validateClusterAndConstructClusterUpdate(cluster *v3clusterpb.Cluster, serv
 		if max := rhc.GetMaximumRingSize(); max != nil {
 			maxSize = max.GetValue()
 		}
+		if minSize > ringHashSizeUpperBound {
+			return ClusterUpdate{}, fmt.Errorf("ring_hash_lb_config.minimum_ring_size %d is greater than upper bound %d in response: %+v", minSize, ringHashSizeUpperBound, cluster)
+		}
+		if maxSize > ringHashSizeUpperBound {
+			return ClusterUpdate{}, fmt.Errorf("ring_hash_lb_config.maximum_ring_size %d is greater than upper bound %d in response: %+v", maxSize, ringHashSizeUpperBound, cluster)
+		}
+		if minSize > maxSize {
+			return ClusterUpdate{}, fmt.Errorf("ring_hash_lb_config.minimum_ring_size %d is greater than maximum_ring_size %d in response: %+v", minSize, maxSize, cluster)
+		}
 
 		rhLBCfg := []byte(fmt.Sprintf("{\"minRingSize\": %d, \"maxRingSize\": %d}", minSize, maxSize))
 		lbPolicy = []byte(fmt.Sprintf(`[{"ring_hash_experimental": %s}]`, rhLBCfg))

internal/xds/xdsclient/xdsresource/unmarshal_cds_test.go

@@ -166,6 +166,16 @@ func (s) TestValidateCluster_Failure(t *testing.T) {
 		{
 			name: "ring-hash-max-bound-greater-than-upper-bound",
 			cluster: &v3clusterpb.Cluster{
+				Name:                 clusterName,
+				ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
+				EdsClusterConfig: &v3clusterpb.Cluster_EdsClusterConfig{
+					EdsConfig: &v3corepb.ConfigSource{
+						ConfigSourceSpecifier: &v3corepb.ConfigSource_Ads{
+							Ads: &v3corepb.AggregatedConfigSource{},
+						},
+					},
+					ServiceName: serviceName,
+				},
 				LbPolicy: v3clusterpb.Cluster_RING_HASH,
 				LbConfig: &v3clusterpb.Cluster_RingHashLbConfig_{
 					RingHashLbConfig: &v3clusterpb.Cluster_RingHashLbConfig{
@@ -175,6 +185,29 @@ func (s) TestValidateCluster_Failure(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "ring-hash-min-greater-than-max",
+			cluster: &v3clusterpb.Cluster{
+				Name:                 clusterName,
+				ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
+				EdsClusterConfig: &v3clusterpb.Cluster_EdsClusterConfig{
+					EdsConfig: &v3corepb.ConfigSource{
+						ConfigSourceSpecifier: &v3corepb.ConfigSource_Ads{
+							Ads: &v3corepb.AggregatedConfigSource{},
+						},
+					},
+					ServiceName: serviceName,
+				},
+				LbPolicy: v3clusterpb.Cluster_RING_HASH,
+				LbConfig: &v3clusterpb.Cluster_RingHashLbConfig_{
+					RingHashLbConfig: &v3clusterpb.Cluster_RingHashLbConfig{
+						MinimumRingSize: wrapperspb.UInt64(5000),
+						MaximumRingSize: wrapperspb.UInt64(100),
+					},
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "ring-hash-max-bound-greater-than-upper-bound-load-balancing-policy",
 			cluster: &v3clusterpb.Cluster{