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

Return SOA/NXDOMAIN when the answer is empty #1195

Merged
merged 1 commit into from
Sep 1, 2015

Conversation

42wim
Copy link
Contributor

@42wim 42wim commented Aug 25, 2015

When a service has A records and no AAAA records (or vice versa), also return SOA and NXDOMAIN when asking for the AAAA records. (wrt RFC2308 and negative caching)

See PR #995 for more info

@ryanuber
Copy link
Member

I'm a little out of context here - but it appears this is already handled starting at line 504. Are there additional edge cases that this patches up?

I'd also like to see some test code for this to make sure we are covered and to clarify what we are fixing.

@42wim
Copy link
Contributor Author

42wim commented Aug 31, 2015

(it's not really an edge case in a dual-stack/ipv6 world :-)
At line 504 it only handles the service 'globally'.

The problem is when you ask for the IN A (and there is only an IN AAAA record) or the IN AAAA (and there is only an IN A record) of the service.

For example, you've got a service with only an IPv4 address: test.service.consul IN A 1.2.3.4
Now if you do

dig IN AAAA test.service.consul

It will pass line 504, cause you've got a service node (ipv4 with IN A 1.2.3.4), but the service node has no IPv6 addresses. (which means the resp.Answer will be 0). If that's the case, you will also need to respond with SOA/NXDOMAIN.

Let me know if this clarifies the issue

@42wim
Copy link
Contributor Author

42wim commented Sep 1, 2015

Added a test for the fix

ryanbreen added a commit that referenced this pull request Sep 1, 2015
Return SOA/NXDOMAIN when the answer is empty
@ryanbreen ryanbreen merged commit f41b79e into hashicorp:master Sep 1, 2015
@ryanbreen
Copy link
Contributor

LGTM!

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