Skip to content
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.

Sign up for GitHub

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

Do not resume on failure #158

Merged
merged 4 commits into from
Dec 5, 2024
Merged

Do not resume on failure #158

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb5136a
integration test: it_runs_next_action_on_failure_when_failuremode_is_…
eguzki Dec 4, 2024
b9df081
Use None in the test
adam-cattermole Dec 4, 2024
c9cc4dc
Do not resume on failure
adam-cattermole Dec 4, 2024
335f615
Add test case for failuremode deny
adam-cattermole Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 28 additions & 13 deletions src/filter/http_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ impl Filter {
}
}
}

fn process_next_op(&self) {
match self.operation_dispatcher.borrow_mut().next() {
Ok(some_op) => {
if some_op.is_none() {
// No more operations left in queue, resuming
self.resume_http_request();
}
}
Err(op_err) => {
// If desired, we could check the error status.
GrpcService::handle_error_on_grpc_response(op_err.failure_mode);
}
}
}
}

impl HttpContext for Filter {
Expand Down Expand Up @@ -134,21 +149,21 @@ impl Context for Filter {

match op_res {
Ok(operation) => {
if let Ok(result) = GrpcService::process_grpc_response(operation, resp_size) {
// add the response headers
self.response_headers_to_add.extend(result.response_headers);
// call the next op
match self.operation_dispatcher.borrow_mut().next() {
Ok(some_op) => {
if some_op.is_none() {
// No more operations left in queue, resuming
self.resume_http_request();
match GrpcService::process_grpc_response(Rc::clone(&operation), resp_size) {
Ok(result) => {
// add the response headers
self.response_headers_to_add.extend(result.response_headers);
// call the next op
self.process_next_op();
}
Err(_) => {
match operation.get_failure_mode() {
FailureMode::Deny => {}
FailureMode::Allow => {
// call the next op
self.process_next_op();
}
}
Err(op_err) => {
// If desired, we could check the error status.
GrpcService::handle_error_on_grpc_response(op_err.failure_mode);
}
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ impl GrpcService {
hostcalls::send_http_response(500, vec![], Some(b"Internal Server Error.\n"))
.expect("failed to send_http_response 500");
}
FailureMode::Allow => {
hostcalls::resume_http_request().expect("failed to resume_http_request")
}
FailureMode::Allow => {}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ fn it_auths() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -362,7 +362,7 @@ fn it_denies() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -563,7 +563,7 @@ fn it_does_not_fold_auth_actions() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -661,7 +661,7 @@ fn it_does_not_fold_auth_actions() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.execute_and_expect(ReturnType::None)
.unwrap();

Expand Down
170 changes: 170 additions & 0 deletions tests/failuremode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
use crate::util::common::wasm_module;
use proxy_wasm_test_framework::tester;
use proxy_wasm_test_framework::types::{Action, BufferType, LogLevel, MapType, ReturnType};
use serial_test::serial;

pub mod util;

#[test]
#[serial]
fn it_runs_next_action_on_failure_when_failuremode_is_allow() {
let args = tester::MockSettings {
wasm_path: wasm_module(),
quiet: false,
allow_unexpected: false,
};
let mut module = tester::mock(args).unwrap();

module
.call_start()
.execute_and_expect(ReturnType::None)
.unwrap();

let root_context = 1;
let cfg = r#"{
"services": {
"limitador": {
"type": "ratelimit",
"endpoint": "limitador-cluster",
"failureMode": "deny",
"timeout": "5s"
},
"limitador-unreachable": {
"type": "ratelimit",
"endpoint": "unreachable-cluster",
"failureMode": "allow",
"timeout": "5s"
}
},
"actionSets": [
{
"name": "some-name",
"routeRuleConditions": {
"hostnames": ["example.com"]
},
"actions": [
{
"service": "limitador-unreachable",
"scope": "a",
"data": [
{
"expression": {
"key": "l",
"value": "1"
}
}
]
},
{
"service": "limitador",
"scope": "a",
"data": [
{
"expression": {
"key": "l",
"value": "1"
}
}
]
}]
}]
}"#;

module
.call_proxy_on_context_create(root_context, 0)
.expect_log(Some(LogLevel::Info), Some("#1 set_root_context"))
.execute_and_expect(ReturnType::None)
.unwrap();
module
.call_proxy_on_configure(root_context, 0)
.expect_log(Some(LogLevel::Info), Some("#1 on_configure"))
.expect_get_buffer_bytes(Some(BufferType::PluginConfiguration))
.returning(Some(cfg.as_bytes()))
.expect_log(Some(LogLevel::Info), None)
.execute_and_expect(ReturnType::Bool(true))
.unwrap();

let http_context = 2;
module
.call_proxy_on_context_create(http_context, root_context)
.expect_log(Some(LogLevel::Debug), Some("#2 create_http_context"))
.execute_and_expect(ReturnType::None)
.unwrap();

let first_call_token_id = 42;
module
.call_proxy_on_request_headers(http_context, 0, false)
.expect_log(Some(LogLevel::Debug), Some("#2 on_http_request_headers"))
.expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority"))
.returning(Some("example.com"))
.expect_log(
Some(LogLevel::Debug),
Some("#2 action_set selected some-name"),
)
// retrieving tracing headers
.expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some("traceparent"))
.returning(None)
.expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some("tracestate"))
.returning(None)
.expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some("baggage"))
.returning(None)
.expect_grpc_call(
Some("unreachable-cluster"),
Some("envoy.service.ratelimit.v3.RateLimitService"),
Some("ShouldRateLimit"),
Some(&[0, 0, 0, 0]),
None,
Some(5000),
)
.returning(Ok(first_call_token_id))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
)
.execute_and_expect(ReturnType::Action(Action::Pause))
.unwrap();

let status_code = 14;
let second_call_token_id = 43;
module
.proxy_on_grpc_close(http_context, first_call_token_id as i32, status_code)
.expect_log(
Some(LogLevel::Debug),
Some(format!("#2 on_grpc_call_response: received gRPC call response: token: {first_call_token_id}, status: {status_code}").as_str()),
)
.expect_get_buffer_bytes(Some(BufferType::GrpcReceiveBuffer))
.returning(Some(&[]))
.expect_grpc_call(
Some("limitador-cluster"),
Some("envoy.service.ratelimit.v3.RateLimitService"),
Some("ShouldRateLimit"),
Some(&[0, 0, 0, 0]),
None,
Some(5000),
)
.returning(Ok(second_call_token_id))
.execute_and_expect(ReturnType::None)
.unwrap();

let grpc_response: [u8; 2] = [8, 1];
module
.call_proxy_on_grpc_receive(
http_context,
second_call_token_id as i32,
grpc_response.len() as i32,
)
.expect_log(
Some(LogLevel::Debug),
Some(format!("#2 on_grpc_call_response: received gRPC call response: token: {second_call_token_id}, status: 0").as_str()),
)
.expect_get_buffer_bytes(Some(BufferType::GrpcReceiveBuffer))
.returning(Some(&grpc_response))
.execute_and_expect(ReturnType::None)
.unwrap();

module
.call_proxy_on_response_headers(http_context, 0, false)
.expect_log(Some(LogLevel::Debug), Some("#2 on_http_response_headers"))
.execute_and_expect(ReturnType::Action(Action::Continue))
.unwrap();
}
10 changes: 5 additions & 5 deletions tests/multi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ fn it_performs_authenticated_rate_limiting() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -225,7 +225,7 @@ fn it_performs_authenticated_rate_limiting() {
None,
Some(5000),
)
.returning(Some(43))
.returning(Ok(43))
.execute_and_expect(ReturnType::None)
.unwrap();

Expand Down Expand Up @@ -393,7 +393,7 @@ fn unauthenticated_does_not_ratelimit() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -653,7 +653,7 @@ fn authenticated_one_ratelimit_action_matches() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -695,7 +695,7 @@ fn authenticated_one_ratelimit_action_matches() {
None,
Some(5000),
)
.returning(Some(43))
.returning(Ok(43))
.execute_and_expect(ReturnType::None)
.unwrap();

Expand Down
8 changes: 4 additions & 4 deletions tests/rate_limited.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fn it_limits() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -326,7 +326,7 @@ fn it_passes_additional_headers() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -459,7 +459,7 @@ fn it_rate_limits_with_empty_predicates() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down Expand Up @@ -697,7 +697,7 @@ fn it_folds_subsequent_actions_to_limitador_into_a_single_one() {
None,
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down
2 changes: 1 addition & 1 deletion tests/remote_address.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn it_limits_based_on_source_address() {
]),
Some(5000),
)
.returning(Some(42))
.returning(Ok(42))
.expect_log(
Some(LogLevel::Debug),
Some("#2 initiated gRPC call (id# 42)"),
Expand Down
Loading