app/netstorage: improve validation for address provided at storageNode

Previously, address was always parsed as "host:port" and added port if
it was missing. This leaded to hard to understand errors in case address
was provided in "http://host:port" format.

Improve error validation in order to provide more precise error message
in case of invalid address format.

See: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/9029

Previous error message: `cannot dial storageNode
"http://localhost:8488": dial tcp4: address http://localhost:8488: too
many colons in address` and vminsert continue running.
Current error message: `cannot normalize
-storageNode="http://localhost:8480": invalid address
"http://localhost:8480"; expected format: host:port` and vminsert exists
with error status code.

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>

lib/netutil/netutil.go

@@ -65,3 +65,30 @@ var Dialer = &net.Dialer{
 	KeepAlive: 30 * time.Second,
 	DualStack: TCP6Enabled(),
 }
+
+// IsErrMissingPort checks if the given error is due to a missing port in the address.
+// It is expected to be used to validate error returned by net.SplitHostPort
+// See https://github.com/golang/go/blob/ed08d2ad0928c0fc77cc2053863616ffb58c5aac/src/net/ipsock.go#L167
+func IsErrMissingPort(err error) bool {
+	if err == nil {
+		return false
+	}
+	return strings.Contains(err.Error(), "missing port in address")
+}
+
+// NormalizeAddr normalizes the given addr by adding defaultPort if it is missing.
+// It returns the normalized address in the form "host:port".
+// It is expected that addr is in the form "host" or "host:port".
+func NormalizeAddr(addr string, defaultPort int) (string, error) {
+	if strings.Index(addr, "/") > 0 {
+		return "", fmt.Errorf("invalid address %q; expected format: host:port", addr)
+	}
+
+	_, _, err := net.SplitHostPort(addr)
+	if IsErrMissingPort(err) {
+		return fmt.Sprintf("%s:%d", addr, defaultPort), nil
+	} else if err != nil {
+		return "", fmt.Errorf("invalid address %q; expected format: host:port", addr)
+	}
+	return addr, nil
+}

lib/netutil/netutil_test.go

@@ -0,0 +1,68 @@
+package netutil
+
+import (
+	"net"
+	"testing"
+)
+
+func TestIsErrMissingPort(t *testing.T) {
+	f := func(addr string, expected bool) {
+		t.Helper()
+		_, _, err := net.SplitHostPort(addr)
+		if IsErrMissingPort(err) != expected {
+			t.Fatalf("unexpected result for %q; got %v; want %v", addr, !expected, expected)
+		}
+	}
+
+	f("127.0.0.1", true)
+	f("http://127.0.0.1:8080", false)
+
+	f("[::1]", true)
+	f("http://[::1]:8080", false)
+
+	f("vmstorage-0", true)
+	f("vmstorage-0.svc.cluster.local.", true)
+	f("http://vmstorage-0:8080", false)
+	f("http://vmstorage-0.svc.cluster.local.:8080", false)
+}
+
+func TestNormalizeAddrSuccess(t *testing.T) {
+	f := func(addr string, defaultPort int, expected string) {
+		t.Helper()
+		result, err := NormalizeAddr(addr, defaultPort)
+		if err != nil {
+			t.Fatalf("unexpected error for %q: %v", addr, err)
+		}
+		if result != expected {
+			t.Fatalf("unexpected result for %q; got %q; want %q", addr, result, expected)
+		}
+	}
+
+	f("127.0.0.1", 80, "127.0.0.1:80")
+	f("127.0.0.1:123", 80, "127.0.0.1:123")
+	f("[::1]", 80, "[::1]:80")
+	f("[::1]:123", 80, "[::1]:123")
+	f("vmstorage-0.svc.cluster.local.", 80, "vmstorage-0.svc.cluster.local.:80")
+	f("vmstorage-0.svc.cluster.local.:123", 80, "vmstorage-0.svc.cluster.local.:123")
+}
+
+func TestNormalizeAddrError(t *testing.T) {
+	f := func(addr string) {
+		t.Helper()
+		_, err := NormalizeAddr(addr, 80)
+		if err == nil {
+			t.Fatalf("expected error for %q, but got none", addr)
+		}
+	}
+
+	// Invalid number of octets in address
+	f("1:2:3:4:5:6:7:8:9:10")
+	// Invalid IPv6 address format
+	f("::1:2:3:4:5:6:7:8:9")
+
+	// Invalid address format
+	f("http://127.0.0.1")
+	f("http://127.0.0.1:80")
+	f("http://vmstorage-0.svc.cluster.local.")
+	f("http://vmstorage-0.svc.cluster.local.:80")
+}