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 <rei@tetrate.io>
envoyproxy · Mar 18, 2020 · ad843aa · ad843aa
1 parent 9d500b0
commit ad843aa
Show file tree

Hide file tree

Showing 27 changed files with 113 additions and 161 deletions.
diff --git 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
@@ -0,0 +1,28 @@
+#pragma once
+
+#include <string>
+
+#include "envoy/config/discovery_service_base.h"
+#include "envoy/config/subscription.h"
+
+#include "common/config/api_type_oracle.h"
+
+namespace Envoy {
+namespace Config {
+template <typename Current> 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
diff --git 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
@@ -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<std::string>
+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
@@ -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<std::string>
+  getEarlierVersionMessageTypeName(const std::string& message_type);
 };
 
 } // namespace Config

diff --git 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
@@ -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<RouteConfigUpdateReceiverImpl>(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
@@ -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<Logger::Id::router> {
+class RdsRouteConfigSubscription
+    : Envoy::Config::SubscriptionBase<envoy::config::route::v3::RouteConfiguration>,
+      Logger::Loggable<Logger::Id::router> {
 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<Envoy::Config::Subscription> subscription_;
   const std::string route_config_name_;

diff --git 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<ScopedConfigImpl>(
@@ -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
@@ -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<envoy::config::route::v3::ScopedRouteConfiguration> {
 public:
   using ScopedRouteConfigurationMap =
       std::map<std::string, envoy::config::route::v3::ScopedRouteConfiguration>;
@@ -164,7 +166,6 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio
     return MessageUtil::anyConvert<envoy::config::route::v3::ScopedRouteConfiguration>(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
@@ -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
@@ -7,6 +7,7 @@
 #include <unordered_set>
 
 #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<envoy::config::route::v3::VirtualHost>,
                          Logger::Loggable<Logger::Id::router> {
 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<envoy::config::route::v3::VirtualHost>(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
@@ -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
@@ -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<SnapshotImpl> 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
@@ -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<Logger::Id::runtim
 
 class LoaderImpl;
 
-struct RtdsSubscription : Config::SubscriptionCallbacks, Logger::Loggable<Logger::Id::runtime> {
+struct RtdsSubscription : Envoy::Config::SubscriptionBase<envoy::service::runtime::v3::Runtime>,
+                          Logger::Loggable<Logger::Id::runtime> {
   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::Loggable<Logger
 
   void start();
   void validateUpdateSize(uint32_t num_resources);
-  static std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version);
 
   LoaderImpl& parent_;
   const envoy::config::core::v3::ConfigSource config_source_;

diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD
@@ -44,6 +44,7 @@ envoy_cc_library(
     srcs = ["sds_api.cc"],
     hdrs = ["sds_api.h"],
     deps = [
+        "//include/envoy/config:discovery_service_base_interface",
         "//include/envoy/config:subscription_factory_interface",
         "//include/envoy/config:subscription_interface",
         "//include/envoy/event:dispatcher_interface",