From dcc38b8ac8fbd8fcfd9c9cf62de7aaeeb9f41e4c Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Wed, 19 Sep 2018 21:47:44 -0400 Subject: [PATCH 01/14] Cluster name from SNI Signed-off-by: Shriram Rajagopalan --- source/extensions/extensions_build_config.bzl | 2 + .../filters/network/sni_cluster/BUILD | 32 ++++++++++++ .../filters/network/sni_cluster/config.cc | 49 +++++++++++++++++++ .../network/sni_cluster/sni_cluster.cc | 25 ++++++++++ .../filters/network/sni_cluster/sni_cluster.h | 34 +++++++++++++ .../filters/network/well_known_names.h | 2 + 6 files changed, 144 insertions(+) create mode 100644 source/extensions/filters/network/sni_cluster/BUILD create mode 100644 source/extensions/filters/network/sni_cluster/config.cc create mode 100644 source/extensions/filters/network/sni_cluster/sni_cluster.cc create mode 100644 source/extensions/filters/network/sni_cluster/sni_cluster.h diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 6c6df524a6d7d..8e0d04c4dd0c9 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -70,6 +70,7 @@ EXTENSIONS = { "envoy.filters.network.redis_proxy": "//source/extensions/filters/network/redis_proxy:config", "envoy.filters.network.tcp_proxy": "//source/extensions/filters/network/tcp_proxy:config", "envoy.filters.network.thrift_proxy": "//source/extensions/filters/network/thrift_proxy:config", + "envoy.filters.network.sni_cluster": "//source/extensions/filters/network/sni_cluster:config", # # Resource monitors @@ -178,6 +179,7 @@ WINDOWS_EXTENSIONS = { #"envoy.filters.network.ratelimit": "//source/extensions/filters/network/ratelimit:config", "envoy.filters.network.tcp_proxy": "//source/extensions/filters/network/tcp_proxy:config", #"envoy.filters.network.thrift_proxy": "//source/extensions/filters/network/thrift_proxy:config", + #"envoy.filters.network.sni_cluster": "//source/extensions/filters/network/sni_cluster:config", # # Stat sinks diff --git a/source/extensions/filters/network/sni_cluster/BUILD b/source/extensions/filters/network/sni_cluster/BUILD new file mode 100644 index 0000000000000..355380e7c9f37 --- /dev/null +++ b/source/extensions/filters/network/sni_cluster/BUILD @@ -0,0 +1,32 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "sni_cluster", + srcs = ["sni_cluster.cc"], + hdrs = ["sni_cluster.h"], + deps = [ + "//include/envoy/network:connection_interface", + "//include/envoy/network:filter_interface", + "//source/common/common:assert_lib", + "//source/common/common:minimal_logger_lib", + ], +) + +envoy_cc_library( + name = "config", + srcs = ["config.cc"], + deps = [ + ":sni_cluster", + "//include/envoy/registry", + "//include/envoy/server:filter_config_interface", + "//source/extensions/filters/network:well_known_names", + ], +) diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc new file mode 100644 index 0000000000000..40a9436f6b8c5 --- /dev/null +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -0,0 +1,49 @@ +#include "envoy/registry/registry.h" +#include "envoy/server/filter_config.h" + +#include "extensions/filters/network/sni_cluster/sni_cluster.h" +#include "extensions/filters/network/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace NetworkFilters { +namespace SniCluster { + +/** + * Config registration for the sni_cluster filter. @see NamedNetworkFilterConfigFactory. + */ +class SniClusterNetworkFilterConfigFactory : public Server::Configuration::NamedNetworkFilterConfigFactory { +public: + // NamedNetworkFilterConfigFactory + Network::FilterFactoryCb createFilterFactory(const Json::Object&, + Server::Configuration::FactoryContext&) override { + // Only used in v1 filters. + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + + Network::FilterFactoryCb + createFilterFactoryFromProto(const Protobuf::Message&, + Server::Configuration::FactoryContext&) override { + return [](Network::FilterManager& filter_manager) -> void { + filter_manager.addReadFilter(std::make_shared()); + }; + } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Empty()}; + } + + std::string name() override { return NetworkFilterNames::get().SniCluster; } +}; + +/** + * Static registration for the sni_cluster filter. @see RegisterFactory. + */ +static Registry::RegisterFactory + registered_; + +} // namespace SniCluster +} // namespace NetworkFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc new file mode 100644 index 0000000000000..02428119705dc --- /dev/null +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -0,0 +1,25 @@ +#include "extensions/filters/network/sni_cluster/sni_cluster.h" + +#include "envoy/network/connection.h" + +#include "common/common/assert.h" + +namespace Envoy { +namespace Extensions { +namespace NetworkFilters { +namespace SniCluster { + +Network::FilterStatus SniClusterFilter::onNewConnection() { + absl::string_view& sni = read_callbacks_->connection.requestedServerName(); + ENVOY_CONN_LOG(trace, "sni_cluster: new connection with server name {}", read_callbacks_->connection(), sni); + if (!sni.empty()) { + // Set the tcp_proxy cluster to the same value as SNI + read_callbacks_->connection().perConnectionState().setData("envoy.tcp_proxy.cluster", sni) + } + return Network::FilterStatus::Continue; +} + +} // namespace SniCluster +} // namespace NetworkFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.h b/source/extensions/filters/network/sni_cluster/sni_cluster.h new file mode 100644 index 0000000000000..2ee6c6d7cd5eb --- /dev/null +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.h @@ -0,0 +1,34 @@ +#pragma once + +#include "envoy/network/filter.h" + +#include "common/common/logger.h" + +namespace Envoy { +namespace Extensions { +namespace NetworkFilters { +namespace SniCluster { + +/** + * Implementation of the sni_cluster filter that sets the upstream cluster name from + * the SNI field in the TLS connection. + */ +class SniClusterFilter : public Network::ReadFilter, Logger::Loggable { +public: + // Network::ReadFilter + Network::FilterStatus onData(Buffer::Instance& data, bool end_stream) override { + return Network::FilterStatus::Continue; + } + Network::FilterStatus onNewConnection() override; + void initializeReadFilterCallbacks(Network::ReadFilterCallbacks& callbacks) override { + read_callbacks_ = &callbacks; + } + +private: + Network::ReadFilterCallbacks* read_callbacks_{}; +}; + +} // namespace SniCluster +} // namespace NetworkFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/network/well_known_names.h b/source/extensions/filters/network/well_known_names.h index 06123fd6df511..de09145f83a3c 100644 --- a/source/extensions/filters/network/well_known_names.h +++ b/source/extensions/filters/network/well_known_names.h @@ -32,6 +32,8 @@ class NetworkFilterNameValues { const std::string ThriftProxy = "envoy.filters.network.thrift_proxy"; // Role based access control filter const std::string Rbac = "envoy.filters.network.rbac"; + // SNI Cluster filter + const std::string SniCluster = "envoy.filters.network.sni_cluster"; // Converts names from v1 to v2 const Config::V1Converter v1_converter_; From f10f6b4917994992e5acc702a87de026ff60f24e Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 20 Sep 2018 15:55:44 +0000 Subject: [PATCH 02/14] tests and more Signed-off-by: Shriram Rajagopalan --- source/common/tcp_proxy/BUILD | 1 + source/common/tcp_proxy/tcp_proxy.cc | 8 +++ source/common/tcp_proxy/tcp_proxy.h | 14 +++++ .../filters/network/sni_cluster/BUILD | 1 + .../filters/network/sni_cluster/config.cc | 3 +- .../network/sni_cluster/sni_cluster.cc | 11 +++- test/common/tcp_proxy/tcp_proxy_test.cc | 19 ++++++ .../filters/network/sni_cluster/BUILD | 25 ++++++++ .../network/sni_cluster/sni_cluster_test.cc | 58 +++++++++++++++++++ 9 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 test/extensions/filters/network/sni_cluster/BUILD create mode 100644 test/extensions/filters/network/sni_cluster/sni_cluster_test.cc diff --git a/source/common/tcp_proxy/BUILD b/source/common/tcp_proxy/BUILD index 7fcc846da64e5..7a91233803740 100644 --- a/source/common/tcp_proxy/BUILD +++ b/source/common/tcp_proxy/BUILD @@ -19,6 +19,7 @@ envoy_cc_library( "//include/envoy/event:dispatcher_interface", "//include/envoy/network:connection_interface", "//include/envoy/network:filter_interface", + "//include/envoy/request_info:filter_state_interface", "//include/envoy/router:router_interface", "//include/envoy/server:filter_config_interface", "//include/envoy/stats:stats_interface", diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 4e148befd3ea2..b6ceada35f253 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -88,6 +88,14 @@ Config::Config(const envoy::config::filter::network::tcp_proxy::v2::TcpProxy& co } const std::string& Config::getRouteFromEntries(Network::Connection& connection) { + // First check if the per-connection state to see if we need to route to a pre-selected cluster + if (connection.perConnectionState().hasDataWithName(PerConnectionTcpProxyConfig::CLUSTER_KEY)) { + const auto per_connection_config = + connection.perConnectionState().getData( + PerConnectionTcpProxyConfig::CLUSTER_KEY); + return per_connection_config.cluster(); + } + for (const Config::Route& route : routes_) { if (!route.source_port_ranges_.empty() && !Network::Utility::portInRangeList(*connection.remoteAddress(), diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 09bf0654123b8..37250199baad1 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -11,6 +11,7 @@ #include "envoy/event/timer.h" #include "envoy/network/connection.h" #include "envoy/network/filter.h" +#include "envoy/request_info/filter_state.h" #include "envoy/server/filter_config.h" #include "envoy/stats/scope.h" #include "envoy/stats/stats_macros.h" @@ -135,6 +136,19 @@ class Config { typedef std::shared_ptr ConfigSharedPtr; +/** + * Per-connection TCP Proxy Filter configuration. + */ +class PerConnectionTcpProxyConfig : public FilterState::Object { +public: + PerConnectionTcpProxyConfig(std::string& cluster) : cluster_(cluster) {} + const std::string& cluster() const { return cluster_; } + static const std::string CLUSTER_KEY = "envoy.tcp_proxy.cluster"; + +private: + std::string cluster_; +}; + /** * An implementation of a TCP (L3/L4) proxy. This filter will instantiate a new outgoing TCP * connection using the defined load balancing proxy for the configured cluster. All data will diff --git a/source/extensions/filters/network/sni_cluster/BUILD b/source/extensions/filters/network/sni_cluster/BUILD index 355380e7c9f37..14474941b74d4 100644 --- a/source/extensions/filters/network/sni_cluster/BUILD +++ b/source/extensions/filters/network/sni_cluster/BUILD @@ -17,6 +17,7 @@ envoy_cc_library( "//include/envoy/network:filter_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", + "//source/common/tcp_proxy:tcp_proxy_lib", ], ) diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc index 40a9436f6b8c5..2c8f3aadae072 100644 --- a/source/extensions/filters/network/sni_cluster/config.cc +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -12,7 +12,8 @@ namespace SniCluster { /** * Config registration for the sni_cluster filter. @see NamedNetworkFilterConfigFactory. */ -class SniClusterNetworkFilterConfigFactory : public Server::Configuration::NamedNetworkFilterConfigFactory { +class SniClusterNetworkFilterConfigFactory + : public Server::Configuration::NamedNetworkFilterConfigFactory { public: // NamedNetworkFilterConfigFactory Network::FilterFactoryCb createFilterFactory(const Json::Object&, diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc index 02428119705dc..53ec0734c03d2 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.cc +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -3,6 +3,7 @@ #include "envoy/network/connection.h" #include "common/common/assert.h" +#include "common/tcp_proxy/tcp_proxy.h" namespace Envoy { namespace Extensions { @@ -10,11 +11,15 @@ namespace NetworkFilters { namespace SniCluster { Network::FilterStatus SniClusterFilter::onNewConnection() { - absl::string_view& sni = read_callbacks_->connection.requestedServerName(); - ENVOY_CONN_LOG(trace, "sni_cluster: new connection with server name {}", read_callbacks_->connection(), sni); + absl::string_view& sni = read_callbacks_->connection.requestedServerName(); + ENVOY_CONN_LOG(trace, "sni_cluster: new connection with server name {}", + read_callbacks_->connection(), sni); if (!sni.empty()) { + // Set the tcp_proxy cluster to the same value as SNI - read_callbacks_->connection().perConnectionState().setData("envoy.tcp_proxy.cluster", sni) + read_callbacks_->connection().perConnectionState().setData( + Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY, + std::make_unique(sni)); } return Network::FilterStatus::Continue; } diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 6b35afc0376fe..ad6bb721aa71c 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1128,5 +1128,24 @@ TEST_F(TcpProxyRoutingTest, RoutableConnection) { EXPECT_EQ(non_routable_cx, config_->stats().downstream_cx_no_route_.value()); } +// Test that the tcp proxy uses the cluster from FilterState if set +TEST_F(TcpProxyTest, UseClusterFromPerConnectionState) { + setup(1); + + auto per_connection_state = std::make_unique("filter_state_cluster"); + filter_callbacks_.connection_.per_connection_state_.setData("envoy.tcp_proxy.cluster", + per_connection_state); + connection_.local_address_ = std::make_shared("1.2.3.4", 9999); + ON_CALL(filter_callbacks_.connection_, perConnectionState) + .WillByDefault(ReturnRef(filter_callbacks_.connection_.per_connection_state_)); + + // Expect filter to try to open a connection to specified cluster. + EXPECT_CALL(factory_context_.cluster_manager_, + tcpConnPoolForCluster("filter_state_cluster", _, _)) + .WillOnce(Return(nullptr)); + + filter_->onNewConnection(); +} + } // namespace TcpProxy } // namespace Envoy diff --git a/test/extensions/filters/network/sni_cluster/BUILD b/test/extensions/filters/network/sni_cluster/BUILD new file mode 100644 index 0000000000000..397d7714cd679 --- /dev/null +++ b/test/extensions/filters/network/sni_cluster/BUILD @@ -0,0 +1,25 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +envoy_package() + +envoy_extension_cc_test( + name = "sni_cluster_test", + srcs = ["sni_cluster_test.cc"], + extension_name = "envoy.filters.network.sni_cluster", + deps = [ + "//test/mocks/network:network_mocks", + "//test/mocks/server:server_mocks", + "//extensions/filters/network/sni_cluster/config_lib", + "//extensions/filters/network/sni_cluster/sni_cluster_lib", + ], +) diff --git a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc new file mode 100644 index 0000000000000..7ea8101d2b881 --- /dev/null +++ b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc @@ -0,0 +1,58 @@ +#include "test/mocks/network/mocks.h" +#include "test/mocks/server/mocks.h" + +#include "extensions/filters/network/sni_cluster/sni_cluster.h" + +#include "common/tcp_proxy/tcp_proxy.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::_; + +namespace Envoy { +namespace Extensions { +namespace NetworkFilters { +namespace SniCluster { + +// Test that a SniCluster filter config works. +TEST(SniCluster, ConfigTest) { + NiceMock context; + SniClusterNetworkFilterConfigFactory factory; + + Network::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*factory.createEmptyConfigProto(), context); + Network::MockConnection connection; + EXPECT_CALL(connection, addReadFilter(_)); + cb(connection); +} + +// Test that per connection filter config is set if SNI is available +TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { + NiceMock filter_callbacks; + SniClusterFilter filter; + filter.initializeReadCallbacks(filter_callbacks); + + // no sni + { + ON_CALL(filter_callbacks.connection, requestedServerName).WillByDefault(Return(EMPTY_STRING)); + filter.onNewConnection(); + + EXPECT_EQ(filter_callbacks.connection.per_connection_state_.hasData(Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY), false); + } + + // with sni + { + ON_CALL(filter_callbacks.connection, requestedServerName).WillByDefault(Return("filter_state_cluster")); + filter.onNewConnection(); + + EXPECT_EQ(filter_callbacks.connection.per_connection_state_.hasData(Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY), true); + + auto per_connection_config = filter_callbacks.connection.per_connection_state_.getData(Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY); + EXPECT_EQ(per_connection_config.cluster(), "filter_state_cluster"); + } +} + +} // namespace SniCluster +} // namespace NetworkFilters +} // namespace Extensions +} // namespace Envoy From c783a1813470e07ac211d779a494027b666622a0 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 20 Sep 2018 16:30:51 +0000 Subject: [PATCH 03/14] fixes Signed-off-by: Shriram Rajagopalan --- source/common/tcp_proxy/tcp_proxy.cc | 2 ++ source/common/tcp_proxy/tcp_proxy.h | 6 +++--- source/extensions/filters/network/sni_cluster/BUILD | 2 +- .../extensions/filters/network/sni_cluster/sni_cluster.cc | 5 +++-- source/extensions/filters/network/sni_cluster/sni_cluster.h | 2 +- test/extensions/filters/network/sni_cluster/BUILD | 4 ++-- 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index b6ceada35f253..d218fbb9e4add 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -21,6 +21,8 @@ namespace Envoy { namespace TcpProxy { +std::string PerConnectionTcpProxyConfig::CLUSTER_KEY = "envoy.tcp_proxy.cluster"; + Config::Route::Route( const envoy::config::filter::network::tcp_proxy::v2::TcpProxy::DeprecatedV1::TCPRoute& config) { cluster_name_ = config.cluster(); diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 37250199baad1..9fde6c100b408 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -139,11 +139,11 @@ typedef std::shared_ptr ConfigSharedPtr; /** * Per-connection TCP Proxy Filter configuration. */ -class PerConnectionTcpProxyConfig : public FilterState::Object { +class PerConnectionTcpProxyConfig : public Envoy::RequestInfo::FilterState::Object { public: - PerConnectionTcpProxyConfig(std::string& cluster) : cluster_(cluster) {} + PerConnectionTcpProxyConfig(std::string cluster) : cluster_(cluster) {} const std::string& cluster() const { return cluster_; } - static const std::string CLUSTER_KEY = "envoy.tcp_proxy.cluster"; + static const std::string CLUSTER_KEY; private: std::string cluster_; diff --git a/source/extensions/filters/network/sni_cluster/BUILD b/source/extensions/filters/network/sni_cluster/BUILD index 14474941b74d4..f87ab15a137f3 100644 --- a/source/extensions/filters/network/sni_cluster/BUILD +++ b/source/extensions/filters/network/sni_cluster/BUILD @@ -17,7 +17,7 @@ envoy_cc_library( "//include/envoy/network:filter_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", - "//source/common/tcp_proxy:tcp_proxy_lib", + "//source/common/tcp_proxy:tcp_proxy", ], ) diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc index 53ec0734c03d2..349c104e406c7 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.cc +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -11,16 +11,17 @@ namespace NetworkFilters { namespace SniCluster { Network::FilterStatus SniClusterFilter::onNewConnection() { - absl::string_view& sni = read_callbacks_->connection.requestedServerName(); + absl::string_view sni = read_callbacks_->connection().requestedServerName(); ENVOY_CONN_LOG(trace, "sni_cluster: new connection with server name {}", read_callbacks_->connection(), sni); - if (!sni.empty()) { + if (!sni.empty()) { // Set the tcp_proxy cluster to the same value as SNI read_callbacks_->connection().perConnectionState().setData( Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY, std::make_unique(sni)); } + return Network::FilterStatus::Continue; } diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.h b/source/extensions/filters/network/sni_cluster/sni_cluster.h index 2ee6c6d7cd5eb..d062368ac6664 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.h +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.h @@ -16,7 +16,7 @@ namespace SniCluster { class SniClusterFilter : public Network::ReadFilter, Logger::Loggable { public: // Network::ReadFilter - Network::FilterStatus onData(Buffer::Instance& data, bool end_stream) override { + Network::FilterStatus onData(Buffer::Instance&, bool) override { return Network::FilterStatus::Continue; } Network::FilterStatus onNewConnection() override; diff --git a/test/extensions/filters/network/sni_cluster/BUILD b/test/extensions/filters/network/sni_cluster/BUILD index 397d7714cd679..7f458e17d1df6 100644 --- a/test/extensions/filters/network/sni_cluster/BUILD +++ b/test/extensions/filters/network/sni_cluster/BUILD @@ -19,7 +19,7 @@ envoy_extension_cc_test( deps = [ "//test/mocks/network:network_mocks", "//test/mocks/server:server_mocks", - "//extensions/filters/network/sni_cluster/config_lib", - "//extensions/filters/network/sni_cluster/sni_cluster_lib", + "//extensions/filters/network/sni_cluster:config", + "//extensions/filters/network/sni_cluster:sni_cluster", ], ) From 9bba07090399b0c3876d5be0320c0cb1f60200bf Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 20 Sep 2018 19:44:09 +0000 Subject: [PATCH 04/14] tests and fixes Signed-off-by: Shriram Rajagopalan --- source/common/tcp_proxy/tcp_proxy.cc | 5 +- source/common/tcp_proxy/tcp_proxy.h | 2 +- .../filters/network/sni_cluster/BUILD | 4 +- .../sni_cluster/{config.cc => config.h} | 0 test/common/tcp_proxy/tcp_proxy_test.cc | 49 ++++++++++++++++--- .../filters/network/sni_cluster/BUILD | 4 +- .../network/sni_cluster/sni_cluster_test.cc | 39 +++++++++++---- 7 files changed, 78 insertions(+), 25 deletions(-) rename source/extensions/filters/network/sni_cluster/{config.cc => config.h} (100%) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index d218fbb9e4add..459a5795001cf 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -21,7 +21,7 @@ namespace Envoy { namespace TcpProxy { -std::string PerConnectionTcpProxyConfig::CLUSTER_KEY = "envoy.tcp_proxy.cluster"; +const std::string PerConnectionTcpProxyConfig::CLUSTER_KEY = "envoy.tcp_proxy.cluster"; Config::Route::Route( const envoy::config::filter::network::tcp_proxy::v2::TcpProxy::DeprecatedV1::TCPRoute& config) { @@ -91,7 +91,8 @@ Config::Config(const envoy::config::filter::network::tcp_proxy::v2::TcpProxy& co const std::string& Config::getRouteFromEntries(Network::Connection& connection) { // First check if the per-connection state to see if we need to route to a pre-selected cluster - if (connection.perConnectionState().hasDataWithName(PerConnectionTcpProxyConfig::CLUSTER_KEY)) { + if (connection.perConnectionState().hasData( + PerConnectionTcpProxyConfig::CLUSTER_KEY)) { const auto per_connection_config = connection.perConnectionState().getData( PerConnectionTcpProxyConfig::CLUSTER_KEY); diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 9fde6c100b408..6c80dda6292b6 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -141,7 +141,7 @@ typedef std::shared_ptr ConfigSharedPtr; */ class PerConnectionTcpProxyConfig : public Envoy::RequestInfo::FilterState::Object { public: - PerConnectionTcpProxyConfig(std::string cluster) : cluster_(cluster) {} + PerConnectionTcpProxyConfig(absl::string_view cluster) : cluster_(cluster) {} const std::string& cluster() const { return cluster_; } static const std::string CLUSTER_KEY; diff --git a/source/extensions/filters/network/sni_cluster/BUILD b/source/extensions/filters/network/sni_cluster/BUILD index f87ab15a137f3..53e41298a2dd6 100644 --- a/source/extensions/filters/network/sni_cluster/BUILD +++ b/source/extensions/filters/network/sni_cluster/BUILD @@ -17,13 +17,13 @@ envoy_cc_library( "//include/envoy/network:filter_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", - "//source/common/tcp_proxy:tcp_proxy", + "//source/common/tcp_proxy", ], ) envoy_cc_library( name = "config", - srcs = ["config.cc"], + hdrs = ["config.h"], deps = [ ":sni_cluster", "//include/envoy/registry", diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.h similarity index 100% rename from source/extensions/filters/network/sni_cluster/config.cc rename to source/extensions/filters/network/sni_cluster/config.h diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index ad6bb721aa71c..36c4327da7ded 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1128,16 +1128,49 @@ TEST_F(TcpProxyRoutingTest, RoutableConnection) { EXPECT_EQ(non_routable_cx, config_->stats().downstream_cx_no_route_.value()); } +class TcpProxyPerConnectionStateTest : public testing::Test { +public: + TcpProxyPerConnectionStateTest() { + envoy::config::filter::network::tcp_proxy::v2::TcpProxy proto_config; + proto_config.set_stat_prefix("name"); + auto* route = proto_config.mutable_deprecated_v1()->mutable_routes()->Add(); + route->set_cluster("fake_cluster"); + + config_.reset(new Config(proto_config, factory_context_)); + } + + ~TcpProxyPerConnectionStateTest() { + if (filter_ != nullptr) { + filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); + } + } + + void setup() { + EXPECT_CALL(filter_callbacks_, connection()).WillRepeatedly(ReturnRef(connection_)); + + filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); + filter_->initializeReadFilterCallbacks(filter_callbacks_); + } + + Event::TimeSystem& timeSystem() { return factory_context_.dispatcher().timeSystem(); } + + ConfigSharedPtr config_; + NiceMock connection_; + NiceMock filter_callbacks_; + NiceMock factory_context_; + std::unique_ptr filter_; +}; + // Test that the tcp proxy uses the cluster from FilterState if set -TEST_F(TcpProxyTest, UseClusterFromPerConnectionState) { - setup(1); +TEST_F(TcpProxyPerConnectionStateTest, UseClusterFromPerConnectionState) { + setup(); - auto per_connection_state = std::make_unique("filter_state_cluster"); - filter_callbacks_.connection_.per_connection_state_.setData("envoy.tcp_proxy.cluster", - per_connection_state); - connection_.local_address_ = std::make_shared("1.2.3.4", 9999); - ON_CALL(filter_callbacks_.connection_, perConnectionState) - .WillByDefault(ReturnRef(filter_callbacks_.connection_.per_connection_state_)); + Envoy::RequestInfo::FilterStateImpl per_connection_state; + per_connection_state.setData( + "envoy.tcp_proxy.cluster", + std::make_unique("filter_state_cluster")); + ON_CALL(connection_, perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); + ON_CALL(Const(connection_), perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); // Expect filter to try to open a connection to specified cluster. EXPECT_CALL(factory_context_.cluster_manager_, diff --git a/test/extensions/filters/network/sni_cluster/BUILD b/test/extensions/filters/network/sni_cluster/BUILD index 7f458e17d1df6..a7d00cbff5193 100644 --- a/test/extensions/filters/network/sni_cluster/BUILD +++ b/test/extensions/filters/network/sni_cluster/BUILD @@ -17,9 +17,9 @@ envoy_extension_cc_test( srcs = ["sni_cluster_test.cc"], extension_name = "envoy.filters.network.sni_cluster", deps = [ + "//source/extensions/filters/network/sni_cluster", + "//source/extensions/filters/network/sni_cluster:config", "//test/mocks/network:network_mocks", "//test/mocks/server:server_mocks", - "//extensions/filters/network/sni_cluster:config", - "//extensions/filters/network/sni_cluster:sni_cluster", ], ) diff --git a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc index 7ea8101d2b881..b5cffc64565f0 100644 --- a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc +++ b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc @@ -1,14 +1,19 @@ -#include "test/mocks/network/mocks.h" -#include "test/mocks/server/mocks.h" +#include "common/tcp_proxy/tcp_proxy.h" +#include "extensions/filters/network/sni_cluster/config.h" #include "extensions/filters/network/sni_cluster/sni_cluster.h" -#include "common/tcp_proxy/tcp_proxy.h" +#include "test/mocks/network/mocks.h" +#include "test/mocks/server/mocks.h" #include "gmock/gmock.h" #include "gtest/gtest.h" using testing::_; +using testing::Matcher; +using testing::NiceMock; +using testing::Return; +using testing::ReturnRef; namespace Envoy { namespace Extensions { @@ -20,7 +25,8 @@ TEST(SniCluster, ConfigTest) { NiceMock context; SniClusterNetworkFilterConfigFactory factory; - Network::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*factory.createEmptyConfigProto(), context); + Network::FilterFactoryCb cb = + factory.createFilterFactoryFromProto(*factory.createEmptyConfigProto(), context); Network::MockConnection connection; EXPECT_CALL(connection, addReadFilter(_)); cb(connection); @@ -29,25 +35,38 @@ TEST(SniCluster, ConfigTest) { // Test that per connection filter config is set if SNI is available TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { NiceMock filter_callbacks; + + Envoy::RequestInfo::FilterStateImpl per_connection_state; + ON_CALL(filter_callbacks.connection_, perConnectionState()) + .WillByDefault(ReturnRef(per_connection_state)); + ON_CALL(Const(filter_callbacks.connection_), perConnectionState()) + .WillByDefault(ReturnRef(per_connection_state)); + SniClusterFilter filter; - filter.initializeReadCallbacks(filter_callbacks); + filter.initializeReadFilterCallbacks(filter_callbacks); // no sni { - ON_CALL(filter_callbacks.connection, requestedServerName).WillByDefault(Return(EMPTY_STRING)); + ON_CALL(filter_callbacks.connection_, requestedServerName()) + .WillByDefault(Return(EMPTY_STRING)); filter.onNewConnection(); - EXPECT_EQ(filter_callbacks.connection.per_connection_state_.hasData(Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY), false); + EXPECT_FALSE(per_connection_state.hasData( + Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY)); } // with sni { - ON_CALL(filter_callbacks.connection, requestedServerName).WillByDefault(Return("filter_state_cluster")); + ON_CALL(filter_callbacks.connection_, requestedServerName()) + .WillByDefault(Return("filter_state_cluster")); filter.onNewConnection(); - EXPECT_EQ(filter_callbacks.connection.per_connection_state_.hasData(Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY), true); + EXPECT_TRUE(per_connection_state.hasData( + Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY)); - auto per_connection_config = filter_callbacks.connection.per_connection_state_.getData(Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY); + auto per_connection_config = + per_connection_state.getData( + Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY); EXPECT_EQ(per_connection_config.cluster(), "filter_state_cluster"); } } From b430ae72c3410fbc74dfba59ea5914f5e7adfe98 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 20 Sep 2018 20:56:24 +0000 Subject: [PATCH 05/14] streamline Signed-off-by: Shriram Rajagopalan --- test/common/tcp_proxy/tcp_proxy_test.cc | 43 +++---------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 36c4327da7ded..66696f7c9efbb 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1128,54 +1128,21 @@ TEST_F(TcpProxyRoutingTest, RoutableConnection) { EXPECT_EQ(non_routable_cx, config_->stats().downstream_cx_no_route_.value()); } -class TcpProxyPerConnectionStateTest : public testing::Test { -public: - TcpProxyPerConnectionStateTest() { - envoy::config::filter::network::tcp_proxy::v2::TcpProxy proto_config; - proto_config.set_stat_prefix("name"); - auto* route = proto_config.mutable_deprecated_v1()->mutable_routes()->Add(); - route->set_cluster("fake_cluster"); - - config_.reset(new Config(proto_config, factory_context_)); - } - - ~TcpProxyPerConnectionStateTest() { - if (filter_ != nullptr) { - filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); - } - } - - void setup() { - EXPECT_CALL(filter_callbacks_, connection()).WillRepeatedly(ReturnRef(connection_)); - - filter_.reset(new Filter(config_, factory_context_.cluster_manager_, timeSystem())); - filter_->initializeReadFilterCallbacks(filter_callbacks_); - } - - Event::TimeSystem& timeSystem() { return factory_context_.dispatcher().timeSystem(); } - - ConfigSharedPtr config_; - NiceMock connection_; - NiceMock filter_callbacks_; - NiceMock factory_context_; - std::unique_ptr filter_; -}; - // Test that the tcp proxy uses the cluster from FilterState if set -TEST_F(TcpProxyPerConnectionStateTest, UseClusterFromPerConnectionState) { +TEST_F(TcpProxyRoutingTest, UseClusterFromPerConnectionState) { setup(); Envoy::RequestInfo::FilterStateImpl per_connection_state; per_connection_state.setData( "envoy.tcp_proxy.cluster", - std::make_unique("filter_state_cluster")); + std::make_unique("fake_cluster")); ON_CALL(connection_, perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); - ON_CALL(Const(connection_), perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); + EXPECT_CALL(Const(connection_), perConnectionState()).WillRepeatedly(ReturnRef(per_connection_state)); // Expect filter to try to open a connection to specified cluster. EXPECT_CALL(factory_context_.cluster_manager_, - tcpConnPoolForCluster("filter_state_cluster", _, _)) - .WillOnce(Return(nullptr)); + tcpConnPoolForCluster("fake_cluster", _, _)) + .WillRepeatedly(Return(nullptr)); filter_->onNewConnection(); } From ac73c4371e6be7401e4586183537194176fe85ca Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 21 Sep 2018 00:35:22 +0000 Subject: [PATCH 06/14] never use auto Signed-off-by: Shriram Rajagopalan --- source/common/tcp_proxy/tcp_proxy.cc | 2 +- test/common/tcp_proxy/tcp_proxy_test.cc | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 459a5795001cf..7e9ab9343b023 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -93,7 +93,7 @@ const std::string& Config::getRouteFromEntries(Network::Connection& connection) // First check if the per-connection state to see if we need to route to a pre-selected cluster if (connection.perConnectionState().hasData( PerConnectionTcpProxyConfig::CLUSTER_KEY)) { - const auto per_connection_config = + const PerConnectionTcpProxyConfig& per_connection_config = connection.perConnectionState().getData( PerConnectionTcpProxyConfig::CLUSTER_KEY); return per_connection_config.cluster(); diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 66696f7c9efbb..6306673a42ef2 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1135,14 +1135,15 @@ TEST_F(TcpProxyRoutingTest, UseClusterFromPerConnectionState) { Envoy::RequestInfo::FilterStateImpl per_connection_state; per_connection_state.setData( "envoy.tcp_proxy.cluster", - std::make_unique("fake_cluster")); + std::make_unique("filter_state_cluster")); ON_CALL(connection_, perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); - EXPECT_CALL(Const(connection_), perConnectionState()).WillRepeatedly(ReturnRef(per_connection_state)); + EXPECT_CALL(Const(connection_), perConnectionState()) + .WillRepeatedly(ReturnRef(per_connection_state)); // Expect filter to try to open a connection to specified cluster. EXPECT_CALL(factory_context_.cluster_manager_, - tcpConnPoolForCluster("fake_cluster", _, _)) - .WillRepeatedly(Return(nullptr)); + tcpConnPoolForCluster("filter_state_cluster", _, _)) + .WillOnce(Return(nullptr)); filter_->onNewConnection(); } From 7a6a37c5fae143e9f2d8403be5ebcf9d952eaff8 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 21 Sep 2018 18:22:16 +0000 Subject: [PATCH 07/14] docs, release notes Signed-off-by: Shriram Rajagopalan --- api/envoy/api/v2/listener/listener.proto | 1 + .../configuration/network_filters/sni_cluster.rst | 13 +++++++++++++ docs/root/intro/version_history.rst | 2 ++ 3 files changed, 16 insertions(+) create mode 100644 docs/root/configuration/network_filters/sni_cluster.rst diff --git a/api/envoy/api/v2/listener/listener.proto b/api/envoy/api/v2/listener/listener.proto index 1e8015dbb2446..b9da6d6409a6f 100644 --- a/api/envoy/api/v2/listener/listener.proto +++ b/api/envoy/api/v2/listener/listener.proto @@ -31,6 +31,7 @@ message Filter { // * :ref:`envoy.ratelimit ` // * :ref:`envoy.redis_proxy ` // * :ref:`envoy.tcp_proxy ` + // * :ref:`envoy.sni_cluster ` string name = 1 [(validate.rules).string.min_bytes = 1]; // Filter specific configuration which depends on the filter being diff --git a/docs/root/configuration/network_filters/sni_cluster.rst b/docs/root/configuration/network_filters/sni_cluster.rst new file mode 100644 index 0000000000000..9a18fd129cd55 --- /dev/null +++ b/docs/root/configuration/network_filters/sni_cluster.rst @@ -0,0 +1,13 @@ +.. _config_network_filters_sni_cluster: + +Upstream Cluster from SNI +========================= + +The `sni_cluster` is a network filter that uses the SNI value in a TLS +connection as the upstream cluster name. The filter will not modify the +upstream cluster for non-TLS connections. + +This filter has no configuration. It must be installed before the +:ref:`tcp_proxy ` filter. + +* :ref:`v2 API reference ` diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index b0a22d9d0dd5c..d7e9b03b74458 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -88,6 +88,8 @@ Version history * config: Fixed stat inconsistency between xDS and ADS implementation. :ref:`update_failure ` stat is incremented in case of network failure and :ref:`update_rejected ` stat is incremented in case of schema/validation error. +* :ref:`sni_cluster `: introduced a new network filter that forwards connections to the + upstream cluster specified by the SNI value presented by the client during a TLS handshake. 1.7.0 =============== From 03dca75fdd76b706d20cda39faf7835a134979a5 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 21 Sep 2018 20:37:20 +0000 Subject: [PATCH 08/14] two files Signed-off-by: Shriram Rajagopalan --- .../filters/network/sni_cluster/BUILD | 1 + .../filters/network/sni_cluster/config.cc | 40 +++++++++++++++++++ .../filters/network/sni_cluster/config.h | 29 +++----------- .../filters/network/sni_cluster/BUILD | 2 +- 4 files changed, 47 insertions(+), 25 deletions(-) create mode 100644 source/extensions/filters/network/sni_cluster/config.cc diff --git a/source/extensions/filters/network/sni_cluster/BUILD b/source/extensions/filters/network/sni_cluster/BUILD index 53e41298a2dd6..60eec7e5c92bd 100644 --- a/source/extensions/filters/network/sni_cluster/BUILD +++ b/source/extensions/filters/network/sni_cluster/BUILD @@ -23,6 +23,7 @@ envoy_cc_library( envoy_cc_library( name = "config", + srcs = ["config.cc"], hdrs = ["config.h"], deps = [ ":sni_cluster", diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc new file mode 100644 index 0000000000000..96f43160c6ba3 --- /dev/null +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -0,0 +1,40 @@ +#include "envoy/registry/registry.h" +#include "envoy/server/filter_config.h" + +#include "extensions/filters/network/sni_cluster/config.h" +#include "extensions/filters/network/sni_cluster/sni_cluster.h" + +namespace Envoy { +namespace Extensions { +namespace NetworkFilters { +namespace SniCluster { + +Network::FilterFactoryCb SniClusterNetworkFilterConfigFactory::createFilterFactory(const Json::Object&, + Server::Configuration::FactoryContext&) { + // Only used in v1 filters. + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +Network::FilterFactoryCb +SniClusterNetworkFilterConfigFactory::createFilterFactoryFromProto(const Protobuf::Message&, + Server::Configuration::FactoryContext&) { + return [](Network::FilterManager& filter_manager) -> void { + filter_manager.addReadFilter(std::make_shared()); + }; +} + +ProtobufTypes::MessagePtr SniClusterNetworkFilterConfigFactory::createEmptyConfigProto() { + return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Empty()}; +} + +/** + * Static registration for the sni_cluster filter. @see RegisterFactory. + */ +static Registry::RegisterFactory + registered_; + +} // namespace SniCluster +} // namespace NetworkFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/network/sni_cluster/config.h b/source/extensions/filters/network/sni_cluster/config.h index 2c8f3aadae072..b4edcd51360b8 100644 --- a/source/extensions/filters/network/sni_cluster/config.h +++ b/source/extensions/filters/network/sni_cluster/config.h @@ -1,7 +1,7 @@ -#include "envoy/registry/registry.h" +#pragma once + #include "envoy/server/filter_config.h" -#include "extensions/filters/network/sni_cluster/sni_cluster.h" #include "extensions/filters/network/well_known_names.h" namespace Envoy { @@ -17,33 +17,14 @@ class SniClusterNetworkFilterConfigFactory public: // NamedNetworkFilterConfigFactory Network::FilterFactoryCb createFilterFactory(const Json::Object&, - Server::Configuration::FactoryContext&) override { - // Only used in v1 filters. - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - } - + Server::Configuration::FactoryContext&) override; Network::FilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message&, - Server::Configuration::FactoryContext&) override { - return [](Network::FilterManager& filter_manager) -> void { - filter_manager.addReadFilter(std::make_shared()); - }; - } - - ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Empty()}; - } - + Server::Configuration::FactoryContext&) override; + ProtobufTypes::MessagePtr createEmptyConfigProto() override; std::string name() override { return NetworkFilterNames::get().SniCluster; } }; -/** - * Static registration for the sni_cluster filter. @see RegisterFactory. - */ -static Registry::RegisterFactory - registered_; - } // namespace SniCluster } // namespace NetworkFilters } // namespace Extensions diff --git a/test/extensions/filters/network/sni_cluster/BUILD b/test/extensions/filters/network/sni_cluster/BUILD index a7d00cbff5193..03842452c55f3 100644 --- a/test/extensions/filters/network/sni_cluster/BUILD +++ b/test/extensions/filters/network/sni_cluster/BUILD @@ -17,7 +17,7 @@ envoy_extension_cc_test( srcs = ["sni_cluster_test.cc"], extension_name = "envoy.filters.network.sni_cluster", deps = [ - "//source/extensions/filters/network/sni_cluster", + "//source/extensions/filters/network/sni_cluster:sni_cluster", "//source/extensions/filters/network/sni_cluster:config", "//test/mocks/network:network_mocks", "//test/mocks/server:server_mocks", From 2e30879f48098dfa5227ad342f8320adbf87755e Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Fri, 21 Sep 2018 23:14:36 +0000 Subject: [PATCH 09/14] docs and formatting Signed-off-by: Shriram Rajagopalan --- .../network_filters/network_filters.rst | 1 + .../{sni_cluster.rst => sni_cluster_filter.rst} | 0 .../filters/network/sni_cluster/config.cc | 13 +++++++------ test/extensions/filters/network/sni_cluster/BUILD | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) rename docs/root/configuration/network_filters/{sni_cluster.rst => sni_cluster_filter.rst} (100%) diff --git a/docs/root/configuration/network_filters/network_filters.rst b/docs/root/configuration/network_filters/network_filters.rst index ed805b7de2d67..5ef34c7dd4724 100644 --- a/docs/root/configuration/network_filters/network_filters.rst +++ b/docs/root/configuration/network_filters/network_filters.rst @@ -19,3 +19,4 @@ filters. redis_proxy_filter tcp_proxy_filter thrift_proxy_filter + sni_cluster_filter diff --git a/docs/root/configuration/network_filters/sni_cluster.rst b/docs/root/configuration/network_filters/sni_cluster_filter.rst similarity index 100% rename from docs/root/configuration/network_filters/sni_cluster.rst rename to docs/root/configuration/network_filters/sni_cluster_filter.rst diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc index 96f43160c6ba3..a2c3fb0072f08 100644 --- a/source/extensions/filters/network/sni_cluster/config.cc +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -1,7 +1,8 @@ +#include "extensions/filters/network/sni_cluster/config.h" + #include "envoy/registry/registry.h" #include "envoy/server/filter_config.h" -#include "extensions/filters/network/sni_cluster/config.h" #include "extensions/filters/network/sni_cluster/sni_cluster.h" namespace Envoy { @@ -9,15 +10,15 @@ namespace Extensions { namespace NetworkFilters { namespace SniCluster { -Network::FilterFactoryCb SniClusterNetworkFilterConfigFactory::createFilterFactory(const Json::Object&, - Server::Configuration::FactoryContext&) { +Network::FilterFactoryCb +SniClusterNetworkFilterConfigFactory::createFilterFactory(const Json::Object&, + Server::Configuration::FactoryContext&) { // Only used in v1 filters. NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } -Network::FilterFactoryCb -SniClusterNetworkFilterConfigFactory::createFilterFactoryFromProto(const Protobuf::Message&, - Server::Configuration::FactoryContext&) { +Network::FilterFactoryCb SniClusterNetworkFilterConfigFactory::createFilterFactoryFromProto( + const Protobuf::Message&, Server::Configuration::FactoryContext&) { return [](Network::FilterManager& filter_manager) -> void { filter_manager.addReadFilter(std::make_shared()); }; diff --git a/test/extensions/filters/network/sni_cluster/BUILD b/test/extensions/filters/network/sni_cluster/BUILD index 03842452c55f3..a7d00cbff5193 100644 --- a/test/extensions/filters/network/sni_cluster/BUILD +++ b/test/extensions/filters/network/sni_cluster/BUILD @@ -17,7 +17,7 @@ envoy_extension_cc_test( srcs = ["sni_cluster_test.cc"], extension_name = "envoy.filters.network.sni_cluster", deps = [ - "//source/extensions/filters/network/sni_cluster:sni_cluster", + "//source/extensions/filters/network/sni_cluster", "//source/extensions/filters/network/sni_cluster:config", "//test/mocks/network:network_mocks", "//test/mocks/server:server_mocks", From f63b194b32080a88e5dd109edcdd285beb7e04e1 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 25 Sep 2018 16:41:24 +0000 Subject: [PATCH 10/14] formatting and renaming Signed-off-by: Shriram Rajagopalan --- api/envoy/api/v2/listener/listener.proto | 1 - source/common/tcp_proxy/tcp_proxy.cc | 12 ++++++------ source/common/tcp_proxy/tcp_proxy.h | 2 +- .../filters/network/sni_cluster/config.cc | 2 +- .../filters/network/sni_cluster/sni_cluster.cc | 4 ++-- test/common/tcp_proxy/tcp_proxy_test.cc | 5 ++--- .../filters/network/sni_cluster/sni_cluster_test.cc | 13 ++++++------- 7 files changed, 18 insertions(+), 21 deletions(-) diff --git a/api/envoy/api/v2/listener/listener.proto b/api/envoy/api/v2/listener/listener.proto index b9da6d6409a6f..1e8015dbb2446 100644 --- a/api/envoy/api/v2/listener/listener.proto +++ b/api/envoy/api/v2/listener/listener.proto @@ -31,7 +31,6 @@ message Filter { // * :ref:`envoy.ratelimit ` // * :ref:`envoy.redis_proxy ` // * :ref:`envoy.tcp_proxy ` - // * :ref:`envoy.sni_cluster ` string name = 1 [(validate.rules).string.min_bytes = 1]; // Filter specific configuration which depends on the filter being diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 407f3a177ffd9..8c353b2ec040f 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -22,7 +22,7 @@ namespace Envoy { namespace TcpProxy { -const std::string PerConnectionTcpProxyConfig::CLUSTER_KEY = "envoy.tcp_proxy.cluster"; +const std::string PerConnectionState::CLUSTER_KEY = "envoy.tcp_proxy.cluster"; Config::Route::Route( const envoy::config::filter::network::tcp_proxy::v2::TcpProxy::DeprecatedV1::TCPRoute& config) { @@ -111,11 +111,11 @@ Config::Config(const envoy::config::filter::network::tcp_proxy::v2::TcpProxy& co const std::string& Config::getRegularRouteFromEntries(Network::Connection& connection) { // First check if the per-connection state to see if we need to route to a pre-selected cluster - if (connection.perConnectionState().hasData( - PerConnectionTcpProxyConfig::CLUSTER_KEY)) { - const PerConnectionTcpProxyConfig& per_connection_config = - connection.perConnectionState().getData( - PerConnectionTcpProxyConfig::CLUSTER_KEY); + if (connection.perConnectionState().hasData( + PerConnectionState::CLUSTER_KEY)) { + const PerConnectionState& per_connection_config = + connection.perConnectionState().getData( + PerConnectionState::CLUSTER_KEY); return per_connection_config.cluster(); } diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 1098edde8b78d..acff4949aef31 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -158,7 +158,7 @@ typedef std::shared_ptr ConfigSharedPtr; /** * Per-connection TCP Proxy Filter configuration. */ -class PerConnectionTcpProxyConfig : public Envoy::RequestInfo::FilterState::Object { +class PerConnectionState : public RequestInfo::FilterState::Object { public: PerConnectionTcpProxyConfig(absl::string_view cluster) : cluster_(cluster) {} const std::string& cluster() const { return cluster_; } diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc index a2c3fb0072f08..38e32b1e065de 100644 --- a/source/extensions/filters/network/sni_cluster/config.cc +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -25,7 +25,7 @@ Network::FilterFactoryCb SniClusterNetworkFilterConfigFactory::createFilterFacto } ProtobufTypes::MessagePtr SniClusterNetworkFilterConfigFactory::createEmptyConfigProto() { - return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Empty()}; + return ProtobufTypes::MessagePtr{new ProtobufWkt::Empty()}; } /** diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc index 349c104e406c7..54219809e2fb7 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.cc +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -18,8 +18,8 @@ Network::FilterStatus SniClusterFilter::onNewConnection() { if (!sni.empty()) { // Set the tcp_proxy cluster to the same value as SNI read_callbacks_->connection().perConnectionState().setData( - Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY, - std::make_unique(sni)); + TcpProxy::PerConnectionState::CLUSTER_KEY, + std::make_unique(sni)); } return Network::FilterStatus::Continue; diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 7581c9747c868..df11136dd3813 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1147,9 +1147,8 @@ TEST_F(TcpProxyRoutingTest, UseClusterFromPerConnectionState) { setup(); Envoy::RequestInfo::FilterStateImpl per_connection_state; - per_connection_state.setData( - "envoy.tcp_proxy.cluster", - std::make_unique("filter_state_cluster")); + per_connection_state.setData("envoy.tcp_proxy.cluster", + std::make_unique("filter_state_cluster")); ON_CALL(connection_, perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); EXPECT_CALL(Const(connection_), perConnectionState()) .WillRepeatedly(ReturnRef(per_connection_state)); diff --git a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc index b5cffc64565f0..b9d4a938ea379 100644 --- a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc +++ b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc @@ -51,8 +51,8 @@ TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { .WillByDefault(Return(EMPTY_STRING)); filter.onNewConnection(); - EXPECT_FALSE(per_connection_state.hasData( - Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY)); + EXPECT_FALSE(per_connection_state.hasData( + Envoy::TcpProxy::PerConnectionState::CLUSTER_KEY)); } // with sni @@ -61,12 +61,11 @@ TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { .WillByDefault(Return("filter_state_cluster")); filter.onNewConnection(); - EXPECT_TRUE(per_connection_state.hasData( - Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY)); + EXPECT_TRUE(per_connection_state.hasData( + Envoy::TcpProxy::PerConnectionState::CLUSTER_KEY)); - auto per_connection_config = - per_connection_state.getData( - Envoy::TcpProxy::PerConnectionTcpProxyConfig::CLUSTER_KEY); + auto per_connection_config = per_connection_state.getData( + Envoy::TcpProxy::PerConnectionState::CLUSTER_KEY); EXPECT_EQ(per_connection_config.cluster(), "filter_state_cluster"); } } From f99d1751a4ec68ada4d98dbac277a57cb5b2824e Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 25 Sep 2018 16:44:58 +0000 Subject: [PATCH 11/14] codeowners Signed-off-by: Shriram Rajagopalan --- CODEOWNERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CODEOWNERS b/CODEOWNERS index 631ab8e8b5afa..c88f01af905c5 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -10,3 +10,5 @@ /*/extensions/filters/http/header_to_metadata @rgs1 @zuercher # alts transport socket extension /*/extensions/transport_sockets/alts @lizan @yangminzhu +# sni_cluster extension +/*/extensions/filters/network/sni_cluster @rshriram @lizan From 8dd96efcd6f764b8e0e034131d1da1899cc597bb Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 25 Sep 2018 17:42:02 +0000 Subject: [PATCH 12/14] assorted fixes Signed-off-by: Shriram Rajagopalan --- .../network_filters/tcp_proxy_filter.rst | 10 ++++++++++ source/common/tcp_proxy/tcp_proxy.cc | 12 +++++------- source/common/tcp_proxy/tcp_proxy.h | 12 ++++++------ .../filters/network/sni_cluster/sni_cluster.cc | 3 +-- test/common/tcp_proxy/tcp_proxy_test.cc | 6 +++--- .../network/sni_cluster/sni_cluster_test.cc | 16 ++++++++-------- 6 files changed, 33 insertions(+), 26 deletions(-) diff --git a/docs/root/configuration/network_filters/tcp_proxy_filter.rst b/docs/root/configuration/network_filters/tcp_proxy_filter.rst index d20a44408d823..bddad5460549e 100644 --- a/docs/root/configuration/network_filters/tcp_proxy_filter.rst +++ b/docs/root/configuration/network_filters/tcp_proxy_filter.rst @@ -7,6 +7,16 @@ TCP proxy * :ref:`v1 API reference ` * :ref:`v2 API reference ` +.. _config_network_filters_tcp_proxy_dynamic_cluster: + +Dynamic cluster selection +------------------------- + +The upstream cluster used by the TCP proxy filter can be dynamically set by +other network filters on a per-connection basis by setting a per-connection +state object under the key `envoy.tcp_proxy.cluster`. See the +implementation for the details. + .. _config_network_filters_tcp_proxy_stats: Statistics diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 8c353b2ec040f..320e1c058fd90 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -22,7 +22,7 @@ namespace Envoy { namespace TcpProxy { -const std::string PerConnectionState::CLUSTER_KEY = "envoy.tcp_proxy.cluster"; +const std::string PerConnectionCluster::KEY = "envoy.tcp_proxy.cluster"; Config::Route::Route( const envoy::config::filter::network::tcp_proxy::v2::TcpProxy::DeprecatedV1::TCPRoute& config) { @@ -111,12 +111,10 @@ Config::Config(const envoy::config::filter::network::tcp_proxy::v2::TcpProxy& co const std::string& Config::getRegularRouteFromEntries(Network::Connection& connection) { // First check if the per-connection state to see if we need to route to a pre-selected cluster - if (connection.perConnectionState().hasData( - PerConnectionState::CLUSTER_KEY)) { - const PerConnectionState& per_connection_config = - connection.perConnectionState().getData( - PerConnectionState::CLUSTER_KEY); - return per_connection_config.cluster(); + if (connection.perConnectionState().hasData(PerConnectionCluster::KEY)) { + const PerConnectionCluster& per_connection_cluster = + connection.perConnectionState().getData(PerConnectionCluster::KEY); + return per_connection_cluster.value(); } for (const Config::Route& route : routes_) { diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index acff4949aef31..ca8dc7b9bf67f 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -156,16 +156,16 @@ class Config { typedef std::shared_ptr ConfigSharedPtr; /** - * Per-connection TCP Proxy Filter configuration. + * Per-connection TCP Proxy Cluster configuration. */ -class PerConnectionState : public RequestInfo::FilterState::Object { +class PerConnectionCluster : public RequestInfo::FilterState::Object { public: - PerConnectionTcpProxyConfig(absl::string_view cluster) : cluster_(cluster) {} - const std::string& cluster() const { return cluster_; } - static const std::string CLUSTER_KEY; + PerConnectionCluster(absl::string_view cluster) : cluster_(cluster) {} + const std::string& value() const { return cluster_; } + static const std::string KEY; private: - std::string cluster_; + const std::string cluster_; }; /** diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc index 54219809e2fb7..93363aba951ad 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.cc +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -18,8 +18,7 @@ Network::FilterStatus SniClusterFilter::onNewConnection() { if (!sni.empty()) { // Set the tcp_proxy cluster to the same value as SNI read_callbacks_->connection().perConnectionState().setData( - TcpProxy::PerConnectionState::CLUSTER_KEY, - std::make_unique(sni)); + TcpProxy::PerConnectionCluster::KEY, std::make_unique(sni)); } return Network::FilterStatus::Continue; diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index df11136dd3813..26edf3a1da3b9 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -1143,12 +1143,12 @@ TEST_F(TcpProxyRoutingTest, RoutableConnection) { } // Test that the tcp proxy uses the cluster from FilterState if set -TEST_F(TcpProxyRoutingTest, UseClusterFromPerConnectionState) { +TEST_F(TcpProxyRoutingTest, UseClusterFromPerConnectionCluster) { setup(); - Envoy::RequestInfo::FilterStateImpl per_connection_state; + RequestInfo::FilterStateImpl per_connection_state; per_connection_state.setData("envoy.tcp_proxy.cluster", - std::make_unique("filter_state_cluster")); + std::make_unique("filter_state_cluster")); ON_CALL(connection_, perConnectionState()).WillByDefault(ReturnRef(per_connection_state)); EXPECT_CALL(Const(connection_), perConnectionState()) .WillRepeatedly(ReturnRef(per_connection_state)); diff --git a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc index b9d4a938ea379..f9a93370b087c 100644 --- a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc +++ b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc @@ -36,7 +36,7 @@ TEST(SniCluster, ConfigTest) { TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { NiceMock filter_callbacks; - Envoy::RequestInfo::FilterStateImpl per_connection_state; + RequestInfo::FilterStateImpl per_connection_state; ON_CALL(filter_callbacks.connection_, perConnectionState()) .WillByDefault(ReturnRef(per_connection_state)); ON_CALL(Const(filter_callbacks.connection_), perConnectionState()) @@ -51,8 +51,8 @@ TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { .WillByDefault(Return(EMPTY_STRING)); filter.onNewConnection(); - EXPECT_FALSE(per_connection_state.hasData( - Envoy::TcpProxy::PerConnectionState::CLUSTER_KEY)); + EXPECT_FALSE(per_connection_state.hasData( + TcpProxy::PerConnectionCluster::KEY)); } // with sni @@ -61,12 +61,12 @@ TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { .WillByDefault(Return("filter_state_cluster")); filter.onNewConnection(); - EXPECT_TRUE(per_connection_state.hasData( - Envoy::TcpProxy::PerConnectionState::CLUSTER_KEY)); + EXPECT_TRUE(per_connection_state.hasData( + TcpProxy::PerConnectionCluster::KEY)); - auto per_connection_config = per_connection_state.getData( - Envoy::TcpProxy::PerConnectionState::CLUSTER_KEY); - EXPECT_EQ(per_connection_config.cluster(), "filter_state_cluster"); + auto per_connection_cluster = per_connection_state.getData( + TcpProxy::PerConnectionCluster::KEY); + EXPECT_EQ(per_connection_cluster.value(), "filter_state_cluster"); } } From e49e40d821925204de2d73c8387c27ade346738d Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 25 Sep 2018 18:21:27 +0000 Subject: [PATCH 13/14] nits Signed-off-by: Shriram Rajagopalan --- source/common/tcp_proxy/tcp_proxy.cc | 6 +++--- source/common/tcp_proxy/tcp_proxy.h | 2 +- source/extensions/filters/network/sni_cluster/config.cc | 2 +- .../extensions/filters/network/sni_cluster/sni_cluster.cc | 2 +- .../filters/network/sni_cluster/sni_cluster_test.cc | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 320e1c058fd90..c44e35c85c944 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -22,7 +22,7 @@ namespace Envoy { namespace TcpProxy { -const std::string PerConnectionCluster::KEY = "envoy.tcp_proxy.cluster"; +const std::string PerConnectionCluster::Key = "envoy.tcp_proxy.cluster"; Config::Route::Route( const envoy::config::filter::network::tcp_proxy::v2::TcpProxy::DeprecatedV1::TCPRoute& config) { @@ -111,9 +111,9 @@ Config::Config(const envoy::config::filter::network::tcp_proxy::v2::TcpProxy& co const std::string& Config::getRegularRouteFromEntries(Network::Connection& connection) { // First check if the per-connection state to see if we need to route to a pre-selected cluster - if (connection.perConnectionState().hasData(PerConnectionCluster::KEY)) { + if (connection.perConnectionState().hasData(PerConnectionCluster::Key)) { const PerConnectionCluster& per_connection_cluster = - connection.perConnectionState().getData(PerConnectionCluster::KEY); + connection.perConnectionState().getData(PerConnectionCluster::Key); return per_connection_cluster.value(); } diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index ca8dc7b9bf67f..e749400fe1b74 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -162,7 +162,7 @@ class PerConnectionCluster : public RequestInfo::FilterState::Object { public: PerConnectionCluster(absl::string_view cluster) : cluster_(cluster) {} const std::string& value() const { return cluster_; } - static const std::string KEY; + static const std::string Key; private: const std::string cluster_; diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc index 38e32b1e065de..7c8da26cf1394 100644 --- a/source/extensions/filters/network/sni_cluster/config.cc +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -25,7 +25,7 @@ Network::FilterFactoryCb SniClusterNetworkFilterConfigFactory::createFilterFacto } ProtobufTypes::MessagePtr SniClusterNetworkFilterConfigFactory::createEmptyConfigProto() { - return ProtobufTypes::MessagePtr{new ProtobufWkt::Empty()}; + return std::make_unique(new ProtobufWkt::Empty()); } /** diff --git a/source/extensions/filters/network/sni_cluster/sni_cluster.cc b/source/extensions/filters/network/sni_cluster/sni_cluster.cc index 93363aba951ad..f5acb2b917b48 100644 --- a/source/extensions/filters/network/sni_cluster/sni_cluster.cc +++ b/source/extensions/filters/network/sni_cluster/sni_cluster.cc @@ -18,7 +18,7 @@ Network::FilterStatus SniClusterFilter::onNewConnection() { if (!sni.empty()) { // Set the tcp_proxy cluster to the same value as SNI read_callbacks_->connection().perConnectionState().setData( - TcpProxy::PerConnectionCluster::KEY, std::make_unique(sni)); + TcpProxy::PerConnectionCluster::Key, std::make_unique(sni)); } return Network::FilterStatus::Continue; diff --git a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc index f9a93370b087c..4d833d378efe1 100644 --- a/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc +++ b/test/extensions/filters/network/sni_cluster/sni_cluster_test.cc @@ -52,7 +52,7 @@ TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { filter.onNewConnection(); EXPECT_FALSE(per_connection_state.hasData( - TcpProxy::PerConnectionCluster::KEY)); + TcpProxy::PerConnectionCluster::Key)); } // with sni @@ -62,10 +62,10 @@ TEST(SniCluster, SetTcpProxyClusterOnlyIfSniIsPresent) { filter.onNewConnection(); EXPECT_TRUE(per_connection_state.hasData( - TcpProxy::PerConnectionCluster::KEY)); + TcpProxy::PerConnectionCluster::Key)); auto per_connection_cluster = per_connection_state.getData( - TcpProxy::PerConnectionCluster::KEY); + TcpProxy::PerConnectionCluster::Key); EXPECT_EQ(per_connection_cluster.value(), "filter_state_cluster"); } } From e2254c0468457e746e9b4660153665d4fc229107 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 25 Sep 2018 18:31:38 +0000 Subject: [PATCH 14/14] std::unique Signed-off-by: Shriram Rajagopalan --- source/extensions/filters/network/sni_cluster/config.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/network/sni_cluster/config.cc b/source/extensions/filters/network/sni_cluster/config.cc index 7c8da26cf1394..8d7b6eba067a7 100644 --- a/source/extensions/filters/network/sni_cluster/config.cc +++ b/source/extensions/filters/network/sni_cluster/config.cc @@ -25,7 +25,7 @@ Network::FilterFactoryCb SniClusterNetworkFilterConfigFactory::createFilterFacto } ProtobufTypes::MessagePtr SniClusterNetworkFilterConfigFactory::createEmptyConfigProto() { - return std::make_unique(new ProtobufWkt::Empty()); + return std::make_unique(); } /**