From 8eaf176868e9807082f725b30e14206f1daf1f8e Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 8 Apr 2024 15:06:27 -0400 Subject: [PATCH] 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 | 3 + client/rpc.go | 8 ++- client/rpc_test.go | 68 +++++++++++++++++++ website/content/docs/configuration/client.mdx | 4 +- .../docs/configuration/server_join.mdx | 7 ++ 5 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 .changelog/20324.txt diff --git a/.changelog/20324.txt b/.changelog/20324.txt new file mode 100644 index 000000000..1db1a4122 --- /dev/null +++ b/.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 +``` diff --git a/client/rpc.go b/client/rpc.go index 6bbbe27f8..5427bb762 100644 --- a/client/rpc.go +++ b/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)) } diff --git a/client/rpc_test.go b/client/rpc_test.go index 11fce2078..3762f6108 100644 --- a/client/rpc_test.go +++ b/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()) + } + }) + } + +} diff --git a/website/content/docs/configuration/client.mdx b/website/content/docs/configuration/client.mdx index f6088f394..0865ef162 100644 --- a/website/content/docs/configuration/client.mdx +++ b/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` ([server_join][server-join]: nil) - Specifies how the Nomad client will connect to Nomad servers. The `start_join` field diff --git a/website/content/docs/configuration/server_join.mdx b/website/content/docs/configuration/server_join.mdx index aacda65b3..74f4dbb2f 100644 --- a/website/content/docs/configuration/server_join.mdx +++ b/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: