diff --git a/nameserver/dns.go b/nameserver/dns.go index c232d45b..e6b4b6a4 100644 --- a/nameserver/dns.go +++ b/nameserver/dns.go @@ -80,6 +80,10 @@ func makeDNSFailResponse(r *dns.Msg) *dns.Msg { return m } +func failHandleFunc(w dns.ResponseWriter, r *dns.Msg) { + w.WriteMsg(makeDNSFailResponse(r)) +} + // get the maximum UDP-reply length func getMaxReplyLen(r *dns.Msg, proto dnsProtocol) int { maxLen := minUDPSize diff --git a/nameserver/names.go b/nameserver/names.go new file mode 100644 index 00000000..edf227ef --- /dev/null +++ b/nameserver/names.go @@ -0,0 +1,28 @@ +package nameserver + +import ( + "strings" + + "github.com/miekg/dns" +) + +// Get the number of components of a name +// For example, "redis" is 1, "redis.weave.local" is 3... +func nameNumComponents(name string) int { + if len(name) == 0 || name == "." { + return 0 + } + return strings.Count(dns.Fqdn(name), ".") +} + +// normalize a name by +// a) adding the local domain when it is a single component name +// b) converting the result to FQDN +func normalizeName(n string, domain string) string { + // we should never add anything to a FQDN like "redis.", but that's the only thing that + // some broken resolvers will do: to convert the name to FQDN... + if nameNumComponents(n) == 1 { + n = dns.Fqdn(n) + domain + } + return dns.Fqdn(n) +} diff --git a/nameserver/names_test.go b/nameserver/names_test.go new file mode 100644 index 00000000..9af2539c --- /dev/null +++ b/nameserver/names_test.go @@ -0,0 +1,34 @@ +package nameserver + +import ( + "testing" + + "github.com/stretchr/testify/require" + . "github.com/weaveworks/weave/common" +) + +func TestNamesNumComponents(t *testing.T) { + EnableDebugLogging(testing.Verbose()) + + assertNumComps := func(n string, e int) { require.Equal(t, e, nameNumComponents(n), "name num components failed") } + + assertNumComps("", 0) + assertNumComps(".", 0) + assertNumComps("something", 1) + assertNumComps("something.", 1) + assertNumComps("something.local", 2) + assertNumComps("something.local.", 2) +} + +func TestNamesNormalization(t *testing.T) { + EnableDebugLogging(testing.Verbose()) + + assertNormalization := func(s, d, e string) { require.Equal(t, e, normalizeName(s, d), "name normalization failed") } + + assertNormalization("", "domain.local.", ".") + assertNormalization(".", "domain.local.", ".") + assertNormalization("something", "domain.local.", "something.domain.local.") + assertNormalization("something.", "domain.local.", "something.domain.local.") + assertNormalization("something.domain.local", "domain.local.", "something.domain.local.") + assertNormalization("something.domain.local.", "domain.local.", "something.domain.local.") +} diff --git a/nameserver/server.go b/nameserver/server.go index 6f36aaf4..4ff42e4a 100644 --- a/nameserver/server.go +++ b/nameserver/server.go @@ -279,9 +279,6 @@ func (s *DNSServer) Stop() error { // We must create two DNS request multiplexers, depending on the protocol used by // clients (as we use the same protocol for asking upstream servers) func (s *DNSServer) createMux(proto dnsProtocol) *dns.ServeMux { - failFallback := func(w dns.ResponseWriter, r *dns.Msg) { - w.WriteMsg(makeDNSFailResponse(r)) - } notUsHandler := s.notUsHandler(proto) notUsFallback := func(w dns.ResponseWriter, r *dns.Msg) { Log.Infof("[dns msgid %d] -> sending to fallback server", r.MsgHdr.Id) @@ -291,7 +288,7 @@ func (s *DNSServer) createMux(proto dnsProtocol) *dns.ServeMux { // create the multiplexer m := dns.NewServeMux() m.HandleFunc(s.Zone.Domain(), s.localHandler(proto, "Query", dns.TypeA, - s.Zone.DomainLookupName, makeAddressReply, s.Zone.ObserveName, failFallback)) + s.Zone.DomainLookupName, makeAddressReply, s.Zone.ObserveName, failHandleFunc)) m.HandleFunc(RDNSDomain, s.localHandler(proto, "Reverse query", dns.TypePTR, s.Zone.DomainLookupInaddr, makePTRReply, s.Zone.ObserveInaddr, notUsFallback)) m.HandleFunc(".", s.notUsHandler(proto)) @@ -373,32 +370,42 @@ 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) + return func(w dns.ResponseWriter, r *dns.Msg) { q := r.Question[0] - - // announce our max payload size as the max payload our client supports - maxLen := getMaxReplyLen(r, proto) - rcopy := r - rcopy.SetEdns0(uint16(maxLen), false) - - Log.Debugf("[dns msgid %d] Fallback query: %+v [%s, max:%d bytes]", rcopy.MsgHdr.Id, q, proto, maxLen) - for _, server := range s.Upstream.Servers { - reply, _, err := dnsClient.Exchange(rcopy, fmt.Sprintf("%s:%s", server, s.Upstream.Port)) - if err != nil { - Log.Debugf("[dns msgid %d] Network error trying %s (%s)", - r.MsgHdr.Id, server, err) - continue - } - if reply != nil && reply.Rcode != dns.RcodeSuccess { - Log.Debugf("[dns msgid %d] Failure reported by %s for query %s", - r.MsgHdr.Id, server, q.Name) - continue - } - Log.Debugf("[dns msgid %d] Given answer by %s for query %s", - r.MsgHdr.Id, server, q.Name) - w.WriteMsg(reply) + nc := nameNumComponents(q.Name) + if nc == 1 { + Log.Debugf("[dns msgid %d] Unqualified query: '%+v' ", r.MsgHdr.Id, q) + localNameResolver(w, r) // short names (eg, "redis.") will follow a local resolution process return + } else if nc > 1 { + // announce our max payload size as the max payload our client supports + maxLen := getMaxReplyLen(r, proto) + rcopy := r + rcopy.SetEdns0(uint16(maxLen), false) + + Log.Debugf("[dns msgid %d] Fallback query: %+v [%s, max:%d bytes]", rcopy.MsgHdr.Id, q, proto, maxLen) + for _, server := range s.Upstream.Servers { + reply, _, err := dnsClient.Exchange(rcopy, fmt.Sprintf("%s:%s", server, s.Upstream.Port)) + if err != nil { + Log.Debugf("[dns msgid %d] Network error trying %s (%s)", + r.MsgHdr.Id, server, err) + continue + } + if reply != nil && reply.Rcode != dns.RcodeSuccess { + Log.Debugf("[dns msgid %d] Failure reported by %s for query %s", + r.MsgHdr.Id, server, q.Name) + continue + } + Log.Debugf("[dns msgid %d] Given answer by %s for query %s", + r.MsgHdr.Id, server, q.Name) + w.WriteMsg(reply) + return + } } + Log.Warningf("[dns msgid %d] Failed lookup for external name %s", r.MsgHdr.Id, q.Name) w.WriteMsg(makeDNSFailResponse(r)) } diff --git a/nameserver/server_test.go b/nameserver/server_test.go index 9c4a1976..9ec63986 100644 --- a/nameserver/server_test.go +++ b/nameserver/server_test.go @@ -32,11 +32,12 @@ func TestUDPDNSServer(t *testing.T) { setupForTest(t) const ( - successTestName = "test1.weave.local." - failTestName = "fail.weave.local." - nonLocalName = "weave.works." - testAddr1 = "10.2.2.1" - containerID = "somecontainer" + successTestName = "test1.weave.local." + successTestNameShort = "test1." + failTestName = "fail.weave.local." + nonLocalName = "weave.works." + testAddr1 = "10.2.2.1" + containerID = "somecontainer" ) testCIDR1 := testAddr1 + "/24" @@ -105,6 +106,11 @@ func TestUDPDNSServer(t *testing.T) { require.IsType(t, (*dns.A)(nil), r.Answer[0], "DNS record") require.Equal(t, testAddr1, r.Answer[0].(*dns.A).A.String(), "IP address") + // Assert that we can get the same result when asking for the same name but with no domain + _, r = assertExchange(t, successTestNameShort, dns.TypeA, testPort, 1, 1, 0) + require.IsType(t, (*dns.A)(nil), r.Answer[0], "DNS record") + require.Equal(t, testAddr1, r.Answer[0].(*dns.A).A.String(), "IP address") + assertExchange(t, failTestName, dns.TypeA, testPort, 0, 0, dns.RcodeNameError) _, r = assertExchange(t, testRDNSsuccess, dns.TypePTR, testPort, 1, 1, 0) diff --git a/nameserver/zone_lookup.go b/nameserver/zone_lookup.go index 6e515b62..32486131 100644 --- a/nameserver/zone_lookup.go +++ b/nameserver/zone_lookup.go @@ -4,7 +4,6 @@ import ( "net" "time" - "github.com/miekg/dns" . "github.com/weaveworks/weave/common" ) @@ -62,7 +61,7 @@ func (zone *ZoneDb) LookupName(name string) (res []ZoneRecord, err error) { defer zone.mx.RUnlock() // note: LookupName() is usually called from the mDNS server, so we do not touch the name - name = dns.Fqdn(name) + name = normalizeName(name, zone.domain) Log.Debugf("[zonedb] Looking for name '%s' in local database", name) return zone.lookup(name, func(ns *nameSet) []*recordEntry { return ns.getEntriesForName(name) }) } @@ -117,7 +116,7 @@ func (zone *ZoneDb) domainLookup(target string, lfun ZoneLookupFunc) (res []Zone // Perform a lookup for a name in the zone // The name can be resolved locally with the local database or with some other resolution method (eg, a mDNS query) func (zone *ZoneDb) DomainLookupName(name string) (res []ZoneRecord, err error) { - name = dns.Fqdn(name) + name = normalizeName(name, zone.domain) Log.Debugf("[zonedb] Looking for name '%s' in local(&remote) database", name) zone.mx.Lock() @@ -267,7 +266,7 @@ func (zone *ZoneDb) updater(name string) (nextTime time.Time) { } // perform the refresh for this name - fullName := dns.Fqdn(name) + fullName := normalizeName(name, zone.domain) startTime := zone.clock.Now() Log.Debugf("[zonedb] Refreshing name '%s' with mDNS...", fullName) res, _ := zone.mdnsCli.InsistentLookupName(fullName) diff --git a/nameserver/zone_lookup_test.go b/nameserver/zone_lookup_test.go index 31789133..784d2612 100644 --- a/nameserver/zone_lookup_test.go +++ b/nameserver/zone_lookup_test.go @@ -9,6 +9,36 @@ import ( . "github.com/weaveworks/weave/common" ) +func TestZoneLookups(t *testing.T) { + var ( + id = "foobar" + name = "foo.weave.local" + addr1 = "10.2.2.3/24" + addr2 = "10.2.7.8/24" + ) + + EnableDebugLogging(testing.Verbose()) + + zone, err := NewZoneDb(ZoneConfig{}) + require.NoError(t, err) + err = zone.Start() + require.NoError(t, err) + defer zone.Stop() + + for _, addr := range []string{addr1, addr2} { + ip, _, _ := net.ParseCIDR(addr) + err := zone.AddRecord(id, name, ip) + require.NoError(t, err) + } + + // Check we can query for the name in many different ways... + for _, qName := range []string{"foo.weave.local", "foo.weave.local.", "foo", "foo."} { + res, err := zone.LookupName(qName) + require.NoError(t, err) + require.Equal(t, len(res), 2, "Unexpected number of responses") + } +} + // Check that the refreshing mechanism works as expected func TestZoneRefresh(t *testing.T) { const ( diff --git a/test/230_dns_unqualified_test.sh b/test/230_dns_unqualified_test.sh index 38e30043..1bbffe83 100755 --- a/test/230_dns_unqualified_test.sh +++ b/test/230_dns_unqualified_test.sh @@ -11,19 +11,19 @@ NAME=seeone.$DOMAIN start_suite "Resolve unqualified names" -launch_dns_on $HOST1 10.2.254.1/24 +launch_dns_on $HOST1 10.2.254.1/24 --debug 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) -check() { - assert "exec_on $HOST1 $1 getent hosts seeone | tr -s ' '" "$C1 $NAME" -} +assert_dns_a_record $HOST1 c2 seeone $C1 $NAME +assert_dns_a_record $HOST1 c3 seeone $C1 $NAME +assert_dns_a_record $HOST1 "$container" seeone $C1 $NAME -check c2 -check c3 -check "$container" +# check that unqualified names are automatically qualified for broken resolvers +weave_on $HOST1 dns-add $C1 c1 -h mysql.weave.local +assert_dns_a_record $HOST1 c1 mysql $C1 end_suite diff --git a/test/config.sh b/test/config.sh index 3d136999..05fd419d 100644 --- a/test/config.sh +++ b/test/config.sh @@ -188,9 +188,10 @@ assert_no_dns_record() { assert_raises "exec_on $host $container getent hosts $name" 2 } -# assert_dns_a_record +# assert_dns_a_record [] assert_dns_a_record() { - assert "exec_on $1 $2 getent hosts $3 | tr -s ' '" "$4 $3" + exp_name=${5:-$3} + assert "exec_on $1 $2 getent hosts $3 | tr -s ' ' | cut -d ' ' -f 1,2" "$4 $exp_name" } # assert_dns_ptr_record