Merge pull request #39 from Kuadrant/fix-not-existing-property

Fix return value when host property does not exist from data.selector

src/filter/http_context.rs

@@ -164,8 +164,7 @@ impl Filter {
  self.context_id, selector_item.selector
  );
  match &selector_item.default {
- // skipping the entire descriptor
- None => "".to_string(),
+ None => return None, // skipping the entire descriptor
  Some(default_value) => default_value.clone(),
  }
  }
@@ -175,8 +174,7 @@ impl Filter {
  Err(e) => {
  debug!(
  "[context_id: {}]: failed to parse selector value: {}, error: {}",
- self.context_id, selector_item.selector, e
- );
+ self.context_id, selector_item.selector, e);
  return None;
  }
  Ok(attribute_value) => attribute_value,

tests/rate_limited.rs

@@ -342,3 +342,188 @@ fn it_passes_additional_headers() {
  .execute_and_expect(ReturnType::Action(Action::Continue))
  .unwrap();
 }
+
+#[test]
+#[serial]
+fn it_rate_limits_with_empty_conditions() {
+ 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#"{
+ "failureMode": "deny",
+ "rateLimitPolicies": [
+ {
+ "name": "some-name",
+ "domain": "RLS-domain",
+ "service": "limitador-cluster",
+ "hostnames": ["*.com"],
+ "rules": [
+ {
+ "data": [
+ {
+ "static": {
+ "key": "admin",
+ "value": "1"
+ }
+ }
+ ]
+ }]
+ }]
+ }"#;
+
+ module
+ .call_proxy_on_context_create(root_context, 0)
+ .expect_log(Some(LogLevel::Info), Some("set_root_context #1"))
+ .execute_and_expect(ReturnType::None)
+ .unwrap();
+ module
+ .call_proxy_on_configure(root_context, 0)
+ .expect_log(Some(LogLevel::Info), Some("on_configure #1"))
+ .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::Info), Some("create_http_context #2"))
+ .execute_and_expect(ReturnType::None)
+ .unwrap();
+
+ module
+ .call_proxy_on_request_headers(http_context, 0, false)
+ .expect_log(Some(LogLevel::Info), Some("on_http_request_headers #2"))
+ .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority"))
+ .returning(Some("a.com"))
+ .expect_grpc_call(
+ Some("limitador-cluster"),
+ Some("envoy.service.ratelimit.v3.RateLimitService"),
+ Some("ShouldRateLimit"),
+ Some(&[0, 0, 0, 0]),
+ Some(&[
+ 10, 10, 82, 76, 83, 45, 100, 111, 109, 97, 105, 110, 18, 12, 10, 10, 10, 5, 97,
+ 100, 109, 105, 110, 18, 1, 49, 24, 1,
+ ]),
+ Some(5000),
+ )
+ .returning(Some(42))
+ .expect_log(
+ Some(LogLevel::Info),
+ Some("Initiated gRPC call (id# 42) to Limitador"),
+ )
+ .execute_and_expect(ReturnType::Action(Action::Pause))
+ .unwrap();
+
+ let grpc_response: [u8; 2] = [8, 1];
+ module
+ .call_proxy_on_grpc_receive(http_context, 42, grpc_response.len() as i32)
+ .expect_log(
+ Some(LogLevel::Info),
+ Some("on_grpc_call_response #2: received gRPC call response: token: 42, status: 0"),
+ )
+ .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("on_http_response_headers #2"))
+ .execute_and_expect(ReturnType::Action(Action::Continue))
+ .unwrap();
+}
+
+#[test]
+#[serial]
+fn it_does_not_rate_limits_when_selector_does_not_exist_and_misses_default_value() {
+ 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#"{
+ "failureMode": "deny",
+ "rateLimitPolicies": [
+ {
+ "name": "some-name",
+ "domain": "RLS-domain",
+ "service": "limitador-cluster",
+ "hostnames": ["*.com"],
+ "rules": [
+ {
+ "data": [
+ {
+ "selector": {
+ "selector": "unknown.path"
+ }
+ }
+ ]
+ }]
+ }]
+ }"#;
+
+ module
+ .call_proxy_on_context_create(root_context, 0)
+ .expect_log(Some(LogLevel::Info), Some("set_root_context #1"))
+ .execute_and_expect(ReturnType::None)
+ .unwrap();
+ module
+ .call_proxy_on_configure(root_context, 0)
+ .expect_log(Some(LogLevel::Info), Some("on_configure #1"))
+ .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::Info), Some("create_http_context #2"))
+ .execute_and_expect(ReturnType::None)
+ .unwrap();
+
+ module
+ .call_proxy_on_request_headers(http_context, 0, false)
+ .expect_log(Some(LogLevel::Info), Some("on_http_request_headers #2"))
+ .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority"))
+ .returning(Some("a.com"))
+ .expect_get_property(Some(vec!["unknown", "path"]))
+ .returning(None)
+ .expect_log(
+ Some(LogLevel::Debug),
+ Some("[context_id: 2]: selector not found: unknown.path"),
+ )
+ .expect_log(
+ Some(LogLevel::Debug),
+ Some("[context_id: 2] empty descriptors"),
+ )
+ .execute_and_expect(ReturnType::Action(Action::Continue))
+ .unwrap();
+
+ module
+ .call_proxy_on_response_headers(http_context, 0, false)
+ .expect_log(Some(LogLevel::Debug), Some("on_http_response_headers #2"))
+ .execute_and_expect(ReturnType::Action(Action::Continue))
+ .unwrap();
+}