Add tls support to frontend connection #6
Conversation
|
@denniskniep hello, have you had the chance to look on this PR? This feature is much needed for our hosted temporal deployment. |
|
Thanks for contributing this. I will look in detail at it soon. Can you add in the meantime related tests?
|
mihaelabalas84
force-pushed
the
add-tls
branch
from
8cbe14e
to
7557d42
Compare
hi @denniskniep Added TLS tests and docker compose setup, test certificates, updated Readme |
mihaelabalas84
force-pushed
the
add-tls
branch
from
c59e0c0
to
9d814a2
Compare
internal/clients/service.go
Outdated
| logger.Debug("Loading client certificate from strings") | ||
| cert, err := tls.X509KeyPair([]byte(conf.CertFile), []byte(conf.KeyFile)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to load client certificate: %w", err) |
There was a problem hiding this comment.
can you plz use errors.Wrap(err, "some detail err msg") (also in the other occurences)
Because it's the default for crossplane providers (see here )
We could think about changing this, because this lib's repo is archived. But then we should change also all other occurrences so that we use the same approach in the entire project.
There was a problem hiding this comment.
@mihaelabalas84 can you also address this comment ?
Else it looks good to me
There was a problem hiding this comment.
done @denniskniep
|
BTW: Unit Tests are failing, I think we must use |
|
@denniskniep I addressed all your comments and pushed new commit with them. |
|
@mihaelabalas84 thanks for this contribution 🚀 |
This PR adds TLS connection support (optional) to Temporal frontend that has TLS enabled.
closes #7