rds: use http filters as part of identifier of rds config provider

envoy/router/route_config_provider_manager.h

@@ -39,12 +39,16 @@ class RouteConfigProviderManager {
    * @param stat_prefix supplies the stat_prefix to use for the provider stats.
    * @param init_manager the Init::Manager used to coordinate initialization of a the underlying RDS
    * subscription.
+   * @param identifier an optional identifier for the created provider. The identifier is used to
+   * share the provider with other HttpConnectionManagers. If the identifier refers to an existing
+   * provider, the existing provider will be returned directly. If the identifier is not set then
+   * hash of the RDS config will be used as the identifier.
    */
   virtual RouteConfigProviderSharedPtr createRdsRouteConfigProvider(
       const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds,
       const OptionalHttpFilters& optional_http_filters,
       Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix,
-      Init::Manager& init_manager) PURE;
+      Init::Manager& init_manager, absl::optional<uint64_t> identifier = {}) PURE;
 
   /**
    * Get a RouteConfigSharedPtr for a statically defined route. Ownership is as described for

source/common/config/config_provider_impl.h

@@ -418,12 +418,15 @@ class ConfigProviderManagerImplBase : public ConfigProviderManager, public Singl
   std::shared_ptr<T>
   getSubscription(const Protobuf::Message& config_source_proto, Init::Manager& init_manager,
                   const std::function<ConfigSubscriptionCommonBaseSharedPtr(
-                      const uint64_t, ConfigProviderManagerImplBase&)>& subscription_factory_fn) {
+                      const uint64_t, ConfigProviderManagerImplBase&)>& subscription_factory_fn,
+                  absl::optional<uint64_t> override_identifier = {}) {
     static_assert(std::is_base_of<ConfigSubscriptionCommonBase, T>::value,
                   "T must be a subclass of ConfigSubscriptionCommonBase");
 
     ConfigSubscriptionCommonBaseSharedPtr subscription;
-    const uint64_t manager_identifier = MessageUtil::hash(config_source_proto);
+    const uint64_t manager_identifier = override_identifier.has_value()
+                                            ? override_identifier.value()
+                                            : MessageUtil::hash(config_source_proto);
 
     auto it = config_subscriptions_.find(manager_identifier);
     if (it == config_subscriptions_.end()) {

source/common/protobuf/utility.cc

@@ -1,5 +1,6 @@
 #include "source/common/protobuf/utility.h"
 
+#include <cstdint>
 #include <limits>
 #include <numeric>
 
@@ -238,6 +239,25 @@ size_t MessageUtil::hash(const Protobuf::Message& message) {
   return HashUtil::xxHash64(text_format);
 }
 
+uint64_t MessageUtil::hash(absl::Span<const Protobuf::Message*> messages) {
+  std::string text_format;
+  Protobuf::io::StringOutputStream stream(&text_format);
+
+  {
+    Protobuf::TextFormat::Printer printer;
+    printer.SetExpandAny(true);
+    printer.SetUseFieldNumber(true);
+    printer.SetSingleLineMode(true);
+    printer.SetHideUnknownFields(true);
+
+    for (const auto message : messages) {
+      printer.Print(*message, &stream);
+    }
+  }
+
+  return HashUtil::xxHash64(text_format);
+}
+
 void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message,
                                ProtobufMessage::ValidationVisitor& validation_visitor) {
   bool has_unknown_field;

source/common/protobuf/utility.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <cstdint>
 #include <numeric>
 
 #include "envoy/api/api.h"
@@ -251,6 +252,12 @@ class MessageUtil {
    */
   static std::size_t hash(const Protobuf::Message& message);
 
