Merge pull request #23910 from markdroth/xds_duplicate_resources

xds: NACK if response includes duplicate resource names.

src/core/ext/xds/xds_api.cc

@@ -1746,6 +1746,12 @@ grpc_error* CdsResponseParse(
         expected_cluster_names.end()) {
       continue;
     }
+    // Fail on duplicate resources.
+    if (cds_update_map->find(cluster_name) != cds_update_map->end()) {
+      return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
+          absl::StrCat("duplicate resource name \"", cluster_name, "\"")
+              .c_str());
+    }
     // Check the cluster_discovery_type.
     if (!envoy_config_cluster_v3_Cluster_has_type(cluster)) {
       return GRPC_ERROR_CREATE_FROM_STATIC_STRING("DiscoveryType not found.");
@@ -1933,8 +1939,7 @@ grpc_error* EdsResponseParse(
           "Can't parse cluster_load_assignment.");
     }
     MaybeLogClusterLoadAssignment(client, tracer, cluster_load_assignment);
-    // Check the cluster name (which actually means eds_service_name). Ignore
-    // unexpected names.
+    // Check the EDS service name.  Ignore unexpected names.
     std::string eds_service_name = UpbStringToStdString(
         envoy_config_endpoint_v3_ClusterLoadAssignment_cluster_name(
             cluster_load_assignment));
@@ -1942,6 +1947,12 @@ grpc_error* EdsResponseParse(
         expected_eds_service_names.end()) {
       continue;
     }
+    // Fail on duplicate resources.
+    if (eds_update_map->find(eds_service_name) != eds_update_map->end()) {
+      return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
+          absl::StrCat("duplicate resource name \"", eds_service_name, "\"")
+              .c_str());
+    }
     // Get the endpoints.
     size_t locality_size;
     const envoy_config_endpoint_v3_LocalityLbEndpoints* const* endpoints =

test/cpp/end2end/xds_end2end_test.cc

@@ -996,7 +996,7 @@ class AdsServiceImpl : public std::enable_shared_from_this<AdsServiceImpl> {
         // (even the unchanged ones)
         for (const auto& p : subscription_name_map) {
           const std::string& resource_name = p.first;
-          if (resources_added_to_response.find(resource_type) ==
+          if (resources_added_to_response.find(resource_name) ==
               resources_added_to_response.end()) {
             const ResourceState& resource_state =
                 parent_->resource_map_[resource_type][resource_name];