Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,10 @@ bug_fixes:

removed_config_or_runtime:
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
- 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
Expand Down
1 change: 0 additions & 1 deletion source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 3 additions & 6 deletions source/common/tcp_proxy/tcp_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
46 changes: 0 additions & 46 deletions test/integration/tcp_tunneling_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy>());
auto tcp_proxy_config =
MessageUtil::anyConvert<envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy>(
*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.
Expand Down
Loading