watch: use CONSUL_TLS_SERVER_NAME for https if defined (#4718) #4727
Conversation
There was a problem hiding this comment.
@igal-s Thanks for the PR. As my other comment suggested I think there may be an even simpler solution here.
Within the APIConfig function if we do not set the TLSConfig.Address at all then when it eventually gets passed to api.NewClient in the RunWithConfig function of the watch plan it should use the env var. I think overriding the addr within the reloadWatches function will only work so long as the TLS server name can be resolved to the address consul is serving the HTTP api on. So to account for the other cases where the server name only resolves to a public IP and the HTTP is listening only on other private/local IPs we need to make sure that the addr passed to the watch plan is an actual IP address and not a hostname.
Could you test out that theory
agent/config/runtime.go
Outdated
| @@ -1475,7 +1476,7 @@ func (c *RuntimeConfig) APIConfig(includeClientCerts bool) (*api.Config, error) | |||
| cfg.Scheme = "https" | |||
| cfg.TLSConfig.CAFile = c.CAFile | |||
| cfg.TLSConfig.CAPath = c.CAPath | |||
| cfg.TLSConfig.Address = httpsAddr | |||
| cfg.TLSConfig.Address = dcfg.TLSConfig.Address | |||
There was a problem hiding this comment.
I think this whole PR may be able to be replaced by just removing this line.
|
@mkeeler you are right, I've tested it and not setting the TLSConfig.Address in the APIConfig function resolves the issue as well. |
I'm not sure that this is the best way of passing the configuration but it does resolve my issue