xds: use federation env var to guard new-style resource name parsing (#29725)

src/core/ext/xds/xds_bootstrap.cc

@@ -44,8 +44,6 @@
 
 namespace grpc_core {
 
-namespace {
-
 // TODO(donnadionne): check to see if federation is enabled, this will be
 // removed once federation is fully integrated and enabled by default.
 bool XdsFederationEnabled() {
@@ -56,6 +54,8 @@ bool XdsFederationEnabled() {
   return parse_succeeded && parsed_value;
 }
 
+namespace {
+
 grpc_error_handle ParseChannelCreds(const Json::Object& json, size_t idx,
                                     XdsBootstrap::XdsServer* server) {
   std::vector<grpc_error_handle> error_list;

src/core/ext/xds/xds_bootstrap.h

@@ -37,6 +37,8 @@
 
 namespace grpc_core {
 
+bool XdsFederationEnabled();
+
 class XdsClient;
 
 class XdsBootstrap {

src/core/ext/xds/xds_client.cc

@@ -802,7 +802,7 @@ void XdsClient::ChannelState::AdsCallState::AdsResponseParser::ParseResource(
   }
   // Check the resource name.
   auto resource_name =
-      XdsClient::ParseXdsResourceName(result->name, result_.type);
+      xds_client()->ParseXdsResourceName(result->name, result_.type);
   if (!resource_name.ok()) {
     result_.errors.emplace_back(absl::StrCat(
         "resource index ", idx, ": Cannot parse xDS resource name \"",
@@ -1802,6 +1802,7 @@ XdsClient::XdsClient(std::unique_ptr<XdsBootstrap> bootstrap,
       bootstrap_(std::move(bootstrap)),
       args_(ModifyChannelArgs(args)),
       request_timeout_(GetRequestTimeout(args)),
+      xds_federation_enabled_(XdsFederationEnabled()),
       interested_parties_(grpc_pollset_set_create()),
       certificate_provider_store_(MakeOrphanable<CertificateProviderStore>(
           bootstrap_->certificate_providers())),
@@ -1942,6 +1943,7 @@ void XdsClient::CancelResourceWatch(const XdsResourceType* type,
   // authority_state_map_, so we check both, just to be safe.
   invalid_watchers_.erase(watcher);
   // Find authority.
+  if (!resource_name.ok()) return;
   auto authority_it = authority_state_map_.find(resource_name->authority);
   if (authority_it == authority_state_map_.end()) return;
   AuthorityState& authority_state = authority_it->second;
@@ -1994,7 +1996,7 @@ absl::StatusOr<XdsClient::XdsResourceName> XdsClient::ParseXdsResourceName(
     absl::string_view name, const XdsResourceType* type) {
   // Old-style names use the empty string for authority.
   // authority is prefixed with "old:" to indicate that it's an old-style name.
-  if (!absl::StartsWith(name, "xdstp:")) {
+  if (!xds_federation_enabled_ || !absl::StartsWith(name, "xdstp:")) {
     return XdsResourceName{"old:", {std::string(name), {}}};
   }
   // New style name.  Parse URI.

src/core/ext/xds/xds_client.h

@@ -286,7 +286,7 @@ class XdsClient : public DualRefCounted<XdsClient> {
   const XdsResourceType* GetResourceTypeLocked(absl::string_view resource_type)
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_);
 
-  static absl::StatusOr<XdsResourceName> ParseXdsResourceName(
+  absl::StatusOr<XdsResourceName> ParseXdsResourceName(
       absl::string_view name, const XdsResourceType* type);
   static std::string ConstructFullXdsResourceName(
       absl::string_view authority, absl::string_view resource_type,
@@ -302,6 +302,7 @@ class XdsClient : public DualRefCounted<XdsClient> {
   std::unique_ptr<XdsBootstrap> bootstrap_;
   grpc_channel_args* args_;
   const Duration request_timeout_;
+  const bool xds_federation_enabled_;
   grpc_pollset_set* interested_parties_;
   OrphanablePtr<CertificateProviderStore> certificate_provider_store_;
   XdsApi api_;

test/cpp/end2end/xds/xds_core_end2end_test.cc

@@ -1057,6 +1057,52 @@ TEST_P(XdsFederationTest, FederationServer) {
   WaitForAllBackends(DEBUG_LOCATION);
 }
 
+//
+// XdsFederationDisabledTest
+//
+
+using XdsFederationDisabledTest = XdsEnd2endTest;
+
+// Runs with RDS so that we know all resource types work properly.
+INSTANTIATE_TEST_SUITE_P(
+    XdsTest, XdsFederationDisabledTest,
+    ::testing::Values(XdsTestType().set_enable_rds_testing()),
+    &XdsTestType::Name);
+
+TEST_P(XdsFederationDisabledTest, FederationDisabledWithNewStyleNames) {
+  const char* kNewRouteConfigName =
+      "xdstp://xds.example.com/envoy.config.route.v3.RouteConfiguration/"
+      "new_route_config_name";
+  const char* kNewClusterName =
+      "xdstp://xds.example.com/envoy.config.cluster.v3.Cluster/"
+      "cluster_name";
+  const char* kNewEdsResourceName =
+      "xdstp://xds.example.com/envoy.config.endpoint.v3.ClusterLoadAssignment/"
+      "edsservice_name";
+  InitClient();
+  CreateAndStartBackends(1);
+  EdsResourceArgs args({{"locality0", CreateEndpointsForBackends()}});
+  balancer_->ads_service()->SetEdsResource(
+      BuildEdsResource(args, kNewEdsResourceName));
+  // New cluster
+  Cluster new_cluster = default_cluster_;
+  new_cluster.set_name(kNewClusterName);
+  new_cluster.mutable_eds_cluster_config()->set_service_name(
+      kNewEdsResourceName);
+  balancer_->ads_service()->SetCdsResource(new_cluster);
+  // New RouteConfig
+  RouteConfiguration new_route_config = default_route_config_;
+  new_route_config.set_name(kNewRouteConfigName);
+  new_route_config.mutable_virtual_hosts(0)
+      ->mutable_routes(0)
+      ->mutable_route()
+      ->set_cluster(kNewClusterName);
+  SetListenerAndRouteConfiguration(balancer_.get(), default_listener_,
+                                   new_route_config);
+  // Channel should work.
+  CheckRpcSendOk(DEBUG_LOCATION);
+}
+
 //
 // XdsFederationLoadReportingTest - xDS federation and load reporting
 //