Consider changing the default for HTTPS to true on Consul.Builder, or remove it #318
sleberknight
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
If you do
Consul.builder().build()
you get an instance that uses only HTTP.You must do this to get HTTPS:
We should consider changing the default to be secure instead of insecure.
In addition, the
withHttps
method changes thescheme
and possiblyurl
fields, if already set bywithMultipleHostAndPort
.Unfortunately, I think there is actually a subtle 🐛 here when
withMultipleHostAndPort
is used in combination with a different value forwithHttps
.For example, suppose the
Collection<HostAndPort>
passed towithMultipleHostAndPort
are all insecure HTTP URLs, butwithHttps(true)
is used. In this case,withHttps
will set thescheme
to"https"
and change theurl
field to a new URL with the scheme as"https"
. But, it will not change anything in theCollection<HostAndPort>
. If there is a connection error and a failover occurs, it will use the original URLs in theCollection<HostAndPort>
, and thus will attempt the failover connection to an insecure HTTP URL!Since this is a lot of complexity to change things that are already configured and having to reach into separate objects and change their internals, it might be better to simply remove the
withHttps
method, use "https" as the default, and put the onus on code that constructs theConsul
instance to set the appropriate URLs (e.g., use "https" in all of them for secure connections).For example, if you want only secure connections:
The main question to answer before doing this is what impact it will have to existing code, to simply change the default. While it really should default to secure, if it requires a lot of modifications to existing code, maybe it's not worth it to make the change.
Beta Was this translation helpful? Give feedback.
All reactions