diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 83a2a7e582e51..35bfa93ea352a 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -746,7 +746,11 @@ message Cluster { // is respected by both the HTTP/1.1 and HTTP/2 connection pool // implementations. If not specified, there is no limit. Setting this // parameter to 1 will effectively disable keep alive. - google.protobuf.UInt32Value max_requests_per_connection = 9; + // + // .. attention:: + // This field has been deprecated in favor of the :ref:`max_requests_per_connection ` field. + google.protobuf.UInt32Value max_requests_per_connection = 9 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // Optional :ref:`circuit breaking ` for the cluster. CircuitBreakers circuit_breakers = 10; diff --git a/api/envoy/config/cluster/v4alpha/cluster.proto b/api/envoy/config/cluster/v4alpha/cluster.proto index 4a52adb8babb9..ebd007bca5780 100644 --- a/api/envoy/config/cluster/v4alpha/cluster.proto +++ b/api/envoy/config/cluster/v4alpha/cluster.proto @@ -631,11 +631,12 @@ message Cluster { [(validate.rules).double = {lte: 3.0 gte: 1.0}]; } - reserved 12, 15, 7, 11, 35, 46, 29, 13, 14, 18, 45, 26, 47; + reserved 12, 15, 7, 11, 35, 9, 46, 29, 13, 14, 18, 45, 26, 47; - reserved "hosts", "tls_context", "extension_protocol_options", "upstream_http_protocol_options", - "common_http_protocol_options", "http_protocol_options", "http2_protocol_options", - "dns_resolvers", "use_tcp_for_dns_lookups", "protocol_selection", "track_timeout_budgets"; + reserved "hosts", "tls_context", "extension_protocol_options", "max_requests_per_connection", + "upstream_http_protocol_options", "common_http_protocol_options", "http_protocol_options", + "http2_protocol_options", "dns_resolvers", "use_tcp_for_dns_lookups", "protocol_selection", + "track_timeout_budgets"; // Configuration to use different transport sockets for different endpoints. // The entry of *envoy.transport_socket_match* in the @@ -751,12 +752,6 @@ message Cluster { // members will be considered healthy at all times. repeated core.v4alpha.HealthCheck health_checks = 8; - // Optional maximum requests for a single upstream connection. This parameter - // is respected by both the HTTP/1.1 and HTTP/2 connection pool - // implementations. If not specified, there is no limit. Setting this - // parameter to 1 will effectively disable keep alive. - google.protobuf.UInt32Value max_requests_per_connection = 9; - // Optional :ref:`circuit breaking ` for the cluster. CircuitBreakers circuit_breakers = 10; diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index d3b56a5ed7680..6589a3ed3a1a4 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -93,7 +93,7 @@ message AlternateProtocolsCacheOptions { google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; } -// [#next-free-field: 6] +// [#next-free-field: 7] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.HttpProtocolOptions"; @@ -157,6 +157,12 @@ message HttpProtocolOptions { // If this setting is not specified, the value defaults to ALLOW. // Note: upstream responses are not affected by this setting. HeadersWithUnderscoresAction headers_with_underscores_action = 5; + + // Optional maximum requests for both upstream and downstream connections. + // If not specified, there is no limit. + // Setting this parameter to 1 will effectively disable keep alive. + // For HTTP/2 and HTTP/3, due to concurrent stream processing, the limit is approximate. + google.protobuf.UInt32Value max_requests_per_connection = 6; } // [#next-free-field: 8] diff --git a/api/envoy/config/core/v4alpha/protocol.proto b/api/envoy/config/core/v4alpha/protocol.proto index 50c47f006938f..37e5af0c72bd1 100644 --- a/api/envoy/config/core/v4alpha/protocol.proto +++ b/api/envoy/config/core/v4alpha/protocol.proto @@ -98,7 +98,7 @@ message AlternateProtocolsCacheOptions { google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; } -// [#next-free-field: 6] +// [#next-free-field: 7] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.HttpProtocolOptions"; @@ -162,6 +162,12 @@ message HttpProtocolOptions { // If this setting is not specified, the value defaults to ALLOW. // Note: upstream responses are not affected by this setting. HeadersWithUnderscoresAction headers_with_underscores_action = 5; + + // Optional maximum requests for both upstream and downstream connections. + // If not specified, there is no limit. + // Setting this parameter to 1 will effectively disable keep alive. + // For HTTP/2 and HTTP/3, due to concurrent stream processing, the limit is approximate. + google.protobuf.UInt32Value max_requests_per_connection = 6; } // [#next-free-field: 8] diff --git a/docs/root/configuration/http/http_conn_man/stats.rst b/docs/root/configuration/http/http_conn_man/stats.rst index 8df5877557b62..3a4d7a568c82f 100644 --- a/docs/root/configuration/http/http_conn_man/stats.rst +++ b/docs/root/configuration/http/http_conn_man/stats.rst @@ -37,6 +37,7 @@ statistics: downstream_cx_drain_close, Counter, Total connections closed due to draining downstream_cx_idle_timeout, Counter, Total connections closed due to idle timeout downstream_cx_max_duration_reached, Counter, Total connections closed due to max connection duration + downstream_cx_max_requests_reached, Counter, Total connections closed due to max requests per connection downstream_cx_overload_disable_keepalive, Counter, Total connections for which HTTP 1.x keepalive has been disabled due to Envoy overload downstream_flow_control_paused_reading_total, Counter, Total number of times reads were disabled due to flow control downstream_flow_control_resumed_reading_total, Counter, Total number of times reads were enabled on the connection due to flow control diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 792dcbe2f1d48..d15d12a019112 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -28,9 +28,12 @@ Removed Config or Runtime New Features ------------ * http: added :ref:`string_match ` in the header matcher. +* http: added support for :ref:`max_requests_per_connection ` for both upstream and downstream connections. Deprecated ---------- +* cluster: :ref:`max_requests_per_connection ` is deprecated in favor of :ref:`max_requests_per_connection `. * http: the HeaderMatcher fields :ref:`exact_match `, :ref:`safe_regex_match `, :ref:`prefix_match `, :ref:`suffix_match ` and :ref:`contains_match ` are deprecated by :ref:`string_match `. + diff --git a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto index 42e86227156fe..137300708e375 100644 --- a/generated_api_shadow/envoy/config/cluster/v3/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v3/cluster.proto @@ -747,7 +747,11 @@ message Cluster { // is respected by both the HTTP/1.1 and HTTP/2 connection pool // implementations. If not specified, there is no limit. Setting this // parameter to 1 will effectively disable keep alive. - google.protobuf.UInt32Value max_requests_per_connection = 9; + // + // .. attention:: + // This field has been deprecated in favor of the :ref:`max_requests_per_connection ` field. + google.protobuf.UInt32Value max_requests_per_connection = 9 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // Optional :ref:`circuit breaking ` for the cluster. CircuitBreakers circuit_breakers = 10; diff --git a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto index afdee8bc0f55f..3baa5c7ec0ac9 100644 --- a/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto +++ b/generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto @@ -755,7 +755,11 @@ message Cluster { // is respected by both the HTTP/1.1 and HTTP/2 connection pool // implementations. If not specified, there is no limit. Setting this // parameter to 1 will effectively disable keep alive. - google.protobuf.UInt32Value max_requests_per_connection = 9; + // + // .. attention:: + // This field has been deprecated in favor of the :ref:`max_requests_per_connection ` field. + google.protobuf.UInt32Value hidden_envoy_deprecated_max_requests_per_connection = 9 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // Optional :ref:`circuit breaking ` for the cluster. CircuitBreakers circuit_breakers = 10; diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index d3b56a5ed7680..6589a3ed3a1a4 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -93,7 +93,7 @@ message AlternateProtocolsCacheOptions { google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; } -// [#next-free-field: 6] +// [#next-free-field: 7] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.HttpProtocolOptions"; @@ -157,6 +157,12 @@ message HttpProtocolOptions { // If this setting is not specified, the value defaults to ALLOW. // Note: upstream responses are not affected by this setting. HeadersWithUnderscoresAction headers_with_underscores_action = 5; + + // Optional maximum requests for both upstream and downstream connections. + // If not specified, there is no limit. + // Setting this parameter to 1 will effectively disable keep alive. + // For HTTP/2 and HTTP/3, due to concurrent stream processing, the limit is approximate. + google.protobuf.UInt32Value max_requests_per_connection = 6; } // [#next-free-field: 8] diff --git a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto index 2017020b3d946..f99ae27f14392 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto @@ -99,7 +99,7 @@ message AlternateProtocolsCacheOptions { google.protobuf.UInt32Value max_entries = 2 [(validate.rules).uint32 = {gt: 0}]; } -// [#next-free-field: 6] +// [#next-free-field: 7] message HttpProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.HttpProtocolOptions"; @@ -163,6 +163,12 @@ message HttpProtocolOptions { // If this setting is not specified, the value defaults to ALLOW. // Note: upstream responses are not affected by this setting. HeadersWithUnderscoresAction headers_with_underscores_action = 5; + + // Optional maximum requests for both upstream and downstream connections. + // If not specified, there is no limit. + // Setting this parameter to 1 will effectively disable keep alive. + // For HTTP/2 and HTTP/3, due to concurrent stream processing, the limit is approximate. + google.protobuf.UInt32Value max_requests_per_connection = 6; } // [#next-free-field: 8] diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index b56821b665d12..cfcb9884eec0a 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -35,6 +35,7 @@ namespace Http { COUNTER(downstream_cx_http3_total) \ COUNTER(downstream_cx_idle_timeout) \ COUNTER(downstream_cx_max_duration_reached) \ + COUNTER(downstream_cx_max_requests_reached) \ COUNTER(downstream_cx_overload_disable_keepalive) \ COUNTER(downstream_cx_protocol_error) \ COUNTER(downstream_cx_rx_bytes_total) \ @@ -495,6 +496,10 @@ class ConnectionManagerConfig { * header. */ virtual bool shouldStripTrailingHostDot() const PURE; + /** + * @return maximum requests for downstream. + */ + virtual uint64_t maxRequestsPerConnection() const PURE; }; } // namespace Http } // namespace Envoy diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 92cfb00102756..8050bb884077f 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -284,6 +284,20 @@ RequestDecoder& ConnectionManagerImpl::newStream(ResponseEncoder& response_encod ActiveStreamPtr new_stream(new ActiveStream(*this, response_encoder.getStream().bufferLimit(), std::move(downstream_request_account))); + accumulated_requests_++; + if (config_.maxRequestsPerConnection() > 0 && + accumulated_requests_ >= config_.maxRequestsPerConnection()) { + if (codec_->protocol() < Protocol::Http2) { + new_stream->state_.saw_connection_close_ = true; + // Prevent erroneous debug log of closing due to incoming connection close header. + drain_state_ = DrainState::Closing; + } else { + startDrainSequence(); + } + ENVOY_CONN_LOG(debug, "max requests per connection reached", read_callbacks_->connection()); + stats_.named_.downstream_cx_max_requests_reached_.inc(); + } + new_stream->state_.is_internally_created_ = is_internally_created; new_stream->response_encoder_ = &response_encoder; new_stream->response_encoder_->getStream().addCallbacks(*new_stream); diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 5b544890469f2..b83e0aa264a03 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -459,6 +459,8 @@ class ConnectionManagerImpl : Logger::Loggable, // Hop by hop headers should always be cleared for Envoy-as-a-proxy but will // not be for Envoy-mobile. bool clear_hop_by_hop_response_headers_{true}; + // The number of requests accumulated on the current connection. + uint64_t accumulated_requests_{}; }; } // namespace Http diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 01ced1e6fe2f3..b5ab427aa988d 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -713,8 +713,9 @@ ClusterInfoImpl::ClusterInfoImpl( extensionProtocolOptionsTyped( "envoy.extensions.upstreams.http.v3.HttpProtocolOptions"), factory_context.messageValidationVisitor())), - max_requests_per_connection_( - PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, max_requests_per_connection, 0)), + max_requests_per_connection_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( + http_protocol_options_->common_http_protocol_options_, max_requests_per_connection, + config.max_requests_per_connection().value())), max_response_headers_count_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( http_protocol_options_->common_http_protocol_options_, max_headers_count, runtime_.snapshot().getInteger(Http::MaxResponseHeadersCountOverrideKey, @@ -767,6 +768,12 @@ ClusterInfoImpl::ClusterInfoImpl( : absl::nullopt), factory_context_( std::make_unique(*stats_scope_, runtime, factory_context)) { + if (config.has_max_requests_per_connection() && + http_protocol_options_->common_http_protocol_options_.has_max_requests_per_connection()) { + throw EnvoyException("Only one of max_requests_per_connection from Cluster or " + "HttpProtocolOptions can be specified"); + } + switch (config.lb_policy()) { case envoy::config::cluster::v3::Cluster::ROUND_ROBIN: lb_type_ = LoadBalancerType::RoundRobin; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 49b6bf8bb0208..4d9c4128fc8ff 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -330,7 +330,9 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( config.common_http_protocol_options().headers_with_underscores_action()), local_reply_(LocalReply::Factory::create(config.local_reply_config(), context)), path_with_escaped_slashes_action_(getPathWithEscapedSlashesAction(config, context)), - strip_trailing_host_dot_(config.strip_trailing_host_dot()) { + strip_trailing_host_dot_(config.strip_trailing_host_dot()), + max_requests_per_connection_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( + config.common_http_protocol_options(), max_requests_per_connection, 0)) { // If idle_timeout_ was not configured in common_http_protocol_options, use value in deprecated // idle_timeout field. // TODO(asraa): Remove when idle_timeout is removed. diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 26b2417ddba0a..b23fd8194c49f 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -217,6 +217,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, originalIpDetectionExtensions() const override { return original_ip_detection_extensions_; } + uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } private: enum class CodecType { HTTP1, HTTP2, HTTP3, AUTO }; @@ -311,6 +312,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, const envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager:: PathWithEscapedSlashesAction path_with_escaped_slashes_action_; const bool strip_trailing_host_dot_; + const uint64_t max_requests_per_connection_; }; /** diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 95db2bba4f8c7..2dbe42b200bfa 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -205,6 +205,7 @@ class AdminImpl : public Admin, return runCallback(path_and_query, response_headers, response, filter); }; } + uint64_t maxRequestsPerConnection() const override { return 0; } private: /** diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 928289062dcef..484389ab62360 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -216,6 +216,7 @@ class FuzzConfig : public ConnectionManagerConfig { originalIpDetectionExtensions() const override { return ip_detection_extensions_; } + uint64_t maxRequestsPerConnection() const override { return 0; } const envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager config_; diff --git a/test/common/http/conn_manager_impl_test_base.h b/test/common/http/conn_manager_impl_test_base.h index 53a19ce2ecd57..6225b61d854d2 100644 --- a/test/common/http/conn_manager_impl_test_base.h +++ b/test/common/http/conn_manager_impl_test_base.h @@ -153,6 +153,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan originalIpDetectionExtensions() const override { return ip_detection_extensions_; } + uint64_t maxRequestsPerConnection() const override { return 0; } Envoy::Event::SimulatedTimeSystem test_time_; NiceMock route_config_provider_; diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 82f34bcc3ba24..969fd4211e966 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -3794,6 +3794,38 @@ TEST(HostPartitionTest, PartitionHostsImmediateFailureExcludeDisabled) { EXPECT_EQ(hosts[2], update_hosts_params.excluded_hosts_per_locality->get()[1][0]); } +TEST_F(ClusterInfoImplTest, MaxRequestsPerConnectionValidation) { + const std::string yaml = R"EOF( + name: cluster1 + type: STRICT_DNS + lb_policy: ROUND_ROBIN + max_requests_per_connection: 3 + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + common_http_protocol_options: + max_requests_per_connection: 3 + use_downstream_protocol_config: {} +)EOF"; + + EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, + "Only one of max_requests_per_connection from Cluster or " + "HttpProtocolOptions can be specified"); +} + +TEST_F(ClusterInfoImplTest, DeprecatedMaxRequestsPerConnection) { + const std::string yaml = R"EOF( + name: cluster1 + type: STRICT_DNS + lb_policy: ROUND_ROBIN + max_requests_per_connection: 3 +)EOF"; + + auto cluster = makeCluster(yaml); + + EXPECT_EQ(3U, cluster->info()->maxRequestsPerConnection()); +} + } // namespace } // namespace Upstream } // namespace Envoy diff --git a/test/config/utility.cc b/test/config/utility.cc index 70a3986632510..0ac40d6d3fffd 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1017,6 +1017,17 @@ void ConfigHelper::setConnectTimeout(std::chrono::milliseconds timeout) { connect_timeout_set_ = true; } +void ConfigHelper::setDownstreamMaxRequestsPerConnection(uint64_t max_requests_per_connection) { + addConfigModifier( + [max_requests_per_connection]( + envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { + hcm.mutable_common_http_protocol_options() + ->mutable_max_requests_per_connection() + ->set_value(max_requests_per_connection); + }); +} + envoy::config::route::v3::VirtualHost ConfigHelper::createVirtualHost(const char* domain, const char* prefix, const char* cluster) { envoy::config::route::v3::VirtualHost virtual_host; diff --git a/test/config/utility.h b/test/config/utility.h index 1c764f11ee21e..55dba9b7768c0 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -211,6 +211,9 @@ class ConfigHelper { // Set the connect timeout on upstream connections. void setConnectTimeout(std::chrono::milliseconds timeout); + // Set the max_requests_per_connection for downstream through the HttpConnectionManager. + void setDownstreamMaxRequestsPerConnection(uint64_t max_requests_per_connection); + envoy::config::route::v3::VirtualHost createVirtualHost(const char* host, const char* route = "/", const char* cluster = "cluster_0"); diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index 67e2347038b79..bf073cd25a2a2 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -886,6 +886,42 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeaderCountConfigurable) { EXPECT_EQ(200, config.maxRequestHeadersCount()); } +// Checking that default max_requests_per_connection is 0. +TEST_F(HttpConnectionManagerConfigTest, DefaultMaxRequestPerConnection) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_, + filter_config_provider_manager_); + EXPECT_EQ(0, config.maxRequestsPerConnection()); +} + +// Check that max_requests_per_connection is configured. +TEST_F(HttpConnectionManagerConfigTest, MaxRequestPerConnectionConfigurable) { + const std::string yaml_string = R"EOF( + stat_prefix: ingress_http + common_http_protocol_options: + max_requests_per_connection: 5 + route_config: + name: local_route + http_filters: + - name: envoy.filters.http.router + )EOF"; + + HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_, + date_provider_, route_config_provider_manager_, + scoped_routes_config_provider_manager_, http_tracer_manager_, + filter_config_provider_manager_); + EXPECT_EQ(5, config.maxRequestsPerConnection()); +} + TEST_F(HttpConnectionManagerConfigTest, ServerOverwrite) { const std::string yaml_string = R"EOF( stat_prefix: ingress_http diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 10c57fa1fac6d..6038ca7fae297 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -440,9 +440,15 @@ TEST_P(IntegrationTest, UpstreamDisconnectWithTwoRequests) { auto* static_resources = bootstrap.mutable_static_resources(); auto* cluster = static_resources->mutable_clusters(0); // Ensure we only have one connection upstream, one request active at a time. - cluster->mutable_max_requests_per_connection()->set_value(1); + ConfigHelper::HttpProtocolOptions protocol_options; + protocol_options.mutable_common_http_protocol_options() + ->mutable_max_requests_per_connection() + ->set_value(1); + protocol_options.mutable_use_downstream_protocol_config(); auto* circuit_breakers = cluster->mutable_circuit_breakers(); circuit_breakers->add_thresholds()->mutable_max_connections()->set_value(1); + ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), + protocol_options); }); initialize(); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 39a95383d33d3..8a31ce5dda486 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -2445,6 +2445,48 @@ TEST_P(DownstreamProtocolIntegrationTest, BasicMaxStreamTimeoutLegacy) { EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("max_duration_timeout")); } +TEST_P(DownstreamProtocolIntegrationTest, MaxRequestsPerConnectionReached) { + config_helper_.setDownstreamMaxRequestsPerConnection(2); + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + // Sending first request and waiting to complete the response. + auto encoder_decoder = codec_client_->startRequest(default_request_headers_); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, 1, true); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(response->complete()); + EXPECT_EQ(test_server_->counter("http.config_test.downstream_cx_max_requests_reached")->value(), + 0); + + // Sending second request and waiting to complete the response. + auto encoder_decoder_2 = codec_client_->startRequest(default_request_headers_); + request_encoder_ = &encoder_decoder_2.first; + auto response_2 = std::move(encoder_decoder_2.second); + codec_client_->sendData(*request_encoder_, 1, true); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + + ASSERT_TRUE(response_2->waitForEndStream()); + EXPECT_TRUE(upstream_request_->complete()); + EXPECT_TRUE(response_2->complete()); + EXPECT_EQ(test_server_->counter("http.config_test.downstream_cx_max_requests_reached")->value(), + 1); + + if (downstream_protocol_ == Http::CodecType::HTTP1) { + EXPECT_EQ(nullptr, response->headers().Connection()); + EXPECT_EQ("close", response_2->headers().getConnectionValue()); + } else { + EXPECT_TRUE(codec_client_->sawGoAway()); + } + ASSERT_TRUE(codec_client_->waitForDisconnect()); +} + // Make sure that invalid authority headers get blocked at or before the HCM. TEST_P(DownstreamProtocolIntegrationTest, InvalidAuthority) { initialize(); diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index 21c17e4f7ad9d..83cf041e07020 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -600,6 +600,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { pathWithEscapedSlashesAction, (), (const)); MOCK_METHOD(const std::vector&, originalIpDetectionExtensions, (), (const)); + MOCK_METHOD(uint64_t, maxRequestsPerConnection, (), (const)); std::unique_ptr internal_address_config_ = std::make_unique();