diff --git a/changelogs/current.yaml b/changelogs/current.yaml index aadaf66a5a289..10555388505a0 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -271,6 +271,10 @@ bug_fixes: removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` +- area: tcp_proxy + change: | + Removed runtime guard ``envoy.reloadable_features.tcp_proxy_set_idle_timer_immediately_on_new_connection`` + and legacy code path. new_features: - area: dynamic_modules diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 6b452a1d5df9c..00469f376455b 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -91,7 +91,6 @@ RUNTIME_GUARD(envoy_reloadable_features_reset_with_error); RUNTIME_GUARD(envoy_reloadable_features_safe_http2_options); RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests); RUNTIME_GUARD(envoy_reloadable_features_tcp_proxy_odcds_over_ads_fix); -RUNTIME_GUARD(envoy_reloadable_features_tcp_proxy_set_idle_timer_immediately_on_new_connection); RUNTIME_GUARD(envoy_reloadable_features_test_feature_true); RUNTIME_GUARD(envoy_reloadable_features_tls_certificate_compression_brotli); RUNTIME_GUARD(envoy_reloadable_features_trace_refresh_after_route_refresh); diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index d052fe2b61088..5d31b162436a3 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -1125,12 +1125,9 @@ Network::FilterStatus Filter::onNewConnection() { idle_timer_ = read_callbacks_->connection().dispatcher().createTimer( [upstream_callbacks = upstream_callbacks_]() { upstream_callbacks->onIdleTimeout(); }); - if (Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.tcp_proxy_set_idle_timer_immediately_on_new_connection")) { - // Start the idle timer immediately so that if no response is received from the upstream, - // the downstream connection will time out. - resetIdleTimer(); - } + // Start the idle timer immediately so that if no response is received from the upstream, + // the downstream connection will time out. + resetIdleTimer(); } // Set UUID for the connection. This is used for logging and tracing. diff --git a/test/integration/tcp_tunneling_integration_test.cc b/test/integration/tcp_tunneling_integration_test.cc index bd7ca58b77ce3..c23156ebbeeee 100644 --- a/test/integration/tcp_tunneling_integration_test.cc +++ b/test/integration/tcp_tunneling_integration_test.cc @@ -1970,52 +1970,6 @@ TEST_P(TcpTunnelingIntegrationTest, UpstreamConnectingDownstreamDisconnect) { ASSERT_TRUE(fake_upstream_connection_->close()); } -// Test idle timeout when connection establishment is prevented by not sending upstream response -TEST_P(TcpTunnelingIntegrationTest, - IdleTimeoutNoUpstreamConnectionNoIdleTimeoutSetOnNewConnection) { - config_helper_.addRuntimeOverride( - "envoy.reloadable_features.tcp_proxy_set_idle_timer_immediately_on_new_connection", "false"); - config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { - auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(1); - auto* filter_chain = listener->mutable_filter_chains(0); - auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); - - ASSERT_TRUE(config_blob->Is()); - auto tcp_proxy_config = - MessageUtil::anyConvert( - *config_blob); - - tcp_proxy_config.mutable_tunneling_config()->set_hostname("foo.lyft.com:80"); - tcp_proxy_config.mutable_idle_timeout()->CopyFrom( - ProtobufUtil::TimeUtil::MillisecondsToDuration(1)); - - config_blob->PackFrom(tcp_proxy_config); - }); - - initialize(); - - // Start downstream TCP connection (CONNECT will be sent upstream). - tcp_client_ = makeTcpConnection(lookupPort("tcp_proxy")); - ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); - ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); - ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); - EXPECT_EQ(upstream_request_->headers().getMethodValue(), "CONNECT"); - - // Don't send response headers - this prevents the tunnel from being fully established. - // The TCP proxy will wait for the response, and the idle timeout will not trigger as - // the idle timeout is not set immediately on new connection. - - // Verify the stream wasn't reset due to timeout - if (upstreamProtocol() == Http::CodecType::HTTP1) { - ASSERT_FALSE(fake_upstream_connection_->waitForDisconnect(std::chrono::milliseconds(10))); - } else { - ASSERT_FALSE(upstream_request_->waitForReset(std::chrono::milliseconds(10))); - } - - // Clean up the TCP client - tcp_client_->close(); -} - // Test that a downstream flush works correctly (all data is flushed) TEST_P(TcpTunnelingIntegrationTest, TcpProxyDownstreamFlush) { // Use a very large size to make sure it is larger than the kernel socket read buffer.