diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 307f66fb59cdd..a3a3dbb3b219b 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -181,11 +181,39 @@ class NamedListenerFilterConfigFactory { virtual std::string name() PURE; }; +/** + * Implemented by filter factories that require more options to process the protocol used by the + * upstream cluster. + */ +class ProtocolOptionsFactory { +public: + virtual ~ProtocolOptionsFactory() {} + + /** + * Create a particular filter's protocol specific options implementation. If the factory + * implementation is unable to produce a factory with the provided parameters, it should throw an + * EnvoyException. + * @param config supplies the protobuf configuration for the filter + * @return Upstream::ProtocoOptionsConfigConstSharedPtr the protocol options + */ + virtual Upstream::ProtocolOptionsConfigConstSharedPtr + createProtocolOptionsConfig(const Protobuf::Message& config) { + UNREFERENCED_PARAMETER(config); + return nullptr; + } + + /** + * @return ProtobufTypes::MessagePtr a newly created empty protocol specific options message or + * nullptr if protocol specific options are not available. + */ + virtual ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() { return nullptr; } +}; + /** * Implemented by each network filter and registered via Registry::registerFactory() * or the convenience class RegisterFactory. */ -class NamedNetworkFilterConfigFactory { +class NamedNetworkFilterConfigFactory : public ProtocolOptionsFactory { public: virtual ~NamedNetworkFilterConfigFactory() {} @@ -220,25 +248,6 @@ class NamedNetworkFilterConfigFactory { */ virtual ProtobufTypes::MessagePtr createEmptyConfigProto() { return nullptr; } - /** - * Create a particular network filter's protocol specific options implementation. If the factory - * implementation is unable to produce a factory with the provided parameters, it should throw an - * EnvoyException. - * @param config supplies the protobuf configuration for the filter - * @return Upstream::ProtocoOptionsConfigConstSharedPtr the protocol options - */ - virtual Upstream::ProtocolOptionsConfigConstSharedPtr - createProtocolOptionsConfig(const Protobuf::Message& config) { - UNREFERENCED_PARAMETER(config); - return nullptr; - } - - /** - * @return ProtobufTypes::MessagePtr a newly created empty protocol specific options message or - * nullptr if protocol specific options are not available. - */ - virtual ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() { return nullptr; } - /** * @return std::string the identifying name for a particular implementation of a network filter * produced by the factory. @@ -250,7 +259,7 @@ class NamedNetworkFilterConfigFactory { * Implemented by each HTTP filter and registered via Registry::registerFactory or the * convenience class RegisterFactory. */ -class NamedHttpFilterConfigFactory { +class NamedHttpFilterConfigFactory : public ProtocolOptionsFactory { public: virtual ~NamedHttpFilterConfigFactory() {} diff --git a/source/common/config/utility.h b/source/common/config/utility.h index 7a88dbe9d5221..9d172f1eb5930 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -280,14 +280,13 @@ class Utility { * @return ProtobufTypes::MessagePtr the translated config * @throws EnvoyException if the factory does not support protocol options */ - static ProtobufTypes::MessagePtr translateToFactoryProtocolOptionsConfig( - const Protobuf::Message& source, - Server::Configuration::NamedNetworkFilterConfigFactory& factory) { + static ProtobufTypes::MessagePtr + translateToFactoryProtocolOptionsConfig(const Protobuf::Message& source, const std::string& name, + Server::Configuration::ProtocolOptionsFactory& factory) { ProtobufTypes::MessagePtr config = factory.createEmptyProtocolOptionsProto(); if (config == nullptr) { - throw EnvoyException( - fmt::format("filter {} does not support protocol options", factory.name())); + throw EnvoyException(fmt::format("filter {} does not support protocol options", name)); } MessageUtil::jsonConvert(source, *config); diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 1f191c007f73b..5e3e4fa91c228 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -119,12 +119,23 @@ parseExtensionProtocolOptions(const envoy::api::v2::Cluster& config) { for (const auto& iter : config.extension_protocol_options()) { const std::string& name = iter.first; const ProtobufWkt::Struct& config_struct = iter.second; + Server::Configuration::ProtocolOptionsFactory* factory = nullptr; - auto& factory = Envoy::Config::Utility::getAndCheckFactory< - Server::Configuration::NamedNetworkFilterConfigFactory>(name); + factory = Registry::FactoryRegistry< + Server::Configuration::NamedNetworkFilterConfigFactory>::getFactory(name); + if (factory == nullptr) { + factory = Registry::FactoryRegistry< + Server::Configuration::NamedHttpFilterConfigFactory>::getFactory(name); + } + + if (factory == nullptr) { + throw EnvoyException(fmt::format( + "Didn't find a registered network or http filter implementation for name: '{}'", name)); + } - auto object = factory.createProtocolOptionsConfig( - *Envoy::Config::Utility::translateToFactoryProtocolOptionsConfig(config_struct, factory)); + auto object = factory->createProtocolOptionsConfig( + *Envoy::Config::Utility::translateToFactoryProtocolOptionsConfig(config_struct, name, + *factory)); if (object) { options[name] = object; } diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 921b327664697..05321b9b8af79 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -1543,17 +1543,33 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForUnknownFilter) { no_such_filter: { option: value } )EOF"; - EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, - "Didn't find a registered implementation for name: 'no_such_filter'"); + EXPECT_THROW_WITH_MESSAGE( + makeCluster(yaml), EnvoyException, + "Didn't find a registered network or http filter implementation for name: 'no_such_filter'"); } -class TestFilterConfigFactory : public Server::Configuration::NamedNetworkFilterConfigFactory { +class TestFilterConfigFactoryBase { public: - TestFilterConfigFactory( + TestFilterConfigFactoryBase( std::function empty_proto, std::function config) : empty_proto_(empty_proto), config_(config) {} + ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() { return empty_proto_(); } + Upstream::ProtocolOptionsConfigConstSharedPtr + createProtocolOptionsConfig(const Protobuf::Message& msg) { + return config_(msg); + } + + std::function empty_proto_; + std::function config_; +}; + +class TestNetworkFilterConfigFactory + : public Server::Configuration::NamedNetworkFilterConfigFactory { +public: + TestNetworkFilterConfigFactory(TestFilterConfigFactoryBase& parent) : parent_(parent) {} + // NamedNetworkFilterConfigFactory Network::FilterFactoryCb createFilterFactory(const Json::Object&, Server::Configuration::FactoryContext&) override { @@ -1565,29 +1581,63 @@ class TestFilterConfigFactory : public Server::Configuration::NamedNetworkFilter NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } ProtobufTypes::MessagePtr createEmptyConfigProto() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() override { return empty_proto_(); } + ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() override { + return parent_.createEmptyProtocolOptionsProto(); + } Upstream::ProtocolOptionsConfigConstSharedPtr createProtocolOptionsConfig(const Protobuf::Message& msg) override { - return config_(msg); + return parent_.createProtocolOptionsConfig(msg); } std::string name() override { CONSTRUCT_ON_FIRST_USE(std::string, "envoy.test.filter"); } - std::function empty_proto_; - std::function config_; + TestFilterConfigFactoryBase& parent_; }; +class TestHttpFilterConfigFactory : public Server::Configuration::NamedHttpFilterConfigFactory { +public: + TestHttpFilterConfigFactory(TestFilterConfigFactoryBase& parent) : parent_(parent) {} + + // NamedNetworkFilterConfigFactory + Http::FilterFactoryCb createFilterFactory(const Json::Object&, const std::string&, + Server::Configuration::FactoryContext&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Http::FilterFactoryCb + createFilterFactoryFromProto(const Protobuf::Message&, const std::string&, + Server::Configuration::FactoryContext&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + ProtobufTypes::MessagePtr createEmptyConfigProto() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + ProtobufTypes::MessagePtr createEmptyRouteConfigProto() override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + Router::RouteSpecificFilterConfigConstSharedPtr + createRouteSpecificFilterConfig(const Protobuf::Message&, + Server::Configuration::FactoryContext&) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + + ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() override { + return parent_.createEmptyProtocolOptionsProto(); + } + Upstream::ProtocolOptionsConfigConstSharedPtr + createProtocolOptionsConfig(const Protobuf::Message& msg) override { + return parent_.createProtocolOptionsConfig(msg); + } + std::string name() override { CONSTRUCT_ON_FIRST_USE(std::string, "envoy.test.filter"); } + + TestFilterConfigFactoryBase& parent_; +}; struct TestFilterProtocolOptionsConfig : public Upstream::ProtocolOptionsConfig {}; // Cluster extension protocol options fails validation when configured for filter that does not // support options. TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithoutOptions) { - TestFilterConfigFactory factory( + TestFilterConfigFactoryBase factoryBase( []() -> ProtobufTypes::MessagePtr { return nullptr; }, [](const Protobuf::Message&) -> Upstream::ProtocolOptionsConfigConstSharedPtr { return nullptr; }); - Registry::InjectFactory registry(factory); - const std::string yaml = R"EOF( name: name connect_timeout: 0.25s @@ -1598,15 +1648,26 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithoutOptions) { envoy.test.filter: { option: value } )EOF"; - EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, - "filter envoy.test.filter does not support protocol options"); + { + TestNetworkFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry( + factory); + EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, + "filter envoy.test.filter does not support protocol options"); + } + { + TestHttpFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry(factory); + EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, + "filter envoy.test.filter does not support protocol options"); + } } // Cluster retrieval of typed extension protocol options. TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { auto protocol_options = std::make_shared(); - TestFilterConfigFactory factory( + TestFilterConfigFactoryBase factoryBase( []() -> ProtobufTypes::MessagePtr { return std::make_unique(); }, [&](const Protobuf::Message& msg) -> Upstream::ProtocolOptionsConfigConstSharedPtr { const auto& msg_struct = dynamic_cast(msg); @@ -1614,7 +1675,6 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { return protocol_options; }); - Registry::InjectFactory registry(factory); const std::string yaml = R"EOF( name: name @@ -1626,14 +1686,33 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { envoy.test.filter: { option: "value" } )EOF"; - auto cluster = makeCluster(yaml); + // This vector is used to gather clusters with extension_protocol_options from the different + // types of extension factories (network, http). + std::vector> clusters; - std::shared_ptr stored_options = - cluster->info()->extensionProtocolOptionsTyped( - factory.name()); - EXPECT_NE(nullptr, protocol_options); - // Same pointer - EXPECT_EQ(stored_options.get(), protocol_options.get()); + { + // Get the cluster with extension_protocol_options for a network filter factory. + TestNetworkFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry( + factory); + clusters.push_back(makeCluster(yaml)); + } + { + // Get the cluster with extension_protocol_options for an http filter factory. + TestHttpFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry(factory); + clusters.push_back(makeCluster(yaml)); + } + + // Make sure that the clusters created from both factories are as expected. + for (auto&& cluster : clusters) { + std::shared_ptr stored_options = + cluster->info()->extensionProtocolOptionsTyped( + "envoy.test.filter"); + EXPECT_NE(nullptr, protocol_options); + // Same pointer + EXPECT_EQ(stored_options.get(), protocol_options.get()); + } } // Validate empty singleton for HostsPerLocalityImpl.