From aba48793f870b12b0021abed47b64ba358aabb3b Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 17 Aug 2022 17:30:48 -0700 Subject: [PATCH] XdsSecurityTest: Use a timeout of 5000ms for RPCs (#30621) * XdsSecurityTest: Use a timeout of 5000ms for RPCs * Reviewer comments * Remove old comment --- .../end2end/xds/xds_cluster_end2end_test.cc | 3 +- test/cpp/end2end/xds/xds_core_end2end_test.cc | 3 +- test/cpp/end2end/xds/xds_end2end_test.cc | 59 ++++++++++++------- test/cpp/end2end/xds/xds_end2end_test_lib.cc | 4 +- test/cpp/end2end/xds/xds_end2end_test_lib.h | 27 +++++---- 5 files changed, 61 insertions(+), 35 deletions(-) diff --git a/test/cpp/end2end/xds/xds_cluster_end2end_test.cc b/test/cpp/end2end/xds/xds_cluster_end2end_test.cc index 140740b596e..6887be3d39b 100644 --- a/test/cpp/end2end/xds/xds_cluster_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_cluster_end2end_test.cc @@ -84,7 +84,8 @@ TEST_P(CdsTest, InvalidClusterStillExistsIfPreviouslyCached) { auto cluster = default_cluster_; cluster.set_type(Cluster::STATIC); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION, StatusCode::OK); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions(), StatusCode::OK); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::ContainsRegex(absl::StrCat( diff --git a/test/cpp/end2end/xds/xds_core_end2end_test.cc b/test/cpp/end2end/xds/xds_core_end2end_test.cc index 23b0cb7c59c..13a061dc2f6 100644 --- a/test/cpp/end2end/xds/xds_core_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_core_end2end_test.cc @@ -359,7 +359,8 @@ TEST_P(GlobalXdsClientTest, InvalidListenerStillExistsIfPreviouslyCached) { auto listener = default_listener_; listener.clear_api_listener(); balancer_->ads_service()->SetLdsResource(listener); - const auto response_state = WaitForLdsNack(DEBUG_LOCATION, StatusCode::OK); + const auto response_state = + WaitForLdsNack(DEBUG_LOCATION, RpcOptions(), StatusCode::OK); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::ContainsRegex(absl::StrCat( diff --git a/test/cpp/end2end/xds/xds_end2end_test.cc b/test/cpp/end2end/xds/xds_end2end_test.cc index 20a2e08b54e..59e339d196d 100644 --- a/test/cpp/end2end/xds/xds_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_end2end_test.cc @@ -385,7 +385,7 @@ class XdsSecurityTest : public XdsEnd2endTest { // error message here. return false; }, - /* timeout_ms= */ 20 * 1000); + /* timeout_ms= */ 20 * 1000, RpcOptions().set_timeout_ms(5000)); } else { backends_[backend_index_]->backend_service()->ResetCounters(); SendRpcsUntil( @@ -409,7 +409,7 @@ class XdsSecurityTest : public XdsEnd2endTest { expected_authenticated_identity); return false; }, - /* timeout_ms= */ 20 * 1000, RpcOptions()); + /* timeout_ms= */ 20 * 1000, RpcOptions().set_timeout_ms(5000)); } } @@ -435,7 +435,8 @@ TEST_P(XdsSecurityTest, UnknownTransportSocket) { auto* transport_socket = cluster.mutable_transport_socket(); transport_socket->set_name("unknown_transport_socket"); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr( @@ -448,7 +449,8 @@ TEST_P(XdsSecurityTest, auto* transport_socket = cluster.mutable_transport_socket(); transport_socket->set_name("envoy.transport_sockets.tls"); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr("TLS configuration provided but no " @@ -467,7 +469,8 @@ TEST_P( *validation_context->add_match_subject_alt_names() = server_san_exact_; transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr("TLS configuration provided but no " @@ -486,7 +489,8 @@ TEST_P( ->set_instance_name(std::string("fake_plugin1")); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr("TLS configuration provided but no " @@ -512,7 +516,8 @@ TEST_P(XdsSecurityTest, RegexSanMatcherDoesNotAllowIgnoreCase) { *validation_context->add_match_subject_alt_names() = matcher; transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr( @@ -530,7 +535,8 @@ TEST_P(XdsSecurityTest, UnknownRootCertificateProvider) { ->set_instance_name("unknown"); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr( @@ -552,7 +558,8 @@ TEST_P(XdsSecurityTest, UnknownIdentityCertificateProvider) { ->set_instance_name("fake_plugin1"); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr( @@ -575,7 +582,8 @@ TEST_P(XdsSecurityTest, ->add_verify_certificate_spki("spki"); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -599,7 +607,8 @@ TEST_P(XdsSecurityTest, ->add_verify_certificate_hash("hash"); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -624,7 +633,8 @@ TEST_P(XdsSecurityTest, ->set_value(true); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -647,7 +657,8 @@ TEST_P(XdsSecurityTest, NacksCertificateValidationContextWithCrl) { ->mutable_crl(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -670,7 +681,8 @@ TEST_P(XdsSecurityTest, ->mutable_custom_validator_config(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -688,7 +700,8 @@ TEST_P(XdsSecurityTest, NacksValidationContextSdsSecretConfig) { ->mutable_validation_context_sds_secret_config(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -708,7 +721,8 @@ TEST_P(XdsSecurityTest, NacksTlsParams) { upstream_tls_context.mutable_common_tls_context()->mutable_tls_params(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr("tls_params unsupported")); @@ -728,7 +742,8 @@ TEST_P(XdsSecurityTest, NacksCustomHandshaker) { ->mutable_custom_handshaker(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr("custom_handshaker unsupported")); @@ -747,7 +762,8 @@ TEST_P(XdsSecurityTest, NacksTlsCertificates) { upstream_tls_context.mutable_common_tls_context()->add_tls_certificates(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT(response_state->error_message, ::testing::HasSubstr("tls_certificates unsupported")); @@ -767,7 +783,8 @@ TEST_P(XdsSecurityTest, NacksTlsCertificateSdsSecretConfigs) { ->add_tls_certificate_sds_secret_configs(); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - const auto response_state = WaitForCdsNack(DEBUG_LOCATION); + const auto response_state = + WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000)); ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK"; EXPECT_THAT( response_state->error_message, @@ -787,7 +804,7 @@ TEST_P(XdsSecurityTest, TestTlsConfigurationInCombinedValidationContext) { ->set_instance_name("fake_plugin1"); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - CheckRpcSendOk(DEBUG_LOCATION); + CheckRpcSendOk(DEBUG_LOCATION, 1, RpcOptions().set_timeout_ms(5000)); } // TODO(yashykt): Remove this test once we stop supporting old fields @@ -804,7 +821,7 @@ TEST_P(XdsSecurityTest, ->set_instance_name("fake_plugin1"); transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); balancer_->ads_service()->SetCdsResource(cluster); - CheckRpcSendOk(DEBUG_LOCATION); + CheckRpcSendOk(DEBUG_LOCATION, 1, RpcOptions().set_timeout_ms(5000)); } TEST_P(XdsSecurityTest, TestMtlsConfigurationWithNoSanMatchers) { diff --git a/test/cpp/end2end/xds/xds_end2end_test_lib.cc b/test/cpp/end2end/xds/xds_end2end_test_lib.cc index c0875cb6bbf..8a3e0df7be8 100644 --- a/test/cpp/end2end/xds/xds_end2end_test_lib.cc +++ b/test/cpp/end2end/xds/xds_end2end_test_lib.cc @@ -1002,7 +1002,7 @@ size_t XdsEnd2endTest::WaitForAllBackends( absl::optional XdsEnd2endTest::WaitForNack( const grpc_core::DebugLocation& debug_location, std::function()> get_state, - StatusCode expected_status) { + const RpcOptions& rpc_options, StatusCode expected_status) { absl::optional response_state; auto deadline = absl::Now() + absl::Seconds(30); auto continue_predicate = [&]() { @@ -1014,7 +1014,7 @@ absl::optional XdsEnd2endTest::WaitForNack( response_state->state != AdsServiceImpl::ResponseState::NACKED; }; do { - const Status status = SendRpc(); + const Status status = SendRpc(rpc_options); EXPECT_EQ(expected_status, status.error_code()) << "code=" << status.error_code() << " message=" << status.error_message() << " at " diff --git a/test/cpp/end2end/xds/xds_end2end_test_lib.h b/test/cpp/end2end/xds/xds_end2end_test_lib.h index a83bc226a42..9b7b4d1a367 100644 --- a/test/cpp/end2end/xds/xds_end2end_test_lib.h +++ b/test/cpp/end2end/xds/xds_end2end_test_lib.h @@ -717,7 +717,7 @@ class XdsEnd2endTest : public ::testing::TestWithParam { struct RpcOptions { RpcService service = SERVICE_ECHO; RpcMethod method = METHOD_ECHO; - int timeout_ms = 5000; + int timeout_ms = 1000; bool wait_for_ready = false; std::vector> metadata; // These options are used by the backend service impl. @@ -927,55 +927,62 @@ class XdsEnd2endTest : public ::testing::TestWithParam { absl::optional WaitForNack( const grpc_core::DebugLocation& debug_location, std::function()> get_state, + const RpcOptions& rpc_options = RpcOptions(), StatusCode expected_status = StatusCode::UNAVAILABLE); // Sends RPCs until an LDS NACK is seen. absl::optional WaitForLdsNack( const grpc_core::DebugLocation& debug_location, + const RpcOptions& rpc_options = RpcOptions(), StatusCode expected_status = StatusCode::UNAVAILABLE) { return WaitForNack( debug_location, [&]() { return balancer_->ads_service()->lds_response_state(); }, - expected_status); + rpc_options, expected_status); } // Sends RPCs until an RDS NACK is seen. absl::optional WaitForRdsNack( const grpc_core::DebugLocation& debug_location, + const RpcOptions& rpc_options = RpcOptions(), StatusCode expected_status = StatusCode::UNAVAILABLE) { return WaitForNack( debug_location, [&]() { return RouteConfigurationResponseState(balancer_.get()); }, - expected_status); + rpc_options, expected_status); } // Sends RPCs until a CDS NACK is seen. absl::optional WaitForCdsNack( const grpc_core::DebugLocation& debug_location, + const RpcOptions& rpc_options = RpcOptions(), StatusCode expected_status = StatusCode::UNAVAILABLE) { return WaitForNack( debug_location, [&]() { return balancer_->ads_service()->cds_response_state(); }, - expected_status); + rpc_options, expected_status); } // Sends RPCs until an EDS NACK is seen. absl::optional WaitForEdsNack( - const grpc_core::DebugLocation& debug_location) { - return WaitForNack(debug_location, [&]() { - return balancer_->ads_service()->eds_response_state(); - }); + const grpc_core::DebugLocation& debug_location, + const RpcOptions& rpc_options = RpcOptions()) { + return WaitForNack( + debug_location, + [&]() { return balancer_->ads_service()->eds_response_state(); }, + rpc_options); } // Convenient front-end to wait for RouteConfiguration to be NACKed, // regardless of whether it's sent in LDS or RDS. absl::optional WaitForRouteConfigNack( const grpc_core::DebugLocation& debug_location, + const RpcOptions& rpc_options = RpcOptions(), StatusCode expected_status = StatusCode::UNAVAILABLE) { if (GetParam().enable_rds_testing()) { - return WaitForRdsNack(debug_location, expected_status); + return WaitForRdsNack(debug_location, rpc_options, expected_status); } - return WaitForLdsNack(debug_location, expected_status); + return WaitForLdsNack(debug_location, rpc_options, expected_status); } // Convenient front-end for accessing xDS response state for a