Pull request: imp-dnsforward-cyclo

Merge in DNS/adguard-home from imp-dnsforward-cyclo to master

Squashed commit of the following:

commit ab05ebbcc62c5f66f838a9df6ce8e51721606bb6
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Sep 6 18:43:18 2022 +0300

    dnsforward: imp cyclo

internal/dnsforward/dnsrewrite.go

@@ -19,61 +19,23 @@ func (s *Server) filterDNSRewriteResponse(
 	rr rules.RRType,
 	v rules.RRValue,
 ) (ans dns.RR, err error) {
-	// TODO(a.garipov): As more types are added, we will probably want to use a
-	// handler-oriented approach here.  So, think of a way to decouple the
-	// answer generation logic from the Server.
-
 	switch rr {
-	case dns.TypeA,
+	case
+		dns.TypeA,
 		dns.TypeAAAA:
-		ip, ok := v.(net.IP)
+		return s.ansFromDNSRewriteIP(v, rr, req)
-		if !ok {
+	case
-			return nil, fmt.Errorf("value for rr type %d has type %T, not net.IP", rr, v)
+		dns.TypePTR,
-		}
-
-		if rr == dns.TypeA {
-			return s.genAnswerA(req, ip.To4()), nil
-		}
-
-		return s.genAnswerAAAA(req, ip), nil
-	case dns.TypePTR,
 		dns.TypeTXT:
-		str, ok := v.(string)
+		return s.ansFromDNSRewriteText(v, rr, req)
-		if !ok {
-			return nil, fmt.Errorf("value for rr type %d has type %T, not string", rr, v)
-		}
-
-		if rr == dns.TypeTXT {
-			return s.genAnswerTXT(req, []string{str}), nil
-		}
-
-		return s.genAnswerPTR(req, str), nil
 	case dns.TypeMX:
-		mx, ok := v.(*rules.DNSMX)
+		return s.ansFromDNSRewriteMX(v, rr, req)
-		if !ok {
+	case
-			return nil, fmt.Errorf("value for rr type %d has type %T, not *rules.DNSMX", rr, v)
+		dns.TypeHTTPS,
-		}
-
-		return s.genAnswerMX(req, mx), nil
-	case dns.TypeHTTPS,
 		dns.TypeSVCB:
-		svcb, ok := v.(*rules.DNSSVCB)
+		return s.ansFromDNSRewriteSVCB(v, rr, req)
-		if !ok {
-			return nil, fmt.Errorf("value for rr type %d has type %T, not *rules.DNSSVCB", rr, v)
-		}
-
-		if rr == dns.TypeHTTPS {
-			return s.genAnswerHTTPS(req, svcb), nil
-		}
-
-		return s.genAnswerSVCB(req, svcb), nil
 	case dns.TypeSRV:
-		srv, ok := v.(*rules.DNSSRV)
+		return s.ansFromDNSRewriteSRV(v, rr, req)
-		if !ok {
-			return nil, fmt.Errorf("value for rr type %d has type %T, not *rules.DNSSRV", rr, v)
-		}
-
-		return s.genAnswerSRV(req, srv), nil
 	default:
 		log.Debug("don't know how to handle dns rr type %d, skipping", rr)
 
@@ -81,6 +43,105 @@ func (s *Server) filterDNSRewriteResponse(
 	}
 }
 
+// ansFromDNSRewriteIP creates a new answer resource record from the A/AAAA
+// dnsrewrite rule data.
+func (s *Server) ansFromDNSRewriteIP(
+	v rules.RRValue,
+	rr rules.RRType,
+	req *dns.Msg,
+) (ans dns.RR, err error) {
+	ip, ok := v.(net.IP)
+	if !ok {
+		return nil, fmt.Errorf("value for rr type %s has type %T, not net.IP", dns.Type(rr), v)
+	}
+
+	if rr == dns.TypeA {
+		return s.genAnswerA(req, ip.To4()), nil
+	}
+
+	return s.genAnswerAAAA(req, ip), nil
+}
+
+// ansFromDNSRewriteText creates a new answer resource record from the TXT/PTR
+// dnsrewrite rule data.
+func (s *Server) ansFromDNSRewriteText(
+	v rules.RRValue,
+	rr rules.RRType,
+	req *dns.Msg,
+) (ans dns.RR, err error) {
+	str, ok := v.(string)
+	if !ok {
+		return nil, fmt.Errorf("value for rr type %s has type %T, not string", dns.Type(rr), v)
+	}
+
+	if rr == dns.TypeTXT {
+		return s.genAnswerTXT(req, []string{str}), nil
+	}
+
+	return s.genAnswerPTR(req, str), nil
+}
+
+// ansFromDNSRewriteMX creates a new answer resource record from the MX
+// dnsrewrite rule data.
+func (s *Server) ansFromDNSRewriteMX(
+	v rules.RRValue,
+	rr rules.RRType,
+	req *dns.Msg,
+) (ans dns.RR, err error) {
+	mx, ok := v.(*rules.DNSMX)
+	if !ok {
+		return nil, fmt.Errorf(
+			"value for rr type %s has type %T, not *rules.DNSMX",
+			dns.Type(rr),
+			v,
+		)
+	}
+
+	return s.genAnswerMX(req, mx), nil
+}
+
+// ansFromDNSRewriteSVCB creates a new answer resource record from the
+// SVCB/HTTPS dnsrewrite rule data.
+func (s *Server) ansFromDNSRewriteSVCB(
+	v rules.RRValue,
+	rr rules.RRType,
+	req *dns.Msg,
+) (ans dns.RR, err error) {
+	svcb, ok := v.(*rules.DNSSVCB)
+	if !ok {
+		return nil, fmt.Errorf(
+			"value for rr type %s has type %T, not *rules.DNSSVCB",
+			dns.Type(rr),
+			v,
+		)
+	}
+
+	if rr == dns.TypeHTTPS {
+		return s.genAnswerHTTPS(req, svcb), nil
+	}
+
+	return s.genAnswerSVCB(req, svcb), nil
+}
+
+// ansFromDNSRewriteSRV creates a new answer resource record from the SRV
+// dnsrewrite rule data.
+func (s *Server) ansFromDNSRewriteSRV(
+	v rules.RRValue,
+	rr rules.RRType,
+	req *dns.Msg,
+) (dns.RR, error) {
+	srv, ok := v.(*rules.DNSSRV)
+	if !ok {
+		return nil, fmt.Errorf(
+			"value for rr type %s has type %T, not *rules.DNSSRV",
+			dns.Type(rr),
+			v,
+		)
+	}
+
+	return s.genAnswerSRV(req, srv), nil
+}
+
 // filterDNSRewrite handles dnsrewrite filters.  It constructs a DNS response
 // and sets it into pctx.Res.  All parameters must not be nil.
 func (s *Server) filterDNSRewrite(

scripts/make/go-lint.sh

@@ -222,13 +222,13 @@ exit_on_output gofumpt --extra -e -l .
 # Apply more lax standards to the code we haven't properly refactored yet.
 gocyclo --over 17 ./internal/querylog/
 gocyclo --over 15 ./internal/home/ ./internal/dhcpd
-gocyclo --over 14 ./internal/dnsforward/
 gocyclo --over 13 ./internal/filtering/
 
 # Apply stricter standards to new or somewhat refactored code.
 gocyclo --over 10 ./internal/aghio/ ./internal/aghnet/ ./internal/aghos/\
-	./internal/aghtest/ ./internal/stats/ ./internal/tools/\
+	./internal/aghtest/ ./internal/dnsforward/ ./internal/stats/\
-	./internal/updater/ ./internal/v1/ ./internal/version/ ./main.go\
+	./internal/tools/ ./internal/updater/ ./internal/v1/ ./internal/version/\
+	./main.go
 
 ineffassign ./...