style(resolver): standardize configuration disabled impls and tests

0xERR0R · Dec 5, 2022 · d501d86 · d501d86
1 parent 7fa7e2e
commit d501d86
Show file tree

Hide file tree

Showing 25 changed files with 139 additions and 119 deletions.
diff --git a/lists/list_cache_test.go b/lists/list_cache_test.go
@@ -349,7 +349,7 @@ var _ = Describe("ListCache", func() {
 
 				c := sut.Configuration()
 				Expect(c).Should(ContainElement("refresh period: 1 hour"))
-				Expect(c).Should(HaveLen(11))
+				Expect(len(c)).Should(BeNumerically(">", 1))
 			})
 		})
 		When("refresh is disabled", func() {

diff --git a/resolver/blocking_resolver.go b/resolver/blocking_resolver.go
@@ -329,34 +329,34 @@ func (r *BlockingResolver) handleBlocked(logger *logrus.Entry,
 
 // Configuration returns the current resolver configuration
 func (r *BlockingResolver) Configuration() (result []string) {
-	if len(r.cfg.ClientGroupsBlock) > 0 {
-		result = append(result, "clientGroupsBlock")
-		for key, val := range r.cfg.ClientGroupsBlock {
-			result = append(result, fmt.Sprintf("  %s = \"%s\"", key, strings.Join(val, ";")))
-		}
+	if len(r.cfg.ClientGroupsBlock) == 0 {
+		return configDisabled
+	}
 
-		blockType := r.cfg.BlockType
-		result = append(result, fmt.Sprintf("blockType = \"%s\"", blockType))
+	result = append(result, "clientGroupsBlock")
+	for key, val := range r.cfg.ClientGroupsBlock {
+		result = append(result, fmt.Sprintf("  %s = \"%s\"", key, strings.Join(val, ";")))
+	}
 
-		if blockType != "NXDOMAIN" {
-			result = append(result, fmt.Sprintf("blockTTL = %s", r.cfg.BlockTTL.String()))
-		}
+	blockType := r.cfg.BlockType
+	result = append(result, fmt.Sprintf("blockType = \"%s\"", blockType))
+
+	if blockType != "NXDOMAIN" {
+		result = append(result, fmt.Sprintf("blockTTL = %s", r.cfg.BlockTTL.String()))
+	}
 
-		result = append(result, fmt.Sprintf("downloadTimeout = %s", r.cfg.DownloadTimeout.String()))
+	result = append(result, fmt.Sprintf("downloadTimeout = %s", r.cfg.DownloadTimeout.String()))
 
-		result = append(result, fmt.Sprintf("FailStartOnListError = %t", r.cfg.FailStartOnListError))
+	result = append(result, fmt.Sprintf("FailStartOnListError = %t", r.cfg.FailStartOnListError))
 
-		result = append(result, "blacklist:")
-		for _, c := range r.blacklistMatcher.Configuration() {
-			result = append(result, fmt.Sprintf("  %s", c))
-		}
+	result = append(result, "blacklist:")
+	for _, c := range r.blacklistMatcher.Configuration() {
+		result = append(result, fmt.Sprintf("  %s", c))
+	}
 
-		result = append(result, "whitelist:")
-		for _, c := range r.whitelistMatcher.Configuration() {
-			result = append(result, fmt.Sprintf("  %s", c))
-		}
-	} else {
-		result = []string{"deactivated"}
+	result = append(result, "whitelist:")
+	for _, c := range r.whitelistMatcher.Configuration() {
+		result = append(result, fmt.Sprintf("  %s", c))
 	}
 
 	return result

diff --git a/resolver/blocking_resolver_test.go b/resolver/blocking_resolver_test.go
@@ -844,8 +844,7 @@ var _ = Describe("BlockingResolver", Label("blockingResolver"), func() {
 		})
 		It("should return 'disabled''", func() {
 			c := sut.Configuration()
-			Expect(c).Should(HaveLen(1))
-			Expect(c).Should(Equal([]string{"deactivated"}))
+			Expect(c).Should(ContainElement(configStatusDisabled))
 		})
 	})
 

diff --git a/resolver/caching_resolver.go b/resolver/caching_resolver.go
@@ -125,9 +125,7 @@ func (r *CachingResolver) onExpired(cacheKey string) (val interface{}, ttl time.
 // Configuration returns a current resolver configuration
 func (r *CachingResolver) Configuration() (result []string) {
 	if r.maxCacheTimeSec < 0 {
-		result = []string{"deactivated"}
-
-		return
+		return configDisabled
 	}
 
 	result = append(result, fmt.Sprintf("minCacheTimeInSec = %d", r.minCacheTimeSec))

diff --git a/resolver/caching_resolver_test.go b/resolver/caching_resolver_test.go
@@ -495,8 +495,7 @@ var _ = Describe("CachingResolver", func() {
 			})
 			It("should return 'disabled'", func() {
 				c := sut.Configuration()
-				Expect(c).Should(HaveLen(1))
-				Expect(c).Should(Equal([]string{"deactivated"}))
+				Expect(c).Should(ContainElement(configStatusDisabled))
 			})
 		})
 

diff --git a/resolver/client_names_resolver.go b/resolver/client_names_resolver.go
@@ -48,24 +48,24 @@ func NewClientNamesResolver(
 
 // Configuration returns current resolver configuration
 func (r *ClientNamesResolver) Configuration() (result []string) {
-	if r.externalResolver != nil || len(r.clientIPMapping) > 0 {
-		result = append(result, fmt.Sprintf("singleNameOrder = \"%v\"", r.singleNameOrder))
+	if r.externalResolver == nil && len(r.clientIPMapping) == 0 {
+		return append(configDisabled, "use only IP address")
+	}
 
-		if r.externalResolver != nil {
-			result = append(result, fmt.Sprintf("externalResolver = \"%s\"", r.externalResolver))
-		}
+	result = append(result, fmt.Sprintf("singleNameOrder = \"%v\"", r.singleNameOrder))
+
+	if r.externalResolver != nil {
+		result = append(result, fmt.Sprintf("externalResolver = \"%s\"", r.externalResolver))
+	}
 
-		result = append(result, fmt.Sprintf("cache item count = %d", r.cache.TotalCount()))
+	result = append(result, fmt.Sprintf("cache item count = %d", r.cache.TotalCount()))
 
-		if len(r.clientIPMapping) > 0 {
-			result = append(result, "client IP mapping:")
+	if len(r.clientIPMapping) > 0 {
+		result = append(result, "client IP mapping:")
 
-			for k, v := range r.clientIPMapping {
-				result = append(result, fmt.Sprintf("%s -> %s", k, v))
-			}
+		for k, v := range r.clientIPMapping {
+			result = append(result, fmt.Sprintf("%s -> %s", k, v))
 		}
-	} else {
-		result = []string{"deactivated, use only IP address"}
 	}
 
 	return

diff --git a/resolver/client_names_resolver_test.go b/resolver/client_names_resolver_test.go
@@ -344,10 +344,9 @@ var _ = Describe("ClientResolver", Label("clientNamesResolver"), func() {
 			BeforeEach(func() {
 				sutConfig = config.ClientLookupConfig{}
 			})
-			It("should return 'deactivated'", func() {
+			It("should return 'disabled'", func() {
 				c := sut.Configuration()
-				Expect(c).Should(HaveLen(1))
-				Expect(c).Should(Equal([]string{"deactivated, use only IP address"}))
+				Expect(c).Should(ContainElement(configStatusDisabled))
 			})
 		})
 

diff --git a/resolver/conditional_upstream_resolver.go b/resolver/conditional_upstream_resolver.go
@@ -41,12 +41,12 @@ func NewConditionalUpstreamResolver(
 
 // Configuration returns current configuration
 func (r *ConditionalUpstreamResolver) Configuration() (result []string) {
-	if len(r.mapping) > 0 {
-		for key, val := range r.mapping {
-			result = append(result, fmt.Sprintf("%s = \"%s\"", key, val))
-		}
-	} else {
-		result = []string{"deactivated"}
+	if len(r.mapping) == 0 {
+		return configDisabled
+	}
+
+	for key, val := range r.mapping {
+		result = append(result, fmt.Sprintf("%s = \"%s\"", key, val))
 	}
 
 	return

diff --git a/resolver/conditional_upstream_resolver_test.go b/resolver/conditional_upstream_resolver_test.go
@@ -145,8 +145,7 @@ var _ = Describe("ConditionalUpstreamResolver", Label("conditionalResolver"), fu
 			})
 			It("should return 'disabled'", func() {
 				c := sut.Configuration()
-				Expect(c).Should(HaveLen(1))
-				Expect(c).Should(Equal([]string{"deactivated"}))
+				Expect(c).Should(ContainElement(configStatusDisabled))
 			})
 		})
 	})

diff --git a/resolver/custom_dns_resolver.go b/resolver/custom_dns_resolver.go
@@ -49,12 +49,12 @@ func NewCustomDNSResolver(cfg config.CustomDNSConfig) ChainedResolver {
 
 // Configuration returns current resolver configuration
 func (r *CustomDNSResolver) Configuration() (result []string) {
-	if len(r.mapping) > 0 {
-		for key, val := range r.mapping {
-			result = append(result, fmt.Sprintf("%s = \"%s\"", key, val))
-		}
-	} else {
-		result = []string{"deactivated"}
+	if len(r.mapping) == 0 {
+		return configDisabled
+	}
+
+	for key, val := range r.mapping {
+		result = append(result, fmt.Sprintf("%s = \"%s\"", key, val))
 	}
 
 	return

diff --git a/resolver/custom_dns_resolver_test.go b/resolver/custom_dns_resolver_test.go
@@ -196,7 +196,7 @@ var _ = Describe("CustomDNSResolver", func() {
 		When("resolver is enabled", func() {
 			It("should return configuration", func() {
 				c := sut.Configuration()
-				Expect(c).Should(HaveLen(3))
+				Expect(len(c)).Should(BeNumerically(">", 1))
 			})
 		})
 
@@ -206,8 +206,7 @@ var _ = Describe("CustomDNSResolver", func() {
 			})
 			It("should return 'disabled'", func() {
 				c := sut.Configuration()
-				Expect(c).Should(HaveLen(1))
-				Expect(c).Should(Equal([]string{"deactivated"}))
+				Expect(c).Should(ContainElement(configStatusDisabled))
 			})
 		})
 	})

diff --git a/resolver/ede_resolver.go b/resolver/ede_resolver.go
@@ -27,17 +27,15 @@ func (r *EdeResolver) Resolve(request *model.Request) (*model.Response, error) {
 		addExtraReasoning(resp)
 	}
 
-	return resp, err
+	return resp, nil
 }
 
 func (r *EdeResolver) Configuration() (result []string) {
-	if r.config.Enable {
-		result = []string{"activated"}
-	} else {
-		result = []string{"deactivated"}
+	if !r.config.Enable {
+		return configDisabled
 	}
 
-	return result
+	return configEnabled
 }
 
 func addExtraReasoning(res *model.Response) {

diff --git a/resolver/ede_resolver_test.go b/resolver/ede_resolver_test.go
@@ -38,13 +38,13 @@ var _ = Describe("EdeResolver", func() {
 		sut.Next(m)
 	})
 
-	When("Ede is disabled", func() {
+	When("ede is disabled", func() {
 		BeforeEach(func() {
 			sutConfig = config.EdeConfig{
 				Enable: false,
 			}
 		})
-		It("Shouldn't add EDE information", func() {
+		It("shouldn't add EDE information", func() {
 			resp, err := sut.Resolve(newRequest("example.com", dns.Type(dns.TypeA)))
 			Expect(err).Should(Succeed())
 			Expect(resp.Res.Rcode).Should(Equal(dns.RcodeSuccess))
@@ -55,19 +55,15 @@ var _ = Describe("EdeResolver", func() {
 			// delegated to next resolver
 			Expect(m.Calls).Should(HaveLen(1))
 		})
-		It("Configure should output deactivated", func() {
-			c := sut.Configuration()
-			Expect(c).Should(HaveLen(1))
-			Expect(c[0]).Should(Equal("deactivated"))
-		})
 	})
-	When("Ede is enabled", func() {
+
+	When("ede is enabled", func() {
 		BeforeEach(func() {
 			sutConfig = config.EdeConfig{
 				Enable: true,
 			}
 		})
-		It("Should add EDE information", func() {
+		It("should add EDE information", func() {
 			resp, err := sut.Resolve(newRequest("example.com", dns.Type(dns.TypeA)))
 			Expect(err).Should(Succeed())
 			Expect(resp.Res.Rcode).Should(Equal(dns.RcodeSuccess))
@@ -82,11 +78,6 @@ var _ = Describe("EdeResolver", func() {
 			Expect(ede.InfoCode).Should(Equal(dns.ExtendedErrorCodeForgedAnswer))
 			Expect(ede.ExtraText).Should(Equal("Test"))
 		})
-		It("Configure should output activated", func() {
-			c := sut.Configuration()
-			Expect(c).Should(HaveLen(1))
-			Expect(c[0]).Should(Equal("activated"))
-		})
 
 		When("resolver returns an error", func() {
 			resolveErr := errors.New("test")
@@ -103,4 +94,26 @@ var _ = Describe("EdeResolver", func() {
 			})
 		})
 	})
+
+	Describe("Configuration output", func() {
+		When("resolver is enabled", func() {
+			BeforeEach(func() {
+				sutConfig = config.EdeConfig{Enable: true}
+			})
+			It("should return configuration", func() {
+				c := sut.Configuration()
+				Expect(c).Should(Equal(configEnabled))
+			})
+		})
+
+		When("resolver is disabled", func() {
+			BeforeEach(func() {
+				sutConfig = config.EdeConfig{Enable: false}
+			})
+			It("should return 'disabled'", func() {
+				c := sut.Configuration()
+				Expect(c).Should(ContainElement(configStatusDisabled))
+			})
+		})
+	})
 })
diff --git a/resolver/filtering_resolver.go b/resolver/filtering_resolver.go
@@ -30,12 +30,14 @@ func (r *FilteringResolver) Resolve(request *model.Request) (*model.Response, er
 }
 
 func (r *FilteringResolver) Configuration() (result []string) {
-	qTypes := make([]string, len(r.queryTypes))
-	ix := 0
+	if len(r.queryTypes) == 0 {
+		return configDisabled
+	}
+
+	qTypes := make([]string, 0, len(r.queryTypes))
 
 	for qType := range r.queryTypes {
-		qTypes[ix] = qType.String()
-		ix++
+		qTypes = append(qTypes, qType.String())
 	}
 
 	sort.Strings(qTypes)

diff --git a/resolver/filtering_resolver_test.go b/resolver/filtering_resolver_test.go
@@ -56,8 +56,7 @@ var _ = Describe("FilteringResolver", func() {
 		})
 		It("Configure should output all query types", func() {
 			c := sut.Configuration()
-			Expect(c).Should(HaveLen(1))
-			Expect(c[0]).Should(Equal("filtering query Types: 'AAAA, MX'"))
+			Expect(c).Should(Equal([]string{"filtering query Types: 'AAAA, MX'"}))
 		})
 	})
 
@@ -74,8 +73,7 @@ var _ = Describe("FilteringResolver", func() {
 		})
 		It("Configure should output 'empty list'", func() {
 			c := sut.Configuration()
-			Expect(c).Should(HaveLen(1))
-			Expect(c[0]).Should(Equal("filtering query Types: ''"))
+			Expect(c).Should(ContainElement(configStatusDisabled))
 		})
 	})
 })
diff --git a/resolver/fqdn_only_resolver.go b/resolver/fqdn_only_resolver.go
@@ -35,11 +35,9 @@ func (r *FqdnOnlyResolver) Resolve(request *model.Request) (*model.Response, err
 }
 
 func (r *FqdnOnlyResolver) Configuration() (result []string) {
-	if r.enabled {
-		result = []string{"activated"}
-	} else {
-		result = []string{"deactivated"}
+	if !r.enabled {
+		return configDisabled
 	}
 
-	return result
+	return configEnabled
 }