From 04835afdae38f7e609ed37e489685634c83c5d1f Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 19 Mar 2024 12:27:03 -0400 Subject: [PATCH] Revert "Support upstream http filters with tcp tunneling (#27183)" (#32980) This reverts commit 50e9108c22c2e46d4ad04a977d24d89201aa0c62. Signed-off-by: Alyssa Wilk --- envoy/router/router.h | 11 - envoy/tcp/BUILD | 1 - envoy/tcp/upstream.h | 15 +- source/common/http/conn_manager_impl.h | 1 - source/common/http/filter_manager.cc | 12 +- source/common/http/filter_manager.h | 5 - source/common/router/router.cc | 10 +- source/common/router/upstream_request.cc | 12 +- source/common/router/upstream_request.h | 23 +- source/common/runtime/runtime_features.cc | 3 - source/common/runtime/runtime_features.h | 2 - source/common/tcp_proxy/BUILD | 7 - source/common/tcp_proxy/tcp_proxy.cc | 122 ++------ source/common/tcp_proxy/tcp_proxy.h | 118 +------- source/common/tcp_proxy/upstream.cc | 223 +-------------- source/common/tcp_proxy/upstream.h | 184 +----------- .../upstreams/http/http/upstream_request.h | 1 - .../upstreams/http/tcp/upstream_request.h | 1 - .../upstreams/http/udp/upstream_request.h | 1 - source/extensions/upstreams/tcp/generic/BUILD | 1 - .../upstreams/tcp/generic/config.cc | 4 +- .../extensions/upstreams/tcp/generic/config.h | 2 - test/common/router/upstream_request_test.cc | 5 +- test/common/tcp_proxy/BUILD | 4 - test/common/tcp_proxy/tcp_proxy_test.cc | 193 ++++--------- test/common/tcp_proxy/tcp_proxy_test_base.h | 5 +- test/common/tcp_proxy/upstream_test.cc | 264 ++---------------- test/extensions/filters/http/cache/BUILD | 1 - test/extensions/filters/http/csrf/BUILD | 1 - .../filters/http/custom_response/BUILD | 2 +- test/extensions/filters/http/fault/BUILD | 1 - .../filters/http/health_check/BUILD | 1 - test/extensions/filters/http/jwt_authn/BUILD | 2 +- test/extensions/filters/http/rbac/BUILD | 2 +- .../http/tcp/upstream_request_test.cc | 2 +- .../http/udp/upstream_request_test.cc | 1 - test/extensions/upstreams/tcp/generic/BUILD | 1 - .../upstreams/tcp/generic/config_test.cc | 46 ++- test/integration/BUILD | 13 +- test/integration/base_integration_test.cc | 1 - test/integration/http_protocol_integration.cc | 24 +- test/integration/http_protocol_integration.h | 4 - test/mocks/http/mocks.cc | 1 - test/mocks/http/mocks.h | 2 - test/mocks/router/BUILD | 9 - test/mocks/router/upstream_request.cc | 13 - test/mocks/router/upstream_request.h | 21 -- 47 files changed, 193 insertions(+), 1185 deletions(-) delete mode 100644 test/mocks/router/upstream_request.cc delete mode 100644 test/mocks/router/upstream_request.h diff --git a/envoy/router/router.h b/envoy/router/router.h index 8d559c280d33..299cb25c2960 100644 --- a/envoy/router/router.h +++ b/envoy/router/router.h @@ -1526,17 +1526,6 @@ class GenericUpstream { * @param trailers supplies the trailers to encode. */ virtual void encodeTrailers(const Http::RequestTrailerMap& trailers) PURE; - - // TODO(vikaschoudhary16): Remove this api. - // This api is only used to enable half-close semantics on the upstream connection. - // This ideally should be done via calling connection.enableHalfClose() but since TcpProxy - // does not have access to the upstream connection, it is done via this api for now. - /** - * Enable half-close semantics on the upstream connection. Reading a remote half-close - * will not fully close the connection. This is off by default. - * @param enabled Whether to set half-close semantics as enabled or disabled. - */ - virtual void enableHalfClose() PURE; /** * Enable/disable further data from this stream. */ diff --git a/envoy/tcp/BUILD b/envoy/tcp/BUILD index 5c3bdca0d9ef..86d70b1774c6 100644 --- a/envoy/tcp/BUILD +++ b/envoy/tcp/BUILD @@ -26,7 +26,6 @@ envoy_cc_library( "//envoy/http:header_evaluator", "//envoy/tcp:conn_pool_interface", "//envoy/upstream:upstream_interface", - "//source/common/router:router_lib", "@envoy_api//envoy/extensions/filters/network/tcp_proxy/v3:pkg_cc_proto", ], ) diff --git a/envoy/tcp/upstream.h b/envoy/tcp/upstream.h index f6191a27513b..200ec7fc9ea7 100644 --- a/envoy/tcp/upstream.h +++ b/envoy/tcp/upstream.h @@ -2,14 +2,11 @@ #include "envoy/buffer/buffer.h" #include "envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.pb.h" -#include "envoy/http/filter.h" #include "envoy/http/header_evaluator.h" #include "envoy/stream_info/stream_info.h" #include "envoy/tcp/conn_pool.h" #include "envoy/upstream/upstream.h" -#include "source/common/router/router.h" - namespace Envoy { namespace Upstream { @@ -51,17 +48,14 @@ class TunnelingConfigHelper { virtual void propagateResponseTrailers(Http::ResponseTrailerMapPtr&& trailers, const StreamInfo::FilterStateSharedPtr& filter_state) const PURE; - virtual const Envoy::Router::FilterConfig& routerFilterConfig() const PURE; - virtual Server::Configuration::ServerFactoryContext& serverFactoryContext() const PURE; }; using TunnelingConfigHelperOptConstRef = OptRef; // An API for wrapping either a TCP or an HTTP connection pool. -class GenericConnPool : public Event::DeferredDeletable, - public Logger::Loggable { +class GenericConnPool : public Logger::Loggable { public: - ~GenericConnPool() override = default; + virtual ~GenericConnPool() = default; /** * Called to create a TCP connection or HTTP stream for "CONNECT" streams. @@ -111,9 +105,9 @@ class GenericConnectionPoolCallbacks { // Interface for a generic Upstream, which can communicate with a TCP or HTTP // upstream. -class GenericUpstream : public Event::DeferredDeletable { +class GenericUpstream { public: - ~GenericUpstream() override = default; + virtual ~GenericUpstream() = default; /** * Enable/disable further data from this stream. @@ -181,7 +175,6 @@ class GenericConnPoolFactory : public Envoy::Config::TypedFactory { TunnelingConfigHelperOptConstRef config, Upstream::LoadBalancerContext* context, Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, - Http::StreamDecoderFilterCallbacks& stream_decoder_callbacks, StreamInfo::StreamInfo& downstream_info) const PURE; }; diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index c8d731afdb3d..11151b157bf5 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -314,7 +314,6 @@ class ConnectionManagerImpl : Logger::Loggable, OptRef tracingConfig() const override; const ScopeTrackedObject& scope() override; OptRef downstreamCallbacks() override { return *this; } - bool isHalfCloseEnabled() override { return false; } // DownstreamStreamFilterCallbacks void setRoute(Router::RouteConstSharedPtr route) override; diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index 070440572660..187bb7b58561 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -901,17 +901,9 @@ FilterManager::commonEncodePrefix(ActiveStreamEncoderFilter* filter, bool end_st FilterIterationStartState filter_iteration_start_state) { // Only do base state setting on the initial call. Subsequent calls for filtering do not touch // the base state. - ENVOY_STREAM_LOG(trace, "commonEncodePrefix end_stream: {}, isHalfCloseEnabled: {}", *this, - end_stream, filter_manager_callbacks_.isHalfCloseEnabled()); if (filter == nullptr) { - // half close is enabled in case tcp proxying is done with http1 encoder. In this case, we - // should not set the local_complete_ flag to true when end_stream is true. - // setting local_complete_ to true will cause any data sent in the upstream direction to be - // dropped. - if (end_stream && !filter_manager_callbacks_.isHalfCloseEnabled()) { - ASSERT(!state_.local_complete_); - state_.local_complete_ = true; - } + ASSERT(!state_.local_complete_); + state_.local_complete_ = end_stream; return encoder_filters_.begin(); } diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index da5ba20033bd..6a671ab99e9b 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -545,11 +545,6 @@ class FilterManagerCallbacks { * Returns the DownstreamStreamFilterCallbacks for downstream HTTP filters. */ virtual OptRef downstreamCallbacks() { return {}; } - /** - * Returns if close from the upstream is to be handled with half-close semantics. - * This is used for HTTP/1.1 codec. - */ - virtual bool isHalfCloseEnabled() PURE; }; /** diff --git a/source/common/router/router.cc b/source/common/router/router.cc index c3baed7b839e..a41e1c3374dc 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -744,9 +744,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, // will never transition from false to true. bool can_use_http3 = !transport_socket_options_ || !transport_socket_options_->http11ProxyInfo().has_value(); - UpstreamRequestPtr upstream_request = - std::make_unique(*this, std::move(generic_conn_pool), can_send_early_data, - can_use_http3, false /*enable_half_close*/); + UpstreamRequestPtr upstream_request = std::make_unique( + *this, std::move(generic_conn_pool), can_send_early_data, can_use_http3); LinkedList::moveIntoList(std::move(upstream_request), upstream_requests_); upstream_requests_.front()->acceptHeadersFromRouter(end_stream); if (streaming_shadows_) { @@ -1988,9 +1987,8 @@ void Filter::doRetry(bool can_send_early_data, bool can_use_http3, TimeoutRetry cleanup(); return; } - UpstreamRequestPtr upstream_request = - std::make_unique(*this, std::move(generic_conn_pool), can_send_early_data, - can_use_http3, false /*enable_tcp_tunneling*/); + UpstreamRequestPtr upstream_request = std::make_unique( + *this, std::move(generic_conn_pool), can_send_early_data, can_use_http3); if (include_attempt_count_in_request_) { downstream_headers_->setEnvoyAttemptCount(attempt_count_); diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index bb0803df3be5..7c9079e58a44 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -80,8 +80,7 @@ class UpstreamFilterManager : public Http::FilterManager { UpstreamRequest::UpstreamRequest(RouterFilterInterface& parent, std::unique_ptr&& conn_pool, - bool can_send_early_data, bool can_use_http3, - bool enable_half_close) + bool can_send_early_data, bool can_use_http3) : parent_(parent), conn_pool_(std::move(conn_pool)), stream_info_(parent_.callbacks()->dispatcher().timeSource(), nullptr), start_time_(parent_.callbacks()->dispatcher().timeSource().monotonicTime()), @@ -94,8 +93,7 @@ UpstreamRequest::UpstreamRequest(RouterFilterInterface& parent, cleaned_up_(false), had_upstream_(false), stream_options_({can_send_early_data, can_use_http3}), grpc_rq_success_deferred_(false), upstream_wait_for_response_headers_before_disabling_read_(Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read")), - enable_half_close_(enable_half_close) { + "envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read")) { if (auto tracing_config = parent_.callbacks()->tracingConfig(); tracing_config.has_value()) { if (tracing_config->spawnUpstreamSpan() || parent_.config().start_child_span_) { span_ = parent_.callbacks()->activeSpan().spawnChild( @@ -260,8 +258,7 @@ void UpstreamRequest::decode1xxHeaders(Http::ResponseHeaderMapPtr&& headers) { // on to the router. void UpstreamRequest::decodeHeaders(Http::ResponseHeaderMapPtr&& headers, bool end_stream) { ASSERT(headers.get()); - ENVOY_STREAM_LOG(trace, "end_stream: {}, upstream response headers:\n{}", *parent_.callbacks(), - end_stream, *headers); + ENVOY_STREAM_LOG(trace, "upstream response headers:\n{}", *parent_.callbacks(), *headers); ScopeTrackerScopeState scope(&parent_.callbacks()->scope(), parent_.callbacks()->dispatcher()); resetPerTryIdleTimer(); @@ -584,9 +581,6 @@ void UpstreamRequest::onPoolReady(std::unique_ptr&& upstream, had_upstream_ = true; // Have the upstream use the account of the downstream. upstream_->setAccount(parent_.callbacks()->account()); - if (enable_half_close_) { - upstream_->enableHalfClose(); - } host->outlierDetector().putResult(Upstream::Outlier::Result::LocalOriginConnectSuccess); diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index c6e7a4ec4c94..9e88bfefda31 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -62,27 +62,29 @@ class UpstreamCodecFilter; * UpstreamCodecFilter. This is accomplished via the UpstreamStreamFilterCallbacks * interface, with the UpstreamFilterManager acting as intermediary. * + * UpstreamRequest is marked as final because no subclasses are expected. + * This class is intended to be used as-is without any specialized inheritance. */ -class UpstreamRequest : public Logger::Loggable, - public UpstreamToDownstream, - public LinkedObject, - public GenericConnectionPoolCallbacks, - public Event::DeferredDeletable { +class UpstreamRequest final : public Logger::Loggable, + public UpstreamToDownstream, + public LinkedObject, + public GenericConnectionPoolCallbacks, + public Event::DeferredDeletable { public: UpstreamRequest(RouterFilterInterface& parent, std::unique_ptr&& conn_pool, - bool can_send_early_data, bool can_use_http3, bool enable_half_close); + bool can_send_early_data, bool can_use_http3); ~UpstreamRequest() override; void deleteIsPending() override { cleanUp(); } // To be called from the destructor, or prior to deferred delete. void cleanUp(); - virtual void acceptHeadersFromRouter(bool end_stream); - virtual void acceptDataFromRouter(Buffer::Instance& data, bool end_stream); + void acceptHeadersFromRouter(bool end_stream); + void acceptDataFromRouter(Buffer::Instance& data, bool end_stream); void acceptTrailersFromRouter(Http::RequestTrailerMap& trailers); void acceptMetadataFromRouter(Http::MetadataMapPtr&& metadata_map_ptr); - virtual void resetStream(); + void resetStream(); void setupPerTryTimeout(); void maybeEndDecode(bool end_stream); void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host, bool pool_success); @@ -256,7 +258,6 @@ class UpstreamRequest : public Logger::Loggable, Http::ConnectionPool::Instance::StreamOptions stream_options_; bool grpc_rq_success_deferred_ : 1; bool upstream_wait_for_response_headers_before_disabling_read_ : 1; - bool enable_half_close_ : 1; }; class UpstreamRequestFilterManagerCallbacks : public Http::FilterManagerCallbacks, @@ -370,7 +371,7 @@ class UpstreamRequestFilterManagerCallbacks : public Http::FilterManagerCallback void setUpstreamToDownstream(UpstreamToDownstream& upstream_to_downstream_interface) override { upstream_request_.upstream_interface_ = upstream_to_downstream_interface; } - bool isHalfCloseEnabled() override { return upstream_request_.enable_half_close_; } + Http::RequestTrailerMapPtr trailers_; Http::ResponseHeaderMapPtr informational_headers_; Http::ResponseHeaderMapPtr response_headers_; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index ec9e7f6a40e3..40e526641425 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -130,9 +130,6 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_runtime_initialized); FALSE_RUNTIME_GUARD(envoy_reloadable_features_always_use_v6); // TODO(wbpcode) complete remove this feature is no one use it. FALSE_RUNTIME_GUARD(envoy_reloadable_features_refresh_rtt_after_request); -// TODO(vikaschoudhary16) flip this to true only after all the -// TcpProxy::Filter::HttpStreamDecoderFilterCallbacks are implemented or commented as unnecessary -FALSE_RUNTIME_GUARD(envoy_restart_features_upstream_http_filters_with_tcp_proxy); // TODO(danzh) false deprecate it once QUICHE has its own enable/disable flag. FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_reject_all); // TODO(suniltheta): Once the newly added http async technique is stabilized move it under diff --git a/source/common/runtime/runtime_features.h b/source/common/runtime/runtime_features.h index e698064a5971..0c2fb2d9b9e7 100644 --- a/source/common/runtime/runtime_features.h +++ b/source/common/runtime/runtime_features.h @@ -26,8 +26,6 @@ void maybeSetRuntimeGuard(absl::string_view name, bool value); void maybeSetDeprecatedInts(absl::string_view name, uint32_t value); constexpr absl::string_view defer_processing_backedup_streams = "envoy.reloadable_features.defer_processing_backedup_streams"; -constexpr absl::string_view upstream_http_filters_with_tcp_proxy = - "envoy.restart_features.upstream_http_filters_with_tcp_proxy"; } // namespace Runtime } // namespace Envoy diff --git a/source/common/tcp_proxy/BUILD b/source/common/tcp_proxy/BUILD index c6883b43aec3..966088ed4a08 100644 --- a/source/common/tcp_proxy/BUILD +++ b/source/common/tcp_proxy/BUILD @@ -17,22 +17,15 @@ envoy_cc_library( "upstream.h", ], deps = [ - "//envoy/http:header_map_interface", - "//envoy/router:router_ratelimit_interface", "//envoy/tcp:conn_pool_interface", "//envoy/tcp:upstream_interface", "//envoy/upstream:cluster_manager_interface", "//envoy/upstream:load_balancer_interface", - "//source/common/http:async_client_lib", "//source/common/http:codec_client_lib", - "//source/common/http:hash_policy_lib", "//source/common/http:header_map_lib", "//source/common/http:headers_lib", "//source/common/http:utility_lib", - "//source/common/network:utility_lib", "//source/common/router:header_parser_lib", - "//source/common/router:router_lib", - "//source/common/router:shadow_writer_lib", ], ) diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 21483aca5463..f60f31ec44db 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -32,10 +32,8 @@ #include "source/common/network/upstream_server_name.h" #include "source/common/network/upstream_socket_options_filter_state.h" #include "source/common/router/metadatamatchcriteria_impl.h" -#include "source/common/router/shadow_writer_impl.h" #include "source/common/stream_info/stream_id_provider_impl.h" #include "source/common/stream_info/uint64_accessor_impl.h" -#include "source/common/tracing/http_tracer_impl.h" namespace Envoy { namespace TcpProxy { @@ -112,7 +110,7 @@ Config::SharedConfig::SharedConfig( } if (config.has_tunneling_config()) { tunneling_config_helper_ = - std::make_unique(*stats_scope_.get(), config, context); + std::make_unique(config.tunneling_config(), context); } if (config.has_max_downstream_connection_duration()) { const uint64_t connection_duration = @@ -232,10 +230,8 @@ UpstreamDrainManager& Config::drainManager() { } Filter::Filter(ConfigSharedPtr config, Upstream::ClusterManager& cluster_manager) - : tracing_config_(Tracing::EgressConfig::get()), config_(config), - cluster_manager_(cluster_manager), downstream_callbacks_(*this), - upstream_callbacks_(new UpstreamCallbacks(this)), - upstream_decoder_filter_callbacks_(HttpStreamDecoderFilterCallbacks(this)) { + : config_(config), cluster_manager_(cluster_manager), downstream_callbacks_(*this), + upstream_callbacks_(new UpstreamCallbacks(this)) { ASSERT(config != nullptr); } @@ -293,11 +289,9 @@ void Filter::onInitFailure(UpstreamFailureReason reason) { // not have started attempting to connect to an upstream and there is no // connection pool callback latency to record. if (initial_upstream_connection_start_time_.has_value()) { - if (!getStreamInfo().upstreamInfo()->upstreamTiming().connectionPoolCallbackLatency()) { - getStreamInfo().upstreamInfo()->upstreamTiming().recordConnectionPoolCallbackLatency( - initial_upstream_connection_start_time_.value(), - read_callbacks_->connection().dispatcher().timeSource()); - } + getStreamInfo().upstreamInfo()->upstreamTiming().recordConnectionPoolCallbackLatency( + initial_upstream_connection_start_time_.value(), + read_callbacks_->connection().dispatcher().timeSource()); } read_callbacks_->connection().close( Network::ConnectionCloseType::NoFlush, @@ -558,16 +552,9 @@ bool Filter::maybeTunnel(Upstream::ThreadLocalCluster& cluster) { if (!factory) { return false; } - if (Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - // TODO(vikaschoudhary16): Initialize route_ once per cluster. - upstream_decoder_filter_callbacks_.route_ = std::make_shared( - cluster.info()->name(), - *std::unique_ptr{new Router::RetryPolicyImpl()}); - } - generic_conn_pool_ = factory->createGenericConnPool( - cluster, config_->tunnelingConfigHelper(), this, *upstream_callbacks_, - upstream_decoder_filter_callbacks_, getStreamInfo()); + + generic_conn_pool_ = factory->createGenericConnPool(cluster, config_->tunnelingConfigHelper(), + this, *upstream_callbacks_, getStreamInfo()); if (generic_conn_pool_) { connecting_ = true; connect_attempts_++; @@ -583,19 +570,7 @@ bool Filter::maybeTunnel(Upstream::ThreadLocalCluster& cluster) { void Filter::onGenericPoolFailure(ConnectionPool::PoolFailureReason reason, absl::string_view failure_reason, Upstream::HostDescriptionConstSharedPtr host) { - if (Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - // generic_conn_pool_ is an instance of TcpProxy::HttpConnPool. - // generic_conn_pool_->newStream() is called in maybeTunnel() which initializes an instance of - // Router::UpstreamRequest. If Router::UpstreamRequest receives headers from the upstream which - // results in end_stream=true, then via callbacks passed to Router::UpstreamRequest, - // TcpProxy::Filter::onGenericPoolFailure() gets invoked. If we do not do deferredDelete here, - // then the same instance of UpstreamRequest which is under execution will go out of scope. - read_callbacks_->connection().dispatcher().deferredDelete(std::move(generic_conn_pool_)); - } else { - generic_conn_pool_.reset(); - } - + generic_conn_pool_.reset(); read_callbacks_->upstreamHost(host); getStreamInfo().upstreamInfo()->setUpstreamHost(host); getStreamInfo().upstreamInfo()->setUpstreamTransportFailureReason(failure_reason); @@ -619,30 +594,23 @@ void Filter::onGenericPoolReady(StreamInfo::StreamInfo* info, Upstream::HostDescriptionConstSharedPtr& host, const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info) { - StreamInfo::UpstreamInfo& upstream_info = *getStreamInfo().upstreamInfo(); - if (!upstream_info.upstreamTiming().connectionPoolCallbackLatency()) { - upstream_info.upstreamTiming().recordConnectionPoolCallbackLatency( - initial_upstream_connection_start_time_.value(), - read_callbacks_->connection().dispatcher().timeSource()); - } upstream_ = std::move(upstream); generic_conn_pool_.reset(); read_callbacks_->upstreamHost(host); - // No need to set information using address_provider in case routing via Router::UpstreamRequest - // because in that case, information is already set by the - // Router::UpstreamRequest::onPoolReady() method before reaching here. - if (upstream_info.upstreamLocalAddress() == nullptr) { - upstream_info.setUpstreamLocalAddress(address_provider.localAddress()); - upstream_info.setUpstreamRemoteAddress(address_provider.remoteAddress()); - } + StreamInfo::UpstreamInfo& upstream_info = *getStreamInfo().upstreamInfo(); + upstream_info.upstreamTiming().recordConnectionPoolCallbackLatency( + initial_upstream_connection_start_time_.value(), + read_callbacks_->connection().dispatcher().timeSource()); upstream_info.setUpstreamHost(host); + upstream_info.setUpstreamLocalAddress(address_provider.localAddress()); + upstream_info.setUpstreamRemoteAddress(address_provider.remoteAddress()); upstream_info.setUpstreamSslConnection(ssl_info); onUpstreamConnection(); read_callbacks_->continueReading(); if (info) { upstream_info.setUpstreamFilterState(info->filterState()); } -} // namespace TcpProxy +} const Router::MetadataMatchCriteria* Filter::metadataMatchCriteria() { const Router::MetadataMatchCriteria* route_criteria = @@ -672,30 +640,14 @@ const std::string& TunnelResponseTrailers::key() { } TunnelingConfigHelperImpl::TunnelingConfigHelperImpl( - Stats::Scope& stats_scope, - const envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy& config_message, + const envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig& + config_message, Server::Configuration::FactoryContext& context) - : use_post_(config_message.tunneling_config().use_post()), - header_parser_(Envoy::Router::HeaderParser::configure( - config_message.tunneling_config().headers_to_add())), - propagate_response_headers_(config_message.tunneling_config().propagate_response_headers()), - propagate_response_trailers_(config_message.tunneling_config().propagate_response_trailers()), - post_path_(config_message.tunneling_config().post_path()), - route_stat_name_storage_("tcpproxy_tunneling", context.scope().symbolTable()), - // TODO(vikaschoudhary16): figure out which of the following router_config_ members are - // not required by tcp_proxy and move them to a different class - router_config_(route_stat_name_storage_.statName(), - context.serverFactoryContext().localInfo(), stats_scope, - context.serverFactoryContext().clusterManager(), - context.serverFactoryContext().runtime(), - context.serverFactoryContext().api().randomGenerator(), - std::make_unique( - context.serverFactoryContext().clusterManager()), - true, false, false, false, false, false, {}, - context.serverFactoryContext().api().timeSource(), - context.serverFactoryContext().httpContext(), - context.serverFactoryContext().routerContext()), - server_factory_context_(context.serverFactoryContext()) { + : use_post_(config_message.use_post()), + header_parser_(Envoy::Router::HeaderParser::configure(config_message.headers_to_add())), + propagate_response_headers_(config_message.propagate_response_headers()), + propagate_response_trailers_(config_message.propagate_response_trailers()), + post_path_(config_message.post_path()) { if (!post_path_.empty() && !use_post_) { throw EnvoyException("Can't set a post path when POST method isn't used"); } @@ -703,7 +655,7 @@ TunnelingConfigHelperImpl::TunnelingConfigHelperImpl( envoy::config::core::v3::SubstitutionFormatString substitution_format_config; substitution_format_config.mutable_text_format_source()->set_inline_string( - config_message.tunneling_config().hostname()); + config_message.hostname()); hostname_fmt_ = Formatter::SubstitutionFormatStringUtils::fromProtoConfig( substitution_format_config, context); } @@ -745,8 +697,8 @@ void Filter::onConnectTimeout() { } Network::FilterStatus Filter::onData(Buffer::Instance& data, bool end_stream) { - ENVOY_CONN_LOG(trace, "downstream connection received {} bytes, end_stream={}, has upstream {}", - read_callbacks_->connection(), data.length(), end_stream, upstream_ != nullptr); + ENVOY_CONN_LOG(trace, "downstream connection received {} bytes, end_stream={}", + read_callbacks_->connection(), data.length(), end_stream); getStreamInfo().getDownstreamBytesMeter()->addWireBytesReceived(data.length()); if (upstream_) { getStreamInfo().getUpstreamBytesMeter()->addWireBytesSent(data.length()); @@ -847,23 +799,14 @@ void Filter::onUpstreamEvent(Network::ConnectionEvent event) { if (event == Network::ConnectionEvent::RemoteClose || event == Network::ConnectionEvent::LocalClose) { - if (Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - read_callbacks_->connection().dispatcher().deferredDelete(std::move(upstream_)); - } else { - upstream_.reset(); - } + upstream_.reset(); disableIdleTimer(); if (connecting) { if (event == Network::ConnectionEvent::RemoteClose) { - getStreamInfo().setResponseFlag(StreamInfo::UpstreamConnectionFailure); - // upstreamHost can be nullptr if we received a disconnect from the upstream before - // receiving any response - if (read_callbacks_->upstreamHost() != nullptr) { - read_callbacks_->upstreamHost()->outlierDetector().putResult( - Upstream::Outlier::Result::LocalOriginConnectFailed); - } + getStreamInfo().setResponseFlag(StreamInfo::CoreResponseFlag::UpstreamConnectionFailure); + read_callbacks_->upstreamHost()->outlierDetector().putResult( + Upstream::Outlier::Result::LocalOriginConnectFailed); } if (!downstream_closed_) { route_ = pickRoute(); @@ -987,9 +930,6 @@ void Filter::disableIdleTimer() { } } -Filter::HttpStreamDecoderFilterCallbacks::HttpStreamDecoderFilterCallbacks(Filter* parent) - : parent_(parent), request_trailer_map_(Http::RequestTrailerMapImpl::create()) {} - UpstreamDrainManager::~UpstreamDrainManager() { // If connections aren't closed before they are destructed an ASSERT fires, // so cancel all pending drains, which causes the connections to be closed. diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h index 4019ea1211ef..45f34b901405 100644 --- a/source/common/tcp_proxy/tcp_proxy.h +++ b/source/common/tcp_proxy/tcp_proxy.h @@ -10,7 +10,6 @@ #include "envoy/common/random_generator.h" #include "envoy/event/timer.h" #include "envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.pb.h" -#include "envoy/http/codec.h" #include "envoy/http/header_evaluator.h" #include "envoy/network/connection.h" #include "envoy/network/filter.h" @@ -23,7 +22,6 @@ #include "envoy/upstream/cluster_manager.h" #include "envoy/upstream/upstream.h" -#include "source/common/common/assert.h" #include "source/common/common/logger.h" #include "source/common/formatter/substitution_format_string.h" #include "source/common/http/header_map_impl.h" @@ -145,29 +143,24 @@ class TunnelResponseTrailers : public Http::TunnelResponseHeadersOrTrailersImpl private: const Http::ResponseTrailerMapPtr response_trailers_; }; -class Config; + class TunnelingConfigHelperImpl : public TunnelingConfigHelper, protected Logger::Loggable { public: TunnelingConfigHelperImpl( - Stats::Scope& scope, - const envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy& config_message, + const envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig& + config_message, Server::Configuration::FactoryContext& context); std::string host(const StreamInfo::StreamInfo& stream_info) const override; bool usePost() const override { return use_post_; } const std::string& postPath() const override { return post_path_; } Envoy::Http::HeaderEvaluator& headerEvaluator() const override { return *header_parser_; } - - const Envoy::Router::FilterConfig& routerFilterConfig() const override { return router_config_; } void propagateResponseHeaders(Http::ResponseHeaderMapPtr&& headers, const StreamInfo::FilterStateSharedPtr& filter_state) const override; void propagateResponseTrailers(Http::ResponseTrailerMapPtr&& trailers, const StreamInfo::FilterStateSharedPtr& filter_state) const override; - Server::Configuration::ServerFactoryContext& serverFactoryContext() const override { - return server_factory_context_; - } private: const bool use_post_; @@ -176,9 +169,6 @@ class TunnelingConfigHelperImpl : public TunnelingConfigHelper, const bool propagate_response_headers_; const bool propagate_response_trailers_; std::string post_path_; - Stats::StatNameManagedStorage route_stat_name_storage_; - const Router::FilterConfig router_config_; - Server::Configuration::ServerFactoryContext& server_factory_context_; }; /** @@ -471,107 +461,6 @@ class Filter : public Network::ReadFilter, }; StreamInfo::StreamInfo& getStreamInfo(); - class HttpStreamDecoderFilterCallbacks : public Http::StreamDecoderFilterCallbacks, - public ScopeTrackedObject { - public: - HttpStreamDecoderFilterCallbacks(Filter* parent); - // Http::StreamDecoderFilterCallbacks - OptRef connection() override { - return parent_->read_callbacks_->connection(); - } - StreamInfo::StreamInfo& streamInfo() override { return parent_->getStreamInfo(); } - const ScopeTrackedObject& scope() override { return *this; } - Event::Dispatcher& dispatcher() override { - return parent_->read_callbacks_->connection().dispatcher(); - } - void resetStream(Http::StreamResetReason, absl::string_view) override { - IS_ENVOY_BUG("Not implemented. Unexpected call to resetStream()"); - }; - Router::RouteConstSharedPtr route() override { return route_; } - Upstream::ClusterInfoConstSharedPtr clusterInfo() override { - return parent_->cluster_manager_.getThreadLocalCluster(parent_->route_->clusterName()) - ->info(); - } - uint64_t streamId() const override { - auto sip = parent_->getStreamInfo().getStreamIdProvider(); - if (sip) { - return sip->toInteger().value(); - } - return 0; - } - Tracing::Span& activeSpan() override { return parent_->active_span_; } - OptRef tracingConfig() const override { - return makeOptRef(parent_->tracing_config_); - } - void continueDecoding() override {} - void addDecodedData(Buffer::Instance&, bool) override {} - void injectDecodedDataToFilterChain(Buffer::Instance&, bool) override {} - Http::RequestTrailerMap& addDecodedTrailers() override { return *request_trailer_map_; } - Http::MetadataMapVector& addDecodedMetadata() override { - static Http::MetadataMapVector metadata_map_vector; - return metadata_map_vector; - } - const Buffer::Instance* decodingBuffer() override { return nullptr; } - void modifyDecodingBuffer(std::function) override {} - void sendLocalReply(Http::Code, absl::string_view, - std::function, - const absl::optional, - absl::string_view) override {} - void encode1xxHeaders(Http::ResponseHeaderMapPtr&&) override {} - Http::ResponseHeaderMapOptRef informationalHeaders() override { return {}; } - void encodeHeaders(Http::ResponseHeaderMapPtr&&, bool, absl::string_view) override {} - Http::ResponseHeaderMapOptRef responseHeaders() override { return {}; } - void encodeData(Buffer::Instance&, bool) override {} - Http::RequestHeaderMapOptRef requestHeaders() override { return {}; } - Http::RequestTrailerMapOptRef requestTrailers() override { return {}; } - void encodeTrailers(Http::ResponseTrailerMapPtr&&) override {} - Http::ResponseTrailerMapOptRef responseTrailers() override { return {}; } - void encodeMetadata(Http::MetadataMapPtr&&) override {} - // TODO(vikaschoudhary16): Implement watermark callbacks and test through flow control e2es. - void onDecoderFilterAboveWriteBufferHighWatermark() override {} - void onDecoderFilterBelowWriteBufferLowWatermark() override {} - void addDownstreamWatermarkCallbacks(Http::DownstreamWatermarkCallbacks&) override {} - void removeDownstreamWatermarkCallbacks(Http::DownstreamWatermarkCallbacks&) override {} - void setDecoderBufferLimit(uint32_t) override {} - uint32_t decoderBufferLimit() override { return 0; } - bool recreateStream(const Http::ResponseHeaderMap*) override { return false; } - void addUpstreamSocketOptions(const Network::Socket::OptionsSharedPtr&) override {} - Network::Socket::OptionsSharedPtr getUpstreamSocketOptions() const override { return nullptr; } - const Router::RouteSpecificFilterConfig* mostSpecificPerFilterConfig() const override { - return nullptr; - } - Buffer::BufferMemoryAccountSharedPtr account() const override { return nullptr; } - void setUpstreamOverrideHost(Upstream::LoadBalancerContext::OverrideHost) override {} - absl::optional - upstreamOverrideHost() const override { - return absl::nullopt; - } - void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) override { - tracked_object_stack.add(*this); - } - void traversePerFilterConfig( - std::function) const override {} - Http::Http1StreamEncoderOptionsOptRef http1StreamEncoderOptions() override { return {}; } - OptRef downstreamCallbacks() override { return {}; } - OptRef upstreamCallbacks() override { return {}; } - void resetIdleTimer() override {} - // absl::optional upstreamOverrideHost() const override { - // return absl::nullopt; - // } - absl::string_view filterConfigName() const override { return ""; } - - // ScopeTrackedObject - void dumpState(std::ostream& os, int indent_level) const override { - const char* spaces = spacesForLevel(indent_level); - os << spaces << "TcpProxy " << this << DUMP_MEMBER(streamId()) << "\n"; - DUMP_DETAILS(parent_->getStreamInfo().upstreamInfo()); - } - Filter* parent_{}; - Http::RequestTrailerMapPtr request_trailer_map_; - std::shared_ptr route_; - }; - Tracing::NullSpan active_span_; - const Tracing::Config& tracing_config_; protected: struct DownstreamCallbacks : public Network::ConnectionCallbacks { @@ -656,7 +545,6 @@ class Filter : public Network::ReadFilter, uint32_t connect_attempts_{}; bool connecting_{}; bool downstream_closed_{}; - HttpStreamDecoderFilterCallbacks upstream_decoder_filter_callbacks_; }; // This class deals with an upstream connection that needs to finish flushing, when the downstream diff --git a/source/common/tcp_proxy/upstream.cc b/source/common/tcp_proxy/upstream.cc index 01e99426b9f6..5e4eaa35338d 100644 --- a/source/common/tcp_proxy/upstream.cc +++ b/source/common/tcp_proxy/upstream.cc @@ -1,19 +1,16 @@ #include "source/common/tcp_proxy/upstream.h" -#include "envoy/http/header_map.h" #include "envoy/upstream/cluster_manager.h" #include "source/common/http/codec_client.h" #include "source/common/http/codes.h" #include "source/common/http/header_map_impl.h" #include "source/common/http/headers.h" -#include "source/common/http/null_route_impl.h" #include "source/common/http/utility.h" #include "source/common/runtime/runtime_features.h" namespace Envoy { namespace TcpProxy { - using TunnelingConfig = envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig; @@ -197,7 +194,6 @@ void HttpUpstream::resetEncoder(Network::ConnectionEvent event, bool inform_down conn_pool_callbacks_->onFailure(); return; } - if (inform_downstream) { upstream_callbacks_.onEvent(event); } @@ -233,7 +229,7 @@ TcpConnPool::~TcpConnPool() { void TcpConnPool::newStream(GenericConnectionPoolCallbacks& callbacks) { callbacks_ = &callbacks; - // Given this function is re-entrant, make sure we only reset the upstream_handle_ if given a + // Given this function is reentrant, make sure we only reset the upstream_handle_ if given a // valid connection handle. If newConnection fails inline it may result in attempting to // select a new host, and a recursive call to establishUpstreamConnection. In this case the // first call to newConnection will return null and the inner call will persist. @@ -274,67 +270,29 @@ HttpConnPool::HttpConnPool(Upstream::ThreadLocalCluster& thread_local_cluster, Upstream::LoadBalancerContext* context, const TunnelingConfigHelper& config, Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, - Http::StreamDecoderFilterCallbacks& stream_decoder_callbacks, Http::CodecType type, StreamInfo::StreamInfo& downstream_info) - : config_(config), type_(type), decoder_filter_callbacks_(&stream_decoder_callbacks), - upstream_callbacks_(upstream_callbacks), downstream_info_(downstream_info) { + : config_(config), type_(type), upstream_callbacks_(upstream_callbacks), + downstream_info_(downstream_info) { absl::optional protocol; if (type_ == Http::CodecType::HTTP3) { protocol = Http::Protocol::Http3; } else if (type_ == Http::CodecType::HTTP2) { protocol = Http::Protocol::Http2; } - if (Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - absl::optional upstream_protocol = protocol; - generic_conn_pool_ = createConnPool(thread_local_cluster, context, upstream_protocol); - return; - } conn_pool_data_ = thread_local_cluster.httpConnPool(Upstream::ResourcePriority::Default, protocol, context); } -std::unique_ptr -HttpConnPool::createConnPool(Upstream::ThreadLocalCluster& cluster, - Upstream::LoadBalancerContext* context, - absl::optional protocol) { - Router::GenericConnPoolFactory* factory = nullptr; - factory = Envoy::Config::Utility::getFactoryByName( - "envoy.filters.connection_pools.http.generic"); - if (!factory) { - return nullptr; - } - - return factory->createGenericConnPool( - cluster, Envoy::Router::GenericConnPoolFactory::UpstreamProtocol::HTTP, - decoder_filter_callbacks_->route()->routeEntry()->priority(), protocol, context); -} - HttpConnPool::~HttpConnPool() { if (upstream_handle_ != nullptr) { // Because HTTP connections are generally shorter lived and have a higher probability of use // before going idle, they are closed with Default rather than CloseExcess. upstream_handle_->cancel(ConnectionPool::CancelPolicy::Default); } - if (combined_upstream_ != nullptr) { - combined_upstream_->onDownstreamEvent(Network::ConnectionEvent::LocalClose); - } } void HttpConnPool::newStream(GenericConnectionPoolCallbacks& callbacks) { callbacks_ = &callbacks; - if (Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - combined_upstream_ = std::make_unique( - *this, upstream_callbacks_, *decoder_filter_callbacks_, config_, downstream_info_); - RouterUpstreamRequestPtr upstream_request = std::make_unique( - *combined_upstream_, std::move(generic_conn_pool_), /*can_send_early_data_=*/false, - /*can_use_http3_=*/true, true /*enable_tcp_tunneling*/); - combined_upstream_->setRouterUpstreamRequest(std::move(upstream_request)); - combined_upstream_->newStream(callbacks); - return; - } - upstream_ = std::make_unique(upstream_callbacks_, config_, downstream_info_, type_); Tcp::ConnectionPool::Cancellable* handle = conn_pool_data_.value().newStream(upstream_->responseDecoder(), *this, @@ -352,15 +310,6 @@ void HttpConnPool::onPoolFailure(ConnectionPool::PoolFailureReason reason, callbacks_->onGenericPoolFailure(reason, failure_reason, host); } -void HttpConnPool::onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host, - bool pool_success) { - if (!pool_success) { - return; - } - combined_upstream_->setConnPoolCallbacks(std::make_unique( - *this, host, downstream_info_.downstreamAddressProvider().sslConnection())); -} - void HttpConnPool::onPoolReady(Http::RequestEncoder& request_encoder, Upstream::HostDescriptionConstSharedPtr host, StreamInfo::StreamInfo& info, absl::optional) { @@ -383,174 +332,8 @@ void HttpConnPool::onPoolReady(Http::RequestEncoder& request_encoder, void HttpConnPool::onGenericPoolReady(Upstream::HostDescriptionConstSharedPtr& host, const Network::ConnectionInfoProvider& address_provider, Ssl::ConnectionInfoConstSharedPtr ssl_info) { - if (Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - - callbacks_->onGenericPoolReady(nullptr, std::move(combined_upstream_), host, address_provider, - ssl_info); - return; - } callbacks_->onGenericPoolReady(nullptr, std::move(upstream_), host, address_provider, ssl_info); } -CombinedUpstream::CombinedUpstream(HttpConnPool& http_conn_pool, - Tcp::ConnectionPool::UpstreamCallbacks& callbacks, - Http::StreamDecoderFilterCallbacks& decoder_callbacks, - const TunnelingConfigHelper& config, - StreamInfo::StreamInfo& downstream_info) - : config_(config), downstream_info_(downstream_info), parent_(http_conn_pool), - decoder_filter_callbacks_(decoder_callbacks), response_decoder_(*this), - upstream_callbacks_(callbacks) { - auto is_ssl = downstream_info_.downstreamAddressProvider().sslConnection(); - downstream_headers_ = Http::createHeaderMap({ - {Http::Headers::get().Method, config_.usePost() ? "POST" : "CONNECT"}, - {Http::Headers::get().Host, config_.host(downstream_info_)}, - }); - - if (config_.usePost()) { - const std::string& scheme = - is_ssl ? Http::Headers::get().SchemeValues.Https : Http::Headers::get().SchemeValues.Http; - downstream_headers_->addReference(Http::Headers::get().Path, config_.postPath()); - downstream_headers_->addReference(Http::Headers::get().Scheme, scheme); - } - - config_.headerEvaluator().evaluateHeaders( - *downstream_headers_, {downstream_info_.getRequestHeaders()}, downstream_info_); -} - -void CombinedUpstream::setRouterUpstreamRequest( - Router::UpstreamRequestPtr router_upstream_request) { - LinkedList::moveIntoList(std::move(router_upstream_request), upstream_requests_); -} - -void CombinedUpstream::newStream(GenericConnectionPoolCallbacks&) { - upstream_requests_.front()->acceptHeadersFromRouter(false); -} - -void CombinedUpstream::encodeData(Buffer::Instance& data, bool end_stream) { - if (upstream_requests_.empty()) { - return; - } - upstream_requests_.front()->acceptDataFromRouter(data, end_stream); - if (end_stream) { - doneWriting(); - } -} - -bool CombinedUpstream::readDisable(bool disable) { - if (upstream_requests_.empty()) { - return false; - } - if (disable) { - upstream_requests_.front()->onAboveWriteBufferHighWatermark(); - } - return true; -} - -Tcp::ConnectionPool::ConnectionData* -CombinedUpstream::onDownstreamEvent(Network::ConnectionEvent event) { - if (upstream_requests_.empty()) { - return nullptr; - } - - if (event == Network::ConnectionEvent::LocalClose || - event == Network::ConnectionEvent::RemoteClose) { - upstream_requests_.front()->resetStream(); - } - return nullptr; -} - -bool CombinedUpstream::isValidResponse(const Http::ResponseHeaderMap& headers) { - switch (parent_.codecType()) { - case Http::CodecType::HTTP1: - // According to RFC7231 any 2xx response indicates that the connection is - // established. - // Any 'Content-Length' or 'Transfer-Encoding' header fields MUST be ignored. - // https://tools.ietf.org/html/rfc7231#section-4.3.6 - return Http::CodeUtility::is2xx(Http::Utility::getResponseStatus(headers)); - case Http::CodecType::HTTP2: - case Http::CodecType::HTTP3: - if (Http::Utility::getResponseStatus(headers) != 200) { - return false; - } - return true; - } - return true; -} - -void CombinedUpstream::onResetEncoder(Network::ConnectionEvent event, bool inform_downstream) { - if (event == Network::ConnectionEvent::LocalClose || - event == Network::ConnectionEvent::RemoteClose) { - if (!upstream_requests_.empty()) { - upstream_requests_.front()->resetStream(); - } - } - - // If we did not receive a valid CONNECT response yet we treat this as a pool - // failure, otherwise we forward the event downstream. - if (conn_pool_callbacks_ != nullptr) { - conn_pool_callbacks_->onFailure(); - return; - } - - if (inform_downstream) { - upstream_callbacks_.onEvent(event); - } -} - -// Router::RouterFilterInterface -void CombinedUpstream::onUpstreamHeaders([[maybe_unused]] uint64_t response_code, - Http::ResponseHeaderMapPtr&& headers, - [[maybe_unused]] UpstreamRequest& upstream_request, - bool end_stream) { - responseDecoder().decodeHeaders(std::move(headers), end_stream); -} - -void CombinedUpstream::onUpstreamData(Buffer::Instance& data, - [[maybe_unused]] UpstreamRequest& upstream_request, - bool end_stream) { - responseDecoder().decodeData(data, end_stream); -} - -void CombinedUpstream::onUpstreamTrailers(Http::ResponseTrailerMapPtr&& trailers, - UpstreamRequest&) { - responseDecoder().decodeTrailers(std::move(trailers)); -} - -Http::RequestHeaderMap* CombinedUpstream::downstreamHeaders() { return downstream_headers_.get(); } - -void CombinedUpstream::doneReading() { - read_half_closed_ = true; - if (write_half_closed_) { - onResetEncoder(Network::ConnectionEvent::LocalClose); - } -} - -void CombinedUpstream::onUpstreamReset(Http::StreamResetReason, absl::string_view, - UpstreamRequest&) { - upstream_callbacks_.onEvent(Network::ConnectionEvent::RemoteClose); -} - -void CombinedUpstream::doneWriting() { - write_half_closed_ = true; - if (read_half_closed_) { - onResetEncoder(Network::ConnectionEvent::LocalClose); - } -} - -void CombinedUpstream::onResetStream(Http::StreamResetReason, absl::string_view) { - read_half_closed_ = true; - write_half_closed_ = true; - onResetEncoder(Network::ConnectionEvent::LocalClose); -} - -void CombinedUpstream::onAboveWriteBufferHighWatermark() { - upstream_callbacks_.onAboveWriteBufferHighWatermark(); -} - -void CombinedUpstream::onBelowWriteBufferLowWatermark() { - upstream_callbacks_.onBelowWriteBufferLowWatermark(); -} - } // namespace TcpProxy } // namespace Envoy diff --git a/source/common/tcp_proxy/upstream.h b/source/common/tcp_proxy/upstream.h index 23af532b37cb..d16126547cc5 100644 --- a/source/common/tcp_proxy/upstream.h +++ b/source/common/tcp_proxy/upstream.h @@ -1,11 +1,7 @@ #pragma once -#include - #include "envoy/http/conn_pool.h" -#include "envoy/http/header_map.h" #include "envoy/network/connection.h" -#include "envoy/router/router_ratelimit.h" #include "envoy/tcp/conn_pool.h" #include "envoy/tcp/upstream.h" #include "envoy/upstream/load_balancer.h" @@ -15,13 +11,7 @@ #include "source/common/buffer/buffer_impl.h" #include "source/common/common/dump_state_utils.h" #include "source/common/http/codec_client.h" -#include "source/common/http/hash_policy.h" -#include "source/common/http/null_route_impl.h" -#include "source/common/network/utility.h" -#include "source/common/router/config_impl.h" #include "source/common/router/header_parser.h" -#include "source/common/router/router.h" -#include "source/extensions/early_data/default_early_data_policy.h" namespace Envoy { namespace TcpProxy { @@ -57,25 +47,16 @@ class TcpConnPool : public GenericConnPool, public Tcp::ConnectionPool::Callback }; class HttpUpstream; -class CombinedUpstream; class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callbacks { public: HttpConnPool(Upstream::ThreadLocalCluster& thread_local_cluster, Upstream::LoadBalancerContext* context, const TunnelingConfigHelper& config, - Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, - Http::StreamDecoderFilterCallbacks&, Http::CodecType type, + Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, Http::CodecType type, StreamInfo::StreamInfo& downstream_info); - - using RouterUpstreamRequest = Router::UpstreamRequest; - using RouterUpstreamRequestPtr = std::unique_ptr; ~HttpConnPool() override; - bool valid() const { return conn_pool_data_.has_value() || generic_conn_pool_; } - Http::CodecType codecType() const { return type_; } - std::unique_ptr createConnPool(Upstream::ThreadLocalCluster&, - Upstream::LoadBalancerContext* context, - absl::optional protocol); + bool valid() const { return conn_pool_data_.has_value(); } // GenericConnPool void newStream(GenericConnectionPoolCallbacks& callbacks) override; @@ -88,32 +69,16 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba Upstream::HostDescriptionConstSharedPtr host, StreamInfo::StreamInfo& info, absl::optional) override; - void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr, bool); - void onHttpPoolReady(Upstream::HostDescriptionConstSharedPtr& host, - Ssl::ConnectionInfoConstSharedPtr ssl_info); - class Callbacks { public: Callbacks(HttpConnPool& conn_pool, Upstream::HostDescriptionConstSharedPtr host, Ssl::ConnectionInfoConstSharedPtr ssl_info) : conn_pool_(&conn_pool), host_(host), ssl_info_(ssl_info) {} virtual ~Callbacks() = default; - virtual void onSuccess(Http::RequestEncoder* request_encoder) { + virtual void onSuccess(Http::RequestEncoder& request_encoder) { ASSERT(conn_pool_ != nullptr); - if (!Runtime::runtimeFeatureEnabled( - "envoy.restart_features.upstream_http_filters_with_tcp_proxy")) { - ASSERT(request_encoder != nullptr); - conn_pool_->onGenericPoolReady(host_, request_encoder->getStream().connectionInfoProvider(), - ssl_info_); - return; - } - - Network::ConnectionInfoProviderSharedPtr local_connection_info_provider( - std::make_shared( - Network::Utility::getCanonicalIpv4LoopbackAddress(), - Network::Utility::getCanonicalIpv4LoopbackAddress())); - - conn_pool_->onGenericPoolReady(host_, *local_connection_info_provider.get(), ssl_info_); + conn_pool_->onGenericPoolReady(host_, request_encoder.getStream().connectionInfoProvider(), + ssl_info_); } virtual void onFailure() { ASSERT(conn_pool_ != nullptr); @@ -139,12 +104,9 @@ class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callba absl::optional conn_pool_data_{}; Http::ConnectionPool::Cancellable* upstream_handle_{}; GenericConnectionPoolCallbacks* callbacks_{}; - Http::StreamDecoderFilterCallbacks* decoder_filter_callbacks_; Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks_; std::unique_ptr upstream_; - std::unique_ptr combined_upstream_; StreamInfo::StreamInfo& downstream_info_; - std::unique_ptr generic_conn_pool_; }; class TcpUpstream : public GenericUpstream { @@ -168,7 +130,6 @@ class HttpUpstream : public GenericUpstream, protected Http::StreamCallbacks { public: using TunnelingConfig = envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig; - HttpUpstream(Tcp::ConnectionPool::UpstreamCallbacks& callbacks, const TunnelingConfigHelper& config, StreamInfo::StreamInfo& downstream_info, Http::CodecType type); @@ -194,7 +155,7 @@ class HttpUpstream : public GenericUpstream, protected Http::StreamCallbacks { void onAboveWriteBufferHighWatermark() override; void onBelowWriteBufferLowWatermark() override; - virtual void setRequestEncoder(Http::RequestEncoder& request_encoder, bool is_ssl); + void setRequestEncoder(Http::RequestEncoder& request_encoder, bool is_ssl); void setConnPoolCallbacks(std::unique_ptr&& callbacks) { conn_pool_callbacks_ = std::move(callbacks); } @@ -209,13 +170,12 @@ class HttpUpstream : public GenericUpstream, protected Http::StreamCallbacks { const TunnelingConfigHelper& config_; // The downstream info that is owned by the downstream connection. StreamInfo::StreamInfo& downstream_info_; - std::unique_ptr downstream_headers_; private: - Upstream::ClusterInfoConstSharedPtr cluster_; class DecoderShim : public Http::ResponseDecoder { public: DecoderShim(HttpUpstream& parent) : parent_(parent) {} + // Http::ResponseDecoder void decode1xxHeaders(Http::ResponseHeaderMapPtr&&) override {} void decodeHeaders(Http::ResponseHeaderMapPtr&& headers, bool end_stream) override { bool is_valid_response = parent_.isValidResponse(*headers); @@ -224,7 +184,7 @@ class HttpUpstream : public GenericUpstream, protected Http::StreamCallbacks { if (!is_valid_response || end_stream) { parent_.resetEncoder(Network::ConnectionEvent::LocalClose); } else if (parent_.conn_pool_callbacks_ != nullptr) { - parent_.conn_pool_callbacks_->onSuccess(parent_.request_encoder_); + parent_.conn_pool_callbacks_->onSuccess(*parent_.request_encoder_); parent_.conn_pool_callbacks_.reset(); } } @@ -264,133 +224,5 @@ class HttpUpstream : public GenericUpstream, protected Http::StreamCallbacks { std::unique_ptr conn_pool_callbacks_; }; -class CombinedUpstream : public GenericUpstream, - protected Http::StreamCallbacks, - public Envoy::Router::RouterFilterInterface { -public: - CombinedUpstream(HttpConnPool& http_conn_pool, Tcp::ConnectionPool::UpstreamCallbacks& callbacks, - Http::StreamDecoderFilterCallbacks& decoder_callbacks, - const TunnelingConfigHelper& config, StreamInfo::StreamInfo& downstream_info); - ~CombinedUpstream() override = default; - using UpstreamRequest = Router::UpstreamRequest; - Http::ResponseDecoder& responseDecoder() { return response_decoder_; } - void doneReading(); - void doneWriting(); - using UpstreamRequestPtr = std::unique_ptr; - void setRouterUpstreamRequest(UpstreamRequestPtr); - void newStream(GenericConnectionPoolCallbacks& callbacks); - void encodeData(Buffer::Instance& data, bool end_stream) override; - Tcp::ConnectionPool::ConnectionData* onDownstreamEvent(Network::ConnectionEvent event) override; - bool isValidResponse(const Http::ResponseHeaderMap&); - bool readDisable(bool disable) override; - void setConnPoolCallbacks(std::unique_ptr&& callbacks) { - conn_pool_callbacks_ = std::move(callbacks); - } - void addBytesSentCallback(Network::Connection::BytesSentCb) override{}; - // HTTP upstream must not implement converting upstream transport - // socket from non-secure to secure mode. - bool startUpstreamSecureTransport() override { return false; } - Ssl::ConnectionInfoConstSharedPtr getUpstreamConnectionSslInfo() override { return nullptr; } - - // Http::StreamCallbacks - void onResetStream(Http::StreamResetReason reason, - absl::string_view transport_failure_reason) override; - void onAboveWriteBufferHighWatermark() override; - void onBelowWriteBufferLowWatermark() override; - - // Router::RouterFilterInterface - void onUpstreamHeaders(uint64_t response_code, Http::ResponseHeaderMapPtr&& headers, - UpstreamRequest& upstream_request, bool end_stream) override; - void onUpstreamData(Buffer::Instance& data, UpstreamRequest& upstream_request, - bool end_stream) override; - void onUpstream1xxHeaders(Http::ResponseHeaderMapPtr&&, UpstreamRequest&) override {} - void onUpstreamTrailers(Http::ResponseTrailerMapPtr&&, UpstreamRequest&) override; - void onUpstreamMetadata(Http::MetadataMapPtr&&) override {} - void onUpstreamReset(Http::StreamResetReason stream_reset_reason, - absl::string_view transport_failure_reason, UpstreamRequest&) override; - void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host, - bool pool_success) override { - parent_.onUpstreamHostSelected(host, pool_success); - } - void onPerTryTimeout(UpstreamRequest&) override {} - void onPerTryIdleTimeout(UpstreamRequest&) override {} - void onStreamMaxDurationReached(UpstreamRequest&) override {} - Http::StreamDecoderFilterCallbacks* callbacks() override { return &decoder_filter_callbacks_; } - Upstream::ClusterInfoConstSharedPtr cluster() override { - return decoder_filter_callbacks_.clusterInfo(); - } - Router::FilterConfig& config() override { - return const_cast(config_.routerFilterConfig()); - } - Router::TimeoutData timeout() override { return {}; } - absl::optional dynamicMaxStreamDuration() const override { - return absl::nullopt; - } - Http::RequestHeaderMap* downstreamHeaders() override; - Http::RequestTrailerMap* downstreamTrailers() override { return nullptr; } - bool downstreamResponseStarted() const override { return false; } - bool downstreamEndStream() const override { return false; } - uint32_t attemptCount() const override { return 0; } - -protected: - void onResetEncoder(Network::ConnectionEvent event, bool inform_downstream = true); - - // The config object that is owned by the downstream network filter chain factory. - const TunnelingConfigHelper& config_; - // The downstream info that is owned by the downstream connection. - StreamInfo::StreamInfo& downstream_info_; - std::list upstream_requests_; - std::unique_ptr downstream_headers_; - HttpConnPool& parent_; - -private: - Http::StreamDecoderFilterCallbacks& decoder_filter_callbacks_; - class DecoderShim : public Http::ResponseDecoder { - public: - DecoderShim(CombinedUpstream& parent) : parent_(parent) {} - // Http::ResponseDecoder - void decode1xxHeaders(Http::ResponseHeaderMapPtr&&) override {} - void decodeHeaders(Http::ResponseHeaderMapPtr&& headers, bool end_stream) override { - bool is_valid_response = parent_.isValidResponse(*headers); - parent_.config_.propagateResponseHeaders(std::move(headers), - parent_.downstream_info_.filterState()); - if (!is_valid_response || end_stream) { - parent_.onResetEncoder(Network::ConnectionEvent::LocalClose); - } else if (parent_.conn_pool_callbacks_ != nullptr) { - parent_.conn_pool_callbacks_->onSuccess(nullptr /*parent_.request_encoder_*/); - parent_.conn_pool_callbacks_.reset(); - } - } - void decodeData(Buffer::Instance& data, bool end_stream) override { - parent_.upstream_callbacks_.onUpstreamData(data, end_stream); - if (end_stream) { - parent_.doneReading(); - } - } - void decodeTrailers(Http::ResponseTrailerMapPtr&& trailers) override { - parent_.config_.propagateResponseTrailers(std::move(trailers), - parent_.downstream_info_.filterState()); - if (Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.tcp_tunneling_send_downstream_fin_on_upstream_trailers")) { - Buffer::OwnedImpl data; - parent_.upstream_callbacks_.onUpstreamData(data, /* end_stream = */ true); - } - parent_.doneReading(); - } - void decodeMetadata(Http::MetadataMapPtr&&) override {} - void dumpState(std::ostream& os, int indent_level) const override { - DUMP_STATE_UNIMPLEMENTED(DecoderShim); - } - - private: - CombinedUpstream& parent_; - }; - DecoderShim response_decoder_; - Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks_; - std::unique_ptr conn_pool_callbacks_; - bool read_half_closed_{}; - bool write_half_closed_{}; -}; - } // namespace TcpProxy } // namespace Envoy diff --git a/source/extensions/upstreams/http/http/upstream_request.h b/source/extensions/upstreams/http/http/upstream_request.h index db8adfc2cc14..d7b29d7c8d53 100644 --- a/source/extensions/upstreams/http/http/upstream_request.h +++ b/source/extensions/upstreams/http/http/upstream_request.h @@ -73,7 +73,6 @@ class HttpUpstream : public Router::GenericUpstream, public Envoy::Http::StreamC void encodeTrailers(const Envoy::Http::RequestTrailerMap& trailers) override { request_encoder_->encodeTrailers(trailers); } - void enableHalfClose() override { request_encoder_->enableTcpTunneling(); } void readDisable(bool disable) override { request_encoder_->getStream().readDisable(disable); } diff --git a/source/extensions/upstreams/http/tcp/upstream_request.h b/source/extensions/upstreams/http/tcp/upstream_request.h index 8c7431250c1e..87e9431fbd30 100644 --- a/source/extensions/upstreams/http/tcp/upstream_request.h +++ b/source/extensions/upstreams/http/tcp/upstream_request.h @@ -72,7 +72,6 @@ class TcpUpstream : public Router::GenericUpstream, void encodeMetadata(const Envoy::Http::MetadataMapVector&) override {} Envoy::Http::Status encodeHeaders(const Envoy::Http::RequestHeaderMap&, bool end_stream) override; void encodeTrailers(const Envoy::Http::RequestTrailerMap&) override; - void enableHalfClose() override {} void readDisable(bool disable) override; void resetStream() override; void setAccount(Buffer::BufferMemoryAccountSharedPtr) override {} diff --git a/source/extensions/upstreams/http/udp/upstream_request.h b/source/extensions/upstreams/http/udp/upstream_request.h index b5f6e25e6540..4d4a132411e1 100644 --- a/source/extensions/upstreams/http/udp/upstream_request.h +++ b/source/extensions/upstreams/http/udp/upstream_request.h @@ -77,7 +77,6 @@ class UdpUpstream : public Router::GenericUpstream, void encodeTrailers(const Envoy::Http::RequestTrailerMap&) override {} void readDisable(bool) override {} void resetStream() override; - void enableHalfClose() override {} void setAccount(Buffer::BufferMemoryAccountSharedPtr) override {} const StreamInfo::BytesMeterSharedPtr& bytesMeter() override { return bytes_meter_; } diff --git a/source/extensions/upstreams/tcp/generic/BUILD b/source/extensions/upstreams/tcp/generic/BUILD index ea02233167e3..a29fa7133934 100644 --- a/source/extensions/upstreams/tcp/generic/BUILD +++ b/source/extensions/upstreams/tcp/generic/BUILD @@ -18,7 +18,6 @@ envoy_cc_extension( ], visibility = ["//visibility:public"], deps = [ - "//envoy/http:filter_interface", "//envoy/stream_info:bool_accessor_interface", "//envoy/stream_info:filter_state_interface", "//source/common/http:codec_client_lib", diff --git a/source/extensions/upstreams/tcp/generic/config.cc b/source/extensions/upstreams/tcp/generic/config.cc index d3e33fdb83f8..e688ab84a510 100644 --- a/source/extensions/upstreams/tcp/generic/config.cc +++ b/source/extensions/upstreams/tcp/generic/config.cc @@ -18,7 +18,6 @@ TcpProxy::GenericConnPoolPtr GenericConnPoolFactory::createGenericConnPool( Upstream::ThreadLocalCluster& thread_local_cluster, TcpProxy::TunnelingConfigHelperOptConstRef config, Upstream::LoadBalancerContext* context, Envoy::Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, - Http::StreamDecoderFilterCallbacks& stream_decoder_callbacks, StreamInfo::StreamInfo& downstream_info) const { if (config.has_value() && !disableTunnelingByFilterState(downstream_info)) { Http::CodecType pool_type; @@ -31,8 +30,7 @@ TcpProxy::GenericConnPoolPtr GenericConnPoolFactory::createGenericConnPool( pool_type = Http::CodecType::HTTP1; } auto ret = std::make_unique( - thread_local_cluster, context, *config, upstream_callbacks, stream_decoder_callbacks, - pool_type, downstream_info); + thread_local_cluster, context, *config, upstream_callbacks, pool_type, downstream_info); return (ret->valid() ? std::move(ret) : nullptr); } auto ret = std::make_unique(thread_local_cluster, context, diff --git a/source/extensions/upstreams/tcp/generic/config.h b/source/extensions/upstreams/tcp/generic/config.h index e7e87a0145c3..c4fee935ef96 100644 --- a/source/extensions/upstreams/tcp/generic/config.h +++ b/source/extensions/upstreams/tcp/generic/config.h @@ -1,7 +1,6 @@ #pragma once #include "envoy/extensions/upstreams/tcp/generic/v3/generic_connection_pool.pb.h" -#include "envoy/http/filter.h" #include "envoy/registry/registry.h" #include "envoy/tcp/upstream.h" @@ -23,7 +22,6 @@ class GenericConnPoolFactory : public TcpProxy::GenericConnPoolFactory { TcpProxy::TunnelingConfigHelperOptConstRef config, Upstream::LoadBalancerContext* context, Envoy::Tcp::ConnectionPool::UpstreamCallbacks& upstream_callbacks, - Http::StreamDecoderFilterCallbacks& stream_decoder_callbacks, StreamInfo::StreamInfo& downstream_info) const override; ProtobufTypes::MessagePtr createEmptyConfigProto() override { diff --git a/test/common/router/upstream_request_test.cc b/test/common/router/upstream_request_test.cc index bb9a8185d4b4..67a6b003cd36 100644 --- a/test/common/router/upstream_request_test.cc +++ b/test/common/router/upstream_request_test.cc @@ -33,9 +33,8 @@ class UpstreamRequestTest : public testing::Test { void initialize() { auto conn_pool = std::make_unique>(); conn_pool_ = conn_pool.get(); - upstream_request_ = - std::make_unique(router_filter_interface_, std::move(conn_pool), false, - true, false /*enable_tcp_tunneling*/); + upstream_request_ = std::make_unique(router_filter_interface_, + std::move(conn_pool), false, true); } Http::FilterFactoryCb createDecoderFilterFactoryCb(Http::StreamDecoderFilterSharedPtr filter) { return [filter](Http::FilterChainFactoryCallbacks& callbacks) { diff --git a/test/common/tcp_proxy/BUILD b/test/common/tcp_proxy/BUILD index d6c47b8a9cbc..0630b106fd5c 100644 --- a/test/common/tcp_proxy/BUILD +++ b/test/common/tcp_proxy/BUILD @@ -79,13 +79,9 @@ envoy_cc_test( deps = [ "//source/common/tcp_proxy", "//test/mocks/http:http_mocks", - "//test/mocks/router:router_filter_interface", - "//test/mocks/router:upstream_request", "//test/mocks/server:factory_context_mocks", - "//test/mocks/stats:stats_mocks", "//test/mocks/tcp:tcp_mocks", "//test/mocks/upstream:cluster_manager_mocks", - "//test/mocks/upstream:load_balancer_context_mock", "//test/test_common:test_runtime_lib", ], ) diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index 144e50f2fbbc..41ad163dba41 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -171,7 +171,7 @@ class TcpProxyTest : public TcpProxyTestBase { std::shared_ptr> mock_access_logger_; }; -TEST_P(TcpProxyTest, ExplicitCluster) { +TEST_F(TcpProxyTest, ExplicitCluster) { configure(defaultConfig()); NiceMock connection; @@ -179,7 +179,7 @@ TEST_P(TcpProxyTest, ExplicitCluster) { } // Tests that half-closes are proxied and don't themselves cause any connection to be closed. -TEST_P(TcpProxyTest, HalfCloseProxy) { +TEST_F(TcpProxyTest, HalfCloseProxy) { setup(1); EXPECT_CALL(filter_callbacks_.connection_, close(_)).Times(0); @@ -200,7 +200,7 @@ TEST_P(TcpProxyTest, HalfCloseProxy) { } // Test with an explicitly configured upstream. -TEST_P(TcpProxyTest, ExplicitFactory) { +TEST_F(TcpProxyTest, ExplicitFactory) { // Explicitly configure an HTTP upstream, to test factory creation. auto& info = factory_context_.server_factory_context_.cluster_manager_.thread_local_cluster_ .cluster_.info_; @@ -224,7 +224,7 @@ TEST_P(TcpProxyTest, ExplicitFactory) { } // Test nothing bad happens if an invalid factory is configured. -TEST_P(TcpProxyTest, BadFactory) { +TEST_F(TcpProxyTest, BadFactory) { auto& info = factory_context_.server_factory_context_.cluster_manager_.thread_local_cluster_ .cluster_.info_; info->upstream_config_ = std::make_unique(); @@ -262,7 +262,7 @@ TEST_P(TcpProxyTest, BadFactory) { } // Test that downstream is closed after an upstream LocalClose. -TEST_P(TcpProxyTest, UpstreamLocalDisconnect) { +TEST_F(TcpProxyTest, UpstreamLocalDisconnect) { setup(1); raiseEventUpstreamConnected(0); @@ -280,7 +280,7 @@ TEST_P(TcpProxyTest, UpstreamLocalDisconnect) { } // Test that downstream is closed after an upstream RemoteClose. -TEST_P(TcpProxyTest, UpstreamRemoteDisconnect) { +TEST_F(TcpProxyTest, UpstreamRemoteDisconnect) { setup(1); timeSystem().advanceTimeWait(std::chrono::microseconds(20)); @@ -304,7 +304,7 @@ TEST_P(TcpProxyTest, UpstreamRemoteDisconnect) { } // Test that reconnect is attempted after a local connect failure -TEST_P(TcpProxyTest, ConnectAttemptsUpstreamLocalFail) { +TEST_F(TcpProxyTest, ConnectAttemptsUpstreamLocalFail) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_max_connect_attempts()->set_value(2); @@ -326,7 +326,7 @@ TEST_P(TcpProxyTest, ConnectAttemptsUpstreamLocalFail) { } // Make sure that the tcp proxy code handles reentrant calls to onPoolFailure. -TEST_P(TcpProxyTest, ConnectAttemptsUpstreamLocalFailReentrant) { +TEST_F(TcpProxyTest, ConnectAttemptsUpstreamLocalFailReentrant) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_max_connect_attempts()->set_value(2); @@ -350,7 +350,7 @@ TEST_P(TcpProxyTest, ConnectAttemptsUpstreamLocalFailReentrant) { } // Test that reconnect is attempted after a remote connect failure -TEST_P(TcpProxyTest, ConnectAttemptsUpstreamRemoteFail) { +TEST_F(TcpProxyTest, ConnectAttemptsUpstreamRemoteFail) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_max_connect_attempts()->set_value(2); setup(2, config); @@ -364,7 +364,7 @@ TEST_P(TcpProxyTest, ConnectAttemptsUpstreamRemoteFail) { } // Test that reconnect is attempted after a connect timeout. -TEST_P(TcpProxyTest, ConnectAttemptsUpstreamTimeout) { +TEST_F(TcpProxyTest, ConnectAttemptsUpstreamTimeout) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_max_connect_attempts()->set_value(2); setup(2, config); @@ -378,7 +378,7 @@ TEST_P(TcpProxyTest, ConnectAttemptsUpstreamTimeout) { } // Test that only the configured number of connect attempts occur -TEST_P(TcpProxyTest, ConnectAttemptsLimit) { +TEST_F(TcpProxyTest, ConnectAttemptsLimit) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = accessLogConfig("%RESPONSE_FLAGS%"); config.mutable_max_connect_attempts()->set_value(3); @@ -409,7 +409,7 @@ TEST_P(TcpProxyTest, ConnectAttemptsLimit) { EXPECT_EQ(access_log_data_, "UF,URX"); } -TEST_P(TcpProxyTest, ConnectedNoOp) { +TEST_F(TcpProxyTest, ConnectedNoOp) { setup(1); raiseEventUpstreamConnected(0); @@ -419,7 +419,7 @@ TEST_P(TcpProxyTest, ConnectedNoOp) { } // Test that the tcp proxy sends the correct notifications to the outlier detector -TEST_P(TcpProxyTest, OutlierDetection) { +TEST_F(TcpProxyTest, OutlierDetection) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_max_connect_attempts()->set_value(3); setup(3, config); @@ -437,7 +437,7 @@ TEST_P(TcpProxyTest, OutlierDetection) { raiseEventUpstreamConnected(2); } -TEST_P(TcpProxyTest, UpstreamDisconnectDownstreamFlowControl) { +TEST_F(TcpProxyTest, UpstreamDisconnectDownstreamFlowControl) { setup(1); raiseEventUpstreamConnected(0); @@ -459,7 +459,7 @@ TEST_P(TcpProxyTest, UpstreamDisconnectDownstreamFlowControl) { filter_callbacks_.connection_.runLowWatermarkCallbacks(); } -TEST_P(TcpProxyTest, DownstreamDisconnectRemote) { +TEST_F(TcpProxyTest, DownstreamDisconnectRemote) { setup(1); raiseEventUpstreamConnected(0); @@ -476,7 +476,7 @@ TEST_P(TcpProxyTest, DownstreamDisconnectRemote) { filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); } -TEST_P(TcpProxyTest, DownstreamDisconnectLocal) { +TEST_F(TcpProxyTest, DownstreamDisconnectLocal) { setup(1); raiseEventUpstreamConnected(0); @@ -493,7 +493,7 @@ TEST_P(TcpProxyTest, DownstreamDisconnectLocal) { filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::LocalClose); } -TEST_P(TcpProxyTest, UpstreamConnectTimeout) { +TEST_F(TcpProxyTest, UpstreamConnectTimeout) { setup(1, accessLogConfig("%RESPONSE_FLAGS%")); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush, _)); @@ -503,7 +503,7 @@ TEST_P(TcpProxyTest, UpstreamConnectTimeout) { EXPECT_EQ(access_log_data_, "UF,URX"); } -TEST_P(TcpProxyTest, UpstreamClusterNotFound) { +TEST_F(TcpProxyTest, UpstreamClusterNotFound) { setup(0, accessLogConfig("%RESPONSE_FLAGS%")); EXPECT_CALL(factory_context_.server_factory_context_.cluster_manager_, getThreadLocalCluster(_)) @@ -514,7 +514,7 @@ TEST_P(TcpProxyTest, UpstreamClusterNotFound) { EXPECT_EQ(access_log_data_.value(), "NC"); } -TEST_P(TcpProxyTest, NoHost) { +TEST_F(TcpProxyTest, NoHost) { EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush, _)); setup(0, accessLogConfig("%RESPONSE_FLAGS%")); EXPECT_EQ(Network::FilterStatus::StopIteration, filter_->onNewConnection()); @@ -522,78 +522,7 @@ TEST_P(TcpProxyTest, NoHost) { EXPECT_EQ(access_log_data_, "UH"); } -// Tests StreamDecoderFilterCallbacks interface implementation -TEST_P(TcpProxyTest, StreamDecoderFilterCallbacks) { - envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = - accessLogConfig("%RESPONSE_FLAGS%"); - config.mutable_tunneling_config()->set_hostname("www.example.com"); - configure(config); - NiceMock thread_local_cluster_; - auto cluster_info = std::make_shared>(); - // EXPECT_CALL(factory_context_.serverFactoryContext().clusterManager(), getThreadLocalCluster(_)) - // .WillRepeatedly(Return(&thread_local_cluster_)); - EXPECT_CALL(thread_local_cluster_, info()).WillRepeatedly(Return(cluster_info)); - filter_ = - std::make_unique(config_, factory_context_.serverFactoryContext().clusterManager()); - filter_->initializeReadFilterCallbacks(filter_callbacks_); - auto stream_decoder_callbacks = Filter::HttpStreamDecoderFilterCallbacks(filter_.get()); - EXPECT_NO_THROW(stream_decoder_callbacks.streamId()); - EXPECT_NO_THROW(stream_decoder_callbacks.connection()); - EXPECT_NO_THROW(stream_decoder_callbacks.dispatcher()); - EXPECT_ENVOY_BUG( - { stream_decoder_callbacks.resetStream(Http::StreamResetReason::RemoteReset, ""); }, - "Not implemented"); - EXPECT_NO_THROW(stream_decoder_callbacks.streamInfo()); - EXPECT_NO_THROW(stream_decoder_callbacks.scope()); - EXPECT_NO_THROW(stream_decoder_callbacks.route()); - EXPECT_NO_THROW(stream_decoder_callbacks.continueDecoding()); - EXPECT_NO_THROW(stream_decoder_callbacks.requestHeaders()); - EXPECT_NO_THROW(stream_decoder_callbacks.requestTrailers()); - EXPECT_NO_THROW(stream_decoder_callbacks.responseHeaders()); - EXPECT_NO_THROW(stream_decoder_callbacks.responseTrailers()); - EXPECT_NO_THROW(stream_decoder_callbacks.encodeMetadata(nullptr)); - EXPECT_NO_THROW(stream_decoder_callbacks.onDecoderFilterAboveWriteBufferHighWatermark()); - EXPECT_NO_THROW(stream_decoder_callbacks.onDecoderFilterBelowWriteBufferLowWatermark()); - EXPECT_NO_THROW(stream_decoder_callbacks.setDecoderBufferLimit(uint32_t{0})); - EXPECT_NO_THROW(stream_decoder_callbacks.decoderBufferLimit()); - EXPECT_NO_THROW(stream_decoder_callbacks.recreateStream(nullptr)); - EXPECT_NO_THROW(stream_decoder_callbacks.getUpstreamSocketOptions()); - Network::Socket::OptionsSharedPtr sock_options = - Network::SocketOptionFactory::buildIpTransparentOptions(); - EXPECT_NO_THROW(stream_decoder_callbacks.addUpstreamSocketOptions(sock_options)); - EXPECT_NO_THROW(stream_decoder_callbacks.mostSpecificPerFilterConfig()); - EXPECT_NO_THROW(stream_decoder_callbacks.account()); - EXPECT_NO_THROW(stream_decoder_callbacks.setUpstreamOverrideHost( - Upstream::LoadBalancerContext::OverrideHost(std::make_pair("foo", true)))); - EXPECT_NO_THROW(stream_decoder_callbacks.http1StreamEncoderOptions()); - EXPECT_NO_THROW(stream_decoder_callbacks.downstreamCallbacks()); - EXPECT_NO_THROW(stream_decoder_callbacks.upstreamCallbacks()); - EXPECT_NO_THROW(stream_decoder_callbacks.upstreamOverrideHost()); - EXPECT_NO_THROW(stream_decoder_callbacks.resetIdleTimer()); - EXPECT_NO_THROW(stream_decoder_callbacks.filterConfigName()); - EXPECT_NO_THROW(stream_decoder_callbacks.activeSpan()); - EXPECT_NO_THROW(stream_decoder_callbacks.tracingConfig()); - Buffer::OwnedImpl inject_data; - EXPECT_NO_THROW(stream_decoder_callbacks.addDecodedData(inject_data, false)); - EXPECT_NO_THROW(stream_decoder_callbacks.injectDecodedDataToFilterChain(inject_data, false)); - EXPECT_NO_THROW(stream_decoder_callbacks.addDecodedData(inject_data, false)); - EXPECT_NO_THROW(stream_decoder_callbacks.addDecodedTrailers()); - EXPECT_NO_THROW(stream_decoder_callbacks.addDecodedMetadata()); - EXPECT_NO_THROW(stream_decoder_callbacks.decodingBuffer()); - auto func = [](Buffer::Instance&) {}; - EXPECT_NO_THROW(stream_decoder_callbacks.modifyDecodingBuffer(func)); - EXPECT_NO_THROW(stream_decoder_callbacks.encode1xxHeaders(nullptr)); - EXPECT_NO_THROW(stream_decoder_callbacks.informationalHeaders()); - EXPECT_NO_THROW(stream_decoder_callbacks.encodeHeaders(nullptr, false, "")); - EXPECT_NO_THROW(stream_decoder_callbacks.encodeData(inject_data, false)); - EXPECT_NO_THROW(stream_decoder_callbacks.encodeTrailers(nullptr)); - EXPECT_NO_THROW(stream_decoder_callbacks.setDecoderBufferLimit(0)); - std::array buffer; - OutputBufferStream ostream{buffer.data(), buffer.size()}; - EXPECT_NO_THROW(stream_decoder_callbacks.dumpState(ostream, 0)); -} - -TEST_P(TcpProxyTest, RouteWithMetadataMatch) { +TEST_F(TcpProxyTest, RouteWithMetadataMatch) { auto v1 = ProtobufWkt::Value(); v1.set_string_value("v1"); auto v2 = ProtobufWkt::Value(); @@ -633,7 +562,7 @@ TEST_P(TcpProxyTest, RouteWithMetadataMatch) { // Tests that the endpoint selector of a weighted cluster gets included into the // LoadBalancerContext. -TEST_P(TcpProxyTest, WeightedClusterWithMetadataMatch) { +TEST_F(TcpProxyTest, WeightedClusterWithMetadataMatch) { const std::string yaml = R"EOF( stat_prefix: name weighted_clusters: @@ -730,7 +659,7 @@ TEST_P(TcpProxyTest, WeightedClusterWithMetadataMatch) { } // Test that metadata match criteria provided on the StreamInfo is used. -TEST_P(TcpProxyTest, StreamInfoDynamicMetadata) { +TEST_F(TcpProxyTest, StreamInfoDynamicMetadata) { configure(defaultConfig()); ProtobufWkt::Value val; @@ -768,7 +697,7 @@ TEST_P(TcpProxyTest, StreamInfoDynamicMetadata) { // Test that if both streamInfo and configuration add metadata match criteria, they // are merged. -TEST_P(TcpProxyTest, StreamInfoDynamicMetadataAndConfigMerged) { +TEST_F(TcpProxyTest, StreamInfoDynamicMetadataAndConfigMerged) { const std::string yaml = R"EOF( stat_prefix: name weighted_clusters: @@ -829,7 +758,7 @@ TEST_P(TcpProxyTest, StreamInfoDynamicMetadataAndConfigMerged) { EXPECT_EQ(hv2, effective_criterions[2]->value()); } -TEST_P(TcpProxyTest, DisconnectBeforeData) { +TEST_F(TcpProxyTest, DisconnectBeforeData) { configure(defaultConfig()); filter_ = std::make_unique(config_, factory_context_.server_factory_context_.cluster_manager_); @@ -840,7 +769,7 @@ TEST_P(TcpProxyTest, DisconnectBeforeData) { // Test that if the downstream connection is closed before the upstream connection // is established, the upstream connection is cancelled. -TEST_P(TcpProxyTest, RemoteClosedBeforeUpstreamConnected) { +TEST_F(TcpProxyTest, RemoteClosedBeforeUpstreamConnected) { setup(1); EXPECT_CALL(*conn_pool_handles_.at(0), cancel(Tcp::ConnectionPool::CancelPolicy::CloseExcess)); filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); @@ -848,13 +777,13 @@ TEST_P(TcpProxyTest, RemoteClosedBeforeUpstreamConnected) { // Test that if the downstream connection is closed before the upstream connection // is established, the upstream connection is cancelled. -TEST_P(TcpProxyTest, LocalClosedBeforeUpstreamConnected) { +TEST_F(TcpProxyTest, LocalClosedBeforeUpstreamConnected) { setup(1); EXPECT_CALL(*conn_pool_handles_.at(0), cancel(Tcp::ConnectionPool::CancelPolicy::CloseExcess)); filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::LocalClose); } -TEST_P(TcpProxyTest, UpstreamConnectFailure) { +TEST_F(TcpProxyTest, UpstreamConnectFailure) { setup(1, accessLogConfig("%RESPONSE_FLAGS%")); EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::NoFlush, _)); @@ -870,7 +799,7 @@ TEST_P(TcpProxyTest, UpstreamConnectFailure) { EXPECT_EQ(access_log_data_, "UF,URX"); } -TEST_P(TcpProxyTest, UpstreamConnectionLimit) { +TEST_F(TcpProxyTest, UpstreamConnectionLimit) { configure(accessLogConfig("%RESPONSE_FLAGS%")); factory_context_.server_factory_context_.cluster_manager_.thread_local_cluster_.cluster_.info_ ->resetResourceManager(0, 0, 0, 0, 0); @@ -887,7 +816,7 @@ TEST_P(TcpProxyTest, UpstreamConnectionLimit) { EXPECT_EQ(access_log_data_, "UO"); } -TEST_P(TcpProxyTest, IdleTimeoutObjectFactory) { +TEST_F(TcpProxyTest, IdleTimeoutObjectFactory) { const std::string name = "envoy.tcp_proxy.per_connection_idle_timeout_ms"; auto* factory = Registry::FactoryRegistry::getFactory(name); @@ -899,7 +828,7 @@ TEST_P(TcpProxyTest, IdleTimeoutObjectFactory) { EXPECT_EQ(duration_in_milliseconds, object->serializeAsString()); } -TEST_P(TcpProxyTest, InvalidIdleTimeoutObjectFactory) { +TEST_F(TcpProxyTest, InvalidIdleTimeoutObjectFactory) { const std::string name = "envoy.tcp_proxy.per_connection_idle_timeout_ms"; auto* factory = Registry::FactoryRegistry::getFactory(name); @@ -908,7 +837,7 @@ TEST_P(TcpProxyTest, InvalidIdleTimeoutObjectFactory) { ASSERT_EQ(nullptr, factory->createFromBytes("not_a_number")); } -TEST_P(TcpProxyTest, IdleTimeoutWithFilterStateOverride) { +TEST_F(TcpProxyTest, IdleTimeoutWithFilterStateOverride) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -948,7 +877,7 @@ TEST_P(TcpProxyTest, IdleTimeoutWithFilterStateOverride) { // Tests that the idle timer closes both connections, and gets updated when either // connection has activity. -TEST_P(TcpProxyTest, IdleTimeout) { +TEST_F(TcpProxyTest, IdleTimeout) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -978,7 +907,7 @@ TEST_P(TcpProxyTest, IdleTimeout) { } // Tests that the idle timer is disabled when the downstream connection is closed. -TEST_P(TcpProxyTest, IdleTimerDisabledDownstreamClose) { +TEST_F(TcpProxyTest, IdleTimerDisabledDownstreamClose) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -992,7 +921,7 @@ TEST_P(TcpProxyTest, IdleTimerDisabledDownstreamClose) { } // Tests that the idle timer is disabled when the upstream connection is closed. -TEST_P(TcpProxyTest, IdleTimerDisabledUpstreamClose) { +TEST_F(TcpProxyTest, IdleTimerDisabledUpstreamClose) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -1006,7 +935,7 @@ TEST_P(TcpProxyTest, IdleTimerDisabledUpstreamClose) { } // Tests that flushing data during an idle timeout doesn't cause problems. -TEST_P(TcpProxyTest, IdleTimeoutWithOutstandingDataFlushed) { +TEST_F(TcpProxyTest, IdleTimeoutWithOutstandingDataFlushed) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -1058,7 +987,7 @@ TEST_P(TcpProxyTest, IdleTimeoutWithOutstandingDataFlushed) { // Checks that %UPSTREAM_WIRE_BYTES_SENT%, %UPSTREAM_WIRE_BYTES_RECEIVED%, // %DOWNSTREAM_WIRE_BYTES_SENT%, and %DOWNSTREAM_WIRE_BYTES_RECEIVED% are // correctly logged. -TEST_P(TcpProxyTest, AccessLogBytesMeterData) { +TEST_F(TcpProxyTest, AccessLogBytesMeterData) { setup(1, accessLogConfig("%UPSTREAM_WIRE_BYTES_SENT% %UPSTREAM_WIRE_BYTES_RECEIVED% " "%DOWNSTREAM_WIRE_BYTES_SENT% %DOWNSTREAM_WIRE_BYTES_RECEIVED%")); raiseEventUpstreamConnected(0); @@ -1076,7 +1005,7 @@ TEST_P(TcpProxyTest, AccessLogBytesMeterData) { // Test that access log fields %UPSTREAM_HOST% and %UPSTREAM_CLUSTER% are correctly logged with the // observability name. -TEST_P(TcpProxyTest, AccessLogUpstreamHost) { +TEST_F(TcpProxyTest, AccessLogUpstreamHost) { setup(1, accessLogConfig("%UPSTREAM_HOST% %UPSTREAM_CLUSTER%")); raiseEventUpstreamConnected(0); filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); @@ -1085,7 +1014,7 @@ TEST_P(TcpProxyTest, AccessLogUpstreamHost) { } // Test that access log field %UPSTREAM_LOCAL_ADDRESS% is correctly logged. -TEST_P(TcpProxyTest, AccessLogUpstreamLocalAddress) { +TEST_F(TcpProxyTest, AccessLogUpstreamLocalAddress) { setup(1, accessLogConfig("%UPSTREAM_LOCAL_ADDRESS%")); raiseEventUpstreamConnected(0); filter_callbacks_.connection_.raiseEvent(Network::ConnectionEvent::RemoteClose); @@ -1094,7 +1023,7 @@ TEST_P(TcpProxyTest, AccessLogUpstreamLocalAddress) { } // Test that access log fields %DOWNSTREAM_PEER_URI_SAN% is correctly logged. -TEST_P(TcpProxyTest, AccessLogPeerUriSan) { +TEST_F(TcpProxyTest, AccessLogPeerUriSan) { filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress( Network::Utility::resolveUrl("tcp://1.1.1.2:20000")); filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( @@ -1112,7 +1041,7 @@ TEST_P(TcpProxyTest, AccessLogPeerUriSan) { } // Test that access log fields %DOWNSTREAM_TLS_SESSION_ID% is correctly logged. -TEST_P(TcpProxyTest, AccessLogTlsSessionId) { +TEST_F(TcpProxyTest, AccessLogTlsSessionId) { filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress( Network::Utility::resolveUrl("tcp://1.1.1.2:20000")); filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( @@ -1132,7 +1061,7 @@ TEST_P(TcpProxyTest, AccessLogTlsSessionId) { // Test that access log fields %DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT% and // %DOWNSTREAM_LOCAL_ADDRESS% are correctly logged. -TEST_P(TcpProxyTest, AccessLogDownstreamAddress) { +TEST_F(TcpProxyTest, AccessLogDownstreamAddress) { filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress( Network::Utility::resolveUrl("tcp://1.1.1.2:20000")); filter_callbacks_.connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( @@ -1144,7 +1073,7 @@ TEST_P(TcpProxyTest, AccessLogDownstreamAddress) { } // Test that intermediate log entry by field %ACCESS_LOG_TYPE%. -TEST_P(TcpProxyTest, IntermediateLogEntry) { +TEST_F(TcpProxyTest, IntermediateLogEntry) { auto config = accessLogConfig("%ACCESS_LOG_TYPE%"); config.mutable_access_log_options()->mutable_access_log_flush_interval()->set_seconds(1); config.mutable_idle_timeout()->set_seconds(0); @@ -1200,7 +1129,7 @@ TEST_P(TcpProxyTest, IntermediateLogEntry) { AccessLogType_Name(AccessLog::AccessLogType::TcpConnectionEnd)); } -TEST_P(TcpProxyTest, TestAccessLogOnUpstreamConnected) { +TEST_F(TcpProxyTest, TestAccessLogOnUpstreamConnected) { auto config = accessLogConfig("%UPSTREAM_HOST% %ACCESS_LOG_TYPE%"); config.mutable_access_log_options()->set_flush_access_log_on_connected(true); @@ -1222,7 +1151,7 @@ TEST_P(TcpProxyTest, TestAccessLogOnUpstreamConnected) { AccessLogType_Name(AccessLog::AccessLogType::TcpConnectionEnd))); } -TEST_P(TcpProxyTest, AccessLogUpstreamSSLConnection) { +TEST_F(TcpProxyTest, AccessLogUpstreamSSLConnection) { setup(1); NiceMock stream_info; @@ -1239,7 +1168,7 @@ TEST_P(TcpProxyTest, AccessLogUpstreamSSLConnection) { } // Tests that upstream flush works properly with no idle timeout configured. -TEST_P(TcpProxyTest, UpstreamFlushNoTimeout) { +TEST_F(TcpProxyTest, UpstreamFlushNoTimeout) { setup(1); raiseEventUpstreamConnected(0); @@ -1263,7 +1192,7 @@ TEST_P(TcpProxyTest, UpstreamFlushNoTimeout) { // Tests that upstream flush works with an idle timeout configured, but the connection // finishes draining before the timer expires. -TEST_P(TcpProxyTest, UpstreamFlushTimeoutConfigured) { +TEST_F(TcpProxyTest, UpstreamFlushTimeoutConfigured) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -1294,7 +1223,7 @@ TEST_P(TcpProxyTest, UpstreamFlushTimeoutConfigured) { } // Tests that upstream flush closes the connection when the idle timeout fires. -TEST_P(TcpProxyTest, UpstreamFlushTimeoutExpired) { +TEST_F(TcpProxyTest, UpstreamFlushTimeoutExpired) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); config.mutable_idle_timeout()->set_seconds(1); setup(1, config); @@ -1322,7 +1251,7 @@ TEST_P(TcpProxyTest, UpstreamFlushTimeoutExpired) { // Tests that upstream flush will close a connection if it reads data from the upstream // connection after the downstream connection is closed (nowhere to send it). -TEST_P(TcpProxyTest, UpstreamFlushReceiveUpstreamData) { +TEST_F(TcpProxyTest, UpstreamFlushReceiveUpstreamData) { setup(1); raiseEventUpstreamConnected(0); @@ -1341,13 +1270,13 @@ TEST_P(TcpProxyTest, UpstreamFlushReceiveUpstreamData) { upstream_callbacks_->onUpstreamData(buffer, false); } -TEST_P(TcpProxyTest, UpstreamSocketOptionsReturnedEmpty) { +TEST_F(TcpProxyTest, UpstreamSocketOptionsReturnedEmpty) { setup(1); auto options = filter_->upstreamSocketOptions(); EXPECT_EQ(options, nullptr); } -TEST_P(TcpProxyTest, TcpProxySetRedirectRecordsToUpstream) { +TEST_F(TcpProxyTest, TcpProxySetRedirectRecordsToUpstream) { setup(1, true); EXPECT_TRUE(filter_->upstreamSocketOptions()); auto iterator = std::find_if( @@ -1367,7 +1296,7 @@ TEST_P(TcpProxyTest, TcpProxySetRedirectRecordsToUpstream) { } // Tests that downstream connection can access upstream connections filter state. -TEST_P(TcpProxyTest, ShareFilterState) { +TEST_F(TcpProxyTest, ShareFilterState) { setup(1); upstream_connections_.at(0)->streamInfo().filterState()->setData( @@ -1383,7 +1312,7 @@ TEST_P(TcpProxyTest, ShareFilterState) { } // Tests that filter callback can access downstream and upstream address and ssl properties. -TEST_P(TcpProxyTest, AccessDownstreamAndUpstreamProperties) { +TEST_F(TcpProxyTest, AccessDownstreamAndUpstreamProperties) { setup(1); raiseEventUpstreamConnected(0); @@ -1396,7 +1325,7 @@ TEST_P(TcpProxyTest, AccessDownstreamAndUpstreamProperties) { upstream_connections_.at(0)->streamInfo().downstreamAddressProvider().sslConnection()); } -TEST_P(TcpProxyTest, PickClusterOnUpstreamFailure) { +TEST_F(TcpProxyTest, PickClusterOnUpstreamFailure) { auto config = defaultConfig(); set2Cluster(config); config.mutable_max_connect_attempts()->set_value(2); @@ -1425,7 +1354,7 @@ TEST_P(TcpProxyTest, PickClusterOnUpstreamFailure) { } // Verify that odcds callback does not re-pick cluster. Upstream connect failure does. -TEST_P(TcpProxyTest, OnDemandCallbackStickToTheSelectedCluster) { +TEST_F(TcpProxyTest, OnDemandCallbackStickToTheSelectedCluster) { auto config = onDemandConfig(); set2Cluster(config); config.mutable_max_connect_attempts()->set_value(2); @@ -1484,7 +1413,7 @@ TEST_P(TcpProxyTest, OnDemandCallbackStickToTheSelectedCluster) { } // Verify the on demand api is not invoked when the target thread local cluster is present. -TEST_P(TcpProxyTest, OdcdsIsIgnoredIfClusterExists) { +TEST_F(TcpProxyTest, OdcdsIsIgnoredIfClusterExists) { auto config = onDemandConfig(); setup(1, config); @@ -1503,7 +1432,7 @@ TEST_P(TcpProxyTest, OdcdsIsIgnoredIfClusterExists) { } // Verify the on demand request is cancelled if the tcp downstream connection is closed. -TEST_P(TcpProxyTest, OdcdsCancelIfConnectionClose) { +TEST_F(TcpProxyTest, OdcdsCancelIfConnectionClose) { auto config = onDemandConfig(); mock_odcds_api_handle_ = Upstream::MockOdCdsApiHandle::create().release(); @@ -1525,7 +1454,7 @@ TEST_P(TcpProxyTest, OdcdsCancelIfConnectionClose) { } // Verify a request can be served after a successful on demand cluster request. -TEST_P(TcpProxyTest, OdcdsBasicDownstreamLocalClose) { +TEST_F(TcpProxyTest, OdcdsBasicDownstreamLocalClose) { auto config = onDemandConfig(); mock_odcds_api_handle_ = Upstream::MockOdCdsApiHandle::create().release(); @@ -1570,7 +1499,7 @@ TEST_P(TcpProxyTest, OdcdsBasicDownstreamLocalClose) { } // Verify the connection is closed after the cluster missing callback is triggered. -TEST_P(TcpProxyTest, OdcdsClusterMissingCauseConnectionClose) { +TEST_F(TcpProxyTest, OdcdsClusterMissingCauseConnectionClose) { auto config = onDemandConfig(); mock_odcds_api_handle_ = Upstream::MockOdCdsApiHandle::create().release(); @@ -1599,7 +1528,7 @@ TEST_P(TcpProxyTest, OdcdsClusterMissingCauseConnectionClose) { } // Test that upstream transport failure message is reflected in access logs. -TEST_P(TcpProxyTest, UpstreamConnectFailureStreamInfoAccessLog) { +TEST_F(TcpProxyTest, UpstreamConnectFailureStreamInfoAccessLog) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); setup(1, accessLogConfig("%UPSTREAM_TRANSPORT_FAILURE_REASON%")); @@ -1616,7 +1545,7 @@ TEST_P(TcpProxyTest, UpstreamConnectFailureStreamInfoAccessLog) { // Test that call to tcp_proxy filter's startUpstreamSecureTransport results // in upstream's startUpstreamSecureTransport call. -TEST_P(TcpProxyTest, UpstreamStartSecureTransport) { +TEST_F(TcpProxyTest, UpstreamStartSecureTransport) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy config = defaultConfig(); setup(1, config); @@ -1625,8 +1554,6 @@ TEST_P(TcpProxyTest, UpstreamStartSecureTransport) { filter_->startUpstreamSecureTransport(); } -INSTANTIATE_TEST_SUITE_P(WithOrWithoutUpstream, TcpProxyTest, ::testing::Bool()); - TEST(PerConnectionCluster, ObjectFactory) { const std::string name = "envoy.tcp_proxy.cluster"; auto* factory = diff --git a/test/common/tcp_proxy/tcp_proxy_test_base.h b/test/common/tcp_proxy/tcp_proxy_test_base.h index a7a7770d218a..9816df3871d9 100644 --- a/test/common/tcp_proxy/tcp_proxy_test_base.h +++ b/test/common/tcp_proxy/tcp_proxy_test_base.h @@ -57,11 +57,9 @@ inline Config constructConfigFromYaml(const std::string& yaml, return {tcp_proxy, context}; } -class TcpProxyTestBase : public testing::TestWithParam { +class TcpProxyTestBase : public testing::Test { public: TcpProxyTestBase() { - scoped_runtime_.mergeValues({{"envoy.restart_features.upstream_http_filters_with_tcp_proxy", - GetParam() ? "true" : "false"}}); ON_CALL(*factory_context_.server_factory_context_.access_log_manager_.file_, write(_)) .WillByDefault(SaveArg<0>(&access_log_data_)); ON_CALL(filter_callbacks_.connection_.stream_info_, setUpstreamClusterInfo(_)) @@ -177,7 +175,6 @@ class TcpProxyTestBase : public testing::TestWithParam { Upstream::HostDescriptionConstSharedPtr upstream_host_{}; Upstream::ClusterInfoConstSharedPtr upstream_cluster_{}; std::string redirect_records_data_ = "some data"; - TestScopedRuntime scoped_runtime_; }; } // namespace TcpProxy diff --git a/test/common/tcp_proxy/upstream_test.cc b/test/common/tcp_proxy/upstream_test.cc index feca11eb4a60..d48d9690c602 100644 --- a/test/common/tcp_proxy/upstream_test.cc +++ b/test/common/tcp_proxy/upstream_test.cc @@ -6,11 +6,8 @@ #include "test/mocks/buffer/mocks.h" #include "test/mocks/http/mocks.h" #include "test/mocks/http/stream_encoder.h" -#include "test/mocks/router/router_filter_interface.h" -#include "test/mocks/router/upstream_request.h" #include "test/mocks/server/factory_context.h" #include "test/mocks/tcp/mocks.h" -#include "test/mocks/upstream/load_balancer_context.h" #include "test/test_common/environment.h" #include "test/test_common/network_utility.h" #include "test/test_common/test_runtime.h" @@ -26,7 +23,7 @@ using testing::Return; namespace Envoy { namespace TcpProxy { namespace { -using envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy; +using envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig; class HttpUpstreamTest : public testing::TestWithParam { public: @@ -40,11 +37,11 @@ class HttpUpstreamTest : public testing::TestWithParam { .WillByDefault(Return(Http::Http1StreamEncoderOptionsOptRef(stream_encoder_options_))); } EXPECT_CALL(stream_encoder_options_, enableHalfClose()).Times(AnyNumber()); - tcp_proxy_.mutable_tunneling_config()->set_hostname("default.host.com:443"); + config_message_.set_hostname("default.host.com:443"); } void setupUpstream() { - config_ = std::make_unique(scope_, tcp_proxy_, context_); + config_ = std::make_unique(config_message_, context_); upstream_ = std::make_unique(callbacks_, *this->config_, downstream_stream_info_, GetParam()); upstream_->setRequestEncoder(encoder_, true); @@ -54,9 +51,7 @@ class HttpUpstreamTest : public testing::TestWithParam { Http::MockRequestEncoder encoder_; Http::MockHttp1StreamEncoderOptions stream_encoder_options_; NiceMock callbacks_; - TcpProxy tcp_proxy_; - NiceMock store_; - Stats::MockScope& scope_{store_.mockScope()}; + TcpProxy_TunnelingConfig config_message_; std::unique_ptr config_; std::unique_ptr upstream_; NiceMock context_; @@ -151,7 +146,7 @@ TEST_P(HttpUpstreamTest, UpstreamWatermarks) { class MockHttpConnPoolCallbacks : public HttpConnPool::Callbacks { public: - MOCK_METHOD(void, onSuccess, (Http::RequestEncoder * request_encoder)); + MOCK_METHOD(void, onSuccess, (Http::RequestEncoder & request_encoder)); MOCK_METHOD(void, onFailure, ()); }; @@ -240,11 +235,11 @@ class HttpUpstreamRequestEncoderTest : public testing::TestWithParamset_hostname("default.host.com:443"); + config_message_.set_hostname("default.host.com:443"); } void setupUpstream() { - config_ = std::make_unique(scope_, tcp_proxy_, context_); + config_ = std::make_unique(config_message_, context_); upstream_ = std::make_unique(callbacks_, *this->config_, this->downstream_stream_info_, GetParam()); } @@ -264,9 +259,7 @@ class HttpUpstreamRequestEncoderTest : public testing::TestWithParam context_; std::unique_ptr upstream_; - TcpProxy tcp_proxy_; - NiceMock store_; - Stats::MockScope& scope_{store_.mockScope()}; + TcpProxy_TunnelingConfig config_message_; std::unique_ptr config_; bool is_http2_ = true; }; @@ -288,7 +281,7 @@ TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoder) { } TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderUsePost) { - this->tcp_proxy_.mutable_tunneling_config()->set_use_post(true); + this->config_message_.set_use_post(true); this->setupUpstream(); std::unique_ptr expected_headers; expected_headers = Http::createHeaderMap({ @@ -307,8 +300,8 @@ TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderUsePost) { } TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderUsePostWithCustomPath) { - this->tcp_proxy_.mutable_tunneling_config()->set_use_post(true); - this->tcp_proxy_.mutable_tunneling_config()->set_post_path("/test"); + this->config_message_.set_use_post(true); + this->config_message_.set_post_path("/test"); this->setupUpstream(); std::unique_ptr expected_headers; expected_headers = Http::createHeaderMap({ @@ -327,25 +320,25 @@ TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderUsePostWithCustomPath) { } TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderConnectWithCustomPath) { - this->tcp_proxy_.mutable_tunneling_config()->set_use_post(false); - this->tcp_proxy_.mutable_tunneling_config()->set_post_path("/test"); + this->config_message_.set_use_post(false); + this->config_message_.set_post_path("/test"); EXPECT_THROW_WITH_MESSAGE(this->setupUpstream(), EnvoyException, "Can't set a post path when POST method isn't used"); } TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHeaders) { - auto* header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + auto* header = this->config_message_.add_headers_to_add(); auto* hdr = header->mutable_header(); hdr->set_key("header0"); hdr->set_value("value0"); - header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + header = this->config_message_.add_headers_to_add(); hdr = header->mutable_header(); hdr->set_key("header1"); hdr->set_value("value1"); header->set_append_action(envoy::config::core::v3::HeaderValueOption::APPEND_IF_EXISTS_OR_ADD); - header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + header = this->config_message_.add_headers_to_add(); hdr = header->mutable_header(); hdr->set_key("header1"); hdr->set_value("value2"); @@ -367,13 +360,13 @@ TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHeaders) { } TEST_P(HttpUpstreamRequestEncoderTest, ConfigReuse) { - auto* header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + auto* header = this->config_message_.add_headers_to_add(); auto* hdr = header->mutable_header(); hdr->set_key("key"); hdr->set_value("value1"); header->set_append_action(envoy::config::core::v3::HeaderValueOption::APPEND_IF_EXISTS_OR_ADD); - header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + header = this->config_message_.add_headers_to_add(); hdr = header->mutable_header(); hdr->set_key("key"); hdr->set_value("value2"); @@ -411,12 +404,12 @@ TEST_P(HttpUpstreamRequestEncoderTest, ConfigReuse) { } TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHeadersWithDownstreamInfo) { - auto* header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + auto* header = this->config_message_.add_headers_to_add(); auto* hdr = header->mutable_header(); hdr->set_key("header0"); hdr->set_value("value0"); - header = this->tcp_proxy_.mutable_tunneling_config()->add_headers_to_add(); + header = this->config_message_.add_headers_to_add(); hdr = header->mutable_header(); hdr->set_key("downstream_local_port"); hdr->set_value("%DOWNSTREAM_LOCAL_PORT%"); @@ -445,7 +438,7 @@ TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHeadersWithDownstreamInfo) TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHostnameWithDownstreamInfoRequestedServerName) { - this->tcp_proxy_.mutable_tunneling_config()->set_hostname("%REQUESTED_SERVER_NAME%:443"); + this->config_message_.set_hostname("%REQUESTED_SERVER_NAME%:443"); this->setupUpstream(); std::unique_ptr expected_headers; @@ -469,8 +462,7 @@ TEST_P(HttpUpstreamRequestEncoderTest, } TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHostnameWithDownstreamInfoDynamicMetadata) { - this->tcp_proxy_.mutable_tunneling_config()->set_hostname( - "%DYNAMIC_METADATA(tunnel:address)%:443"); + this->config_message_.set_hostname("%DYNAMIC_METADATA(tunnel:address)%:443"); this->setupUpstream(); std::unique_ptr expected_headers; @@ -490,218 +482,6 @@ TEST_P(HttpUpstreamRequestEncoderTest, RequestEncoderHostnameWithDownstreamInfoD EXPECT_CALL(this->encoder_, encodeHeaders(HeaderMapEqualRef(expected_headers.get()), false)); this->upstream_->setRequestEncoder(this->encoder_, false); } - -class CombinedUpstreamTest : public testing::Test { -public: - CombinedUpstreamTest() { - EXPECT_CALL(encoder_, getStream()).Times(AnyNumber()); - EXPECT_CALL(encoder_, http1StreamEncoderOptions()).Times(AnyNumber()); - EXPECT_CALL(encoder_, enableTcpTunneling()).Times(AnyNumber()); - EXPECT_CALL(stream_encoder_options_, enableHalfClose()).Times(AnyNumber()); - tcp_proxy_.mutable_tunneling_config()->set_hostname("default.host.com:443"); - } - - void setup() { - tunnel_config_ = std::make_unique(scope_, tcp_proxy_, context_); - conn_pool_ = std::make_unique(cluster_, &lb_context_, *tunnel_config_, callbacks_, - decoder_callbacks_, Http::CodecType::HTTP2, - downstream_stream_info_); - upstream_ = std::make_unique(*conn_pool_, callbacks_, decoder_callbacks_, - *tunnel_config_, downstream_stream_info_); - auto mock_conn_pool = std::make_unique>(); - std::unique_ptr generic_conn_pool = std::move(mock_conn_pool); - config_ = std::make_shared(tcp_proxy_, factory_context_); - filter_ = - std::make_unique(config_, factory_context_.serverFactoryContext().clusterManager()); - filter_->initializeReadFilterCallbacks(filter_callbacks_); - auto mock_upst = std::make_unique>( - *upstream_, std::move(generic_conn_pool)); - mock_router_upstream_request_ = mock_upst.get(); - upstream_->setRouterUpstreamRequest(std::move(mock_upst)); - EXPECT_CALL(*mock_router_upstream_request_, acceptHeadersFromRouter(false)); - EXPECT_NO_THROW(tunnel_config_->serverFactoryContext()); - upstream_->newStream(*filter_); - } - - Router::MockUpstreamRequest* mock_router_upstream_request_{}; - NiceMock router_filter_interface_; - NiceMock factory_context_; - ConfigSharedPtr config_; - NiceMock filter_callbacks_; - std::unique_ptr filter_; - - NiceMock downstream_stream_info_; - Http::MockRequestEncoder encoder_; - Http::MockHttp1StreamEncoderOptions stream_encoder_options_; - NiceMock callbacks_; - TcpProxy tcp_proxy_; - NiceMock decoder_callbacks_; - NiceMock cluster_; - NiceMock lb_context_; - std::unique_ptr conn_pool_; - NiceMock store_; - Stats::MockScope& scope_{store_.mockScope()}; - std::unique_ptr tunnel_config_; - std::unique_ptr upstream_; - NiceMock context_; -}; -TEST_F(CombinedUpstreamTest, RouterFilterInterface) { - this->setup(); - EXPECT_EQ(this->upstream_->startUpstreamSecureTransport(), false); - EXPECT_EQ(this->upstream_->getUpstreamConnectionSslInfo(), nullptr); - auto mock_conn_pool = std::make_unique>(); - std::unique_ptr generic_conn_pool = std::move(mock_conn_pool); - auto mock_upst = std::make_unique>( - *this->upstream_, std::move(generic_conn_pool)); - EXPECT_NO_THROW(this->upstream_->onUpstream1xxHeaders(nullptr, *mock_upst.get())); - EXPECT_NO_THROW(this->upstream_->onUpstreamMetadata(nullptr)); - EXPECT_NO_THROW(this->upstream_->onPerTryTimeout(*mock_upst.get())); - EXPECT_NO_THROW(this->upstream_->onPerTryIdleTimeout(*mock_upst.get())); - EXPECT_NO_THROW(this->upstream_->onStreamMaxDurationReached(*mock_upst.get())); - EXPECT_EQ(this->upstream_->dynamicMaxStreamDuration(), absl::nullopt); - EXPECT_EQ(this->upstream_->downstreamTrailers(), nullptr); - EXPECT_EQ(this->upstream_->downstreamResponseStarted(), false); - EXPECT_EQ(this->upstream_->downstreamEndStream(), false); - EXPECT_EQ(this->upstream_->attemptCount(), 0); -} - -TEST_F(CombinedUpstreamTest, WriteUpstream) { - this->setup(); - EXPECT_CALL(*this->mock_router_upstream_request_, - acceptDataFromRouter(BufferStringEqual("foo"), false /*end_stream*/)); - Buffer::OwnedImpl buffer1("foo"); - this->upstream_->encodeData(buffer1, false); - - EXPECT_CALL(*this->mock_router_upstream_request_, - acceptDataFromRouter(BufferStringEqual("bar"), true /*end_stream*/)); - Buffer::OwnedImpl buffer2("bar"); - this->upstream_->encodeData(buffer2, true); - - // New upstream with no encoder. - this->upstream_ = std::make_unique(*conn_pool_, callbacks_, decoder_callbacks_, - *tunnel_config_, downstream_stream_info_); - this->upstream_->encodeData(buffer2, true); -} - -TEST_F(CombinedUpstreamTest, WriteDownstream) { - this->setup(); - EXPECT_CALL(this->callbacks_, onUpstreamData(BufferStringEqual("foo"), false)); - Buffer::OwnedImpl buffer1("foo"); - this->upstream_->responseDecoder().decodeData(buffer1, false); - - EXPECT_CALL(this->callbacks_, onUpstreamData(BufferStringEqual("bar"), true)); - Buffer::OwnedImpl buffer2("bar"); - this->upstream_->responseDecoder().decodeData(buffer2, true); -} - -TEST_F(CombinedUpstreamTest, InvalidUpgradeWithEarlyFin) { - this->setup(); - EXPECT_CALL(this->callbacks_, onEvent(_)); - Http::ResponseHeaderMapPtr headers{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}; - this->upstream_->responseDecoder().decodeHeaders(std::move(headers), true); -} - -TEST_F(CombinedUpstreamTest, InvalidUpgradeWithNon200) { - this->setup(); - EXPECT_CALL(this->callbacks_, onEvent(_)); - Http::ResponseHeaderMapPtr headers{new Http::TestResponseHeaderMapImpl{{":status", "301"}}}; - this->upstream_->responseDecoder().decodeHeaders(std::move(headers), false); -} - -TEST_F(CombinedUpstreamTest, ReadDisable) { - this->setup(); - EXPECT_CALL(*this->mock_router_upstream_request_, onAboveWriteBufferHighWatermark()); - EXPECT_TRUE(this->upstream_->readDisable(true)); - - EXPECT_CALL(*this->mock_router_upstream_request_, onAboveWriteBufferHighWatermark()).Times(0); - EXPECT_TRUE(this->upstream_->readDisable(false)); - - // New upstream with no encoder. - this->upstream_ = std::make_unique(*conn_pool_, callbacks_, decoder_callbacks_, - *tunnel_config_, downstream_stream_info_); - EXPECT_FALSE(this->upstream_->readDisable(true)); -} - -TEST_F(CombinedUpstreamTest, AddBytesSentCallbackForCoverage) { - this->setup(); - this->upstream_->addBytesSentCallback([&](uint64_t) { return true; }); -} - -TEST_F(CombinedUpstreamTest, DownstreamDisconnect) { - this->setup(); - EXPECT_CALL(*this->mock_router_upstream_request_, resetStream()); - EXPECT_CALL(this->callbacks_, onEvent(_)).Times(0); - EXPECT_TRUE(this->upstream_->onDownstreamEvent(Network::ConnectionEvent::LocalClose) == nullptr); -} - -TEST_F(CombinedUpstreamTest, UpstreamReset) { - this->setup(); - EXPECT_CALL(*this->mock_router_upstream_request_, resetStream()); - EXPECT_CALL(this->callbacks_, onEvent(_)); - this->upstream_->onResetStream(Http::StreamResetReason::ConnectionTermination, ""); -} - -TEST_F(CombinedUpstreamTest, UpstreamWatermarks) { - this->setup(); - EXPECT_CALL(this->callbacks_, onAboveWriteBufferHighWatermark()); - this->upstream_->onAboveWriteBufferHighWatermark(); - - EXPECT_CALL(this->callbacks_, onBelowWriteBufferLowWatermark()); - this->upstream_->onBelowWriteBufferLowWatermark(); -} - -TEST_F(CombinedUpstreamTest, OnSuccessCalledOnValidResponse) { - this->setup(); - auto conn_pool_callbacks = std::make_unique(); - auto conn_pool_callbacks_raw = conn_pool_callbacks.get(); - this->upstream_->setConnPoolCallbacks(std::move(conn_pool_callbacks)); - EXPECT_CALL(*conn_pool_callbacks_raw, onFailure()).Times(0); - EXPECT_CALL(*conn_pool_callbacks_raw, onSuccess(_)); - Http::ResponseHeaderMapPtr headers{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}; - this->upstream_->responseDecoder().decodeHeaders(std::move(headers), false); -} - -TEST_F(CombinedUpstreamTest, OnFailureCalledOnInvalidResponse) { - this->setup(); - auto conn_pool_callbacks = std::make_unique(); - auto conn_pool_callbacks_raw = conn_pool_callbacks.get(); - this->upstream_->setConnPoolCallbacks(std::move(conn_pool_callbacks)); - EXPECT_CALL(*conn_pool_callbacks_raw, onFailure()); - EXPECT_CALL(*conn_pool_callbacks_raw, onSuccess(_)).Times(0); - Http::ResponseHeaderMapPtr headers{new Http::TestResponseHeaderMapImpl{{":status", "404"}}}; - this->upstream_->responseDecoder().decodeHeaders(std::move(headers), false); -} - -TEST_F(CombinedUpstreamTest, DumpsResponseDecoderWithoutAllocatingMemory) { - std::array buffer; - OutputBufferStream ostream{buffer.data(), buffer.size()}; - this->setup(); - - Stats::TestUtil::MemoryTest memory_test; - this->upstream_->responseDecoder().dumpState(ostream, 1); - EXPECT_EQ(memory_test.consumedBytes(), 0); - EXPECT_THAT(ostream.contents(), EndsWith("has not implemented dumpState\n")); -} -TEST_F(CombinedUpstreamTest, UpstreamTrailersMarksDoneReading) { - this->setup(); - EXPECT_CALL(*this->mock_router_upstream_request_, resetStream()); - this->upstream_->doneWriting(); - Http::ResponseTrailerMapPtr trailers{new Http::TestResponseTrailerMapImpl{{"key", "value"}}}; - this->upstream_->responseDecoder().decodeTrailers(std::move(trailers)); -} - -TEST_F(CombinedUpstreamTest, UpstreamTrailersDontPropagateFinDownstreamWhenFeatureDisabled) { - TestScopedRuntime scoped_runtime; - scoped_runtime.mergeValues( - {{"envoy.reloadable_features.tcp_tunneling_send_downstream_fin_on_upstream_trailers", - "false"}}); - this->setup(); - EXPECT_CALL(*this->mock_router_upstream_request_, resetStream()); - upstream_->doneWriting(); - EXPECT_CALL(callbacks_, onUpstreamData(_, _)).Times(0); - Http::ResponseTrailerMapPtr trailers{new Http::TestResponseTrailerMapImpl{{"key", "value"}}}; - upstream_->responseDecoder().decodeTrailers(std::move(trailers)); -} } // namespace } // namespace TcpProxy } // namespace Envoy diff --git a/test/extensions/filters/http/cache/BUILD b/test/extensions/filters/http/cache/BUILD index ce6100368be6..1d1f5f0ab270 100644 --- a/test/extensions/filters/http/cache/BUILD +++ b/test/extensions/filters/http/cache/BUILD @@ -124,7 +124,6 @@ envoy_extension_cc_test( "cache_filter_integration_test.cc", ], extension_names = ["envoy.filters.http.cache"], - shard_count = 2, deps = [ "//source/extensions/filters/http/cache:config", "//source/extensions/filters/http/cache:http_cache_lib", diff --git a/test/extensions/filters/http/csrf/BUILD b/test/extensions/filters/http/csrf/BUILD index 2a3b5ef8c3a4..5a958bce60e8 100644 --- a/test/extensions/filters/http/csrf/BUILD +++ b/test/extensions/filters/http/csrf/BUILD @@ -33,7 +33,6 @@ envoy_extension_cc_test( size = "large", srcs = ["csrf_filter_integration_test.cc"], extension_names = ["envoy.filters.http.csrf"], - shard_count = 2, deps = [ "//source/extensions/filters/http/csrf:config", "//test/config:utility_lib", diff --git a/test/extensions/filters/http/custom_response/BUILD b/test/extensions/filters/http/custom_response/BUILD index 01c7daba1d5c..704c20d197f4 100644 --- a/test/extensions/filters/http/custom_response/BUILD +++ b/test/extensions/filters/http/custom_response/BUILD @@ -81,7 +81,7 @@ envoy_extension_cc_test( "custom_response_integration_test.cc", ], extension_names = ["envoy.filters.http.custom_response"], - shard_count = 4, + shard_count = 2, tags = [ "cpu:3", ], diff --git a/test/extensions/filters/http/fault/BUILD b/test/extensions/filters/http/fault/BUILD index 55c19ce038bf..a7413da5feaf 100644 --- a/test/extensions/filters/http/fault/BUILD +++ b/test/extensions/filters/http/fault/BUILD @@ -55,7 +55,6 @@ envoy_extension_cc_test( size = "large", srcs = ["fault_filter_integration_test.cc"], extension_names = ["envoy.filters.http.fault"], - shard_count = 4, deps = [ "//source/extensions/filters/http/fault:config", "//test/integration:http_protocol_integration_lib", diff --git a/test/extensions/filters/http/health_check/BUILD b/test/extensions/filters/http/health_check/BUILD index 7023b7b02bf6..8d7a622ebf1d 100644 --- a/test/extensions/filters/http/health_check/BUILD +++ b/test/extensions/filters/http/health_check/BUILD @@ -45,7 +45,6 @@ envoy_extension_cc_test( "health_check_integration_test.cc", ], extension_names = ["envoy.filters.http.health_check"], - shard_count = 2, deps = [ "//source/extensions/filters/http/buffer:config", "//source/extensions/filters/http/health_check:config", diff --git a/test/extensions/filters/http/jwt_authn/BUILD b/test/extensions/filters/http/jwt_authn/BUILD index 321ebccf2c83..9b84849c432f 100644 --- a/test/extensions/filters/http/jwt_authn/BUILD +++ b/test/extensions/filters/http/jwt_authn/BUILD @@ -162,7 +162,7 @@ envoy_extension_cc_test( "envoy.filters.http.jwt_authn", "envoy.filters.http.set_filter_state", ], - shard_count = 8, + shard_count = 4, tags = [ "cpu:3", ], diff --git a/test/extensions/filters/http/rbac/BUILD b/test/extensions/filters/http/rbac/BUILD index 33538419eebd..4c397d1e0389 100644 --- a/test/extensions/filters/http/rbac/BUILD +++ b/test/extensions/filters/http/rbac/BUILD @@ -58,7 +58,7 @@ envoy_extension_cc_test( size = "large", srcs = ["rbac_filter_integration_test.cc"], extension_names = ["envoy.filters.http.rbac"], - shard_count = 10, + shard_count = 3, tags = ["skip_on_windows"], deps = [ "//source/extensions/clusters/dynamic_forward_proxy:cluster", diff --git a/test/extensions/upstreams/http/tcp/upstream_request_test.cc b/test/extensions/upstreams/http/tcp/upstream_request_test.cc index 9bacca7d18af..e48d6a4ad7e4 100644 --- a/test/extensions/upstreams/http/tcp/upstream_request_test.cc +++ b/test/extensions/upstreams/http/tcp/upstream_request_test.cc @@ -114,7 +114,7 @@ class TcpUpstreamTest : public ::testing::Test { EXPECT_CALL(mock_router_filter_, callbacks()).Times(AnyNumber()); upstream_request_ = std::make_unique( mock_router_filter_, std::make_unique>(), false, - false, false /*enable_tcp_tunneling*/); + false); auto data = std::make_unique>(); EXPECT_CALL(*data, connection()).Times(AnyNumber()).WillRepeatedly(ReturnRef(connection())); tcp_upstream_ = std::make_unique(upstream_request_.get(), std::move(data)); diff --git a/test/extensions/upstreams/http/udp/upstream_request_test.cc b/test/extensions/upstreams/http/udp/upstream_request_test.cc index c6020febfea6..423b21589589 100644 --- a/test/extensions/upstreams/http/udp/upstream_request_test.cc +++ b/test/extensions/upstreams/http/udp/upstream_request_test.cc @@ -46,7 +46,6 @@ class UdpUpstreamTest : public ::testing::Test { udp_upstream_ = std::make_unique(&mock_upstream_to_downstream_, std::move(mock_socket), std::move(mock_host), mock_dispatcher_); - EXPECT_NO_THROW(udp_upstream_->enableHalfClose()); } protected: diff --git a/test/extensions/upstreams/tcp/generic/BUILD b/test/extensions/upstreams/tcp/generic/BUILD index 57cc2e3fcc9a..2a9375294a53 100644 --- a/test/extensions/upstreams/tcp/generic/BUILD +++ b/test/extensions/upstreams/tcp/generic/BUILD @@ -17,6 +17,5 @@ envoy_cc_test( "//source/extensions/upstreams/tcp/generic:config", "//test/mocks/server:factory_context_mocks", "//test/mocks/upstream:upstream_mocks", - "@envoy_api//envoy/extensions/filters/network/tcp_proxy/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/upstreams/tcp/generic/config_test.cc b/test/extensions/upstreams/tcp/generic/config_test.cc index 08d2ff0ead46..f2c62b449174 100644 --- a/test/extensions/upstreams/tcp/generic/config_test.cc +++ b/test/extensions/upstreams/tcp/generic/config_test.cc @@ -1,10 +1,7 @@ -#include "envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.pb.h" - #include "source/common/stream_info/bool_accessor_impl.h" #include "source/common/tcp_proxy/tcp_proxy.h" #include "source/extensions/upstreams/tcp/generic/config.h" -#include "test/mocks/http/mocks.h" #include "test/mocks/server/factory_context.h" #include "test/mocks/tcp/mocks.h" #include "test/mocks/upstream/cluster_manager.h" @@ -17,13 +14,13 @@ using testing::_; using testing::AnyNumber; using testing::NiceMock; using testing::Return; -using testing::ReturnRef; namespace Envoy { namespace Extensions { namespace Upstreams { namespace Tcp { namespace Generic { + class TcpConnPoolTest : public ::testing::Test { public: TcpConnPoolTest() { @@ -35,10 +32,6 @@ class TcpConnPoolTest : public ::testing::Test { NiceMock downstream_stream_info_; NiceMock connection_; Upstream::MockLoadBalancerContext lb_context_; - envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy tcp_proxy_; - NiceMock store_; - Stats::MockScope& scope_{store_.mockScope()}; - NiceMock decoder_callbacks_; NiceMock context_; }; @@ -46,13 +39,13 @@ TEST_F(TcpConnPoolTest, TestNoTunnelingConfig) { EXPECT_CALL(thread_local_cluster_, tcpConnPool(_, _)).WillOnce(Return(absl::nullopt)); EXPECT_EQ(nullptr, factory_.createGenericConnPool( thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(), - &lb_context_, callbacks_, decoder_callbacks_, downstream_stream_info_)); + &lb_context_, callbacks_, downstream_stream_info_)); } TEST_F(TcpConnPoolTest, TestTunnelingDisabledByFilterState) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; - tcp_proxy_.mutable_tunneling_config()->set_hostname("host"); - const TcpProxy::TunnelingConfigHelperImpl config(scope_, tcp_proxy_, context_); + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); downstream_stream_info_.filterState()->setData( TcpProxy::DisableTunnelingFilterStateKey, @@ -62,13 +55,13 @@ TEST_F(TcpConnPoolTest, TestTunnelingDisabledByFilterState) { EXPECT_CALL(thread_local_cluster_, tcpConnPool(_, _)).WillOnce(Return(absl::nullopt)); EXPECT_EQ(nullptr, factory_.createGenericConnPool( thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), - &lb_context_, callbacks_, decoder_callbacks_, downstream_stream_info_)); + &lb_context_, callbacks_, downstream_stream_info_)); } TEST_F(TcpConnPoolTest, TestTunnelingNotDisabledIfFilterStateHasFalseValue) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; - tcp_proxy_.mutable_tunneling_config()->set_hostname("host"); - const TcpProxy::TunnelingConfigHelperImpl config(scope_, tcp_proxy_, context_); + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); downstream_stream_info_.filterState()->setData( TcpProxy::DisableTunnelingFilterStateKey, @@ -78,47 +71,46 @@ TEST_F(TcpConnPoolTest, TestTunnelingNotDisabledIfFilterStateHasFalseValue) { EXPECT_CALL(thread_local_cluster_, httpConnPool(_, _, _)).WillOnce(Return(absl::nullopt)); EXPECT_EQ(nullptr, factory_.createGenericConnPool( thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), - &lb_context_, callbacks_, decoder_callbacks_, downstream_stream_info_)); + &lb_context_, callbacks_, downstream_stream_info_)); } TEST_F(TcpConnPoolTest, TestNoConnPool) { envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; - tcp_proxy_.mutable_tunneling_config()->set_hostname("host"); - const TcpProxy::TunnelingConfigHelperImpl config(scope_, tcp_proxy_, context_); + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); EXPECT_CALL(thread_local_cluster_, httpConnPool(_, _, _)).WillOnce(Return(absl::nullopt)); EXPECT_EQ(nullptr, factory_.createGenericConnPool( thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), - &lb_context_, callbacks_, decoder_callbacks_, downstream_stream_info_)); + &lb_context_, callbacks_, downstream_stream_info_)); } TEST_F(TcpConnPoolTest, Http2Config) { auto info = std::make_shared(); - const std::string fake_cluster_name = "fake_cluster"; - + EXPECT_CALL(*info, features()).WillOnce(Return(Upstream::ClusterInfo::Features::HTTP2)); + EXPECT_CALL(thread_local_cluster_, info).WillOnce(Return(info)); envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; - tcp_proxy_.mutable_tunneling_config()->set_hostname("host"); - const TcpProxy::TunnelingConfigHelperImpl config(scope_, tcp_proxy_, context_); + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); EXPECT_CALL(thread_local_cluster_, httpConnPool(_, _, _)).WillOnce(Return(absl::nullopt)); EXPECT_EQ(nullptr, factory_.createGenericConnPool( thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), - &lb_context_, callbacks_, decoder_callbacks_, downstream_stream_info_)); + &lb_context_, callbacks_, downstream_stream_info_)); } TEST_F(TcpConnPoolTest, Http3Config) { auto info = std::make_shared(); - const std::string fake_cluster_name = "fake_cluster"; EXPECT_CALL(*info, features()) .Times(AnyNumber()) .WillRepeatedly(Return(Upstream::ClusterInfo::Features::HTTP3)); EXPECT_CALL(thread_local_cluster_, info).Times(AnyNumber()).WillRepeatedly(Return(info)); envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy_TunnelingConfig config_proto; - tcp_proxy_.mutable_tunneling_config()->set_hostname("host"); - const TcpProxy::TunnelingConfigHelperImpl config(scope_, tcp_proxy_, context_); + config_proto.set_hostname("host"); + const TcpProxy::TunnelingConfigHelperImpl config(config_proto, context_); EXPECT_CALL(thread_local_cluster_, httpConnPool(_, _, _)).WillOnce(Return(absl::nullopt)); EXPECT_EQ(nullptr, factory_.createGenericConnPool( thread_local_cluster_, TcpProxy::TunnelingConfigHelperOptConstRef(config), - &lb_context_, callbacks_, decoder_callbacks_, downstream_stream_info_)); + &lb_context_, callbacks_, downstream_stream_info_)); } TEST(DisableTunnelingObjectFactory, CreateFromBytes) { diff --git a/test/integration/BUILD b/test/integration/BUILD index 72636c49d709..fce0e38d657d 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -329,7 +329,6 @@ envoy_cc_test( srcs = envoy_select_admin_functionality([ "drain_close_integration_test.cc", ]), - shard_count = 6, tags = [ "cpu:3", ], @@ -615,7 +614,6 @@ envoy_cc_test( srcs = [ "buffer_accounting_integration_test.cc", ], - shard_count = 2, tags = [ "cpu:3", ], @@ -987,7 +985,7 @@ envoy_cc_test( srcs = ["idle_timeout_integration_test.cc"], # As this test has many pauses for idle timeouts, it takes a while to run. # Shard it enough to bring the run time in line with other integration tests. - shard_count = 16, + shard_count = 4, tags = [ "cpu:3", ], @@ -1357,7 +1355,7 @@ envoy_cc_test( srcs = [ "redirect_integration_test.cc", ], - shard_count = 4, + shard_count = 2, tags = [ "cpu:3", "nofips", @@ -1393,7 +1391,6 @@ envoy_cc_test( name = "websocket_integration_test", size = "large", srcs = ["websocket_integration_test.cc"], - shard_count = 2, tags = [ "cpu:3", ], @@ -1540,7 +1537,7 @@ envoy_cc_test( name = "overload_integration_test", size = "large", srcs = ["overload_integration_test.cc"], - shard_count = 10, + shard_count = 4, tags = [ "cpu:3", ], @@ -1814,7 +1811,7 @@ envoy_cc_test( data = [ "//test/config/integration/certs", ], - shard_count = 30, + shard_count = 8, tags = [ "cpu:3", ], @@ -1840,7 +1837,6 @@ envoy_cc_test( data = [ "//test/config/integration/certs", ], - shard_count = 4, deps = [ ":http_integration_lib", ":http_protocol_integration_lib", @@ -2285,7 +2281,6 @@ envoy_cc_test( srcs = [ "local_reply_integration_test.cc", ], - shard_count = 2, tags = [ "cpu:2", ], diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc index baab4dd3be91..13295850af25 100644 --- a/test/integration/base_integration_test.cc +++ b/test/integration/base_integration_test.cc @@ -427,7 +427,6 @@ void BaseIntegrationTest::registerTestServerPorts(const std::vector const auto admin_addr = test_server->server().admin()->socket().connectionInfoProvider().localAddress(); if (admin_addr->type() == Network::Address::Type::Ip) { - ENVOY_LOG(debug, "registered 'admin' as port {}.", admin_addr->ip()->port()); registerPort("admin", admin_addr->ip()->port()); } } diff --git a/test/integration/http_protocol_integration.cc b/test/integration/http_protocol_integration.cc index 914fffefcd99..50f79fdf6366 100644 --- a/test/integration/http_protocol_integration.cc +++ b/test/integration/http_protocol_integration.cc @@ -39,17 +39,15 @@ std::vector HttpProtocolIntegrationTest::getProtocolTest #else use_header_validator_values.push_back(false); #endif - for (const bool tunneling_with_upstream_filters : {false, true}) { - for (Http1ParserImpl http1_implementation : http1_implementations) { - for (Http2Impl http2_implementation : http2_implementations) { - for (bool defer_processing : http2_bool_values) { - for (bool deprecate_callback_visitor : http2_bool_values) { - for (bool use_header_validator : use_header_validator_values) { - ret.push_back(HttpProtocolTestParams{ - ip_version, downstream_protocol, upstream_protocol, http1_implementation, - http2_implementation, defer_processing, use_header_validator, - deprecate_callback_visitor, tunneling_with_upstream_filters}); - } + for (Http1ParserImpl http1_implementation : http1_implementations) { + for (Http2Impl http2_implementation : http2_implementations) { + for (bool defer_processing : http2_bool_values) { + for (bool deprecate_callback_visitor : http2_bool_values) { + for (bool use_header_validator : use_header_validator_values) { + ret.push_back(HttpProtocolTestParams{ + ip_version, downstream_protocol, upstream_protocol, http1_implementation, + http2_implementation, defer_processing, use_header_validator, + deprecate_callback_visitor}); } } } @@ -104,11 +102,9 @@ std::string HttpProtocolIntegrationTest::protocolTestParamsToString( http2ImplementationToString(params.param.http2_implementation), params.param.defer_processing_backedup_streams ? "WithDeferredProcessing" : "NoDeferredProcessing", - params.param.use_universal_header_validator ? "Uhv" : "Legacy", params.param.deprecate_callback_visitor ? "WithCallbackVisitor" : "NoCallbackVisitor", - params.param.tunneling_with_upstream_filters ? "WithUpstreamHttpFilters" - : "WithoutUpstreamHttpFilters"); + params.param.use_universal_header_validator ? "Uhv" : "Legacy"); } void HttpProtocolIntegrationTest::setUpstreamOverrideStreamErrorOnInvalidHttpMessage() { diff --git a/test/integration/http_protocol_integration.h b/test/integration/http_protocol_integration.h index 1fa57f3d1992..0c0dd6c1bbc0 100644 --- a/test/integration/http_protocol_integration.h +++ b/test/integration/http_protocol_integration.h @@ -15,7 +15,6 @@ struct HttpProtocolTestParams { Http2Impl http2_implementation; bool defer_processing_backedup_streams; bool use_universal_header_validator; - bool tunneling_with_upstream_filters; bool deprecate_callback_visitor; }; @@ -87,9 +86,6 @@ class HttpProtocolIntegrationTest : public testing::TestWithParam static void initializeMockStreamFilterCallbacks(T& callbacks) MockStreamDecoderFilterCallbacks::MockStreamDecoderFilterCallbacks() { initializeMockStreamFilterCallbacks(*this); - ON_CALL(*this, dispatcher()).WillByDefault(ReturnRef(dispatcher_)); ON_CALL(*this, decodingBuffer()).WillByDefault(Invoke(&buffer_, &Buffer::InstancePtr::get)); ON_CALL(*this, addDownstreamWatermarkCallbacks(_)) diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index f7770ac37b96..3ec3f7c5eb71 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -109,7 +109,6 @@ class MockFilterManagerCallbacks : public FilterManagerCallbacks { MOCK_METHOD(OptRef, tracingConfig, (), (const)); MOCK_METHOD(const ScopeTrackedObject&, scope, ()); MOCK_METHOD(void, restoreContextOnContinue, (ScopeTrackedObjectStack&)); - MOCK_METHOD(bool, isHalfCloseEnabled, ()); ResponseHeaderMapPtr informational_headers_; ResponseHeaderMapPtr response_headers_; @@ -331,7 +330,6 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks, bool is_grpc_request_{}; bool is_head_request_{false}; bool stream_destroyed_{}; - NiceMock dispatcher_; }; class MockStreamEncoderFilterCallbacks : public StreamEncoderFilterCallbacks, diff --git a/test/mocks/router/BUILD b/test/mocks/router/BUILD index 2d1364e40bc1..79a35cb2b25d 100644 --- a/test/mocks/router/BUILD +++ b/test/mocks/router/BUILD @@ -49,12 +49,3 @@ envoy_cc_mock( "//test/test_common:test_time_lib", ], ) - -envoy_cc_mock( - name = "upstream_request", - srcs = ["upstream_request.cc"], - hdrs = ["upstream_request.h"], - deps = [ - "//source/common/router:router_lib", - ], -) diff --git a/test/mocks/router/upstream_request.cc b/test/mocks/router/upstream_request.cc deleted file mode 100644 index aabf4baf8a05..000000000000 --- a/test/mocks/router/upstream_request.cc +++ /dev/null @@ -1,13 +0,0 @@ -#include "test/mocks/router/upstream_request.h" - -namespace Envoy { -namespace Router { - -MockUpstreamRequest::MockUpstreamRequest(RouterFilterInterface& router_interface, - std::unique_ptr&& conn_pool) - : UpstreamRequest(router_interface, std::move(conn_pool), false, true, true) {} - -MockUpstreamRequest::~MockUpstreamRequest() = default; - -} // namespace Router -} // namespace Envoy diff --git a/test/mocks/router/upstream_request.h b/test/mocks/router/upstream_request.h deleted file mode 100644 index 10688fdde5d1..000000000000 --- a/test/mocks/router/upstream_request.h +++ /dev/null @@ -1,21 +0,0 @@ -#pragma once - -#include "source/common/router/upstream_request.h" - -#include "gmock/gmock.h" - -namespace Envoy { -namespace Router { - -class MockUpstreamRequest : public UpstreamRequest { -public: - MockUpstreamRequest(RouterFilterInterface& router_interface, std::unique_ptr&&); - ~MockUpstreamRequest() override; - MOCK_METHOD(void, acceptHeadersFromRouter, (bool end_stream), (override)); - MOCK_METHOD(void, acceptDataFromRouter, (Buffer::Instance & data, bool end_stream), (override)); - MOCK_METHOD(void, onAboveWriteBufferHighWatermark, (), (override)); - MOCK_METHOD(void, resetStream, (), (override)); -}; - -} // namespace Router -} // namespace Envoy