diff --git a/include/envoy/network/dns.h b/include/envoy/network/dns.h index 26f1f3cbfaa2..6684a215a628 100644 --- a/include/envoy/network/dns.h +++ b/include/envoy/network/dns.h @@ -48,7 +48,7 @@ class DnsResolver { /** * Called when a resolution attempt is complete. * @param response supplies the list of resolved IP addresses and TTLs. The list will be empty if - * the resolution failed. + * the resolution failed. */ using ResolveCb = std::function&& response)>; @@ -62,6 +62,17 @@ class DnsResolver { */ virtual ActiveDnsQuery* resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback) PURE; + + /** + * Initiate an async DNS resolution for an SRV record. + * @param dns_name supplies the DNS name to lookup. + * @param dns_lookup_family the DNS IP version lookup policy. + * @param callback supplies the callback to invoke when the resolution is complete. + * @return if non-null, a handle that can be used to cancel the resolution. + * This is only valid until the invocation of callback or ~DnsResolver(). + */ + virtual ActiveDnsQuery* resolveSrv(const std::string& dns_name, DnsLookupFamily dns_lookup_family, + ResolveCb callback) PURE; }; using DnsResolverSharedPtr = std::shared_ptr; diff --git a/source/common/config/well_known_names.h b/source/common/config/well_known_names.h index 255b86927ae1..da908cb28867 100644 --- a/source/common/config/well_known_names.h +++ b/source/common/config/well_known_names.h @@ -50,6 +50,8 @@ class AddressResolverNameValues { public: // Basic IP resolver const std::string IP = "envoy.ip"; + // DNS SRV resolver + const std::string SRV = "envoy.srv"; }; using AddressResolverNames = ConstSingleton; diff --git a/source/common/network/BUILD b/source/common/network/BUILD index b43a9ad1e8a7..7c35d6fd5ef4 100644 --- a/source/common/network/BUILD +++ b/source/common/network/BUILD @@ -192,7 +192,10 @@ envoy_cc_library( hdrs = ["resolver_impl.h"], deps = [ ":utility_lib", + "//include/envoy/event:dispatcher_interface", + "//include/envoy/event:timer_interface", "//include/envoy/network:address_interface", + "//include/envoy/network:dns_interface", "//include/envoy/network:resolver_interface", "//include/envoy/registry", "//source/common/config:well_known_names", @@ -260,6 +263,7 @@ envoy_cc_library( "//source/common/common:assert_lib", "//source/common/common:cleanup_lib", "//source/common/common:utility_lib", + "//source/common/config:well_known_names", "//source/common/protobuf", "@envoy_api//envoy/api/v2/core:address_cc", "@envoy_api//envoy/api/v2/core:base_cc", diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index c4409d473b6b..9efd0abb9ff3 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -1,5 +1,6 @@ #include "common/network/dns_impl.h" +#include #include #include #include @@ -67,6 +68,30 @@ void DnsResolverImpl::initializeChannel(ares_options* options, int optmask) { ares_init_options(&channel_, options, optmask | ARES_OPT_SOCK_STATE_CB); } +bool DnsResolverImpl::PendingResolutionBase::fireCallback(std::list&& response) { + if (completed_) { + if (!cancelled_) { + try { + callback_(std::move(response)); + } catch (const EnvoyException& e) { + ENVOY_LOG(critical, "EnvoyException in c-ares callback"); + dispatcher_.post([s = std::string(e.what())] { throw EnvoyException(s); }); + } catch (const std::exception& e) { + ENVOY_LOG(critical, "std::exception in c-ares callback"); + dispatcher_.post([s = std::string(e.what())] { throw EnvoyException(s); }); + } catch (...) { + ENVOY_LOG(critical, "Unknown exception in c-ares callback"); + dispatcher_.post([] { throw EnvoyException("unknown"); }); + } + } + if (owned_) { + delete this; + return true; + } + } + return false; +} + void DnsResolverImpl::PendingResolution::onAresGetAddrInfoCallback(int status, int timeouts, ares_addrinfo* addrinfo) { // We receive ARES_EDESTRUCTION when destructing with pending queries. @@ -120,25 +145,8 @@ void DnsResolverImpl::PendingResolution::onAresGetAddrInfoCallback(int status, i ENVOY_LOG(debug, "DNS request timed out {} times", timeouts); } - if (completed_) { - if (!cancelled_) { - try { - callback_(std::move(address_list)); - } catch (const EnvoyException& e) { - ENVOY_LOG(critical, "EnvoyException in c-ares callback"); - dispatcher_.post([s = std::string(e.what())] { throw EnvoyException(s); }); - } catch (const std::exception& e) { - ENVOY_LOG(critical, "std::exception in c-ares callback"); - dispatcher_.post([s = std::string(e.what())] { throw EnvoyException(s); }); - } catch (...) { - ENVOY_LOG(critical, "Unknown exception in c-ares callback"); - dispatcher_.post([] { throw EnvoyException("unknown"); }); - } - } - if (owned_) { - delete this; - return; - } + if (fireCallback(std::move(address_list))) { + return; } if (!completed_ && fallback_if_failed_) { @@ -192,6 +200,23 @@ void DnsResolverImpl::onAresSocketStateChange(int fd, int read, int write) { (write ? Event::FileReadyType::Write : 0)); } +ActiveDnsQuery* DnsResolverImpl::preparePendingResolution( + std::unique_ptr pending_resolution) { + if (pending_resolution->completed_) { + // Resolution does not need asynchronous behavior or network events. For + // example, localhost lookup. + return nullptr; + } else { + // Enable timer to wake us up if the request times out. + updateAresTimer(); + + // The PendingResolutionBase will self-delete when the request completes + // (including if cancelled or if ~DnsResolverImpl() happens). + pending_resolution->owned_ = true; + return pending_resolution.release(); + } +} + ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback) { // TODO(hennna): Add DNS caching which will allow testing the edge case of a @@ -209,19 +234,7 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, pending_resolution->getAddrInfo(AF_INET6); } - if (pending_resolution->completed_) { - // Resolution does not need asynchronous behavior or network events. For - // example, localhost lookup. - return nullptr; - } else { - // Enable timer to wake us up if the request times out. - updateAresTimer(); - - // The PendingResolution will self-delete when the request completes - // (including if cancelled or if ~DnsResolverImpl() happens). - pending_resolution->owned_ = true; - return pending_resolution.release(); - } + return preparePendingResolution(std::move(pending_resolution)); } void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { @@ -242,5 +255,90 @@ void DnsResolverImpl::PendingResolution::getAddrInfo(int family) { this); } +ActiveDnsQuery* DnsResolverImpl::resolveSrv(const std::string& dns_name, + DnsLookupFamily dns_lookup_family, ResolveCb callback) { + std::unique_ptr pending_srv_res( + new PendingSrvResolution(callback, dispatcher_, channel_, dns_name, dns_lookup_family, this)); + pending_srv_res->getSrvByName(); + return preparePendingResolution(std::move(pending_srv_res)); +} + +void DnsResolverImpl::PendingSrvResolution::onAresSrvStartCallback(int status, int timeouts, + unsigned char* buf, int len) { + // We receive ARES_EDESTRUCTION when destructing with pending queries. + if (status == ARES_EDESTRUCTION) { + ASSERT(owned_); + delete this; + return; + } + + bool replies_parsed = false; + if (status == ARES_SUCCESS) { + struct ares_srv_reply* srv_reply; + status = ares_parse_srv_reply(buf, len, &srv_reply); + + if (status == ARES_SUCCESS) { + size_t total = 0; + for (ares_srv_reply* current_reply = srv_reply; current_reply != NULL; + current_reply = current_reply->next) { + total++; + } + + std::shared_ptr> finished = std::make_shared>(0); + std::shared_ptr> srv_records = + std::make_shared>(); + std::shared_ptr mutex = std::make_shared(); + for (ares_srv_reply* current_reply = srv_reply; current_reply != NULL; + current_reply = current_reply->next) { + resolver_->resolve( + current_reply->host, this->dns_lookup_family_, + [this, total, finished, srv_records, mutex, + current_reply](const std::list&& response) { + for (auto instance = response.begin(); instance != response.end(); ++instance) { + Address::InstanceConstSharedPtr inst_with_port( + Utility::getAddressWithPort(*instance->address_, current_reply->port)); + mutex->lock(); + srv_records->emplace_back(DnsResponse(inst_with_port, instance->ttl_)); + mutex->unlock(); + } + if (static_cast(++(*finished)) == total) { + onAresSrvFinishCallback(std::move(*srv_records)); + } + }); + } + replies_parsed = true; + } + + ares_free_data(srv_reply); + } + + if (timeouts > 0) { + ENVOY_LOG(debug, "DNS request timed out {} times while querying for SRV records", timeouts); + } + + if (!replies_parsed) { + onAresSrvFinishCallback({}); + } +} + +void DnsResolverImpl::PendingSrvResolution::onAresSrvFinishCallback( + std::list&& srv_records) { + if (!srv_records.empty()) { + completed_ = true; + } + + fireCallback(std::move(srv_records)); +} + +void DnsResolverImpl::PendingSrvResolution::getSrvByName() { + ares_query( + channel_, dns_name_.c_str(), ns_c_in, ns_t_srv, + [](void* arg, int status, int timeouts, unsigned char* abuf, int alen) { + static_cast(arg)->onAresSrvStartCallback(status, timeouts, abuf, + alen); + }, + this); +} + } // namespace Network } // namespace Envoy diff --git a/source/common/network/dns_impl.h b/source/common/network/dns_impl.h index 096c2c71178f..0373e10efe33 100644 --- a/source/common/network/dns_impl.h +++ b/source/common/network/dns_impl.h @@ -34,13 +34,15 @@ class DnsResolverImpl : public DnsResolver, protected Logger::Loggable&& response); // Caller supplied callback to invoke on query completion or error. const ResolveCb callback_; @@ -73,13 +64,64 @@ class DnsResolverImpl : public DnsResolver, protected Logger::Loggable&& srv_records); + + // wrapper function of call to ares_query(). + void getSrvByName(); + + // The DnsLookupFamily for the SRV record. + const DnsLookupFamily dns_lookup_family_; + // The resolver instance. + DnsResolverImpl* resolver_; + }; + // Callback for events on sockets tracked in events_. void onEventCallback(int fd, uint32_t events); // c-ares callback when a socket state changes, indicating that libevent @@ -89,6 +131,13 @@ class DnsResolverImpl : public DnsResolver, protected Logger::Loggable pending_resolution); Event::Dispatcher& dispatcher_; Event::TimerPtr timer_; diff --git a/source/common/network/resolver_impl.cc b/source/common/network/resolver_impl.cc index 1bad0215fae2..b3efee289e9b 100644 --- a/source/common/network/resolver_impl.cc +++ b/source/common/network/resolver_impl.cc @@ -2,14 +2,18 @@ #include "envoy/api/v2/core/address.pb.h" #include "envoy/common/exception.h" +#include "envoy/event/dispatcher.h" +#include "envoy/event/timer.h" #include "envoy/network/address.h" #include "envoy/network/resolver.h" #include "envoy/registry/registry.h" -#include "common/config/well_known_names.h" #include "common/network/address_impl.h" #include "common/network/utility.h" +#include "absl/strings/match.h" +#include "absl/synchronization/blocking_counter.h" + namespace Envoy { namespace Network { namespace Address { @@ -43,6 +47,54 @@ class IpResolver : public Resolver { */ static Registry::RegisterFactory ip_registered_; +InstanceConstSharedPtr +SrvResolver::resolve(const envoy::api::v2::core::SocketAddress& socket_address) { + switch (socket_address.port_specifier_case()) { + case envoy::api::v2::core::SocketAddress::kNamedPort: + if (!absl::EqualsIgnoreCase(socket_address.named_port(), "srv")) { + throw EnvoyException( + fmt::format("SRV resolver can't handle the named port {}", socket_address.named_port())); + } + return resolve(socket_address.address()); + + default: + throw EnvoyException(fmt::format("SRV resolver can't handle port specifier type {}", + socket_address.port_specifier_case())); + } +} + +InstanceConstSharedPtr SrvResolver::resolve(std::string socket_address) { + bool timed_out; + InstanceConstSharedPtr address; + absl::BlockingCounter latch(1); + + Network::ActiveDnsQuery* active_query = dns_resolver_->resolveSrv( + socket_address, DnsLookupFamily::Auto, + [&address, &timed_out, &latch](const std::list&& srv_records) -> void { + address = srv_records.front().address_; + timed_out = false; + latch.DecrementCount(); + }); + + Event::TimerPtr dns_timeout = dispatcher_.createTimer([&timed_out, &latch]() -> void { + timed_out = true; + latch.DecrementCount(); + }); + dns_timeout->enableTimer(std::chrono::seconds(5)); + + latch.Wait(); + dns_timeout->disableTimer(); + dns_timeout.reset(); + if (active_query) { + active_query->cancel(); + } + + if (timed_out) { + throw EnvoyException(fmt::format("SRV resolver timed out for address {}", socket_address)); + } + return address; +} + InstanceConstSharedPtr resolveProtoAddress(const envoy::api::v2::core::Address& address) { switch (address.address_case()) { case envoy::api::v2::core::Address::kSocketAddress: diff --git a/source/common/network/resolver_impl.h b/source/common/network/resolver_impl.h index 7341bdc46d04..69ebb298f832 100644 --- a/source/common/network/resolver_impl.h +++ b/source/common/network/resolver_impl.h @@ -3,13 +3,36 @@ #include "envoy/api/v2/core/address.pb.h" #include "envoy/network/address.h" #include "envoy/network/connection.h" +#include "envoy/network/dns.h" #include "envoy/network/resolver.h" +#include "common/config/well_known_names.h" #include "common/network/address_impl.h" namespace Envoy { namespace Network { namespace Address { + +/** + * Implementation of a resolver for SRV records. + */ +class SrvResolver : public Resolver { +public: + SrvResolver(Network::DnsResolverSharedPtr dns_resolver, Event::Dispatcher& dispatcher) + : dns_resolver_(dns_resolver), dispatcher_(dispatcher) {} + + // Address::Resolver + InstanceConstSharedPtr + resolve(const envoy::api::v2::core::SocketAddress& socket_address) override; + std::string name() const override { return Config::AddressResolverNames::get().SRV; } + +private: + InstanceConstSharedPtr resolve(std::string socket_address); + + Network::DnsResolverSharedPtr dns_resolver_; + Event::Dispatcher& dispatcher_; +}; + /** * Create an Instance from a envoy::api::v2::core::Address. * @param address supplies the address proto to resolve. @@ -24,6 +47,7 @@ Address::InstanceConstSharedPtr resolveProtoAddress(const envoy::api::v2::core:: */ Address::InstanceConstSharedPtr resolveProtoSocketAddress(const envoy::api::v2::core::SocketAddress& address); + } // namespace Address } // namespace Network } // namespace Envoy diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index b6b90c5a8acc..15dc6e5d2a5b 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -28,6 +28,7 @@ #include "common/common/assert.h" #include "common/common/cleanup.h" #include "common/common/utility.h" +#include "common/config/well_known_names.h" #include "common/network/address_impl.h" #include "common/protobuf/protobuf.h" @@ -101,6 +102,25 @@ uint32_t portFromUrl(const std::string& url, const std::string& scheme, } // namespace +std::string Utility::urlFromSocketAddress(const envoy::api::v2::core::SocketAddress& address, + const std::string& cluster_type) { + if (address.resolver_name().empty() || + address.resolver_name() == Config::AddressResolverNames::get().IP) { + return fmt::format("tcp://{}:{}", address.address(), address.port_value()); + } else if (address.resolver_name() == Config::AddressResolverNames::get().SRV) { + if (absl::EndsWithIgnoreCase(address.named_port(), "srv")) { + return fmt::format("tcp://{}:{}", address.address(), address.named_port()); + } else { + throw EnvoyException( + fmt::format("named_port must be set to \"srv\" when using the {} resolver type", + Config::AddressResolverNames::get().SRV)); + } + } + throw EnvoyException(fmt::format("{} clusters only support {} and {} resolver types", + cluster_type, Config::AddressResolverNames::get().IP, + Config::AddressResolverNames::get().SRV)); +} + std::string Utility::hostFromTcpUrl(const std::string& url) { return hostFromUrl(url, TCP_SCHEME, "TCP"); } diff --git a/source/common/network/utility.h b/source/common/network/utility.h index 24eb526b6a6e..0e1a66ecc33d 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -65,6 +65,15 @@ class Utility { */ static bool urlIsUnixScheme(const std::string& url); + /** + * Generates a TCP URL given a SocketAddress instance + * @param the SocketAddress to generate the TCP URL from + * @param the cluster implementation requesting the URL generation + * @return std::string the generated TCP URL + */ + static std::string urlFromSocketAddress(const envoy::api::v2::core::SocketAddress& address, + const std::string& cluster_type); + /** * Parses the host from a TCP URL * @param the URL to parse host from diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index f215651682dd..3d462623b558 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -273,7 +273,7 @@ EdsClusterFactory::createClusterImpl( } /** - * Static registration for the strict dns cluster factory. @see RegisterFactory. + * Static registration for the EDS cluster factory. @see RegisterFactory. */ REGISTER_FACTORY(EdsClusterFactory, ClusterFactory); diff --git a/source/common/upstream/logical_dns_cluster.cc b/source/common/upstream/logical_dns_cluster.cc index 06c1e415b845..3d4b673c1a8c 100644 --- a/source/common/upstream/logical_dns_cluster.cc +++ b/source/common/upstream/logical_dns_cluster.cc @@ -46,14 +46,15 @@ LogicalDnsCluster::LogicalDnsCluster( const envoy::api::v2::core::SocketAddress& socket_address = lbEndpoint().endpoint().address().socket_address(); - if (!socket_address.resolver_name().empty()) { - throw EnvoyException("LOGICAL_DNS clusters must NOT have a custom resolver name set"); - } - - dns_url_ = fmt::format("tcp://{}:{}", socket_address.address(), socket_address.port_value()); + dns_url_ = Network::Utility::urlFromSocketAddress(socket_address, "LOGICAL_DNS"); hostname_ = Network::Utility::hostFromTcpUrl(dns_url_); - Network::Utility::portFromTcpUrl(dns_url_); dns_lookup_family_ = getDnsLookupFamilyFromCluster(cluster); + + if (absl::EndsWithIgnoreCase(dns_url_, ":srv")) { + srv_ = true; + } else { + Network::Utility::portFromTcpUrl(dns_url_); + } } void LogicalDnsCluster::startPreInit() { startResolve(); } @@ -64,57 +65,76 @@ LogicalDnsCluster::~LogicalDnsCluster() { } } +void LogicalDnsCluster::updateHosts( + std::string dns_address, const std::list&& response, + std::function + translate) { + active_dns_query_ = nullptr; + ENVOY_LOG(debug, "async DNS resolution complete for {}", dns_address); + info_->stats().update_success_.inc(); + + std::chrono::milliseconds refresh_rate = dns_refresh_rate_ms_; + if (!response.empty()) { + // TODO(mattklein123): Move port handling into the DNS interface. + ASSERT(response.front().address_ != nullptr); + Network::Address::InstanceConstSharedPtr new_address = translate(response.front()); + + if (respect_dns_ttl_ && response.front().ttl_ != std::chrono::seconds(0)) { + refresh_rate = response.front().ttl_; + } + + if (!logical_host_) { + logical_host_.reset(new LogicalHost(info_, hostname_, new_address, localityLbEndpoint(), + lbEndpoint(), nullptr)); + + const auto& locality_lb_endpoint = localityLbEndpoint(); + PriorityStateManager priority_state_manager(*this, local_info_, nullptr); + priority_state_manager.initializePriorityFor(locality_lb_endpoint); + priority_state_manager.registerHostForPriority(logical_host_, locality_lb_endpoint); + + const uint32_t priority = locality_lb_endpoint.priority(); + priority_state_manager.updateClusterPrioritySet( + priority, std::move(priority_state_manager.priorityState()[priority].first), + absl::nullopt, absl::nullopt, absl::nullopt); + } + + if (!current_resolved_address_ || !(*new_address == *current_resolved_address_)) { + current_resolved_address_ = new_address; + + // Make sure that we have an updated address for admin display, health + // checking, and creating real host connections. + logical_host_->setNewAddress(new_address, lbEndpoint()); + } + } + + onPreInitComplete(); + resolve_timer_->enableTimer(refresh_rate); +} + void LogicalDnsCluster::startResolve() { std::string dns_address = Network::Utility::hostFromTcpUrl(dns_url_); ENVOY_LOG(debug, "starting async DNS resolution for {}", dns_address); info_->stats().update_attempt_.inc(); - active_dns_query_ = dns_resolver_->resolve( - dns_address, dns_lookup_family_, - [this, dns_address](std::list&& response) -> void { - active_dns_query_ = nullptr; - ENVOY_LOG(debug, "async DNS resolution complete for {}", dns_address); - info_->stats().update_success_.inc(); - - std::chrono::milliseconds refresh_rate = dns_refresh_rate_ms_; - if (!response.empty()) { - // TODO(mattklein123): Move port handling into the DNS interface. - ASSERT(response.front().address_ != nullptr); - Network::Address::InstanceConstSharedPtr new_address = - Network::Utility::getAddressWithPort(*(response.front().address_), - Network::Utility::portFromTcpUrl(dns_url_)); - - if (respect_dns_ttl_ && response.front().ttl_ != std::chrono::seconds(0)) { - refresh_rate = response.front().ttl_; - } - - if (!logical_host_) { - logical_host_.reset(new LogicalHost(info_, hostname_, new_address, localityLbEndpoint(), - lbEndpoint(), nullptr)); - - const auto& locality_lb_endpoint = localityLbEndpoint(); - PriorityStateManager priority_state_manager(*this, local_info_, nullptr); - priority_state_manager.initializePriorityFor(locality_lb_endpoint); - priority_state_manager.registerHostForPriority(logical_host_, locality_lb_endpoint); - - const uint32_t priority = locality_lb_endpoint.priority(); - priority_state_manager.updateClusterPrioritySet( - priority, std::move(priority_state_manager.priorityState()[priority].first), - absl::nullopt, absl::nullopt, absl::nullopt); - } - - if (!current_resolved_address_ || !(*new_address == *current_resolved_address_)) { - current_resolved_address_ = new_address; - - // Make sure that we have an updated address for admin display, health - // checking, and creating real host connections. - logical_host_->setNewAddress(new_address, lbEndpoint()); - } - } - - onPreInitComplete(); - resolve_timer_->enableTimer(refresh_rate); - }); + if (srv_) { + active_dns_query_ = dns_resolver_->resolveSrv( + dns_address, dns_lookup_family_, + [this, dns_address](std::list&& response) -> void { + updateHosts( + dns_address, std::move(response), + [](const Network::DnsResponse& dns_response) { return dns_response.address_; }); + }); + } else { + active_dns_query_ = dns_resolver_->resolve( + dns_address, dns_lookup_family_, + [this, dns_address](std::list&& response) -> void { + updateHosts(dns_address, std::move(response), + [this](const Network::DnsResponse& dns_response) { + return Network::Utility::getAddressWithPort( + *dns_response.address_, Network::Utility::portFromTcpUrl(dns_url_)); + }); + }); + } } std::pair @@ -131,7 +151,7 @@ LogicalDnsClusterFactory::createClusterImpl( } /** - * Static registration for the strict dns cluster factory. @see RegisterFactory. + * Static registration for the logical dns cluster factory. @see RegisterFactory. */ REGISTER_FACTORY(LogicalDnsClusterFactory, ClusterFactory); diff --git a/source/common/upstream/logical_dns_cluster.h b/source/common/upstream/logical_dns_cluster.h index 68e786c80af6..0bdafd260a1c 100644 --- a/source/common/upstream/logical_dns_cluster.h +++ b/source/common/upstream/logical_dns_cluster.h @@ -55,6 +55,11 @@ class LogicalDnsCluster : public ClusterImplBase { return localityLbEndpoint().lb_endpoints()[0]; } + void + updateHosts(std::string dns_address, const std::list&& response, + std::function + translate); + void startResolve(); // ClusterImplBase @@ -67,6 +72,7 @@ class LogicalDnsCluster : public ClusterImplBase { Event::TimerPtr resolve_timer_; std::string dns_url_; std::string hostname_; + bool srv_{false}; Network::Address::InstanceConstSharedPtr current_resolved_address_; LogicalHostSharedPtr logical_host_; Network::ActiveDnsQuery* active_dns_query_{}; diff --git a/source/common/upstream/original_dst_cluster.cc b/source/common/upstream/original_dst_cluster.cc index e061a7c390cc..2e95e5a92df0 100644 --- a/source/common/upstream/original_dst_cluster.cc +++ b/source/common/upstream/original_dst_cluster.cc @@ -213,7 +213,7 @@ OriginalDstClusterFactory::createClusterImpl( } /** - * Static registration for the strict dns cluster factory. @see RegisterFactory. + * Static registration for the original destination cluster factory. @see RegisterFactory. */ REGISTER_FACTORY(OriginalDstClusterFactory, ClusterFactory); diff --git a/source/common/upstream/strict_dns_cluster.cc b/source/common/upstream/strict_dns_cluster.cc index 71aa2b3c71e0..9c66e103b20b 100644 --- a/source/common/upstream/strict_dns_cluster.cc +++ b/source/common/upstream/strict_dns_cluster.cc @@ -22,12 +22,7 @@ StrictDnsClusterImpl::StrictDnsClusterImpl( for (const auto& locality_lb_endpoint : locality_lb_endpoints) { for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { const auto& socket_address = lb_endpoint.endpoint().address().socket_address(); - if (!socket_address.resolver_name().empty()) { - throw EnvoyException("STRICT_DNS clusters must NOT have a custom resolver name set"); - } - - const std::string& url = - fmt::format("tcp://{}:{}", socket_address.address(), socket_address.port_value()); + const std::string& url = Network::Utility::urlFromSocketAddress(socket_address, "STRICT_DNS"); resolve_targets.emplace_back(new ResolveTarget(*this, factory_context.dispatcher(), url, locality_lb_endpoint, lb_endpoint)); } @@ -75,9 +70,14 @@ StrictDnsClusterImpl::ResolveTarget::ResolveTarget( const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint) : parent_(parent), dns_address_(Network::Utility::hostFromTcpUrl(url)), - port_(Network::Utility::portFromTcpUrl(url)), resolve_timer_(dispatcher.createTimer([this]() -> void { startResolve(); })), - locality_lb_endpoint_(locality_lb_endpoint), lb_endpoint_(lb_endpoint) {} + locality_lb_endpoint_(locality_lb_endpoint), lb_endpoint_(lb_endpoint) { + if (absl::EndsWithIgnoreCase(url, ":srv")) { + srv_ = true; + } else { + port_ = Network::Utility::portFromTcpUrl(url); + } +} StrictDnsClusterImpl::ResolveTarget::~ResolveTarget() { if (active_query_) { @@ -85,67 +85,80 @@ StrictDnsClusterImpl::ResolveTarget::~ResolveTarget() { } } +void StrictDnsClusterImpl::ResolveTarget::updateHosts( + const std::list&& response, + std::function + translate) { + active_query_ = nullptr; + ENVOY_LOG(trace, "async DNS resolution complete for {}", dns_address_); + parent_.info_->stats().update_success_.inc(); + + std::unordered_map updated_hosts; + HostVector new_hosts; + std::chrono::seconds ttl_refresh_rate = std::chrono::seconds::max(); + for (const auto& resp : response) { + ASSERT(resp.address_ != nullptr); + new_hosts.emplace_back( + new HostImpl(parent_.info_, dns_address_, translate(resp), lb_endpoint_.metadata(), + lb_endpoint_.load_balancing_weight().value(), locality_lb_endpoint_.locality(), + lb_endpoint_.endpoint().health_check_config(), + locality_lb_endpoint_.priority(), lb_endpoint_.health_status())); + ttl_refresh_rate = min(ttl_refresh_rate, resp.ttl_); + } + + HostVector hosts_added; + HostVector hosts_removed; + if (parent_.updateDynamicHostList(new_hosts, hosts_, hosts_added, hosts_removed, updated_hosts, + all_hosts_)) { + ENVOY_LOG(debug, "DNS hosts have changed for {}", dns_address_); + ASSERT(std::all_of(hosts_.begin(), hosts_.end(), [&](const auto& host) { + return host->priority() == locality_lb_endpoint_.priority(); + })); + parent_.updateAllHosts(hosts_added, hosts_removed, locality_lb_endpoint_.priority()); + } else { + parent_.info_->stats().update_no_rebuild_.inc(); + } + + all_hosts_ = std::move(updated_hosts); + + // If there is an initialize callback, fire it now. Note that if the cluster refers to + // multiple DNS names, this will return initialized after a single DNS resolution + // completes. This is not perfect but is easier to code and unclear if the extra + // complexity is needed so will start with this. + parent_.onPreInitComplete(); + + std::chrono::milliseconds final_refresh_rate = parent_.dns_refresh_rate_ms_; + + if (parent_.respect_dns_ttl_ && ttl_refresh_rate != std::chrono::seconds(0)) { + final_refresh_rate = ttl_refresh_rate; + ENVOY_LOG(debug, "DNS refresh rate reset for {}, refresh rate {} ms", dns_address_, + final_refresh_rate.count()); + } + + resolve_timer_->enableTimer(final_refresh_rate); +} + void StrictDnsClusterImpl::ResolveTarget::startResolve() { ENVOY_LOG(trace, "starting async DNS resolution for {}", dns_address_); parent_.info_->stats().update_attempt_.inc(); - active_query_ = parent_.dns_resolver_->resolve( - dns_address_, parent_.dns_lookup_family_, - [this](std::list&& response) -> void { - active_query_ = nullptr; - ENVOY_LOG(trace, "async DNS resolution complete for {}", dns_address_); - parent_.info_->stats().update_success_.inc(); - - std::unordered_map updated_hosts; - HostVector new_hosts; - std::chrono::seconds ttl_refresh_rate = std::chrono::seconds::max(); - for (const auto& resp : response) { - // TODO(mattklein123): Currently the DNS interface does not consider port. We need to - // make a new address that has port in it. We need to both support IPv6 as well as - // potentially move port handling into the DNS interface itself, which would work better - // for SRV. - ASSERT(resp.address_ != nullptr); - new_hosts.emplace_back(new HostImpl( - parent_.info_, dns_address_, - Network::Utility::getAddressWithPort(*(resp.address_), port_), - lb_endpoint_.metadata(), lb_endpoint_.load_balancing_weight().value(), - locality_lb_endpoint_.locality(), lb_endpoint_.endpoint().health_check_config(), - locality_lb_endpoint_.priority(), lb_endpoint_.health_status())); - - ttl_refresh_rate = min(ttl_refresh_rate, resp.ttl_); - } - - HostVector hosts_added; - HostVector hosts_removed; - if (parent_.updateDynamicHostList(new_hosts, hosts_, hosts_added, hosts_removed, - updated_hosts, all_hosts_)) { - ENVOY_LOG(debug, "DNS hosts have changed for {}", dns_address_); - ASSERT(std::all_of(hosts_.begin(), hosts_.end(), [&](const auto& host) { - return host->priority() == locality_lb_endpoint_.priority(); - })); - parent_.updateAllHosts(hosts_added, hosts_removed, locality_lb_endpoint_.priority()); - } else { - parent_.info_->stats().update_no_rebuild_.inc(); - } - - all_hosts_ = std::move(updated_hosts); - - // If there is an initialize callback, fire it now. Note that if the cluster refers to - // multiple DNS names, this will return initialized after a single DNS resolution - // completes. This is not perfect but is easier to code and unclear if the extra - // complexity is needed so will start with this. - parent_.onPreInitComplete(); - - std::chrono::milliseconds final_refresh_rate = parent_.dns_refresh_rate_ms_; - - if (parent_.respect_dns_ttl_ && ttl_refresh_rate != std::chrono::seconds(0)) { - final_refresh_rate = ttl_refresh_rate; - ENVOY_LOG(debug, "DNS refresh rate reset for {}, refresh rate {} ms", dns_address_, - final_refresh_rate.count()); - } - - resolve_timer_->enableTimer(final_refresh_rate); - }); + if (srv_) { + active_query_ = parent_.dns_resolver_->resolveSrv( + dns_address_, parent_.dns_lookup_family_, + [this](std::list&& response) -> void { + updateHosts(std::move(response), [](const Network::DnsResponse& dns_response) { + return dns_response.address_; + }); + }); + } else { + active_query_ = parent_.dns_resolver_->resolve( + dns_address_, parent_.dns_lookup_family_, + [this](std::list&& response) -> void { + updateHosts(std::move(response), [this](const Network::DnsResponse& dns_response) { + return Network::Utility::getAddressWithPort(*dns_response.address_, port_); + }); + }); + } } std::pair diff --git a/source/common/upstream/strict_dns_cluster.h b/source/common/upstream/strict_dns_cluster.h index 2b0c8f4f135d..6861f1658287 100644 --- a/source/common/upstream/strict_dns_cluster.h +++ b/source/common/upstream/strict_dns_cluster.h @@ -27,11 +27,18 @@ class StrictDnsClusterImpl : public BaseDynamicClusterImpl { const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint); ~ResolveTarget(); + + void + updateHosts(const std::list&& response, + std::function + translate); + void startResolve(); StrictDnsClusterImpl& parent_; Network::ActiveDnsQuery* active_query_{}; std::string dns_address_; + bool srv_{false}; uint32_t port_; Event::TimerPtr resolve_timer_; HostVector hosts_; diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index bf5160805944..d7016ef05f04 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -126,6 +126,8 @@ RedisCluster::DnsDiscoveryResolveTarget::~DnsDiscoveryResolveTarget() { void RedisCluster::DnsDiscoveryResolveTarget::startResolveDns() { ENVOY_LOG(trace, "starting async DNS resolution for {}", dns_address_); + // TODO(venilnoronha): Wire this with the SRV resolver also? + active_query_ = parent_.dns_resolver_->resolve( dns_address_, parent_.dns_lookup_family_, [this](std::list&& response) -> void { diff --git a/source/server/BUILD b/source/server/BUILD index 9eb61a7fca69..701cbebd1552 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -369,6 +369,7 @@ envoy_cc_library( "//source/common/local_info:local_info_lib", "//source/common/memory:heap_shrinker_lib", "//source/common/memory:stats_lib", + "//source/common/network:resolver_lib", "//source/common/protobuf:utility_lib", "//source/common/router:rds_lib", "//source/common/runtime:runtime_lib", diff --git a/source/server/config_validation/dns.cc b/source/server/config_validation/dns.cc index 178e41df18e1..f9412fe50c41 100644 --- a/source/server/config_validation/dns.cc +++ b/source/server/config_validation/dns.cc @@ -9,5 +9,11 @@ ActiveDnsQuery* ValidationDnsResolver::resolve(const std::string&, DnsLookupFami return nullptr; } +ActiveDnsQuery* ValidationDnsResolver::resolveSrv(const std::string&, DnsLookupFamily, + ResolveCb callback) { + callback({}); + return nullptr; +} + } // namespace Network } // namespace Envoy diff --git a/source/server/config_validation/dns.h b/source/server/config_validation/dns.h index 3777256579b0..857b0b7dd54b 100644 --- a/source/server/config_validation/dns.h +++ b/source/server/config_validation/dns.h @@ -17,6 +17,8 @@ class ValidationDnsResolver : public DnsResolver { // Network::DnsResolver ActiveDnsQuery* resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback) override; + ActiveDnsQuery* resolveSrv(const std::string& dns_name, DnsLookupFamily dns_lookup_family, + ResolveCb callback) override; }; } // namespace Network diff --git a/source/server/server.cc b/source/server/server.cc index 072a7612ec2b..a9c6f46d5a96 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -28,10 +28,12 @@ #include "common/common/version.h" #include "common/config/resources.h" #include "common/config/utility.h" +#include "common/config/well_known_names.h" #include "common/http/codes.h" #include "common/local_info/local_info_impl.h" #include "common/memory/stats.h" #include "common/network/address_impl.h" +#include "common/network/resolver_impl.h" #include "common/protobuf/utility.h" #include "common/router/rds_impl.h" #include "common/runtime/runtime_impl.h" @@ -260,6 +262,18 @@ void InstanceImpl::initialize(const Options& options, Registry::FactoryRegistry< Configuration::UpstreamTransportSocketConfigFactory>::allFactoryNames()); + // Dynamically register the SrvResolver while injecting the necessary dependencies. The nullptr + // check ensures that tests don't fail due to double registration of SrvResolver. + // + // TODO (venilnoronha): Create an abstraction for handling dynamic registration of factories by + // enabling dependency injection at runtime. + if (Registry::FactoryRegistry::getFactory( + Config::AddressResolverNames::get().SRV) == nullptr) { + Network::Address::SrvResolver* srv_resolver = + new Network::Address::SrvResolver(dns_resolver_, *dispatcher_); + Registry::FactoryRegistry::registerFactory(*srv_resolver); + } + // Enable the selected buffer implementation (old libevent evbuffer version or new native // version) early in the initialization, before any buffers can be created. Buffer::OwnedImpl::useOldImpl(options.libeventBufferEnabled()); diff --git a/test/common/network/utility_test.cc b/test/common/network/utility_test.cc index fe738d359e36..d475a00642e0 100644 --- a/test/common/network/utility_test.cc +++ b/test/common/network/utility_test.cc @@ -19,6 +19,101 @@ namespace Envoy { namespace Network { namespace { +TEST(NetworkUtility, urlFromSocketAddress) { + { + envoy::api::v2::core::SocketAddress address; + address.set_address("127.0.0.1"); + address.set_port_value(1234); + EXPECT_EQ("tcp://127.0.0.1:1234", Utility::urlFromSocketAddress(address, "TEST")); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("foo"); + address.set_port_value(1234); + EXPECT_EQ("tcp://foo:1234", Utility::urlFromSocketAddress(address, "TEST")); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("127.0.0.1"); + address.set_port_value(1234); + address.set_resolver_name("envoy.ip"); + EXPECT_EQ("tcp://127.0.0.1:1234", Utility::urlFromSocketAddress(address, "TEST")); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("foo"); + address.set_port_value(1234); + address.set_resolver_name("envoy.ip"); + EXPECT_EQ("tcp://foo:1234", Utility::urlFromSocketAddress(address, "TEST")); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("127.0.0.1"); + address.set_port_value(1234); + address.set_resolver_name("envoy.srv"); + EXPECT_THROW_WITH_MESSAGE( + Utility::urlFromSocketAddress(address, "TEST"), EnvoyException, + "named_port must be set to \"srv\" when using the envoy.srv resolver type"); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("foo"); + address.set_port_value(1234); + address.set_resolver_name("envoy.srv"); + EXPECT_THROW_WITH_MESSAGE( + Utility::urlFromSocketAddress(address, "TEST"), EnvoyException, + "named_port must be set to \"srv\" when using the envoy.srv resolver type"); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("127.0.0.1"); + address.set_named_port("srv"); + address.set_resolver_name("envoy.srv"); + EXPECT_EQ("tcp://127.0.0.1:srv", Utility::urlFromSocketAddress(address, "TEST")); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("foo"); + address.set_named_port("SRV"); + address.set_resolver_name("envoy.srv"); + EXPECT_EQ("tcp://foo:SRV", Utility::urlFromSocketAddress(address, "TEST")); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("127.0.0.1"); + address.set_named_port("invalid"); + address.set_resolver_name("envoy.srv"); + EXPECT_THROW_WITH_MESSAGE( + Utility::urlFromSocketAddress(address, "TEST"), EnvoyException, + "named_port must be set to \"srv\" when using the envoy.srv resolver type"); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("foo"); + address.set_named_port("invalid"); + address.set_resolver_name("envoy.srv"); + EXPECT_THROW_WITH_MESSAGE( + Utility::urlFromSocketAddress(address, "TEST"), EnvoyException, + "named_port must be set to \"srv\" when using the envoy.srv resolver type"); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("127.0.0.1"); + address.set_named_port("srv"); + address.set_resolver_name("invalid"); + EXPECT_THROW_WITH_MESSAGE(Utility::urlFromSocketAddress(address, "TEST"), EnvoyException, + "TEST clusters only support envoy.ip and envoy.srv resolver types"); + } + { + envoy::api::v2::core::SocketAddress address; + address.set_address("foo"); + address.set_named_port("SRV"); + address.set_resolver_name("invalid"); + EXPECT_THROW_WITH_MESSAGE(Utility::urlFromSocketAddress(address, "TEST"), EnvoyException, + "TEST clusters only support envoy.ip and envoy.srv resolver types"); + } +} + TEST(NetworkUtility, Url) { EXPECT_EQ("foo", Utility::hostFromTcpUrl("tcp://foo:1234")); EXPECT_EQ(1234U, Utility::portFromTcpUrl("tcp://foo:1234")); diff --git a/test/common/upstream/logical_dns_cluster_test.cc b/test/common/upstream/logical_dns_cluster_test.cc index 03170a23af5a..bf938cfe220a 100644 --- a/test/common/upstream/logical_dns_cluster_test.cc +++ b/test/common/upstream/logical_dns_cluster_test.cc @@ -389,8 +389,9 @@ TEST_F(LogicalDnsClusterTest, BadConfig) { port_value: 8000 )EOF"; - EXPECT_THROW_WITH_MESSAGE(setupFromV2Yaml(custom_resolver_yaml), EnvoyException, - "LOGICAL_DNS clusters must NOT have a custom resolver name set"); + EXPECT_THROW_WITH_MESSAGE( + setupFromV2Yaml(custom_resolver_yaml), EnvoyException, + "LOGICAL_DNS clusters only support envoy.ip and envoy.srv resolver types"); } TEST_F(LogicalDnsClusterTest, Basic) { diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index b0b8094c55b2..12d2197b0716 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -883,7 +883,7 @@ TEST_F(StrictDnsClusterImplTest, CustomResolverFails) { EXPECT_THROW_WITH_MESSAGE( std::make_unique(cluster_config, runtime_, dns_resolver_, factory_context, std::move(scope), false), - EnvoyException, "STRICT_DNS clusters must NOT have a custom resolver name set"); + EnvoyException, "STRICT_DNS clusters only support envoy.ip and envoy.srv resolver types"); } TEST_F(StrictDnsClusterImplTest, RecordTtlAsDnsRefreshRate) { diff --git a/test/mocks/network/mocks.cc b/test/mocks/network/mocks.cc index b3307515d5cc..56c09ff0af14 100644 --- a/test/mocks/network/mocks.cc +++ b/test/mocks/network/mocks.cc @@ -37,6 +37,7 @@ MockActiveDnsQuery::~MockActiveDnsQuery() {} MockDnsResolver::MockDnsResolver() { ON_CALL(*this, resolve(_, _, _)).WillByDefault(Return(&active_query_)); + ON_CALL(*this, resolveSrv(_, _, _)).WillByDefault(Return(&active_query_)); } MockDnsResolver::~MockDnsResolver() {} diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 80fec6e54c42..04ffcdf000da 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -43,6 +43,8 @@ class MockDnsResolver : public DnsResolver { // Network::DnsResolver MOCK_METHOD3(resolve, ActiveDnsQuery*(const std::string& dns_name, DnsLookupFamily dns_lookup_family, ResolveCb callback)); + MOCK_METHOD3(resolveSrv, ActiveDnsQuery*(const std::string& dns_name, + DnsLookupFamily dns_lookup_family, ResolveCb callback)); testing::NiceMock active_query_; };