+  /**
+   * A hash function uses Protobuf::TextFormat to force deterministic serialization recursively
+   * including known types in google.protobuf.Any.
+   */
+  static uint64_t hash(absl::Span<const Protobuf::Message*> messages);
+
   static void loadFromJson(const std::string& json, Protobuf::Message& message,
                            ProtobufMessage::ValidationVisitor& validation_visitor);
   /**

source/common/rds/route_config_provider_manager.cc

@@ -75,18 +75,18 @@ RouteConfigProviderPtr RouteConfigProviderManager::addStaticProvider(
 
 RouteConfigProviderSharedPtr RouteConfigProviderManager::addDynamicProvider(
     const Protobuf::Message& rds, const std::string& route_config_name, Init::Manager& init_manager,
-    std::function<
-        std::pair<RouteConfigProviderSharedPtr, const Init::Target*>(uint64_t manager_identifier)>
-        create_dynamic_provider) {
+    RouteConfigProviderCb provider_cb, absl::optional<uint64_t> identifier) {
   // RdsRouteConfigSubscriptions are unique based on their serialized RDS config.
-  const uint64_t manager_identifier = MessageUtil::hash(rds);
+  const uint64_t manager_identifier =
+      identifier.has_value() ? identifier.value() : MessageUtil::hash(rds);
+
   auto existing_provider =
       reuseDynamicProvider(manager_identifier, init_manager, route_config_name);
 
   if (existing_provider) {
     return existing_provider;
   }
-  auto new_provider = create_dynamic_provider(manager_identifier);
+  auto new_provider = provider_cb(manager_identifier);
   init_manager.add(*new_provider.second);
   dynamic_route_config_providers_.insert({manager_identifier, new_provider});
   return new_provider.first;

source/common/rds/route_config_provider_manager.h

@@ -34,12 +34,25 @@ class RouteConfigProviderManager {
 
   RouteConfigProviderPtr
   addStaticProvider(std::function<RouteConfigProviderPtr()> create_static_provider);
-  RouteConfigProviderSharedPtr
-  addDynamicProvider(const Protobuf::Message& rds, const std::string& route_config_name,
-                     Init::Manager& init_manager,
-                     std::function<std::pair<RouteConfigProviderSharedPtr, const Init::Target*>(
-                         uint64_t manager_identifier)>
-                         create_dynamic_provider);
+
+  using RouteConfigProviderCb =
+      std::function<std::pair<RouteConfigProviderSharedPtr, const Init::Target*>(uint64_t)>;
+
+  /**
+   * @param rds RDS config.
+   * @param route_config_name the name of the route config.
+   * @param init_manager the init manager to register the init target with.
+   * @param provider_cb a function that creates a RouteConfigProviderSharedPtr and Init::Target*.
+   * The RouteConfigProviderSharedPtr is the provider that will be returned by this function.
+   * The Init::Target* is the init target that will be registered with the init manager.
+   * @param identifier an optional identifier for the provider. If not provided, the hash of the
+   * RDS config will be used.
+   */
+  RouteConfigProviderSharedPtr addDynamicProvider(const Protobuf::Message& rds,
+                                                  const std::string& route_config_name,
+                                                  Init::Manager& init_manager,
+                                                  RouteConfigProviderCb provider_cb,
+                                                  absl::optional<uint64_t> identifier = {});
 
 private:
   // TODO(jsedgwick) These two members are prime candidates for the owned-entry list/map

source/common/router/rds_impl.cc

@@ -29,8 +29,13 @@ RouteConfigProviderSharedPtr RouteConfigProviderUtil::create(
     ProtobufMessage::ValidationVisitor& validator, Init::Manager& init_manager,
     const std::string& stat_prefix, RouteConfigProviderManager& route_config_provider_manager) {
   OptionalHttpFilters optional_http_filters;
+  std::vector<const Protobuf::Message*> identifier_messages;
+
   auto& filters = config.http_filters();
+  identifier_messages.reserve(filters.size() + 1);
+
   for (const auto& filter : filters) {
+    identifier_messages.push_back(&filter);
     if (filter.is_optional()) {
       optional_http_filters.insert(filter.name());
     }
@@ -42,10 +47,12 @@ RouteConfigProviderSharedPtr RouteConfigProviderUtil::create(
         config.route_config(), optional_http_filters, factory_context, validator);
   case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager::
       RouteSpecifierCase::kRds:
+    identifier_messages.push_back(&config.rds());
     return route_config_provider_manager.createRdsRouteConfigProvider(
         // At the creation of a RDS route config provider, the factory_context's initManager is
         // always valid, though the init manager may go away later when the listener goes away.
-        config.rds(), optional_http_filters, factory_context, stat_prefix, init_manager);
+        config.rds(), optional_http_filters, factory_context, stat_prefix, init_manager,
+        MessageUtil::hash({identifier_messages.data(), identifier_messages.size()}));
   case envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager::
       RouteSpecifierCase::kScopedRoutes:
     FALLTHRU; // PANIC
@@ -226,23 +233,24 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::createRdsRo
     const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds,
     const OptionalHttpFilters& optional_http_filters,
     Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix,
-    Init::Manager& init_manager) {
+    Init::Manager& init_manager, absl::optional<uint64_t> identifier) {
   auto provider = manager_.addDynamicProvider(
       rds, rds.route_config_name(), init_manager,
       [&optional_http_filters, &factory_context, &rds, &stat_prefix,
-       this](uint64_t manager_identifier) {
+       this](uint64_t provider_identifier) {
         auto config_update = std::make_unique<RouteConfigUpdateReceiverImpl>(
             proto_traits_, factory_context, optional_http_filters);
         auto resource_decoder = std::make_shared<
             Envoy::Config::OpaqueResourceDecoderImpl<envoy::config::route::v3::RouteConfiguration>>(
             factory_context.messageValidationContext().dynamicValidationVisitor(), "name");
         auto subscription = std::make_shared<RdsRouteConfigSubscription>(
-            std::move(config_update), std::move(resource_decoder), rds, manager_identifier,
+            std::move(config_update), std::move(resource_decoder), rds, provider_identifier,
             factory_context, stat_prefix, manager_);
         auto provider =
             std::make_shared<RdsRouteConfigProviderImpl>(std::move(subscription), factory_context);
         return std::make_pair(provider, &provider->subscription().initTarget());
-      });
+      },
+      identifier);
   ASSERT(dynamic_cast<RouteConfigProvider*>(provider.get()));
   return std::static_pointer_cast<RouteConfigProvider>(provider);
 }

source/common/router/rds_impl.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -191,7 +192,7 @@ class RouteConfigProviderManagerImpl : public RouteConfigProviderManager,
       const envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds,
       const OptionalHttpFilters& optional_http_filters,
       Server::Configuration::ServerFactoryContext& factory_context, const std::string& stat_prefix,
-      Init::Manager& init_manager) override;
+      Init::Manager& init_manager, absl::optional<uint64_t> identifier = {}) override;
 
   RouteConfigProviderPtr
   createStaticRouteConfigProvider(const envoy::config::route::v3::RouteConfiguration& route_config,

source/common/router/scoped_rds.cc

@@ -43,8 +43,13 @@ ConfigProviderPtr create(
          envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager::
              RouteSpecifierCase::kScopedRoutes);
   OptionalHttpFilters optional_http_filters;
+  std::vector<const Protobuf::Message*> identifier_messages;
+
   auto& filters = config.http_filters();
+  identifier_messages.reserve(filters.size() + 1);
+
   for (const auto& filter : filters) {
+    identifier_messages.push_back(&filter);
     if (filter.is_optional()) {
       optional_http_filters.insert(filter.name());
     }
@@ -60,17 +65,21 @@ ConfigProviderPtr create(
             envoy::config::route::v3::ScopedRouteConfiguration,
             ProtobufTypes::ConstMessagePtrVector>(scoped_route_list.scoped_route_configurations()),
         factory_context,
-        ScopedRoutesConfigProviderManagerOptArg(
-            config.scoped_routes().name(), config.scoped_routes().rds_config_source(),
-            config.scoped_routes().scope_key_builder(), optional_http_filters));
+        ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(),
+                                                config.scoped_routes().rds_config_source(),
+                                                config.scoped_routes().scope_key_builder(),
+                                                optional_http_filters, config.http_filters(), 0));
   }
   case envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes::
       ConfigSpecifierCase::kScopedRds:
+    identifier_messages.push_back(&config.scoped_routes().scoped_rds());
     return scoped_routes_config_provider_manager.createXdsConfigProvider(
         config.scoped_routes().scoped_rds(), factory_context, init_manager, stat_prefix,
         ScopedRoutesConfigProviderManagerOptArg(
             config.scoped_routes().name(), config.scoped_routes().rds_config_source(),
-            config.scoped_routes().scope_key_builder(), optional_http_filters));
+            config.scoped_routes().scope_key_builder(), optional_http_filters,
+            config.http_filters(),
+            MessageUtil::hash({identifier_messages.data(), identifier_messages.size()})));
   case envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes::
       ConfigSpecifierCase::CONFIG_SPECIFIER_NOT_SET:
     PANIC("not implemented");
