Moving TransferParse from file to its own package (#1286)

* Moving TransferParse from file to its own package

* Adding tests for parse

plugin/auto/setup.go

@@ -10,9 +10,9 @@ import (
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
-	"github.com/coredns/coredns/plugin/file"
 	"github.com/coredns/coredns/plugin/metrics"
 	"github.com/coredns/coredns/plugin/pkg/dnsutil"
+	"github.com/coredns/coredns/plugin/pkg/parse"
 	"github.com/coredns/coredns/plugin/proxy"
 
 	"github.com/mholt/caddy"
@@ -158,7 +158,7 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 				a.loader.proxy = proxy.NewLookup(ups)
 
 			default:
-				t, _, e := file.TransferParse(c, false)
+				t, _, e := parse.Transfer(c, false)
 				if e != nil {
 					return a, e
 				}

plugin/file/setup.go

@@ -1,13 +1,13 @@
 package file
 
 import (
-	"fmt"
 	"os"
 	"path"
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/plugin/pkg/dnsutil"
+	"github.com/coredns/coredns/plugin/pkg/parse"
 	"github.com/coredns/coredns/plugin/proxy"
 
 	"github.com/mholt/caddy"
@@ -97,7 +97,7 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 		for c.NextBlock() {
 			switch c.Val() {
 			case "transfer":
-				t, _, e = TransferParse(c, false)
+				t, _, e = parse.Transfer(c, false)
 				if e != nil {
 					return Zones{}, e
 				}
@@ -130,42 +130,3 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 	}
 	return Zones{Z: z, Names: names}, nil
 }
-
-// TransferParse parses transfer statements: 'transfer to [address...]'.
-func TransferParse(c *caddy.Controller, secondary bool) (tos, froms []string, err error) {
-	if !c.NextArg() {
-		return nil, nil, c.ArgErr()
-	}
-	value := c.Val()
-	switch value {
-	case "to":
-		tos = c.RemainingArgs()
-		for i := range tos {
-			if tos[i] != "*" {
-				normalized, err := dnsutil.ParseHostPort(tos[i], "53")
-				if err != nil {
-					return nil, nil, err
-				}
-				tos[i] = normalized
-			}
-		}
-
-	case "from":
-		if !secondary {
-			return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary")
-		}
-		froms = c.RemainingArgs()
-		for i := range froms {
-			if froms[i] != "*" {
-				normalized, err := dnsutil.ParseHostPort(froms[i], "53")
-				if err != nil {
-					return nil, nil, err
-				}
-				froms[i] = normalized
-			} else {
-				return nil, nil, fmt.Errorf("can't use '*' in transfer from")
-			}
-		}
-	}
-	return
-}

plugin/pkg/parse/parse.go

@@ -0,0 +1,47 @@
+package parse
+
+import (
+	"fmt"
+
+	"github.com/coredns/coredns/plugin/pkg/dnsutil"
+	"github.com/mholt/caddy"
+)
+
+// Transfer parses transfer statements: 'transfer [to|from] [address...]'.
+func Transfer(c *caddy.Controller, secondary bool) (tos, froms []string, err error) {
+	if !c.NextArg() {
+		return nil, nil, c.ArgErr()
+	}
+	value := c.Val()
+	switch value {
+	case "to":
+		tos = c.RemainingArgs()
+		for i := range tos {
+			if tos[i] != "*" {
+				normalized, err := dnsutil.ParseHostPort(tos[i], "53")
+				if err != nil {
+					return nil, nil, err
+				}
+				tos[i] = normalized
+			}
+		}
+
+	case "from":
+		if !secondary {
+			return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary")
+		}
+		froms = c.RemainingArgs()
+		for i := range froms {
+			if froms[i] != "*" {
+				normalized, err := dnsutil.ParseHostPort(froms[i], "53")
+				if err != nil {
+					return nil, nil, err
+				}
+				froms[i] = normalized
+			} else {
+				return nil, nil, fmt.Errorf("can't use '*' in transfer from")
+			}
+		}
+	}
+	return
+}

plugin/pkg/parse/parse_test.go

@@ -0,0 +1,92 @@
+package parse
+
+import (
+	"testing"
+
+	"github.com/mholt/caddy"
+)
+
+func TestTransfer(t *testing.T) {
+	tests := []struct {
+		inputFileRules string
+		shouldErr      bool
+		secondary      bool
+		expectedTo     []string
+		expectedFrom   []string
+	}{
+		// OK transfer to
+		{
+			`to 127.0.0.1`,
+			false, false, []string{"127.0.0.1:53"}, []string{},
+		},
+		// OK transfer tos
+		{
+			`to 127.0.0.1 127.0.0.2`,
+			false, false, []string{"127.0.0.1:53", "127.0.0.2:53"}, []string{},
+		},
+		// OK transfer from
+		{
+			`from 127.0.0.1`,
+			false, true, []string{}, []string{"127.0.0.1:53"},
+		},
+		// OK transfer froms
+		{
+			`from 127.0.0.1 127.0.0.2`,
+			false, true, []string{}, []string{"127.0.0.1:53", "127.0.0.2:53"},
+		},
+		// OK transfer tos/froms
+		{
+			`to 127.0.0.1 127.0.0.2
+			from 127.0.0.1 127.0.0.2`,
+			false, true, []string{"127.0.0.1:53", "127.0.0.2:53"}, []string{"127.0.0.1:53", "127.0.0.2:53"},
+		},
+		// Bad transfer from, secondary false
+		{
+			`from 127.0.0.1`,
+			true, false, []string{}, []string{},
+		},
+		// Bad transfer from garbage
+		{
+			`from !@#$%^&*()`,
+			true, true, []string{}, []string{},
+		},
+		// Bad transfer from no args
+		{
+			`from`,
+			true, false, []string{}, []string{},
+		},
+		// Bad transfer from *
+		{
+			`from *`,
+			true, true, []string{}, []string{},
+		},
+	}
+
+	for i, test := range tests {
+		c := caddy.NewTestController("dns", test.inputFileRules)
+		tos, froms, err := Transfer(c, test.secondary)
+
+		if err == nil && test.shouldErr {
+			t.Fatalf("Test %d expected errors, but got no error %+v %+v", i, err, test)
+		} else if err != nil && !test.shouldErr {
+			t.Fatalf("Test %d expected no errors, but got '%v'", i, err)
+		}
+
+		if test.expectedTo != nil {
+			for j, got := range tos {
+				if got != test.expectedTo[j] {
+					t.Fatalf("Test %d expected %v, got %v", i, test.expectedTo[j], got)
+				}
+			}
+		}
+		if test.expectedFrom != nil {
+			for j, got := range froms {
+				if got != test.expectedFrom[j] {
+					t.Fatalf("Test %d expected %v, got %v", i, test.expectedFrom[j], got)
+				}
+			}
+		}
+
+	}
+
+}

plugin/secondary/setup.go

@@ -5,6 +5,7 @@ import (
 	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/plugin/file"
 	"github.com/coredns/coredns/plugin/pkg/dnsutil"
+	"github.com/coredns/coredns/plugin/pkg/parse"
 	"github.com/coredns/coredns/plugin/proxy"
 
 	"github.com/mholt/caddy"
@@ -74,7 +75,7 @@ func secondaryParse(c *caddy.Controller) (file.Zones, error) {
 
 				switch c.Val() {
 				case "transfer":
-					t, f, e = file.TransferParse(c, true)
+					t, f, e = parse.Transfer(c, true)
 					if e != nil {
 						return file.Zones{}, e
 					}