Skip to content

Commit

Permalink
[yugabyte#3644] Allow ability to specify and prefer IPv6 addresses in…
Browse files Browse the repository at this point in the history
… bind addresses

Summary:
The primary goal of this diff is to allow yugabyte to work in IPv6-only environments without disturbing much of how it works presently in IPv4 environments. Some of the behavior in dual stack environments may still need fixes in future diffs.

1. Allow resolution to return IPv6 addresses

2. Add the ability to prefer IPv4/IPv6 addresses of certain types when multiple addresses are available. This is the case when a single domain name resolves to multiple IP addrs (via getaddrinfo, family AF_UNSPEC). This is also the case when bind to 0.0.0.0 or [::] is specified and multiple local addresses are available to advertise to other nodes (AddHostPortPBs -> GetLocalAddresses). In particular, IPv6 link local addresses like 'fe80::1%lo' which are commonly present on macbooks are not browser friendly, so added an ipv6_non_link_local filter that prefers loopback address like [::1] over [fe80::1%lo] when both are available.

3. Include HostPort parsing from the PR yugabyte#3765 and fix some of the behavior within.

Test Plan:
1. Test yb-ctl on local dev server, verify IP4 behavior is unchanged.
2. Tested on dual stack nodes with both IPv4 and IPv6 addresses. Verified that when 0.0.0.0 is specified and local_address_filter, resolve_address_filter are at default values (preferring ipv4), the server binds to and advertises IPv4 interfaces. When '[::]' is specified and local_address_filter, resolve_address_filter are changed to 'ipv6_external,ipv4_non_link_local,ipv6_all', verify that the server binds to and advertises IPv6 IPs.
3. Verified binding to DNS names with local_address_filter, resolve_address_filter set to 'ipv6_external,ipv6_non_link_local,ipv6_all' that the rpc and web server bind to IPv6 IPs. pgsql_proxy_bind_address works differently with postgres binding to both IPv4 and IPv6 addresses when a DNS name with both IPv4 and IPv6 addresses is specified (because it does not understand these filter flags).
4. Added some unit tests to verify the bind address parsing and the address filtering functionality.

Reviewers: bogdan, sergei

Reviewed By: sergei

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D8675
  • Loading branch information
iSignal authored and deeps1991 committed Jul 22, 2020
1 parent 5422a15 commit 93bbc25
Show file tree
Hide file tree
Showing 16 changed files with 383 additions and 83 deletions.
7 changes: 4 additions & 3 deletions src/yb/client/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1431,9 +1431,10 @@ Status YBClient::GetMasterUUID(const string& host,
data_->proxy_cache_.get(), {hp}, default_rpc_timeout(), &server));

if (server.has_error()) {
return STATUS(RuntimeError,
strings::Substitute("Error $0 while getting uuid of $1:$2.",
"", host, port));
return STATUS_FORMAT(
RuntimeError,
"Error while getting uuid of $0.",
HostPortToString(host, port));
}

*uuid = server.instance_id().permanent_uuid();
Expand Down
9 changes: 7 additions & 2 deletions src/yb/common/wire_protocol.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ DEFINE_string(use_private_ip, "never",
"zone - would use private IP if destination node is located in the same cloud, "
"region and zone."
"never - would never use private IP if broadcast address is specified.");

