address comments

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Arkelenia

source/common/router/route_config_update_receiver_impl.cc

@@ -8,6 +8,7 @@
 
 #include "source/common/common/assert.h"
 #include "source/common/common/fmt.h"
+#include "source/common/common/logger.h"
 #include "source/common/common/thread.h"
 #include "source/common/config/resource_name.h"
 #include "source/common/protobuf/utility.h"
@@ -26,8 +27,9 @@ void rebuildRouteConfigVirtualHosts(
     envoy::config::route::v3::RouteConfiguration& route_config) {
   route_config.clear_virtual_hosts();
   for (const auto& vhost : rds_vhosts) {
-    auto found = vhds_vhosts.find(vhost.first);
-    if (found != vhds_vhosts.end()) {
+    if (vhds_vhosts.contains(vhost.first)) {
+      ENVOY_LOG_MISC(debug, "VHDS virtual host '{}' overrides RDS virtual host with the same name",
+                      vhost.first);
       continue;
     }
     route_config.mutable_virtual_hosts()->Add()->CheckTypeAndMergeFrom(vhost.second);

test/common/router/vhds_test.cc

@@ -373,8 +373,8 @@ name: my_route
       found_rds_only = true;
     }
   }
-  EXPECT_TRUE(found_vhds_version);
-  EXPECT_TRUE(found_rds_only);
+  EXPECT_TRUE(found_vhds_version) << "VHDS version of overlapping_vhost not found";
+  EXPECT_TRUE(found_rds_only) << "vhost_rds_only not found";
 }
 
 // verify VHDS precedence is maintained after an RDS update
@@ -441,12 +441,75 @@ name: my_route
   EXPECT_EQ(2UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
 
   // Verify VHDS version still takes precedence after RDS update
+  bool found_vhds_version = false;
   for (int i = 0; i < config_update_info->protobufConfigurationCast().virtual_hosts_size(); ++i) {
     const auto& vh = config_update_info->protobufConfigurationCast().virtual_hosts(i);
     if (vh.name() == "overlapping_vhost") {
       EXPECT_EQ("vhost.vhds.domain", vh.domains(0));
+      found_vhds_version = true;
     }
   }
+  EXPECT_TRUE(found_vhds_version) << "VHDS version of overlapping_vhost not found after RDS update";
+}
+
+// verify that removing a VHDS virtual host that was overriding an RDS one causes the RDS vhost to
+// reappear
+TEST_F(VhdsTest, RemovingVhdsVirtualHostRestoresRdsVirtualHost) {
+  const auto route_config =
+      TestUtility::parseYaml<envoy::config::route::v3::RouteConfiguration>(R"EOF(
+name: my_route
+virtual_hosts:
+- name: overlapping_vhost
+  domains: ["vhost.rds.domain"]
+  routes:
+  - match: { prefix: "/rds" }
+    route: { cluster: rds_service }
+vhds:
+  config_source:
+    api_config_source:
+      api_type: DELTA_GRPC
+      grpc_services:
+        envoy_grpc:
+          cluster_name: xds_cluster
+  )EOF");
+  RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config);
+
+  VhdsSubscriptionPtr subscription = VhdsSubscription::createVhdsSubscription(
+                                         config_update_info, factory_context_, context_, provider_)
+                                         .value();
+  EXPECT_EQ(1UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
+
+  // Add a VHDS virtual host with the same name as the RDS one
+  auto vhost = buildVirtualHost("overlapping_vhost", "vhost.vhds.domain");
+  const auto& added_resources = buildAddedResources({vhost});
+  const auto decoded_resources =
+      TestUtility::decodeResources<envoy::config::route::v3::VirtualHost>(added_resources);
+  Protobuf::RepeatedPtrField<std::string> removed_resources;
+  EXPECT_TRUE(factory_context_.cluster_manager_.subscription_factory_.callbacks_
+                  ->onConfigUpdate(decoded_resources.refvec_, removed_resources, "1")
+                  .ok());
+
+  // VHDS version should override
+  EXPECT_EQ(1UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
+  EXPECT_EQ("vhost.vhds.domain",
+            config_update_info->protobufConfigurationCast().virtual_hosts(0).domains(0));
+
+  // Now remove the VHDS virtual host
+  const auto no_added_resources = buildAddedResources({});
+  const auto no_decoded_resources =
+      TestUtility::decodeResources<envoy::config::route::v3::VirtualHost>(no_added_resources);
+  Protobuf::RepeatedPtrField<std::string> resources_to_remove;
+  *resources_to_remove.Add() = "overlapping_vhost";
+  EXPECT_TRUE(factory_context_.cluster_manager_.subscription_factory_.callbacks_
+                  ->onConfigUpdate(no_decoded_resources.refvec_, resources_to_remove, "2")
+                  .ok());
+
+  // The RDS virtual host should reappear
+  EXPECT_EQ(1UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
+  EXPECT_EQ("overlapping_vhost",
+            config_update_info->protobufConfigurationCast().virtual_hosts(0).name());
+  EXPECT_EQ("vhost.rds.domain",
+            config_update_info->protobufConfigurationCast().virtual_hosts(0).domains(0));
 }
 
 } // namespace