Skip to content

Commit

Permalink
upstream: allow extension protocol options to be used with http filte…
Browse files Browse the repository at this point in the history
…rs (#4165)

#4098 Added protocol extensions for clusters and network filters. This PR expands this to http filters as well (this can be used for example in a filter that translates http to a different protocol, like our NATS Streaming filter)

Risk Level: low, ignored by existing protocols
Testing: unit tests
Docs Changes: inline
Release Notes: n/a

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
  • Loading branch information
yuval-k authored and htuch committed Aug 20, 2018
1 parent fbf6af7 commit de4e2f4
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 52 deletions.
51 changes: 30 additions & 21 deletions include/envoy/server/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

Expand Down Expand Up @@ -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.
Expand All @@ -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() {}

Expand Down
9 changes: 4 additions & 5 deletions source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 15 additions & 4 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
123 changes: 101 additions & 22 deletions test/common/upstream/upstream_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProtobufTypes::MessagePtr()> empty_proto,
std::function<Upstream::ProtocolOptionsConfigConstSharedPtr(const Protobuf::Message&)> 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<ProtobufTypes::MessagePtr()> empty_proto_;
std::function<Upstream::ProtocolOptionsConfigConstSharedPtr(const Protobuf::Message&)> config_;
};

class TestNetworkFilterConfigFactory
: public Server::Configuration::NamedNetworkFilterConfigFactory {
public:
TestNetworkFilterConfigFactory(TestFilterConfigFactoryBase& parent) : parent_(parent) {}

// NamedNetworkFilterConfigFactory
Network::FilterFactoryCb createFilterFactory(const Json::Object&,
Server::Configuration::FactoryContext&) override {
Expand All @@ -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<ProtobufTypes::MessagePtr()> empty_proto_;
std::function<Upstream::ProtocolOptionsConfigConstSharedPtr(const Protobuf::Message&)> 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<Server::Configuration::NamedNetworkFilterConfigFactory> registry(factory);

const std::string yaml = R"EOF(
name: name
connect_timeout: 0.25s
Expand All @@ -1598,23 +1648,33 @@ 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<Server::Configuration::NamedNetworkFilterConfigFactory> registry(
factory);
EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException,
"filter envoy.test.filter does not support protocol options");
}
{
TestHttpFilterConfigFactory factory(factoryBase);
Registry::InjectFactory<Server::Configuration::NamedHttpFilterConfigFactory> 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<TestFilterProtocolOptionsConfig>();

TestFilterConfigFactory factory(
TestFilterConfigFactoryBase factoryBase(
[]() -> ProtobufTypes::MessagePtr { return std::make_unique<ProtobufWkt::Struct>(); },
[&](const Protobuf::Message& msg) -> Upstream::ProtocolOptionsConfigConstSharedPtr {
const auto& msg_struct = dynamic_cast<const ProtobufWkt::Struct&>(msg);
EXPECT_TRUE(msg_struct.fields().find("option") != msg_struct.fields().end());

return protocol_options;
});
Registry::InjectFactory<Server::Configuration::NamedNetworkFilterConfigFactory> registry(factory);

const std::string yaml = R"EOF(
name: name
Expand All @@ -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<std::unique_ptr<StrictDnsClusterImpl>> clusters;

std::shared_ptr<const TestFilterProtocolOptionsConfig> stored_options =
cluster->info()->extensionProtocolOptionsTyped<TestFilterProtocolOptionsConfig>(
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<Server::Configuration::NamedNetworkFilterConfigFactory> registry(
factory);
clusters.push_back(makeCluster(yaml));
}
{
// Get the cluster with extension_protocol_options for an http filter factory.
TestHttpFilterConfigFactory factory(factoryBase);
Registry::InjectFactory<Server::Configuration::NamedHttpFilterConfigFactory> 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<const TestFilterProtocolOptionsConfig> stored_options =
cluster->info()->extensionProtocolOptionsTyped<TestFilterProtocolOptionsConfig>(
"envoy.test.filter");
EXPECT_NE(nullptr, protocol_options);
// Same pointer
EXPECT_EQ(stored_options.get(), protocol_options.get());
}
}

// Validate empty singleton for HostsPerLocalityImpl.
Expand Down

0 comments on commit de4e2f4

Please sign in to comment.