diff --git a/test/integration/circuit_breakers_integration_test.cc b/test/integration/circuit_breakers_integration_test.cc index ddea875046902..63702ccc4cb4f 100644 --- a/test/integration/circuit_breakers_integration_test.cc +++ b/test/integration/circuit_breakers_integration_test.cc @@ -196,117 +196,45 @@ TEST_P(CircuitBreakersIntegrationTest, CircuitBreakerRuntimeProto) { #endif } -class OutlierDetectionIntegrationTest : public HttpProtocolIntegrationTest { +// Parameterized integration test. The parameters in the tuple are: +// - name of the test +// - basic (legacy) outlier detection config snippet +// - outlier detection extension config snippet to be added to the cluster +// - response code to be sent from upstream +// - optional response header value +class OutlierDetectionIntegrationTest + : public HttpProtocolIntegrationTestWithParams> { public: - void initialize() override { HttpProtocolIntegrationTest::initialize(); } -}; + static constexpr size_t TEST_NAME = 0; + static constexpr size_t BASIC_OD_CONFIG = 1; + static constexpr size_t EXT_OD_CONFIG = 2; + static constexpr size_t RESPONSE_CODE = 3; + static constexpr size_t OPTIONAL_HEADER = 4; -INSTANTIATE_TEST_SUITE_P( - Protocols, OutlierDetectionIntegrationTest, - testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParamsWithoutHTTP3()), + static std::string protocolTestParamsToString(const ::testing::TestParamInfo& params) { - HttpProtocolIntegrationTest::protocolTestParamsToString); - -// Test verifies that empty outlier detection setting in protocol options -// do not interfere with existing outlier detection. -TEST_P(OutlierDetectionIntegrationTest, NoClusterOverwrite) { - config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { - auto* static_resources = bootstrap.mutable_static_resources(); - auto* cluster = static_resources->mutable_clusters(0); - - cluster->mutable_common_lb_config()->mutable_healthy_panic_threshold()->set_value(0); - - auto* outlier_detection = cluster->mutable_outlier_detection(); - - TestUtility::loadFromYaml(R"EOF( + return absl::StrCat( + HttpProtocolIntegrationTestBase::testNameFromTestParams(std::get<0>(params.param)), "_", + std::get(std::get<1>(params.param))); + } + // Set of basic outlier detection configs. + static constexpr absl::string_view consecutive_5xx_only = R"EOF( consecutive_5xx: 2 max_ejection_percent: 100 - )EOF", - *outlier_detection); - - ConfigHelper::HttpProtocolOptions protocol_options; - std::string protocol_options_yaml; - if (absl::StrContains(::testing::UnitTest::GetInstance()->current_test_info()->name(), - "Http2Upstream")) { - protocol_options_yaml += R"EOF( - explicit_http_config: - http2_protocol_options: {} )EOF"; - } else { - ASSERT(absl::StrContains(::testing::UnitTest::GetInstance()->current_test_info()->name(), - "HttpUpstream")); - protocol_options_yaml += R"EOF( - explicit_http_config: - http_protocol_options: {} - )EOF"; - } - TestUtility::loadFromYaml(protocol_options_yaml, protocol_options); - ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), - protocol_options); - }); - - initialize(); - codec_client_ = makeHttpConnection(lookupPort("http")); - // return en error from upstream server. - default_response_headers_.setStatus(500); - for (auto i = 1; i <= 2; i++) { - auto response = - sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0); - - ASSERT_TRUE(response->waitForEndStream()); - // 500 error should be propagated to downstream client. - EXPECT_EQ("500", response->headers().getStatusValue()); - } - - // Send another request. It should not reach upstream and should be handled by envoy. - // The only existing endpoint in the cluster has been marked as unhealthy. - IntegrationStreamDecoderPtr response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_TRUE(response->complete()); - EXPECT_EQ("503", response->headers().getStatusValue()); - - codec_client_->close(); -} - -// Test verifies that non-5xx codes defined in cluster's protocol options -// are thread as errors and cause outlier detection to mark a host as unhealthy. -TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteNon5xxAsErrors) { - config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { - auto* static_resources = bootstrap.mutable_static_resources(); - auto* cluster = static_resources->mutable_clusters(0); - - cluster->mutable_common_lb_config()->mutable_healthy_panic_threshold()->set_value(0); - - auto* outlier_detection = cluster->mutable_outlier_detection(); - - TestUtility::loadFromYaml(R"EOF( - consecutive_5xx: 2 + static constexpr absl::string_view gateway_only = R"EOF( + consecutive_5xx: 0 + consecutive_gateway_failure: 2 + enforcing_consecutive_gateway_failure: 100 max_ejection_percent: 100 - )EOF", - *outlier_detection); - - ConfigHelper::HttpProtocolOptions protocol_options; - std::string protocol_options_yaml; - if (absl::StrContains(::testing::UnitTest::GetInstance()->current_test_info()->name(), - "Http2Upstream")) { - protocol_options_yaml += R"EOF( - explicit_http_config: - http2_protocol_options: {} - )EOF"; - } else { - ASSERT(absl::StrContains(::testing::UnitTest::GetInstance()->current_test_info()->name(), - "HttpUpstream")); - protocol_options_yaml += R"EOF( - explicit_http_config: - http_protocol_options: {} - )EOF"; - } + )EOF"; - // Configure any response with code 300-305 or response test-header containing - // string "treat-as-error" to be treated as 5xx code. - protocol_options_yaml += R"EOF( + // set of different outlier detection extension configs. + // Configure any response with code 300-305 or response test-header containing + // string "treat-as-error" to be treated as 5xx code. + static constexpr absl::string_view error_3xx_and_header = R"EOF( outlier_detection: error_matcher: or_match: @@ -322,46 +250,25 @@ TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteNon5xxAsErrors) { - name: "test-header" string_match: contains: "treat-as-error" - )EOF", - - TestUtility::loadFromYaml(protocol_options_yaml, protocol_options); - ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), - protocol_options); - }); - - initialize(); - - codec_client_ = makeHttpConnection(lookupPort("http")); - - // respond with status code 301. It should be treated as error. - default_response_headers_.setStatus(301); - auto response = - sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_EQ("301", response->headers().getStatusValue()); - - // Respond with status code 200 with "test-header". - // It should be treated as error. - default_response_headers_.setStatus(200); - default_response_headers_.appendCopy(Http::LowerCaseString("test-header"), "treat-as-error"); - - response = - sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_EQ("200", response->headers().getStatusValue()); + )EOF"; - // Now send a request. It will be captured by Envoy and 503 will be returned as the only upstream - // is unhealthy now.. - response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_TRUE(response->complete()); - EXPECT_EQ("503", response->headers().getStatusValue()); + static constexpr absl::string_view gateway_error = R"EOF( + outlier_detection: + error_matcher: + http_response_headers_match: + headers: + - name: ":status" + range_match: + start: 502 + end: 503 + )EOF"; +}; - codec_client_->close(); -} -// Test verifies that 5xx gateway errors configured in cluster protocol options are -// forwarded to outlier detection in the original form and are not converted to code 500. -TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteGatewayErrors) { +// Test configures outlier detection extensions in cluster. +// Then it sends several http requests and responses. +// Responses match configured outlier extensions and cause the endpoint to be marked +// as unhealthy. +TEST_P(OutlierDetectionIntegrationTest, ExtensionsTest) { config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* static_resources = bootstrap.mutable_static_resources(); auto* cluster = static_resources->mutable_clusters(0); @@ -370,12 +277,7 @@ TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteGatewayErrors) { auto* outlier_detection = cluster->mutable_outlier_detection(); - TestUtility::loadFromYaml(R"EOF( - consecutive_5xx: 0 - consecutive_gateway_failure: 2 - enforcing_consecutive_gateway_failure: 100 - max_ejection_percent: 100 - )EOF", + TestUtility::loadFromYaml(std::string(std::get(std::get<1>(GetParam()))), *outlier_detection); ConfigHelper::HttpProtocolOptions protocol_options; @@ -395,18 +297,10 @@ TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteGatewayErrors) { )EOF"; } - protocol_options_yaml += R"EOF( - outlier_detection: - error_matcher: - http_response_headers_match: - headers: - - name: ":status" - range_match: - start: 502 - end: 503 - )EOF", + // Add config outlier detection extension config + protocol_options_yaml += std::get(std::get<1>(GetParam())); - TestUtility::loadFromYaml(protocol_options_yaml, protocol_options); + TestUtility::loadFromYaml(protocol_options_yaml, protocol_options); ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), protocol_options); }); @@ -414,17 +308,24 @@ TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteGatewayErrors) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); - default_response_headers_.setStatus(502); + // Set the response code. + const uint32_t response_code = std::get(std::get<1>(GetParam())); + default_response_headers_.setStatus(response_code); + // Add header if test parameter includes one. + if (!std::get(std::get<1>(GetParam())).empty()) { + default_response_headers_.appendCopy(Http::LowerCaseString("test-header"), + std::get(std::get<1>(GetParam()))); + } for (auto i = 1; i <= 2; i++) { auto response = sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0); ASSERT_TRUE(response->waitForEndStream()); - EXPECT_EQ("502", response->headers().getStatusValue()); + EXPECT_EQ(std::to_string(response_code), response->headers().getStatusValue()); } // Now send a request. It will be captured by Envoy and 503 will be returned as the only upstream - // is unhealthy now.. + // is unhealthy now. IntegrationStreamDecoderPtr response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); ASSERT_TRUE(response->waitForEndStream()); @@ -434,5 +335,30 @@ TEST_P(OutlierDetectionIntegrationTest, ClusterOverwriteGatewayErrors) { codec_client_->close(); } +INSTANTIATE_TEST_SUITE_P( + Protocols, OutlierDetectionIntegrationTest, + testing::Combine( + testing::ValuesIn(HttpProtocolIntegrationTestBase::getProtocolTestParamsWithoutHTTP3()), + testing::Values( + // Regression test. Test verifies that empty outlier detection setting in protocol + // options do not interfere with existing outlier detection. + std::make_tuple(std::string("test1"), + OutlierDetectionIntegrationTest::consecutive_5xx_only, "", 500, ""), + // In this test, outlier extensions define 3xx codes as errors. + std::make_tuple(std::string("test2"), + OutlierDetectionIntegrationTest::consecutive_5xx_only, + OutlierDetectionIntegrationTest::error_3xx_and_header, 301, ""), + // Test verifies that when outlier extensions include gateway code 502, that code + // is forwarded in original form and is not converted to generic code 500. + std::make_tuple(std::string("test3"), OutlierDetectionIntegrationTest::gateway_only, + OutlierDetectionIntegrationTest::gateway_error, 502, ""), + // Test verifies that responses where a matcher matches not code, but response + // header are treated as errors. + std::make_tuple(std::string("test4"), + OutlierDetectionIntegrationTest::consecutive_5xx_only, + OutlierDetectionIntegrationTest::error_3xx_and_header, 200, + "treat-as-error"))), + OutlierDetectionIntegrationTest::protocolTestParamsToString); + } // namespace } // namespace Envoy diff --git a/test/integration/http_protocol_integration.cc b/test/integration/http_protocol_integration.cc index d417d2e315e49..a3ec7e5a4da84 100644 --- a/test/integration/http_protocol_integration.cc +++ b/test/integration/http_protocol_integration.cc @@ -3,7 +3,7 @@ #include "absl/strings/str_cat.h" namespace Envoy { -std::vector HttpProtocolIntegrationTest::getProtocolTestParams( +std::vector HttpProtocolIntegrationTestBase::getProtocolTestParams( const std::vector& downstream_protocols, const std::vector& upstream_protocols) { std::vector ret; @@ -54,16 +54,21 @@ std::vector HttpProtocolIntegrationTest::getProtocolTest return ret; } -std::string HttpProtocolIntegrationTest::protocolTestParamsToString( +std::string +HttpProtocolIntegrationTestBase::testNameFromTestParams(const HttpProtocolTestParams& params) { + return absl::StrCat((params.version == Network::Address::IpVersion::v4 ? "IPv4_" : "IPv6_"), + downstreamToString(params.downstream_protocol), + upstreamToString(params.upstream_protocol), + http2ImplementationToString(params.http2_implementation), + params.use_universal_header_validator ? "Uhv" : "Legacy"); +} + +std::string HttpProtocolIntegrationTestBase::protocolTestParamsToString( const ::testing::TestParamInfo& params) { - return absl::StrCat((params.param.version == Network::Address::IpVersion::v4 ? "IPv4_" : "IPv6_"), - downstreamToString(params.param.downstream_protocol), - upstreamToString(params.param.upstream_protocol), - http2ImplementationToString(params.param.http2_implementation), - params.param.use_universal_header_validator ? "Uhv" : "Legacy"); + return testNameFromTestParams(params.param); } -void HttpProtocolIntegrationTest::setUpstreamOverrideStreamErrorOnInvalidHttpMessage() { +void HttpProtocolIntegrationTestBase::setUpstreamOverrideStreamErrorOnInvalidHttpMessage() { config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { RELEASE_ASSERT(bootstrap.mutable_static_resources()->clusters_size() >= 1, ""); ConfigHelper::HttpProtocolOptions protocol_options; @@ -83,7 +88,7 @@ void HttpProtocolIntegrationTest::setUpstreamOverrideStreamErrorOnInvalidHttpMes }); } -void HttpProtocolIntegrationTest::setDownstreamOverrideStreamErrorOnInvalidHttpMessage() { +void HttpProtocolIntegrationTestBase::setDownstreamOverrideStreamErrorOnInvalidHttpMessage() { config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { diff --git a/test/integration/http_protocol_integration.h b/test/integration/http_protocol_integration.h index ecda682013507..d77a7f0362e05 100644 --- a/test/integration/http_protocol_integration.h +++ b/test/integration/http_protocol_integration.h @@ -32,8 +32,7 @@ absl::string_view http2ImplementationToString(Http2Impl impl); // .... // } // TODO(#20996) consider switching to SimulatedTimeSystem instead of using real time. -class HttpProtocolIntegrationTest : public testing::TestWithParam, - public HttpIntegrationTest { +class HttpProtocolIntegrationTestBase : public HttpIntegrationTest, public testing::Test { public: // By default returns 8 combinations of // [HTTP upstream / HTTP downstream] x [Ipv4, IPv6] @@ -64,24 +63,30 @@ class HttpProtocolIntegrationTest : public testing::TestWithParam& p); - HttpProtocolIntegrationTest() - : HttpProtocolIntegrationTest(ConfigHelper::httpProxyConfig( - /*downstream_is_quic=*/GetParam().downstream_protocol == Http::CodecType::HTTP3)) {} - - HttpProtocolIntegrationTest(const std::string config) - : HttpIntegrationTest(GetParam().downstream_protocol, GetParam().version, config), - use_universal_header_validator_(GetParam().use_universal_header_validator) { - setupHttp2ImplOverrides(GetParam().http2_implementation); + HttpProtocolIntegrationTestBase(const HttpProtocolTestParams& test_params) + : HttpProtocolIntegrationTestBase( + ConfigHelper::httpProxyConfig( + /*downstream_is_quic=*/test_params.downstream_protocol == Http::CodecType::HTTP3), + test_params) {} + + HttpProtocolIntegrationTestBase(const std::string config, + const HttpProtocolTestParams& test_params) + : HttpIntegrationTest(test_params.downstream_protocol, test_params.version, config), + test_params_(test_params), + use_universal_header_validator_(test_params.use_universal_header_validator) { + setupHttp2ImplOverrides(test_params.http2_implementation); config_helper_.addRuntimeOverride("envoy.reloadable_features.enable_universal_header_validator", - GetParam().use_universal_header_validator ? "true" : "false"); + test_params.use_universal_header_validator ? "true" + : "false"); } void SetUp() override { - setDownstreamProtocol(GetParam().downstream_protocol); - setUpstreamProtocol(GetParam().upstream_protocol); + setDownstreamProtocol(test_params_.downstream_protocol); + setUpstreamProtocol(test_params_.upstream_protocol); } void initialize() override { @@ -95,10 +100,35 @@ class HttpProtocolIntegrationTest : public testing::TestWithParam +class HttpProtocolIntegrationTestWithParams + : public testing::WithParamInterface< + typename std::conditional<(sizeof...(T) > 0), std::tuple, + HttpProtocolTestParams>::type>, + public HttpProtocolIntegrationTestBase { + static const HttpProtocolTestParams& getTestBaseParam() { + if constexpr (sizeof...(T) > 0) { + return std::get<0>( + testing::TestWithParam>::GetParam()); + } else { + return testing::TestWithParam::GetParam(); + } + } + +public: + HttpProtocolIntegrationTestWithParams() : HttpProtocolIntegrationTestBase(getTestBaseParam()) {} +}; + +// Basic class used for testing without additional parameters. +using HttpProtocolIntegrationTest = HttpProtocolIntegrationTestWithParams<>; + class UpstreamDownstreamIntegrationTest : public testing::TestWithParam>, public HttpIntegrationTest {