namespace yb {

namespace {
Expand Down Expand Up @@ -284,10 +283,12 @@ Status AddHostPortPBs(const std::vector<Endpoint>& addrs,
HostPortPB* pb = pbs->Add();
pb->set_port(addr.port());
if (addr.address().is_unspecified()) {
VLOG(4) << " Asked to add unspecified address: " << addr.address();
auto status = GetFQDN(pb->mutable_host());
if (!status.ok()) {
std::vector<IpAddress> locals;
if (!GetLocalAddresses(&locals, AddressFilter::EXTERNAL).ok() || locals.empty()) {
if (!GetLocalAddresses(FLAGS_net_address_filter, &locals).ok() ||
locals.empty()) {
return status;
}
for (auto& address : locals) {
Expand All @@ -296,11 +297,15 @@ Status AddHostPortPBs(const std::vector<Endpoint>& addrs,
pb->set_port(addr.port());
}
pb->set_host(address.to_string());
VLOG(4) << "Adding local address: " << pb->host();
pb = nullptr;
}
} else {
VLOG(4) << "Adding FQDN " << pb->host();
}
} else {
pb->set_host(addr.address().to_string());
VLOG(4) << "Adding specific address: " << pb->host();
}
}
return Status::OK();
Expand Down
2 changes: 1 addition & 1 deletion src/yb/integration-tests/external_mini_cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ string ExternalMiniCluster::GetTabletServerAddresses() const {
if (!peer_addrs.empty()) {
peer_addrs += ",";
}
peer_addrs += Format("$0:$1", ts->bind_host(), ts->rpc_port());
peer_addrs += HostPortToString(ts->bind_host(), ts->rpc_port());
}
return peer_addrs;
}
Expand Down
29 changes: 12 additions & 17 deletions src/yb/master/master-path-handlers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,8 @@ void MasterPathHandlers::RedirectToLeader(const Webserver::WebRequest& req,
if (master.role() == consensus::RaftPeerPB::LEADER) {
// URI already starts with a /, so none is needed between $1 and $2.
redirect = Substitute(
"http://$0:$1$2$3",
master.registration().http_addresses(0).host(),
master.registration().http_addresses(0).port(),
"http://$0$1$2",
HostPortPBToString(master.registration().http_addresses(0)),
req.redirect_uri,
req.query_string.empty() ? "?raw" : "?" + req.query_string + "&raw");
break;
Expand Down Expand Up @@ -258,9 +257,7 @@ void MasterPathHandlers::TServerDisplay(const std::string& current_uuid,
if (desc->placement_uuid() == current_uuid) {
const string time_since_hb = StringPrintf("%.1fs", desc->TimeSinceHeartbeat().ToSeconds());
TSRegistrationPB reg = desc->GetRegistration();
string host_port = Substitute("$0:$1",
reg.common().http_addresses(0).host(),
reg.common().http_addresses(0).port());
string host_port = HostPortPBToString(reg.common().http_addresses(0));
*output << " <tr>\n";
*output << " <td>" << RegistrationToHtml(reg.common(), host_port) << "</br>";
*output << " " << desc->permanent_uuid() << "</td>";
Expand Down Expand Up @@ -502,9 +499,7 @@ void MasterPathHandlers::HandleGetTserverStatus(const Webserver::WebRequest& req
for (auto desc : descs) {
if (desc->placement_uuid() == cur_uuid) {
TSRegistrationPB reg = desc->GetRegistration();
string host_port = Substitute("$0:$1",
reg.common().http_addresses(0).host(),
reg.common().http_addresses(0).port());
string host_port = HostPortPBToString(reg.common().http_addresses(0));
jw.String(host_port);

jw.StartObject();
Expand Down Expand Up @@ -1176,8 +1171,7 @@ void MasterPathHandlers::HandleMasters(const Webserver::WebRequest& req,
continue;
}
auto reg = master.registration();
string host_port = Substitute("$0:$1",
reg.http_addresses(0).host(), reg.http_addresses(0).port());
string host_port = HostPortPBToString(reg.http_addresses(0));
string reg_text = RegistrationToHtml(reg, host_port);
if (master.instance_id().permanent_uuid() == master_->instance_pb().permanent_uuid()) {
reg_text = Substitute("<b>$0</b>", reg_text);
Expand Down Expand Up @@ -1323,7 +1317,7 @@ class JsonTabletDumper : public Visitor<PersistentTabletInfo>, public JsonDumper

jw_->String("addr");
const auto& host_port = peer.last_known_private_addr()[0];
jw_->String(Format("$0:$1", host_port.host(), host_port.port()));
jw_->String(HostPortPBToString(host_port));

jw_->EndObject();
}
Expand Down Expand Up @@ -1520,8 +1514,9 @@ string MasterPathHandlers::TSDescriptorToHtml(const TSDescriptor& desc,

if (reg.common().http_addresses().size() > 0) {
return Substitute(
"<a href=\"http://$0:$1/tablet?id=$2\">$3</a>", reg.common().http_addresses(0).host(),
reg.common().http_addresses(0).port(), EscapeForHtmlToString(tablet_id),
"<a href=\"http://$0/tablet?id=$1\">$2</a>",
HostPortPBToString(reg.common().http_addresses(0)),
EscapeForHtmlToString(tablet_id),
EscapeForHtmlToString(reg.common().http_addresses(0).host()));
} else {
return EscapeForHtmlToString(desc.permanent_uuid());
Expand All @@ -1532,9 +1527,9 @@ string MasterPathHandlers::RegistrationToHtml(
const ServerRegistrationPB& reg, const std::string& link_text) const {
string link_html = EscapeForHtmlToString(link_text);
if (reg.http_addresses().size() > 0) {
link_html = Substitute("<a href=\"http://$0:$1/\">$2</a>",
reg.http_addresses(0).host(),
reg.http_addresses(0).port(), link_html);
link_html = Substitute("<a href=\"http://$0/\">$1</a>",
HostPortPBToString(reg.http_addresses(0)),
link_html);
}
return link_html;
}
Expand Down
28 changes: 20 additions & 8 deletions src/yb/server/server_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@ using strings::Substitute;
namespace yb {
namespace server {

static const string kWildCardHostAddress = "0.0.0.0";

namespace {

// Disambiguates between servers when in a minicluster.
Expand All @@ -131,6 +129,9 @@ struct CommonMemTrackers {

std::unique_ptr<CommonMemTrackers> common_mem_trackers;

static const string kWildCardHostAddressV6 = "::";
static const string kWildCardHostAddressV4 = "0.0.0.0";

} // anonymous namespace

std::shared_ptr<MemTracker> CreateMemTrackerForServer() {
Expand Down Expand Up @@ -492,13 +493,21 @@ Status RpcAndWebServerBase::GetRegistration(ServerRegistrationPB* reg, RpcOnly r
std::vector<HostPort> addrs = CHECK_NOTNULL(rpc_server())->GetRpcHostPort();

// Fall back to hostname resolution if the rpc hostname is a wildcard.
if (addrs.size() > 1 || addrs[0].host() == kWildCardHostAddress || addrs[0].port() == 0) {
auto addrs = CHECK_NOTNULL(rpc_server())->GetBoundAddresses();
RETURN_NOT_OK_PREPEND(AddHostPortPBs(addrs, reg->mutable_private_rpc_addresses()),
"Failed to add RPC endpoints to registration");
if (addrs.size() > 1 || addrs[0].host() == kWildCardHostAddressV4 ||
addrs[0].host() == kWildCardHostAddressV6 || addrs[0].port() == 0) {
vector<Endpoint> endpoints =
CHECK_NOTNULL(rpc_server())->GetBoundAddresses();
RETURN_NOT_OK_PREPEND(
AddHostPortPBs(endpoints, reg->mutable_private_rpc_addresses()),
"Failed to add RPC endpoints to registration");
for (const auto &addr : reg->private_rpc_addresses()) {
LOG(INFO) << " Using private rpc addresses: ( " << addr.ShortDebugString()
<< " )";
}
} else {
HostPortsToPBs(addrs, reg->mutable_private_rpc_addresses());
LOG(INFO) << "Using private ip address " << reg->private_rpc_addresses(0).host();
LOG(INFO) << "Using private rpc address "
<< reg->private_rpc_addresses(0).host();
}

HostPortsToPBs(options_.broadcast_addresses, reg->mutable_broadcast_addresses());
Expand All @@ -511,6 +520,9 @@ Status RpcAndWebServerBase::GetRegistration(ServerRegistrationPB* reg, RpcOnly r
RETURN_NOT_OK_PREPEND(AddHostPortPBs(
web_addrs, reg->mutable_http_addresses()),
"Failed to add HTTP addresses to registration");
for (const auto &addr : reg->http_addresses()) {
LOG(INFO) << "Using http addresses: ( " << addr.ShortDebugString() << " )";
}
}
reg->mutable_cloud_info()->set_placement_cloud(options_.placement_cloud());
reg->mutable_cloud_info()->set_placement_region(options_.placement_region());
Expand Down Expand Up @@ -614,7 +626,7 @@ std::string TEST_RpcAddress(int index, Private priv) {
}

string TEST_RpcBindEndpoint(int index, uint16_t port) {
return Format("$0:$1", TEST_RpcAddress(index, Private::kTrue), port);
return HostPortToString(TEST_RpcAddress(index, Private::kTrue), port);
}

constexpr int kMaxServers = 20;
Expand Down
7 changes: 6 additions & 1 deletion src/yb/server/webserver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,19 @@ bool Webserver::IsSecure() const {
Status Webserver::BuildListenSpec(string* spec) const {
std::vector<Endpoint> addrs;
RETURN_NOT_OK(ParseAddressList(http_address_, 80, &addrs));

if (addrs.empty()) {
return STATUS_FORMAT(
ConfigurationError,
"No IPs available for address $0", http_address_);
}
std::vector<string> parts;
for (const auto& addr : addrs) {
// Mongoose makes sockets with 's' suffixes accept SSL traffic only
parts.push_back(ToString(addr) + (IsSecure() ? "s" : ""));
}

JoinStrings(parts, ",", spec);
LOG(INFO) << "Webserver listen spec is " << *spec;
return Status::OK();
}

Expand Down
10 changes: 3 additions & 7 deletions src/yb/tools/yb-admin_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,12 @@ static constexpr const char* kDBTypePrefixYsql = "ysql";
static constexpr const char* kDBTypePrefixRedis = "yedis";
static constexpr const char* kTableIDPrefix = "tableid";

string FormatHostPort(const HostPortPB& host_port) {
return Format("$0:$1", host_port.host(), host_port.port());
}

string FormatFirstHostPort(
const RepeatedPtrField<HostPortPB>& rpc_addresses) {
if (rpc_addresses.empty()) {
return "N/A";
} else {
return FormatHostPort(rpc_addresses.Get(0));
return HostPortPBToString(rpc_addresses.Get(0));
}
}

Expand Down Expand Up @@ -1166,7 +1162,7 @@ Status ClusterAdminClient::ListTablets(const YBTableName& table_name, int max_ta
for (const auto& replica : locations_of_this_tablet.replicas()) {
if (replica.role() == RaftPeerPB::Role::RaftPeerPB_Role_LEADER) {
if (leader_host_port.empty()) {
leader_host_port = FormatHostPort(replica.ts_info().private_rpc_addresses(0));
leader_host_port = HostPortPBToString(replica.ts_info().private_rpc_addresses(0));
leader_uuid = replica.ts_info().permanent_uuid();
} else {
LOG(ERROR) << "Multiple leader replicas found for tablet " << tablet_uuid
Expand Down Expand Up @@ -1210,7 +1206,7 @@ Status ClusterAdminClient::ListPerTabletTabletServers(const TabletId& tablet_id)
}
for (const auto& replica : locs.replicas()) {
cout << replica.ts_info().permanent_uuid() << kColumnSep
<< RightPadToWidth(FormatHostPort(replica.ts_info().private_rpc_addresses(0)),
<< RightPadToWidth(HostPortPBToString(replica.ts_info().private_rpc_addresses(0)),
kHostPortColWidth) << kColumnSep
<< PBEnumToString(replica.role()) << endl;
}
Expand Down
5 changes: 2 additions & 3 deletions src/yb/tserver/tablet_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,8 @@ TabletServiceImpl* TabletServer::tablet_server_service() {

string GetDynamicUrlTile(const string path, const string host, const int port) {

vector<std::string> parsed_hostname = strings::Split(host, ":");
std::string link = strings::Substitute("http://$0:$1$2",
parsed_hostname[0], yb::ToString(port), path);
std::string link =
strings::Substitute("http://$0$1", HostPortToString(host, port), path);
return link;
}

Expand Down
6 changes: 5 additions & 1 deletion src/yb/util/net/dns_resolver-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ TEST_F(DnsResolverTest, TestResolution) {
ASSERT_TRUE(!addrs.empty());
for (const auto& addr : addrs) {
LOG(INFO) << "Address: " << addr;
EXPECT_TRUE(HasPrefixString(ToString(addr), "127."));
if (addr.address().is_v4()) {
EXPECT_TRUE(HasPrefixString(ToString(addr), "127."));
} else if (addr.address().is_v6()) {
EXPECT_TRUE(HasPrefixString(ToString(addr), "[::1]"));
}
EXPECT_TRUE(HasSuffixString(ToString(addr), ":12345"));
}
}
Expand Down
14 changes: 9 additions & 5 deletions src/yb/util/net/dns_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
DEFINE_int32(dns_num_resolver_threads, 1, "The number of threads to use for DNS resolution");
TAG_FLAG(dns_num_resolver_threads, advanced);

DECLARE_string(resolver_address_filter);
using std::vector;

namespace yb {
Expand Down Expand Up @@ -136,12 +137,13 @@ Result<InetAddress> PickResolvedAddress(
if (error) {
return STATUS_FORMAT(NetworkError, "Resolve failed $0: $1", host, error.message());
}
std::vector<InetAddress> addresses, addresses_v6;
std::vector<IpAddress> addresses;
for (const auto& entry : entries) {
auto& dest = entry.endpoint().address().is_v4() ? addresses : addresses_v6;
dest.emplace_back(entry.endpoint().address());
addresses.push_back(entry.endpoint().address());
VLOG(3) << "Resolved address " << entry.endpoint().address().to_string()
<< " for host " << host;
}
addresses.insert(addresses.end(), addresses_v6.begin(), addresses_v6.end());
FilterAddresses(FLAGS_net_address_filter, &addresses);
if (addresses.empty()) {
return STATUS_FORMAT(NetworkError, "No endpoints resolved for: $0", host);
}
Expand All @@ -151,7 +153,9 @@ Result<InetAddress> PickResolvedAddress(
<< yb::ToString(addresses.front());
}

return addresses.front();
VLOG(3) << "Returned address " << addresses[0].to_string() << " for host "
<< host;
return InetAddress(addresses.front());
}


Expand Down
56 changes: 56 additions & 0 deletions src/yb/util/net/inetaddress-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "yb/util/test_macros.h"
#include "yb/util/test_util.h"
#include "yb/util/net/net_fwd.h"

namespace yb {

Expand Down Expand Up @@ -108,4 +109,59 @@ TEST_F(InetAddressTest, TestHostName) {
ASSERT_EQ("192.0.2.235", addr.ToString());
}

TEST_F(InetAddressTest, FilterAddresses) {

// Create a list of ipv6 and ipv4 addresses
const vector<string> address_strs = {
"::1", "10.150.0.148",
"2600:1f18:1094:c832:36e6:43b9:e6c8:f02", "127.0.0.1",
"127.0.0.2", "0.0.0.0",
"::", "fe80::4001:aff:fe96:94",
"fe80::2%lo"
};
vector<IpAddress> addresses;
for (const auto &address_str : address_strs) {
LOG(INFO) << address_str;
addresses.push_back(IpAddress::from_string(address_str));
}
LOG(INFO) << "Starting test";
auto test_addresses = addresses;
FilterAddresses("ipv4_all", &test_addresses);
ASSERT_EQ(test_addresses.size(), 4);

test_addresses = addresses;
FilterAddresses("ipv6_all", &test_addresses);
ASSERT_EQ(test_addresses.size(), 5);

test_addresses = addresses;
FilterAddresses("ipv6_all,ipv4_all", &test_addresses);
ASSERT_EQ(test_addresses.size(), 9);
ASSERT_TRUE(test_addresses[0].is_v6());
ASSERT_TRUE(test_addresses[test_addresses.size() - 1].is_v4());

test_addresses = addresses;
FilterAddresses("ipv6_external", &test_addresses);
ASSERT_EQ(test_addresses.size(), 1);

test_addresses = addresses;
FilterAddresses("ipv6_non_link_local", &test_addresses);
ASSERT_EQ(test_addresses.size(), 2);

test_addresses = addresses;
FilterAddresses("ipv4_external", &test_addresses);
ASSERT_EQ(test_addresses.size(), 1);

test_addresses = addresses;
FilterAddresses("ipv4_external,ipv6_external", &test_addresses);
ASSERT_EQ(test_addresses.size(), 2);
ASSERT_TRUE(test_addresses[0].is_v4());
ASSERT_TRUE(test_addresses[test_addresses.size() - 1].is_v6());

test_addresses = addresses;
FilterAddresses("ipv6_external,ipv4_external", &test_addresses);
ASSERT_EQ(test_addresses.size(), 2);
ASSERT_TRUE(test_addresses[0].is_v6());
ASSERT_TRUE(test_addresses[test_addresses.size() - 1].is_v4());
}

} // namespace yb
Loading

0 comments on commit 93bbc25

Please sign in to comment.