From ad843aa02079f35ab05149bfd7c123a64911bc52 Mon Sep 17 00:00:00 2001 From: Rei Shimizu Date: Thu, 19 Mar 2020 07:21:53 +0900 Subject: [PATCH] xDS: type url aggregation (#9649) This is only refactoring. Aggregated xDS type urls. In previous implementation, loadTypeUrl is scattered on all of xDS subscription classes. That is poor outlook so refactored all of them. Risk Level: Low Testing: None Relates to issues 9468 #9526 Signed-off-by: shikugawa --- include/envoy/config/BUILD | 10 +++++++ include/envoy/config/discovery_service_base.h | 28 +++++++++++++++++++ include/envoy/upstream/BUILD | 1 + source/common/config/api_type_oracle.cc | 24 ++++++++++------ source/common/config/api_type_oracle.h | 5 ++++ source/common/router/BUILD | 3 ++ source/common/router/rds_impl.cc | 21 ++------------ source/common/router/rds_impl.h | 6 ++-- source/common/router/scoped_rds.cc | 22 +++------------ source/common/router/scoped_rds.h | 7 +++-- source/common/router/vhds.cc | 19 ++----------- source/common/router/vhds.h | 4 +-- source/common/runtime/BUILD | 1 + source/common/runtime/runtime_impl.cc | 19 ++----------- source/common/runtime/runtime_impl.h | 5 ++-- source/common/secret/BUILD | 1 + source/common/secret/sds_api.cc | 18 ++---------- source/common/secret/sds_api.h | 5 ++-- source/common/upstream/BUILD | 2 ++ source/common/upstream/cds_api_impl.cc | 18 ++---------- source/common/upstream/cds_api_impl.h | 4 +-- source/common/upstream/eds.cc | 20 ++----------- source/common/upstream/eds.h | 6 ++-- source/server/BUILD | 1 + source/server/lds_api.cc | 17 ++--------- source/server/lds_api.h | 4 +-- test/common/config/api_type_oracle_test.cc | 3 ++ 27 files changed, 113 insertions(+), 161 deletions(-) create mode 100644 include/envoy/config/discovery_service_base.h diff --git a/include/envoy/config/BUILD b/include/envoy/config/BUILD index 2b47956887a0..cd909d922ddb 100644 --- a/include/envoy/config/BUILD +++ b/include/envoy/config/BUILD @@ -75,3 +75,13 @@ envoy_cc_library( "//source/common/protobuf", ], ) + +envoy_cc_library( + name = "discovery_service_base_interface", + hdrs = ["discovery_service_base.h"], + deps = [ + ":subscription_interface", + "//source/common/config:api_type_oracle_lib", + "//source/common/protobuf", + ], +) diff --git a/include/envoy/config/discovery_service_base.h b/include/envoy/config/discovery_service_base.h new file mode 100644 index 000000000000..52ae80e127c6 --- /dev/null +++ b/include/envoy/config/discovery_service_base.h @@ -0,0 +1,28 @@ +#pragma once + +#include + +#include "envoy/config/discovery_service_base.h" +#include "envoy/config/subscription.h" + +#include "common/config/api_type_oracle.h" + +namespace Envoy { +namespace Config { +template struct SubscriptionBase : public Config::SubscriptionCallbacks { + static std::string getResourceName(envoy::config::core::v3::ApiVersion resource_api_version) { + switch (resource_api_version) { + case envoy::config::core::v3::ApiVersion::AUTO: + case envoy::config::core::v3::ApiVersion::V2: + return ApiTypeOracle::getEarlierVersionMessageTypeName(Current().GetDescriptor()->full_name()) + .value(); + case envoy::config::core::v3::ApiVersion::V3: + return Current().GetDescriptor()->full_name(); + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } + } +}; + +} // namespace Config +} // namespace Envoy \ No newline at end of file diff --git a/include/envoy/upstream/BUILD b/include/envoy/upstream/BUILD index 6bf20f77a8e2..2ecf8e601b7c 100644 --- a/include/envoy/upstream/BUILD +++ b/include/envoy/upstream/BUILD @@ -143,6 +143,7 @@ envoy_cc_library( ":locality_lib", ":resource_manager_interface", "//include/envoy/common:callback", + "//include/envoy/config:subscription_interface", "//include/envoy/config:typed_metadata_interface", "//include/envoy/http:codec_interface", "//include/envoy/network:connection_interface", diff --git a/source/common/config/api_type_oracle.cc b/source/common/config/api_type_oracle.cc index 8e89254b7cb8..161ecf058610 100644 --- a/source/common/config/api_type_oracle.cc +++ b/source/common/config/api_type_oracle.cc @@ -7,21 +7,29 @@ namespace Config { const Protobuf::Descriptor* ApiTypeOracle::getEarlierVersionDescriptor(const std::string& message_type) { + const auto previous_message_string = getEarlierVersionMessageTypeName(message_type); + if (previous_message_string != absl::nullopt) { + const Protobuf::Descriptor* earlier_desc = + Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName( + previous_message_string.value()); + return earlier_desc; + } else { + return nullptr; + } +} + +const absl::optional +ApiTypeOracle::getEarlierVersionMessageTypeName(const std::string& message_type) { // Determine if there is an earlier API version for message_type. const Protobuf::Descriptor* desc = Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(std::string{message_type}); if (desc == nullptr) { - return nullptr; + return absl::nullopt; } if (desc->options().HasExtension(udpa::annotations::versioning)) { - const Protobuf::Descriptor* earlier_desc = - Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName( - desc->options().GetExtension(udpa::annotations::versioning).previous_message_type()); - return earlier_desc; + return desc->options().GetExtension(udpa::annotations::versioning).previous_message_type(); } - - return nullptr; + return absl::nullopt; } - } // namespace Config } // namespace Envoy diff --git a/source/common/config/api_type_oracle.h b/source/common/config/api_type_oracle.h index 8c57b5c1243f..cd0c5971ee52 100644 --- a/source/common/config/api_type_oracle.h +++ b/source/common/config/api_type_oracle.h @@ -2,6 +2,8 @@ #include "common/protobuf/protobuf.h" +#include "absl/types/optional.h" + namespace Envoy { namespace Config { @@ -17,6 +19,9 @@ class ApiTypeOracle { * corresponding to message, if any, otherwise nullptr. */ static const Protobuf::Descriptor* getEarlierVersionDescriptor(const std::string& message_type); + + static const absl::optional + getEarlierVersionMessageTypeName(const std::string& message_type); }; } // namespace Config diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 2c1ca31ddfaa..f237a458d157 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -123,6 +123,7 @@ envoy_cc_library( hdrs = ["vhds.h"], deps = [ ":config_lib", + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:subscription_interface", "//include/envoy/http:codes_interface", "//include/envoy/local_info:local_info_interface", @@ -151,6 +152,7 @@ envoy_cc_library( hdrs = ["rds_impl.h"], deps = [ ":config_lib", + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:subscription_interface", "//include/envoy/http:codes_interface", "//include/envoy/local_info:local_info_interface", @@ -208,6 +210,7 @@ envoy_cc_library( ":rds_lib", ":scoped_config_lib", "//include/envoy/config:config_provider_interface", + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:subscription_interface", "//include/envoy/router:route_config_provider_manager_interface", "//include/envoy/stats:stats_interface", diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 11fdc8c4cbe5..a52fc43a25eb 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -82,11 +82,10 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( stat_prefix_(stat_prefix), stats_({ALL_RDS_STATS(POOL_COUNTER(*scope_))}), route_config_provider_manager_(route_config_provider_manager), manager_identifier_(manager_identifier) { - + const auto resource_name = getResourceName(rds.config_source().resource_api_version()); subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - rds.config_source(), loadTypeUrl(rds.config_source().resource_api_version()), *scope_, - *this); + rds.config_source(), Grpc::Common::typeUrl(resource_name), *scope_, *this); local_init_manager_.add(local_init_target_); config_update_info_ = std::make_unique(factory_context.timeSource(), validator_); @@ -223,22 +222,6 @@ bool RdsRouteConfigSubscription::validateUpdateSize(int num_resources) { return true; } -std::string -RdsRouteConfigSubscription::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::config::route::v3::RouteConfiguration().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} - RdsRouteConfigProviderImpl::RdsRouteConfigProviderImpl( RdsRouteConfigSubscriptionSharedPtr&& subscription, Server::Configuration::ServerFactoryContext& factory_context) diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index f5121a4a8c41..09d73ab38048 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -111,8 +111,9 @@ class RdsRouteConfigProviderImpl; * A class that fetches the route configuration dynamically using the RDS API and updates them to * RDS config providers. */ -class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, - Logger::Loggable { +class RdsRouteConfigSubscription + : Envoy::Config::SubscriptionBase, + Logger::Loggable { public: ~RdsRouteConfigSubscription() override; @@ -151,7 +152,6 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, RouteConfigProviderManagerImpl& route_config_provider_manager); bool validateUpdateSize(int num_resources); - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); std::unique_ptr subscription_; const std::string route_config_name_; diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 08df35675ca2..a75e81e08fd0 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -19,6 +19,7 @@ #include "common/config/version_converter.h" #include "common/init/manager_impl.h" #include "common/init/watcher_impl.h" +#include "common/router/rds_impl.h" #include "absl/strings/str_join.h" @@ -104,10 +105,11 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( rds_config_source_(std::move(rds_config_source)), validation_visitor_(factory_context.messageValidationContext().dynamicValidationVisitor()), stat_prefix_(stat_prefix), route_config_provider_manager_(route_config_provider_manager) { + const auto resource_name = getResourceName(rds_config_source_.resource_api_version()); subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - scoped_rds.scoped_rds_config_source(), - loadTypeUrl(rds_config_source_.resource_api_version()), *scope_, *this); + scoped_rds.scoped_rds_config_source(), Grpc::Common::typeUrl(resource_name), *scope_, + *this); initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr { return std::make_shared( @@ -345,22 +347,6 @@ void ScopedRdsConfigSubscription::onConfigUpdate( onConfigUpdate(to_add_repeated, to_remove_repeated, version_info); } -std::string -ScopedRdsConfigSubscription::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl(API_NO_BOOST( - envoy::config::route::v3::ScopedRouteConfiguration().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} - ScopedRdsConfigProvider::ScopedRdsConfigProvider( ScopedRdsConfigSubscriptionSharedPtr&& subscription) : MutableConfigProviderCommonBase(std::move(subscription), ConfigProvider::ApiType::Delta) {} diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index 372caef576cf..9b57aad1b213 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -4,6 +4,7 @@ #include "envoy/common/callback.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/route/v3/scoped_route.pb.h" #include "envoy/config/subscription.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" @@ -85,8 +86,9 @@ struct ScopedRdsStats { }; // A scoped RDS subscription to be used with the dynamic scoped RDS ConfigProvider. -class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptionInstance, - Envoy::Config::SubscriptionCallbacks { +class ScopedRdsConfigSubscription + : public Envoy::Config::DeltaConfigSubscriptionInstance, + Envoy::Config::SubscriptionBase { public: using ScopedRouteConfigurationMap = std::map; @@ -164,7 +166,6 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio return MessageUtil::anyConvert(resource) .name(); } - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); // Propagate RDS updates to ScopeConfigImpl in workers. void onRdsConfigUpdate(const std::string& scope_name, RdsRouteConfigSubscription& rds_subscription); diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index c83fa87ea5b2..a59f0dcccb51 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -40,11 +40,11 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, if (config_source != envoy::config::core::v3::ApiConfigSource::DELTA_GRPC) { throw EnvoyException("vhds: only 'DELTA_GRPC' is supported as an api_type."); } - + const auto resource_name = getResourceName(resource_api_version); subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( config_update_info_->routeConfiguration().vhds().config_source(), - loadTypeUrl(resource_api_version), *scope_, *this); + Grpc::Common::typeUrl(resource_name), *scope_, *this); } void VhdsSubscription::updateOnDemand(const std::string& with_route_config_name_prefix) { @@ -75,20 +75,5 @@ void VhdsSubscription::onConfigUpdate( init_target_.ready(); } -std::string -VhdsSubscription::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} } // namespace Router } // namespace Envoy diff --git a/source/common/router/vhds.h b/source/common/router/vhds.h index a77ac53e0809..fad39432700a 100644 --- a/source/common/router/vhds.h +++ b/source/common/router/vhds.h @@ -7,6 +7,7 @@ #include #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/route/v3/route_components.pb.h" #include "envoy/config/subscription.h" #include "envoy/http/codes.h" @@ -34,7 +35,7 @@ struct VhdsStats { ALL_VHDS_STATS(GENERATE_COUNTER_STRUCT) }; -class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, +class VhdsSubscription : Envoy::Config::SubscriptionBase, Logger::Loggable { public: VhdsSubscription(RouteConfigUpdatePtr& config_update_info, @@ -69,7 +70,6 @@ class VhdsSubscription : Envoy::Config::SubscriptionCallbacks, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); RouteConfigUpdatePtr& config_update_info_; Stats::ScopePtr scope_; diff --git a/source/common/runtime/BUILD b/source/common/runtime/BUILD index a5ac6bcaef76..ffeae6eb64d8 100644 --- a/source/common/runtime/BUILD +++ b/source/common/runtime/BUILD @@ -34,6 +34,7 @@ envoy_cc_library( external_deps = ["ssl"], deps = [ ":runtime_features_lib", + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/init:manager_interface", diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index a47069518104..0589defa6e90 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -583,8 +583,9 @@ void RtdsSubscription::start() { // We have to delay the subscription creation until init-time, since the // cluster manager resources are not available in the constructor when // instantiated in the server instance. + const auto resource_name = getResourceName(config_source_.resource_api_version()); subscription_ = parent_.cm_->subscriptionFactory().subscriptionFromConfigSource( - config_source_, loadTypeUrl(config_source_.resource_api_version()), store_, *this); + config_source_, Grpc::Common::typeUrl(resource_name), store_, *this); subscription_->start({resource_name_}); } @@ -596,22 +597,6 @@ void RtdsSubscription::validateUpdateSize(uint32_t num_resources) { } } -std::string -RtdsSubscription::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::service::discovery::v2::Runtime().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::service::runtime::v3::Runtime().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} - void LoaderImpl::loadNewSnapshot() { std::shared_ptr ptr = createNewSnapshot(); tls_->set([ptr](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index 56310be1eb6b..cc10dd5aa759 100644 --- a/source/common/runtime/runtime_impl.h +++ b/source/common/runtime/runtime_impl.h @@ -9,6 +9,7 @@ #include "envoy/common/exception.h" #include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/subscription.h" #include "envoy/init/manager.h" #include "envoy/runtime/runtime.h" @@ -199,7 +200,8 @@ class ProtoLayer : public OverrideLayerImpl, Logger::Loggable { +struct RtdsSubscription : Envoy::Config::SubscriptionBase, + Logger::Loggable { RtdsSubscription(LoaderImpl& parent, const envoy::config::bootstrap::v3::RuntimeLayer::RtdsLayer& rtds_layer, Stats::Store& store, ProtobufMessage::ValidationVisitor& validation_visitor); @@ -220,7 +222,6 @@ struct RtdsSubscription : Config::SubscriptionCallbacks, Logger::Loggablestart({sds_config_name_}); } -std::string SdsApi::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::auth::Secret().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl(API_NO_BOOST( - envoy::extensions::transport_sockets::tls::v3::Secret().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} - SdsApi::SecretData SdsApi::secretData() { return secret_data_; } } // namespace Secret diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index 94474113520a..e1648c8f7f91 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -4,6 +4,7 @@ #include "envoy/api/api.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/subscription.h" #include "envoy/config/subscription_factory.h" #include "envoy/event/dispatcher.h" @@ -31,7 +32,8 @@ namespace Secret { /** * SDS API implementation that fetches secrets from SDS server via Subscription. */ -class SdsApi : public Config::SubscriptionCallbacks { +class SdsApi : public Envoy::Config::SubscriptionBase< + envoy::extensions::transport_sockets::tls::v3::Secret> { public: struct SecretData { const std::string resource_name_; @@ -63,7 +65,6 @@ class SdsApi : public Config::SubscriptionCallbacks { return MessageUtil::anyConvert(resource) .name(); } - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); private: void validateUpdateSize(int num_resources); diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index f4156b536da3..f4af28441a57 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -13,6 +13,7 @@ envoy_cc_library( srcs = ["cds_api_impl.cc"], hdrs = ["cds_api_impl.h"], deps = [ + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", "//include/envoy/local_info:local_info_interface", @@ -344,6 +345,7 @@ envoy_cc_library( deps = [ ":cluster_factory_lib", ":upstream_includes", + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:grpc_mux_interface", "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index e68a2c6f33b4..d7cf1534f6b5 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -32,8 +32,9 @@ CdsApiImpl::CdsApiImpl(const envoy::config::core::v3::ConfigSource& cds_config, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor) : cm_(cm), scope_(scope.createScope("cluster_manager.cds.")), validation_visitor_(validation_visitor) { + const auto resource_name = getResourceName(cds_config.resource_api_version()); subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource( - cds_config, loadTypeUrl(cds_config.resource_api_version()), *scope_, *this); + cds_config, Grpc::Common::typeUrl(resource_name), *scope_, *this); } void CdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, @@ -127,20 +128,5 @@ void CdsApiImpl::runInitializeCallbackIfAny() { } } -std::string CdsApiImpl::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::Cluster().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::config::cluster::v3::Cluster().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} - } // namespace Upstream } // namespace Envoy diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 6653fffeb1a4..5eca7076befe 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -5,6 +5,7 @@ #include "envoy/api/api.h" #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/subscription.h" #include "envoy/event/dispatcher.h" #include "envoy/local_info/local_info.h" @@ -21,7 +22,7 @@ namespace Upstream { * CDS API implementation that fetches via Subscription. */ class CdsApiImpl : public CdsApi, - Config::SubscriptionCallbacks, + Envoy::Config::SubscriptionBase, Logger::Loggable { public: static CdsApiPtr create(const envoy::config::core::v3::ConfigSource& cds_config, @@ -48,7 +49,6 @@ class CdsApiImpl : public CdsApi, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); CdsApiImpl(const envoy::config::core::v3::ConfigSource& cds_config, ClusterManager& cm, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor); void runInitializeCallbackIfAny(); diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index dfe6ac4d059d..81e2a8deb11f 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -36,10 +36,11 @@ EdsClusterImpl::EdsClusterImpl( } else { initialize_phase_ = InitializePhase::Secondary; } + const auto resource_name = + getResourceName(cluster.eds_cluster_config().eds_config().resource_api_version()); subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( - eds_config, loadTypeUrl(cluster.eds_cluster_config().eds_config().resource_api_version()), - info_->statsScope(), *this); + eds_config, Grpc::Common::typeUrl(resource_name), info_->statsScope(), *this); } void EdsClusterImpl::startPreInit() { subscription_->start({cluster_name_}); } @@ -227,21 +228,6 @@ void EdsClusterImpl::reloadHealthyHostsHelper(const HostSharedPtr& host) { } } -std::string EdsClusterImpl::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::ClusterLoadAssignment().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl(API_NO_BOOST( - envoy::config::endpoint::v3::ClusterLoadAssignment().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} - bool EdsClusterImpl::updateHostsPerLocality( const uint32_t priority, const uint32_t overprovisioning_factor, const HostVector& new_hosts, LocalityWeightsMap& locality_weights_map, LocalityWeightsMap& new_locality_weights_map, diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 79470a5259db..ebe8dc375523 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -3,6 +3,7 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" #include "envoy/config/subscription.h" #include "envoy/config/subscription_factory.h" @@ -23,7 +24,9 @@ namespace Upstream { /** * Cluster implementation that reads host information from the Endpoint Discovery Service. */ -class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallbacks { +class EdsClusterImpl + : public BaseDynamicClusterImpl, + Envoy::Config::SubscriptionBase { public: EdsClusterImpl(const envoy::config::cluster::v3::Cluster& cluster, Runtime::Loader& runtime, Server::Configuration::TransportSocketFactoryContextImpl& factory_context, @@ -44,7 +47,6 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba return MessageUtil::anyConvert(resource) .cluster_name(); } - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); using LocalityWeightsMap = std::unordered_map; bool updateHostsPerLocality(const uint32_t priority, const uint32_t overprovisioning_factor, diff --git a/source/server/BUILD b/source/server/BUILD index e1ac3844964e..635b3c80e4f8 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -254,6 +254,7 @@ envoy_cc_library( srcs = ["lds_api.cc"], hdrs = ["lds_api.h"], deps = [ + "//include/envoy/config:discovery_service_base_interface", "//include/envoy/config:subscription_factory_interface", "//include/envoy/config:subscription_interface", "//include/envoy/init:manager_interface", diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 65f112495fb9..b799140835be 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -29,8 +29,9 @@ LdsApiImpl::LdsApiImpl(const envoy::config::core::v3::ConfigSource& lds_config, : listener_manager_(lm), scope_(scope.createScope("listener_manager.lds.")), cm_(cm), init_target_("LDS", [this]() { subscription_->start({}); }), validation_visitor_(validation_visitor) { + const auto resource_name = getResourceName(lds_config.resource_api_version()); subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource( - lds_config, loadTypeUrl(lds_config.resource_api_version()), *scope_, *this); + lds_config, Grpc::Common::typeUrl(resource_name), *scope_, *this); init_manager.add(init_target_); } @@ -133,19 +134,5 @@ void LdsApiImpl::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason r init_target_.ready(); } -std::string LdsApiImpl::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { - switch (resource_api_version) { - // automatically set api version as V2 - case envoy::config::core::v3::ApiVersion::AUTO: - case envoy::config::core::v3::ApiVersion::V2: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::api::v2::Listener().GetDescriptor()->full_name())); - case envoy::config::core::v3::ApiVersion::V3: - return Grpc::Common::typeUrl( - API_NO_BOOST(envoy::config::listener::v3::Listener().GetDescriptor()->full_name())); - default: - NOT_REACHED_GCOVR_EXCL_LINE; - } -} } // namespace Server } // namespace Envoy diff --git a/source/server/lds_api.h b/source/server/lds_api.h index 7bc446836a61..5a1f64d916fd 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -3,6 +3,7 @@ #include #include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/discovery_service_base.h" #include "envoy/config/listener/v3/listener.pb.h" #include "envoy/config/subscription.h" #include "envoy/config/subscription_factory.h" @@ -21,7 +22,7 @@ namespace Server { * LDS API implementation that fetches via Subscription. */ class LdsApiImpl : public LdsApi, - Config::SubscriptionCallbacks, + Envoy::Config::SubscriptionBase, Logger::Loggable { public: LdsApiImpl(const envoy::config::core::v3::ConfigSource& lds_config, Upstream::ClusterManager& cm, @@ -44,7 +45,6 @@ class LdsApiImpl : public LdsApi, std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); } - static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); std::unique_ptr subscription_; std::string system_version_info_; diff --git a/test/common/config/api_type_oracle_test.cc b/test/common/config/api_type_oracle_test.cc index ef333bff0341..327d4dc32e54 100644 --- a/test/common/config/api_type_oracle_test.cc +++ b/test/common/config/api_type_oracle_test.cc @@ -24,6 +24,9 @@ TEST(ApiTypeOracleTest, All) { ApiTypeOracle::getEarlierVersionDescriptor(v3_config.GetDescriptor()->full_name()); EXPECT_EQ(envoy::config::filter::http::ip_tagging::v2::IPTagging::descriptor()->full_name(), desc->full_name()); + EXPECT_EQ(envoy::config::filter::http::ip_tagging::v2::IPTagging::descriptor()->full_name(), + ApiTypeOracle::getEarlierVersionMessageTypeName(v3_config.GetDescriptor()->full_name()) + .value()); } } // namespace