From cb932ca23103485d67e447eeddd855007015d30e Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Wed, 19 Sep 2018 08:16:04 +0100
Subject: [PATCH] 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>
---
core/dnsserver/address.go | 3 +-
core/dnsserver/register.go | 5 +--
plugin/dnstap/setup.go | 4 +--
plugin/forward/setup.go | 6 ++--
plugin/normalize.go | 5 ++-
plugin/pkg/{dnsutil => parse}/host.go | 14 ++++-----
plugin/pkg/{dnsutil => parse}/host_test.go | 10 +++---
plugin/pkg/parse/parse.go | 7 +++--
plugin/pkg/parse/transport.go | 33 ++++++++++++++++++++
plugin/pkg/parse/transport_test.go | 25 +++++++++++++++
plugin/pkg/transport/transport.go | 36 +++-------------------
plugin/pkg/transport/transport_test.go | 21 -------------
plugin/pkg/upstream/upstream.go | 4 +--
plugin/proxy/upstream.go | 4 +--
14 files changed, 95 insertions(+), 82 deletions(-)
rename plugin/pkg/{dnsutil => parse}/host.go (86%)
rename plugin/pkg/{dnsutil => parse}/host_test.go (88%)
create mode 100644 plugin/pkg/parse/transport.go
create mode 100644 plugin/pkg/parse/transport_test.go
delete mode 100644 plugin/pkg/transport/transport_test.go
diff --git a/core/dnsserver/address.go b/core/dnsserver/address.go
index 36894aeea..5d3d2b393 100644
--- a/core/dnsserver/address.go
+++ b/core/dnsserver/address.go
@@ -6,6 +6,7 @@ import (
"strings"
"github.com/coredns/coredns/plugin"
+ "github.com/coredns/coredns/plugin/pkg/parse"
"github.com/coredns/coredns/plugin/pkg/transport"
"github.com/miekg/dns"
@@ -34,7 +35,7 @@ func normalizeZone(str string) (zoneAddr, error) {
var err error
var trans string
- trans, str = transport.Parse(str)
+ trans, str = parse.Transport(str)
host, port, ipnet, err := plugin.SplitHostPort(str)
if err != nil {
diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go
index 47595b5e3..89066d108 100644
--- a/core/dnsserver/register.go
+++ b/core/dnsserver/register.go
@@ -9,6 +9,7 @@ import (
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/dnsutil"
+ "github.com/coredns/coredns/plugin/pkg/parse"
"github.com/coredns/coredns/plugin/pkg/transport"
"github.com/mholt/caddy"
@@ -112,7 +113,7 @@ func (h *dnsContext) MakeServers() ([]caddy.Server, error) {
var servers []caddy.Server
for addr, group := range groups {
// switch on addr
- switch tr, _ := transport.Parse(addr); tr {
+ switch tr, _ := parse.Transport(addr); tr {
case transport.DNS:
s, err := NewServer(addr, group)
if err != nil {
@@ -236,7 +237,7 @@ func groupConfigsByListenAddr(configs []*Config) (map[string][]*Config, error) {
}
// DefaultPort is the default port.
-const DefaultPort = "53"
+const DefaultPort = transport.Port
// These "soft defaults" are configurable by
// command line flags, etc.
diff --git a/plugin/dnstap/setup.go b/plugin/dnstap/setup.go
index c7050b35c..70896722a 100644
--- a/plugin/dnstap/setup.go
+++ b/plugin/dnstap/setup.go
@@ -6,8 +6,8 @@ import (
"github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/dnstap/dnstapio"
- "github.com/coredns/coredns/plugin/pkg/dnsutil"
clog "github.com/coredns/coredns/plugin/pkg/log"
+ "github.com/coredns/coredns/plugin/pkg/parse"
"github.com/mholt/caddy"
"github.com/mholt/caddy/caddyfile"
@@ -44,7 +44,7 @@ func parseConfig(d *caddyfile.Dispenser) (c config, err error) {
if strings.HasPrefix(c.target, "tcp://") {
// remote IP endpoint
- servers, err := dnsutil.ParseHostPortOrFile(c.target[6:])
+ servers, err := parse.HostPortOrFile(c.target[6:])
if err != nil {
return c, d.ArgErr()
}
diff --git a/plugin/forward/setup.go b/plugin/forward/setup.go
index 6179b0d2d..5743d73b3 100644
--- a/plugin/forward/setup.go
+++ b/plugin/forward/setup.go
@@ -8,7 +8,7 @@ import (
"github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/metrics"
- "github.com/coredns/coredns/plugin/pkg/dnsutil"
+ "github.com/coredns/coredns/plugin/pkg/parse"
pkgtls "github.com/coredns/coredns/plugin/pkg/tls"
"github.com/coredns/coredns/plugin/pkg/transport"
@@ -103,14 +103,14 @@ func ParseForwardStanza(c *caddyfile.Dispenser) (*Forward, error) {
return f, c.ArgErr()
}
- toHosts, err := dnsutil.ParseHostPortOrFile(to...)
+ toHosts, err := parse.HostPortOrFile(to...)
if err != nil {
return f, err
}
transports := make([]string, len(toHosts))
for i, host := range toHosts {
- trans, h := transport.Parse(host)
+ trans, h := parse.Transport(host)
p := NewProxy(h, trans)
f.proxies = append(f.proxies, p)
transports[i] = trans
diff --git a/plugin/normalize.go b/plugin/normalize.go
index e38d5fa08..dc295ce01 100644
--- a/plugin/normalize.go
+++ b/plugin/normalize.go
@@ -6,8 +6,7 @@ import (
"strconv"
"strings"
- "github.com/coredns/coredns/plugin/pkg/transport"
-
+ "github.com/coredns/coredns/plugin/pkg/parse"
"github.com/miekg/dns"
)
@@ -64,7 +63,7 @@ type (
// of any port or transport. The host will also be fully qualified and lowercased.
func (h Host) Normalize() string {
s := string(h)
- _, s = transport.Parse(s)
+ _, s = parse.Transport(s)
// The error can be ignore here, because this function is called after the corefile has already been vetted.
host, _, _, _ := SplitHostPort(s)
diff --git a/plugin/pkg/dnsutil/host.go b/plugin/pkg/parse/host.go
similarity index 86%
rename from plugin/pkg/dnsutil/host.go
rename to plugin/pkg/parse/host.go
index b03b39586..87177125f 100644
--- a/plugin/pkg/dnsutil/host.go
+++ b/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
diff --git a/plugin/pkg/dnsutil/host_test.go b/plugin/pkg/parse/host_test.go
similarity index 88%
rename from plugin/pkg/dnsutil/host_test.go
rename to plugin/pkg/parse/host_test.go
index cc55f4570..f6e771f29 100644
--- a/plugin/pkg/dnsutil/host_test.go
+++ b/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
diff --git a/plugin/pkg/parse/parse.go b/plugin/pkg/parse/parse.go
index 17d2641ef..17fe75fb0 100644
--- a/plugin/pkg/parse/parse.go
+++ b/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
}
diff --git a/plugin/pkg/parse/transport.go b/plugin/pkg/parse/transport.go
new file mode 100644
index 000000000..d632120d7
--- /dev/null
+++ b/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
+}
diff --git a/plugin/pkg/parse/transport_test.go b/plugin/pkg/parse/transport_test.go
new file mode 100644
index 000000000..d0e0fcddd
--- /dev/null
+++ b/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)
+ }
+ }
+}
diff --git a/plugin/pkg/transport/transport.go b/plugin/pkg/transport/transport.go
index 690b7768c..85b3bee5f 100644
--- a/plugin/pkg/transport/transport.go
+++ b/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.
diff --git a/plugin/pkg/transport/transport_test.go b/plugin/pkg/transport/transport_test.go
deleted file mode 100644
index 5f93266eb..000000000
--- a/plugin/pkg/transport/transport_test.go
+++ /dev/null
@@ -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)
- }
- }
-}
diff --git a/plugin/pkg/upstream/upstream.go b/plugin/pkg/upstream/upstream.go
index 466da8990..239d3dd96 100644
--- a/plugin/pkg/upstream/upstream.go
+++ b/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
}
diff --git a/plugin/proxy/upstream.go b/plugin/proxy/upstream.go
index 72d9e04a7..0d62f45e2 100644
--- a/plugin/proxy/upstream.go
+++ b/plugin/proxy/upstream.go
@@ -7,8 +7,8 @@ import (
"time"
"github.com/coredns/coredns/plugin"
- "github.com/coredns/coredns/plugin/pkg/dnsutil"
"github.com/coredns/coredns/plugin/pkg/healthcheck"
+ "github.com/coredns/coredns/plugin/pkg/parse"
"github.com/coredns/coredns/plugin/pkg/tls"
"github.com/mholt/caddy/caddyfile"
"github.com/miekg/dns"
@@ -60,7 +60,7 @@ func NewStaticUpstream(c *caddyfile.Dispenser) (Upstream, error) {
}
// process the host list, substituting in any nameservers in files
- toHosts, err := dnsutil.ParseHostPortOrFile(to...)
+ toHosts, err := parse.HostPortOrFile(to...)
if err != nil {
return upstream, err
}