From 7db1fb31f7c754d92f8cb951c168765c1c50ec4a Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Sat, 18 Apr 2020 21:45:35 -0700 Subject: [PATCH 1/8] Add support for setting grpc status in FI response Signed-off-by: Vijay Rajput --- .../filters/http/fault/v3/fault.proto | 10 +- .../http/http_filters/fault_filter.rst | 8 ++ docs/root/version_history/current.rst | 2 + .../filters/http/fault/v3/fault.proto | 12 +- .../filters/common/fault/fault_config.cc | 35 ++++- .../filters/common/fault/fault_config.h | 49 +++++-- .../filters/http/fault/fault_filter.cc | 77 ++++++++--- .../filters/http/fault/fault_filter.h | 11 +- .../filters/common/fault/fault_config_test.cc | 51 +++++-- .../fault/fault_filter_integration_test.cc | 66 +++++++++ .../filters/http/fault/fault_filter_test.cc | 130 ++++++++++++++++++ 11 files changed, 400 insertions(+), 51 deletions(-) diff --git a/api/envoy/extensions/filters/http/fault/v3/fault.proto b/api/envoy/extensions/filters/http/fault/v3/fault.proto index 534a0da35b16c..bc8bc24eaec4f 100644 --- a/api/envoy/extensions/filters/http/fault/v3/fault.proto +++ b/api/envoy/extensions/filters/http/fault/v3/fault.proto @@ -21,6 +21,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Fault Injection :ref:`configuration overview `. // [#extension: envoy.filters.http.fault] +// [#next-free-field: 6] message FaultAbort { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.fault.v2.FaultAbort"; @@ -41,6 +42,9 @@ message FaultAbort { // HTTP status code to use to abort the HTTP request. uint32 http_status = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}]; + // gRPC status code to use to abort the gRPC request. + uint32 grpc_status = 5 [(validate.rules).uint32 = {lte: 16}]; + // Fault aborts are controlled via an HTTP header (if applicable). HeaderAbort header_abort = 4; } @@ -50,7 +54,7 @@ message FaultAbort { type.v3.FractionalPercent percentage = 3; } -// [#next-free-field: 14] +// [#next-free-field: 15] message HTTPFault { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.fault.v2.HTTPFault"; @@ -133,4 +137,8 @@ message HTTPFault { // The runtime key to override the :ref:`default ` // runtime. The default is: fault.http.rate_limit.response_percent string response_rate_limit_percent_runtime = 13; + + // The runtime key to override the :ref:`default ` + // runtime. The default is: fault.http.abort.grpc_status + string abort_grpc_status_runtime = 14; } diff --git a/docs/root/configuration/http/http_filters/fault_filter.rst b/docs/root/configuration/http/http_filters/fault_filter.rst index 80678714db699..cec46954dbd78 100644 --- a/docs/root/configuration/http/http_filters/fault_filter.rst +++ b/docs/root/configuration/http/http_filters/fault_filter.rst @@ -144,6 +144,14 @@ fault.http.abort.http_status available regardless of whether the filter is :ref:`configured for abort `. +fault.http.abort.grpc_status + gRPC status code that will be used as the response status code of requests that will be + aborted if the headers match. Defaults to the gRPC status code specified + in the config. If this field is missing from both the runtime and the config, + gRPC status code in the response will be derived from *fault.http.abort.http_status* field. + This runtime key is only available when the filter is :ref:`configured for abort + `. + fault.http.delay.fixed_delay_percent % of requests that will be delayed if the headers match. Defaults to the *delay_percent* specified in the config or 0 otherwise. This runtime key is only available when diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index fc637cc644931..3da93218ee184 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -8,6 +8,8 @@ Changes * dynamic forward proxy: added :ref:`SNI based dynamic forward proxy ` support. * fault: added support for controlling the percentage of requests that abort, delay and response rate limits faults are applied to using :ref:`HTTP headers ` to the HTTP fault filter. +* fault: added support for specifying grpc_status code in abort faults using + :ref:`HTTP header ` or abort fault configuration in HTTP fault filter. * filter: add `upstram_rq_time` stats to the GPRC stats filter. Disabled by default and can be enabled via :ref:`enable_upstream_stats `. * http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests. diff --git a/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto b/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto index 07996a9507ff9..a05f840eacc73 100644 --- a/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto +++ b/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto @@ -21,6 +21,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Fault Injection :ref:`configuration overview `. // [#extension: envoy.filters.http.fault] +// [#next-free-field: 6] message FaultAbort { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.fault.v2.FaultAbort"; @@ -41,16 +42,19 @@ message FaultAbort { oneof error_type { option (validate.required) = true; - // Fault aborts are controlled via an HTTP header (if applicable). + // gRPC status code to use to abort the gRPC request. uint32 http_status = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}]; + // Fault aborts are controlled via an HTTP header (if applicable). + uint32 grpc_status = 5 [(validate.rules).uint32 = {lte: 16}]; + // The percentage of requests/operations/connections that will be aborted with the error code // provided. HeaderAbort header_abort = 4; } } -// [#next-free-field: 14] +// [#next-free-field: 15] message HTTPFault { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.fault.v2.HTTPFault"; @@ -133,4 +137,8 @@ message HTTPFault { // The runtime key to override the :ref:`default ` // runtime. The default is: fault.http.rate_limit.response_percent string response_rate_limit_percent_runtime = 13; + + // The runtime key to override the :ref:`default ` + // runtime. The default is: fault.http.abort.grpc_status + string abort_grpc_status_runtime = 14; } diff --git a/source/extensions/filters/common/fault/fault_config.cc b/source/extensions/filters/common/fault/fault_config.cc index 7bcdd765a3eff..7054a899ce0d1 100644 --- a/source/extensions/filters/common/fault/fault_config.cc +++ b/source/extensions/filters/common/fault/fault_config.cc @@ -34,7 +34,14 @@ FaultAbortConfig::FaultAbortConfig( switch (abort_config.error_type_case()) { case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kHttpStatus: provider_ = - std::make_unique(abort_config.http_status(), abort_config.percentage()); + std::make_unique(static_cast(abort_config.http_status()), + absl::nullopt, abort_config.percentage()); + break; + case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kGrpcStatus: + // If gRPC status code is set, then HTTP will be set to 200 + provider_ = std::make_unique( + Http::Code::OK, static_cast(abort_config.grpc_status()), + abort_config.percentage()); break; case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kHeaderAbort: provider_ = std::make_unique(abort_config.percentage()); @@ -44,10 +51,9 @@ FaultAbortConfig::FaultAbortConfig( } } -absl::optional FaultAbortConfig::HeaderAbortProvider::statusCode( - const Http::RequestHeaderMap* request_headers) const { - absl::optional ret; - const auto header = request_headers->get(HeaderNames::get().AbortRequest); +absl::optional +FaultAbortConfig::HeaderAbortProvider::httpStatusCode(const Http::HeaderEntry* header) const { + absl::optional ret = absl::nullopt; if (header == nullptr) { return ret; } @@ -64,6 +70,25 @@ absl::optional FaultAbortConfig::HeaderAbortProvider::statusCode( return ret; } +absl::optional +FaultAbortConfig::HeaderAbortProvider::grpcStatusCode(const Http::HeaderEntry* header) const { + absl::optional ret = absl::nullopt; + if (header == nullptr) { + return ret; + } + + uint64_t code; + if (!absl::SimpleAtoi(header->value().getStringView(), &code)) { + return ret; + } + + if (code <= 16) { + ret = static_cast(code); + } + + return ret; +} + FaultDelayConfig::FaultDelayConfig( const envoy::extensions::filters::common::fault::v3::FaultDelay& delay_config) { switch (delay_config.fault_delay_secifier_case()) { diff --git a/source/extensions/filters/common/fault/fault_config.h b/source/extensions/filters/common/fault/fault_config.h index 2bf80a1e67d27..3bf512a852f18 100644 --- a/source/extensions/filters/common/fault/fault_config.h +++ b/source/extensions/filters/common/fault/fault_config.h @@ -2,6 +2,7 @@ #include "envoy/extensions/filters/common/fault/v3/fault.pb.h" #include "envoy/extensions/filters/http/fault/v3/fault.pb.h" +#include "envoy/grpc/status.h" #include "envoy/http/header_map.h" #include "envoy/type/v3/percent.pb.h" @@ -20,6 +21,7 @@ class HeaderNameValues { const char* prefix() { return ThreadSafeSingleton::get().prefix(); } const Http::LowerCaseString AbortRequest{absl::StrCat(prefix(), "-fault-abort-request")}; + const Http::LowerCaseString AbortGrpcRequest{absl::StrCat(prefix(), "-fault-abort-grpc-request")}; const Http::LowerCaseString AbortRequestPercentage{ absl::StrCat(prefix(), "-fault-abort-request-percentage")}; const Http::LowerCaseString DelayRequest{absl::StrCat(prefix(), "-fault-delay-request")}; @@ -53,8 +55,11 @@ class FaultAbortConfig { public: FaultAbortConfig(const envoy::extensions::filters::http::fault::v3::FaultAbort& abort_config); - absl::optional statusCode(const Http::RequestHeaderMap* request_headers) const { - return provider_->statusCode(request_headers); + absl::optional httpStatusCode(const Http::HeaderEntry* header) const { + return provider_->httpStatusCode(header); + } + absl::optional grpcStatusCode(const Http::HeaderEntry* header) const { + return provider_->grpcStatusCode(header); } envoy::type::v3::FractionalPercent @@ -70,23 +75,35 @@ class FaultAbortConfig { // Return the HTTP status code to use. Optionally passed HTTP headers that may contain the // HTTP status code depending on the provider implementation. - virtual absl::optional - statusCode(const Http::RequestHeaderMap* request_headers) const PURE; + virtual absl::optional httpStatusCode(const Http::HeaderEntry* header) const PURE; + + // Return the gRPC status code to use. Optionally passed an HTTP header that may contain the + // gRPC status code depending on the provider implementation. + virtual absl::optional + grpcStatusCode(const Http::HeaderEntry* header) const PURE; + // Return what percentage of requests abort faults should be applied to. Optionally passed // HTTP headers that may contain the percentage depending on the provider implementation. virtual envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const PURE; }; - // Delay provider that uses a fixed abort status code. + // Abort provider that uses a fixed abort status code. class FixedAbortProvider : public AbortProvider { public: - FixedAbortProvider(uint64_t status_code, const envoy::type::v3::FractionalPercent& percentage) - : status_code_(status_code), percentage_(percentage) {} + FixedAbortProvider(Http::Code http_status_code, + absl::optional grpc_status_code, + const envoy::type::v3::FractionalPercent& percentage) + : http_status_code_(http_status_code), grpc_status_code_(grpc_status_code), + percentage_(percentage) {} - // AbortProvider - absl::optional statusCode(const Http::RequestHeaderMap*) const override { - return static_cast(status_code_); + absl::optional httpStatusCode(const Http::HeaderEntry*) const override { + return http_status_code_; + } + + absl::optional + grpcStatusCode(const Http::HeaderEntry*) const override { + return grpc_status_code_; } envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap*) const override { @@ -94,7 +111,8 @@ class FaultAbortConfig { } private: - const uint64_t status_code_; + const absl::optional http_status_code_; + const absl::optional grpc_status_code_; const envoy::type::v3::FractionalPercent percentage_; }; @@ -103,9 +121,11 @@ class FaultAbortConfig { public: HeaderAbortProvider(const envoy::type::v3::FractionalPercent& percentage) : HeaderPercentageProvider(HeaderNames::get().AbortRequestPercentage, percentage) {} - // AbortProvider - absl::optional - statusCode(const Http::RequestHeaderMap* request_headers) const override; + + absl::optional httpStatusCode(const Http::HeaderEntry* header) const override; + + absl::optional + grpcStatusCode(const Http::HeaderEntry*) const override; envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const override { @@ -180,6 +200,7 @@ class FaultDelayConfig { public: HeaderDelayProvider(const envoy::type::v3::FractionalPercent& percentage) : HeaderPercentageProvider(HeaderNames::get().DelayRequestPercentage, percentage) {} + // DelayProvider absl::optional duration(const Http::RequestHeaderMap* request_headers) const override; diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index f3e277edfe6b7..c895337c09339 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -43,6 +43,8 @@ FaultSettings::FaultSettings(const envoy::extensions::filters::http::fault::v3:: RuntimeKeys::get().DelayDurationKey)), abort_http_status_runtime_(PROTOBUF_GET_STRING_OR_DEFAULT( fault, abort_http_status_runtime, RuntimeKeys::get().AbortHttpStatusKey)), + abort_grpc_status_runtime_(PROTOBUF_GET_STRING_OR_DEFAULT( + fault, abort_grpc_status_runtime, RuntimeKeys::get().AbortGrpcStatusKey)), max_active_faults_runtime_(PROTOBUF_GET_STRING_OR_DEFAULT( fault, max_active_faults_runtime, RuntimeKeys::get().MaxActiveFaultsKey)), response_rate_limit_percent_runtime_( @@ -164,9 +166,12 @@ Http::FilterHeadersStatus FaultFilter::decodeHeaders(Http::RequestHeaderMap& hea return Http::FilterHeadersStatus::StopIteration; } - const auto abort_code = abortHttpStatus(headers); - if (abort_code.has_value()) { - abortWithHTTPStatus(abort_code.value()); + absl::optional http_status; + absl::optional grpc_status; + std::tie(http_status, grpc_status) = abortStatus(headers); + + if (http_status.has_value()) { + abortWithStatus(http_status.value(), grpc_status); return Http::FilterHeadersStatus::StopIteration; } @@ -284,29 +289,57 @@ FaultFilter::delayDuration(const Http::RequestHeaderMap& request_headers) { return ret; } -absl::optional -FaultFilter::abortHttpStatus(const Http::RequestHeaderMap& request_headers) { +AbortHttpAndGrpcStatus FaultFilter::abortStatus(const Http::RequestHeaderMap& request_headers) { if (!isAbortEnabled(request_headers)) { - return absl::nullopt; + return std::make_pair(absl::nullopt, absl::nullopt); } + auto grpc_status = abortGrpcStatus(request_headers); + + // If gRPC status code is set, then HTTP will be set to 200 + return grpc_status.has_value() ? std::make_pair(Http::Code::OK, grpc_status) + : std::make_pair(abortHttpStatus(request_headers), grpc_status); +} + +absl::optional +FaultFilter::abortHttpStatus(const Http::RequestHeaderMap& request_headers) { // See if the configured abort provider has a default status code, if not there is no abort status // code (e.g., header configuration and no/invalid header). - const auto config_abort = fault_settings_->requestAbort()->statusCode(&request_headers); - if (!config_abort.has_value()) { + auto http_status = fault_settings_->requestAbort()->httpStatusCode( + request_headers.get(Filters::Common::Fault::HeaderNames::get().AbortRequest)); + if (!http_status.has_value()) { return absl::nullopt; } - auto status_code = static_cast(config_abort.value()); - auto code = static_cast(config_->runtime().snapshot().getInteger( - fault_settings_->abortHttpStatusRuntime(), status_code)); + auto default_http_status_code = static_cast(http_status.value()); + auto runtime_http_status_code = static_cast(config_->runtime().snapshot().getInteger( + fault_settings_->abortHttpStatusRuntime(), default_http_status_code)); if (!downstream_cluster_abort_http_status_key_.empty()) { - code = static_cast(config_->runtime().snapshot().getInteger( - downstream_cluster_abort_http_status_key_, status_code)); + runtime_http_status_code = static_cast(config_->runtime().snapshot().getInteger( + downstream_cluster_abort_http_status_key_, default_http_status_code)); } - return code; + return runtime_http_status_code; +} + +absl::optional +FaultFilter::abortGrpcStatus(const Http::RequestHeaderMap& request_headers) { + auto grpc_status = fault_settings_->requestAbort()->grpcStatusCode( + request_headers.get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest)); + + auto default_grpc_status_code = grpc_status.has_value() + ? static_cast(grpc_status.value()) + : std::numeric_limits::max(); + + auto runtime_grpc_status_code = config_->runtime().snapshot().getInteger( + fault_settings_->abortGrpcStatusRuntime(), default_grpc_status_code); + + if (runtime_grpc_status_code == std::numeric_limits::max()) { + return absl::nullopt; + } + + return static_cast(runtime_grpc_status_code); } void FaultFilter::recordDelaysInjectedStats() { @@ -375,19 +408,23 @@ void FaultFilter::postDelayInjection(const Http::RequestHeaderMap& request_heade resetTimerState(); // Delays can be followed by aborts - const auto abort_code = abortHttpStatus(request_headers); - if (abort_code.has_value()) { - abortWithHTTPStatus(abort_code.value()); + absl::optional http_status; + absl::optional grpc_status; + std::tie(http_status, grpc_status) = abortStatus(request_headers); + + if (http_status.has_value()) { + abortWithStatus(http_status.value(), grpc_status); } else { // Continue request processing. decoder_callbacks_->continueDecoding(); } } -void FaultFilter::abortWithHTTPStatus(Http::Code abort_code) { +void FaultFilter::abortWithStatus(Http::Code http_status_code, + absl::optional grpc_status_code) { decoder_callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::FaultInjected); - decoder_callbacks_->sendLocalReply(abort_code, "fault filter abort", nullptr, absl::nullopt, - RcDetails::get().FaultAbort); + decoder_callbacks_->sendLocalReply(http_status_code, "fault filter abort", nullptr, + grpc_status_code, RcDetails::get().FaultAbort); recordAbortsInjectedStats(); } diff --git a/source/extensions/filters/http/fault/fault_filter.h b/source/extensions/filters/http/fault/fault_filter.h index bdbcbd9752821..ed06313fa3adf 100644 --- a/source/extensions/filters/http/fault/fault_filter.h +++ b/source/extensions/filters/http/fault/fault_filter.h @@ -67,6 +67,7 @@ class FaultSettings : public Router::RouteSpecificFilterConfig { const std::string& abortPercentRuntime() const { return abort_percent_runtime_; } const std::string& delayPercentRuntime() const { return delay_percent_runtime_; } const std::string& abortHttpStatusRuntime() const { return abort_http_status_runtime_; } + const std::string& abortGrpcStatusRuntime() const { return abort_grpc_status_runtime_; } const std::string& delayDurationRuntime() const { return delay_duration_runtime_; } const std::string& maxActiveFaultsRuntime() const { return max_active_faults_runtime_; } const std::string& responseRateLimitPercentRuntime() const { @@ -80,6 +81,7 @@ class FaultSettings : public Router::RouteSpecificFilterConfig { const std::string AbortPercentKey = "fault.http.abort.abort_percent"; const std::string DelayDurationKey = "fault.http.delay.fixed_duration_ms"; const std::string AbortHttpStatusKey = "fault.http.abort.http_status"; + const std::string AbortGrpcStatusKey = "fault.http.abort.grpc_status"; const std::string MaxActiveFaultsKey = "fault.http.max_active_faults"; const std::string ResponseRateLimitPercentKey = "fault.http.rate_limit.response_percent"; }; @@ -98,6 +100,7 @@ class FaultSettings : public Router::RouteSpecificFilterConfig { const std::string abort_percent_runtime_; const std::string delay_duration_runtime_; const std::string abort_http_status_runtime_; + const std::string abort_grpc_status_runtime_; const std::string max_active_faults_runtime_; const std::string response_rate_limit_percent_runtime_; }; @@ -203,6 +206,8 @@ class StreamRateLimiter : Logger::Loggable { Buffer::WatermarkBuffer buffer_; }; +using AbortHttpAndGrpcStatus = + std::pair, absl::optional>; /** * A filter that is capable of faulting an entire request before dispatching it upstream. */ @@ -245,7 +250,8 @@ class FaultFilter : public Http::StreamFilter, Logger::Loggable grpc_status_code); bool matchesTargetUpstreamCluster(); bool matchesDownstreamNodes(const Http::RequestHeaderMap& headers); bool isAbortEnabled(const Http::RequestHeaderMap& request_headers); @@ -253,7 +259,10 @@ class FaultFilter : public Http::StreamFilter, Logger::Loggable delayDuration(const Http::RequestHeaderMap& request_headers); + AbortHttpAndGrpcStatus abortStatus(const Http::RequestHeaderMap& request_headers); absl::optional abortHttpStatus(const Http::RequestHeaderMap& request_headers); + absl::optional + abortGrpcStatus(const Http::RequestHeaderMap& request_headers); void maybeIncActiveFaults(); void maybeSetupResponseRateLimit(const Http::RequestHeaderMap& request_headers); diff --git a/test/extensions/filters/common/fault/fault_config_test.cc b/test/extensions/filters/common/fault/fault_config_test.cc index 340cd2c3dd70b..a82d29e2865b6 100644 --- a/test/extensions/filters/common/fault/fault_config_test.cc +++ b/test/extensions/filters/common/fault/fault_config_test.cc @@ -18,21 +18,56 @@ TEST(FaultConfigTest, FaultAbortHeaderConfig) { proto_config.mutable_header_abort(); FaultAbortConfig config(proto_config); + // No header. + EXPECT_EQ(absl::nullopt, config.httpStatusCode(nullptr)); + + // Header with bad data. + Http::TestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "abc"}}; + EXPECT_EQ(absl::nullopt, config.httpStatusCode(bad_headers.get(HeaderNames::get().AbortRequest))); + + // Out of range header - value too low. + Http::TestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request", "199"}}; + EXPECT_EQ(absl::nullopt, + config.httpStatusCode(too_low_headers.get(HeaderNames::get().AbortRequest))); + + // Out of range header - value too high. + Http::TestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-request", "600"}}; + EXPECT_EQ(absl::nullopt, + config.httpStatusCode(too_high_headers.get(HeaderNames::get().AbortRequest))); + + // Valid header. + Http::TestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request", "401"}}; + EXPECT_EQ(Http::Code::Unauthorized, + config.httpStatusCode(good_headers.get(HeaderNames::get().AbortRequest)).value()); +} + +TEST(FaultConfigTest, FaultAbortGrpcHeaderConfig) { + envoy::extensions::filters::http::fault::v3::FaultAbort proto_config; + proto_config.mutable_header_abort(); + FaultAbortConfig config(proto_config); + + // No header. + EXPECT_EQ(absl::nullopt, config.grpcStatusCode(nullptr)); + // Header with bad data. - Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "abc"}}; - EXPECT_EQ(absl::nullopt, config.statusCode(&bad_headers)); + Http::TestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-grpc-request", "abc"}}; + EXPECT_EQ(absl::nullopt, + config.grpcStatusCode(bad_headers.get(HeaderNames::get().AbortGrpcRequest))); // Out of range header - value too low. - Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request", "199"}}; - EXPECT_EQ(absl::nullopt, config.statusCode(&too_low_headers)); + Http::TestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-grpc-request", "-1"}}; + EXPECT_EQ(absl::nullopt, + config.grpcStatusCode(too_low_headers.get(HeaderNames::get().AbortGrpcRequest))); // Out of range header - value too high. - Http::TestRequestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-request", "600"}}; - EXPECT_EQ(absl::nullopt, config.statusCode(&too_high_headers)); + Http::TestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-grpc-request", "17"}}; + EXPECT_EQ(absl::nullopt, + config.grpcStatusCode(too_high_headers.get(HeaderNames::get().AbortGrpcRequest))); // Valid header. - Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request", "401"}}; - EXPECT_EQ(Http::Code::Unauthorized, config.statusCode(&good_headers).value()); + Http::TestHeaderMapImpl good_headers{{"x-envoy-fault-abort-grpc-request", "5"}}; + EXPECT_EQ(Grpc::Status::NotFound, + config.grpcStatusCode(good_headers.get(HeaderNames::get().AbortGrpcRequest)).value()); } TEST(FaultConfigTest, FaultAbortPercentageHeaderConfig) { diff --git a/test/extensions/filters/http/fault/fault_filter_integration_test.cc b/test/extensions/filters/http/fault/fault_filter_integration_test.cc index 6059287a66c72..9ce0b1fb9eea2 100644 --- a/test/extensions/filters/http/fault/fault_filter_integration_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_integration_test.cc @@ -46,6 +46,17 @@ name: fault percentage: numerator: 100 )EOF"; + + const std::string abort_grpc_fault_config_ = + R"EOF( +name: fault +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault + abort: + percentage: + numerator: 100 + grpc_status: 5 +)EOF"; }; // Fault integration tests that should run with all protocols, useful for testing various @@ -214,6 +225,61 @@ TEST_P(FaultIntegrationTestAllProtocols, HeaderFaultConfigNoHeaders) { EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.response_rl_injected")->value()); } +// Request abort with grpc status, controlled via header configuration. +TEST_P(FaultIntegrationTestAllProtocols, HeaderFaultAbortGrpcConfig) { + initializeFilter(header_fault_config_); + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + + auto response = codec_client_->makeHeaderOnlyRequest( + Http::TestRequestHeaderMapImpl{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-envoy-fault-abort-grpc-request", "5"}, + {"content-type", "application/grpc"}}); + response->waitForEndStream(); + + EXPECT_TRUE(response->complete()); + EXPECT_THAT(response->headers(), Envoy::Http::HttpStatusIs("200")); + EXPECT_THAT(response->headers(), + HeaderValueOf(Http::Headers::get().ContentType, "application/grpc")); + EXPECT_THAT(response->headers(), HeaderValueOf(Http::Headers::get().GrpcStatus, "5")); + EXPECT_THAT(response->headers(), + HeaderValueOf(Http::Headers::get().GrpcMessage, "fault filter abort")); + EXPECT_EQ(nullptr, response->trailers()); + + EXPECT_EQ(1UL, test_server_->counter("http.config_test.fault.aborts_injected")->value()); + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.delays_injected")->value()); + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.response_rl_injected")->value()); +} + +// Request abort with grpc status, controlled via configuration. +TEST_P(FaultIntegrationTestAllProtocols, FaultAbortGrpcConfig) { + initializeFilter(abort_grpc_fault_config_); + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + + auto response = codec_client_->makeHeaderOnlyRequest( + Http::TestRequestHeaderMapImpl{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"content-type", "application/grpc"}}); + response->waitForEndStream(); + + EXPECT_TRUE(response->complete()); + EXPECT_THAT(response->headers(), Envoy::Http::HttpStatusIs("200")); + EXPECT_THAT(response->headers(), + HeaderValueOf(Http::Headers::get().ContentType, "application/grpc")); + EXPECT_THAT(response->headers(), HeaderValueOf(Http::Headers::get().GrpcStatus, "5")); + EXPECT_THAT(response->headers(), + HeaderValueOf(Http::Headers::get().GrpcMessage, "fault filter abort")); + EXPECT_EQ(nullptr, response->trailers()); + + EXPECT_EQ(1UL, test_server_->counter("http.config_test.fault.aborts_injected")->value()); + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.delays_injected")->value()); + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.response_rl_injected")->value()); +} + // Fault integration tests that run with HTTP/2 only, used for fully testing trailers. class FaultIntegrationTestHttp2 : public FaultIntegrationTest {}; INSTANTIATE_TEST_SUITE_P(Protocols, FaultIntegrationTestHttp2, diff --git a/test/extensions/filters/http/fault/fault_filter_test.cc b/test/extensions/filters/http/fault/fault_filter_test.cc index ce783bdca8e53..b0a2f0532597a 100644 --- a/test/extensions/filters/http/fault/fault_filter_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_test.cc @@ -255,6 +255,10 @@ TEST_F(FaultFilterTest, AbortWithHttpStatus) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 429)) .WillOnce(Return(429)); + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + Http::TestResponseHeaderMapImpl response_headers{ {":status", "429"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -302,6 +306,10 @@ TEST_F(FaultFilterTest, HeaderAbortWithHttpStatus) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 429)) .WillOnce(Return(429)); + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + Http::TestResponseHeaderMapImpl response_headers{ {":status", "429"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -326,6 +334,109 @@ TEST_F(FaultFilterTest, HeaderAbortWithHttpStatus) { EXPECT_EQ("fault_filter_abort", decoder_filter_callbacks_.details_); } +TEST_F(FaultFilterTest, AbortWithGrpcStatus) { + decoder_filter_callbacks_.is_grpc_request_ = true; + + envoy::extensions::filters::http::fault::v3::HTTPFault fault; + fault.mutable_abort()->mutable_percentage()->set_numerator(100); + fault.mutable_abort()->mutable_percentage()->set_denominator( + envoy::type::v3::FractionalPercent::HUNDRED); + fault.mutable_abort()->set_grpc_status(5); + SetUpTest(fault); + + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.max_active_faults", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + + EXPECT_CALL(decoder_filter_callbacks_, continueDecoding()).Times(0); + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::DelayInjected)) + .Times(0); + + // Abort related calls + EXPECT_CALL(runtime_.snapshot_, + featureEnabled("fault.http.abort.abort_percent", + Matcher(Percent(100)))) + .WillOnce(Return(true)); + + EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.grpc_status", 5)) + .WillOnce(Return(5)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}, + {"content-type", "application/grpc"}, + {"grpc-status", "5"}, + {"grpc-message", "fault filter abort"}}; + EXPECT_CALL(decoder_filter_callbacks_, + encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::FaultInjected)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + Http::MetadataMap metadata_map{{"metadata", "metadata"}}; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); + EXPECT_EQ(1UL, config_->stats().active_faults_.value()); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); + filter_->onDestroy(); + + EXPECT_EQ(0UL, config_->stats().delays_injected_.value()); + EXPECT_EQ(1UL, config_->stats().aborts_injected_.value()); + EXPECT_EQ(0UL, config_->stats().active_faults_.value()); + EXPECT_EQ("fault_filter_abort", decoder_filter_callbacks_.details_); +} + +TEST_F(FaultFilterTest, HeaderAbortWithGrpcStatus) { + decoder_filter_callbacks_.is_grpc_request_ = true; + SetUpTest(header_abort_only_yaml); + + request_headers_.addCopy("x-envoy-fault-abort-grpc-request", "5"); + + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.max_active_faults", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + + EXPECT_CALL(decoder_filter_callbacks_, continueDecoding()).Times(0); + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::DelayInjected)) + .Times(0); + + // Abort related calls + EXPECT_CALL(runtime_.snapshot_, + featureEnabled("fault.http.abort.abort_percent", + Matcher(Percent(100)))) + .WillOnce(Return(true)); + + EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.grpc_status", 5)) + .WillOnce(Return(5)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}, + {"content-type", "application/grpc"}, + {"grpc-status", "5"}, + {"grpc-message", "fault filter abort"}}; + + EXPECT_CALL(decoder_filter_callbacks_, + encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); + + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::FaultInjected)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + Http::MetadataMap metadata_map{{"metadata", "metadata"}}; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); + EXPECT_EQ(1UL, config_->stats().active_faults_.value()); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); + filter_->onDestroy(); + + EXPECT_EQ(0UL, config_->stats().delays_injected_.value()); + EXPECT_EQ(1UL, config_->stats().aborts_injected_.value()); + EXPECT_EQ(0UL, config_->stats().active_faults_.value()); + EXPECT_EQ("fault_filter_abort", decoder_filter_callbacks_.details_); +} + TEST_F(FaultFilterTest, FixedDelayZeroDuration) { SetUpTest(fixed_delay_only_yaml); @@ -504,6 +615,10 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortDownstream) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.cluster.abort.http_status", 503)) .WillOnce(Return(500)); + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + Http::TestResponseHeaderMapImpl response_headers{ {":status", "500"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -562,6 +677,10 @@ TEST_F(FaultFilterTest, FixedDelayAndAbort) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 503)) .WillOnce(Return(503)); + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + Http::TestResponseHeaderMapImpl response_headers{ {":status", "503"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -614,6 +733,10 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortDownstreamNodes) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 503)) .WillOnce(Return(503)); + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + Http::TestResponseHeaderMapImpl response_headers{ {":status", "503"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -680,6 +803,10 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortHeaderMatchSuccess) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 503)) .WillOnce(Return(503)); + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + Http::TestResponseHeaderMapImpl response_headers{ {":status", "503"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -1096,6 +1223,7 @@ TEST_F(FaultFilterSettingsTest, CheckDefaultRuntimeKeys) { EXPECT_EQ("fault.http.abort.abort_percent", settings.abortPercentRuntime()); EXPECT_EQ("fault.http.delay.fixed_duration_ms", settings.delayDurationRuntime()); EXPECT_EQ("fault.http.abort.http_status", settings.abortHttpStatusRuntime()); + EXPECT_EQ("fault.http.abort.grpc_status", settings.abortGrpcStatusRuntime()); EXPECT_EQ("fault.http.max_active_faults", settings.maxActiveFaultsRuntime()); EXPECT_EQ("fault.http.rate_limit.response_percent", settings.responseRateLimitPercentRuntime()); } @@ -1105,6 +1233,7 @@ TEST_F(FaultFilterSettingsTest, CheckOverrideRuntimeKeys) { fault.set_abort_percent_runtime(std::string("fault.abort_percent_runtime")); fault.set_delay_percent_runtime(std::string("fault.delay_percent_runtime")); fault.set_abort_http_status_runtime(std::string("fault.abort_http_status_runtime")); + fault.set_abort_grpc_status_runtime(std::string("fault.abort_grpc_status_runtime")); fault.set_delay_duration_runtime(std::string("fault.delay_duration_runtime")); fault.set_max_active_faults_runtime(std::string("fault.max_active_faults_runtime")); fault.set_response_rate_limit_percent_runtime( @@ -1116,6 +1245,7 @@ TEST_F(FaultFilterSettingsTest, CheckOverrideRuntimeKeys) { EXPECT_EQ("fault.abort_percent_runtime", settings.abortPercentRuntime()); EXPECT_EQ("fault.delay_duration_runtime", settings.delayDurationRuntime()); EXPECT_EQ("fault.abort_http_status_runtime", settings.abortHttpStatusRuntime()); + EXPECT_EQ("fault.abort_grpc_status_runtime", settings.abortGrpcStatusRuntime()); EXPECT_EQ("fault.max_active_faults_runtime", settings.maxActiveFaultsRuntime()); EXPECT_EQ("fault.response_rate_limit_percent_runtime", settings.responseRateLimitPercentRuntime()); From 0b369ea96c626ab28aa641afaec34bd342eeec07 Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Sun, 19 Apr 2020 00:23:43 -0700 Subject: [PATCH 2/8] Add header info Signed-off-by: Vijay Rajput --- docs/root/configuration/http/http_filters/fault_filter.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/root/configuration/http/http_filters/fault_filter.rst b/docs/root/configuration/http/http_filters/fault_filter.rst index cec46954dbd78..dc499fb552639 100644 --- a/docs/root/configuration/http/http_filters/fault_filter.rst +++ b/docs/root/configuration/http/http_filters/fault_filter.rst @@ -42,6 +42,12 @@ x-envoy-fault-abort-request In order for the header to work, :ref:`header_abort ` needs to be set. +x-envoy-fault-abort-grpc-request + gRPC status code to abort a request with. The header value should be an integer that specifies + the gRPC status code to return in response to a request and must be in the range [0, 16]. + When this header is set, the HTTP response status code will be set to 200. In order for the header to work, + :ref:`header_abort ` needs to be set. + x-envoy-fault-abort-request-percentage The percentage of requests that should be failed with a status code that's defined by the value of *x-envoy-fault-abort-request* HTTP header. The header value should be an integer From 160a6faf2648fd15e9e85846e62be30f42404f80 Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Wed, 22 Apr 2020 09:23:27 -0700 Subject: [PATCH 3/8] Allow setting not well-defined gRPC codes and address review feedback. Signed-off-by: Vijay Rajput --- .../filters/http/fault/v3/fault.proto | 2 +- .../http/http_filters/fault_filter.rst | 9 +++-- .../filters/http/fault/v3/fault.proto | 2 +- .../filters/common/fault/fault_config.cc | 29 +++++++++----- .../filters/common/fault/fault_config.h | 20 +++++----- .../filters/http/fault/fault_filter.cc | 14 +++---- .../filters/common/fault/fault_config_test.cc | 40 ++++++++++--------- 7 files changed, 63 insertions(+), 53 deletions(-) diff --git a/api/envoy/extensions/filters/http/fault/v3/fault.proto b/api/envoy/extensions/filters/http/fault/v3/fault.proto index bc8bc24eaec4f..d28ed28b11100 100644 --- a/api/envoy/extensions/filters/http/fault/v3/fault.proto +++ b/api/envoy/extensions/filters/http/fault/v3/fault.proto @@ -43,7 +43,7 @@ message FaultAbort { uint32 http_status = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}]; // gRPC status code to use to abort the gRPC request. - uint32 grpc_status = 5 [(validate.rules).uint32 = {lte: 16}]; + uint32 grpc_status = 5; // Fault aborts are controlled via an HTTP header (if applicable). HeaderAbort header_abort = 4; diff --git a/docs/root/configuration/http/http_filters/fault_filter.rst b/docs/root/configuration/http/http_filters/fault_filter.rst index dc499fb552639..fc3fe4739c2c5 100644 --- a/docs/root/configuration/http/http_filters/fault_filter.rst +++ b/docs/root/configuration/http/http_filters/fault_filter.rst @@ -43,10 +43,11 @@ x-envoy-fault-abort-request ` needs to be set. x-envoy-fault-abort-grpc-request - gRPC status code to abort a request with. The header value should be an integer that specifies - the gRPC status code to return in response to a request and must be in the range [0, 16]. - When this header is set, the HTTP response status code will be set to 200. In order for the header to work, - :ref:`header_abort ` needs to be set. + gRPC status code to abort a request with. The header value should be a non-negative integer that specifies + the gRPC status code to return in response to a request. Its value range is [0, UInt32.Max] instead of [0, 16] + to allow testing even not well-defined gRPC status codes. When this header is set, the HTTP response status code + will be set to 200. In order for the header to work, :ref:`header_abort + ` needs to be set. x-envoy-fault-abort-request-percentage The percentage of requests that should be failed with a status code that's defined diff --git a/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto b/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto index a05f840eacc73..9bba2f134cdf2 100644 --- a/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto +++ b/generated_api_shadow/envoy/extensions/filters/http/fault/v3/fault.proto @@ -46,7 +46,7 @@ message FaultAbort { uint32 http_status = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}]; // Fault aborts are controlled via an HTTP header (if applicable). - uint32 grpc_status = 5 [(validate.rules).uint32 = {lte: 16}]; + uint32 grpc_status = 5; // The percentage of requests/operations/connections that will be aborted with the error code // provided. diff --git a/source/extensions/filters/common/fault/fault_config.cc b/source/extensions/filters/common/fault/fault_config.cc index 7054a899ce0d1..25aa86a0463fc 100644 --- a/source/extensions/filters/common/fault/fault_config.cc +++ b/source/extensions/filters/common/fault/fault_config.cc @@ -13,6 +13,10 @@ namespace Fault { envoy::type::v3::FractionalPercent HeaderPercentageProvider::percentage(const Http::RequestHeaderMap* request_headers) const { + if (request_headers == nullptr) { + return percentage_; + } + const auto header = request_headers->get(header_name_); if (header == nullptr) { return percentage_; @@ -52,8 +56,13 @@ FaultAbortConfig::FaultAbortConfig( } absl::optional -FaultAbortConfig::HeaderAbortProvider::httpStatusCode(const Http::HeaderEntry* header) const { +FaultAbortConfig::HeaderAbortProvider::httpStatusCode(const Http::RequestHeaderMap* request_headers) const { + if(request_headers == nullptr) { + return absl::nullopt; + } + absl::optional ret = absl::nullopt; + auto header = request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortRequest); if (header == nullptr) { return ret; } @@ -71,22 +80,22 @@ FaultAbortConfig::HeaderAbortProvider::httpStatusCode(const Http::HeaderEntry* h } absl::optional -FaultAbortConfig::HeaderAbortProvider::grpcStatusCode(const Http::HeaderEntry* header) const { - absl::optional ret = absl::nullopt; +FaultAbortConfig::HeaderAbortProvider::grpcStatusCode(const Http::RequestHeaderMap* request_headers) const { + if(request_headers == nullptr) { + return absl::nullopt; + } + + auto header = request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest); if (header == nullptr) { - return ret; + return absl::nullopt; } uint64_t code; if (!absl::SimpleAtoi(header->value().getStringView(), &code)) { - return ret; - } - - if (code <= 16) { - ret = static_cast(code); + return absl::nullopt; } - return ret; + return static_cast(code); } FaultDelayConfig::FaultDelayConfig( diff --git a/source/extensions/filters/common/fault/fault_config.h b/source/extensions/filters/common/fault/fault_config.h index 3bf512a852f18..78695ffb2ff03 100644 --- a/source/extensions/filters/common/fault/fault_config.h +++ b/source/extensions/filters/common/fault/fault_config.h @@ -55,11 +55,11 @@ class FaultAbortConfig { public: FaultAbortConfig(const envoy::extensions::filters::http::fault::v3::FaultAbort& abort_config); - absl::optional httpStatusCode(const Http::HeaderEntry* header) const { - return provider_->httpStatusCode(header); + absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const { + return provider_->httpStatusCode(request_headers); } - absl::optional grpcStatusCode(const Http::HeaderEntry* header) const { - return provider_->grpcStatusCode(header); + absl::optional grpcStatusCode(const Http::RequestHeaderMap* request_headers) const { + return provider_->grpcStatusCode(request_headers); } envoy::type::v3::FractionalPercent @@ -75,12 +75,12 @@ class FaultAbortConfig { // Return the HTTP status code to use. Optionally passed HTTP headers that may contain the // HTTP status code depending on the provider implementation. - virtual absl::optional httpStatusCode(const Http::HeaderEntry* header) const PURE; + virtual absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const PURE; // Return the gRPC status code to use. Optionally passed an HTTP header that may contain the // gRPC status code depending on the provider implementation. virtual absl::optional - grpcStatusCode(const Http::HeaderEntry* header) const PURE; + grpcStatusCode(const Http::RequestHeaderMap* request_headers) const PURE; // Return what percentage of requests abort faults should be applied to. Optionally passed // HTTP headers that may contain the percentage depending on the provider implementation. @@ -97,12 +97,12 @@ class FaultAbortConfig { : http_status_code_(http_status_code), grpc_status_code_(grpc_status_code), percentage_(percentage) {} - absl::optional httpStatusCode(const Http::HeaderEntry*) const override { + absl::optional httpStatusCode(const Http::RequestHeaderMap*) const override { return http_status_code_; } absl::optional - grpcStatusCode(const Http::HeaderEntry*) const override { + grpcStatusCode(const Http::RequestHeaderMap*) const override { return grpc_status_code_; } @@ -122,10 +122,10 @@ class FaultAbortConfig { HeaderAbortProvider(const envoy::type::v3::FractionalPercent& percentage) : HeaderPercentageProvider(HeaderNames::get().AbortRequestPercentage, percentage) {} - absl::optional httpStatusCode(const Http::HeaderEntry* header) const override; + absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const override; absl::optional - grpcStatusCode(const Http::HeaderEntry*) const override; + grpcStatusCode(const Http::RequestHeaderMap* request_headers) const override; envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const override { diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index c895337c09339..173b3f83072ef 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -291,22 +291,21 @@ FaultFilter::delayDuration(const Http::RequestHeaderMap& request_headers) { AbortHttpAndGrpcStatus FaultFilter::abortStatus(const Http::RequestHeaderMap& request_headers) { if (!isAbortEnabled(request_headers)) { - return std::make_pair(absl::nullopt, absl::nullopt); + return AbortHttpAndGrpcStatus(absl::nullopt, absl::nullopt); } auto grpc_status = abortGrpcStatus(request_headers); - // If gRPC status code is set, then HTTP will be set to 200 - return grpc_status.has_value() ? std::make_pair(Http::Code::OK, grpc_status) - : std::make_pair(abortHttpStatus(request_headers), grpc_status); + // If gRPC status code is set, then HTTP will be set to Http::Code::OK (200). + return grpc_status.has_value() ? AbortHttpAndGrpcStatus(Http::Code::OK, grpc_status) + : AbortHttpAndGrpcStatus(abortHttpStatus(request_headers), absl::nullopt); } absl::optional FaultFilter::abortHttpStatus(const Http::RequestHeaderMap& request_headers) { // See if the configured abort provider has a default status code, if not there is no abort status // code (e.g., header configuration and no/invalid header). - auto http_status = fault_settings_->requestAbort()->httpStatusCode( - request_headers.get(Filters::Common::Fault::HeaderNames::get().AbortRequest)); + auto http_status = fault_settings_->requestAbort()->httpStatusCode(&request_headers); if (!http_status.has_value()) { return absl::nullopt; } @@ -325,8 +324,7 @@ FaultFilter::abortHttpStatus(const Http::RequestHeaderMap& request_headers) { absl::optional FaultFilter::abortGrpcStatus(const Http::RequestHeaderMap& request_headers) { - auto grpc_status = fault_settings_->requestAbort()->grpcStatusCode( - request_headers.get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest)); + auto grpc_status = fault_settings_->requestAbort()->grpcStatusCode(&request_headers); auto default_grpc_status_code = grpc_status.has_value() ? static_cast(grpc_status.value()) diff --git a/test/extensions/filters/common/fault/fault_config_test.cc b/test/extensions/filters/common/fault/fault_config_test.cc index a82d29e2865b6..73c84c8e666ca 100644 --- a/test/extensions/filters/common/fault/fault_config_test.cc +++ b/test/extensions/filters/common/fault/fault_config_test.cc @@ -22,23 +22,23 @@ TEST(FaultConfigTest, FaultAbortHeaderConfig) { EXPECT_EQ(absl::nullopt, config.httpStatusCode(nullptr)); // Header with bad data. - Http::TestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "abc"}}; - EXPECT_EQ(absl::nullopt, config.httpStatusCode(bad_headers.get(HeaderNames::get().AbortRequest))); + Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "abc"}}; + EXPECT_EQ(absl::nullopt, config.httpStatusCode(&bad_headers)); // Out of range header - value too low. - Http::TestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request", "199"}}; + Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request", "199"}}; EXPECT_EQ(absl::nullopt, - config.httpStatusCode(too_low_headers.get(HeaderNames::get().AbortRequest))); + config.httpStatusCode(&too_low_headers)); // Out of range header - value too high. - Http::TestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-request", "600"}}; + Http::TestRequestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-request", "600"}}; EXPECT_EQ(absl::nullopt, - config.httpStatusCode(too_high_headers.get(HeaderNames::get().AbortRequest))); + config.httpStatusCode(&too_high_headers)); // Valid header. - Http::TestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request", "401"}}; + Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request", "401"}}; EXPECT_EQ(Http::Code::Unauthorized, - config.httpStatusCode(good_headers.get(HeaderNames::get().AbortRequest)).value()); + config.httpStatusCode(&good_headers)); } TEST(FaultConfigTest, FaultAbortGrpcHeaderConfig) { @@ -50,24 +50,23 @@ TEST(FaultConfigTest, FaultAbortGrpcHeaderConfig) { EXPECT_EQ(absl::nullopt, config.grpcStatusCode(nullptr)); // Header with bad data. - Http::TestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-grpc-request", "abc"}}; + Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-grpc-request", "abc"}}; EXPECT_EQ(absl::nullopt, - config.grpcStatusCode(bad_headers.get(HeaderNames::get().AbortGrpcRequest))); + config.grpcStatusCode(&bad_headers)); // Out of range header - value too low. - Http::TestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-grpc-request", "-1"}}; + Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-grpc-request", "-1"}}; EXPECT_EQ(absl::nullopt, - config.grpcStatusCode(too_low_headers.get(HeaderNames::get().AbortGrpcRequest))); - - // Out of range header - value too high. - Http::TestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-grpc-request", "17"}}; - EXPECT_EQ(absl::nullopt, - config.grpcStatusCode(too_high_headers.get(HeaderNames::get().AbortGrpcRequest))); + config.grpcStatusCode(&too_low_headers)); // Valid header. - Http::TestHeaderMapImpl good_headers{{"x-envoy-fault-abort-grpc-request", "5"}}; + Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-grpc-request", "5"}}; EXPECT_EQ(Grpc::Status::NotFound, - config.grpcStatusCode(good_headers.get(HeaderNames::get().AbortGrpcRequest)).value()); + config.grpcStatusCode(&good_headers)); + + // Valid header - unknown gRPC code (>0). + Http::TestRequestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-grpc-request", "100"}}; + EXPECT_EQ(100, config.grpcStatusCode(&too_high_headers)); } TEST(FaultConfigTest, FaultAbortPercentageHeaderConfig) { @@ -77,6 +76,9 @@ TEST(FaultConfigTest, FaultAbortPercentageHeaderConfig) { proto_config.mutable_percentage()->set_denominator(envoy::type::v3::FractionalPercent::HUNDRED); FaultAbortConfig config(proto_config); + // No header. + EXPECT_EQ(proto_config.percentage().numerator(), config.percentage(nullptr).numerator()); + // Header with bad data - fallback to proto config. Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request-percentage", "abc"}}; const auto bad_headers_percentage = config.percentage(&bad_headers); From 535fa006871f90d1d6051ed5a9124277c698e485 Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Wed, 22 Apr 2020 21:50:05 -0700 Subject: [PATCH 4/8] support grpc fault requests percentage via header Signed-off-by: Vijay Rajput --- .../http/http_filters/fault_filter.rst | 20 ++++-- .../filters/common/fault/fault_config.cc | 24 +++++-- .../filters/common/fault/fault_config.h | 44 ++++++++---- .../filters/http/fault/fault_filter.cc | 30 ++++---- .../filters/http/fault/fault_filter.h | 1 + .../filters/common/fault/fault_config_test.cc | 69 +++++++++++++++---- .../fault/fault_filter_integration_test.cc | 22 ++++++ .../filters/http/fault/fault_filter_test.cc | 24 ------- 8 files changed, 160 insertions(+), 74 deletions(-) diff --git a/docs/root/configuration/http/http_filters/fault_filter.rst b/docs/root/configuration/http/http_filters/fault_filter.rst index fc3fe4739c2c5..9fe2484469812 100644 --- a/docs/root/configuration/http/http_filters/fault_filter.rst +++ b/docs/root/configuration/http/http_filters/fault_filter.rst @@ -42,6 +42,18 @@ x-envoy-fault-abort-request In order for the header to work, :ref:`header_abort ` needs to be set. +x-envoy-fault-abort-request-percentage + The percentage of requests that should be failed with a status code that's defined + by the value of *x-envoy-fault-abort-request* HTTP header. The header value should be an integer + that specifies the numerator of the percentage of request to apply aborts to and must be greater + or equal to 0 and its maximum value is capped by the value of the numerator of + :ref:`percentage ` field. + Percentage's denominator is equal to default percentage's denominator + :ref:`percentage ` field. + In order for the header to work, :ref:`header_abort + ` needs to be set and + *x-envoy-fault-abort-request* HTTP header needs to be a part of a request. + x-envoy-fault-abort-grpc-request gRPC status code to abort a request with. The header value should be a non-negative integer that specifies the gRPC status code to return in response to a request. Its value range is [0, UInt32.Max] instead of [0, 16] @@ -49,9 +61,9 @@ x-envoy-fault-abort-grpc-request will be set to 200. In order for the header to work, :ref:`header_abort ` needs to be set. -x-envoy-fault-abort-request-percentage - The percentage of requests that should be failed with a status code that's defined - by the value of *x-envoy-fault-abort-request* HTTP header. The header value should be an integer +x-envoy-fault-abort-grpc-request-percentage + The percentage of gRPC requests that should be failed with a status code that's defined + by the value of *x-envoy-fault-abort-grpc-request* HTTP header. The header value should be an integer that specifies the numerator of the percentage of request to apply aborts to and must be greater or equal to 0 and its maximum value is capped by the value of the numerator of :ref:`percentage ` field. @@ -59,7 +71,7 @@ x-envoy-fault-abort-request-percentage :ref:`percentage ` field. In order for the header to work, :ref:`header_abort ` needs to be set and - *x-envoy-fault-abort-request* HTTP header needs to be a part of a request. + *x-envoy-fault-abort-grpc-request* HTTP header needs to be a part of a request. x-envoy-fault-delay-request The duration to delay a request by. The header value should be an integer that specifies the number diff --git a/source/extensions/filters/common/fault/fault_config.cc b/source/extensions/filters/common/fault/fault_config.cc index 25aa86a0463fc..ac18d2c0637ec 100644 --- a/source/extensions/filters/common/fault/fault_config.cc +++ b/source/extensions/filters/common/fault/fault_config.cc @@ -55,9 +55,9 @@ FaultAbortConfig::FaultAbortConfig( } } -absl::optional -FaultAbortConfig::HeaderAbortProvider::httpStatusCode(const Http::RequestHeaderMap* request_headers) const { - if(request_headers == nullptr) { +absl::optional FaultAbortConfig::HeaderAbortProvider::httpStatusCode( + const Http::RequestHeaderMap* request_headers) const { + if (request_headers == nullptr) { return absl::nullopt; } @@ -79,9 +79,9 @@ FaultAbortConfig::HeaderAbortProvider::httpStatusCode(const Http::RequestHeaderM return ret; } -absl::optional -FaultAbortConfig::HeaderAbortProvider::grpcStatusCode(const Http::RequestHeaderMap* request_headers) const { - if(request_headers == nullptr) { +absl::optional FaultAbortConfig::HeaderAbortProvider::grpcStatusCode( + const Http::RequestHeaderMap* request_headers) const { + if (request_headers == nullptr) { return absl::nullopt; } @@ -98,6 +98,18 @@ FaultAbortConfig::HeaderAbortProvider::grpcStatusCode(const Http::RequestHeaderM return static_cast(code); } +envoy::type::v3::FractionalPercent FaultAbortConfig::HeaderAbortProvider::percentage( + const Http::RequestHeaderMap* request_headers) const { + // if the abort fault is for grpc status, then use grpc fault request percentage. + if (request_headers != nullptr && + request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest) != + nullptr) { + return grpcHeaderPercentageProvider_.percentage(request_headers); + } + + return httpHeaderPercentageProvider_.percentage(request_headers); +} + FaultDelayConfig::FaultDelayConfig( const envoy::extensions::filters::common::fault::v3::FaultDelay& delay_config) { switch (delay_config.fault_delay_secifier_case()) { diff --git a/source/extensions/filters/common/fault/fault_config.h b/source/extensions/filters/common/fault/fault_config.h index 78695ffb2ff03..bbe5510f8c870 100644 --- a/source/extensions/filters/common/fault/fault_config.h +++ b/source/extensions/filters/common/fault/fault_config.h @@ -21,9 +21,11 @@ class HeaderNameValues { const char* prefix() { return ThreadSafeSingleton::get().prefix(); } const Http::LowerCaseString AbortRequest{absl::StrCat(prefix(), "-fault-abort-request")}; - const Http::LowerCaseString AbortGrpcRequest{absl::StrCat(prefix(), "-fault-abort-grpc-request")}; const Http::LowerCaseString AbortRequestPercentage{ absl::StrCat(prefix(), "-fault-abort-request-percentage")}; + const Http::LowerCaseString AbortGrpcRequest{absl::StrCat(prefix(), "-fault-abort-grpc-request")}; + const Http::LowerCaseString AbortGrpcRequestPercentage{ + absl::StrCat(prefix(), "-fault-abort-grpc-request-percentage")}; const Http::LowerCaseString DelayRequest{absl::StrCat(prefix(), "-fault-delay-request")}; const Http::LowerCaseString DelayRequestPercentage{ absl::StrCat(prefix(), "-fault-delay-request-percentage")}; @@ -58,7 +60,8 @@ class FaultAbortConfig { absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const { return provider_->httpStatusCode(request_headers); } - absl::optional grpcStatusCode(const Http::RequestHeaderMap* request_headers) const { + absl::optional + grpcStatusCode(const Http::RequestHeaderMap* request_headers) const { return provider_->grpcStatusCode(request_headers); } @@ -75,7 +78,8 @@ class FaultAbortConfig { // Return the HTTP status code to use. Optionally passed HTTP headers that may contain the // HTTP status code depending on the provider implementation. - virtual absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const PURE; + virtual absl::optional + httpStatusCode(const Http::RequestHeaderMap* request_headers) const PURE; // Return the gRPC status code to use. Optionally passed an HTTP header that may contain the // gRPC status code depending on the provider implementation. @@ -117,20 +121,24 @@ class FaultAbortConfig { }; // Abort provider the reads a status code from an HTTP header. - class HeaderAbortProvider : public AbortProvider, public HeaderPercentageProvider { + class HeaderAbortProvider : public AbortProvider { public: HeaderAbortProvider(const envoy::type::v3::FractionalPercent& percentage) - : HeaderPercentageProvider(HeaderNames::get().AbortRequestPercentage, percentage) {} + : grpcHeaderPercentageProvider_(HeaderNames::get().AbortGrpcRequestPercentage, percentage), + httpHeaderPercentageProvider_(HeaderNames::get().AbortRequestPercentage, percentage) {} - absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const override; + absl::optional + httpStatusCode(const Http::RequestHeaderMap* request_headers) const override; absl::optional grpcStatusCode(const Http::RequestHeaderMap* request_headers) const override; envoy::type::v3::FractionalPercent - percentage(const Http::RequestHeaderMap* request_headers) const override { - return HeaderPercentageProvider::percentage(request_headers); - } + percentage(const Http::RequestHeaderMap* request_headers) const override; + + private: + HeaderPercentageProvider grpcHeaderPercentageProvider_; + HeaderPercentageProvider httpHeaderPercentageProvider_; }; using AbortProviderPtr = std::unique_ptr; @@ -196,10 +204,10 @@ class FaultDelayConfig { }; // Delay provider the reads a delay from an HTTP header. - class HeaderDelayProvider : public DelayProvider, public HeaderPercentageProvider { + class HeaderDelayProvider : public DelayProvider { public: HeaderDelayProvider(const envoy::type::v3::FractionalPercent& percentage) - : HeaderPercentageProvider(HeaderNames::get().DelayRequestPercentage, percentage) {} + : headerPercentageProvider_(HeaderNames::get().DelayRequestPercentage, percentage) {} // DelayProvider absl::optional @@ -207,8 +215,11 @@ class FaultDelayConfig { envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const override { - return HeaderPercentageProvider::percentage(request_headers); + return headerPercentageProvider_.percentage(request_headers); } + + private: + HeaderPercentageProvider headerPercentageProvider_; }; using DelayProviderPtr = std::unique_ptr; @@ -273,16 +284,19 @@ class FaultRateLimitConfig { }; // Rate limit provider that reads the rate limit from an HTTP header. - class HeaderRateLimitProvider : public RateLimitProvider, public HeaderPercentageProvider { + class HeaderRateLimitProvider : public RateLimitProvider { public: HeaderRateLimitProvider(const envoy::type::v3::FractionalPercent& percentage) - : HeaderPercentageProvider(HeaderNames::get().ThroughputResponsePercentage, percentage) {} + : headerPercentageProvider_(HeaderNames::get().ThroughputResponsePercentage, percentage) {} // RateLimitProvider absl::optional rateKbps(const Http::RequestHeaderMap* request_headers) const override; envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const override { - return HeaderPercentageProvider::percentage(request_headers); + return headerPercentageProvider_.percentage(request_headers); } + + private: + HeaderPercentageProvider headerPercentageProvider_; }; using RateLimitProviderPtr = std::unique_ptr; diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 173b3f83072ef..5e1c1f44f19ea 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -151,6 +151,8 @@ Http::FilterHeadersStatus FaultFilter::decodeHeaders(Http::RequestHeaderMap& hea fmt::format("fault.http.{}.delay.fixed_duration_ms", downstream_cluster_); downstream_cluster_abort_http_status_key_ = fmt::format("fault.http.{}.abort.http_status", downstream_cluster_); + downstream_cluster_abort_grpc_status_key_ = + fmt::format("fault.http.{}.abort.grpc_status", downstream_cluster_); } maybeSetupResponseRateLimit(headers); @@ -297,8 +299,9 @@ AbortHttpAndGrpcStatus FaultFilter::abortStatus(const Http::RequestHeaderMap& re auto grpc_status = abortGrpcStatus(request_headers); // If gRPC status code is set, then HTTP will be set to Http::Code::OK (200). - return grpc_status.has_value() ? AbortHttpAndGrpcStatus(Http::Code::OK, grpc_status) - : AbortHttpAndGrpcStatus(abortHttpStatus(request_headers), absl::nullopt); + return grpc_status.has_value() + ? AbortHttpAndGrpcStatus(Http::Code::OK, grpc_status) + : AbortHttpAndGrpcStatus(abortHttpStatus(request_headers), absl::nullopt); } absl::optional @@ -311,30 +314,31 @@ FaultFilter::abortHttpStatus(const Http::RequestHeaderMap& request_headers) { } auto default_http_status_code = static_cast(http_status.value()); - auto runtime_http_status_code = static_cast(config_->runtime().snapshot().getInteger( - fault_settings_->abortHttpStatusRuntime(), default_http_status_code)); + auto runtime_http_status_code = config_->runtime().snapshot().getInteger( + fault_settings_->abortHttpStatusRuntime(), default_http_status_code); if (!downstream_cluster_abort_http_status_key_.empty()) { - runtime_http_status_code = static_cast(config_->runtime().snapshot().getInteger( - downstream_cluster_abort_http_status_key_, default_http_status_code)); + runtime_http_status_code = config_->runtime().snapshot().getInteger( + downstream_cluster_abort_http_status_key_, default_http_status_code); } - return runtime_http_status_code; + return static_cast(runtime_http_status_code); } absl::optional FaultFilter::abortGrpcStatus(const Http::RequestHeaderMap& request_headers) { auto grpc_status = fault_settings_->requestAbort()->grpcStatusCode(&request_headers); + if (!grpc_status.has_value()) { + return absl::nullopt; + } - auto default_grpc_status_code = grpc_status.has_value() - ? static_cast(grpc_status.value()) - : std::numeric_limits::max(); - + auto default_grpc_status_code = static_cast(grpc_status.value()); auto runtime_grpc_status_code = config_->runtime().snapshot().getInteger( fault_settings_->abortGrpcStatusRuntime(), default_grpc_status_code); - if (runtime_grpc_status_code == std::numeric_limits::max()) { - return absl::nullopt; + if (!downstream_cluster_abort_grpc_status_key_.empty()) { + runtime_grpc_status_code = config_->runtime().snapshot().getInteger( + downstream_cluster_abort_grpc_status_key_, default_grpc_status_code); } return static_cast(runtime_grpc_status_code); diff --git a/source/extensions/filters/http/fault/fault_filter.h b/source/extensions/filters/http/fault/fault_filter.h index ed06313fa3adf..2dfed7c9167d4 100644 --- a/source/extensions/filters/http/fault/fault_filter.h +++ b/source/extensions/filters/http/fault/fault_filter.h @@ -279,6 +279,7 @@ class FaultFilter : public Http::StreamFilter, Logger::Loggable0). Http::TestRequestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-grpc-request", "100"}}; @@ -107,6 +101,57 @@ TEST(FaultConfigTest, FaultAbortPercentageHeaderConfig) { greater_numerator_headers_percentage.denominator()); } +TEST(FaultConfigTest, FaultAbortGrpcPercentageHeaderConfig) { + envoy::extensions::filters::http::fault::v3::FaultAbort proto_config; + proto_config.mutable_header_abort(); + proto_config.mutable_percentage()->set_numerator(80); + proto_config.mutable_percentage()->set_denominator(envoy::type::v3::FractionalPercent::HUNDRED); + FaultAbortConfig config(proto_config); + + // No header. + EXPECT_EQ(proto_config.percentage().numerator(), config.percentage(nullptr).numerator()); + + // Header with bad data - fallback to proto config. + Http::TestRequestHeaderMapImpl bad_headers{ + {"x-envoy-fault-abort-grpc-request", "5"}, + {"x-envoy-fault-abort-grpc-request-percentage", "abc"}}; + const auto bad_headers_percentage = config.percentage(&bad_headers); + EXPECT_EQ(proto_config.percentage().numerator(), bad_headers_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), bad_headers_percentage.denominator()); + + // Out of range header, value too low - fallback to proto config. + Http::TestRequestHeaderMapImpl too_low_headers{ + {"x-envoy-fault-abort-grpc-request", "5"}, + {"x-envoy-fault-abort-grpc-request-percentage", "-1"}}; + const auto too_low_headers_percentage = config.percentage(&too_low_headers); + EXPECT_EQ(proto_config.percentage().numerator(), too_low_headers_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), too_low_headers_percentage.denominator()); + + // Missing grpc request percentage header - use proto config. + Http::TestRequestHeaderMapImpl missing_header{{"x-envoy-fault-abort-grpc-request", "5"}}; + const auto missing_header_percentage = config.percentage(&missing_header); + EXPECT_EQ(proto_config.percentage().numerator(), missing_header_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), missing_header_percentage.denominator()); + + // Valid header with value greater than the value of the numerator of default percentage - use + // proto config. + Http::TestRequestHeaderMapImpl good_headers{ + {"x-envoy-fault-abort-grpc-request", "5"}, + {"x-envoy-fault-abort-grpc-request-percentage", "90"}}; + const auto good_headers_percentage = config.percentage(&good_headers); + EXPECT_EQ(proto_config.percentage().numerator(), good_headers_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), good_headers_percentage.denominator()); + + // Valid header with value lesser than the value of the numerator of default percentage. + Http::TestRequestHeaderMapImpl greater_numerator_headers{ + {"x-envoy-fault-abort-grpc-request", "5"}, + {"x-envoy-fault-abort-grpc-request-percentage", "60"}}; + const auto greater_numerator_headers_percentage = config.percentage(&greater_numerator_headers); + EXPECT_EQ(60, greater_numerator_headers_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), + greater_numerator_headers_percentage.denominator()); +} + TEST(FaultConfigTest, FaultDelayHeaderConfig) { envoy::extensions::filters::common::fault::v3::FaultDelay proto_config; proto_config.mutable_header_delay(); diff --git a/test/extensions/filters/http/fault/fault_filter_integration_test.cc b/test/extensions/filters/http/fault/fault_filter_integration_test.cc index 9ce0b1fb9eea2..1ff7a8acdb515 100644 --- a/test/extensions/filters/http/fault/fault_filter_integration_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_integration_test.cc @@ -253,6 +253,28 @@ TEST_P(FaultIntegrationTestAllProtocols, HeaderFaultAbortGrpcConfig) { EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.response_rl_injected")->value()); } +// Request abort with grpc status, controlled via header configuration. +TEST_P(FaultIntegrationTestAllProtocols, HeaderFaultAbortGrpcConfig0PercentageHeader) { + initializeFilter(header_fault_config_); + codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); + + auto response = codec_client_->makeHeaderOnlyRequest( + Http::TestRequestHeaderMapImpl{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-envoy-fault-abort-grpc-request", "5"}, + {"x-envoy-fault-abort-grpc-request-percentage", "0"}, + {"content-type", "application/grpc"}}); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(default_response_headers_, true); + response->waitForEndStream(); + + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.aborts_injected")->value()); + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.delays_injected")->value()); + EXPECT_EQ(0UL, test_server_->counter("http.config_test.fault.response_rl_injected")->value()); +} + // Request abort with grpc status, controlled via configuration. TEST_P(FaultIntegrationTestAllProtocols, FaultAbortGrpcConfig) { initializeFilter(abort_grpc_fault_config_); diff --git a/test/extensions/filters/http/fault/fault_filter_test.cc b/test/extensions/filters/http/fault/fault_filter_test.cc index b0a2f0532597a..d71d2b69c566a 100644 --- a/test/extensions/filters/http/fault/fault_filter_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_test.cc @@ -255,10 +255,6 @@ TEST_F(FaultFilterTest, AbortWithHttpStatus) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 429)) .WillOnce(Return(429)); - EXPECT_CALL(runtime_.snapshot_, - getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) - .WillOnce(Return(std::numeric_limits::max())); - Http::TestResponseHeaderMapImpl response_headers{ {":status", "429"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -306,10 +302,6 @@ TEST_F(FaultFilterTest, HeaderAbortWithHttpStatus) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 429)) .WillOnce(Return(429)); - EXPECT_CALL(runtime_.snapshot_, - getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) - .WillOnce(Return(std::numeric_limits::max())); - Http::TestResponseHeaderMapImpl response_headers{ {":status", "429"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -615,10 +607,6 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortDownstream) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.cluster.abort.http_status", 503)) .WillOnce(Return(500)); - EXPECT_CALL(runtime_.snapshot_, - getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) - .WillOnce(Return(std::numeric_limits::max())); - Http::TestResponseHeaderMapImpl response_headers{ {":status", "500"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -677,10 +665,6 @@ TEST_F(FaultFilterTest, FixedDelayAndAbort) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 503)) .WillOnce(Return(503)); - EXPECT_CALL(runtime_.snapshot_, - getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) - .WillOnce(Return(std::numeric_limits::max())); - Http::TestResponseHeaderMapImpl response_headers{ {":status", "503"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -733,10 +717,6 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortDownstreamNodes) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 503)) .WillOnce(Return(503)); - EXPECT_CALL(runtime_.snapshot_, - getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) - .WillOnce(Return(std::numeric_limits::max())); - Http::TestResponseHeaderMapImpl response_headers{ {":status", "503"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, @@ -803,10 +783,6 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortHeaderMatchSuccess) { EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 503)) .WillOnce(Return(503)); - EXPECT_CALL(runtime_.snapshot_, - getInteger("fault.http.abort.grpc_status", std::numeric_limits::max())) - .WillOnce(Return(std::numeric_limits::max())); - Http::TestResponseHeaderMapImpl response_headers{ {":status", "503"}, {"content-length", "18"}, {"content-type", "text/plain"}}; EXPECT_CALL(decoder_filter_callbacks_, From e6ff2728403a1bd3bc85f39fe4f35d954c17431a Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Wed, 22 Apr 2020 22:42:54 -0700 Subject: [PATCH 5/8] Fix variables name Signed-off-by: Vijay Rajput --- .../filters/common/fault/fault_config.cc | 4 ++-- .../filters/common/fault/fault_config.h | 22 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/source/extensions/filters/common/fault/fault_config.cc b/source/extensions/filters/common/fault/fault_config.cc index ac18d2c0637ec..e4e4b0a287916 100644 --- a/source/extensions/filters/common/fault/fault_config.cc +++ b/source/extensions/filters/common/fault/fault_config.cc @@ -104,10 +104,10 @@ envoy::type::v3::FractionalPercent FaultAbortConfig::HeaderAbortProvider::percen if (request_headers != nullptr && request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest) != nullptr) { - return grpcHeaderPercentageProvider_.percentage(request_headers); + return grpc_header_percentage_provider_.percentage(request_headers); } - return httpHeaderPercentageProvider_.percentage(request_headers); + return http_header_percentage_provider_.percentage(request_headers); } FaultDelayConfig::FaultDelayConfig( diff --git a/source/extensions/filters/common/fault/fault_config.h b/source/extensions/filters/common/fault/fault_config.h index bbe5510f8c870..824228ecd9408 100644 --- a/source/extensions/filters/common/fault/fault_config.h +++ b/source/extensions/filters/common/fault/fault_config.h @@ -124,8 +124,9 @@ class FaultAbortConfig { class HeaderAbortProvider : public AbortProvider { public: HeaderAbortProvider(const envoy::type::v3::FractionalPercent& percentage) - : grpcHeaderPercentageProvider_(HeaderNames::get().AbortGrpcRequestPercentage, percentage), - httpHeaderPercentageProvider_(HeaderNames::get().AbortRequestPercentage, percentage) {} + : grpc_header_percentage_provider_(HeaderNames::get().AbortGrpcRequestPercentage, + percentage), + http_header_percentage_provider_(HeaderNames::get().AbortRequestPercentage, percentage) {} absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const override; @@ -137,8 +138,8 @@ class FaultAbortConfig { percentage(const Http::RequestHeaderMap* request_headers) const override; private: - HeaderPercentageProvider grpcHeaderPercentageProvider_; - HeaderPercentageProvider httpHeaderPercentageProvider_; + HeaderPercentageProvider grpc_header_percentage_provider_; + HeaderPercentageProvider http_header_percentage_provider_; }; using AbortProviderPtr = std::unique_ptr; @@ -207,7 +208,7 @@ class FaultDelayConfig { class HeaderDelayProvider : public DelayProvider { public: HeaderDelayProvider(const envoy::type::v3::FractionalPercent& percentage) - : headerPercentageProvider_(HeaderNames::get().DelayRequestPercentage, percentage) {} + : header_percentage_provider_(HeaderNames::get().DelayRequestPercentage, percentage) {} // DelayProvider absl::optional @@ -215,11 +216,11 @@ class FaultDelayConfig { envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const override { - return headerPercentageProvider_.percentage(request_headers); + return header_percentage_provider_.percentage(request_headers); } private: - HeaderPercentageProvider headerPercentageProvider_; + HeaderPercentageProvider header_percentage_provider_; }; using DelayProviderPtr = std::unique_ptr; @@ -287,16 +288,17 @@ class FaultRateLimitConfig { class HeaderRateLimitProvider : public RateLimitProvider { public: HeaderRateLimitProvider(const envoy::type::v3::FractionalPercent& percentage) - : headerPercentageProvider_(HeaderNames::get().ThroughputResponsePercentage, percentage) {} + : header_percentage_provider_(HeaderNames::get().ThroughputResponsePercentage, percentage) { + } // RateLimitProvider absl::optional rateKbps(const Http::RequestHeaderMap* request_headers) const override; envoy::type::v3::FractionalPercent percentage(const Http::RequestHeaderMap* request_headers) const override { - return headerPercentageProvider_.percentage(request_headers); + return header_percentage_provider_.percentage(request_headers); } private: - HeaderPercentageProvider headerPercentageProvider_; + HeaderPercentageProvider header_percentage_provider_; }; using RateLimitProviderPtr = std::unique_ptr; From 4576e1941980a4c13a2d800b03717f3154876791 Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Thu, 23 Apr 2020 09:01:13 -0700 Subject: [PATCH 6/8] Attempt to make "Linux-x64 clang_tidy" happy Signed-off-by: Vijay Rajput --- source/extensions/filters/http/fault/fault_filter.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 5e1c1f44f19ea..df4eb33891e13 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -293,15 +293,15 @@ FaultFilter::delayDuration(const Http::RequestHeaderMap& request_headers) { AbortHttpAndGrpcStatus FaultFilter::abortStatus(const Http::RequestHeaderMap& request_headers) { if (!isAbortEnabled(request_headers)) { - return AbortHttpAndGrpcStatus(absl::nullopt, absl::nullopt); + return AbortHttpAndGrpcStatus{absl::nullopt, absl::nullopt}; } auto grpc_status = abortGrpcStatus(request_headers); // If gRPC status code is set, then HTTP will be set to Http::Code::OK (200). return grpc_status.has_value() - ? AbortHttpAndGrpcStatus(Http::Code::OK, grpc_status) - : AbortHttpAndGrpcStatus(abortHttpStatus(request_headers), absl::nullopt); + ? AbortHttpAndGrpcStatus{Http::Code::OK, grpc_status} + : AbortHttpAndGrpcStatus{abortHttpStatus(request_headers), absl::nullopt}; } absl::optional From 8d19697c16f2953f205f1e72e3b12809a8ee2489 Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Fri, 24 Apr 2020 12:12:51 -0700 Subject: [PATCH 7/8] Change the precedence order of grpc_status vs http_status 1. If both x-envoy-fault-abort-grpc-request and x-envoy-fault-abort-request headers are set, prefer x-envoy-fault-abort-request. 2. Remove nullptr checks per feedback. Signed-off-by: Vijay Rajput --- .../http/http_filters/fault_filter.rst | 8 ++- .../filters/common/fault/fault_config.cc | 25 ++------- .../filters/common/fault/fault_config.h | 2 +- .../filters/http/fault/fault_filter.cc | 15 ++++-- .../filters/common/fault/fault_config_test.cc | 52 ++++++++++++------- .../fault/fault_filter_integration_test.cc | 2 +- .../filters/http/fault/fault_filter_test.cc | 50 ++++++++++++++++++ 7 files changed, 108 insertions(+), 46 deletions(-) diff --git a/docs/root/configuration/http/http_filters/fault_filter.rst b/docs/root/configuration/http/http_filters/fault_filter.rst index 9fe2484469812..ad4da9bc36d6f 100644 --- a/docs/root/configuration/http/http_filters/fault_filter.rst +++ b/docs/root/configuration/http/http_filters/fault_filter.rst @@ -59,7 +59,10 @@ x-envoy-fault-abort-grpc-request the gRPC status code to return in response to a request. Its value range is [0, UInt32.Max] instead of [0, 16] to allow testing even not well-defined gRPC status codes. When this header is set, the HTTP response status code will be set to 200. In order for the header to work, :ref:`header_abort - ` needs to be set. + ` needs to be set. If both + *x-envoy-fault-abort-request* and *x-envoy-fault-abort-grpc-request* headers are set then + *x-envoy-fault-abort-grpc-request* header will be **ignored** and fault response http status code will be + set to *x-envoy-fault-abort-request* header value. x-envoy-fault-abort-grpc-request-percentage The percentage of gRPC requests that should be failed with a status code that's defined @@ -71,7 +74,8 @@ x-envoy-fault-abort-grpc-request-percentage :ref:`percentage ` field. In order for the header to work, :ref:`header_abort ` needs to be set and - *x-envoy-fault-abort-grpc-request* HTTP header needs to be a part of a request. + *x-envoy-fault-abort-grpc-request* HTTP header needs to be a part of the request and + *x-envoy-fault-abort-request* header should not be present in the request. x-envoy-fault-delay-request The duration to delay a request by. The header value should be an integer that specifies the number diff --git a/source/extensions/filters/common/fault/fault_config.cc b/source/extensions/filters/common/fault/fault_config.cc index e4e4b0a287916..2e4bb2b04cebe 100644 --- a/source/extensions/filters/common/fault/fault_config.cc +++ b/source/extensions/filters/common/fault/fault_config.cc @@ -13,10 +13,6 @@ namespace Fault { envoy::type::v3::FractionalPercent HeaderPercentageProvider::percentage(const Http::RequestHeaderMap* request_headers) const { - if (request_headers == nullptr) { - return percentage_; - } - const auto header = request_headers->get(header_name_); if (header == nullptr) { return percentage_; @@ -42,9 +38,8 @@ FaultAbortConfig::FaultAbortConfig( absl::nullopt, abort_config.percentage()); break; case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kGrpcStatus: - // If gRPC status code is set, then HTTP will be set to 200 provider_ = std::make_unique( - Http::Code::OK, static_cast(abort_config.grpc_status()), + absl::nullopt, static_cast(abort_config.grpc_status()), abort_config.percentage()); break; case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kHeaderAbort: @@ -57,10 +52,6 @@ FaultAbortConfig::FaultAbortConfig( absl::optional FaultAbortConfig::HeaderAbortProvider::httpStatusCode( const Http::RequestHeaderMap* request_headers) const { - if (request_headers == nullptr) { - return absl::nullopt; - } - absl::optional ret = absl::nullopt; auto header = request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortRequest); if (header == nullptr) { @@ -81,10 +72,6 @@ absl::optional FaultAbortConfig::HeaderAbortProvider::httpStatusCode absl::optional FaultAbortConfig::HeaderAbortProvider::grpcStatusCode( const Http::RequestHeaderMap* request_headers) const { - if (request_headers == nullptr) { - return absl::nullopt; - } - auto header = request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest); if (header == nullptr) { return absl::nullopt; @@ -100,14 +87,12 @@ absl::optional FaultAbortConfig::HeaderAbortProvider:: envoy::type::v3::FractionalPercent FaultAbortConfig::HeaderAbortProvider::percentage( const Http::RequestHeaderMap* request_headers) const { - // if the abort fault is for grpc status, then use grpc fault request percentage. - if (request_headers != nullptr && - request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortGrpcRequest) != - nullptr) { - return grpc_header_percentage_provider_.percentage(request_headers); + // if the abort fault contains http status header, then use http status request percentage. + if (request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortRequest) != nullptr) { + return http_header_percentage_provider_.percentage(request_headers); } - return http_header_percentage_provider_.percentage(request_headers); + return grpc_header_percentage_provider_.percentage(request_headers); } FaultDelayConfig::FaultDelayConfig( diff --git a/source/extensions/filters/common/fault/fault_config.h b/source/extensions/filters/common/fault/fault_config.h index 824228ecd9408..898e731c62f23 100644 --- a/source/extensions/filters/common/fault/fault_config.h +++ b/source/extensions/filters/common/fault/fault_config.h @@ -95,7 +95,7 @@ class FaultAbortConfig { // Abort provider that uses a fixed abort status code. class FixedAbortProvider : public AbortProvider { public: - FixedAbortProvider(Http::Code http_status_code, + FixedAbortProvider(absl::optional http_status_code, absl::optional grpc_status_code, const envoy::type::v3::FractionalPercent& percentage) : http_status_code_(http_status_code), grpc_status_code_(grpc_status_code), diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index df4eb33891e13..6cdd23947f80f 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -296,12 +296,19 @@ AbortHttpAndGrpcStatus FaultFilter::abortStatus(const Http::RequestHeaderMap& re return AbortHttpAndGrpcStatus{absl::nullopt, absl::nullopt}; } + auto http_status = abortHttpStatus(request_headers); + // If http status code is set, then gRPC status won't be used. + if (http_status.has_value()) { + return AbortHttpAndGrpcStatus{http_status, absl::nullopt}; + } + auto grpc_status = abortGrpcStatus(request_headers); + // If gRPC status code is set, then http status will be set to Http::Code::OK (200) + if (grpc_status.has_value()) { + return AbortHttpAndGrpcStatus{Http::Code::OK, grpc_status}; + } - // If gRPC status code is set, then HTTP will be set to Http::Code::OK (200). - return grpc_status.has_value() - ? AbortHttpAndGrpcStatus{Http::Code::OK, grpc_status} - : AbortHttpAndGrpcStatus{abortHttpStatus(request_headers), absl::nullopt}; + return AbortHttpAndGrpcStatus{absl::nullopt, absl::nullopt}; } absl::optional diff --git a/test/extensions/filters/common/fault/fault_config_test.cc b/test/extensions/filters/common/fault/fault_config_test.cc index 875dffb89d9f7..8e27d16af388a 100644 --- a/test/extensions/filters/common/fault/fault_config_test.cc +++ b/test/extensions/filters/common/fault/fault_config_test.cc @@ -18,9 +18,6 @@ TEST(FaultConfigTest, FaultAbortHeaderConfig) { proto_config.mutable_header_abort(); FaultAbortConfig config(proto_config); - // No header. - EXPECT_EQ(absl::nullopt, config.httpStatusCode(nullptr)); - // Header with bad data. Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "abc"}}; EXPECT_EQ(absl::nullopt, config.httpStatusCode(&bad_headers)); @@ -43,9 +40,6 @@ TEST(FaultConfigTest, FaultAbortGrpcHeaderConfig) { proto_config.mutable_header_abort(); FaultAbortConfig config(proto_config); - // No header. - EXPECT_EQ(absl::nullopt, config.grpcStatusCode(nullptr)); - // Header with bad data. Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-grpc-request", "abc"}}; EXPECT_EQ(absl::nullopt, config.grpcStatusCode(&bad_headers)); @@ -54,11 +48,11 @@ TEST(FaultConfigTest, FaultAbortGrpcHeaderConfig) { Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-grpc-request", "-1"}}; EXPECT_EQ(absl::nullopt, config.grpcStatusCode(&too_low_headers)); - // Valid header. + // Valid header - with well-defined gRPC status code in [0,16] range. Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-grpc-request", "5"}}; EXPECT_EQ(Grpc::Status::NotFound, config.grpcStatusCode(&good_headers)); - // Valid header - unknown gRPC code (>0). + // Valid header - with not well-defined gRPC status code (> 16). Http::TestRequestHeaderMapImpl too_high_headers{{"x-envoy-fault-abort-grpc-request", "100"}}; EXPECT_EQ(100, config.grpcStatusCode(&too_high_headers)); } @@ -70,31 +64,56 @@ TEST(FaultConfigTest, FaultAbortPercentageHeaderConfig) { proto_config.mutable_percentage()->set_denominator(envoy::type::v3::FractionalPercent::HUNDRED); FaultAbortConfig config(proto_config); - // No header. - EXPECT_EQ(proto_config.percentage().numerator(), config.percentage(nullptr).numerator()); - // Header with bad data - fallback to proto config. - Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request-percentage", "abc"}}; + Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "401"}, + {"x-envoy-fault-abort-request-percentage", "abc"}}; const auto bad_headers_percentage = config.percentage(&bad_headers); EXPECT_EQ(proto_config.percentage().numerator(), bad_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), bad_headers_percentage.denominator()); // Out of range header, value too low - fallback to proto config. - Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request-percentage", "-1"}}; + Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request", "401"}, + {"x-envoy-fault-abort-request-percentage", "-1"}}; const auto too_low_headers_percentage = config.percentage(&too_low_headers); EXPECT_EQ(proto_config.percentage().numerator(), too_low_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), too_low_headers_percentage.denominator()); // Valid header with value greater than the value of the numerator of default percentage - use // proto config. - Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request-percentage", "90"}}; + Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request", "401"}, + {"x-envoy-fault-abort-request-percentage", "90"}}; const auto good_headers_percentage = config.percentage(&good_headers); EXPECT_EQ(proto_config.percentage().numerator(), good_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), good_headers_percentage.denominator()); + // Headers contains grpc-request-percentage instead of abort-request-percentage - fallback to + // proto config. + Http::TestRequestHeaderMapImpl grpc_request_numerator_headers{ + {"x-envoy-fault-abort-request", "401"}, + {"x-envoy-fault-abort-grpc-request-percentage", "60"}}; + const auto grpc_request_numerator_headers_percentage = + config.percentage(&grpc_request_numerator_headers); + EXPECT_EQ(proto_config.percentage().numerator(), + grpc_request_numerator_headers_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), + grpc_request_numerator_headers_percentage.denominator()); + + // Headers contains both grpc-request-percentage and abort-request-percentage - use + // abort-request-percentage header value. + Http::TestRequestHeaderMapImpl http_and_grpc_request_numerator_headers{ + {"x-envoy-fault-abort-request", "401"}, + {"x-envoy-fault-abort-request-percentage", "60"}, + {"x-envoy-fault-abort-grpc-request", "401"}, + {"x-envoy-fault-abort-grpc-request-percentage", "40"}}; + const auto http_and_grpc_request_numerator_headers_percentage = + config.percentage(&http_and_grpc_request_numerator_headers); + EXPECT_EQ(60, http_and_grpc_request_numerator_headers_percentage.numerator()); + EXPECT_EQ(proto_config.percentage().denominator(), + http_and_grpc_request_numerator_headers_percentage.denominator()); + // Valid header with value lesser than the value of the numerator of default percentage. Http::TestRequestHeaderMapImpl greater_numerator_headers{ - {"x-envoy-fault-abort-request-percentage", "60"}}; + {"x-envoy-fault-abort-request", "401"}, {"x-envoy-fault-abort-request-percentage", "60"}}; const auto greater_numerator_headers_percentage = config.percentage(&greater_numerator_headers); EXPECT_EQ(60, greater_numerator_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), @@ -108,9 +127,6 @@ TEST(FaultConfigTest, FaultAbortGrpcPercentageHeaderConfig) { proto_config.mutable_percentage()->set_denominator(envoy::type::v3::FractionalPercent::HUNDRED); FaultAbortConfig config(proto_config); - // No header. - EXPECT_EQ(proto_config.percentage().numerator(), config.percentage(nullptr).numerator()); - // Header with bad data - fallback to proto config. Http::TestRequestHeaderMapImpl bad_headers{ {"x-envoy-fault-abort-grpc-request", "5"}, diff --git a/test/extensions/filters/http/fault/fault_filter_integration_test.cc b/test/extensions/filters/http/fault/fault_filter_integration_test.cc index 1ff7a8acdb515..5967b9b84c53d 100644 --- a/test/extensions/filters/http/fault/fault_filter_integration_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_integration_test.cc @@ -53,9 +53,9 @@ name: fault typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault abort: + grpc_status: 5 percentage: numerator: 100 - grpc_status: 5 )EOF"; }; diff --git a/test/extensions/filters/http/fault/fault_filter_test.cc b/test/extensions/filters/http/fault/fault_filter_test.cc index d71d2b69c566a..a29c5cc858161 100644 --- a/test/extensions/filters/http/fault/fault_filter_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_test.cc @@ -429,6 +429,56 @@ TEST_F(FaultFilterTest, HeaderAbortWithGrpcStatus) { EXPECT_EQ("fault_filter_abort", decoder_filter_callbacks_.details_); } +TEST_F(FaultFilterTest, HeaderAbortWithHttpAndGrpcStatus) { + SetUpTest(header_abort_only_yaml); + + request_headers_.addCopy("x-envoy-fault-abort-request", "429"); + request_headers_.addCopy("x-envoy-fault-abort-grpc-request", "5"); + + EXPECT_CALL(runtime_.snapshot_, + getInteger("fault.http.max_active_faults", std::numeric_limits::max())) + .WillOnce(Return(std::numeric_limits::max())); + + EXPECT_CALL(decoder_filter_callbacks_, continueDecoding()).Times(0); + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::DelayInjected)) + .Times(0); + + // Abort related calls + EXPECT_CALL(runtime_.snapshot_, + featureEnabled("fault.http.abort.abort_percent", + Matcher(Percent(100)))) + .WillOnce(Return(true)); + + EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.http_status", 429)) + .WillOnce(Return(429)); + + EXPECT_CALL(runtime_.snapshot_, getInteger("fault.http.abort.grpc_status", 5)).Times(0); + + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "429"}, {"content-length", "18"}, {"content-type", "text/plain"}}; + EXPECT_CALL(decoder_filter_callbacks_, + encodeHeaders_(HeaderMapEqualRef(&response_headers), false)); + EXPECT_CALL(decoder_filter_callbacks_, encodeData(_, true)); + + EXPECT_CALL(decoder_filter_callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::FaultInjected)); + + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, false)); + Http::MetadataMap metadata_map{{"metadata", "metadata"}}; + EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); + EXPECT_EQ(1UL, config_->stats().active_faults_.value()); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_)); + filter_->onDestroy(); + + EXPECT_EQ(0UL, config_->stats().delays_injected_.value()); + EXPECT_EQ(1UL, config_->stats().aborts_injected_.value()); + EXPECT_EQ(0UL, config_->stats().active_faults_.value()); + EXPECT_EQ("fault_filter_abort", decoder_filter_callbacks_.details_); +} + TEST_F(FaultFilterTest, FixedDelayZeroDuration) { SetUpTest(fixed_delay_only_yaml); From 0ff968f767a9ec5e52e91ce57a09b14126d23e4f Mon Sep 17 00:00:00 2001 From: Vijay Rajput Date: Fri, 24 Apr 2020 18:28:11 -0700 Subject: [PATCH 8/8] Switch back to using single header for abort fault percentage Signed-off-by: Vijay Rajput --- .../http/http_filters/fault_filter.rst | 34 +++----- .../filters/common/fault/fault_config.cc | 10 --- .../filters/common/fault/fault_config.h | 13 ++- .../filters/http/fault/fault_filter.cc | 6 +- .../filters/common/fault/fault_config_test.cc | 84 +------------------ .../fault/fault_filter_integration_test.cc | 2 +- 6 files changed, 23 insertions(+), 126 deletions(-) diff --git a/docs/root/configuration/http/http_filters/fault_filter.rst b/docs/root/configuration/http/http_filters/fault_filter.rst index ad4da9bc36d6f..f79fc4d44a7a1 100644 --- a/docs/root/configuration/http/http_filters/fault_filter.rst +++ b/docs/root/configuration/http/http_filters/fault_filter.rst @@ -42,18 +42,6 @@ x-envoy-fault-abort-request In order for the header to work, :ref:`header_abort ` needs to be set. -x-envoy-fault-abort-request-percentage - The percentage of requests that should be failed with a status code that's defined - by the value of *x-envoy-fault-abort-request* HTTP header. The header value should be an integer - that specifies the numerator of the percentage of request to apply aborts to and must be greater - or equal to 0 and its maximum value is capped by the value of the numerator of - :ref:`percentage ` field. - Percentage's denominator is equal to default percentage's denominator - :ref:`percentage ` field. - In order for the header to work, :ref:`header_abort - ` needs to be set and - *x-envoy-fault-abort-request* HTTP header needs to be a part of a request. - x-envoy-fault-abort-grpc-request gRPC status code to abort a request with. The header value should be a non-negative integer that specifies the gRPC status code to return in response to a request. Its value range is [0, UInt32.Max] instead of [0, 16] @@ -64,18 +52,17 @@ x-envoy-fault-abort-grpc-request *x-envoy-fault-abort-grpc-request* header will be **ignored** and fault response http status code will be set to *x-envoy-fault-abort-request* header value. -x-envoy-fault-abort-grpc-request-percentage - The percentage of gRPC requests that should be failed with a status code that's defined - by the value of *x-envoy-fault-abort-grpc-request* HTTP header. The header value should be an integer - that specifies the numerator of the percentage of request to apply aborts to and must be greater - or equal to 0 and its maximum value is capped by the value of the numerator of +x-envoy-fault-abort-request-percentage + The percentage of requests that should be failed with a status code that's defined + by the value of *x-envoy-fault-abort-request* or *x-envoy-fault-abort-grpc-request* HTTP headers. + The header value should be an integer that specifies the numerator of the percentage of request to apply aborts + to and must be greater or equal to 0 and its maximum value is capped by the value of the numerator of :ref:`percentage ` field. Percentage's denominator is equal to default percentage's denominator :ref:`percentage ` field. In order for the header to work, :ref:`header_abort ` needs to be set and - *x-envoy-fault-abort-grpc-request* HTTP header needs to be a part of the request and - *x-envoy-fault-abort-request* header should not be present in the request. + either *x-envoy-fault-abort-request* or *x-envoy-fault-abort-grpc-request* HTTP header needs to be a part of the request. x-envoy-fault-delay-request The duration to delay a request by. The header value should be an integer that specifies the number @@ -169,11 +156,10 @@ fault.http.abort.http_status fault.http.abort.grpc_status gRPC status code that will be used as the response status code of requests that will be - aborted if the headers match. Defaults to the gRPC status code specified - in the config. If this field is missing from both the runtime and the config, - gRPC status code in the response will be derived from *fault.http.abort.http_status* field. - This runtime key is only available when the filter is :ref:`configured for abort - `. + aborted if the headers match. Defaults to the gRPC status code specified in the config. + If this field is missing from both the runtime and the config, gRPC status code in the response + will be derived from *fault.http.abort.http_status* field. This runtime key is only available when + the filter is :ref:`configured for abort `. fault.http.delay.fixed_delay_percent % of requests that will be delayed if the headers match. Defaults to the diff --git a/source/extensions/filters/common/fault/fault_config.cc b/source/extensions/filters/common/fault/fault_config.cc index 2e4bb2b04cebe..ebbb86e2fd95e 100644 --- a/source/extensions/filters/common/fault/fault_config.cc +++ b/source/extensions/filters/common/fault/fault_config.cc @@ -85,16 +85,6 @@ absl::optional FaultAbortConfig::HeaderAbortProvider:: return static_cast(code); } -envoy::type::v3::FractionalPercent FaultAbortConfig::HeaderAbortProvider::percentage( - const Http::RequestHeaderMap* request_headers) const { - // if the abort fault contains http status header, then use http status request percentage. - if (request_headers->get(Filters::Common::Fault::HeaderNames::get().AbortRequest) != nullptr) { - return http_header_percentage_provider_.percentage(request_headers); - } - - return grpc_header_percentage_provider_.percentage(request_headers); -} - FaultDelayConfig::FaultDelayConfig( const envoy::extensions::filters::common::fault::v3::FaultDelay& delay_config) { switch (delay_config.fault_delay_secifier_case()) { diff --git a/source/extensions/filters/common/fault/fault_config.h b/source/extensions/filters/common/fault/fault_config.h index 898e731c62f23..e253814273b34 100644 --- a/source/extensions/filters/common/fault/fault_config.h +++ b/source/extensions/filters/common/fault/fault_config.h @@ -24,8 +24,6 @@ class HeaderNameValues { const Http::LowerCaseString AbortRequestPercentage{ absl::StrCat(prefix(), "-fault-abort-request-percentage")}; const Http::LowerCaseString AbortGrpcRequest{absl::StrCat(prefix(), "-fault-abort-grpc-request")}; - const Http::LowerCaseString AbortGrpcRequestPercentage{ - absl::StrCat(prefix(), "-fault-abort-grpc-request-percentage")}; const Http::LowerCaseString DelayRequest{absl::StrCat(prefix(), "-fault-delay-request")}; const Http::LowerCaseString DelayRequestPercentage{ absl::StrCat(prefix(), "-fault-delay-request-percentage")}; @@ -124,9 +122,7 @@ class FaultAbortConfig { class HeaderAbortProvider : public AbortProvider { public: HeaderAbortProvider(const envoy::type::v3::FractionalPercent& percentage) - : grpc_header_percentage_provider_(HeaderNames::get().AbortGrpcRequestPercentage, - percentage), - http_header_percentage_provider_(HeaderNames::get().AbortRequestPercentage, percentage) {} + : header_percentage_provider_(HeaderNames::get().AbortRequestPercentage, percentage) {} absl::optional httpStatusCode(const Http::RequestHeaderMap* request_headers) const override; @@ -135,11 +131,12 @@ class FaultAbortConfig { grpcStatusCode(const Http::RequestHeaderMap* request_headers) const override; envoy::type::v3::FractionalPercent - percentage(const Http::RequestHeaderMap* request_headers) const override; + percentage(const Http::RequestHeaderMap* request_headers) const override { + return header_percentage_provider_.percentage(request_headers); + } private: - HeaderPercentageProvider grpc_header_percentage_provider_; - HeaderPercentageProvider http_header_percentage_provider_; + HeaderPercentageProvider header_percentage_provider_; }; using AbortProviderPtr = std::unique_ptr; diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 6cdd23947f80f..0f7f2ad1ca8df 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -430,10 +430,10 @@ void FaultFilter::postDelayInjection(const Http::RequestHeaderMap& request_heade } void FaultFilter::abortWithStatus(Http::Code http_status_code, - absl::optional grpc_status_code) { + absl::optional grpc_status) { decoder_callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::FaultInjected); - decoder_callbacks_->sendLocalReply(http_status_code, "fault filter abort", nullptr, - grpc_status_code, RcDetails::get().FaultAbort); + decoder_callbacks_->sendLocalReply(http_status_code, "fault filter abort", nullptr, grpc_status, + RcDetails::get().FaultAbort); recordAbortsInjectedStats(); } diff --git a/test/extensions/filters/common/fault/fault_config_test.cc b/test/extensions/filters/common/fault/fault_config_test.cc index 8e27d16af388a..784aa2d88ab46 100644 --- a/test/extensions/filters/common/fault/fault_config_test.cc +++ b/test/extensions/filters/common/fault/fault_config_test.cc @@ -65,103 +65,27 @@ TEST(FaultConfigTest, FaultAbortPercentageHeaderConfig) { FaultAbortConfig config(proto_config); // Header with bad data - fallback to proto config. - Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request", "401"}, - {"x-envoy-fault-abort-request-percentage", "abc"}}; + Http::TestRequestHeaderMapImpl bad_headers{{"x-envoy-fault-abort-request-percentage", "abc"}}; const auto bad_headers_percentage = config.percentage(&bad_headers); EXPECT_EQ(proto_config.percentage().numerator(), bad_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), bad_headers_percentage.denominator()); // Out of range header, value too low - fallback to proto config. - Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request", "401"}, - {"x-envoy-fault-abort-request-percentage", "-1"}}; + Http::TestRequestHeaderMapImpl too_low_headers{{"x-envoy-fault-abort-request-percentage", "-1"}}; const auto too_low_headers_percentage = config.percentage(&too_low_headers); EXPECT_EQ(proto_config.percentage().numerator(), too_low_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), too_low_headers_percentage.denominator()); // Valid header with value greater than the value of the numerator of default percentage - use // proto config. - Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request", "401"}, - {"x-envoy-fault-abort-request-percentage", "90"}}; - const auto good_headers_percentage = config.percentage(&good_headers); - EXPECT_EQ(proto_config.percentage().numerator(), good_headers_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), good_headers_percentage.denominator()); - - // Headers contains grpc-request-percentage instead of abort-request-percentage - fallback to - // proto config. - Http::TestRequestHeaderMapImpl grpc_request_numerator_headers{ - {"x-envoy-fault-abort-request", "401"}, - {"x-envoy-fault-abort-grpc-request-percentage", "60"}}; - const auto grpc_request_numerator_headers_percentage = - config.percentage(&grpc_request_numerator_headers); - EXPECT_EQ(proto_config.percentage().numerator(), - grpc_request_numerator_headers_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), - grpc_request_numerator_headers_percentage.denominator()); - - // Headers contains both grpc-request-percentage and abort-request-percentage - use - // abort-request-percentage header value. - Http::TestRequestHeaderMapImpl http_and_grpc_request_numerator_headers{ - {"x-envoy-fault-abort-request", "401"}, - {"x-envoy-fault-abort-request-percentage", "60"}, - {"x-envoy-fault-abort-grpc-request", "401"}, - {"x-envoy-fault-abort-grpc-request-percentage", "40"}}; - const auto http_and_grpc_request_numerator_headers_percentage = - config.percentage(&http_and_grpc_request_numerator_headers); - EXPECT_EQ(60, http_and_grpc_request_numerator_headers_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), - http_and_grpc_request_numerator_headers_percentage.denominator()); - - // Valid header with value lesser than the value of the numerator of default percentage. - Http::TestRequestHeaderMapImpl greater_numerator_headers{ - {"x-envoy-fault-abort-request", "401"}, {"x-envoy-fault-abort-request-percentage", "60"}}; - const auto greater_numerator_headers_percentage = config.percentage(&greater_numerator_headers); - EXPECT_EQ(60, greater_numerator_headers_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), - greater_numerator_headers_percentage.denominator()); -} - -TEST(FaultConfigTest, FaultAbortGrpcPercentageHeaderConfig) { - envoy::extensions::filters::http::fault::v3::FaultAbort proto_config; - proto_config.mutable_header_abort(); - proto_config.mutable_percentage()->set_numerator(80); - proto_config.mutable_percentage()->set_denominator(envoy::type::v3::FractionalPercent::HUNDRED); - FaultAbortConfig config(proto_config); - - // Header with bad data - fallback to proto config. - Http::TestRequestHeaderMapImpl bad_headers{ - {"x-envoy-fault-abort-grpc-request", "5"}, - {"x-envoy-fault-abort-grpc-request-percentage", "abc"}}; - const auto bad_headers_percentage = config.percentage(&bad_headers); - EXPECT_EQ(proto_config.percentage().numerator(), bad_headers_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), bad_headers_percentage.denominator()); - - // Out of range header, value too low - fallback to proto config. - Http::TestRequestHeaderMapImpl too_low_headers{ - {"x-envoy-fault-abort-grpc-request", "5"}, - {"x-envoy-fault-abort-grpc-request-percentage", "-1"}}; - const auto too_low_headers_percentage = config.percentage(&too_low_headers); - EXPECT_EQ(proto_config.percentage().numerator(), too_low_headers_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), too_low_headers_percentage.denominator()); - - // Missing grpc request percentage header - use proto config. - Http::TestRequestHeaderMapImpl missing_header{{"x-envoy-fault-abort-grpc-request", "5"}}; - const auto missing_header_percentage = config.percentage(&missing_header); - EXPECT_EQ(proto_config.percentage().numerator(), missing_header_percentage.numerator()); - EXPECT_EQ(proto_config.percentage().denominator(), missing_header_percentage.denominator()); - - // Valid header with value greater than the value of the numerator of default percentage - use - // proto config. - Http::TestRequestHeaderMapImpl good_headers{ - {"x-envoy-fault-abort-grpc-request", "5"}, - {"x-envoy-fault-abort-grpc-request-percentage", "90"}}; + Http::TestRequestHeaderMapImpl good_headers{{"x-envoy-fault-abort-request-percentage", "90"}}; const auto good_headers_percentage = config.percentage(&good_headers); EXPECT_EQ(proto_config.percentage().numerator(), good_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), good_headers_percentage.denominator()); // Valid header with value lesser than the value of the numerator of default percentage. Http::TestRequestHeaderMapImpl greater_numerator_headers{ - {"x-envoy-fault-abort-grpc-request", "5"}, - {"x-envoy-fault-abort-grpc-request-percentage", "60"}}; + {"x-envoy-fault-abort-request-percentage", "60"}}; const auto greater_numerator_headers_percentage = config.percentage(&greater_numerator_headers); EXPECT_EQ(60, greater_numerator_headers_percentage.numerator()); EXPECT_EQ(proto_config.percentage().denominator(), diff --git a/test/extensions/filters/http/fault/fault_filter_integration_test.cc b/test/extensions/filters/http/fault/fault_filter_integration_test.cc index 5967b9b84c53d..0864793d136fd 100644 --- a/test/extensions/filters/http/fault/fault_filter_integration_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_integration_test.cc @@ -264,7 +264,7 @@ TEST_P(FaultIntegrationTestAllProtocols, HeaderFaultAbortGrpcConfig0PercentageHe {":scheme", "http"}, {":authority", "host"}, {"x-envoy-fault-abort-grpc-request", "5"}, - {"x-envoy-fault-abort-grpc-request-percentage", "0"}, + {"x-envoy-fault-abort-request-percentage", "0"}, {"content-type", "application/grpc"}}); waitForNextUpstreamRequest(); upstream_request_->encodeHeaders(default_response_headers_, true);