proxy-wasm / spec

WebAssembly for Proxies (ABI specification)
Apache License 2.0
544 stars 27 forks source link

Offline validation for config passed to proxy wasm #35

Open jcchavezs opened 2 years ago

jcchavezs commented 2 years ago

proxy-wasm extensions allows people to extend proxy functionalities in the request/response lifecycle and are deployed along with proxies but have their own lifecycle.

One big issue right now is that the config passed to those extensions are mainly validated in runtime, meaning that user can't have feedback about incorrect config before deployment have been attempted for example:

A new proxy_validate_config method could be added to the ABI:

proxy_validate_config
params:
i32 (uint32_t) root_context_id
i32 (size_t) plugin_configuration_size
returns:
i32 (proxy_result_t) call_result

the result could be a list of errors e.g. [LINE 1] Unexpected token {or going forward a flatmap with the location e.g. .rules[0] but that assumes all configs are structured which I am not 100% sure.

Ping @mathetake @PiotrSikora

Related: https://github.com/corazawaf/coraza-proxy-wasm/pull/88#issuecomment-1318333776

mathetake commented 2 years ago

even if this function is added, Envoy/Istio has no way to report back to the users. Plus this seems essentially the same as the return value from the existing proxy_on_configure ABI. I suggest instead we just instantiate plugins at the control plane layer, and see the return value from proxy_on_configure

jcchavezs commented 2 years ago

Yeah, sorry for not being clear. This function has not the purpose of being loaded by istio/envoy but by control planes. Also as for loading the extension and attempt to start the plugin, that was my initial ide about the output (error message) isn't the best one.

On Mon, 21 Nov 2022, 04:04 Takeshi Yoneda, @.***> wrote:

even if this function is added, Envoy/Istio has no way to report back to the users. Plus this seems essentially the same as the return value from the existing proxy_on_configure ABI. I suggest instead we just instantiate plugins at the control plane layer, and see the return value from proxy_on_configure

— Reply to this email directly, view it on GitHub https://github.com/proxy-wasm/spec/issues/35#issuecomment-1321391430, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAV5YTJ7DI55ZW3IC7DWJLRC5ANCNFSM6AAAAAASDHFXUI . You are receiving this because you authored the thread.Message ID: @.***>

PiotrSikora commented 2 years ago

The original ABI had a function exactly like that, and Proxy-Wasm C++ SDK still has it (see: proxy_validate_configuration, RootContext::validateConfiguration), but I'm not aware of any plugins using it.

Note there are a few problems with such approach: 1) proxy_validate_configuration might be only slightly different from proxy_on_configure(configuration), which can result in code duplication and both functions could easily get out-of-sync over time, 2) some plugins require access to runtime variables (e.g. environment variables or host metadata), which control plane might not know about, 3) some plugins fetch parts of configuration from remote HTTP/gRPC endpoints at startup.

Basically, proxy_validate_configuration might catch some errors (e.g. parsing), but it cannot catch all of them, and it could introduce false negatives, so it's not a replacement for the proper solution (i.e. Envoy rejecting the configuration, and Istio propagating this information back to the operator).

PiotrSikora commented 2 years ago

Another consideration here is that we access to those content types that are being inspected and only buffer when inspection.

Could you elaborate? I don't understand how this is related to control plane performing config validation.

jcchavezs commented 2 years ago

proxy_validate_configuration might be only slightly different from proxy_on_configure(configuration), which can result in code duplication and both functions could easily get out-of-sync over time

Agree and I think the main difference between the two is the output but functionality wise they are quite similar.

some plugins require access to runtime variables (e.g. environment variables or host metadata), which control plane might not know about,some plugins fetch parts of configuration from remote HTTP/gRPC endpoints at startup.

Agree, this is mainly a best effort to allow users to validate the config offline.

So providing the method in the wasm extension isn't a perfect solution it just guarantees cohesion meaning that the same wasm that is running in prod is the one validating the config as opposed to provide another wasm (to be able to load it in different languages) or per language libraries (mostly limited to the wasm extension language)