search

dapr / proposals

Proposals for new features in Dapr
Apache License 2.0
15 stars 33 forks source link

Dapr endpoint env and TLS support in SDKs #27

Closed artursouza closed 1 year ago

XavierGeerinck commented 1 year ago

+1, I like this idea!

shubham1172 commented 1 year ago

+1 binding

yaron2 commented 1 year ago

+1 binding

artursouza commented 1 year ago

+1 binding

halspang commented 1 year ago

+1 binding

ItalyPaleAle commented 1 year ago

@artursouza @yaron2 sorry for reopening the conversation here. Working on the injector reminded me that we also have support for Unix Domain Sockets when using gRPC.

Should we expose that somehow using the env vars added here? for example DAPR_GRPC_ENDPOINT set to a path to a UDS? (That may require coordination with SDKs)

JoshVanL commented 1 year ago

@artursouza @yaron2 Similarly sorry for also re-pinging a closed proposal :smile:

I have just now noticed that the gRPC endpoint environment variable accepts a scheme- specifically https://. Semantically- this is incorrect, and as @ItalyPaleAle alluded to above, would be incompatible when attempting to use another transport (e.g. UDP) at the same time. The gRPC documentation is clear on what schemes can be used and HTTP/HTTPS isn't defined because it doesn't make much sense in the context of resovlers.

I would have expected no scheme to default to DNS, and no port or a port of 443 to default to using TLS. In other cases, beside port 80, the clients would optimistically attempt TLS and fall back to insecure (or some other agreed upon behavior). Having a SECURE and INSECURE gRPC env var, preferring SECURE, as another option. I think misusing https:// here to signal to the client to use TLS is confusing for users, be incompatible with their use case, and potentially put off potential new users to dapr.

artursouza commented 1 year ago

Let's discuss this. The gRPC client for .Net uses this environment variable as-is without the Dapr SDK requiring any parsing.

So, we need to understand why .Net operates like that but we cannot.

JoshVanL commented 1 year ago

As I understand, the grpc-dotnet project has decided to use the http/https schemes to signal whether to enable TLS or not (same as this proposal) as a deliberate design decision to be consistent with another client network library. I'm assuming (?) that the library only supports DNS, and not other resolvers (or resolver plugins). This has caused some user confusion there.