Revert "Nested policy configuration (#6)"

This reverts commit 48958d5.

README.md

@@ -26,9 +26,9 @@ Each incoming DNS query that hits the CoreDNS fanout plugin will be replicated i
 * `worker-count` is the number of parallel queries per request. By default equals to count of IP list. Use this only for reducing parallel queries per request.
 * `policy` - specifies the policy of DNS server selection mechanism. The default is `sequential`.
   * `sequential` - select DNS servers one-by-one based on its order
-  * `weighted-random` - select DNS servers randomly based on `server-count` and `load-factor` params:
-    * `server-count` is the number of DNS servers to be requested. Equals to the number of specified IPs by default.
-    * `load-factor` - the probability of selecting a server. This is specified in the order of the list of IP addresses and takes values between 1 and 100. By default, all servers have an equal probability of 100.
+  * `weighted-random` - select DNS servers randomly based on `weighted-random-server-count` and `weighted-random-load-factor` params.
+* `weighted-random-server-count` is the number of DNS servers to be requested. Equals to the number of specified IPs by default. Used only with the `weighted-random` policy.
+* `weighted-random-load-factor` - the probability of selecting a server. This is specified in the order of the list of IP addresses and takes values between 1 and 100. By default, all servers have an equal probability of 100. Used only with the `weighted-random` policy.
 * `network` is a specific network protocol. Could be `tcp`, `udp`, `tcp-tls`.
 * `except` is a list is a space-separated list of domains to exclude from proxying.
 * `except-file` is the path to file with line-separated list of domains to exclude from proxying.
@@ -116,14 +116,13 @@ If `race` is enable, we will get `NXDOMAIN` result quickly, otherwise we will ge
 }
 ~~~
 
-Sends parallel requests between two randomly selected resolvers. Note, that `127.0.0.1:9007` would be selected more frequently as it has the highest `load-factor`.
+Sends parallel requests between two randomly selected resolvers. Note, that `127.0.0.1:9007` would be selected more frequently as it has the highest `weighted-random-load-factor`.
 ~~~ corefile
 example.org {
     fanout . 127.0.0.1:9005 127.0.0.1:9006 127.0.0.1:9007 {
-      policy weighted-random {
-        server-count 2
-        load-factor 50 70 100
-      }
+      policy weighted-random
+      weighted-random-server-count 2
+      weighted-random-load-factor 50 70 100
     }
 }
 ~~~
@@ -132,7 +131,7 @@ Sends parallel requests between three resolver sequentially (default mode).
 ~~~ corefile
 example.org {
     fanout . 127.0.0.1:9005 127.0.0.1:9006 127.0.0.1:9007 {
-      policy sequential
+        policy sequential
     }
 }
 ~~~

fanout.go

