-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configurable Consul Service Address #3971
Configurable Consul Service Address #3971
Conversation
Setting an explicit service address eliminates the ability for Consul to dynamically decide what it should be based on its translate_wan_addrs setting. translate_wan_addrs configures Consul to return its lan address to nodes in its same datacenter but return its wan address to nodes in foreign datacenters.
I'm very wary of this change as it may well break things for other people. I think the right way to go would be to instead add a |
@jefferai yeah, that's a good call. Thanks for the feedback. I'll see if I can get that worked out 👍 |
This parameter allows users to override the use of what Vault knows to be its HA redirect address. This option is particularly commpelling because if set to a blank string, Consul will leverage the node configuration where the service is registered which includes the `translate_wan_addrs` option. This option conditionally associates nodes' lan or wan address based on where requests originate.
Ensures that the service_address configuration parameter is setting the serviceAddress field of ConsulBackend instances properly. If the "service_address" parameter is not set, the ConsulBackend serviceAddress field must instantiate as nil to indicate that it can be ignored.
@jefferai I think this is ready for another review. Something to note is that the service ID will always use the convention, |
physical/consul/consul.go
Outdated
serviceAddrStr, ok := conf["service_address"] | ||
if ok { | ||
serviceAddr = &serviceAddrStr | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This else block is unnecessary as nil is the default value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jefferai thanks! Just removed the else block 👍
*string variables are inherently nil when instantiated so there is no need to explicitly set serviceAddr to nil.
Thanks! |
* oss/master: (35 commits) helper/gpgkeys: fix for vault 1.10 (#4038) Move local cluster parameters to atomic values to fix some potential data races (#4036) Port some replicated cluster changes from ent (#4037) Add core object to policy store for some ent uses changelog++ Configurable Consul Service Address (#3971) Fix certutil test Fixed a broken link (#4032) Update comment to replication consts Add a helpful comment to replication consts changelog++ auth/aws: Add functional test for detached RSA signature (#4031) Change Go min version check changelog++ Revert Go dep to 1.9 *Partially* revert "Remove now-unneeded PKCS8 code and update certutil tests for Go 1.10" Revert "Remove unneeded looping since Go 1.10 cover it already (#4010)" Bump pkcs7 library version to fix #4024 Revert "Switch to a forked copy of pkcs7 to fix aws pkcs7 verification error (#4024)" changelog++ ...
Setting an explicit service address makes Consul ignore the
translate_wan_addrs
setting. When set, Consul nodes will return their WAN address on queries from foreign datacenters. This seems to me to be a nice option to have instead of defining theredirectHost
as the service-specific address.This behavior is mentioned here: hashicorp/consul#2390 (comment).