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.

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

why proxy_wasm_intrinsics.proto is necessary for this SDK? #183

Open
wbpcode opened this issue Feb 26, 2025 · 4 comments · May be fixed by #184
Open

why proxy_wasm_intrinsics.proto is necessary for this SDK? #183

wbpcode opened this issue Feb 26, 2025 · 4 comments · May be fixed by #184

Comments

@wbpcode
Copy link

wbpcode commented Feb 26, 2025

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
Copy link
Author

wbpcode commented Feb 26, 2025

cc @mpwarres

@wbpcode
Copy link
Author

wbpcode commented Feb 26, 2025

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.

@PiotrSikora
Copy link
Member

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: istio/proxy#5550), and it's most likely used by other consumers, so it would be a breaking change.

@wbpcode
Copy link
Author

wbpcode commented Feb 26, 2025

However, this was used by Istio's Stackdriver extension until the last year (see: istio/proxy#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants