Add configuration to support broker SSL #156
Conversation
|
Interesting, it looks like |
|
I think it's OK, given that we also plan to break backwards compatibility for the producer and potentially the consumer. Maybe we can create a branch for the last commit that is 1.1 & 1.2 compatible? |
|
We should wait with merging before we can test this behavior as well; that will likely require the compiled release of 0.8.3 to be available for download. |
|
I like the idea of maintaining a compat branch for a while for old go versions, and the old consumer/producer API. Definitely waiting for something to test with before merging this, I just wrote it now because interfaces made it so darn easy :) |
eapache
force-pushed
the
support-ssl
branch
2 times, most recently
from
72c818b to
0480ee3
Compare
|
|
||
| TLS struct { | ||
| Enable bool // Whether or not to use TLS when connecting to the broker (defaults to false). | ||
| Config *tls.Config // The TLS configuration to use for secure connections if specified by UseTLS (defaults to nil). |
There was a problem hiding this comment.
godoc: UseTLS is probably an old name, and should be Enable?
There was a problem hiding this comment.
Good catch, this was left over from a rebase before unified config.
Possibly implements #154, if my assumptions about the implementation are correct.
|
@wvanbergen @uovobw Rebased this branch correctly and added a note about compat guarantees. If this looks good to everyone I'll merge it. |
|
🎉 Does it use a default config if you leave the config set to I'd like to wait for a confirmation that this works as expected for @uovobw before merging. |
|
|
@wvanbergen thanks for the trust but i no longer have the infrastructure in place to test this completely (since broker-originating connections do not support ssl) and currently do not have time to setup a local broker and test against this. if this merge is not urgent i think i can on this in a couple of days. thanks for the feedback guys |
|
@uovobw Given that you are the only person we know of that needs this feature, you can take your time :) |
Possibly implements #154, if my assumptions about the implementation are
correct.
@joestein @wvanbergen