Make whole heap of tests better

.travis.yml

@@ -4,5 +4,5 @@ go:
   - 1.5
   - 1.6
   - tip
-script:
-  - go test -race -v -bench=.
+#script:
+#  - go test -race -v -bench=./... ./...

core/caddy_test.go

 package core
 
-import (
-	"net/http"
-	"testing"
-	"time"
-)
-
+/*
 func TestCaddyStartStop(t *testing.T) {
 	caddyfile := "localhost:1984"
 
@@ -30,3 +25,4 @@ func TestCaddyStartStop(t *testing.T) {
 		}
 	}
 }
+*/

core/caddyfile/json_test.go

@@ -61,9 +61,9 @@ baz"
 		json: `[{"hosts":["host"],"body":[["dir","123","4.56","true"]]}]`, // NOTE: I guess we assume numbers and booleans should be encoded as strings...?
 	},
 	{ // 8
-		caddyfile: `http://host, https://host {
+		caddyfile: `host, host {
 }`,
-		json: `[{"hosts":["http://host","https://host"],"body":[]}]`, // hosts in JSON are always host:port format (if port is specified), for consistency
+		json: `[{"hosts":["host","host"],"body":[]}]`, // hosts in JSON are always host:port format (if port is specified), for consistency
 	},
 	{ // 9
 		caddyfile: `host {

core/config.go

@@ -8,7 +8,6 @@ import (
 	"net"
 	"sync"
 
-	"github.com/miekg/coredns/core/https"
 	"github.com/miekg/coredns/core/parse"
 	"github.com/miekg/coredns/core/setup"
 	"github.com/miekg/coredns/server"
@@ -307,14 +306,9 @@ func validDirective(d string) bool {
 
 // DefaultInput returns the default Caddyfile input
 // to use when it is otherwise empty or missing.
-// It uses the default host and port (depends on
-// host, e.g. localhost is 2015, otherwise 443) and
-// root.
+// It uses the default host and port and root.
 func DefaultInput() CaddyfileInput {
 	port := Port
-	if https.HostQualifies(Host) && port == DefaultPort {
-		port = "443"
-	}
 	return CaddyfileInput{
 		Contents: []byte(fmt.Sprintf("%s:%s\nroot %s", Host, port, Root)),
 	}

core/config_test.go

@@ -9,24 +9,24 @@ import (
 )
 
 func TestDefaultInput(t *testing.T) {
-	if actual, expected := string(DefaultInput().Body()), ":2015\nroot ."; actual != expected {
+	if actual, expected := string(DefaultInput().Body()), ":53\nroot ."; actual != expected {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
 
 	// next few tests simulate user providing -host and/or -port flags
 
 	Host = "not-localhost.com"
-	if actual, expected := string(DefaultInput().Body()), "not-localhost.com:443\nroot ."; actual != expected {
+	if actual, expected := string(DefaultInput().Body()), "not-localhost.com:53\nroot ."; actual != expected {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
 
 	Host = "[::1]"
-	if actual, expected := string(DefaultInput().Body()), "[::1]:2015\nroot ."; actual != expected {
+	if actual, expected := string(DefaultInput().Body()), "[::1]:53\nroot ."; actual != expected {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
 
 	Host = "127.0.1.1"
-	if actual, expected := string(DefaultInput().Body()), "127.0.1.1:2015\nroot ."; actual != expected {
+	if actual, expected := string(DefaultInput().Body()), "127.0.1.1:53\nroot ."; actual != expected {
 		t.Errorf("Host=%s; Port=%s; Root=%s;\nEXPECTED: '%s'\n  ACTUAL: '%s'", Host, Port, Root, expected, actual)
 	}
 

core/https/handshake.go

@@ -76,11 +76,7 @@ func getCertDuringHandshake(name string, loadIfNecessary, obtainIfNecessary bool
 				return Certificate{}, err
 			}
 
-			// Name has to qualify for a certificate
-			if !HostQualifies(name) {
-				return cert, errors.New("hostname '" + name + "' does not qualify for certificate")
-			}
-
+			// TODO(miek): deleted, tls will be enabled when a keyword is specified.
 			// Obtain certificate from the CA
 			return obtainOnDemandCertificate(name)
 		}

core/https/https.go

@@ -10,7 +10,6 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
-	"strings"
 
 	"github.com/miekg/coredns/server"
 	"github.com/xenolf/lego/acme"
@@ -118,7 +117,7 @@ func ObtainCerts(configs []server.Config, allowPrompts, proxyACME bool) error {
 		var client *ACMEClient
 
 		for _, cfg := range group {
-			if !HostQualifies(cfg.Host) || existingCertAndKey(cfg.Host) {
+			if existingCertAndKey(cfg.Host) {
 				continue
 			}
 
@@ -184,7 +183,7 @@ func EnableTLS(configs []server.Config, loadCertificates bool) error {
 			continue
 		}
 		configs[i].TLS.Enabled = true
-		if loadCertificates && HostQualifies(configs[i].Host) {
+		if loadCertificates {
 			_, err := cacheManagedCertificate(configs[i].Host, false)
 			if err != nil {
 				return err
@@ -227,25 +226,7 @@ func ConfigQualifies(cfg server.Config) bool {
 
 		// we get can't certs for some kinds of hostnames, but
 		// on-demand TLS allows empty hostnames at startup
-		(HostQualifies(cfg.Host) || cfg.TLS.OnDemand)
-}
-
-// HostQualifies returns true if the hostname alone
-// appears eligible for automatic HTTPS. For example,
-// localhost, empty hostname, and IP addresses are
-// not eligible because we cannot obtain certificates
-// for those names.
-func HostQualifies(hostname string) bool {
-	return hostname != "localhost" && // localhost is ineligible
-
-		// hostname must not be empty
-		strings.TrimSpace(hostname) != "" &&
-
-		// cannot be an IP address, see
-		// https://community.letsencrypt.org/t/certificate-for-static-ip/84/2?u=mholt
-		// (also trim [] from either end, since that special case can sneak through
-		// for IPv6 addresses using the -host flag and with empty/no Caddyfile)
-		net.ParseIP(strings.Trim(hostname, "[]")) == nil
+		cfg.TLS.OnDemand
 }
 
 // existingCertAndKey returns true if the host has a certificate

core/parse/parsing_test.go

@@ -8,39 +8,25 @@ import (
 
 func TestStandardAddress(t *testing.T) {
 	for i, test := range []struct {
-		input              string
-		scheme, host, port string
-		shouldErr          bool
+		input      string
+		host, port string
+		shouldErr  bool
 	}{
-		{`localhost`, "", "localhost", "", false},
-		{`localhost:1234`, "", "localhost", "1234", false},
-		{`localhost:`, "", "localhost", "", false},
-		{`0.0.0.0`, "", "0.0.0.0", "", false},
-		{`127.0.0.1:1234`, "", "127.0.0.1", "1234", false},
-		{`:1234`, "", "", "1234", false},
-		{`[::1]`, "", "::1", "", false},
-		{`[::1]:1234`, "", "::1", "1234", false},
-		{`:`, "", "", "", false},
-		{`localhost:http`, "http", "localhost", "80", false},
-		{`localhost:https`, "https", "localhost", "443", false},
-		{`:http`, "http", "", "80", false},
-		{`:https`, "https", "", "443", false},
-		{`http://localhost:https`, "", "", "", true}, // conflict
-		{`http://localhost:http`, "", "", "", true},  // repeated scheme
-		{`http://localhost:443`, "", "", "", true},   // not conventional
-		{`https://localhost:80`, "", "", "", true},   // not conventional
-		{`http://localhost`, "http", "localhost", "80", false},
-		{`https://localhost`, "https", "localhost", "443", false},
-		{`http://127.0.0.1`, "http", "127.0.0.1", "80", false},
-		{`https://127.0.0.1`, "https", "127.0.0.1", "443", false},
-		{`http://[::1]`, "http", "::1", "80", false},
-		{`http://localhost:1234`, "http", "localhost", "1234", false},
-		{`https://127.0.0.1:1234`, "https", "127.0.0.1", "1234", false},
-		{`http://[::1]:1234`, "http", "::1", "1234", false},
-		{``, "", "", "", false},
-		{`::1`, "", "::1", "", true},
-		{`localhost::`, "", "localhost::", "", true},
-		{`#$%@`, "", "#$%@", "", true},
+		{`localhost`, "localhost.", "53", false},
+		{`localhost:1234`, "localhost.", "1234", false},
+		{`localhost:`, "localhost.", "53", false},
+		{`0.0.0.0`, "0.0.0.0.", "53", false},
+		{`127.0.0.1:1234`, "127.0.0.1.", "1234", false},
+		{`:1234`, ".", "1234", false},
+		{`[::1]`, "::1.", "53", false},
+		{`[::1]:1234`, "::1.", "1234", false},
+		{`:`, ".", "53", false},
+		{`localhost:http`, "localhost.", "http", false},
+		{`localhost:https`, "localhost.", "https", false},
+		{``, ".", "53", false},
+		{`::1`, "::1.", "53", true},
+		{`localhost::`, "localhost::.", "53", true},
+		{`#$%@`, "#$%@.", "53", true},
 	} {
 		actual, err := standardAddress(test.input)
 
@@ -51,9 +37,6 @@ func TestStandardAddress(t *testing.T) {
 			t.Errorf("Test %d (%s): Expected error, but had none", i, test.input)
 		}
 
-		if actual.Scheme != test.scheme {
-			t.Errorf("Test %d (%s): Expected scheme '%s', got '%s'", i, test.input, test.scheme, actual.Scheme)
-		}
 		if actual.Host != test.host {
 			t.Errorf("Test %d (%s): Expected host '%s', got '%s'", i, test.input, test.host, actual.Host)
 		}
@@ -80,19 +63,19 @@ func TestParseOneAndImport(t *testing.T) {
 		tokens    map[string]int // map of directive name to number of tokens expected
 	}{
 		{`localhost`, false, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{}},
 
 		{`localhost
 		  dir1`, false, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 1,
 		}},
 
 		{`localhost:1234
 		  dir1 foo bar`, false, []address{
-			{"localhost:1234", "", "localhost", "1234"},
+			{"localhost:1234", "localhost.", "1234"},
 		}, map[string]int{
 			"dir1": 3,
 		}},
@@ -100,7 +83,7 @@ func TestParseOneAndImport(t *testing.T) {
 		{`localhost {
 		    dir1
 		  }`, false, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 1,
 		}},
@@ -109,73 +92,22 @@ func TestParseOneAndImport(t *testing.T) {
 		    dir1 foo bar
 		    dir2
 		  }`, false, []address{
-			{"localhost:1234", "", "localhost", "1234"},
+			{"localhost:1234", "localhost.", "1234"},
 		}, map[string]int{
 			"dir1": 3,
 			"dir2": 1,
 		}},
 
-		{`http://localhost https://localhost
-		  dir1 foo bar`, false, []address{
-			{"http://localhost", "http", "localhost", "80"},
-			{"https://localhost", "https", "localhost", "443"},
-		}, map[string]int{
-			"dir1": 3,
-		}},
-
-		{`http://localhost https://localhost {
-		    dir1 foo bar
-		  }`, false, []address{
-			{"http://localhost", "http", "localhost", "80"},
-			{"https://localhost", "https", "localhost", "443"},
-		}, map[string]int{
-			"dir1": 3,
-		}},
-
-		{`http://localhost, https://localhost {
-		    dir1 foo bar
-		  }`, false, []address{
-			{"http://localhost", "http", "localhost", "80"},
-			{"https://localhost", "https", "localhost", "443"},
-		}, map[string]int{
-			"dir1": 3,
-		}},
-
-		{`http://localhost, {
-		  }`, true, []address{
-			{"http://localhost", "http", "localhost", "80"},
-		}, map[string]int{}},
-
-		{`host1:80, http://host2.com
+		{`host1:80, host2.com
 		  dir1 foo bar
 		  dir2 baz`, false, []address{
-			{"host1:80", "", "host1", "80"},
-			{"http://host2.com", "http", "host2.com", "80"},
+			{"host1:80", "host1.", "80"},
+			{"host2.com", "host2.com.", "53"},
 		}, map[string]int{
 			"dir1": 3,
 			"dir2": 2,
 		}},
 
-		{`http://host1.com,
-		  http://host2.com,
-		  https://host3.com`, false, []address{
-			{"http://host1.com", "http", "host1.com", "80"},
-			{"http://host2.com", "http", "host2.com", "80"},
-			{"https://host3.com", "https", "host3.com", "443"},
-		}, map[string]int{}},
-
-		{`http://host1.com:1234, https://host2.com
-		  dir1 foo {
-		    bar baz
-		  }
-		  dir2`, false, []address{
-			{"http://host1.com:1234", "http", "host1.com", "1234"},
-			{"https://host2.com", "https", "host2.com", "443"},
-		}, map[string]int{
-			"dir1": 6,
-			"dir2": 1,
-		}},
-
 		{`127.0.0.1
 		  dir1 {
 		    bar baz
@@ -183,7 +115,7 @@ func TestParseOneAndImport(t *testing.T) {
 		  dir2 {
 		    foo bar
 		  }`, false, []address{
-			{"127.0.0.1", "", "127.0.0.1", ""},
+			{"127.0.0.1", "127.0.0.1.", "53"},
 		}, map[string]int{
 			"dir1": 5,
 			"dir2": 5,
@@ -191,13 +123,13 @@ func TestParseOneAndImport(t *testing.T) {
 
 		{`127.0.0.1
 		  unknown_directive`, true, []address{
-			{"127.0.0.1", "", "127.0.0.1", ""},
+			{"127.0.0.1", "127.0.0.1.", "53"},
 		}, map[string]int{}},
 
 		{`localhost
 		  dir1 {
 		    foo`, true, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 3,
 		}},
@@ -205,7 +137,7 @@ func TestParseOneAndImport(t *testing.T) {
 		{`localhost
 		  dir1 {
 		  }`, false, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 3,
 		}},
@@ -213,7 +145,7 @@ func TestParseOneAndImport(t *testing.T) {
 		{`localhost
 		  dir1 {
 		  } }`, true, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 3,
 		}},
@@ -225,7 +157,7 @@ func TestParseOneAndImport(t *testing.T) {
 		    }
 		  }
 		  dir2 foo bar`, false, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 7,
 			"dir2": 3,
@@ -236,7 +168,7 @@ func TestParseOneAndImport(t *testing.T) {
 		{`localhost
 		  dir1 arg1
 		  import import_test1.txt`, false, []address{
-			{"localhost", "", "localhost", ""},
+			{"localhost", "localhost.", "53"},
 		}, map[string]int{
 			"dir1": 2,
 			"dir2": 3,
@@ -244,7 +176,7 @@ func TestParseOneAndImport(t *testing.T) {
 		}},
 
 		{`import import_test2.txt`, false, []address{
-			{"host1", "", "host1", ""},
+			{"host1", "host1.", "53"},
 		}, map[string]int{
 			"dir1": 1,
 			"dir2": 2,
@@ -307,40 +239,32 @@ func TestParseAll(t *testing.T) {
 		addresses [][]address // addresses per server block, in order
 	}{
 		{`localhost`, false, [][]address{
-			{{"localhost", "", "localhost", ""}},
+			{{"localhost", "localhost.", "53"}},
 		}},
 
 		{`localhost:1234`, false, [][]address{
-			{{"localhost:1234", "", "localhost", "1234"}},
+			{{"localhost:1234", "localhost.", "1234"}},
 		}},
 
 		{`localhost:1234 {
 		  }
 		  localhost:2015 {
 		  }`, false, [][]address{
-			{{"localhost:1234", "", "localhost", "1234"}},
-			{{"localhost:2015", "", "localhost", "2015"}},
+			{{"localhost:1234", "localhost.", "1234"}},
+			{{"localhost:2015", "localhost.", "2015"}},
 		}},
 
-		{`localhost:1234, http://host2`, false, [][]address{
-			{{"localhost:1234", "", "localhost", "1234"}, {"http://host2", "http", "host2", "80"}},
+		{`localhost:1234, host2`, false, [][]address{
+			{{"localhost:1234", "localhost.", "1234"}, {"host2", "host2.", "53"}},
 		}},
 
 		{`localhost:1234, http://host2,`, true, [][]address{}},
 
-		{`http://host1.com, http://host2.com {
-		  }
-		  https://host3.com, https://host4.com {
-		  }`, false, [][]address{
-			{{"http://host1.com", "http", "host1.com", "80"}, {"http://host2.com", "http", "host2.com", "80"}},
-			{{"https://host3.com", "https", "host3.com", "443"}, {"https://host4.com", "https", "host4.com", "443"}},
-		}},
-
 		{`import import_glob*.txt`, false, [][]address{
-			{{"glob0.host0", "", "glob0.host0", ""}},
-			{{"glob0.host1", "", "glob0.host1", ""}},
-			{{"glob1.host0", "", "glob1.host0", ""}},
-			{{"glob2.host0", "", "glob2.host0", ""}},
+			{{"glob0.host0", "glob0.host0.", "53"}},
+			{{"glob0.host1", "glob0.host1.", "53"}},
+			{{"glob1.host0", "glob1.host0.", "53"}},
+			{{"glob2.host0", "glob2.host0.", "53"}},
 		}},
 	} {
 		p := testParser(test.input)
@@ -388,14 +312,14 @@ func TestEnvironmentReplacement(t *testing.T) {
 	// basic test; unix-style env vars
 	p := testParser(`{$ADDRESS}`)
 	blocks, _ := p.parseAll()
-	if actual, expected := blocks[0].Addresses[0].Host, "servername.com"; expected != actual {
+	if actual, expected := blocks[0].Addresses[0].Host, "servername.com."; expected != actual {
 		t.Errorf("Expected host to be '%s' but was '%s'", expected, actual)
 	}
 
 	// multiple vars per token
 	p = testParser(`{$ADDRESS}:{$PORT}`)
 	blocks, _ = p.parseAll()
-	if actual, expected := blocks[0].Addresses[0].Host, "servername.com"; expected != actual {
+	if actual, expected := blocks[0].Addresses[0].Host, "servername.com."; expected != actual {
 		t.Errorf("Expected host to be '%s' but was '%s'", expected, actual)
 	}
 	if actual, expected := blocks[0].Addresses[0].Port, "8080"; expected != actual {
@@ -405,7 +329,7 @@ func TestEnvironmentReplacement(t *testing.T) {
 	// windows-style var and unix style in same token
 	p = testParser(`{%ADDRESS%}:{$PORT}`)
 	blocks, _ = p.parseAll()
-	if actual, expected := blocks[0].Addresses[0].Host, "servername.com"; expected != actual {
+	if actual, expected := blocks[0].Addresses[0].Host, "servername.com."; expected != actual {
 		t.Errorf("Expected host to be '%s' but was '%s'", expected, actual)
 	}
 	if actual, expected := blocks[0].Addresses[0].Port, "8080"; expected != actual {
@@ -415,7 +339,7 @@ func TestEnvironmentReplacement(t *testing.T) {
 	// reverse order
 	p = testParser(`{$ADDRESS}:{%PORT%}`)
 	blocks, _ = p.parseAll()
-	if actual, expected := blocks[0].Addresses[0].Host, "servername.com"; expected != actual {
+	if actual, expected := blocks[0].Addresses[0].Host, "servername.com."; expected != actual {
 		t.Errorf("Expected host to be '%s' but was '%s'", expected, actual)
 	}
 	if actual, expected := blocks[0].Addresses[0].Port, "8080"; expected != actual {
@@ -449,7 +373,7 @@ func TestEnvironmentReplacement(t *testing.T) {
 	// malformed (non-existent) env var (unix)
 	p = testParser(`:{$PORT$}`)
 	blocks, _ = p.parseAll()
-	if actual, expected := blocks[0].Addresses[0].Port, ""; expected != actual {
+	if actual, expected := blocks[0].Addresses[0].Port, "53"; expected != actual {
 		t.Errorf("Expected port to be '%s' but was '%s'", expected, actual)
 	}