Better naming (#2104)

* Move functions from pkg/transport to pkg/parse

Although "parse" is a fairly generic name I believe this is somewhat
better named. pkg/transport keeps a few constants that are uses
throughout for the rest is is renaming a bunch (and the fallout from
there to make things compile again).

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix tests

Signed-off-by: Miek Gieben <miek@miek.nl>

plugin/pkg/parse/host.go

@@ -1,4 +1,4 @@
-package dnsutil
+package parse
 
 import (
 	"fmt"
@@ -10,15 +10,15 @@ import (
 	"github.com/miekg/dns"
 )
 
-// ParseHostPortOrFile parses the strings in s, each string can either be a
+// HostPortOrFile parses the strings in s, each string can either be a
 // address, [scheme://]address:port or a filename. The address part is checked
 // and in case of filename a resolv.conf like file is (assumed) and parsed and
 // the nameservers found are returned.
-func ParseHostPortOrFile(s ...string) ([]string, error) {
+func HostPortOrFile(s ...string) ([]string, error) {
 	var servers []string
 	for _, h := range s {
 
-		trans, host := transport.Parse(h)
+		trans, host := Transport(h)
 
 		addr, _, err := net.SplitHostPort(host)
 		if err != nil {
@@ -35,7 +35,7 @@ func ParseHostPortOrFile(s ...string) ([]string, error) {
 			var ss string
 			switch trans {
 			case transport.DNS:
-				ss = net.JoinHostPort(host, "53")
+				ss = net.JoinHostPort(host, transport.Port)
 			case transport.TLS:
 				ss = transport.TLS + "://" + net.JoinHostPort(host, transport.TLSPort)
 			case transport.GRPC:
@@ -77,9 +77,9 @@ func tryFile(s string) ([]string, error) {
 	return servers, nil
 }
 
-// ParseHostPort will check if the host part is a valid IP address, if the
+// HostPort will check if the host part is a valid IP address, if the
 // IP address is valid, but no port is found, defaultPort is added.
-func ParseHostPort(s, defaultPort string) (string, error) {
+func HostPort(s, defaultPort string) (string, error) {
 	addr, port, err := net.SplitHostPort(s)
 	if port == "" {
 		port = defaultPort

plugin/pkg/parse/host_test.go

@@ -1,12 +1,14 @@
-package dnsutil
+package parse
 
 import (
 	"io/ioutil"
 	"os"
 	"testing"
+
+	"github.com/coredns/coredns/plugin/pkg/transport"
 )
 
-func TestParseHostPortOrFile(t *testing.T) {
+func TestHostPortOrFile(t *testing.T) {
 	tests := []struct {
 		in        string
 		expected  string
@@ -41,7 +43,7 @@ func TestParseHostPortOrFile(t *testing.T) {
 	defer os.Remove("resolv.conf")
 
 	for i, tc := range tests {
-		got, err := ParseHostPortOrFile(tc.in)
+		got, err := HostPortOrFile(tc.in)
 		if err == nil && tc.shouldErr {
 			t.Errorf("Test %d, expected error, got nil", i)
 			continue
@@ -70,7 +72,7 @@ func TestParseHostPort(t *testing.T) {
 	}
 
 	for i, tc := range tests {
-		got, err := ParseHostPort(tc.in, "53")
+		got, err := HostPort(tc.in, transport.Port)
 		if err == nil && tc.shouldErr {
 			t.Errorf("Test %d, expected error, got nil", i)
 			continue

plugin/pkg/parse/parse.go

@@ -4,7 +4,8 @@ package parse
 import (
 	"fmt"
 
-	"github.com/coredns/coredns/plugin/pkg/dnsutil"
+	"github.com/coredns/coredns/plugin/pkg/transport"
+
 	"github.com/mholt/caddy"
 )
 
@@ -19,7 +20,7 @@ func Transfer(c *caddy.Controller, secondary bool) (tos, froms []string, err err
 		tos = c.RemainingArgs()
 		for i := range tos {
 			if tos[i] != "*" {
-				normalized, err := dnsutil.ParseHostPort(tos[i], "53")
+				normalized, err := HostPort(tos[i], transport.Port)
 				if err != nil {
 					return nil, nil, err
 				}
@@ -34,7 +35,7 @@ func Transfer(c *caddy.Controller, secondary bool) (tos, froms []string, err err
 		froms = c.RemainingArgs()
 		for i := range froms {
 			if froms[i] != "*" {
-				normalized, err := dnsutil.ParseHostPort(froms[i], "53")
+				normalized, err := HostPort(froms[i], transport.Port)
 				if err != nil {
 					return nil, nil, err
 				}

plugin/pkg/parse/transport.go

@@ -0,0 +1,33 @@
+package parse
+
+import (
+	"strings"
+
+	"github.com/coredns/coredns/plugin/pkg/transport"
+)
+
+// Transport returns the transport defined in s and a string where the
+// transport prefix is removed (if there was any). If no transport is defined
+// we default to TransportDNS
+func Transport(s string) (trans string, addr string) {
+	switch {
+	case strings.HasPrefix(s, transport.TLS+"://"):
+		s = s[len(transport.TLS+"://"):]
+		return transport.TLS, s
+
+	case strings.HasPrefix(s, transport.DNS+"://"):
+		s = s[len(transport.DNS+"://"):]
+		return transport.DNS, s
+
+	case strings.HasPrefix(s, transport.GRPC+"://"):
+		s = s[len(transport.GRPC+"://"):]
+		return transport.GRPC, s
+
+	case strings.HasPrefix(s, transport.HTTPS+"://"):
+		s = s[len(transport.HTTPS+"://"):]
+
+		return transport.HTTPS, s
+	}
+
+	return transport.DNS, s
+}

plugin/pkg/parse/transport_test.go

@@ -0,0 +1,25 @@
+package parse
+
+import (
+	"testing"
+
+	"github.com/coredns/coredns/plugin/pkg/transport"
+)
+
+func TestTransport(t *testing.T) {
+	for i, test := range []struct {
+		input    string
+		expected string
+	}{
+		{"dns://.:53", transport.DNS},
+		{"2003::1/64.:53", transport.DNS},
+		{"grpc://example.org:1443 ", transport.GRPC},
+		{"tls://example.org ", transport.TLS},
+		{"https://example.org ", transport.HTTPS},
+	} {
+		actual, _ := Transport(test.input)
+		if actual != test.expected {
+			t.Errorf("Test %d: Expected %s but got %s", i, test.expected, actual)
+		}
+	}
+}

plugin/pkg/transport/transport.go

@@ -1,36 +1,6 @@
 package transport
 
-import (
-	"strings"
-)
-
-// Parse returns the transport defined in s and a string where the
-// transport prefix is removed (if there was any). If no transport is defined
-// we default to TransportDNS
-func Parse(s string) (transport string, addr string) {
-	switch {
-	case strings.HasPrefix(s, TLS+"://"):
-		s = s[len(TLS+"://"):]
-		return TLS, s
-
-	case strings.HasPrefix(s, DNS+"://"):
-		s = s[len(DNS+"://"):]
-		return DNS, s
-
-	case strings.HasPrefix(s, GRPC+"://"):
-		s = s[len(GRPC+"://"):]
-		return GRPC, s
-
-	case strings.HasPrefix(s, HTTPS+"://"):
-		s = s[len(HTTPS+"://"):]
-
-		return HTTPS, s
-	}
-
-	return DNS, s
-}
-
-// Supported transports.
+// These transports are supported by CoreDNS.
 const (
 	DNS   = "dns"
 	TLS   = "tls"
@@ -38,8 +8,10 @@ const (
 	HTTPS = "https"
 )
 
-// Port numbers for the various protocols
+// Port numbers for the various transports.
 const (
+	// Port is the default port for DNS
+	Port = "53"
 	// TLSPort is the default port for DNS-over-TLS.
 	TLSPort = "853"
 	// GRPCPort is the default port for DNS-over-gRPC.

plugin/pkg/transport/transport_test.go

@@ -1,21 +0,0 @@
-package transport
-
-import "testing"
-
-func TestParse(t *testing.T) {
-	for i, test := range []struct {
-		input    string
-		expected string
-	}{
-		{"dns://.:53", DNS},
-		{"2003::1/64.:53", DNS},
-		{"grpc://example.org:1443 ", GRPC},
-		{"tls://example.org ", TLS},
-		{"https://example.org ", HTTPS},
-	} {
-		actual, _ := Parse(test.input)
-		if actual != test.expected {
-			t.Errorf("Test %d: Expected %s but got %s", i, test.expected, actual)
-		}
-	}
-}

plugin/pkg/upstream/upstream.go

@@ -6,8 +6,8 @@ import (
 	"github.com/miekg/dns"
 
 	"github.com/coredns/coredns/core/dnsserver"
-	"github.com/coredns/coredns/plugin/pkg/dnsutil"
 	"github.com/coredns/coredns/plugin/pkg/nonwriter"
+	"github.com/coredns/coredns/plugin/pkg/parse"
 	"github.com/coredns/coredns/plugin/proxy"
 	"github.com/coredns/coredns/request"
 )
@@ -27,7 +27,7 @@ func New(dests []string) (Upstream, error) {
 		return u, nil
 	}
 	u.self = false
-	ups, err := dnsutil.ParseHostPortOrFile(dests...)
+	ups, err := parse.HostPortOrFile(dests...)
 	if err != nil {
 		return u, err
 	}