client: fix IPv6 parsing for client.servers block (#20324)

When the `client.servers` block is parsed, we split the port from the
address. This does not correctly handle IPv6 addresses when they are in URL
format (wrapped in brackets), which we require to disambiguate the port and
address.

Fix the parser to correctly split out the port and handle a missing port value
for IPv6. Update the documentation to make the URL format requirement clear.

Fixes: https://github.com/hashicorp/nomad/issues/20310

.changelog/20324.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+config: Fixed a bug where IPv6 addresses were not accepted without ports for `client.servers` blocks
+```

client/rpc.go

@@ -448,11 +448,15 @@ func resolveServer(s string) (net.Addr, error) {
 	host, port, err := net.SplitHostPort(s)
 	if err != nil {
 		if strings.Contains(err.Error(), "missing port") {
-			host = s
-			port = defaultClientPort
+			// with IPv6 addresses the `host` variable will have brackets
+			// removed, so send the original value thru again with only the
+			// correct port suffix
+			return resolveServer(s + ":" + defaultClientPort)
 		} else {
 			return nil, err
 		}
+	} else if port == "" {
+		return resolveServer(s + defaultClientPort)
 	}
 	return net.ResolveTCPAddr("tcp", net.JoinHostPort(host, port))
 }

client/rpc_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/structs"
 	sconfig "github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
@@ -117,3 +118,70 @@ func TestRpc_streamingRpcConn_badEndpoint_TLS(t *testing.T) {
 	require.NotNil(err)
 	require.Contains(err.Error(), "Unknown rpc method: \"Bogus\"")
 }
+
+func Test_resolveServer(t *testing.T) {
+
+	// note: we can't test a DNS name here without making an external DNS query,
+	// which we don't want to do from CI
+	testCases := []struct {
+		name      string
+		addr      string
+		expect    string
+		expectErr string
+	}{
+		{
+			name:      "ipv6 no brackets",
+			addr:      "2001:db8::1",
+			expectErr: "address 2001:db8::1: too many colons in address",
+		},
+		{
+			name:   "ipv6 no port",
+			addr:   "[2001:db8::1]",
+			expect: "[2001:db8::1]:4647",
+		},
+		{
+			name:   "ipv6 trailing port colon",
+			addr:   "[2001:db8::1]:",
+			expect: "[2001:db8::1]:4647",
+		},
+		{
+			name:      "ipv6 malformed",
+			addr:      "[2001:db8::1]:]",
+			expectErr: "address [2001:db8::1]:]: unexpected ']' in address",
+		},
+		{
+			name:   "ipv6 with port",
+			addr:   "[2001:db8::1]:6647",
+			expect: "[2001:db8::1]:6647",
+		},
+		{
+			name:   "ipv4 no port",
+			addr:   "192.168.1.117",
+			expect: "192.168.1.117:4647",
+		},
+		{
+			name:   "ipv4 trailing port colon",
+			addr:   "192.168.1.117:",
+			expect: "192.168.1.117:4647",
+		},
+		{
+			name:   "ipv4 with port",
+			addr:   "192.168.1.117:6647",
+			expect: "192.168.1.117:6647",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			addr, err := resolveServer(tc.addr)
+			if tc.expectErr != "" {
+				must.Nil(t, addr)
+				must.EqError(t, err, tc.expectErr)
+			} else {
+				must.NoError(t, err)
+				must.Eq(t, tc.expect, addr.String())
+			}
+		})
+	}
+
+}

website/content/docs/configuration/client.mdx

@@ -108,7 +108,9 @@ client {
   servers this client should join. This list is used to register the client with
   the server nodes and advertise the available resources so that the agent can
   receive work. This may be specified as an IP address or DNS, with or without
-  the port. If the port is omitted, the default port of `4647` is used.
+  the port. If the port is omitted, the default port of `4647` is used. If you
+  are specifying IPv6 addresses, they must be in URL format with brackets
+  (ex. `"[2001:db8::1]"`).
 
 - `server_join` <code>([server_join][server-join]: nil)</code> - Specifies
   how the Nomad client will connect to Nomad servers. The `start_join` field

website/content/docs/configuration/server_join.mdx

@@ -108,6 +108,13 @@ done in the `ip:port` format, such as:
 1.2.3.4:5678
 ```
 
+If the IP address is an IPv6 address, it must be in URL format surrounded by
+brackets. For example:
+
+```
+[2001:db8::1]:5678
+```
+
 If the port option is omitted, it defaults to the Serf port, which is 4648
 unless configured otherwise: