xDS: don't NACK RouteConfig with a VirtualHost containing no valid routes (#32069)

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_manager.cc

@@ -101,8 +101,6 @@ class XdsClusterManagerLbConfig : public LoadBalancingPolicy::Config {
   }
 
   static const JsonLoaderInterface* JsonLoader(const JsonArgs&);
-  void JsonPostLoad(const Json& json, const JsonArgs&,
-                    ValidationErrors* errors);
 
  private:
   std::map<std::string, Child> cluster_map_;
@@ -666,16 +664,6 @@ const JsonLoaderInterface* XdsClusterManagerLbConfig::JsonLoader(
   return loader;
 }
 
-void XdsClusterManagerLbConfig::JsonPostLoad(const Json&, const JsonArgs&,
-                                             ValidationErrors* errors) {
-  if (cluster_map_.empty()) {
-    ValidationErrors::ScopedField field(errors, ".children");
-    if (!errors->FieldHasErrors()) {
-      errors->AddError("no valid children configured");
-    }
-  }
-}
-
 class XdsClusterManagerLbFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -373,7 +373,7 @@ class XdsResolver : public Resolver {
 
   std::string route_config_name_;
   RouteConfigWatcher* route_config_watcher_ = nullptr;
-  XdsRouteConfigResource::VirtualHost current_virtual_host_;
+  absl::optional<XdsRouteConfigResource::VirtualHost> current_virtual_host_;
   std::map<std::string /*cluster_specifier_plugin_name*/,
            std::string /*LB policy config*/>
       cluster_specifier_plugin_map_;
@@ -443,8 +443,8 @@ XdsResolver::XdsConfigSelector::XdsConfigSelector(
   // weighted_cluster_state field points to the memory in the route field, so
   // moving the entry in a reallocation will cause the string_view to point to
   // invalid data.
-  route_table_.reserve(resolver_->current_virtual_host_.routes.size());
-  for (auto& route : resolver_->current_virtual_host_.routes) {
+  route_table_.reserve(resolver_->current_virtual_host_->routes.size());
+  for (auto& route : resolver_->current_virtual_host_->routes) {
     if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_resolver_trace)) {
       gpr_log(GPR_INFO, "[xds_resolver %p] XdsConfigSelector %p: route: %s",
               resolver_.get(), this, route.ToString().c_str());
@@ -598,7 +598,7 @@ XdsResolver::XdsConfigSelector::CreateMethodConfig(
       static_cast<const GrpcXdsBootstrap&>(resolver_->xds_client_->bootstrap())
           .http_filter_registry(),
       resolver_->current_listener_.http_filters,
-      resolver_->current_virtual_host_, route, cluster_weight,
+      resolver_->current_virtual_host_.value(), route, cluster_weight,
       resolver_->args_);
   if (!result.ok()) return result.status();
   for (const auto& p : result->per_filter_configs) {
@@ -999,7 +999,7 @@ void XdsResolver::OnResourceDoesNotExist(std::string context) {
   if (xds_client_ == nullptr) {
     return;
   }
-  current_virtual_host_.routes.clear();
+  current_virtual_host_.reset();
   Result result;
   result.addresses.emplace();
   result.service_config = ServiceConfigImpl::Create(args_, "{}");
@@ -1051,7 +1051,7 @@ XdsResolver::CreateServiceConfig() {
 }
 
 void XdsResolver::GenerateResult() {
-  if (current_virtual_host_.routes.empty()) return;
+  if (!current_virtual_host_.has_value()) return;
   // First create XdsConfigSelector, which may add new entries to the cluster
   // state map, and then CreateServiceConfig for LB policies.
   absl::Status status;

src/core/ext/xds/xds_route_config.cc

@@ -1056,7 +1056,6 @@ XdsRouteConfigResource XdsRouteConfigResource::Parse(
     }
     // Parse routes.
     ValidationErrors::ScopedField field2(errors, ".routes");
-    const size_t original_error_size = errors->size();
     size_t num_routes;
     const envoy_config_route_v3_Route* const* routes =
         envoy_config_route_v3_VirtualHost_routes(virtual_hosts[i], &num_routes);
@@ -1067,9 +1066,6 @@ XdsRouteConfigResource XdsRouteConfigResource::Parse(
                               &cluster_specifier_plugins_not_seen, errors);
       if (route.has_value()) vhost.routes.emplace_back(std::move(*route));
     }
-    if (errors->size() == original_error_size && vhost.routes.empty()) {
-      errors->AddError("no valid routes in VirtualHost");
-    }
   }
   // For cluster specifier plugins that were not used in any route action,
   // delete them from the update, since they will never be used.

test/core/xds/xds_route_config_resource_type_test.cc

@@ -274,25 +274,6 @@ TEST_F(VirtualHostTest, NoDomainsSpecified) {
       << decode_result.resource.status();
 }
 
-TEST_F(VirtualHostTest, NoRoutesInVirtualHost) {
-  RouteConfiguration route_config;
-  route_config.set_name("foo");
-  auto* vhost = route_config.add_virtual_hosts();
-  vhost->add_domains("*");
-  std::string serialized_resource;
-  ASSERT_TRUE(route_config.SerializeToString(&serialized_resource));
-  auto* resource_type = XdsRouteConfigResourceType::Get();
-  auto decode_result =
-      resource_type->Decode(decode_context_, serialized_resource);
-  EXPECT_EQ(decode_result.resource.status().code(),
-            absl::StatusCode::kInvalidArgument);
-  EXPECT_EQ(decode_result.resource.status().message(),
-            "errors validating RouteConfiguration resource: ["
-            "field:virtual_hosts[0].routes "
-            "error:no valid routes in VirtualHost]")
-      << decode_result.resource.status();
-}
-
 //
 // typed_per_filter_config tests
 //

test/cpp/end2end/xds/xds_routing_end2end_test.cc

@@ -482,13 +482,25 @@ TEST_P(LdsRdsTest, NoMatchingRoute) {
   EXPECT_EQ(response_state->state, AdsServiceImpl::ResponseState::ACKED);
 }
 
+TEST_P(LdsRdsTest, EmptyRouteList) {
+  RouteConfiguration route_config = default_route_config_;
+  route_config.mutable_virtual_hosts(0)->clear_routes();
+  SetRouteConfiguration(balancer_.get(), route_config);
+  CheckRpcSendFailure(DEBUG_LOCATION, StatusCode::UNAVAILABLE,
+                      "No matching route found in xDS route config");
+  // Do a bit of polling, to allow the ACK to get to the ADS server.
+  channel_->WaitForConnected(grpc_timeout_milliseconds_to_deadline(100));
+  auto response_state = RouteConfigurationResponseState(balancer_.get());
+  ASSERT_TRUE(response_state.has_value());
+  EXPECT_EQ(response_state->state, AdsServiceImpl::ResponseState::ACKED);
+}
+
 // Testing just one example of an invalid resource here.
 // Unit tests for XdsRouteConfigResourceType have exhaustive tests for all
 // of the invalid cases.
 TEST_P(LdsRdsTest, NacksInvalidRouteConfig) {
   RouteConfiguration route_config = default_route_config_;
-  auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0);
-  route1->mutable_match()->set_prefix("grpc.testing.EchoTest1Service/");
+  route_config.mutable_virtual_hosts(0)->mutable_routes(0)->clear_match();
   SetRouteConfiguration(balancer_.get(), route_config);
   const auto response_state = WaitForRdsNack(DEBUG_LOCATION);
   ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK";
@@ -505,8 +517,8 @@ TEST_P(LdsRdsTest, NacksInvalidRouteConfig) {
                 "field:api_listener.api_listener.value["
                 "envoy.extensions.filters.network.http_connection_manager.v3"
                 ".HttpConnectionManager].route_config.",
-          "virtual_hosts[0].routes "
-          "error:no valid routes in VirtualHost]]"));
+          "virtual_hosts[0].routes[0].match "
+          "error:field not present]]"));
 }
 
 // Tests that LDS client should fail RPCs with UNAVAILABLE status code if the