-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose fault.http.abort.http_status setting via HTTP header #10294
Changes from 32 commits
ec89763
d9caef6
9b28c7d
31126b4
e1cf997
c534968
386cf81
ec19b1a
e8657fe
49042c3
239c406
a63b408
d20a624
bd32f55
e3c7ca6
3cd511b
a43c6ef
17ed854
559465e
93a6332
3483611
28e4cf8
ac46ced
58e21f7
97ab5b3
18f9152
f337341
224fa21
109531e
4ba49b7
08ecf36
2228f5f
60abf18
87c61e8
7db79dc
0c21567
deec559
5d62ece
1c1420f
e647685
95941c0
26651ab
651ba63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,9 @@ The fault filter has the capability to allow fault configuration to be specified | |
This is useful in certain scenarios in which it is desired to allow the client to specify its own | ||
fault configuration. The currently supported header controls are: | ||
|
||
* Request abort configuration via the *x-envoy-fault-abort-request* header. The header value | ||
should be an integer that specifies the HTTP status code to return in response to a request | ||
and must be in the range [200, 600). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth pointing out that |
||
* Request delay configuration via the *x-envoy-fault-delay-request* header. The header value | ||
should be an integer that specifies the number of milliseconds to throttle the latency for. | ||
* Response rate limit configuration via the *x-envoy-fault-throughput-response* header. The | ||
|
@@ -57,6 +60,10 @@ options: | |
typed_config: | ||
"@type": type.googleapis.com/envoy.config.filter.http.fault.v2.HTTPFault | ||
max_active_faults: 100 | ||
abort: | ||
header_abort: {} | ||
percentage: | ||
numerator: 100 | ||
delay: | ||
header_delay: {} | ||
percentage: | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
#include "extensions/filters/common/fault/fault_config.h" | ||
|
||
#include "envoy/extensions/filters/common/fault/v3/fault.pb.h" | ||
#include "envoy/extensions/filters/http/fault/v3/fault.pb.h" | ||
|
||
#include "common/protobuf/utility.h" | ||
|
||
|
@@ -10,6 +11,40 @@ namespace Filters { | |
namespace Common { | ||
namespace Fault { | ||
|
||
FaultAbortConfig::FaultAbortConfig( | ||
const envoy::extensions::filters::http::fault::v3::FaultAbort& abort_config) | ||
: percentage_(abort_config.percentage()) { | ||
switch (abort_config.error_type_case()) { | ||
case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kHttpStatus: | ||
provider_ = std::make_unique<FixedAbortProvider>(abort_config.http_status()); | ||
break; | ||
case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::kHeaderAbort: | ||
provider_ = std::make_unique<HeaderAbortProvider>(); | ||
break; | ||
case envoy::extensions::filters::http::fault::v3::FaultAbort::ErrorTypeCase::ERROR_TYPE_NOT_SET: | ||
NOT_REACHED_GCOVR_EXCL_LINE; | ||
} | ||
} | ||
|
||
absl::optional<Http::Code> | ||
FaultAbortConfig::HeaderAbortProvider::statusCode(const Http::HeaderEntry* header) const { | ||
absl::optional<Http::Code> ret; | ||
if (header == nullptr) { | ||
return ret; | ||
} | ||
|
||
uint64_t code; | ||
if (!absl::SimpleAtoi(header->value().getStringView(), &code)) { | ||
return ret; | ||
} | ||
|
||
if (code >= 200 and code < 600) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: s/and/&& (some C++ compilers don't support it and it's not widely used), also please add some out of range tests. |
||
ret = static_cast<Http::Code>(code); | ||
} | ||
|
||
return ret; | ||
} | ||
|
||
FaultDelayConfig::FaultDelayConfig( | ||
const envoy::extensions::filters::common::fault::v3::FaultDelay& delay_config) | ||
: percentage_(delay_config.percentage()) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call this
header_error
. The abort is still controlled by the normal mechanisms, it's just the nature of the error is expressed in the header.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it but I think that
header_abort
is a better name because:header_error
can be in theory used to inject status codes such as200
or204
that are not errors.header_abort
follows a naming scheme used byheader_limit
andheader_delay
fields.header_error
would've been a better choice ifheader_limit
washeader_kbs
andheader_delay
washeader_duration
.Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would be inclined to stick with
header_abort
for the reasons that @Augustyniak outlines. @htuch WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough.