From 39cdfc55800c426f82443c05729a79e910d342c1 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 11 Jun 2020 15:58:23 -0400 Subject: [PATCH 1/3] upstreams: in place factories Signed-off-by: Alyssa Wilk --- include/envoy/upstream/upstream.h | 7 ++++ source/common/router/BUILD | 1 + source/common/router/router.cc | 23 ++++++----- source/common/upstream/upstream_impl.cc | 7 +++- source/common/upstream/upstream_impl.h | 5 +++ source/extensions/upstreams/http/BUILD | 17 ++++++++ .../extensions/upstreams/http/generic/BUILD | 9 ++++- .../upstreams/http/generic/config.cc | 28 +++++++++++++ .../upstreams/http/generic/config.h | 39 +++++++++++++++++++ source/extensions/upstreams/http/http/BUILD | 9 ++++- .../extensions/upstreams/http/http/config.cc | 24 ++++++++++++ .../extensions/upstreams/http/http/config.h | 39 +++++++++++++++++++ source/extensions/upstreams/http/tcp/BUILD | 9 ++++- .../extensions/upstreams/http/tcp/config.cc | 24 ++++++++++++ source/extensions/upstreams/http/tcp/config.h | 37 ++++++++++++++++++ .../upstreams/http/well_known_names.h | 35 +++++++++++++++++ test/common/router/BUILD | 6 +++ test/common/router/router_test.cc | 31 +++++++++++++++ test/mocks/upstream/cluster_info.cc | 1 + test/mocks/upstream/cluster_info.h | 3 ++ 20 files changed, 335 insertions(+), 19 deletions(-) create mode 100644 source/extensions/upstreams/http/BUILD create mode 100644 source/extensions/upstreams/http/generic/config.cc create mode 100644 source/extensions/upstreams/http/generic/config.h create mode 100644 source/extensions/upstreams/http/http/config.cc create mode 100644 source/extensions/upstreams/http/http/config.h create mode 100644 source/extensions/upstreams/http/tcp/config.cc create mode 100644 source/extensions/upstreams/http/tcp/config.h create mode 100644 source/extensions/upstreams/http/well_known_names.h diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index dbb89c88be670..139eb8ebb3a3f 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -789,6 +789,13 @@ class ClusterInfo { virtual const absl::optional& lbOriginalDstConfig() const PURE; + /** + * @return const absl::optional& the configuration + * for the upstream, if a custom upstream is configured. + */ + virtual const absl::optional& + upstreamConfig() const PURE; + /** * @return Whether the cluster is currently in maintenance mode and should not be routed to. * Different filters may handle this situation in different ways. The implementation diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 610cb0f10d75f..3ef1c7ab7ee1a 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -311,6 +311,7 @@ envoy_cc_library( "//source/common/tracing:http_tracer_lib", "//source/common/upstream:load_balancer_lib", "//source/extensions/common/proxy_protocol:proxy_protocol_header_lib", + "//source/extensions/upstreams/http:well_known_names", "@envoy_api//envoy/extensions/filters/http/router/v3:pkg_cc_proto", ], ) diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 5ab68cc2d17ce..6c91fb6c175dc 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -40,6 +40,7 @@ #include "common/tracing/http_tracer_impl.h" #include "extensions/filters/http/well_known_names.h" +#include "extensions/upstreams/http/well_known_names.h" namespace Envoy { namespace Router { @@ -597,22 +598,20 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } std::unique_ptr Filter::createConnPool() { + GenericConnPoolFactory* factory = nullptr; + if (cluster_->upstreamConfig().has_value()) { + factory = &Envoy::Config::Utility::getAndCheckFactory( + cluster_->upstreamConfig().value()); + } else { + factory = &Envoy::Config::Utility::getAndCheckFactoryByName( + Extensions::Upstreams::Http::HttpConnectionPoolNames::get().Generic); + } const bool should_tcp_proxy = route_entry_->connectConfig().has_value() && downstream_headers_->getMethodValue() == Http::Headers::get().MethodValues.Connect; Http::Protocol protocol = cluster_->upstreamHttpProtocol(callbacks_->streamInfo().protocol()); - if (should_tcp_proxy) { - auto pool = std::make_unique(config_.cm_, *route_entry_, protocol, this); - if (pool->valid()) { - return pool; - } - } else { - auto pool = std::make_unique(config_.cm_, *route_entry_, protocol, this); - if (pool->valid()) { - return pool; - } - } - return nullptr; + return factory->createGenericConnPool(config_.cm_, should_tcp_proxy, *route_entry_, protocol, + this); } void Filter::sendNoHealthyUpstreamResponse() { diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index a78a331cb9cc0..a11e06f2039e5 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -694,7 +694,12 @@ ClusterInfoImpl::ClusterInfoImpl( source_address_(getSourceAddress(config, bind_config)), lb_least_request_config_(config.least_request_lb_config()), lb_ring_hash_config_(config.ring_hash_lb_config()), - lb_original_dst_config_(config.original_dst_lb_config()), added_via_api_(added_via_api), + lb_original_dst_config_(config.original_dst_lb_config()), + upstream_config_(config.has_upstream_config() + ? absl::make_optional( + config.upstream_config()) + : absl::nullopt), + added_via_api_(added_via_api), lb_subset_(LoadBalancerSubsetInfoImpl(config.lb_subset_config())), metadata_(config.metadata()), typed_metadata_(config.metadata()), common_lb_config_(config.common_lb_config()), diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 722aa5e5dcc33..372716d078949 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -565,6 +565,10 @@ class ClusterInfoImpl : public ClusterInfo, protected Logger::Loggable& + upstreamConfig() const override { + return upstream_config_; + } bool maintenanceMode() const override; uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } uint32_t maxResponseHeadersCount() const override { return max_response_headers_count_; } @@ -645,6 +649,7 @@ class ClusterInfoImpl : public ClusterInfo, protected Logger::Loggable lb_ring_hash_config_; absl::optional lb_original_dst_config_; + absl::optional upstream_config_; const bool added_via_api_; LoadBalancerSubsetInfoImpl lb_subset_; const envoy::config::core::v3::Metadata metadata_; diff --git a/source/extensions/upstreams/http/BUILD b/source/extensions/upstreams/http/BUILD new file mode 100644 index 0000000000000..06456dbbcb5e7 --- /dev/null +++ b/source/extensions/upstreams/http/BUILD @@ -0,0 +1,17 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_library( + name = "well_known_names", + hdrs = ["well_known_names.h"], + deps = [ + "//source/common/singleton:const_singleton", + ], +) diff --git a/source/extensions/upstreams/http/generic/BUILD b/source/extensions/upstreams/http/generic/BUILD index 7aa32b16a2199..f61d9801103f6 100644 --- a/source/extensions/upstreams/http/generic/BUILD +++ b/source/extensions/upstreams/http/generic/BUILD @@ -1,5 +1,3 @@ -# placeholder build files for security_posture -# Will be filled in as #11327 lands. load( "//bazel:envoy_build_system.bzl", "envoy_cc_extension", @@ -13,8 +11,15 @@ envoy_package() envoy_cc_extension( name = "config", srcs = [ + "config.cc", ], hdrs = [ + "config.h", ], security_posture = "robust_to_untrusted_downstream", + visibility = ["//visibility:public"], + deps = [ + "//source/common/router:router_lib", + "@envoy_api//envoy/extensions/upstreams/http/generic/v3:pkg_cc_proto", + ], ) diff --git a/source/extensions/upstreams/http/generic/config.cc b/source/extensions/upstreams/http/generic/config.cc new file mode 100644 index 0000000000000..f3057ccb8561d --- /dev/null +++ b/source/extensions/upstreams/http/generic/config.cc @@ -0,0 +1,28 @@ +#include "extensions/upstreams/http/generic/config.h" + +#include "common/router/upstream_request.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { +namespace Generic { + +Router::GenericConnPoolPtr GenericGenericConnPoolFactory::createGenericConnPool( + Upstream::ClusterManager& cm, bool is_connect, const Router::RouteEntry& route_entry, + Envoy::Http::Protocol protocol, Upstream::LoadBalancerContext* ctx) const { + if (is_connect) { + auto ret = std::make_unique(cm, route_entry, protocol, ctx); + return (ret->valid() ? std::move(ret) : nullptr); + } + auto ret = std::make_unique(cm, route_entry, protocol, ctx); + return (ret->valid() ? std::move(ret) : nullptr); +} + +REGISTER_FACTORY(GenericGenericConnPoolFactory, Router::GenericConnPoolFactory); + +} // namespace Generic +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/upstreams/http/generic/config.h b/source/extensions/upstreams/http/generic/config.h new file mode 100644 index 0000000000000..384697c297005 --- /dev/null +++ b/source/extensions/upstreams/http/generic/config.h @@ -0,0 +1,39 @@ +#pragma once + +#include "envoy/extensions/upstreams/http/generic/v3/generic_connection_pool.pb.h" +#include "envoy/registry/registry.h" +#include "envoy/router/router.h" + +#include "extensions/upstreams/http/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { +namespace Generic { + +/** + * Config registration for the GenericConnPool. * @see Router::GenericConnPoolFactory + */ +class GenericGenericConnPoolFactory : public Router::GenericConnPoolFactory { +public: + std::string name() const override { return HttpConnectionPoolNames::get().Generic; } + std::string category() const override { return "envoy.upstreams"; } + Router::GenericConnPoolPtr + createGenericConnPool(Upstream::ClusterManager& cm, bool is_connect, + const Router::RouteEntry& route_entry, Envoy::Http::Protocol protocol, + Upstream::LoadBalancerContext* ctx) const override; + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::upstreams::http::generic::v3::GenericConnectionPoolProto>(); + } +}; + +DECLARE_FACTORY(GenericGenericConnPoolFactory); + +} // namespace Generic +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/upstreams/http/http/BUILD b/source/extensions/upstreams/http/http/BUILD index 7aa32b16a2199..caf8f766e6f34 100644 --- a/source/extensions/upstreams/http/http/BUILD +++ b/source/extensions/upstreams/http/http/BUILD @@ -1,5 +1,3 @@ -# placeholder build files for security_posture -# Will be filled in as #11327 lands. load( "//bazel:envoy_build_system.bzl", "envoy_cc_extension", @@ -13,8 +11,15 @@ envoy_package() envoy_cc_extension( name = "config", srcs = [ + "config.cc", ], hdrs = [ + "config.h", ], security_posture = "robust_to_untrusted_downstream", + visibility = ["//visibility:public"], + deps = [ + "//source/common/router:router_lib", + "@envoy_api//envoy/extensions/upstreams/http/http/v3:pkg_cc_proto", + ], ) diff --git a/source/extensions/upstreams/http/http/config.cc b/source/extensions/upstreams/http/http/config.cc new file mode 100644 index 0000000000000..a257b7e39b378 --- /dev/null +++ b/source/extensions/upstreams/http/http/config.cc @@ -0,0 +1,24 @@ +#include "extensions/upstreams/http/http/config.h" + +#include "common/router/upstream_request.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { +namespace Http { + +Router::GenericConnPoolPtr HttpGenericConnPoolFactory::createGenericConnPool( + Upstream::ClusterManager& cm, bool, const Router::RouteEntry& route_entry, + Envoy::Http::Protocol protocol, Upstream::LoadBalancerContext* ctx) const { + auto ret = std::make_unique(cm, route_entry, protocol, ctx); + return (ret->valid() ? std::move(ret) : nullptr); +} + +REGISTER_FACTORY(HttpGenericConnPoolFactory, Router::GenericConnPoolFactory); + +} // namespace Http +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/upstreams/http/http/config.h b/source/extensions/upstreams/http/http/config.h new file mode 100644 index 0000000000000..0ae4fbc2eac67 --- /dev/null +++ b/source/extensions/upstreams/http/http/config.h @@ -0,0 +1,39 @@ +#pragma once + +#include "envoy/extensions/upstreams/http/http/v3/http_connection_pool.pb.h" +#include "envoy/registry/registry.h" +#include "envoy/router/router.h" + +#include "extensions/upstreams/http/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { +namespace Http { + +/** + * Config registration for the HttpConnPool. @see Router::GenericConnPoolFactory + */ +class HttpGenericConnPoolFactory : public Router::GenericConnPoolFactory { +public: + std::string name() const override { return HttpConnectionPoolNames::get().Http; } + std::string category() const override { return "envoy.upstreams"; } + Router::GenericConnPoolPtr + createGenericConnPool(Upstream::ClusterManager& cm, bool is_connect, + const Router::RouteEntry& route_entry, Envoy::Http::Protocol protocol, + Upstream::LoadBalancerContext* ctx) const override; + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::upstreams::http::http::v3::HttpConnectionPoolProto>(); + } +}; + +DECLARE_FACTORY(HttpGenericConnPoolFactory); + +} // namespace Http +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/upstreams/http/tcp/BUILD b/source/extensions/upstreams/http/tcp/BUILD index 7aa32b16a2199..960f8b4b9c0d4 100644 --- a/source/extensions/upstreams/http/tcp/BUILD +++ b/source/extensions/upstreams/http/tcp/BUILD @@ -1,5 +1,3 @@ -# placeholder build files for security_posture -# Will be filled in as #11327 lands. load( "//bazel:envoy_build_system.bzl", "envoy_cc_extension", @@ -13,8 +11,15 @@ envoy_package() envoy_cc_extension( name = "config", srcs = [ + "config.cc", ], hdrs = [ + "config.h", ], security_posture = "robust_to_untrusted_downstream", + visibility = ["//visibility:public"], + deps = [ + "//source/common/router:router_lib", + "@envoy_api//envoy/extensions/upstreams/http/tcp/v3:pkg_cc_proto", + ], ) diff --git a/source/extensions/upstreams/http/tcp/config.cc b/source/extensions/upstreams/http/tcp/config.cc new file mode 100644 index 0000000000000..ffd0412b643eb --- /dev/null +++ b/source/extensions/upstreams/http/tcp/config.cc @@ -0,0 +1,24 @@ +#include "extensions/upstreams/http/tcp/config.h" + +#include "common/router/upstream_request.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { +namespace Tcp { + +Router::GenericConnPoolPtr TcpGenericConnPoolFactory::createGenericConnPool( + Upstream::ClusterManager& cm, bool, const Router::RouteEntry& route_entry, + Envoy::Http::Protocol protocol, Upstream::LoadBalancerContext* ctx) const { + auto ret = std::make_unique(cm, route_entry, protocol, ctx); + return (ret->valid() ? std::move(ret) : nullptr); +} + +REGISTER_FACTORY(TcpGenericConnPoolFactory, Router::GenericConnPoolFactory); + +} // namespace Tcp +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/upstreams/http/tcp/config.h b/source/extensions/upstreams/http/tcp/config.h new file mode 100644 index 0000000000000..37aae7be6597c --- /dev/null +++ b/source/extensions/upstreams/http/tcp/config.h @@ -0,0 +1,37 @@ +#pragma once + +#include "envoy/extensions/upstreams/http/tcp/v3/tcp_connection_pool.pb.h" +#include "envoy/registry/registry.h" +#include "envoy/router/router.h" + +#include "extensions/upstreams/http/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { +namespace Tcp { + +/** + * Config registration for the TcpConnPool. @see Router::GenericConnPoolFactory + */ +class TcpGenericConnPoolFactory : public Router::GenericConnPoolFactory { +public: + std::string name() const override { return HttpConnectionPoolNames::get().Tcp; } + std::string category() const override { return "envoy.upstreams"; } + Router::GenericConnPoolPtr + createGenericConnPool(Upstream::ClusterManager& cm, bool is_connect, + const Router::RouteEntry& route_entry, Envoy::Http::Protocol protocol, + Upstream::LoadBalancerContext* ctx) const override; + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } +}; + +DECLARE_FACTORY(TcpGenericConnPoolFactory); + +} // namespace Tcp +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/upstreams/http/well_known_names.h b/source/extensions/upstreams/http/well_known_names.h new file mode 100644 index 0000000000000..df65dd23531bd --- /dev/null +++ b/source/extensions/upstreams/http/well_known_names.h @@ -0,0 +1,35 @@ +#pragma once + +#include + +#include "common/singleton/const_singleton.h" + +namespace Envoy { +namespace Extensions { +namespace Upstreams { +namespace Http { + +/** + * Well-known http connection pool types. + * NOTE: New connection pools should use the well known name: + * envoy.filters.connection_pools.http.name + */ +class HttpConnectionPoolNameValues { +public: + // The normal case: sending HTTP traffic over an HTTP connection. + const std::string Http = "envoy.filters.connection_pools.http.http"; + + // The "CONNECT" upstream: sending CONNECT payload over a TCP connection. + const std::string Tcp = "envoy.filters.connection_pools.http.tcp"; + + // The default upstream, returning TCP upstream for CONNECT requests, HTTP for non-CONNECT + // requests. + const std::string Generic = "envoy.filters.connection_pools.http.generic"; +}; + +using HttpConnectionPoolNames = ConstSingleton; + +} // namespace Http +} // namespace Upstreams +} // namespace Extensions +} // namespace Envoy diff --git a/test/common/router/BUILD b/test/common/router/BUILD index d1c5b7c036ed1..81a4b7e7a63b9 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -262,6 +262,9 @@ envoy_cc_test( "//source/common/stream_info:uint32_accessor_lib", "//source/common/upstream:upstream_includes", "//source/common/upstream:upstream_lib", + "//source/extensions/upstreams/http/generic:config", + "//source/extensions/upstreams/http/http:config", + "//source/extensions/upstreams/http/tcp:config", "//test/common/http:common_lib", "//test/mocks/http:http_mocks", "//test/mocks/local_info:local_info_mocks", @@ -276,6 +279,8 @@ envoy_cc_test( "//test/test_common:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/upstreams/http/http/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/upstreams/http/tcp/v3:pkg_cc_proto", "@envoy_api//envoy/type/v3:pkg_cc_proto", ], ) @@ -292,6 +297,7 @@ envoy_cc_test( "//source/common/upstream:upstream_includes", "//source/common/upstream:upstream_lib", "//source/extensions/access_loggers/file:config", + "//source/extensions/upstreams/http/generic:config", "//test/common/http:common_lib", "//test/mocks/access_log:access_log_mocks", "//test/mocks/filesystem:filesystem_mocks", diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index c0bd7deb1ee9f..ef860a231f879 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -5,6 +5,8 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/extensions/transport_sockets/tls/v3/cert.pb.h" +#include "envoy/extensions/upstreams/http/http/v3/http_connection_pool.pb.h" +#include "envoy/extensions/upstreams/http/tcp/v3/tcp_connection_pool.pb.h" #include "envoy/type/v3/percent.pb.h" #include "common/buffer/buffer_impl.h" @@ -5932,6 +5934,14 @@ TEST_F(RouterTest, ConnectPauseAndResume) { // Verify that CONNECT payload is not sent upstream if non-200 response headers are received. TEST_F(RouterTest, ConnectPauseNoResume) { + // Explicitly configure an HTTP upstream, to test factory creation. + cm_.thread_local_cluster_.cluster_.info_->upstream_config_ = + absl::make_optional(); + envoy::extensions::upstreams::http::http::v3::HttpConnectionPoolProto http_config; + cm_.thread_local_cluster_.cluster_.info_->upstream_config_.value() + .mutable_typed_config() + ->PackFrom(http_config); + NiceMock encoder; Http::ResponseDecoder* response_decoder = nullptr; EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) @@ -5962,6 +5972,27 @@ TEST_F(RouterTest, ConnectPauseNoResume) { response_decoder->decodeHeaders(std::move(response_headers), true); } +TEST_F(RouterTest, ConnectExplicitTcpUpstream) { + // Explicitly configure an TCP upstream, to test factory creation. + cm_.thread_local_cluster_.cluster_.info_->upstream_config_ = + absl::make_optional(); + envoy::extensions::upstreams::http::tcp::v3::TcpConnectionPoolProto tcp_config; + cm_.thread_local_cluster_.cluster_.info_->upstream_config_.value() + .mutable_typed_config() + ->PackFrom(tcp_config); + callbacks_.route_->route_entry_.connect_config_ = + absl::make_optional(); + + // Make sure newConnection is called on the TCP pool, not newStream on the HTTP pool. + EXPECT_CALL(cm_.tcp_conn_pool_, newConnection(_)); + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + headers.setMethod("CONNECT"); + router_.decodeHeaders(headers, false); + + router_.onDestroy(); +} + class WatermarkTest : public RouterTest { public: void sendRequest(bool header_only_request = true, bool pool_ready = true) { diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index dd428aa124c28..3a2c1aa8af8df 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -81,6 +81,7 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, lbSubsetInfo()).WillByDefault(ReturnRef(lb_subset_)); ON_CALL(*this, lbRingHashConfig()).WillByDefault(ReturnRef(lb_ring_hash_config_)); ON_CALL(*this, lbOriginalDstConfig()).WillByDefault(ReturnRef(lb_original_dst_config_)); + ON_CALL(*this, upstreamConfig()).WillByDefault(ReturnRef(upstream_config_)); ON_CALL(*this, lbConfig()).WillByDefault(ReturnRef(lb_config_)); ON_CALL(*this, clusterSocketOptions()).WillByDefault(ReturnRef(cluster_socket_options_)); ON_CALL(*this, metadata()).WillByDefault(ReturnRef(metadata_)); diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 2e99eea091ef9..e3bce01c62826 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -108,6 +108,8 @@ class MockClusterInfo : public ClusterInfo { lbLeastRequestConfig, (), (const)); MOCK_METHOD(const absl::optional&, lbOriginalDstConfig, (), (const)); + MOCK_METHOD(const absl::optional&, upstreamConfig, + (), (const)); MOCK_METHOD(bool, maintenanceMode, (), (const)); MOCK_METHOD(uint32_t, maxResponseHeadersCount, (), (const)); MOCK_METHOD(uint64_t, maxRequestsPerConnection, (), (const)); @@ -163,6 +165,7 @@ class MockClusterInfo : public ClusterInfo { upstream_http_protocol_options_; absl::optional lb_ring_hash_config_; absl::optional lb_original_dst_config_; + absl::optional upstream_config_; Network::ConnectionSocket::OptionsSharedPtr cluster_socket_options_; envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config_; envoy::config::core::v3::Metadata metadata_; From 5582d2e6a01ef4e3bb495deedcb47b84534518ba Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Thu, 11 Jun 2020 16:40:30 -0400 Subject: [PATCH 2/3] stats integration test PR Signed-off-by: Alyssa Wilk --- source/server/BUILD | 3 +++ test/common/http/BUILD | 1 + test/integration/stats_integration_test.cc | 6 ++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/source/server/BUILD b/source/server/BUILD index e927018621887..b1d5ce852b964 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -79,6 +79,9 @@ envoy_cc_library( "//source/common/stats:timespan_lib", "//source/common/stream_info:stream_info_lib", "//source/extensions/transport_sockets:well_known_names", + "//source/extensions/upstreams/http/generic:config", + "//source/extensions/upstreams/http/http:config", + "//source/extensions/upstreams/http/tcp:config", ], ) diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 5a42fb54f59bc..e723a48abeb01 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -23,6 +23,7 @@ envoy_cc_test( "//source/common/http:context_lib", "//source/common/http:headers_lib", "//source/common/http:utility_lib", + "//source/extensions/upstreams/http/generic:config", "//test/mocks:common_lib", "//test/mocks/buffer:buffer_mocks", "//test/mocks/http:http_mocks", diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 43985c20fead3..558835feae559 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -277,6 +277,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/05/05 10908 44233 44600 router: add InternalRedirectPolicy and predicate // 2020/05/13 10531 44425 44600 Refactor resource manager // 2020/05/20 11223 44491 44600 Add primary clusters tracking to cluster manager. + // 2020/06/10 11561 44491 44811 Make upstreams pluggable // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -291,7 +292,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. EXPECT_MEMORY_EQ(m_per_cluster, 44491); - EXPECT_MEMORY_LE(m_per_cluster, 44600); + EXPECT_MEMORY_LE(m_per_cluster, 44811); } TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { @@ -339,6 +340,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/05/05 10908 36345 36800 router: add InternalRedirectPolicy and predicate // 2020/05/13 10531 36537 36800 Refactor resource manager // 2020/05/20 11223 36603 36800 Add primary clusters tracking to cluster manager. + // 2020/06/10 11561 36603 36923 Make upstreams pluggable // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -353,7 +355,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. EXPECT_MEMORY_EQ(m_per_cluster, 36603); - EXPECT_MEMORY_LE(m_per_cluster, 36800); + EXPECT_MEMORY_LE(m_per_cluster, 36923); } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) { From 8ec31d814a8447ea46bc57228d13485a48d5e274 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 16 Jun 2020 09:43:33 -0400 Subject: [PATCH 3/3] -wkn Signed-off-by: Alyssa Wilk --- source/common/router/BUILD | 1 - source/common/router/router.cc | 3 +- source/extensions/upstreams/http/BUILD | 17 --------- .../upstreams/http/generic/config.h | 4 +-- .../extensions/upstreams/http/http/config.h | 4 +-- source/extensions/upstreams/http/tcp/config.h | 4 +-- .../upstreams/http/well_known_names.h | 35 ------------------- 7 files changed, 4 insertions(+), 64 deletions(-) delete mode 100644 source/extensions/upstreams/http/BUILD delete mode 100644 source/extensions/upstreams/http/well_known_names.h diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 3ef1c7ab7ee1a..610cb0f10d75f 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -311,7 +311,6 @@ envoy_cc_library( "//source/common/tracing:http_tracer_lib", "//source/common/upstream:load_balancer_lib", "//source/extensions/common/proxy_protocol:proxy_protocol_header_lib", - "//source/extensions/upstreams/http:well_known_names", "@envoy_api//envoy/extensions/filters/http/router/v3:pkg_cc_proto", ], ) diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 6c91fb6c175dc..64f8450de4177 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -40,7 +40,6 @@ #include "common/tracing/http_tracer_impl.h" #include "extensions/filters/http/well_known_names.h" -#include "extensions/upstreams/http/well_known_names.h" namespace Envoy { namespace Router { @@ -604,7 +603,7 @@ std::unique_ptr Filter::createConnPool() { cluster_->upstreamConfig().value()); } else { factory = &Envoy::Config::Utility::getAndCheckFactoryByName( - Extensions::Upstreams::Http::HttpConnectionPoolNames::get().Generic); + "envoy.filters.connection_pools.http.generic"); } const bool should_tcp_proxy = route_entry_->connectConfig().has_value() && diff --git a/source/extensions/upstreams/http/BUILD b/source/extensions/upstreams/http/BUILD deleted file mode 100644 index 06456dbbcb5e7..0000000000000 --- a/source/extensions/upstreams/http/BUILD +++ /dev/null @@ -1,17 +0,0 @@ -load( - "//bazel:envoy_build_system.bzl", - "envoy_cc_library", - "envoy_package", -) - -licenses(["notice"]) # Apache 2 - -envoy_package() - -envoy_cc_library( - name = "well_known_names", - hdrs = ["well_known_names.h"], - deps = [ - "//source/common/singleton:const_singleton", - ], -) diff --git a/source/extensions/upstreams/http/generic/config.h b/source/extensions/upstreams/http/generic/config.h index 384697c297005..048e9998a4034 100644 --- a/source/extensions/upstreams/http/generic/config.h +++ b/source/extensions/upstreams/http/generic/config.h @@ -4,8 +4,6 @@ #include "envoy/registry/registry.h" #include "envoy/router/router.h" -#include "extensions/upstreams/http/well_known_names.h" - namespace Envoy { namespace Extensions { namespace Upstreams { @@ -17,7 +15,7 @@ namespace Generic { */ class GenericGenericConnPoolFactory : public Router::GenericConnPoolFactory { public: - std::string name() const override { return HttpConnectionPoolNames::get().Generic; } + std::string name() const override { return "envoy.filters.connection_pools.http.generic"; } std::string category() const override { return "envoy.upstreams"; } Router::GenericConnPoolPtr createGenericConnPool(Upstream::ClusterManager& cm, bool is_connect, diff --git a/source/extensions/upstreams/http/http/config.h b/source/extensions/upstreams/http/http/config.h index 0ae4fbc2eac67..9481f742a78d5 100644 --- a/source/extensions/upstreams/http/http/config.h +++ b/source/extensions/upstreams/http/http/config.h @@ -4,8 +4,6 @@ #include "envoy/registry/registry.h" #include "envoy/router/router.h" -#include "extensions/upstreams/http/well_known_names.h" - namespace Envoy { namespace Extensions { namespace Upstreams { @@ -17,7 +15,7 @@ namespace Http { */ class HttpGenericConnPoolFactory : public Router::GenericConnPoolFactory { public: - std::string name() const override { return HttpConnectionPoolNames::get().Http; } + std::string name() const override { return "envoy.filters.connection_pools.http.http"; } std::string category() const override { return "envoy.upstreams"; } Router::GenericConnPoolPtr createGenericConnPool(Upstream::ClusterManager& cm, bool is_connect, diff --git a/source/extensions/upstreams/http/tcp/config.h b/source/extensions/upstreams/http/tcp/config.h index 37aae7be6597c..8fdaa9c31d033 100644 --- a/source/extensions/upstreams/http/tcp/config.h +++ b/source/extensions/upstreams/http/tcp/config.h @@ -4,8 +4,6 @@ #include "envoy/registry/registry.h" #include "envoy/router/router.h" -#include "extensions/upstreams/http/well_known_names.h" - namespace Envoy { namespace Extensions { namespace Upstreams { @@ -17,7 +15,7 @@ namespace Tcp { */ class TcpGenericConnPoolFactory : public Router::GenericConnPoolFactory { public: - std::string name() const override { return HttpConnectionPoolNames::get().Tcp; } + std::string name() const override { return "envoy.filters.connection_pools.http.tcp"; } std::string category() const override { return "envoy.upstreams"; } Router::GenericConnPoolPtr createGenericConnPool(Upstream::ClusterManager& cm, bool is_connect, diff --git a/source/extensions/upstreams/http/well_known_names.h b/source/extensions/upstreams/http/well_known_names.h deleted file mode 100644 index df65dd23531bd..0000000000000 --- a/source/extensions/upstreams/http/well_known_names.h +++ /dev/null @@ -1,35 +0,0 @@ -#pragma once - -#include - -#include "common/singleton/const_singleton.h" - -namespace Envoy { -namespace Extensions { -namespace Upstreams { -namespace Http { - -/** - * Well-known http connection pool types. - * NOTE: New connection pools should use the well known name: - * envoy.filters.connection_pools.http.name - */ -class HttpConnectionPoolNameValues { -public: - // The normal case: sending HTTP traffic over an HTTP connection. - const std::string Http = "envoy.filters.connection_pools.http.http"; - - // The "CONNECT" upstream: sending CONNECT payload over a TCP connection. - const std::string Tcp = "envoy.filters.connection_pools.http.tcp"; - - // The default upstream, returning TCP upstream for CONNECT requests, HTTP for non-CONNECT - // requests. - const std::string Generic = "envoy.filters.connection_pools.http.generic"; -}; - -using HttpConnectionPoolNames = ConstSingleton; - -} // namespace Http -} // namespace Upstreams -} // namespace Extensions -} // namespace Envoy