Consolidate default DNS port logic in config

[#156210589](https://www.pivotaltracker.com/story/show/156210589) Signed-off-by: Michael Xu <[email protected]>
cloudfoundry · Mar 26, 2018 · 91a71e3 · 91a71e3
1 parent 59272d1
commit 91a71e3
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 35 deletions.
diff --git a/src/bosh-dns/dns/config/config.go b/src/bosh-dns/dns/config/config.go
@@ -101,6 +101,11 @@ func LoadFromFile(configFilePath string) (Config, error) {
 		return Config{}, err
 	}
 
+	c.ExcludedRecursors, err = AppendDefaultDNSPortIfMissing(c.ExcludedRecursors)
+	if err != nil {
+		return Config{}, err
+	}
+
 	return c, nil
 }
 

diff --git a/src/bosh-dns/dns/config/config_test.go b/src/bosh-dns/dns/config/config_test.go
@@ -68,7 +68,7 @@ var _ = Describe("Config", func() {
 			"handlers_files_glob":  handlersFileGlob,
 			"port":                 listenPort,
 			"recursor_timeout":     recursorTimeout,
-			"excluded_recursors":   []string{"169.254.169.254"},
+			"excluded_recursors":   []string{"169.254.169.254", "169.10.10.10:1234"},
 			"timeout":              timeout,
 			"upcheck_domains":      upcheckDomains,
 			"api": map[string]interface{}{
@@ -125,7 +125,7 @@ var _ = Describe("Config", func() {
 			Timeout:            config.DurationJSON(timeoutDuration),
 			RecursorTimeout:    config.DurationJSON(recursorTimeoutDuration),
 			Recursors:          []string{},
-			ExcludedRecursors:  []string{"169.254.169.254"},
+			ExcludedRecursors:  []string{"169.254.169.254:53", "169.10.10.10:1234"},
 			UpcheckDomains:     []string{"upcheck.domain.", "health2.bosh."},
 			AliasFilesGlob:     aliasesFileGlob,
 			HandlersFilesGlob:  handlersFileGlob,

diff --git a/src/bosh-dns/dns/config/recursor.go b/src/bosh-dns/dns/config/recursor.go
@@ -1,10 +1,5 @@
 package config
 
-import (
-	"fmt"
-	"strings"
-)
-
 //go:generate counterfeiter . RecursorReader
 
 type RecursorReader interface {
@@ -35,9 +30,7 @@ func ConfigureRecursors(reader RecursorReader, shuffler StringShuffler, dnsConfi
 		shouldAdd := true
 
 		for _, excludedRecursor := range dnsConfig.ExcludedRecursors {
-			formattedRecursor := addPortIfNecessary(recursor)
-			excludedRecursor = addPortIfNecessary(excludedRecursor)
-			if formattedRecursor == excludedRecursor {
+			if recursor == excludedRecursor {
 				shouldAdd = false
 
 				break
@@ -53,11 +46,3 @@ func ConfigureRecursors(reader RecursorReader, shuffler StringShuffler, dnsConfi
 
 	return nil
 }
-
-func addPortIfNecessary(recursor string) string {
-	if strings.HasSuffix(recursor, ":53") {
-		return recursor
-	}
-
-	return fmt.Sprintf("%s:53", recursor)
-}
diff --git a/src/bosh-dns/dns/config/recursor_reader.go b/src/bosh-dns/dns/config/recursor_reader.go
@@ -1,8 +1,6 @@
 package config
 
 import (
-	"fmt"
-
 	"bosh-dns/dns/manager"
 )
 
@@ -21,20 +19,19 @@ type recursorReader struct {
 }
 
 func (r recursorReader) Get() ([]string, error) {
-	recursors := []string{}
-
 	nameservers, err := r.manager.Read()
 	if err != nil {
 		return nil, err
 	}
 
+	validRecursors := []string{}
 	for _, server := range nameservers {
 		if r.isValid(server) {
-			recursors = append(recursors, fmt.Sprintf("%s:53", server))
+			validRecursors = append(validRecursors, server)
 		}
 	}
 
-	return recursors, nil
+	return AppendDefaultDNSPortIfMissing(validRecursors)
 }
 
 func (r recursorReader) isNameServer(s string) bool {

diff --git a/src/bosh-dns/dns/config/recursor_reader_test.go b/src/bosh-dns/dns/config/recursor_reader_test.go
@@ -80,15 +80,15 @@ var _ = Describe("RecursorReader", func() {
 
 	Context("when multiple recursors are configured", func() {
 		BeforeEach(func() {
-			dnsManager.ReadReturns(append(dnsServerDomainNames, "recursor-1", "recursor-2"), nil)
+			dnsManager.ReadReturns(append(dnsServerDomainNames, "recursor-1", "recursor-2", "recursor-custom:1234"), nil)
 		})
 
 		It("returns all entries except the DNS server itself", func() {
 			recursors, err := recursorReader.Get()
 
 			Expect(err).ToNot(HaveOccurred())
-			Expect(recursors).To(HaveLen(2))
-			Expect(recursors).To(ConsistOf("recursor-1:53", "recursor-2:53"))
+			Expect(recursors).To(HaveLen(3))
+			Expect(recursors).To(ConsistOf("recursor-1:53", "recursor-2:53", "recursor-custom:1234"))
 		})
 	})
 

diff --git a/src/bosh-dns/dns/config/recursor_test.go b/src/bosh-dns/dns/config/recursor_test.go
@@ -25,21 +25,21 @@ var _ = Describe("Recursor", func() {
 
 	Context("when dns config does not have any recursors configured", func() {
 		BeforeEach(func() {
-			resolvConfReader.GetReturns([]string{"some-recursor-1:53", "some-recursor-2:53"}, nil)
+			resolvConfReader.GetReturns([]string{"some-recursor-1:53", "some-recursor-2:53", "recursor-custom:1234"}, nil)
 		})
 
 		It("should generate recursors from the resolv.conf, shuffled", func() {
 			err := config.ConfigureRecursors(resolvConfReader, stringShuffler, &dnsConfig)
 			Expect(err).ToNot(HaveOccurred())
-			Expect(dnsConfig.Recursors).Should(Equal([]string{"some-recursor-1:53", "some-recursor-2:53"}))
+			Expect(dnsConfig.Recursors).Should(Equal([]string{"some-recursor-1:53", "some-recursor-2:53", "recursor-custom:1234"}))
 			Expect(stringShuffler.ShuffleCallCount()).To(Equal(1))
 
 			Expect(resolvConfReader.GetCallCount()).To(Equal(1))
 		})
 
 		Context("when unable to Get fails on RecursorReader", func() {
 			BeforeEach(func() {
-				resolvConfReader.GetReturns([]string{"some-recursor-1"}, errors.New("some-error"))
+				resolvConfReader.GetReturns([]string{"some-recursor-1:53"}, errors.New("some-error"))
 			})
 
 			It("should return the error", func() {
@@ -52,7 +52,7 @@ var _ = Describe("Recursor", func() {
 
 		Context("when excluding recursors", func() {
 			BeforeEach(func() {
-				dnsConfig.ExcludedRecursors = []string{"some-recursor-1"}
+				dnsConfig.ExcludedRecursors = []string{"some-recursor-1:53", "recursor-custom:1234"}
 			})
 
 			It("should exclude the recursor", func() {
@@ -67,27 +67,27 @@ var _ = Describe("Recursor", func() {
 	Context("when dns config does has recursors configured", func() {
 		BeforeEach(func() {
 			dnsConfig = config.Config{
-				Recursors: []string{"some-recursor-1", "some-recursor-2"},
+				Recursors: []string{"some-recursor-1:53", "some-recursor-2:53", "recursor-custom:1234"},
 			}
 		})
 
 		It("should shuffle the recursors", func() {
 			err := config.ConfigureRecursors(resolvConfReader, stringShuffler, &dnsConfig)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(stringShuffler.ShuffleCallCount()).To(Equal(1))
-			Expect(dnsConfig.Recursors).Should(Equal([]string{"some-recursor-1", "some-recursor-2"}))
+			Expect(dnsConfig.Recursors).Should(Equal([]string{"some-recursor-1:53", "some-recursor-2:53", "recursor-custom:1234"}))
 		})
 
 		Context("when excluding recursors", func() {
 			BeforeEach(func() {
-				dnsConfig.ExcludedRecursors = []string{"some-recursor-1"}
+				dnsConfig.ExcludedRecursors = []string{"some-recursor-1:53", "recursor-custom:1234"}
 			})
 
 			It("should exclude the recursor", func() {
 				err := config.ConfigureRecursors(resolvConfReader, stringShuffler, &dnsConfig)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(stringShuffler.ShuffleCallCount()).To(Equal(1))
-				Expect(dnsConfig.Recursors).Should(Equal([]string{"some-recursor-2"}))
+				Expect(dnsConfig.Recursors).Should(Equal([]string{"some-recursor-2:53"}))
 			})
 		})
 	})