@@ -49,6 +49,8 @@ type Fanout struct {
 	attempts              int
 	workerCount           int
 	serverCount           int
+	loadFactor            []int
+	policyType            string
 	serverSelectionPolicy policy
 	tapPlugin             *dnstap.Dnstap
 	Next                  plugin.Handler

setup.go

@@ -122,14 +122,15 @@ func parsefanoutStanza(c *caddyfile.Dispenser) (*Fanout, error) {
 		return f, err
 	}
 	for c.NextBlock() {
-		err = parseValue(strings.ToLower(c.Val()), f, c, toHosts)
+		err = parseValue(strings.ToLower(c.Val()), f, c)
 		if err != nil {
 			return nil, err
 		}
 	}
 	initClients(f, toHosts)
-	if f.serverCount > len(f.clients) || f.serverCount == 0 {
-		f.serverCount = len(f.clients)
+	err = initServerSelectionPolicy(f)
+	if err != nil {
+		return nil, err
 	}
 
 	if f.workerCount > len(f.clients) || f.workerCount == 0 {
@@ -155,7 +156,30 @@ func initClients(f *Fanout, hosts []string) {
 	}
 }
 
-func parseValue(v string, f *Fanout, c *caddyfile.Dispenser, hosts []string) error {
+func initServerSelectionPolicy(f *Fanout) error {
+	if f.serverCount > len(f.clients) || f.serverCount == 0 {
+		f.serverCount = len(f.clients)
+	}
+
+	loadFactor := f.loadFactor
+	if len(loadFactor) == 0 {
+		for i := 0; i < len(f.clients); i++ {
+			loadFactor = append(loadFactor, maxLoadFactor)
+		}
+	}
+	if len(loadFactor) != len(f.clients) {
+		return errors.New("load-factor params count must be the same as the number of hosts")
+	}
+
+	f.serverSelectionPolicy = &sequentialPolicy{}
+	if f.policyType == policyWeightedRandom {
+		f.serverSelectionPolicy = &weightedPolicy{loadFactor: loadFactor}
+	}
+
+	return nil
+}
+
+func parseValue(v string, f *Fanout, c *caddyfile.Dispenser) error {
 	switch v {
 	case "tls":
 		return parseTLS(f, c)
@@ -166,7 +190,13 @@ func parseValue(v string, f *Fanout, c *caddyfile.Dispenser, hosts []string) err
 	case "worker-count":
 		return parseWorkerCount(f, c)
 	case "policy":
-		return parsePolicy(f, c, hosts)
+		return parsePolicy(f, c)
+	case "weighted-random-server-count":
+		serverCount, err := parsePositiveInt(c)
+		f.serverCount = serverCount
+		return err
+	case "weighted-random-load-factor":
+		return parseLoadFactor(f, c)
 	case "timeout":
 		return parseTimeout(f, c)
 	case "race":
@@ -184,55 +214,16 @@ func parseValue(v string, f *Fanout, c *caddyfile.Dispenser, hosts []string) err
 	}
 }
 
-func parsePolicy(f *Fanout, c *caddyfile.Dispenser, hosts []string) error {
+func parsePolicy(f *Fanout, c *caddyfile.Dispenser) error {
 	if !c.NextArg() {
 		return c.ArgErr()
 	}
 
-	switch c.Val() {
-	case policyWeightedRandom:
-		// omit "{"
-		c.Next()
-		if c.Val() != "{" {
-			return c.Err("Wrong policy configuration")
-		}
-	case policySequential:
-		f.serverSelectionPolicy = &sequentialPolicy{}
-		return nil
-	default:
+	policyType := strings.ToLower(c.Val())
+	if policyType != policyWeightedRandom && policyType != policySequential {
 		return errors.Errorf("unknown policy %q", c.Val())
 	}
-
-	var loadFactor []int
-	for c.Next() {
-		if c.Val() == "}" {
-			break
-		}
-
-		var err error
-		switch c.Val() {
-		case "server-count":
-			f.serverCount, err = parsePositiveInt(c)
-		case "load-factor":
-			loadFactor, err = parseLoadFactor(c)
-		default:
-			return errors.Errorf("unknown property %q", c.Val())
-		}
-		if err != nil {
-			return err
-		}
-	}
-
-	if len(loadFactor) == 0 {
-		for i := 0; i < len(hosts); i++ {
-			loadFactor = append(loadFactor, maxLoadFactor)
-		}
-	}
-	if len(loadFactor) != len(hosts) {
-		return errors.New("load-factor params count must be the same as the number of hosts")
-	}
-
-	f.serverSelectionPolicy = &weightedPolicy{loadFactor: loadFactor}
+	f.policyType = policyType
 
 	return nil
 }
@@ -304,30 +295,29 @@ func parseWorkerCount(f *Fanout, c *caddyfile.Dispenser) error {
 	return err
 }
 
-func parseLoadFactor(c *caddyfile.Dispenser) ([]int, error) {
+func parseLoadFactor(f *Fanout, c *caddyfile.Dispenser) error {
 	args := c.RemainingArgs()
 	if len(args) == 0 {
-		return nil, c.ArgErr()
+		return c.ArgErr()
 	}
 
-	result := make([]int, 0, len(args))
 	for _, arg := range args {
 		loadFactor, err := strconv.Atoi(arg)
 		if err != nil {
-			return nil, c.ArgErr()
+			return c.ArgErr()
 		}
 
 		if loadFactor < minLoadFactor {
-			return nil, errors.New("load-factor should be more or equal 1")
+			return errors.New("load-factor should be more or equal 1")
 		}
 		if loadFactor > maxLoadFactor {
-			return nil, errors.Errorf("load-factor %d should be less than %d", loadFactor, maxLoadFactor)
+			return errors.Errorf("load-factor %d should be less than %d", loadFactor, maxLoadFactor)
 		}
 
-		result = append(result, loadFactor)
+		f.loadFactor = append(f.loadFactor, loadFactor)
 	}
 
-	return result, nil
+	return nil
 }
 
 func parsePositiveInt(c *caddyfile.Dispenser) (int, error) {

setup_test.go

@@ -40,18 +40,19 @@ func TestSetup(t *testing.T) {
 		expectedNetwork     string
 		expectedServerCount int
 		expectedLoadFactor  []int
+		expectedPolicy      string
 		expectedErr         string
 	}{
 		// positive
-		{input: "fanout . 127.0.0.1 {\npolicy weighted-random {\nserver-count 5 load-factor 100\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 1, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: []int{100}},
-		{input: "fanout . 127.0.0.1", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 1, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: nil},
-		{input: "fanout . 127.0.0.1 {\npolicy weighted-random {\nserver-count 5 load-factor 100\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 1, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: []int{100}},
-		{input: "fanout . 127.0.0.1 {\nexcept a b\nworker-count 3\n}", expectedFrom: ".", expectedTimeout: defaultTimeout, expectedAttempts: 3, expectedWorkers: 1, expectedIgnored: []string{"a.", "b."}, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: nil},
-		{input: "fanout . 127.0.0.1 127.0.0.2 {\nnetwork tcp\n}", expectedFrom: ".", expectedTimeout: defaultTimeout, expectedAttempts: 3, expectedWorkers: 2, expectedNetwork: "tcp", expectedTo: []string{"127.0.0.1:53", "127.0.0.2:53"}, expectedServerCount: 2, expectedLoadFactor: nil},
-		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 127.0.0.4 {\nworker-count 3\ntimeout 1m\n}", expectedTimeout: time.Minute, expectedAttempts: 3, expectedFrom: ".", expectedWorkers: 3, expectedNetwork: "udp", expectedServerCount: 4, expectedLoadFactor: nil},
-		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 127.0.0.4 {\nattempt-count 2\n}", expectedTimeout: defaultTimeout, expectedFrom: ".", expectedAttempts: 2, expectedWorkers: 4, expectedNetwork: "udp", expectedServerCount: 4, expectedLoadFactor: nil},
-		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 {\npolicy weighted-random {}\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 3, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 3, expectedLoadFactor: []int{100, 100, 100}},
-		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 {\npolicy sequential\nworker-count 3\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 3, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 3, expectedLoadFactor: nil},
+		{input: "fanout . 127.0.0.1 {\npolicy weighted-random \nweighted-random-server-count 5 weighted-random-load-factor 100\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 1, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: []int{100}, expectedPolicy: policyWeightedRandom},
+		{input: "fanout . 127.0.0.1", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 1, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: nil, expectedPolicy: ""},
+		{input: "fanout . 127.0.0.1 {\npolicy weighted-random \nserver-count 5 load-factor 100\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 1, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: []int{100}, expectedPolicy: policyWeightedRandom},
+		{input: "fanout . 127.0.0.1 {\nexcept a b\nworker-count 3\n}", expectedFrom: ".", expectedTimeout: defaultTimeout, expectedAttempts: 3, expectedWorkers: 1, expectedIgnored: []string{"a.", "b."}, expectedNetwork: "udp", expectedServerCount: 1, expectedLoadFactor: nil, expectedPolicy: ""},
+		{input: "fanout . 127.0.0.1 127.0.0.2 {\nnetwork tcp\n}", expectedFrom: ".", expectedTimeout: defaultTimeout, expectedAttempts: 3, expectedWorkers: 2, expectedNetwork: "tcp", expectedTo: []string{"127.0.0.1:53", "127.0.0.2:53"}, expectedServerCount: 2, expectedLoadFactor: nil, expectedPolicy: ""},
+		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 127.0.0.4 {\nworker-count 3\ntimeout 1m\n}", expectedTimeout: time.Minute, expectedAttempts: 3, expectedFrom: ".", expectedWorkers: 3, expectedNetwork: "udp", expectedServerCount: 4, expectedLoadFactor: nil, expectedPolicy: ""},
+		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 127.0.0.4 {\nattempt-count 2\n}", expectedTimeout: defaultTimeout, expectedFrom: ".", expectedAttempts: 2, expectedWorkers: 4, expectedNetwork: "udp", expectedServerCount: 4, expectedLoadFactor: nil, expectedPolicy: ""},
+		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 {\npolicy weighted-random \n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 3, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 3, expectedLoadFactor: []int{100, 100, 100}, expectedPolicy: policyWeightedRandom},
+		{input: "fanout . 127.0.0.1 127.0.0.2 127.0.0.3 {\npolicy sequential\nworker-count 3\n}", expectedFrom: ".", expectedAttempts: 3, expectedWorkers: 3, expectedTimeout: defaultTimeout, expectedNetwork: "udp", expectedServerCount: 3, expectedLoadFactor: nil, expectedPolicy: policySequential},
 
 		// negative
 		{input: "fanout . aaa", expectedErr: "not an IP address or file"},
@@ -60,13 +61,12 @@ func TestSetup(t *testing.T) {
 		{input: "fanout . 127.0.0.1 {\nexcept a b\nworker-count ten\n}", expectedErr: "'ten'"},
 		{input: "fanout . 127.0.0.1 {\nexcept a:\nworker-count ten\n}", expectedErr: "unable to normalize 'a:'"},
 		{input: "fanout . 127.0.0.1 127.0.0.2 {\nnetwork XXX\n}", expectedErr: "unknown network protocol"},
-		{input: "fanout . 127.0.0.1 {\npolicy weighted-random {\nserver-count -100\n}\n}", expectedErr: "Wrong argument count or unexpected line ending"},
-		{input: "fanout . 127.0.0.1 {\npolicy weighted-random {\nload-factor 150\n}\n}", expectedErr: "load-factor 150 should be less than 100"},
-		{input: "fanout . 127.0.0.1 {\npolicy weighted-random {\nload-factor 0\n}\n}", expectedErr: "load-factor should be more or equal 1"},
-		{input: "fanout . 127.0.0.1 {\npolicy weighted-random {\nload-factor 50 100\n}\n}", expectedErr: "load-factor params count must be the same as the number of hosts"},
-		{input: "fanout . 127.0.0.1 127.0.0.2 {\npolicy weighted-random {\nload-factor 50\n}\n}", expectedErr: "load-factor params count must be the same as the number of hosts"},
-		{input: "fanout . 127.0.0.1 127.0.0.2 {\npolicy weighted-random {\nload-factor \n}\n}", expectedErr: "Wrong argument count or unexpected line ending"},
-		{input: "fanout . 127.0.0.1 127.0.0.2 {\npolicy weighted-random\nworker-count 10\n}", expectedErr: "Wrong policy configuration"},
+		{input: "fanout . 127.0.0.1 {\npolicy weighted-random \nweighted-random-server-count -100\n}", expectedErr: "Wrong argument count or unexpected line ending"},
+		{input: "fanout . 127.0.0.1 {\npolicy weighted-random \nweighted-random-load-factor 150\n}", expectedErr: "load-factor 150 should be less than 100"},
+		{input: "fanout . 127.0.0.1 {\npolicy weighted-random \nweighted-random-load-factor 0\n}", expectedErr: "load-factor should be more or equal 1"},
+		{input: "fanout . 127.0.0.1 {\npolicy weighted-random \nweighted-random-load-factor 50 100\n}", expectedErr: "load-factor params count must be the same as the number of hosts"},
+		{input: "fanout . 127.0.0.1 127.0.0.2 {\npolicy weighted-random \nweighted-random-load-factor 50\n}", expectedErr: "load-factor params count must be the same as the number of hosts"},
+		{input: "fanout . 127.0.0.1 127.0.0.2 {\npolicy weighted-random \nweighted-random-load-factor \n}", expectedErr: "Wrong argument count or unexpected line ending"},
 	}
 
 	for i, test := range tests {
@@ -116,6 +116,9 @@ func TestSetup(t *testing.T) {
 		if f.serverCount != test.expectedServerCount {
 			t.Fatalf("Test %d: expected: %d, got: %d", i, test.expectedServerCount, f.serverCount)
 		}
+		if f.policyType != test.expectedPolicy {
+			t.Fatalf("Test %d: expected: %s, got: %s", i, test.expectedPolicy, f.policyType)
+		}
 
 		selectionPolicy, ok := f.serverSelectionPolicy.(*weightedPolicy)
 		if len(test.expectedLoadFactor) > 0 {