Kuadrant / wasm-shim

A Proxy-Wasm module allowing communication to Authorino and Limitador.
Apache License 2.0
5 stars 5 forks source link

when expression in unknown, connection hangs #139

Open eguzki opened 2 weeks ago

eguzki commented 2 weeks ago

When expression is something like:

expression:
  key: "a"
  value: "unknown.path"

The wasm panics. Envoy's logs show

envoy-1  | [2024-11-08 15:00:30.432][61][debug][wasm] [source/extensions/common/wasm/context.cc:1192] wasm log kuadrant_wasm kuadrant_wasm vm.sentinel.kuadrant_wasm: get_property: path: ["unknown", "path"]
envoy-1  | [2024-11-08 15:00:30.432][61][error][wasm] [source/extensions/common/wasm/context.cc:1201] wasm log kuadrant_wasm kuadrant_wasm vm.sentinel.kuadrant_wasm: Failed to evaluate unknown.path: UndeclaredReference("unknown")
envoy-1  | [2024-11-08 15:00:30.432][61][critical][wasm] [source/extensions/common/wasm/context.cc:1204] wasm log kuadrant_wasm kuadrant_wasm vm.sentinel.kuadrant_wasm: panicked at src/configuration/action.rs:79:29:
envoy-1  | Err out of this!
envoy-1  | [2024-11-08 15:00:30.433][61][error][wasm] [source/extensions/common/wasm/wasm_vm.cc:38] Function: proxy_on_request_headers failed: Uncaught RuntimeError: unreachable
envoy-1  | Proxy-Wasm plugin in-VM backtrace:
envoy-1  |   0:  0x92b799 - __rust_start_panic
envoy-1  |   1:  0x92b73c - rust_panic
envoy-1  |   2:  0x92b641 - std::panicking::rust_panic_with_hook::h33fe77d38d305ca3
envoy-1  |   3:  0x92ab5f - std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::h98de848d678bad07
envoy-1  |   4:  0x92aa9e - std::sys::backtrace::__rust_end_short_backtrace::h2bcfc60c3cf0a312
envoy-1  |   5:  0x92b1d2 - rust_begin_unwind
envoy-1  |   6:  0x92c5db - core::panicking::panic_fmt::hde8b7aa66e2831e1
envoy-1  |   7:  0xd75a - wasm_shim::configuration::action::Action::build_single_descriptor::hd83b4cb300b5cd88
envoy-1  |   8:  0xc2d4 - wasm_shim::configuration::action::Action::build_descriptors::he5f043d9fd30ec0a
envoy-1  |   9:  0x2378ec - wasm_shim::service::grpc_message::GrpcMessageRequest::new::h5c57b41bf8d3bc03
envoy-1  | [2024-11-08 15:00:30.433][61][debug][http] [source/common/http/filter_manager.cc:1025] [Tags: "ConnectionId":"2","StreamId":"11090475888245840067"] Preparing local reply with details wasm_fail_stream
envoy-1  | [2024-11-08 15:00:30.433][61][debug][http] [source/common/http/filter_manager.cc:1067] [Tags: "ConnectionId":"2","StreamId":"11090475888245840067"] Executing sending local reply.

And the curl triggering the request hangs and never returns.

curl -v --resolve test.example.com:18000:127.0.0.1 "http://test.example.com:18000"
* Added test.example.com:18000:127.0.0.1 to DNS cache
* Hostname test.example.com was found in DNS cache
*   Trying 127.0.0.1:18000...
* Connected to test.example.com (127.0.0.1) port 18000
> GET / HTTP/1.1
> Host: test.example.com:18000
> User-Agent: curl/8.5.0
> Accept: */*
>
alexsnaps commented 1 week ago

That's sort of an expected behavior tho... what would you rather see? We (currently) have no way of reporting this back up to the user (or policy author). Also, I don't think this should use the failureMode setting, as this isn't failure but a user error.

eguzki commented 1 week ago

well, certainly, it should not panic and block the request. Maybe return 500 Internal Server Error. Maybe, if it's in a predicate, evaluate to false. Maybe, if it's in a expression to generate a descriptor entry, just do not add the descriptor entry.

Too many Maybe's, I known.

Wasm module can only evaluate at request time. But it is not downstream client's fault if the expression is incorrect. It is policy owner's fault.

My take? 500 Internal Server Error.

alexsnaps commented 1 week ago

502 or 503 maybe?