proxy-wasm-cpp-sdk icon indicating copy to clipboard operation
proxy-wasm-cpp-sdk copied to clipboard

why `proxy_wasm_intrinsics.proto` is necessary for this SDK?

Open wbpcode opened this issue 11 months ago • 4 comments

Hi, community, I have some questions about this SDK.

It's typically that the proxy_wasm_intrinsics.proto should be a proto lib of specific plugin and this SDK never use them. It shouldn't be a part of sdk self. And if bazel is used, why we always need to generate and store the pb.h/pb.cc?

Any responses would be appreciated 🙏

wbpcode avatar Feb 26 '25 09:02 wbpcode

cc @mpwarres

wbpcode avatar Feb 26 '25 09:02 wbpcode

Ideally, I think we can remove all these unnecessary proto and pb.h/pb.cc. And then try to make the SDK a thin SDK with only multiple files.

wbpcode avatar Feb 26 '25 09:02 wbpcode

proxy_wasm_intrinsics.proto contains the gRPC definition of Envoy's GrpcService, which can be passed as the service argument in proxy_grpc_call and proxy_grpc_stream calls to configure the destination gRPC service at runtime, instead of relying on the host environment to have it pre-configured and exposed by name only.

I'm all in for removing it (along with the ability to pass opaque data as a service), and I refused to document it and/or add this capability to the Rust SDK, since it's effectively an undocumented backdoor in the C++ SDK and Envoy, that breaks both: the portability across proxies, and the sandboxing promise that the plugins are only allowed to communicate with destinations exposed by the host environment.

However, this was used by Istio's Stackdriver extension until the last year (see: https://github.com/istio/proxy/pull/5550), and it's most likely used by other consumers, so it would be a breaking change.

PiotrSikora avatar Feb 26 '25 10:02 PiotrSikora

However, this was used by Istio's Stackdriver extension until the last year (see: https://github.com/istio/proxy/pull/5550), and it's most likely used by other consumers, so it would be a breaking change.

This change won't break our ABI. It only force the developers to introduce the necessary proto by them self if they updated the SDK to new version. So, I think this is fine.

I'm all in for removing it (along with the ability to pass opaque data as a service), and I refused to document it and/or add this capability to the Rust SDK, since it's effectively an undocumented backdoor in the C++ SDK and Envoy, that breaks both: the portability across proxies, and the sandboxing promise that the plugins are only allowed to communicate with destinations exposed by the host environment.

Although I am ok to add envoy specific feature to proxy-wasm (considering my position), but I also agree it's pretty weird to use this way to pass the host-specific data to host.

wbpcode avatar Feb 26 '25 11:02 wbpcode