Resolve unqualified names in the local WeaveDNS domain #1050

inercia · 2015-06-30T13:23:01Z

Fixes #987

nameserver/names.go

+// normalize a name by
+// a) adding the local domain when it is a single component name
+// b) converting the result to FQDN
+func nameNormalize(n string, domain string) string {


rade · 2015-07-01T10:32:00Z

I think we want some smoke test for this.

Would like to hear from @bboreham and @awh about any gotchas to this 'hack'.

bboreham · 2015-07-01T10:38:59Z

It seems plausible to me that a user has some set-up where they are already using unqualified names in their environment outside of Docker. So whilst I agree with the premise of #987, it might be safer to have an option to disable this new behaviour.

inercia · 2015-07-01T10:40:30Z

@bboreham suggestion seems reasonable. I will add a command line flag...

rade · 2015-07-01T10:47:20Z

Nooooooo! Not more command line flags! We can think about adding that when we have a user running into problems because of it.

inercia · 2015-07-01T10:50:55Z

Ok, then no more command line flags...

nameserver/server.go

@@ -372,9 +369,19 @@ func (s *DNSServer) localHandler(proto dnsProtocol, kind string, qtype uint16,
 func (s *DNSServer) notUsHandler(proto dnsProtocol) dns.HandlerFunc {
 	dnsClient := proto.GetNewClient(DefaultUDPBuflen, s.timeout)

+	localNameResolver := s.localHandler(proto, "Query", dns.TypeA,
+		s.Zone.DomainLookupName, makeAddressReply, s.Zone.ObserveName, failHandleFunc)
+


awh · 2015-07-01T13:00:33Z

Would like to hear from @bboreham and @awh about any gotchas to this 'hack'.

It will prevent fallback lookup of TLDs. Probably not common use case though.

inercia · 2015-07-01T13:03:13Z

That's right: dig .com would be equivalent to dig com.weave.local., but, as @awh mentioned, it is not a big deal...

rade · 2015-07-02T15:47:37Z

@inercia anything left to do here

inercia · 2015-07-02T16:27:16Z

If there are no more comments on your side, I think I'm done with it...

rade · 2015-07-02T16:43:22Z

I think I'm done with it...

ok. next time please assign back to me.

test/230_dns_unqualified_test.sh

+# check that unqualified names are automatically qualified when looking up
+weave_on $HOST1 dns-add $C2 c2 -h name1.$DOMAIN
+assert_dns_a_record $HOST1 c2 name1 $C2 name1.$DOMAIN
+


inercia · 2015-07-03T13:35:05Z

Seems that alpine will not even add a . at the end of a short name like redis, so our DNS server mux is not catching those names. When a user does a getent hosts redis in alpine, our DNS server will receive a query about just redis. Not redis., let alone redis.weave.local. And the problem is that we cannot install a handler in our DNS library for that kind of names...

rade · 2015-07-03T13:45:30Z

the problem is that we cannot install a handler in our DNS library for that kind of names...

why not?

inercia · 2015-07-03T13:47:23Z

Because the minimum pattern seems to be .

rade · 2015-07-03T15:02:05Z

@awh @bboreham any idea what to do with this?

rade · 2015-07-06T17:23:23Z

@awh @bboreham ping

awh · 2015-07-07T09:50:21Z

Raise an issue against miekg/dns to allow the empty string so it can serve broken resolvers? I had a look for musl's issue tracker to see if this is getting fixed soon enough for us to ignore it, there doesn't appear to be one...

bboreham · 2015-07-07T10:01:54Z

Doesn't this line mean that pattern "." will match a string like "redis" ?

inercia · 2015-07-07T10:05:36Z

@bboreham I'm think it doesn't... . is just the minimum FQDN name.

bboreham · 2015-07-07T10:23:01Z

I added a debug print at the top of notUsHandler and tried your suggested test; it seems to get the period appended:

$ sudo weave reset
$ sudo weave launch-router
a6d98c4c87518cd78c46e26cac11c6bc04d09e83a98b301ee6498f0c979dc9a5
$ sudo weave launch-dns -debug
027cc73866fae420fb08ac0660bffde24cec3cb306be6f64381ab45ec3c9378c
$ sudo weave run -ti gliderlabs/alpine /bin/sh
5d22fbb050d3c4ceb4b192fb5e1b7eb0d0a5729bd0f842eb1d4a528e25c4d487
$ sudo docker exec 5d22 getent hosts redis
$ sudo docker logs weavedns
INFO: 2015/07/07 10:19:31.148076 [main] WeaveDNS version git-149fe870abe1

INFO: 2015/07/07 10:19:31.148143 [main] Waiting for mDNS interface ethwe to come up
INFO: 2015/07/07 10:19:31.248575 [main] Interface ethwe is up
INFO: 2015/07/07 10:19:31.248624 [main] Waiting for HTTP interface eth0 to come up
INFO: 2015/07/07 10:19:31.248959 [main] Interface eth0 is up
INFO: 2015/07/07 10:19:31.249128 [zonedb] Using mDNS on &{Index:127 MTU:65535 Name:ethwe HardwareAddr:2e:8a:e3:89:84:16 Flags:up|broadcast|multicast}
INFO: 2015/07/07 10:19:31.250556 [docker] Using Docker API on unix:///var/run/docker.sock: &[KernelVersion=3.13.0-53-generic Os=linux Version=1.6.2 ApiVersion=1.18 Arch=amd64 GitCommit=7c8fca2 GoVersion=go1.4.2]
INFO: 2015/07/07 10:19:31.251298 [main] HTTP API listening on 172.17.0.26:6785
INFO: 2015/07/07 10:19:31.251358 [dns] Upstream server(s): &{Servers:[8.8.8.8 8.8.4.4] Search:[localdomain] Port:53 Ndots:1 Timeout:5 Attempts:2}
INFO: 2015/07/07 10:19:31.251423 [dns] Cache: 8192 entries
DEBU: 2015/07/07 10:19:31.251454 [dns] Creating UDP DNS client with 5s timeout
DEBU: 2015/07/07 10:19:31.251464 [dns] Creating UDP DNS client with 5s timeout
DEBU: 2015/07/07 10:19:31.251480 [dns] Creating TCP DNS client with 5s timeout
DEBU: 2015/07/07 10:19:31.251485 [dns] Creating TCP DNS client with 5s timeout
INFO: 2015/07/07 10:19:31.252286 [dns] Listening for DNS on :53 (UDP)
INFO: 2015/07/07 10:19:31.253136 [dns] Listening for DNS on :53 (TCP)
INFO: 2015/07/07 10:19:31.410668 [zonedb] Container e2ab206d967b9c699ce4dd5c32c43a44a108ac038d54f89aa4fff32424d02865 down. Removing records
INFO: 2015/07/07 10:19:35.826293 [zonedb] Container f0210ae3ffa7ece9e3315e03523ebc3bff34720c83e933eb3a311170c69165b6 down. Removing records
INFO: 2015/07/07 10:19:36.362694 [zonedb] Container a8d1b78881706ccda5b39d401155f6167691cf6d4719dc66fd6afbed7805f27f down. Removing records
DEBU: 2015/07/07 10:19:44.881717 NotUsHandler ;; opcode: QUERY, status: NOERROR, id: 48265
;; flags: rd; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;redis. IN   A

DEBU: 2015/07/07 10:19:44.881818 [dns msgid 48265] Short name 'redis.' -> resolving locally
DEBU: 2015/07/07 10:19:44.881896 [dns msgid 48265] Query: {Name:redis. Qtype:1 Qclass:1}
DEBU: 2015/07/07 10:19:44.881926 [cache msgid 48265] addind in pending state
DEBU: 2015/07/07 10:19:44.882002 [zonedb] Looking for name 'redis.weave.local.' in local(&remote) database
DEBU: 2015/07/07 10:19:44.882037 [zonedb] 'redis.weave.local.' not in local database... trying with mDNS
DEBU: 2015/07/07 10:19:44.882272 [mdns] srv: mDNS query from local host ('10.128.0.1:47436'): ignored
DEBU: 2015/07/07 10:19:45.382892 [zonedb] mDNS lookup error for 'redis.weave.local.': Unable to find redis.weave.local.
INFO: 2015/07/07 10:19:45.383049 [dns msgid 48265] No results for type A query for 'redis.' [caching no-local]

rade · 2015-07-13T13:16:05Z

What is left to do here?

inercia · 2015-07-13T13:42:37Z

I will close this PR, as it will have to be reimplemented once the new DNS code is in place...

rade · 2015-07-13T14:07:46Z

I will close this PR, as it will have to be reimplemented once the new DNS code is in place.

If this is ready to merge then we should do so. What is missing?

Unit tests

inercia · 2015-07-14T13:09:26Z

It seems that broken resolvers add a . after all, as @bboreham discovered. So this PR could be reviewed again...

test/230_dns_unqualified_test.sh


 start_container          $HOST1 $C1/24 --name=c1 -h $NAME
 start_container_with_dns $HOST1 $C2/24 --name=c2 -h seetwo.$DOMAIN
 start_container_with_dns $HOST1 $C3/24 --name=c3 --dns-search=$DOMAIN
 container=$(start_container_with_dns $HOST1 $C4/24)
+start_container          $HOST1 $C5/24 --name=c5 -h short.$DOMAIN


bboreham · 2015-07-14T13:23:54Z

LGTM

nameserver/server.go

 					r.MsgHdr.Id, server, q.Name)
-				continue
+				w.WriteMsg(reply)
+				return


Resolve unqualified names in the local WeaveDNS domain Closes #987.

rade · 2015-07-14T14:06:52Z

I've had to revert this since it fails the integration tests on master.

rade reviewed Jul 1, 2015
View reviewed changes

rade assigned inercia Jul 1, 2015

rade reviewed Jul 1, 2015
View reviewed changes

inercia force-pushed the issues/987 branch from fc4234d to 149fe87 Compare July 2, 2015 11:02

rade assigned rade and unassigned inercia Jul 2, 2015

rade reviewed Jul 2, 2015
View reviewed changes

rade assigned inercia and unassigned rade Jul 2, 2015

inercia mentioned this pull request Jul 8, 2015

Gossip-based DNS embedded in the router. #1065

Merged

17 tasks

inercia force-pushed the issues/987 branch from 149fe87 to 651dafd Compare July 14, 2015 08:41

Resolve unqualified names in the local WeaveDNS domain

efb0ead

Unit tests

inercia force-pushed the issues/987 branch from 651dafd to 630bc8b Compare July 14, 2015 08:47

inercia assigned rade and unassigned inercia Jul 14, 2015

inercia reviewed Jul 14, 2015
View reviewed changes

Smoke tests

4c519fc

inercia force-pushed the issues/987 branch from 630bc8b to 4c519fc Compare July 14, 2015 13:12

rade reviewed Jul 14, 2015
View reviewed changes

nameserver/server.go

r.MsgHdr.Id, server, q.Name)

continue

w.WriteMsg(reply)

return

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

rade added a commit that referenced this pull request Jul 14, 2015

Merge pull request #1050 from /issues/987

39a1f21

Resolve unqualified names in the local WeaveDNS domain Closes #987.

rade merged commit 39a1f21 into master Jul 14, 2015

This was referenced Jul 14, 2015

[proxy] More flexible container name -> hostname derivation #1126

Merged

Revert "Resolve unqualified names in the local WeaveDNS domain" #1133

Merged

rade deleted the issues/987 branch July 14, 2015 14:08

rade modified the milestones: current, 1.1.0 Jul 15, 2015

Resolve unqualified names in the local WeaveDNS domain #1050

Resolve unqualified names in the local WeaveDNS domain #1050

Conversation

inercia commented Jun 30, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

rade commented Jul 1, 2015

bboreham commented Jul 1, 2015

inercia commented Jul 1, 2015

rade commented Jul 1, 2015

inercia commented Jul 1, 2015

This comment was marked as abuse.

awh commented Jul 1, 2015

inercia commented Jul 1, 2015

rade commented Jul 2, 2015

inercia commented Jul 2, 2015

rade commented Jul 2, 2015

This comment was marked as abuse.

This comment was marked as abuse.

inercia commented Jul 3, 2015

rade commented Jul 3, 2015

inercia commented Jul 3, 2015

rade commented Jul 3, 2015

rade commented Jul 6, 2015

awh commented Jul 7, 2015

bboreham commented Jul 7, 2015

inercia commented Jul 7, 2015

bboreham commented Jul 7, 2015

rade commented Jul 13, 2015

inercia commented Jul 13, 2015

rade commented Jul 13, 2015

inercia commented Jul 14, 2015

This comment was marked as abuse.

bboreham commented Jul 14, 2015

This comment was marked as abuse.

This comment was marked as abuse.

rade commented Jul 14, 2015