@@ -131,6 +140,8 @@ InlineScopedRoutesConfigProvider::InlineScopedRoutesConfigProvider(
 
 ScopedRdsConfigSubscription::ScopedRdsConfigSubscription(
     const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRds& scoped_rds,
+    const Protobuf::RepeatedPtrField<
+        envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter>& http_filters,
     const OptionalHttpFilters& optional_http_filters, const uint64_t manager_identifier,
     const std::string& name,
     const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes::
@@ -148,7 +159,7 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription(
       stats_({ALL_SCOPED_RDS_STATS(POOL_COUNTER(*scope_), POOL_GAUGE(*scope_))}),
       scope_key_builder_(scope_key_builder), rds_config_source_(std::move(rds_config_source)),
       stat_prefix_(stat_prefix), route_config_provider_manager_(route_config_provider_manager),
-      optional_http_filters_(optional_http_filters) {
+      optional_http_filters_(optional_http_filters), http_filters_(http_filters) {
   const auto resource_name = getResourceName();
   if (scoped_rds.srds_resources_locator().empty()) {
     subscription_ =
@@ -224,10 +235,19 @@ void ScopedRdsConfigSubscription::RdsRouteConfigProviderHelper::runOnDemandUpdat
 void ScopedRdsConfigSubscription::RdsRouteConfigProviderHelper::initRdsConfigProvider(
     envoy::extensions::filters::network::http_connection_manager::v3::Rds& rds,
     Init::Manager& init_manager) {
+
+  std::vector<const Protobuf::Message*> identifier_messages;
+  identifier_messages.reserve(parent_.http_filters_.size() + 1);
+  for (const auto& http_filter : parent_.http_filters_) {
+    identifier_messages.push_back(&http_filter);
+  }
+  identifier_messages.push_back(&rds);
+
   route_provider_ = std::dynamic_pointer_cast<RdsRouteConfigProviderImpl>(
       parent_.route_config_provider_manager_.createRdsRouteConfigProvider(
           rds, parent_.optional_http_filters_, parent_.factory_context_, parent_.stat_prefix_,
-          init_manager));
+          init_manager,
+          MessageUtil::hash({identifier_messages.data(), identifier_messages.size()})));
 
   rds_update_callback_handle_ = route_provider_->subscription().addUpdateCallback([this]() {
     // Subscribe to RDS update.
@@ -616,13 +636,15 @@ ConfigProviderPtr ScopedRoutesConfigProviderManager::createXdsConfigProvider(
                 const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRds&>(
                 config_source_proto);
             return std::make_shared<ScopedRdsConfigSubscription>(
-                scoped_rds_config_source, typed_optarg.optional_http_filters_, manager_identifier,
+                scoped_rds_config_source, typed_optarg.http_filters_,
+                typed_optarg.optional_http_filters_, manager_identifier,
                 typed_optarg.scoped_routes_name_, typed_optarg.scope_key_builder_, factory_context,
                 stat_prefix, typed_optarg.rds_config_source_,
                 static_cast<ScopedRoutesConfigProviderManager&>(config_provider_manager)
                     .routeConfigProviderManager(),
                 static_cast<ScopedRoutesConfigProviderManager&>(config_provider_manager));
-          });
+          },
+          typed_optarg.identifier_);
 
   return std::make_unique<ScopedRdsConfigProvider>(std::move(subscription));
 }

source/common/router/scoped_rds.h

@@ -112,6 +112,9 @@ class ScopedRdsConfigSubscription
 
   ScopedRdsConfigSubscription(
       const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRds& scoped_rds,
+      const Protobuf::RepeatedPtrField<
+          envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter>&
+          http_filters,
       const OptionalHttpFilters& optional_http_filters, const uint64_t manager_identifier,
       const std::string& name,
       const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes::
@@ -241,6 +244,9 @@ class ScopedRdsConfigSubscription
   // A map of (hash, scope-name), used to detect the key conflict between scopes.
   absl::flat_hash_map<uint64_t, std::string> scope_name_by_hash_;
   const OptionalHttpFilters optional_http_filters_;
+  const Protobuf::RepeatedPtrField<
+      envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter>
+      http_filters_;
 };
 
 using ScopedRdsConfigSubscriptionSharedPtr = std::shared_ptr<ScopedRdsConfigSubscription>;
@@ -316,15 +322,23 @@ class ScopedRoutesConfigProviderManagerOptArg
       const envoy::config::core::v3::ConfigSource& rds_config_source,
       const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes::
           ScopeKeyBuilder& scope_key_builder,
-      const OptionalHttpFilters& optional_http_filters)
+      const OptionalHttpFilters& optional_http_filters,
+      const Protobuf::RepeatedPtrField<
+          envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter>&
+          http_filters,
+      uint64_t identifier)
       : scoped_routes_name_(std::move(scoped_routes_name)), rds_config_source_(rds_config_source),
-        scope_key_builder_(scope_key_builder), optional_http_filters_(optional_http_filters) {}
+        scope_key_builder_(scope_key_builder), optional_http_filters_(optional_http_filters),
+        http_filters_(http_filters), identifier_(identifier) {}
 
   const std::string scoped_routes_name_;
   const envoy::config::core::v3::ConfigSource& rds_config_source_;
   const envoy::extensions::filters::network::http_connection_manager::v3::ScopedRoutes::
       ScopeKeyBuilder& scope_key_builder_;
   const OptionalHttpFilters& optional_http_filters_;
+  const Protobuf::RepeatedPtrField<
+      envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter>& http_filters_;
+  const uint64_t identifier_{};
 };
 
 } // namespace Router