From f2e547a54e50162caa25b562e49e2846dd92b575 Mon Sep 17 00:00:00 2001
From: Aleksey Dmitrevskiy <ad@adguard.com>
Date: Wed, 6 Mar 2019 16:17:15 +0300
Subject: [PATCH] [change] config, control, openapi: fix issues from reviw

---
 config.go            |  2 +-
 control.go           | 30 ++++++++++++++++--------------
 openapi/openapi.yaml | 20 +++-----------------
 3 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/config.go b/config.go
index 83bd1764..d6178c38 100644
--- a/config.go
+++ b/config.go
@@ -60,7 +60,7 @@ type dnsConfig struct {
 	UpstreamDNS []string `yaml:"upstream_dns"`
 }
 
-var defaultDNS = []string{"tls://1.1.1.1", "tls://1.0.0.1"}
+var defaultDNS = []string{"https://dns.adguard.com/dns-query", "https://dns.cloudflare.com/dns-query"}
 var defaultBootstrap = []string{"1.1.1.1"}
 
 type tlsConfigSettings struct {
diff --git a/control.go b/control.go
index 6f36c3ca..d4daa621 100644
--- a/control.go
+++ b/control.go
@@ -327,20 +327,18 @@ func handleSetUpstreamConfig(w http.ResponseWriter, r *http.Request) {
 		config.DNS.UpstreamDNS = newconfig.Upstreams
 	}
 
-	// bootstrap servers are plain DNS only. We should remove tls:// https:// and sdns:// hosts from slice
-	bootstraps := []string{}
+	// bootstrap servers are plain DNS only. We should return http error if there are tls:// https:// or sdns:// hosts in slice
 	for _, host := range newconfig.BootstrapDNS {
 		err := checkBootstrapDNS(host)
 		if err != nil {
-			log.Tracef("%s can not be used as bootstrap DNS cause: %s", host, err)
-			continue
+			httpError(w, http.StatusBadRequest, "%s can not be used as bootstrap dns cause: %s", host, err)
+			return
 		}
-		bootstraps = append(bootstraps, host)
 	}
 
 	config.DNS.BootstrapDNS = defaultBootstrap
-	if len(bootstraps) > 0 {
-		config.DNS.BootstrapDNS = bootstraps
+	if len(newconfig.BootstrapDNS) > 0 {
+		config.DNS.BootstrapDNS = newconfig.BootstrapDNS
 	}
 
 	config.DNS.AllServers = newconfig.AllServers
@@ -361,22 +359,22 @@ func checkBootstrapDNS(host string) error {
 
 func handleTestUpstreamDNS(w http.ResponseWriter, r *http.Request) {
 	log.Tracef("%s %v", r.Method, r.URL)
-	hosts := []string{}
-	err := json.NewDecoder(r.Body).Decode(&hosts)
+	upstreamConfig := upstreamConfig{}
+	err := json.NewDecoder(r.Body).Decode(&upstreamConfig)
 	if err != nil {
 		httpError(w, http.StatusBadRequest, "Failed to read request body: %s", err)
 		return
 	}
 
-	if len(hosts) == 0 {
+	if len(upstreamConfig.Upstreams) == 0 {
 		httpError(w, http.StatusBadRequest, "No servers specified")
 		return
 	}
 
 	result := map[string]string{}
 
-	for _, host := range hosts {
-		err = checkDNS(host)
+	for _, host := range upstreamConfig.Upstreams {
+		err = checkDNS(host, upstreamConfig.BootstrapDNS)
 		if err != nil {
 			log.Info("%v", err)
 			result[host] = err.Error()
@@ -398,9 +396,13 @@ func handleTestUpstreamDNS(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func checkDNS(input string) error {
+func checkDNS(input string, bootstrap []string) error {
+	if len(bootstrap) == 0 {
+		bootstrap = defaultBootstrap
+	}
+
 	log.Debug("Checking if DNS %s works...", input)
-	u, err := upstream.AddressToUpstream(input, upstream.Options{Timeout: dnsforward.DefaultTimeout})
+	u, err := upstream.AddressToUpstream(input, upstream.Options{Bootstrap: bootstrap, Timeout: dnsforward.DefaultTimeout})
 	if err != nil {
 		return fmt.Errorf("failed to choose upstream for %s: %s", input, err)
 	}
diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml
index d84e90ec..c40648bc 100644
--- a/openapi/openapi.yaml
+++ b/openapi/openapi.yaml
@@ -111,15 +111,15 @@ paths:
             tags:
                 - global
             operationId: testUpstreamDNS
-            summary: "Test upstream DNS"
+            summary: "Test upstream configuration"
             consumes:
                 - application/json
             parameters:
                 -   in: "body"
                     name: "body"
-                    description: "Upstream servers to be tested"
+                    description: "Upstream configuration to be tested"
                     schema:
-                        $ref: "#/definitions/TestUpstreams"
+                        $ref: "#/definitions/UpstreamsConfig"
             responses:
                 200:
                     description: 'Status of testing each requested server, with "OK" meaning that server works, any other text means an error.'
@@ -814,20 +814,6 @@ definitions:
             all_servers:
                 type: "boolean"
                 description: "If true, parallel queries to all configured upstream servers are enabled"
-    TestUpstreams:
-        type: "object"
-        description: "Upstream servers to be tested"
-        required:
-            - "upstream_dns"
-        properties:
-            upstream_dns:
-                type: "array"
-                description: 'Upstream servers, port is optional after colon'
-                items:
-                    type: "string"
-                example:
-                    - "tls://1.1.1.1"
-                    - "tls://1.0.0.1"
     Filter:
         type: "object"
         description: "Filter subscription info"