xds: use filter type URL as the primary way to discover extensions

include/envoy/registry/BUILD

@@ -13,6 +13,7 @@ envoy_cc_library(
     hdrs = ["registry.h"],
     deps = [
         "//source/common/common:assert_lib",
+        "//source/common/config:api_type_oracle_lib",
         "//source/common/protobuf:utility_lib",
         "@envoy_api//envoy/config/core/v3alpha:pkg_cc_proto",
     ],

include/envoy/registry/registry.h

@@ -12,6 +12,7 @@
 #include "common/common/assert.h"
 #include "common/common/fmt.h"
 #include "common/common/logger.h"
+#include "common/config/api_type_oracle.h"
 #include "common/protobuf/utility.h"
 
 #include "absl/base/attributes.h"
@@ -188,6 +189,50 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
     return *deprecated_factory_names;
   }
 
+  /**
+   * Lazily constructs a mapping from the configuration message type to a factory,
+   * including the deprecated configuration message types.
+   * Must be invoked after factory registration is completed.
+   */
+  static absl::flat_hash_map<std::string, Base*>& factoriesByType() {
+    static absl::flat_hash_map<std::string, Base*>* factoriesByType = [] {
+      auto* mapping = new absl::flat_hash_map<std::string, Base*>();
+
+      for (const auto& factory : factories()) {
+        if (factory.second == nullptr) {
+          continue;
+        }
+
+        // skip untyped factories and factories that consume Struct
+        std::string config_type = factory.second->configType();
+        if (config_type.empty() || config_type == "google.protobuf.Struct") {
+          continue;
+        }
+
+        // traverse the deprecated message type chain
+        while (true) {
+          auto it = mapping->find(config_type);
+          if (it != mapping->end() && it->second != factory.second) {
+            throw EnvoyException(fmt::format("Double registration for type: '{}' by '{}' and '{}'",
+                                             config_type, factory.second->name(),
+                                             it->second->name()));
+          }
+          mapping->emplace(std::make_pair(config_type, factory.second));
+
+          const Protobuf::Descriptor* previous =
+              Config::ApiTypeOracle::getEarlierVersionDescriptor(config_type);
+          if (previous == nullptr) {
+            break;
+          }
+          config_type = previous->full_name();
+        }
+      }
+      return mapping;
+    }();
+
+    return *factoriesByType;
+  }
+
   /**
    * instead_value are used when passed name was deprecated.
    */
@@ -262,6 +307,14 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
     return it->second;
   }
 
+  static Base* getFactoryByType(absl::string_view type) {
+    auto it = factoriesByType().find(type);
+    if (it == factoriesByType().end()) {
+      return nullptr;
+    }
+    return it->second;
+  }
+
   /**
    * @return the canonical name of the factory. If the given name is a
    * deprecated factory name, the canonical name is returned instead.
@@ -319,6 +372,7 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
 
     factories().emplace(factory.name(), &factory);
     RELEASE_ASSERT(getFactory(factory.name()) == &factory, "");
+    factoriesByType().emplace(factory.configType(), &factory);
 
     return displaced;
   }
@@ -327,9 +381,10 @@ template <class Base> class FactoryRegistry : public Logger::Loggable<Logger::Id
    * Remove a factory by name. This method should only be used for testing purposes.
    * @param name is the name of the factory to remove.
    */
-  static void removeFactoryForTest(absl::string_view name) {
+  static void removeFactoryForTest(absl::string_view name, absl::string_view config_type) {
     auto result = factories().erase(name);
     RELEASE_ASSERT(result == 1, "");
+    factoriesByType().erase(config_type);
   }
 };
 

source/common/config/api_type_oracle.cc

