Using a IPv6 literal as the address in a STRICT_DNS cluster generates invalid TCP URLs

[jpeach]

Envoy fails to parse a URL generated by resolving the xDS server name to an IPv6 address:

[2020-03-23 12:32:58.813][1][debug][init] [source/common/init/watcher_impl.cc:27] init manager Server destroyed
malformed url: tcp://2001:db8:10a:c751::ffff:42ba:8001

The URL should be tcp://[2001:db8:10a:c751::ffff:42ba]:8001, which is parsed by Utility::parseInternetAddressAndPort().

It is likely that the caller is using Utility::portFromTcpUrl(), which doesn't call the full parser. This seems like a related but distinct problem.

A brief grep looks like there are a few places that could generate URLs of this form:

source/common/upstream/logical_dns_cluster.cc:  dns_url_ = fmt::format("tcp://{}:{}", socket_address.address(), socket_address.port_value());
source/common/upstream/strict_dns_cluster.cc:          fmt::format("tcp://{}:{}", socket_address.address(), socket_address.port_value());
test/common/network/connection_impl_test.cc:          fmt::format("tcp://{}:1", Network::Test::getLoopbackAddressUrlString(GetParam()))),
test/extensions/filters/udp/udp_proxy/udp_proxy_integration_test.cc:      fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port));
test/extensions/filters/udp/udp_proxy/udp_proxy_integration_test.cc:      fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port));
test/extensions/filters/udp/udp_proxy/udp_proxy_integration_test.cc:      fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port));
test/integration/http_integration.cc:      cluster, fmt::format("tcp://{}:80", Network::Test::getLoopbackAddressUrlString(version_)))};
test/integration/integration.cc:          fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version), port)),
test/integration/integration.cc:          fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port)),
test/integration/utility.cc:      fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(ip_version), port));
test/integration/utility.cc:          fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version), port)),

Relevant Links:

Reported in https://github.com/projectcontour/contour/issues/2354

The contour issue contains the full bootstrap config, but the relevant part is most likely the cluster that points to the xDS server:

  "static_resources":
    {
      "clusters":
        [
          {
            "name": "contour",
            "alt_stat_name": "projectcontour_contour_8001",
            "type": "STRICT_DNS",
            "connect_timeout": "5s",
            "load_assignment":
              {
                "cluster_name": "contour",
                "endpoints":
                  [
                    {
                      "lb_endpoints":
                        [
                          {
                            "endpoint":
                              {
                                "address":
                                  {
                                    "socket_address":
                                      {
                                        "address": "contour",
                                        "port_value": 8001,
                                      },
                                  },
                              },
                          },
                        ],
                    },
                  ],
              },
...

[jpeach]

Further upstream triage; we were configuring an IPv6 address literal for a STRICT_DNS cluster. Envoy documents this as requiring a socket address with hostname rather than an address. The fact that using an IPv4 address here works in practice is probably just luck.

So we could close this as "behaves correctly", or loosen the rules to accept an address literal.

[jewertow]

This issue was fixed in v1.23, so it can be closed.