Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions source/extensions/clusters/eds/eds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,10 @@ void EdsClusterImpl::update(
// Create a new LEDS subscription and add it to the subscriptions map.
LedsSubscriptionPtr leds_locality_subscription = std::make_unique<LedsSubscription>(
leds_config, edsServiceName(), *transport_factory_context_, info_->statsScope(),
[&, used_load_assignment]() {
// Called upon an update to the locality.
[this]() {
if (validateAllLedsUpdated()) {
BatchUpdateHelper helper(*this, *used_load_assignment);
ASSERT(cluster_load_assignment_ != nullptr);
BatchUpdateHelper helper(*this, *cluster_load_assignment_);
priority_set_.batchHostUpdate(helper);
}
});
Expand Down
60 changes: 60 additions & 0 deletions test/extensions/clusters/eds/eds_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2992,6 +2992,66 @@ TEST_F(EdsTest, OnConfigUpdateLedsAndEndpoints) {
"(resource: xdstp://foo/leds/collection) and a list of endpoints.");
}

class EdsWithLedsTest : public EdsTest {
public:
EdsWithLedsTest() {
ON_CALL(server_context_.cluster_manager_.subscription_factory_,
collectionSubscriptionFromUrl(_, _, _, _, _, _))
.WillByDefault(Invoke(
[this](const xds::core::v3::ResourceLocator&,
const envoy::config::core::v3::ConfigSource&, absl::string_view, Stats::Scope&,
Envoy::Config::SubscriptionCallbacks& callbacks,
Envoy::Config::OpaqueResourceDecoderSharedPtr) -> Config::SubscriptionPtr {
auto ret = std::make_unique<NiceMock<Config::MockSubscription>>();
leds_callbacks_ = &callbacks;
return ret;
}));
}

envoy::config::endpoint::v3::ClusterLoadAssignment
makeLedsClusterLoadAssignment(const std::string& cluster_name) {
envoy::config::endpoint::v3::ClusterLoadAssignment cla;
cla.set_cluster_name(cluster_name);
auto* endpoints = cla.add_endpoints();
auto* locality = endpoints->mutable_locality();
locality->set_region("us-east-1");
locality->set_zone("us-east-1a");
auto* leds_conf = endpoints->mutable_leds_cluster_locality_config();
leds_conf->set_leds_collection_name(
"xdstp://test/envoy.config.endpoint.v3.LbEndpoint/foo-endpoints/*");
auto* leds_config_source = leds_conf->mutable_leds_config()->mutable_api_config_source();
leds_config_source->set_api_type(envoy::config::core::v3::ApiConfigSource::DELTA_GRPC);
leds_config_source->add_grpc_services()->mutable_envoy_grpc()->set_cluster_name("xds_cluster");
return cla;
}

Config::SubscriptionCallbacks* leds_callbacks_{};
};

// Regression test: a LEDS callback firing after a subsequent EDS update must
// not dereference the stale cluster_load_assignment_ pointer.
TEST_F(EdsWithLedsTest, LedsCallbackAfterSubsequentEdsUpdateNoCrash) {
initialize();

auto cla = makeLedsClusterLoadAssignment("fare");

// First EDS update creates the LEDS subscription (not yet updated).
doOnConfigUpdateVerifyNoThrow(cla);
ASSERT_NE(nullptr, leds_callbacks_);

// Second EDS update destroys the old cluster_load_assignment_ but keeps
// the existing LEDS subscription.
doOnConfigUpdateVerifyNoThrow(cla);

// LEDS failure triggers the callback; would segfault on the dangling pointer
// without the fix.
leds_callbacks_->onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::FetchTimedout,
nullptr);

EXPECT_TRUE(initialized_);
EXPECT_EQ(0UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size());
}

class EdsCachedAssignmentTest : public testing::Test {
public:
EdsCachedAssignmentTest() { resetCluster(); }
Expand Down
Loading