xds: Nack xds response when weighted cluster total weight sums zero (#…

…9738)
grpc · Dec 7, 2022 · 7864170 · 7864170
1 parent bf0b92a
commit 7864170
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 0 deletions.
diff --git a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java
@@ -511,6 +511,7 @@ static StructOrError<RouteAction> parseRouteAction(
           return StructOrError.fromError("No cluster found in weighted cluster list");
         }
         List<ClusterWeight> weightedClusters = new ArrayList<>();
+        int clusterWeightSum = 0;
         for (io.envoyproxy.envoy.config.route.v3.WeightedCluster.ClusterWeight clusterWeight
             : clusterWeights) {
           StructOrError<ClusterWeight> clusterWeightOrError =
@@ -519,8 +520,12 @@ static StructOrError<RouteAction> parseRouteAction(
             return StructOrError.fromError("RouteAction contains invalid ClusterWeight: "
                 + clusterWeightOrError.getErrorDetail());
           }
+          clusterWeightSum += clusterWeight.getWeight().getValue();
           weightedClusters.add(clusterWeightOrError.getStruct());
         }
+        if (clusterWeightSum <= 0) {
+          return StructOrError.fromError("Sum of cluster weights should be above 0.");
+        }
         return StructOrError.fromStruct(VirtualHost.Route.RouteAction.forWeightedClusters(
             weightedClusters, hashPolicies, timeoutNano, retryPolicy));
       case CLUSTER_SPECIFIER_PLUGIN:

diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java
@@ -521,6 +521,28 @@ public void parseRouteAction_withWeightedCluster() {
         ClusterWeight.create("cluster-bar", 70, ImmutableMap.<String, FilterConfig>of()));
   }
 
+  @Test
+  public void parseRouteAction_weightedClusterSum() {
+    io.envoyproxy.envoy.config.route.v3.RouteAction proto =
+        io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder()
+            .setWeightedClusters(
+                WeightedCluster.newBuilder()
+                    .addClusters(
+                        WeightedCluster.ClusterWeight
+                            .newBuilder()
+                            .setName("cluster-foo")
+                            .setWeight(UInt32Value.newBuilder().setValue(0)))
+                    .addClusters(WeightedCluster.ClusterWeight
+                        .newBuilder()
+                        .setName("cluster-bar")
+                        .setWeight(UInt32Value.newBuilder().setValue(0))))
+            .build();
+    StructOrError<RouteAction> struct =
+        XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, false,
+            ImmutableMap.of(), ImmutableSet.of());
+    assertThat(struct.getErrorDetail()).isEqualTo("Sum of cluster weights should be above 0.");
+  }
+
   @Test
   public void parseRouteAction_withTimeoutByGrpcTimeoutHeaderMax() {
     io.envoyproxy.envoy.config.route.v3.RouteAction proto =

diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java
@@ -40,6 +40,7 @@
 import com.google.protobuf.util.Durations;
 import io.envoyproxy.envoy.config.cluster.v3.OutlierDetection;
 import io.envoyproxy.envoy.config.route.v3.FilterConfig;
+import io.envoyproxy.envoy.config.route.v3.WeightedCluster;
 import io.envoyproxy.envoy.extensions.filters.http.router.v3.Router;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance;
 import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext;
@@ -1363,6 +1364,51 @@ public void rdsResponseErrorHandling_someResourcesFailedUnpack() {
     verify(rdsResourceWatcher).onChanged(any(RdsUpdate.class));
   }
 
+  @Test
+  public void rdsResponseErrorHandling_nackWeightedSumZero() {
+    DiscoveryRpcCall call = startResourceWatcher(XdsRouteConfigureResource.getInstance(),
+        RDS_RESOURCE, rdsResourceWatcher);
+    verifyResourceMetadataRequested(RDS, RDS_RESOURCE);
+
+    io.envoyproxy.envoy.config.route.v3.RouteAction routeAction =
+        io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder()
+            .setWeightedClusters(
+                WeightedCluster.newBuilder()
+                    .addClusters(
+                        WeightedCluster.ClusterWeight
+                            .newBuilder()
+                            .setName("cluster-foo")
+                            .setWeight(UInt32Value.newBuilder().setValue(0)))
+                    .addClusters(WeightedCluster.ClusterWeight
+                        .newBuilder()
+                        .setName("cluster-bar")
+                        .setWeight(UInt32Value.newBuilder().setValue(0))))
+            .build();
+    io.envoyproxy.envoy.config.route.v3.Route route =
+        io.envoyproxy.envoy.config.route.v3.Route.newBuilder()
+            .setName("route-blade")
+            .setMatch(
+                io.envoyproxy.envoy.config.route.v3.RouteMatch.newBuilder()
+                    .setPath("/service/method"))
+            .setRoute(routeAction)
+            .build();
+
+    Any zeroWeightSum = Any.pack(mf.buildRouteConfiguration(RDS_RESOURCE,
+        Arrays.asList(mf.buildVirtualHost(Arrays.asList(route), ImmutableMap.of()))));
+    List<Any> resources = ImmutableList.of(zeroWeightSum);
+    call.sendResponse(RDS, resources, VERSION_1, "0000");
+
+    List<String> errors = ImmutableList.of(
+        "RDS response RouteConfiguration \'route-configuration.googleapis.com\' validation error: "
+            + "RouteConfiguration contains invalid virtual host: Virtual host [do not care] "
+            + "contains invalid route : Route [route-blade] contains invalid RouteAction: "
+            + "Sum of cluster weights should be above 0.");
+    verifySubscribedResourcesMetadataSizes(0, 0, 1, 0);
+    // The response is NACKed with the same error message.
+    call.verifyRequestNack(RDS, RDS_RESOURCE, "", "0000", NODE, errors);
+    verify(rdsResourceWatcher, never()).onChanged(any(RdsUpdate.class));
+  }
+
   /**
    * Tests a subscribed RDS resource transitioned to and from the invalid state.
    *