Skip to content

Commit 3d79966

Browse files
ArkeleniaArkelenia
committed
Pick VHDS virtual hosts over RDS ones when their names collide
Signed-off-by: Frederic Hemery <frederic.hemery@datadoghq.com>
1 parent ccb42a3 commit 3d79966

2 files changed

Lines changed: 202 additions & 0 deletions

File tree

‎source/common/router/route_config_update_receiver_impl.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "source/common/common/assert.h"
1010
#include "source/common/common/fmt.h"
11+
#include "source/common/common/logger.h"
1112
#include "source/common/common/thread.h"
1213
#include "source/common/config/resource_name.h"
1314
#include "source/common/protobuf/utility.h"
@@ -26,6 +27,11 @@ void rebuildRouteConfigVirtualHosts(
2627
envoy::config::route::v3::RouteConfiguration& route_config) {
2728
route_config.clear_virtual_hosts();
2829
for (const auto& vhost : rds_vhosts) {
30+
if (vhds_vhosts.contains(vhost.first)) {
31+
ENVOY_LOG_MISC(debug, "VHDS virtual host '{}' overrides RDS virtual host with the same name",
32+
vhost.first);
33+
continue;
34+
}
2935
route_config.mutable_virtual_hosts()->Add()->CheckTypeAndMergeFrom(vhost.second);
3036
}
3137
for (const auto& vhost : vhds_vhosts) {

‎test/common/router/vhds_test.cc‎

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,202 @@ name: my_route
316316
"vhost_vhds1" == actual_vhost_2.name());
317317
}
318318

319+
// verify that when RDS and VHDS define virtual hosts with the same name, VHDS takes precedence
320+
TEST_F(VhdsTest, VhdsOverridesRdsVirtualHostWithSameName) {
321+
const auto route_config =
322+
TestUtility::parseYaml<envoy::config::route::v3::RouteConfiguration>(R"EOF(
323+
name: my_route
324+
virtual_hosts:
325+
- name: overlapping_vhost
326+
domains: ["vhost.rds.domain"]
327+
routes:
328+
- match: { prefix: "/rds" }
329+
route: { cluster: rds_service }
330+
- name: vhost_rds_only
331+
domains: ["vhost.rds.only"]
332+
routes:
333+
- match: { prefix: "/rdsonly" }
334+
route: { cluster: rds_only_service }
335+
vhds:
336+
config_source:
337+
api_config_source:
338+
api_type: DELTA_GRPC
339+
grpc_services:
340+
envoy_grpc:
341+
cluster_name: xds_cluster
342+
)EOF");
343+
RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config);
344+
345+
VhdsSubscriptionPtr subscription = VhdsSubscription::createVhdsSubscription(
346+
config_update_info, factory_context_, context_, provider_)
347+
.value();
348+
EXPECT_EQ(2UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
349+
350+
// Add a VHDS virtual host with the same name as one of the RDS virtual hosts
351+
auto vhost = buildVirtualHost("overlapping_vhost", "vhost.vhds.domain");
352+
const auto& added_resources = buildAddedResources({vhost});
353+
const auto decoded_resources =
354+
TestUtility::decodeResources<envoy::config::route::v3::VirtualHost>(added_resources);
355+
const Protobuf::RepeatedPtrField<std::string> removed_resources;
356+
EXPECT_TRUE(factory_context_.cluster_manager_.subscription_factory_.callbacks_
357+
->onConfigUpdate(decoded_resources.refvec_, removed_resources, "1")
358+
.ok());
359+
360+
// Should have 2 virtual hosts: vhost_rds_only from RDS + overlapping_vhost from VHDS
361+
EXPECT_EQ(2UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
362+
363+
// Verify the VHDS version of the overlapping vhost is used (with vhds domain, not rds domain)
364+
bool found_vhds_version = false;
365+
bool found_rds_only = false;
366+
for (int i = 0; i < config_update_info->protobufConfigurationCast().virtual_hosts_size(); ++i) {
367+
const auto& vh = config_update_info->protobufConfigurationCast().virtual_hosts(i);
368+
if (vh.name() == "overlapping_vhost") {
369+
// The VHDS version should have "vhost.vhds.domain", not "vhost.rds.domain"
370+
EXPECT_EQ("vhost.vhds.domain", vh.domains(0));
371+
found_vhds_version = true;
372+
} else if (vh.name() == "vhost_rds_only") {
373+
found_rds_only = true;
374+
}
375+
}
376+
EXPECT_TRUE(found_vhds_version) << "VHDS version of overlapping_vhost not found";
377+
EXPECT_TRUE(found_rds_only) << "vhost_rds_only not found";
378+
}
379+
380+
// verify VHDS precedence is maintained after an RDS update
381+
TEST_F(VhdsTest, VhdsOverridesRdsVirtualHostAfterRdsUpdate) {
382+
const auto route_config =
383+
TestUtility::parseYaml<envoy::config::route::v3::RouteConfiguration>(R"EOF(
384+
name: my_route
385+
virtual_hosts:
386+
- name: overlapping_vhost
387+
domains: ["vhost.rds.domain"]
388+
routes:
389+
- match: { prefix: "/rds" }
390+
route: { cluster: rds_service }
391+
vhds:
392+
config_source:
393+
api_config_source:
394+
api_type: DELTA_GRPC
395+
grpc_services:
396+
envoy_grpc:
397+
cluster_name: xds_cluster
398+
)EOF");
399+
RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config);
400+
401+
VhdsSubscriptionPtr subscription = VhdsSubscription::createVhdsSubscription(
402+
config_update_info, factory_context_, context_, provider_)
403+
.value();
404+
405+
// Add a VHDS virtual host with the same name
406+
auto vhost = buildVirtualHost("overlapping_vhost", "vhost.vhds.domain");
407+
const auto& added_resources = buildAddedResources({vhost});
408+
const auto decoded_resources =
409+
TestUtility::decodeResources<envoy::config::route::v3::VirtualHost>(added_resources);
410+
const Protobuf::RepeatedPtrField<std::string> removed_resources;
411+
EXPECT_TRUE(factory_context_.cluster_manager_.subscription_factory_.callbacks_
412+
->onConfigUpdate(decoded_resources.refvec_, removed_resources, "1")
413+
.ok());
414+
415+
// Now trigger an RDS update that still has the same-named vhost
416+
const auto updated_route_config =
417+
TestUtility::parseYaml<envoy::config::route::v3::RouteConfiguration>(R"EOF(
418+
name: my_route
419+
virtual_hosts:
420+
- name: overlapping_vhost
421+
domains: ["vhost.rds.updated"]
422+
routes:
423+
- match: { prefix: "/rds-updated" }
424+
route: { cluster: rds_updated_service }
425+
- name: vhost_rds_new
426+
domains: ["vhost.rds.new"]
427+
routes:
428+
- match: { prefix: "/rdsnew" }
429+
route: { cluster: rds_new_service }
430+
vhds:
431+
config_source:
432+
api_config_source:
433+
api_type: DELTA_GRPC
434+
grpc_services:
435+
envoy_grpc:
436+
cluster_name: xds_cluster
437+
)EOF");
438+
config_update_info->onRdsUpdate(updated_route_config, "2");
439+
440+
// Should have 2: vhost_rds_new from RDS + overlapping_vhost from VHDS
441+
EXPECT_EQ(2UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
442+
443+
// Verify VHDS version still takes precedence after RDS update
444+
bool found_vhds_version = false;
445+
for (int i = 0; i < config_update_info->protobufConfigurationCast().virtual_hosts_size(); ++i) {
446+
const auto& vh = config_update_info->protobufConfigurationCast().virtual_hosts(i);
447+
if (vh.name() == "overlapping_vhost") {
448+
EXPECT_EQ("vhost.vhds.domain", vh.domains(0));
449+
found_vhds_version = true;
450+
}
451+
}
452+
EXPECT_TRUE(found_vhds_version) << "VHDS version of overlapping_vhost not found after RDS update";
453+
}
454+
455+
// verify that removing a VHDS virtual host that was overriding an RDS one causes the RDS vhost to
456+
// reappear
457+
TEST_F(VhdsTest, RemovingVhdsVirtualHostRestoresRdsVirtualHost) {
458+
const auto route_config =
459+
TestUtility::parseYaml<envoy::config::route::v3::RouteConfiguration>(R"EOF(
460+
name: my_route
461+
virtual_hosts:
462+
- name: overlapping_vhost
463+
domains: ["vhost.rds.domain"]
464+
routes:
465+
- match: { prefix: "/rds" }
466+
route: { cluster: rds_service }
467+
vhds:
468+
config_source:
469+
api_config_source:
470+
api_type: DELTA_GRPC
471+
grpc_services:
472+
envoy_grpc:
473+
cluster_name: xds_cluster
474+
)EOF");
475+
RouteConfigUpdatePtr config_update_info = makeRouteConfigUpdate(route_config);
476+
477+
VhdsSubscriptionPtr subscription = VhdsSubscription::createVhdsSubscription(
478+
config_update_info, factory_context_, context_, provider_)
479+
.value();
480+
EXPECT_EQ(1UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
481+
482+
// Add a VHDS virtual host with the same name as the RDS one
483+
auto vhost = buildVirtualHost("overlapping_vhost", "vhost.vhds.domain");
484+
const auto& added_resources = buildAddedResources({vhost});
485+
const auto decoded_resources =
486+
TestUtility::decodeResources<envoy::config::route::v3::VirtualHost>(added_resources);
487+
Protobuf::RepeatedPtrField<std::string> removed_resources;
488+
EXPECT_TRUE(factory_context_.cluster_manager_.subscription_factory_.callbacks_
489+
->onConfigUpdate(decoded_resources.refvec_, removed_resources, "1")
490+
.ok());
491+
492+
// VHDS version should override
493+
EXPECT_EQ(1UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
494+
EXPECT_EQ("vhost.vhds.domain",
495+
config_update_info->protobufConfigurationCast().virtual_hosts(0).domains(0));
496+
497+
// Now remove the VHDS virtual host
498+
const auto no_added_resources = buildAddedResources({});
499+
const auto no_decoded_resources =
500+
TestUtility::decodeResources<envoy::config::route::v3::VirtualHost>(no_added_resources);
501+
Protobuf::RepeatedPtrField<std::string> resources_to_remove;
502+
*resources_to_remove.Add() = "overlapping_vhost";
503+
EXPECT_TRUE(factory_context_.cluster_manager_.subscription_factory_.callbacks_
504+
->onConfigUpdate(no_decoded_resources.refvec_, resources_to_remove, "2")
505+
.ok());
506+
507+
// The RDS virtual host should reappear
508+
EXPECT_EQ(1UL, config_update_info->protobufConfigurationCast().virtual_hosts_size());
509+
EXPECT_EQ("overlapping_vhost",
510+
config_update_info->protobufConfigurationCast().virtual_hosts(0).name());
511+
EXPECT_EQ("vhost.rds.domain",
512+
config_update_info->protobufConfigurationCast().virtual_hosts(0).domains(0));
513+
}
514+
319515
} // namespace
320516
} // namespace Router
321517
} // namespace Envoy

0 commit comments

Comments
 (0)