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

BindAddr may be "::" without "[]" (supported) #2285

Closed
wants to merge 1 commit into from
Closed

BindAddr may be "::" without "[]" (supported) #2285

wants to merge 1 commit into from

Conversation

jovandeginste
Copy link
Contributor

@jovandeginste jovandeginste commented Aug 17, 2016

(alternative is to not allow -bind ::)

Using :: and [::] is practically equivalent (https://www.ietf.org/rfc/rfc2732.txt) depending on some context.

@slackpad slackpad added this to the 0.7.2 milestone Nov 18, 2016
@slackpad
Copy link
Contributor

@sean- can you plz. look at this one wrt. to go-sockaddr?

@sean-
Copy link
Contributor

sean- commented Dec 2, 2016

:: and [::] are semantically equivalent in go-sockaddr and should be in Consul, too, but...

Consul doesn't need to use square brackets when establishing an IPv6 default address. We could/should make this change, but I'd suggest we align this with the other non-BC changes for 0.8. We could/should also interpret :: and [::] to their semantic values before performing any comparisons in command/agent/agent.go. Doing this correctly is a larger, more invasive change than I think we want to introduce in a point release because someone could be relying on :: as a way of binding to IN_ADDRANY for v6 addresses and bypassing the call to consul.GetPublicIPv6(). And for the record, it is possible to -bind=:: right now:

$ consul agent -dev -bind="::" -client='::1' -advertise='::1'

The issue is if you don't specify an advertise address, Consul dies because you can't advertise an endpoint that's the IPv6 equiv of -advertise=0.0.0.0/32.

I'm also pretty indifferent, but would guess given AWS's announcement re: IPv6 today, I think this is going to be a more common source of toe-stubbing in the near future. The question is, how far do we want to go in terms of fixing this? I think we do it right, but that involves some non-BC changes.

TL;DR: go-sockaddr will parse to the semantically equiv value, but Consul is currently using the string representations and there are corner cases where the behavior could be different.

@slackpad slackpad modified the milestones: 0.7.4, 0.7.2 Dec 15, 2016
@slackpad slackpad removed this from the Triaged milestone Apr 18, 2017
@magiconair magiconair removed their assignment May 2, 2017
@magiconair
Copy link
Contributor

#2961 bans the use of an INADDR_ANY for the advertise address. Since we support "[::]" we should support "::" if they are equivalent. I can make this change once #3028 is merged. @sean- any objections?

@magiconair magiconair self-assigned this May 10, 2017
@sean-
Copy link
Contributor

sean- commented May 10, 2017

@magiconair None, go for it.

@magiconair magiconair added the type/enhancement Proposed improvement or new feature label May 15, 2017
magiconair added a commit that referenced this pull request May 15, 2017
magiconair added a commit that referenced this pull request May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants