Merge pull request #43510 from thaJeztah/daemon_fix_hosts_validation_step1b

opts: remove hacks for old go versions, and improve coverage

opts/hosts.go

@@ -132,13 +132,6 @@ func ParseTCPAddr(tryAddr string, defaultAddr string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	// url.Parse fails for trailing colon on IPv6 brackets on Go 1.5, but
-	// not 1.4. See https://github.com/golang/go/issues/12200 and
-	// https://github.com/golang/go/issues/6530.
-	if strings.HasSuffix(addr, "]:") {
-		addr += defaultPort
-	}
-
 	u, err := url.Parse("tcp://" + addr)
 	if err != nil {
 		return "", err

opts/hosts_test.go

@@ -7,20 +7,20 @@ import (
 )
 
 func TestParseHost(t *testing.T) {
-	invalid := []string{
-		"something with spaces",
-		"://",
-		"unknown://",
-		"tcp://:port",
-		"tcp://invalid:port",
-		"tcp://:5555/",
-		"tcp://:5555/p",
-		"tcp://0.0.0.0:5555/",
-		"tcp://0.0.0.0:5555/p",
-		"tcp://[::1]:/",
-		"tcp://[::1]:5555/",
-		"tcp://[::1]:5555/p",
-		" tcp://:5555/path ",
+	invalid := map[string]string{
+		"something with spaces": `parse "tcp://something with spaces": invalid character " " in host name`,
+		"://":                   `Invalid bind address format: ://`,
+		"unknown://":            `Invalid bind address format: unknown://`,
+		"tcp://:port":           `parse "tcp://:port": invalid port ":port" after host`,
+		"tcp://invalid:port":    `parse "tcp://invalid:port": invalid port ":port" after host`,
+		"tcp://:5555/":          `invalid bind address (:5555/): should not contain a path element`,
+		"tcp://:5555/p":         `invalid bind address (:5555/p): should not contain a path element`,
+		"tcp://0.0.0.0:5555/":   `invalid bind address (0.0.0.0:5555/): should not contain a path element`,
+		"tcp://0.0.0.0:5555/p":  `invalid bind address (0.0.0.0:5555/p): should not contain a path element`,
+		"tcp://[::1]:/":         `invalid bind address ([::1]:/): should not contain a path element`,
+		"tcp://[::1]:5555/":     `invalid bind address ([::1]:5555/): should not contain a path element`,
+		"tcp://[::1]:5555/p":    `invalid bind address ([::1]:5555/p): should not contain a path element`,
+		" tcp://:5555/path ":    `invalid bind address (:5555/path): should not contain a path element`,
 	}
 
 	valid := map[string]string{
@@ -31,41 +31,51 @@ func TestParseHost(t *testing.T) {
 		"fd://something":           "fd://something",
 		"tcp://host:":              fmt.Sprintf("tcp://host:%d", DefaultHTTPPort),
 		"tcp://":                   DefaultTCPHost,
-		"tcp://:2375":              fmt.Sprintf("tcp://%s:2375", DefaultHTTPHost),
-		"tcp://:2376":              fmt.Sprintf("tcp://%s:2376", DefaultHTTPHost),
-		"tcp://0.0.0.0:8080":       "tcp://0.0.0.0:8080",
-		"tcp://192.168.0.0:12000":  "tcp://192.168.0.0:12000",
-		"tcp://192.168:8080":       "tcp://192.168:8080",
+		"tcp://:":                  DefaultTCPHost,
+		"tcp://:5555":              fmt.Sprintf("tcp://%s:5555", DefaultHTTPHost),
+		"tcp://[::1]:":             fmt.Sprintf(`tcp://[::1]:%d`, DefaultHTTPPort),
+		"tcp://[::1]:5555":         `tcp://[::1]:5555`,
+		"tcp://0.0.0.0:5555":       "tcp://0.0.0.0:5555",
+		"tcp://192.168:5555":       "tcp://192.168:5555",
+		"tcp://192.168.0.1:5555":   "tcp://192.168.0.1:5555",
 		"tcp://0.0.0.0:1234567890": "tcp://0.0.0.0:1234567890", // yeah it's valid :P
-		"tcp://docker.com:2375":    "tcp://docker.com:2375",
+		"tcp://docker.com:5555":    "tcp://docker.com:5555",
 		"unix://":                  "unix://" + DefaultUnixSocket,
 		"unix://path/to/socket":    "unix://path/to/socket",
 		"npipe://":                 "npipe://" + DefaultNamedPipe,
 		"npipe:////./pipe/foo":     "npipe:////./pipe/foo",
 	}
 
-	for _, value := range invalid {
-		if _, err := ParseHost(false, false, value); err == nil {
-			t.Errorf("Expected an error for %v, got [nil]", value)
-		}
+	for value, expectedError := range invalid {
+		t.Run(value, func(t *testing.T) {
+			_, err := ParseHost(false, false, value)
+			if err == nil || err.Error() != expectedError {
+				t.Errorf(`expected error "%s", got "%v"`, expectedError, err)
+			}
+		})
 	}
 
 	for value, expected := range valid {
-		if actual, err := ParseHost(false, false, value); err != nil || actual != expected {
-			t.Errorf("Expected for %v [%v], got [%v, %v]", value, expected, actual, err)
-		}
+		t.Run(value, func(t *testing.T) {
+			actual, err := ParseHost(false, false, value)
+			if err != nil {
+				t.Errorf(`unexpected error: "%v"`, err)
+			}
+			if actual != expected {
+				t.Errorf(`expected "%s", got "%s""`, expected, actual)
+			}
+		})
 	}
 }
 
 func TestParseDockerDaemonHost(t *testing.T) {
 	invalids := map[string]string{
-
-		"tcp:a.b.c.d":                   "",
-		"tcp:a.b.c.d/path":              "",
+		"tcp:a.b.c.d":                   `parse "tcp://tcp:a.b.c.d": invalid port ":a.b.c.d" after host`,
+		"tcp:a.b.c.d/path":              `parse "tcp://tcp:a.b.c.d/path": invalid port ":a.b.c.d" after host`,
 		"udp://127.0.0.1":               "Invalid bind address format: udp://127.0.0.1",
-		"udp://127.0.0.1:2375":          "Invalid bind address format: udp://127.0.0.1:2375",
+		"udp://127.0.0.1:5555":          "Invalid bind address format: udp://127.0.0.1:5555",
 		"tcp://unix:///run/docker.sock": "Invalid proto, expected tcp: unix:///run/docker.sock",
-		" tcp://:7777/path ":            "Invalid bind address format:  tcp://:7777/path ",
+		" tcp://:5555/path ":            "Invalid bind address format:  tcp://:5555/path ",
 		"":                              "Invalid bind address format: ",
 		":5555/path":                    "invalid bind address (:5555/path): should not contain a path element",
 		"0.0.0.1:5555/path":             "invalid bind address (0.0.0.1:5555/path): should not contain a path element",
@@ -75,41 +85,61 @@ func TestParseDockerDaemonHost(t *testing.T) {
 		"localhost:5555/path":           "invalid bind address (localhost:5555/path): should not contain a path element",
 	}
 	valids := map[string]string{
-		"0.0.0.1:":                "tcp://0.0.0.1:2375",
+		":":                       DefaultTCPHost,
+		":5555":                   fmt.Sprintf("tcp://%s:5555", DefaultHTTPHost),
+		"0.0.0.1:":                fmt.Sprintf("tcp://0.0.0.1:%d", DefaultHTTPPort),
 		"0.0.0.1:5555":            "tcp://0.0.0.1:5555",
-		"[::1]:":                  "tcp://[::1]:2375",
-		"[0:0:0:0:0:0:0:1]:":      "tcp://[0:0:0:0:0:0:0:1]:2375",
-		":6666":                   fmt.Sprintf("tcp://%s:6666", DefaultHTTPHost),
-		"tcp://":                  DefaultTCPHost,
-		"tcp://:7777":             fmt.Sprintf("tcp://%s:7777", DefaultHTTPHost),
-		"unix:///run/docker.sock": "unix:///run/docker.sock",
-		"unix://":                 "unix://" + DefaultUnixSocket,
+		"[::1]:":                  fmt.Sprintf("tcp://[::1]:%d", DefaultHTTPPort),
+		"[::1]:5555":              "tcp://[::1]:5555",
+		"[0:0:0:0:0:0:0:1]:":      fmt.Sprintf("tcp://[0:0:0:0:0:0:0:1]:%d", DefaultHTTPPort),
+		"[0:0:0:0:0:0:0:1]:5555":  "tcp://[0:0:0:0:0:0:0:1]:5555",
+		"localhost":               fmt.Sprintf("tcp://localhost:%d", DefaultHTTPPort),
+		"localhost:":              fmt.Sprintf("tcp://localhost:%d", DefaultHTTPPort),
+		"localhost:5555":          "tcp://localhost:5555",
 		"fd://":                   "fd://",
 		"fd://something":          "fd://something",
-		"localhost:":              "tcp://localhost:2375",
-		"localhost:5555":          "tcp://localhost:5555",
+		"npipe://":                "npipe://" + DefaultNamedPipe,
+		"npipe:////./pipe/foo":    "npipe:////./pipe/foo",
+		"tcp://":                  DefaultTCPHost,
+		"tcp://:5555":             fmt.Sprintf("tcp://%s:5555", DefaultHTTPHost),
+		"tcp://[::1]:":            fmt.Sprintf("tcp://[::1]:%d", DefaultHTTPPort),
+		"tcp://[::1]:5555":        "tcp://[::1]:5555",
+		"unix://":                 "unix://" + DefaultUnixSocket,
+		"unix:///run/docker.sock": "unix:///run/docker.sock",
 	}
 	for invalidAddr, expectedError := range invalids {
-		if addr, err := parseDaemonHost(invalidAddr); err == nil || expectedError != "" && err.Error() != expectedError {
-			t.Errorf("tcp %v address expected error %q return, got %q and addr %v", invalidAddr, expectedError, err, addr)
-		}
+		t.Run(invalidAddr, func(t *testing.T) {
+			addr, err := parseDaemonHost(invalidAddr)
+			if err == nil || err.Error() != expectedError {
+				t.Errorf(`expected error "%s", got "%v"`, expectedError, err)
+			}
+			if addr != "" {
+				t.Errorf(`expected addr to be empty, got "%s""`, addr)
+			}
+		})
 	}
 	for validAddr, expectedAddr := range valids {
-		if addr, err := parseDaemonHost(validAddr); err != nil || addr != expectedAddr {
-			t.Errorf("%v -> expected %v, got (%v) addr (%v)", validAddr, expectedAddr, err, addr)
-		}
+		t.Run(validAddr, func(t *testing.T) {
+			addr, err := parseDaemonHost(validAddr)
+			if err != nil {
+				t.Errorf(`unexpected error: "%v"`, err)
+			}
+			if addr != expectedAddr {
+				t.Errorf(`expected "%s", got "%s""`, expectedAddr, addr)
+			}
+		})
 	}
 }
 
 func TestParseTCP(t *testing.T) {
 	var (
-		defaultHTTPHost = "tcp://127.0.0.1:2376"
+		defaultHTTPHost = "tcp://127.0.0.1:8888"
 	)
 	invalids := map[string]string{
-		"tcp:a.b.c.d":                 "",
-		"tcp:a.b.c.d/path":            "",
+		"tcp:a.b.c.d":                 `parse "tcp://tcp:a.b.c.d": invalid port ":a.b.c.d" after host`,
+		"tcp:a.b.c.d/path":            `parse "tcp://tcp:a.b.c.d/path": invalid port ":a.b.c.d" after host`,
 		"udp://127.0.0.1":             "Invalid proto, expected tcp: udp://127.0.0.1",
-		"udp://127.0.0.1:2375":        "Invalid proto, expected tcp: udp://127.0.0.1:2375",
+		"udp://127.0.0.1:5555":        "Invalid proto, expected tcp: udp://127.0.0.1:5555",
 		":5555/path":                  "invalid bind address (:5555/path): should not contain a path element",
 		"0.0.0.1:5555/path":           "invalid bind address (0.0.0.1:5555/path): should not contain a path element",
 		"[::1]:5555/path":             "invalid bind address ([::1]:5555/path): should not contain a path element",
@@ -119,27 +149,44 @@ func TestParseTCP(t *testing.T) {
 	}
 	valids := map[string]string{
 		"":                       defaultHTTPHost,
-		"tcp://":                 defaultHTTPHost,
-		"0.0.0.1:":               "tcp://0.0.0.1:2376",
+		"0.0.0.1":                "tcp://0.0.0.1:8888",
+		"0.0.0.1:":               "tcp://0.0.0.1:8888",
 		"0.0.0.1:5555":           "tcp://0.0.0.1:5555",
-		":6666":                  "tcp://127.0.0.1:6666",
-		"tcp://:7777":            "tcp://127.0.0.1:7777",
-		"[::1]:":                 "tcp://[::1]:2376",
+		":":                      "tcp://127.0.0.1:8888",
+		":5555":                  "tcp://127.0.0.1:5555",
+		"::1":                    "tcp://[::1]:8888",
+		"[::1]:":                 "tcp://[::1]:8888",
 		"[::1]:5555":             "tcp://[::1]:5555",
-		"[0:0:0:0:0:0:0:1]:":     "tcp://[0:0:0:0:0:0:0:1]:2376",
+		"[0:0:0:0:0:0:0:1]:":     "tcp://[0:0:0:0:0:0:0:1]:8888",
 		"[0:0:0:0:0:0:0:1]:5555": "tcp://[0:0:0:0:0:0:0:1]:5555",
-		"localhost:":             "tcp://localhost:2376",
+		"localhost":              "tcp://localhost:8888",
+		"localhost:":             "tcp://localhost:8888",
 		"localhost:5555":         "tcp://localhost:5555",
+		"tcp://":                 defaultHTTPHost,
+		"tcp://:":                defaultHTTPHost,
+		"tcp://:5555":            "tcp://127.0.0.1:5555",
 	}
 	for invalidAddr, expectedError := range invalids {
-		if addr, err := ParseTCPAddr(invalidAddr, defaultHTTPHost); err == nil || expectedError != "" && err.Error() != expectedError {
-			t.Errorf("tcp %v address expected error %v return, got %s and addr %v", invalidAddr, expectedError, err, addr)
-		}
+		t.Run(invalidAddr, func(t *testing.T) {
+			addr, err := ParseTCPAddr(invalidAddr, defaultHTTPHost)
+			if err == nil || err.Error() != expectedError {
+				t.Errorf(`expected error "%s", got "%v"`, expectedError, err)
+			}
+			if addr != "" {
+				t.Errorf(`expected addr to be empty, got "%s""`, addr)
+			}
+		})
 	}
 	for validAddr, expectedAddr := range valids {
-		if addr, err := ParseTCPAddr(validAddr, defaultHTTPHost); err != nil || addr != expectedAddr {
-			t.Errorf("%v -> expected %v, got %v and addr %v", validAddr, expectedAddr, err, addr)
-		}
+		t.Run(validAddr, func(t *testing.T) {
+			addr, err := ParseTCPAddr(validAddr, defaultHTTPHost)
+			if err != nil {
+				t.Errorf(`unexpected error: "%v"`, err)
+			}
+			if addr != expectedAddr {
+				t.Errorf(`expected "%s", got "%s""`, expectedAddr, addr)
+			}
+		})
 	}
 }