diff --git a/test/config/utility.cc b/test/config/utility.cc index 53bb66556a6a..265fec66f477 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1049,6 +1049,12 @@ void ConfigHelper::setConnectTimeout(std::chrono::milliseconds timeout) { connect_timeout_set_ = true; } +void ConfigHelper::disableDelayClose() { + addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { hcm.mutable_delayed_close_timeout()->set_nanos(0); }); +} + void ConfigHelper::setDownstreamMaxRequestsPerConnection(uint64_t max_requests_per_connection) { addConfigModifier( [max_requests_per_connection]( diff --git a/test/config/utility.h b/test/config/utility.h index adbebf501c52..29500b9c8746 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -246,6 +246,10 @@ class ConfigHelper { // Set the connect timeout on upstream connections. void setConnectTimeout(std::chrono::milliseconds timeout); + // Disable delay close. This is especially useful for tests doing raw TCP for + // HTTP/1.1 which functionally frame by connection close. + void disableDelayClose(); + // Set the max_requests_per_connection for downstream through the HttpConnectionManager. void setDownstreamMaxRequestsPerConnection(uint64_t max_requests_per_connection); diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc index dbcda3278f09..edb2559eb8c6 100644 --- a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc @@ -271,6 +271,7 @@ name: router // Verify the grpc cached logger is available after the initial logger filter is destroyed. // Regression test for https://github.com/envoyproxy/envoy/issues/18066 TEST_P(AccessLogIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) { + config_helper_.disableDelayClose(); autonomous_upstream_ = true; // The grpc access logger connection never closes. It's ok to see an incomplete logging stream. autonomous_allow_incomplete_streams_ = true; @@ -301,16 +302,8 @@ TEST_P(AccessLogIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) { // HTTP 1.1 is allowed and the connection is kept open until the listener update. std::string response; - auto connection = - createConnectionDriver(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\n\r\n", - [&response, &dispatcher = *dispatcher_]( - Network::ClientConnection&, const Buffer::Instance& data) -> void { - response.append(data.toString()); - if (response.find("\r\n\r\n") != std::string::npos) { - dispatcher.exit(); - } - }); - connection->run(); + sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\n\r\n", + &response, true); EXPECT_TRUE(response.find("HTTP/1.1 200") == 0); test_server_->waitForCounterEq("access_logs.grpc_access_log.logs_written", 2); diff --git a/test/extensions/filters/network/direct_response/direct_response_integration_test.cc b/test/extensions/filters/network/direct_response/direct_response_integration_test.cc index 49878a65eea2..8a4fa79043a7 100644 --- a/test/extensions/filters/network/direct_response/direct_response_integration_test.cc +++ b/test/extensions/filters/network/direct_response/direct_response_integration_test.cc @@ -43,7 +43,7 @@ TEST_P(DirectResponseIntegrationTest, DirectResponseOnConnection) { response.append(data.toString()); conn.close(Network::ConnectionCloseType::FlushWrite); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_EQ("hello, world!\n", response); EXPECT_THAT(waitForAccessLog(listener_access_log_name_), testing::HasSubstr(StreamInfo::ResponseCodeDetails::get().DirectResponse)); diff --git a/test/extensions/filters/network/echo/echo_integration_test.cc b/test/extensions/filters/network/echo/echo_integration_test.cc index b965d3254d1e..e18b7cec2ee5 100644 --- a/test/extensions/filters/network/echo/echo_integration_test.cc +++ b/test/extensions/filters/network/echo/echo_integration_test.cc @@ -43,7 +43,7 @@ TEST_P(EchoIntegrationTest, Hello) { response.append(data.toString()); conn.close(Network::ConnectionCloseType::FlushWrite); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_EQ("hello", response); } @@ -90,7 +90,7 @@ name: new_listener response.append(data.toString()); conn.close(Network::ConnectionCloseType::FlushWrite); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_EQ("hello", response); // Remove the listener. diff --git a/test/extensions/stats_sinks/hystrix/hystrix_integration_test.cc b/test/extensions/stats_sinks/hystrix/hystrix_integration_test.cc index 758c9edacf10..67dea9e1e98e 100644 --- a/test/extensions/stats_sinks/hystrix/hystrix_integration_test.cc +++ b/test/extensions/stats_sinks/hystrix/hystrix_integration_test.cc @@ -34,7 +34,7 @@ TEST_P(HystrixIntegrationTest, NoChunkEncoding) { conn.close(Network::ConnectionCloseType::NoFlush); } }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n")); // Make sure that the response is not actually chunk encoded, but it does have the hystrix flush // trailer. diff --git a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc index 81db1f99d292..b3bf460a075a 100644 --- a/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc +++ b/test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc @@ -333,7 +333,7 @@ class RawWriteSslIntegrationTest : public SslIntegrationTest { // Drive the connection until we get a response. while (response.empty()) { - connection->run(Event::Dispatcher::RunType::NonBlock); + EXPECT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, testing::HasSubstr("HTTP/1.1 200 OK\r\n")); diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc index fb2bd2633a85..7a7a16b0858c 100644 --- a/test/integration/base_integration_test.cc +++ b/test/integration/base_integration_test.cc @@ -446,7 +446,10 @@ void BaseIntegrationTest::sendRawHttpAndWaitForResponse( }, std::move(transport_socket)); - connection->run(); + if (connection->run() != testing::AssertionSuccess()) { + FAIL() << "Failed to get expected response within the time bound\n" + << "received " << *response << "\n"; + } } void BaseIntegrationTest::useListenerAccessLog(absl::string_view format) { diff --git a/test/integration/command_formatter_extension_integration_test.cc b/test/integration/command_formatter_extension_integration_test.cc index fe0643b39a90..e0fe588e6130 100644 --- a/test/integration/command_formatter_extension_integration_test.cc +++ b/test/integration/command_formatter_extension_integration_test.cc @@ -17,6 +17,7 @@ class CommandFormatterExtensionIntegrationTest : public testing::Test, public Ht }; TEST_F(CommandFormatterExtensionIntegrationTest, BasicExtension) { + autonomous_upstream_ = true; TestCommandFactory factory; Registry::InjectFactory command_register(factory); std::vector formatters; diff --git a/test/integration/http_timeout_integration_test.cc b/test/integration/http_timeout_integration_test.cc index 7475a68a7e7c..bf92d4f3a46c 100644 --- a/test/integration/http_timeout_integration_test.cc +++ b/test/integration/http_timeout_integration_test.cc @@ -533,13 +533,13 @@ TEST_P(HttpTimeoutIntegrationTest, RequestHeaderTimeout) { }); while (!connection_driver->allBytesSent()) { - connection_driver->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection_driver->run(Event::Dispatcher::RunType::NonBlock)); } test_server_->waitForGaugeGe("http.config_test.downstream_rq_active", 1); ASSERT_FALSE(connection_driver->closed()); timeSystem().advanceTimeWait(std::chrono::milliseconds(1001)); - connection_driver->run(); + ASSERT_TRUE(connection_driver->run()); // The upstream should send a 40x response and send a local reply. EXPECT_TRUE(connection_driver->closed()); diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index 167a79630c70..2ace78b9e121 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -12,6 +12,7 @@ namespace { class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { public: void initialize() override { + config_helper_.disableDelayClose(); useAccessLog("%RESPONSE_CODE_DETAILS%"); config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 8e5a849ec9a4..2fb98617fcd9 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -717,6 +717,7 @@ TEST_P(IntegrationTest, UpstreamDisconnectWithTwoRequests) { } TEST_P(IntegrationTest, TestSmuggling) { + config_helper_.disableDelayClose(); initialize(); // Make sure the http parser rejects having content-length and transfer-encoding: chunked @@ -1106,13 +1107,13 @@ TEST_P(IntegrationTest, Pipeline) { }); // First response should be success. while (response.find("200") == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n")); // Second response should be 400 (no host) while (response.find("400") == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); connection->close(); @@ -1155,14 +1156,14 @@ TEST_P(IntegrationTest, PipelineWithTrailers) { // First response should be success. size_t pos; while ((pos = response.find("200")) == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, StartsWith("HTTP/1.1 200 OK\r\n")); while (response.find("200", pos + 1) == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } while (response.find("400") == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); @@ -1188,12 +1189,12 @@ TEST_P(IntegrationTest, PipelineInline) { }); while (response.find("400") == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, StartsWith("HTTP/1.1 400 Bad Request\r\n")); while (response.find("426") == std::string::npos) { - connection->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(connection->run(Event::Dispatcher::RunType::NonBlock)); } EXPECT_THAT(response, HasSubstr("HTTP/1.1 426 Upgrade Required\r\n")); connection->close(); diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc index d35fc0a4fe48..579416946fcf 100644 --- a/test/integration/multiplexed_integration_test.cc +++ b/test/integration/multiplexed_integration_test.cc @@ -972,6 +972,7 @@ TEST_P(MultiplexedIntegrationTest, CodecErrorAfterStreamStart) { } TEST_P(MultiplexedIntegrationTest, Http2BadMagic) { + config_helper_.disableDelayClose(); if (downstreamProtocol() == Http::CodecType::HTTP3) { // The "magic" payload is an HTTP/2 specific thing. return; @@ -983,7 +984,7 @@ TEST_P(MultiplexedIntegrationTest, Http2BadMagic) { [&response](Network::ClientConnection&, const Buffer::Instance& data) -> void { response.append(data.toString()); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_EQ("", response); } @@ -997,7 +998,7 @@ TEST_P(MultiplexedIntegrationTest, BadFrame) { [&response](Network::ClientConnection&, const Buffer::Instance& data) -> void { response.append(data.toString()); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_TRUE(response.find("SETTINGS expected") != std::string::npos); } @@ -1324,14 +1325,14 @@ TEST_P(MultiplexedIntegrationTest, DelayedCloseAfterBadFrame) { connection.dispatcher().exit(); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_THAT(response, HasSubstr("SETTINGS expected")); // Due to the multiple dispatchers involved (one for the RawConnectionDriver and another for the // Envoy server), it's possible the delayed close timer could fire and close the server socket // prior to the data callback above firing. Therefore, we may either still be connected, or have // received a remote close. if (connection->lastConnectionEvent() == Network::ConnectionEvent::Connected) { - connection->run(); + ASSERT_TRUE(connection->run()); } EXPECT_EQ(connection->lastConnectionEvent(), Network::ConnectionEvent::RemoteClose); EXPECT_EQ(test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value(), @@ -1353,13 +1354,13 @@ TEST_P(MultiplexedIntegrationTest, DelayedCloseDisabled) { connection.dispatcher().exit(); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_THAT(response, HasSubstr("SETTINGS expected")); // Due to the multiple dispatchers involved (one for the RawConnectionDriver and another for the // Envoy server), it's possible for the 'connection' to receive the data and exit the dispatcher // prior to the FIN being received from the server. if (connection->lastConnectionEvent() == Network::ConnectionEvent::Connected) { - connection->run(); + ASSERT_TRUE(connection->run()); } EXPECT_EQ(connection->lastConnectionEvent(), Network::ConnectionEvent::RemoteClose); EXPECT_EQ(test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value(), diff --git a/test/integration/original_ip_detection_integration_test.cc b/test/integration/original_ip_detection_integration_test.cc index 8bc353e0da49..6e1a5f8a26e7 100644 --- a/test/integration/original_ip_detection_integration_test.cc +++ b/test/integration/original_ip_detection_integration_test.cc @@ -18,6 +18,7 @@ class OriginalIPDetectionIntegrationTest OriginalIPDetectionIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {} void runTest(const std::string& ip) { + autonomous_upstream_ = true; useAccessLog("%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%"); config_helper_.addConfigModifier( [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 1d58b5c0f12b..eabdddfc0180 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -3466,6 +3466,7 @@ TEST_P(ProtocolIntegrationTest, UpstreamDisconnectBeforeResponseCompleteWireByte } TEST_P(DownstreamProtocolIntegrationTest, BadRequest) { + config_helper_.disableDelayClose(); // we only care about upstream protocol. if (downstreamProtocol() != Http::CodecType::HTTP1) { return; diff --git a/test/integration/socket_interface_integration_test.cc b/test/integration/socket_interface_integration_test.cc index 9cae29698a00..ddadc7b5d5ab 100644 --- a/test/integration/socket_interface_integration_test.cc +++ b/test/integration/socket_interface_integration_test.cc @@ -61,7 +61,7 @@ TEST_P(SocketInterfaceIntegrationTest, Basic) { response.append(data.toString()); conn.close(Network::ConnectionCloseType::FlushWrite); }); - connection->run(); + ASSERT_TRUE(connection->run()); EXPECT_EQ("hello", response); } diff --git a/test/integration/utility.cc b/test/integration/utility.cc index 4836a62a2b44..6e0473943bc1 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -267,12 +267,14 @@ RawConnectionDriver::RawConnectionDriver(uint32_t port, DoWriteCallback write_re : dispatcher_(dispatcher), remaining_bytes_to_send_(0) { api_ = Api::createApiForTest(stats_store_); Event::GlobalTimeSystem time_system; - callbacks_ = std::make_unique([this, write_request_callback]() { - Buffer::OwnedImpl buffer; - const bool close_after = write_request_callback(buffer); - remaining_bytes_to_send_ += buffer.length(); - client_->write(buffer, close_after); - }); + callbacks_ = std::make_unique( + [this, write_request_callback]() { + Buffer::OwnedImpl buffer; + const bool close_after = write_request_callback(buffer); + remaining_bytes_to_send_ += buffer.length(); + client_->write(buffer, close_after); + }, + dispatcher); if (transport_socket == nullptr) { transport_socket = Network::Test::createRawBufferSocket(); @@ -307,7 +309,19 @@ void RawConnectionDriver::waitForConnection() { } } -void RawConnectionDriver::run(Event::Dispatcher::RunType run_type) { dispatcher_.run(run_type); } +testing::AssertionResult RawConnectionDriver::run(Event::Dispatcher::RunType run_type, + std::chrono::milliseconds timeout) { + Event::TimerPtr timeout_timer = dispatcher_.createTimer([this]() -> void { dispatcher_.exit(); }); + timeout_timer->enableTimer(timeout); + + dispatcher_.run(run_type); + + if (timeout_timer->enabled()) { + timeout_timer->disableTimer(); + return testing::AssertionSuccess(); + } + return testing::AssertionFailure(); +} void RawConnectionDriver::close() { client_->close(Network::ConnectionCloseType::FlushWrite); } diff --git a/test/integration/utility.h b/test/integration/utility.h index 8db43d1d86e9..bc907e8a1d06 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -19,6 +19,7 @@ #include "test/test_common/printers.h" #include "test/test_common/test_time.h" +#include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -85,7 +86,9 @@ class RawConnectionDriver { Network::TransportSocketPtr transport_socket = nullptr); ~RawConnectionDriver(); const Network::Connection& connection() { return *client_; } - void run(Event::Dispatcher::RunType run_type = Event::Dispatcher::RunType::Block); + testing::AssertionResult + run(Event::Dispatcher::RunType run_type = Event::Dispatcher::RunType::Block, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout); void close(); Network::ConnectionEvent lastConnectionEvent() const { return callbacks_->last_connection_event_; @@ -115,7 +118,8 @@ class RawConnectionDriver { struct ConnectionCallbacks : public Network::ConnectionCallbacks { using WriteCb = std::function; - ConnectionCallbacks(WriteCb write_cb) : write_cb_(write_cb) {} + ConnectionCallbacks(WriteCb write_cb, Event::Dispatcher& dispatcher) + : write_cb_(write_cb), dispatcher_(dispatcher) {} bool connected() const { return connected_; } bool closed() const { return closed_; } @@ -124,11 +128,15 @@ class RawConnectionDriver { if (!connected_ && event == Network::ConnectionEvent::Connected) { write_cb_(); } - last_connection_event_ = event; + closed_ |= (event == Network::ConnectionEvent::RemoteClose || event == Network::ConnectionEvent::LocalClose); connected_ |= (event == Network::ConnectionEvent::Connected); + + if (closed_) { + dispatcher_.exit(); + } } void onAboveWriteBufferHighWatermark() override {} void onBelowWriteBufferLowWatermark() override { write_cb_(); } @@ -137,6 +145,7 @@ class RawConnectionDriver { private: WriteCb write_cb_; + Event::Dispatcher& dispatcher_; bool connected_{false}; bool closed_{false}; }; diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index 8e866132782b..820eecefac89 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -220,7 +220,7 @@ TEST_P(LdsInplaceUpdateTcpProxyIntegrationTest, ReloadConfigDeletingFilterChain) ASSERT_TRUE(fake_upstream_connection_0->write("world")); while (response_0.find("world") == std::string::npos) { - client_conn_0->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(client_conn_0->run(Event::Dispatcher::RunType::NonBlock)); } client_conn_0->close(); while (!client_conn_0->closed()) { @@ -266,7 +266,7 @@ TEST_P(LdsInplaceUpdateTcpProxyIntegrationTest, ReloadConfigAddingFilterChain) { ASSERT_TRUE(fake_upstream_connection_2->write("world2")); while (response_2.find("world2") == std::string::npos) { - client_conn_2->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(client_conn_2->run(Event::Dispatcher::RunType::NonBlock)); } client_conn_2->close(); while (!client_conn_2->closed()) { @@ -279,7 +279,7 @@ TEST_P(LdsInplaceUpdateTcpProxyIntegrationTest, ReloadConfigAddingFilterChain) { ASSERT_TRUE(fake_upstream_connection_0->write("world")); while (response_0.find("world") == std::string::npos) { - client_conn_0->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(client_conn_0->run(Event::Dispatcher::RunType::NonBlock)); } client_conn_0->close(); while (!client_conn_0->closed()) { @@ -550,6 +550,7 @@ INSTANTIATE_TEST_SUITE_P(Protocols, LdsIntegrationTest, // Sample test making sure our config framework correctly reloads listeners. TEST_P(LdsIntegrationTest, ReloadConfig) { + config_helper_.disableDelayClose(); autonomous_upstream_ = true; initialize(); // Given we're using LDS in this test, initialize() will not complete until @@ -669,7 +670,7 @@ TEST_P(LdsStsIntegrationTest, TcpListenerRemoveFilterChainCalledAfterListenerIsR ASSERT_TRUE(fake_upstream_connection_0->write("world")); while (response_0.find("world") == std::string::npos) { - client_conn_0->run(Event::Dispatcher::RunType::NonBlock); + ASSERT_TRUE(client_conn_0->run(Event::Dispatcher::RunType::NonBlock)); } client_conn_0->close(); while (!client_conn_0->closed()) {