Skip to content
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

Don't advertise a '::' bindAddr #198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Freeaqingme
Copy link

@Freeaqingme Freeaqingme commented Aug 24, 2019

If no explicit advertiseAddr is specified and we're bound
to a wildcard address, we don't want to advertise the wildcard
address if it's '::'.

Instead of checking for 0.0.0.0 use the Go stdlib to select
for all possible wild card addresses

If no no explicit advertiseAddr is specified and we're bound
to a wildcard address, we don't want to advertise the wildcard
address if it's '::'.

Instead of checking for 0.0.0.0 use the Go stdlib to select
for all possible wild card addresses
@hashicorp-cla
Copy link

hashicorp-cla commented Aug 24, 2019

CLA assistant check
All committers have signed the CLA.

@Freeaqingme
Copy link
Author

@mkeeler May I gently bring this PR to your attention? :)

Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good to me. Could you add a test in net_test.go for FinalAdvertiseAddr ignoring both 0.0.0.0 and ::? I went looking for an existing test to update but apparently there are none for this function. I don't think you need to test all the existing functionality of providing a specific advertise addr or but rather just ensure that if a normal IP is specified as the bind addr it is used and if its unspecified that some other address is chosen. You wont be able to check for any specific IP as the sockaddr library is going to choose that IP based on the local system. So basically just assert that the advertised address is not 0.0.0.0 or ::

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants