client support tls connections #40
Conversation
|
The change looks fine to me. Alternatively, could we enable TLS whenever we see that the address is HTTPS (and not HTTP), and not introduce the flag? |
I agree, that solution would also be fine. However it would require parsing logic similar to this https://github.com/dapr/go-sdk/blob/main/client/internal/parse.go#L31 in order to support the many ways in which tls can be specified in grpc. What I did in this PR is much simpler |
|
It just occurred to me that the endpoint configuration doesn't include the scheme (like http or https), so you can't really use that for a TLS hint. I'm okay with this as a simple enough solution. |
|
Looks like the test and build pipeline is failing. @kaibocai, is this something you're able to take a look at? |
|
Hi @famarting, thanks for your contribution. Can you try rebasing your PR against the main branch? We had a fix on the CI let's see if that makes the CI green for your PR. Thanks. |
famarting
force-pushed
the
client-support-tls
branch
from
25325f4
to
9a23571
Compare
|
updated |
|
Hi @cgillum, I fixed that in #42 when there was a timeout issue in the test. Hi @famarting, Amazing! It's passed now! 🥳 Hi @kaibocai, I think this PR can move forward as it gets updated. 😊 |
|
Hi all! Thank you for merging the PR, my intention was to use this change in the Dapr js sdk to implement a new feature. Could any of you please create a new release of this library so I can use it from the Dapr js sdk? |
This PR adds support for tls connections. The change is made in a non breaking manner by adding the parameter as an optional and at the end of the constructors, so existing client instantiations will continue compiling without problems.
The idea for this change is to use it in the Dapr js SDK for workflows... currently the normal Dapr js sdk supports TLS connections but the workflows SDK (based on durabletask) does not support TLS https://github.com/dapr/js-sdk/blob/main/src/workflow/client/DaprWorkflowClient.ts#L48