@@ -9,12 +9,10 @@ namespace Envoy {
 namespace Config {
 
 const Protobuf::Descriptor*
-ApiTypeOracle::getEarlierVersionDescriptor(const Protobuf::Message& message) {
-  const std::string target_type = message.GetDescriptor()->full_name();
-
-  // Determine if there is an earlier API version for target_type.
+ApiTypeOracle::getEarlierVersionDescriptor(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{target_type});
+      Protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(std::string{message_type});
   if (desc == nullptr) {
     return nullptr;
   }

source/common/config/api_type_oracle.h

@@ -12,11 +12,11 @@ class ApiTypeOracle {
    * this message. If so, return the descriptor for the earlier
    * message, to support upgrading via VersionConverter::upgrade().
    *
-   * @param message protobuf message.
+   * @param message_type protobuf message type
    * @return const Protobuf::Descriptor* descriptor for earlier message version
    *         corresponding to message, if any, otherwise nullptr.
    */
-  static const Protobuf::Descriptor* getEarlierVersionDescriptor(const Protobuf::Message& message);
+  static const Protobuf::Descriptor* getEarlierVersionDescriptor(const std::string& message_type);
 };
 
 } // namespace Config

source/common/config/utility.cc

@@ -24,8 +24,6 @@
 #include "common/stats/stats_matcher_impl.h"
 #include "common/stats/tag_producer_impl.h"
 
-#include "udpa/type/v1/typed_struct.pb.h"
-
 namespace Envoy {
 namespace Config {
 

source/common/config/utility.h

@@ -26,6 +26,8 @@
 #include "common/protobuf/utility.h"
 #include "common/singleton/const_singleton.h"
 
+#include "udpa/type/v1/typed_struct.pb.h"
+
 namespace Envoy {
 namespace Config {
 
@@ -209,13 +211,50 @@ class Utility {
     return *factory;
   }
 
+  template <class Factory> static Factory& getAndCheckFactoryByType(absl::string_view type) {
+    if (type.empty()) {
+      throw EnvoyException("Provided type for static registration lookup was empty.");
+    }
+
+    Factory* factory = Registry::FactoryRegistry<Factory>::getFactoryByType(type);
+
+    if (factory == nullptr) {
+      throw EnvoyException(
+          fmt::format("Didn't find a registered implementation for type: '{}'", type));
+    }
+
+    return *factory;
+  }
+
   /**
    * Get a Factory from the registry with error checking to ensure the name and the factory are
    * valid.
    * @param message proto that contains fields 'name' and 'typed_config'.
    */
   template <class Factory, class ProtoMessage>
   static Factory& getAndCheckFactory(const ProtoMessage& message) {
+    const ProtobufWkt::Any& typed_config = message.typed_config();
+    static const std::string struct_type =
+        ProtobufWkt::Struct::default_instance().GetDescriptor()->full_name();
+    static const std::string typed_struct_type =
+        udpa::type::v1::TypedStruct::default_instance().GetDescriptor()->full_name();
+
+    if (!typed_config.value().empty()) {
+      // Unpack methods will only use the fully qualified type name after the last '/'.
+      // https://github.com/protocolbuffers/protobuf/blob/3.6.x/src/google/protobuf/any.proto#L87
+      absl::string_view type = TypeUtil::typeUrlToDescriptorFullName(typed_config.type_url());
+
+      if (type == typed_struct_type) {
+        udpa::type::v1::TypedStruct typed_struct;
+        MessageUtil::unpackTo(typed_config, typed_struct);
+        // Not handling nested structs or typed structs in typed structs
+        return Utility::getAndCheckFactoryByType<Factory>(
+            TypeUtil::typeUrlToDescriptorFullName(typed_struct.type_url()));
+      } else if (type != struct_type) {
+        return Utility::getAndCheckFactoryByType<Factory>(type);
+      }
+    }
+
     return Utility::getAndCheckFactoryByName<Factory>(message.name());
   }
 

source/common/config/version_converter.cc

@@ -23,7 +23,8 @@ void VersionConverter::upgrade(const Protobuf::Message& prev_message,
 
 DowngradedMessagePtr VersionConverter::downgrade(const Protobuf::Message& message) {
   auto downgraded_message = std::make_unique<DowngradedMessage>();
-  const Protobuf::Descriptor* prev_desc = ApiTypeOracle::getEarlierVersionDescriptor(message);
+  const Protobuf::Descriptor* prev_desc =
+      ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
   if (prev_desc != nullptr) {
     downgraded_message->msg_.reset(
         downgraded_message->dynamic_msg_factory_.GetPrototype(prev_desc)->New());

source/common/protobuf/utility.cc

@@ -134,7 +134,7 @@ class ApiBoostRetryException : public EnvoyException {
 // vN/v(N+1) mechanical transforms.
 void tryWithApiBoosting(MessageXformFn f, Protobuf::Message& message) {
   const Protobuf::Descriptor* earlier_version_desc =
-      Config::ApiTypeOracle::getEarlierVersionDescriptor(message);
+      Config::ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
   // If there is no earlier version of a message, just apply f directly.
   if (earlier_version_desc == nullptr) {
     f(message, MessageVersion::LATEST_VERSION);
@@ -493,7 +493,7 @@ void MessageUtil::unpackTo(const ProtobufWkt::Any& any_message, Protobuf::Messag
       TypeUtil::typeUrlToDescriptorFullName(any_message.type_url());
   if (any_full_name != message.GetDescriptor()->full_name()) {
     const Protobuf::Descriptor* earlier_version_desc =
-        Config::ApiTypeOracle::getEarlierVersionDescriptor(message);
+        Config::ApiTypeOracle::getEarlierVersionDescriptor(message.GetDescriptor()->full_name());
     // If the earlier version matches, unpack and upgrade.
     if (earlier_version_desc != nullptr && any_full_name == earlier_version_desc->full_name()) {
       Protobuf::DynamicMessageFactory dmf;

test/common/config/api_type_oracle_test.cc

@@ -16,9 +16,12 @@ TEST(ApiTypeOracleTest, All) {
   envoy::extensions::filters::http::ip_tagging::v3alpha::IPTagging v3_config;
   ProtobufWkt::Any non_api_type;
 
-  EXPECT_EQ(nullptr, ApiTypeOracle::getEarlierVersionDescriptor(non_api_type));
-  EXPECT_EQ(nullptr, ApiTypeOracle::getEarlierVersionDescriptor(v2_config));
-  const auto* desc = ApiTypeOracle::getEarlierVersionDescriptor(v3_config);
+  EXPECT_EQ(nullptr,
+            ApiTypeOracle::getEarlierVersionDescriptor(non_api_type.GetDescriptor()->full_name()));
+  EXPECT_EQ(nullptr,
+            ApiTypeOracle::getEarlierVersionDescriptor(v2_config.GetDescriptor()->full_name()));
+  const auto* desc =
+      ApiTypeOracle::getEarlierVersionDescriptor(v3_config.GetDescriptor()->full_name());
   EXPECT_EQ(envoy::config::filter::http::ip_tagging::v2::IPTagging::descriptor()->full_name(),
             desc->full_name());
 }

test/extensions/access_loggers/file/config_test.cc

@@ -32,26 +32,26 @@ TEST(FileAccessLogConfigTest, ValidateFail) {
 TEST(FileAccessLogConfigTest, ConfigureFromProto) {
   envoy::config::accesslog::v3alpha::AccessLog config;
 
+  NiceMock<Server::Configuration::MockFactoryContext> context;
+  EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
+                            "Provided name for static registration lookup was empty.");
+
+  config.set_name("INVALID");
+
+  EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
+                            "Didn't find a registered implementation for name: 'INVALID'");
+
   envoy::extensions::access_loggers::file::v3alpha::FileAccessLog fal_config;
   fal_config.set_path("/dev/null");
 
   config.mutable_typed_config()->PackFrom(fal_config);
 
-  NiceMock<Server::Configuration::MockFactoryContext> context;
-  EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
-                            "Provided name for static registration lookup was empty.");
-
   config.set_name(AccessLogNames::get().File);
 
   AccessLog::InstanceSharedPtr log = AccessLog::AccessLogFactory::fromProto(config, context);
 
   EXPECT_NE(nullptr, log);
   EXPECT_NE(nullptr, dynamic_cast<FileAccessLog*>(log.get()));
-
-  config.set_name("INVALID");
-
-  EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
-                            "Didn't find a registered implementation for name: 'INVALID'");
 }
 
 TEST(FileAccessLogConfigTest, FileAccessLogTest) {
@@ -92,23 +92,22 @@ TEST(FileAccessLogConfigTest, FileAccessLogJsonTest) {
   EXPECT_EQ(fal_config.access_log_format_case(),
             envoy::extensions::access_loggers::file::v3alpha::FileAccessLog::AccessLogFormatCase::
                 kJsonFormat);
-  config.mutable_typed_config()->PackFrom(fal_config);
-
   NiceMock<Server::Configuration::MockFactoryContext> context;
   EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
                             "Provided name for static registration lookup was empty.");
 
+  config.set_name("INVALID");
+
+  EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
+                            "Didn't find a registered implementation for name: 'INVALID'");
+  config.mutable_typed_config()->PackFrom(fal_config);
+
   config.set_name(AccessLogNames::get().File);
 
   AccessLog::InstanceSharedPtr log = AccessLog::AccessLogFactory::fromProto(config, context);
 
   EXPECT_NE(nullptr, log);
   EXPECT_NE(nullptr, dynamic_cast<FileAccessLog*>(log.get()));
-
-  config.set_name("INVALID");
-
-  EXPECT_THROW_WITH_MESSAGE(AccessLog::AccessLogFactory::fromProto(config, context), EnvoyException,
-                            "Didn't find a registered implementation for name: 'INVALID'");
 }
 
 TEST(FileAccessLogConfigTest, FileAccessLogTypedJsonTest) {

test/integration/xds_integration_test.cc

@@ -120,6 +120,7 @@ TEST_P(LdsIntegrationTest, FailConfigLoad) {
       [&](envoy::config::bootstrap::v3alpha::Bootstrap& bootstrap) -> void {
         auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0);
         auto* filter_chain = listener->mutable_filter_chains(0);
+        filter_chain->mutable_filters(0)->clear_typed_config();
         filter_chain->mutable_filters(0)->set_name("grewgragra");
       });
   EXPECT_DEATH_LOG_TO_STDERR(initialize(),

test/server/configuration_impl_test.cc

@@ -277,9 +277,9 @@ TEST_F(ConfigurationImplTest, ConfigurationFailsWhenInvalidTracerSpecified) {
 
   auto bootstrap = Upstream::parseBootstrapFromV2Json(json);
   MainImpl config;
-  EXPECT_THROW_WITH_MESSAGE(config.initialize(bootstrap, server_, cluster_manager_factory_),
-                            EnvoyException,
-                            "Didn't find a registered implementation for name: 'invalid'");
+  EXPECT_THROW_WITH_MESSAGE(
+      config.initialize(bootstrap, server_, cluster_manager_factory_), EnvoyException,
+      "Didn't find a registered implementation for type: 'envoy.config.trace.v2.LightstepConfig'");
 }
 
 TEST_F(ConfigurationImplTest, ProtoSpecifiedStatsSink) {
@@ -336,7 +336,6 @@ TEST_F(ConfigurationImplTest, StatsSinkWithInvalidName) {
 
   envoy::config::metrics::v3alpha::StatsSink& sink = *bootstrap.mutable_stats_sinks()->Add();
   sink.set_name("envoy.invalid");
-  addStatsdFakeClusterConfig(sink);
 
   MainImpl config;
   EXPECT_THROW_WITH_MESSAGE(config.initialize(bootstrap, server_, cluster_manager_factory_),
@@ -365,8 +364,7 @@ TEST_F(ConfigurationImplTest, StatsSinkWithNoName) {
 
   auto bootstrap = Upstream::parseBootstrapFromV2Json(json);
 
-  auto& sink = *bootstrap.mutable_stats_sinks()->Add();
-  addStatsdFakeClusterConfig(sink);
+  bootstrap.mutable_stats_sinks()->Add();
 
   MainImpl config;
   EXPECT_THROW_WITH_MESSAGE(config.initialize(bootstrap, server_, cluster_manager_factory_),

test/test_common/registry.h

@@ -24,7 +24,8 @@ template <class Base> class InjectFactory {
       auto injected = Registry::FactoryRegistry<Base>::replaceFactoryForTest(*displaced_);
       EXPECT_EQ(injected, &instance_);
     } else {
-      Registry::FactoryRegistry<Base>::removeFactoryForTest(instance_.name());
+      Registry::FactoryRegistry<Base>::removeFactoryForTest(instance_.name(),
+                                                            instance_.configType());
     }
   }