diff --git a/STYLE.md b/STYLE.md index 265ba39f36e70..fdb344e160152 100644 --- a/STYLE.md +++ b/STYLE.md @@ -17,6 +17,7 @@ includes both const and non-const references. * Function names using camel case starting with a lower case letter (e.g., "doFoo()"). * Struct/Class member variables have a '\_' postfix (e.g., "int foo\_;"). +* Enum values using PascalCase (e.g., `RoundRobin`). * 100 columns is the line limit. * Use your GitHub name in TODO comments, e.g. `TODO(foobar): blah`. * Smart pointers are type aliased: diff --git a/docs/configuration/cluster_manager/cluster.rst b/docs/configuration/cluster_manager/cluster.rst index 21c84243f37f8..b3b6cc4ab4e75 100644 --- a/docs/configuration/cluster_manager/cluster.rst +++ b/docs/configuration/cluster_manager/cluster.rst @@ -20,6 +20,7 @@ Cluster "features": "...", "http_codec_options": "...", "dns_refresh_rate_ms": "...", + "dns_lookup_family": "...", "outlier_detection": "{...}" } @@ -144,6 +145,17 @@ dns_refresh_rate_ms the value defaults to 5000. For cluster types other than *strict_dns* and *logical_dns* this setting is ignored. +.. _config_cluster_manager_cluster_dns_lookup_family: + +dns_lookup_family + *(optional, string)* The DNS IP address resolution policy. The options are *v4_only*, *v6_only*, + and *auto*. If this setting is not specified, the value defaults to *v4_only*. When *v4_only* is selected, + the DNS resolver will only perform a lookup for addresses in the IPv4 family. If *v6_only* is selected, + the DNS resolver will only perform a lookup for addresses in the IPv6 family. If *auto* is specified, + the DNS resolver will first perform a lookup for addresses in the IPv6 family and fallback to a lookup for + addresses in the IPv4 family. For cluster types other than *strict_dns* and *logical_dns*, this setting + is ignored. + .. _config_cluster_manager_cluster_outlier_detection: outlier_detection diff --git a/include/envoy/network/dns.h b/include/envoy/network/dns.h index 30658930967ac..477434940b030 100644 --- a/include/envoy/network/dns.h +++ b/include/envoy/network/dns.h @@ -24,6 +24,8 @@ class ActiveDnsQuery { virtual void cancel() PURE; }; +enum class DnsLookupFamily { V4Only, V6Only, Auto }; + /** * An asynchronous DNS resolver. */ @@ -41,11 +43,13 @@ class DnsResolver { /** * Initiate an async DNS resolution. * @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* resolve(const std::string& dns_name, ResolveCb callback) PURE; + virtual ActiveDnsQuery* resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, + ResolveCb callback) PURE; }; typedef std::unique_ptr DnsResolverPtr; diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index 48e758386345c..c427fd80611b7 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -1210,6 +1210,10 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF( "minimum" : 0, "exclusiveMinimum" : true }, + "dns_lookup_family" : { + "type" : "string", + "enum" : ["v4_only", "v6_only", "auto"] + }, "outlier_detection" : { "type" : "object", "properties" : { diff --git a/source/common/network/dns_impl.cc b/source/common/network/dns_impl.cc index 72567dd1209ab..37a6db10ae7d3 100644 --- a/source/common/network/dns_impl.cc +++ b/source/common/network/dns_impl.cc @@ -52,26 +52,51 @@ void DnsResolverImpl::PendingResolution::onAresHostCallback(int status, hostent* delete this; return; } + if (status == ARES_SUCCESS || !fallback_if_failed_) { + completed_ = true; + } + std::list address_list; - completed_ = true; if (status == ARES_SUCCESS) { - ASSERT(hostent->h_addrtype == AF_INET); - for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) { - ASSERT(hostent->h_length == sizeof(in_addr)); - sockaddr_in address; - memset(&address, 0, sizeof(address)); - // TODO(mattklein123): IPv6 support. - address.sin_family = AF_INET; - address.sin_port = 0; - address.sin_addr = *reinterpret_cast(hostent->h_addr_list[i]); - address_list.emplace_back(new Address::Ipv4Instance(&address)); + if (hostent->h_addrtype == AF_INET) { + for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) { + ASSERT(hostent->h_length == sizeof(in_addr)); + sockaddr_in address; + memset(&address, 0, sizeof(address)); + address.sin_family = AF_INET; + address.sin_port = 0; + address.sin_addr = *reinterpret_cast(hostent->h_addr_list[i]); + address_list.emplace_back(new Address::Ipv4Instance(&address)); + } + } else if (hostent->h_addrtype == AF_INET6) { + for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) { + ASSERT(hostent->h_length == sizeof(in6_addr)); + sockaddr_in6 address; + memset(&address, 0, sizeof(address)); + address.sin6_family = AF_INET6; + address.sin6_port = 0; + address.sin6_addr = *reinterpret_cast(hostent->h_addr_list[i]); + address_list.emplace_back(new Address::Ipv6Instance(address)); + } } } - if (!cancelled_) { - callback_(std::move(address_list)); + + if (completed_) { + if (!cancelled_) { + callback_(std::move(address_list)); + } + if (owned_) { + delete this; + return; + } } - if (owned_) { - delete this; + + if (status != ARES_SUCCESS && fallback_if_failed_) { + fallback_if_failed_ = false; + getHostByName(AF_INET); + // Note: Nothing can follow this call to getHostByName due to deletion of this + // object upon synchronous resolution. + return; } } @@ -115,17 +140,26 @@ void DnsResolverImpl::onAresSocketStateChange(int fd, int read, int write) { (write ? Event::FileReadyType::Write : 0)); } -ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, ResolveCb callback) { - std::unique_ptr pending_resolution(new PendingResolution()); - pending_resolution->callback_ = callback; +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 + // failed intial call to getHostbyName followed by a synchronous IPv4 + // resolution. + std::unique_ptr pending_resolution( + new PendingResolution(callback, channel_, dns_name)); + if (dns_lookup_family == DnsLookupFamily::Auto) { + pending_resolution->fallback_if_failed_ = true; + } - ares_gethostbyname(channel_, dns_name.c_str(), - AF_INET, [](void* arg, int status, int timeouts, hostent* hostent) { - static_cast(arg)->onAresHostCallback(status, hostent); - UNREFERENCED_PARAMETER(timeouts); - }, pending_resolution.get()); + if (dns_lookup_family == DnsLookupFamily::V4Only) { + pending_resolution->getHostByName(AF_INET); + } else { + pending_resolution->getHostByName(AF_INET6); + } if (pending_resolution->completed_) { + // Resolution does not need asynchronous behavior or network events. For + // example, localhost lookup. return nullptr; } else { // The PendingResolution will self-delete when the request completes @@ -135,5 +169,12 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, ResolveCb } } +void DnsResolverImpl::PendingResolution::getHostByName(int family) { + ares_gethostbyname(channel_, dns_name_.c_str(), + family, [](void* arg, int status, int, hostent* hostent) { + static_cast(arg)->onAresHostCallback(status, hostent); + }, this); +} + } // Network } // Envoy diff --git a/source/common/network/dns_impl.h b/source/common/network/dns_impl.h index 7980994cd8072..7ff4d8c1f7d44 100644 --- a/source/common/network/dns_impl.h +++ b/source/common/network/dns_impl.h @@ -29,23 +29,36 @@ class DnsResolverImpl : public DnsResolver { ~DnsResolverImpl() override; // Network::DnsResolver - ActiveDnsQuery* resolve(const std::string& dns_name, ResolveCb callback) override; + ActiveDnsQuery* resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family, + ResolveCb callback) override; private: friend class DnsResolverImplPeer; struct PendingResolution : public ActiveDnsQuery { // Network::ActiveDnsQuery + PendingResolution(ResolveCb callback, ares_channel channel, const std::string& dns_name) + : callback_(callback), channel_(channel), dns_name_(dns_name) {} + void cancel() override { // c-ares only supports channel-wide cancellation, so we just allow the // network events to continue but don't invoke the callback on completion. cancelled_ = true; } - // c-ares ares_gethostbyname() query callback. + /** + * c-ares ares_gethostbyname() query callback. + * @param status return status of call to ares_gethostbyname. + * @param hostent structure that stores information about a given host. + */ void onAresHostCallback(int status, hostent* hostent); + /** + * wrapper function of call to ares_gethostbyname. + * @param family currently AF_INET and AF_INET6 are supported. + */ + void getHostByName(int family); // Caller supplied callback to invoke on query completion or error. - ResolveCb callback_; + const ResolveCb callback_; // Does the object own itself? Resource reclamation occurs via self-deleting // on query completion or error. bool owned_ = false; @@ -53,6 +66,11 @@ class DnsResolverImpl : public DnsResolver { bool completed_ = false; // Was the query cancelled via cancel()? bool cancelled_ = false; + // If dns_lookup_family is "fallback", fallback to v4 address if v6 + // resolution failed. + bool fallback_if_failed_ = false; + const ares_channel channel_; + const std::string dns_name_; }; // Callback for events on sockets tracked in events_. diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index 92356a6610a93..38ccf07f95770 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -299,6 +299,17 @@ Address::InstanceConstSharedPtr Utility::getIpv6AnyAddress() { return any; } +Address::InstanceConstSharedPtr Utility::getAddressWithPort(const Address::Instance& address, + uint32_t port) { + switch (address.ip()->version()) { + case Network::Address::IpVersion::v4: + return std::make_shared(address.ip()->addressAsString(), port); + case Network::Address::IpVersion::v6: + return std::make_shared(address.ip()->addressAsString(), port); + } + NOT_REACHED; +} + Address::InstanceConstSharedPtr Utility::getOriginalDst(int fd) { sockaddr_storage orig_addr; socklen_t addr_len = sizeof(sockaddr_storage); diff --git a/source/common/network/utility.h b/source/common/network/utility.h index 3f105f16f9c1b..244b6b47d7fd6 100644 --- a/source/common/network/utility.h +++ b/source/common/network/utility.h @@ -146,6 +146,14 @@ class Utility { */ static Address::InstanceConstSharedPtr getIpv6AnyAddress(); + /** + * @param address IP address instance. + * @param port to update. + * @return Address::InstanceConstSharedPtr a new address instance with updated port. + */ + static Address::InstanceConstSharedPtr getAddressWithPort(const Address::Instance& address, + uint32_t port); + /** * Retrieve the original destination address from an accepted fd. * The address (IP and port) may be not local and the port may differ from diff --git a/source/common/upstream/logical_dns_cluster.cc b/source/common/upstream/logical_dns_cluster.cc index f974a50574f82..b3f570f06e855 100644 --- a/source/common/upstream/logical_dns_cluster.cc +++ b/source/common/upstream/logical_dns_cluster.cc @@ -20,12 +20,20 @@ LogicalDnsCluster::LogicalDnsCluster(const Json::Object& config, Runtime::Loader std::chrono::milliseconds(config.getInteger("dns_refresh_rate_ms", 5000))), tls_(tls), tls_slot_(tls.allocateSlot()), initialized_(false), resolve_timer_(dispatcher.createTimer([this]() -> void { startResolve(); })) { - std::vector hosts_json = config.getObjectArray("hosts"); if (hosts_json.size() != 1) { throw EnvoyException("logical_dns clusters must have a single host"); } + std::string dns_lookup_family = config.getString("dns_lookup_family", "v4_only"); + if (dns_lookup_family == "v6_only") { + dns_lookup_family_ = Network::DnsLookupFamily::V6Only; + } else if (dns_lookup_family == "auto") { + dns_lookup_family_ = Network::DnsLookupFamily::Auto; + } else { + ASSERT(dns_lookup_family == "v4_only"); + dns_lookup_family_ = Network::DnsLookupFamily::V4Only; + } dns_url_ = hosts_json[0]->getString("url"); hostname_ = Network::Utility::hostFromTcpUrl(dns_url_); Network::Utility::portFromTcpUrl(dns_url_); @@ -51,18 +59,19 @@ void LogicalDnsCluster::startResolve() { info_->stats().update_attempt_.inc(); active_dns_query_ = dns_resolver_.resolve( - dns_address, [this, dns_address]( - std::list&& address_list) -> void { + dns_address, dns_lookup_family_, + [this, dns_address]( + std::list&& address_list) -> void { active_dns_query_ = nullptr; log_debug("async DNS resolution complete for {}", dns_address); info_->stats().update_success_.inc(); if (!address_list.empty()) { - // TODO(mattklein123): IPv6 support as well as moving port handling into the DNS - // interface. - Network::Address::InstanceConstSharedPtr new_address( - new Network::Address::Ipv4Instance(address_list.front()->ip()->addressAsString(), - Network::Utility::portFromTcpUrl(dns_url_))); + // TODO(mattklein123): Move port handling into the DNS interface. + ASSERT(address_list.front() != nullptr); + Network::Address::InstanceConstSharedPtr new_address = + Network::Utility::getAddressWithPort(*address_list.front(), + Network::Utility::portFromTcpUrl(dns_url_)); if (!current_resolved_address_ || !(*new_address == *current_resolved_address_)) { current_resolved_address_ = new_address; // Capture URL to avoid a race with another update. @@ -77,8 +86,16 @@ void LogicalDnsCluster::startResolve() { // to show the friendly DNS name in that output, but currently there is no way to // express a DNS name inside of an Address::Instance. For now this is OK but we might // want to do better again later. - logical_host_.reset(new LogicalHost( - info_, hostname_, Network::Utility::resolveUrl("tcp://0.0.0.0:0"), *this)); + switch (address_list.front()->ip()->version()) { + case Network::Address::IpVersion::v4: + logical_host_.reset( + new LogicalHost(info_, hostname_, Network::Utility::getIpv4AnyAddress(), *this)); + break; + case Network::Address::IpVersion::v6: + logical_host_.reset( + new LogicalHost(info_, hostname_, Network::Utility::getIpv6AnyAddress(), *this)); + break; + } HostVectorSharedPtr new_hosts(new std::vector()); new_hosts->emplace_back(logical_host_); updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_, diff --git a/source/common/upstream/logical_dns_cluster.h b/source/common/upstream/logical_dns_cluster.h index 6572ff901412c..f3c938743ed4b 100644 --- a/source/common/upstream/logical_dns_cluster.h +++ b/source/common/upstream/logical_dns_cluster.h @@ -88,6 +88,7 @@ class LogicalDnsCluster : public ClusterImplBase { Network::DnsResolver& dns_resolver_; const std::chrono::milliseconds dns_refresh_rate_ms_; + Network::DnsLookupFamily dns_lookup_family_; ThreadLocal::Instance& tls_; uint32_t tls_slot_; std::function initialize_callback_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 9289f793c119b..d22e8c7576514 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -370,6 +370,16 @@ StrictDnsClusterImpl::StrictDnsClusterImpl(const Json::Object& config, Runtime:: : BaseDynamicClusterImpl(config, runtime, stats, ssl_context_manager), dns_resolver_(dns_resolver), dns_refresh_rate_ms_(std::chrono::milliseconds( config.getInteger("dns_refresh_rate_ms", 5000))) { + std::string dns_lookup_family = config.getString("dns_lookup_family", "v4_only"); + if (dns_lookup_family == "v6_only") { + dns_lookup_family_ = Network::DnsLookupFamily::V6Only; + } else if (dns_lookup_family == "auto") { + dns_lookup_family_ = Network::DnsLookupFamily::Auto; + } else { + ASSERT(dns_lookup_family == "v4_only"); + dns_lookup_family_ = Network::DnsLookupFamily::V4Only; + } + for (Json::ObjectSharedPtr host : config.getObjectArray("hosts")) { resolve_targets_.emplace_back(new ResolveTarget(*this, dispatcher, host->getString("url"))); } @@ -413,7 +423,7 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() { parent_.info_->stats().update_attempt_.inc(); active_query_ = parent_.dns_resolver_.resolve( - dns_address_, + dns_address_, parent_.dns_lookup_family_, [this](std::list&& address_list) -> void { active_query_ = nullptr; log_debug("async DNS resolution complete for {}", dns_address_); @@ -424,11 +434,10 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() { // 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. - new_hosts.emplace_back(new HostImpl( - parent_.info_, dns_address_, - Network::Address::InstanceConstSharedPtr{ - new Network::Address::Ipv4Instance(address->ip()->addressAsString(), port_)}, - false, 1, "")); + ASSERT(address != nullptr); + new_hosts.emplace_back(new HostImpl(parent_.info_, dns_address_, + Network::Utility::getAddressWithPort(*address, port_), + false, 1, "")); } std::vector hosts_added; diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index c340cd0a88b5f..1c62a19243edd 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -350,6 +350,7 @@ class StrictDnsClusterImpl : public BaseDynamicClusterImpl { Network::DnsResolver& dns_resolver_; std::list resolve_targets_; const std::chrono::milliseconds dns_refresh_rate_ms_; + Network::DnsLookupFamily dns_lookup_family_; }; } // Upstream diff --git a/test/common/network/BUILD b/test/common/network/BUILD index 96a7fdc6895da..4588070ffd84f 100644 --- a/test/common/network/BUILD +++ b/test/common/network/BUILD @@ -63,6 +63,8 @@ envoy_cc_test( "//source/common/network:listen_socket_lib", "//source/common/stats:stats_lib", "//test/mocks/network:network_mocks", + "//test/test_common:environment_lib", + "//test/test_common:network_utility_lib", ], ) diff --git a/test/common/network/dns_impl_test.cc b/test/common/network/dns_impl_test.cc index 93a3f92853333..ba5c12646b4a2 100644 --- a/test/common/network/dns_impl_test.cc +++ b/test/common/network/dns_impl_test.cc @@ -20,6 +20,8 @@ #include "common/stats/stats_impl.h" #include "test/mocks/network/mocks.h" +#include "test/test_common/environment.h" +#include "test/test_common/network_utility.h" #include "test/test_common/printers.h" #include "ares.h" @@ -35,13 +37,14 @@ namespace { typedef std::list IpList; // Map from hostname to IpList. typedef std::unordered_map HostMap; - // Represents a single TestDnsServer query state and lifecycle. This implements // just enough of RFC 1035 to handle queries we generate in the tests below. +enum record_type { A, AAAA }; + class TestDnsServerQuery { public: - TestDnsServerQuery(ConnectionPtr connection, const HostMap& hosts) - : connection_(std::move(connection)), hosts_(hosts) { + TestDnsServerQuery(ConnectionPtr connection, const HostMap& hosts_A, const HostMap& hosts_AAAA) + : connection_(std::move(connection)), hosts_A_(hosts_A), hosts_AAAA_(hosts_AAAA) { connection_->addReadFilter(Network::ReadFilterSharedPtr{new ReadFilter(*this)}); } @@ -88,19 +91,33 @@ class TestDnsServerQuery { // The number of bytes the encoded question name takes up in the request. // Useful in the response when generating resource records containing the // name. - long question_len; + long name_len; + // Get host name from query and use the name to lookup a record + // in a host map. If the query type is of type A, then perform the lookup in + // the hosts_A_ host map. If the query type is of type AAAA, then perform the + // lookup in the hosts_AAAA_ host map. char* name; - ASSERT_EQ(ARES_SUCCESS, ares_expand_name(question, request, size_, &name, &question_len)); - auto it = parent_.hosts_.find(name); + ASSERT_EQ(ARES_SUCCESS, ares_expand_name(question, request, size_, &name, &name_len)); const std::list* ips = nullptr; - if (it != parent_.hosts_.end()) { - ips = &it->second; + // We only expect resources of type A or AAAA. + const int q_type = DNS_QUESTION_TYPE(question + name_len); + ASSERT_TRUE(q_type == T_A || q_type == T_AAAA); + if (q_type == T_A) { + auto it = parent_.hosts_A_.find(name); + if (it != parent_.hosts_A_.end()) { + ips = &it->second; + } + } else { + auto it = parent_.hosts_AAAA_.find(name); + if (it != parent_.hosts_AAAA_.end()) { + ips = &it->second; + } } ares_free_string(name); // The response begins with the intial part of the request // (including the question section). - const size_t response_base_len = HFIXEDSZ + question_len + QFIXEDSZ; + const size_t response_base_len = HFIXEDSZ + name_len + QFIXEDSZ; unsigned char response_base[response_base_len]; memcpy(response_base, request, response_base_len); DNS_HEADER_SET_QR(response_base, 1); @@ -110,15 +127,26 @@ class TestDnsServerQuery { DNS_HEADER_SET_NSCOUNT(response_base, 0); DNS_HEADER_SET_ARCOUNT(response_base, 0); - // An A resource record for each IP found in the host map. - const size_t response_rest_len = - ips != nullptr ? ips->size() * (question_len + RRFIXEDSZ + sizeof(in_addr)) : 0; + // Create a resource record for each IP found in the host map. unsigned char response_rr_fixed[RRFIXEDSZ]; - DNS_RR_SET_TYPE(response_rr_fixed, T_A); + if (q_type == T_A) { + DNS_RR_SET_TYPE(response_rr_fixed, T_A); + DNS_RR_SET_LEN(response_rr_fixed, sizeof(in_addr)); + } else { + DNS_RR_SET_TYPE(response_rr_fixed, T_AAAA); + DNS_RR_SET_LEN(response_rr_fixed, sizeof(in6_addr)); + } DNS_RR_SET_CLASS(response_rr_fixed, C_IN); DNS_RR_SET_TTL(response_rr_fixed, 0); - DNS_RR_SET_LEN(response_rr_fixed, sizeof(in_addr)); + size_t response_rest_len; + if (q_type == T_A) { + response_rest_len = + ips != nullptr ? ips->size() * (name_len + RRFIXEDSZ + sizeof(in_addr)) : 0; + } else { + response_rest_len = + ips != nullptr ? ips->size() * (name_len + RRFIXEDSZ + sizeof(in6_addr)) : 0; + } // Send response to client. const uint16_t response_size_n = htons(response_base_len + response_rest_len); Buffer::OwnedImpl write_buffer_; @@ -126,11 +154,17 @@ class TestDnsServerQuery { write_buffer_.add(response_base, response_base_len); if (ips != nullptr) { for (auto it : *ips) { - write_buffer_.add(question, question_len); - write_buffer_.add(response_rr_fixed, 10); - in_addr addr; - ASSERT_EQ(1, inet_pton(AF_INET, it.c_str(), &addr)); - write_buffer_.add(&addr, sizeof(addr)); + write_buffer_.add(question, name_len); + write_buffer_.add(response_rr_fixed, RRFIXEDSZ); + if (q_type == T_A) { + in_addr addr; + ASSERT_EQ(1, inet_pton(AF_INET, it.c_str(), &addr)); + write_buffer_.add(&addr, sizeof(addr)); + } else { + in6_addr addr; + ASSERT_EQ(1, inet_pton(AF_INET6, it.c_str(), &addr)); + write_buffer_.add(&addr, sizeof(addr)); + } } } parent_.connection_->write(write_buffer_); @@ -152,20 +186,29 @@ class TestDnsServerQuery { private: ConnectionPtr connection_; - const HostMap& hosts_; + const HostMap& hosts_A_; + const HostMap& hosts_AAAA_; }; class TestDnsServer : public ListenerCallbacks { public: void onNewConnection(ConnectionPtr&& new_connection) override { - TestDnsServerQuery* query = new TestDnsServerQuery(std::move(new_connection), hosts_); + TestDnsServerQuery* query = + new TestDnsServerQuery(std::move(new_connection), hosts_A_, hosts_AAAA_); queries_.emplace_back(query); } - void addHosts(const std::string& hostname, const IpList& ip) { hosts_[hostname] = ip; } + void addHosts(const std::string& hostname, const IpList& ip, const record_type& type) { + if (type == A) { + hosts_A_[hostname] = ip; + } else if (type == AAAA) { + hosts_AAAA_[hostname] = ip; + } + } private: - HostMap hosts_; + HostMap hosts_A_; + HostMap hosts_AAAA_; // All queries are tracked so we can do resource reclamation when the test is // over. std::vector> queries_; @@ -197,7 +240,7 @@ class DnsResolverImplPeer { DnsResolverImpl* resolver_; }; -class DnsImplTest : public testing::Test { +class DnsImplTest : public testing::TestWithParam { public: void SetUp() override { resolver_ = dispatcher_.createDnsResolver(); @@ -205,7 +248,7 @@ class DnsImplTest : public testing::Test { // Instantiate TestDnsServer and listen on a random port on the loopback address. server_.reset(new TestDnsServer()); socket_.reset( - new Network::TcpListenSocket(Network::Utility::getCanonicalIpv4LoopbackAddress(), true)); + new Network::TcpListenSocket(Network::Test::getCanonicalLoopbackAddress(GetParam()), true)); listener_ = dispatcher_.createListener(connection_handler_, *socket_, *server_, stats_store_, {.bind_to_port_ = true, .use_proxy_proto_ = false, @@ -247,16 +290,21 @@ static bool hasAddress(const std::list& results return false; } +// Parameterize the DNS test server socket address. +INSTANTIATE_TEST_CASE_P(IpVersions, DnsImplTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + // Validate that when DnsResolverImpl is destructed with outstanding requests, // that we don't invoke any callbacks. This is a regression test from // development, where segfaults were encountered due to callback invocations on // destruction. -TEST_F(DnsImplTest, DestructPending) { - EXPECT_NE(nullptr, resolver_->resolve( - "", [&](std::list&& results) -> void { - FAIL(); - UNREFERENCED_PARAMETER(results); - })); +TEST_P(DnsImplTest, DestructPending) { + EXPECT_NE(nullptr, + resolver_->resolve("", DnsLookupFamily::V4Only, + [&](std::list&& results) -> void { + FAIL(); + UNREFERENCED_PARAMETER(results); + })); // Also validate that pending events are around to exercise the resource // reclamation path. EXPECT_GT(peer_->events().size(), 0U); @@ -265,30 +313,116 @@ TEST_F(DnsImplTest, DestructPending) { // Validate basic success/fail lookup behavior. The empty request will connect // to TestDnsServer, but localhost should resolve via the hosts file with no // asynchronous behavior or network events. -TEST_F(DnsImplTest, LocalLookup) { +TEST_P(DnsImplTest, LocalLookup) { std::list address_list; - EXPECT_NE(nullptr, resolver_->resolve( - "", [&](std::list&& results) -> void { - address_list = results; - dispatcher_.exit(); - })); + EXPECT_NE(nullptr, + resolver_->resolve("", DnsLookupFamily::V4Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); dispatcher_.run(Event::Dispatcher::RunType::Block); EXPECT_TRUE(address_list.empty()); - EXPECT_EQ(nullptr, resolver_->resolve("localhost", - [&](std::list&& results) - -> void { address_list = results; })); - EXPECT_TRUE(hasAddress(address_list, "127.0.0.1")); + if (GetParam() == Address::IpVersion::v4) { + EXPECT_EQ(nullptr, resolver_->resolve("localhost", DnsLookupFamily::V4Only, + [&](std::list&& results) + -> void { address_list = results; })); + EXPECT_TRUE(hasAddress(address_list, "127.0.0.1")); + EXPECT_FALSE(hasAddress(address_list, "::1")); + } + + if (GetParam() == Address::IpVersion::v6) { + EXPECT_EQ(nullptr, resolver_->resolve("localhost", DnsLookupFamily::Auto, + [&](std::list&& results) + -> void { address_list = results; })); + EXPECT_FALSE(hasAddress(address_list, "127.0.0.1")); + EXPECT_TRUE(hasAddress(address_list, "::1")); + + EXPECT_EQ(nullptr, resolver_->resolve("localhost", DnsLookupFamily::V6Only, + [&](std::list&& results) + -> void { address_list = results; })); + EXPECT_TRUE(hasAddress(address_list, "::1")); + EXPECT_FALSE(hasAddress(address_list, "127.0.0.1")); + } +} + +TEST_P(DnsImplTest, DnsIpAddressVersionV6) { + std::list address_list; + server_->addHosts("some.good.domain", {"1::2"}, AAAA); + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::Auto, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "1::2")); + + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::V4Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_FALSE(hasAddress(address_list, "1::2")); + + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::V6Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "1::2")); +} + +TEST_P(DnsImplTest, DnsIpAddressVersion) { + std::list address_list; + server_->addHosts("some.good.domain", {"1.2.3.4"}, A); + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::Auto, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "1.2.3.4")); + + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::V4Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "1.2.3.4")); + + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::V6Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_FALSE(hasAddress(address_list, "1.2.3.4")); } // Validate success/fail lookup behavior via TestDnsServer. This exercises the // network event handling in DnsResolverImpl. -TEST_F(DnsImplTest, RemoteAsyncLookup) { - server_->addHosts("some.good.domain", {"201.134.56.7"}); +TEST_P(DnsImplTest, RemoteAsyncLookup) { + server_->addHosts("some.good.domain", {"201.134.56.7"}, A); std::list address_list; EXPECT_NE(nullptr, - resolver_->resolve("some.bad.domain", + resolver_->resolve("some.bad.domain", DnsLookupFamily::Auto, [&](std::list&& results) -> void { address_list = results; dispatcher_.exit(); @@ -298,7 +432,7 @@ TEST_F(DnsImplTest, RemoteAsyncLookup) { EXPECT_TRUE(address_list.empty()); EXPECT_NE(nullptr, - resolver_->resolve("some.good.domain", + resolver_->resolve("some.good.domain", DnsLookupFamily::Auto, [&](std::list&& results) -> void { address_list = results; dispatcher_.exit(); @@ -309,11 +443,11 @@ TEST_F(DnsImplTest, RemoteAsyncLookup) { } // Validate that multiple A records are correctly passed to the callback. -TEST_F(DnsImplTest, MultiARecordLookup) { - server_->addHosts("some.good.domain", {"201.134.56.7", "123.4.5.6", "6.5.4.3"}); +TEST_P(DnsImplTest, MultiARecordLookup) { + server_->addHosts("some.good.domain", {"201.134.56.7", "123.4.5.6", "6.5.4.3"}, A); std::list address_list; EXPECT_NE(nullptr, - resolver_->resolve("some.good.domain", + resolver_->resolve("some.good.domain", DnsLookupFamily::V4Only, [&](std::list&& results) -> void { address_list = results; dispatcher_.exit(); @@ -325,16 +459,58 @@ TEST_F(DnsImplTest, MultiARecordLookup) { EXPECT_TRUE(hasAddress(address_list, "6.5.4.3")); } +TEST_P(DnsImplTest, MultiARecordLookupWithV6) { + server_->addHosts("some.good.domain", {"201.134.56.7", "123.4.5.6", "6.5.4.3"}, A); + server_->addHosts("some.good.domain", {"1::2", "1::2:3", "1::2:3:4"}, AAAA); + std::list address_list; + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::V4Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "201.134.56.7")); + EXPECT_TRUE(hasAddress(address_list, "123.4.5.6")); + EXPECT_TRUE(hasAddress(address_list, "6.5.4.3")); + + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::Auto, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "1::2")); + EXPECT_TRUE(hasAddress(address_list, "1::2:3")); + EXPECT_TRUE(hasAddress(address_list, "1::2:3:4")); + + EXPECT_NE(nullptr, + resolver_->resolve("some.good.domain", DnsLookupFamily::V6Only, + [&](std::list&& results) -> void { + address_list = results; + dispatcher_.exit(); + })); + + dispatcher_.run(Event::Dispatcher::RunType::Block); + EXPECT_TRUE(hasAddress(address_list, "1::2")); + EXPECT_TRUE(hasAddress(address_list, "1::2:3")); + EXPECT_TRUE(hasAddress(address_list, "1::2:3:4")); +} + // Validate working of cancellation provided by ActiveDnsQuery return. -TEST_F(DnsImplTest, Cancel) { - server_->addHosts("some.good.domain", {"201.134.56.7"}); +TEST_P(DnsImplTest, Cancel) { + server_->addHosts("some.good.domain", {"201.134.56.7"}, A); - ActiveDnsQuery* query = resolver_->resolve( - "some.domain", [](std::list && ) -> void { FAIL(); }); + ActiveDnsQuery* query = + resolver_->resolve("some.domain", DnsLookupFamily::Auto, + [](std::list && ) -> void { FAIL(); }); std::list address_list; EXPECT_NE(nullptr, - resolver_->resolve("some.good.domain", + resolver_->resolve("some.good.domain", DnsLookupFamily::Auto, [&](std::list&& results) -> void { address_list = results; dispatcher_.exit(); @@ -352,12 +528,16 @@ class DnsImplZeroTimeoutTest : public DnsImplTest { bool zero_timeout() const override { return true; } }; +// Parameterize the DNS test server socket address. +INSTANTIATE_TEST_CASE_P(IpVersions, DnsImplZeroTimeoutTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + // Validate that timeouts result in an empty callback. -TEST_F(DnsImplZeroTimeoutTest, Timeout) { - server_->addHosts("some.good.domain", {"201.134.56.7"}); +TEST_P(DnsImplZeroTimeoutTest, Timeout) { + server_->addHosts("some.good.domain", {"201.134.56.7"}, A); std::list address_list; EXPECT_NE(nullptr, - resolver_->resolve("some.good.domain", + resolver_->resolve("some.good.domain", DnsLookupFamily::V4Only, [&](std::list&& results) -> void { address_list = results; dispatcher_.exit(); diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 335eb864204e2..63dbd2f1d9274 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -669,8 +669,8 @@ TEST_F(ClusterManagerImplTest, DynamicHostRemove) { Network::DnsResolver::ResolveCb dns_callback; Event::MockTimer* dns_timer_ = new NiceMock(&factory_.dispatcher_); Network::MockActiveDnsQuery active_dns_query; - EXPECT_CALL(factory_.dns_resolver_, resolve(_, _)) - .WillRepeatedly(DoAll(SaveArg<1>(&dns_callback), Return(&active_dns_query))); + EXPECT_CALL(factory_.dns_resolver_, resolve(_, _, _)) + .WillRepeatedly(DoAll(SaveArg<2>(&dns_callback), Return(&active_dns_query))); create(*loader); // Test for no hosts returning the correct values before we have hosts. diff --git a/test/common/upstream/logical_dns_cluster_test.cc b/test/common/upstream/logical_dns_cluster_test.cc index 97ff34a999b5b..8d7f2798f80ad 100644 --- a/test/common/upstream/logical_dns_cluster_test.cc +++ b/test/common/upstream/logical_dns_cluster_test.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include #include "common/network/utility.h" @@ -36,13 +37,13 @@ class LogicalDnsClusterTest : public testing::Test { cluster_->setInitializedCb([&]() -> void { initialized_.ready(); }); } - void expectResolve() { - EXPECT_CALL(dns_resolver_, resolve("foo.bar.com", _)) - .WillOnce(Invoke([&](const std::string&, Network::DnsResolver::ResolveCb cb) - -> Network::ActiveDnsQuery* { - dns_callback_ = cb; - return &active_dns_query_; - })); + void expectResolve(Network::DnsLookupFamily dns_lookup_family) { + EXPECT_CALL(dns_resolver_, resolve("foo.bar.com", dns_lookup_family, _)) + .WillOnce(Invoke([&](const std::string&, Network::DnsLookupFamily, + Network::DnsResolver::ResolveCb cb) -> Network::ActiveDnsQuery* { + dns_callback_ = cb; + return &active_dns_query_; + })); } Stats::IsolatedStoreImpl stats_store_; @@ -59,42 +60,68 @@ class LogicalDnsClusterTest : public testing::Test { NiceMock dispatcher_; }; -TEST_F(LogicalDnsClusterTest, BadConfig) { - std::string json = R"EOF( +typedef std::tuple> + LogicalDnsConfigTuple; +std::vector generateLogicalDnsParams() { + std::vector dns_config; { - "name": "name", - "connect_timeout_ms": 250, - "type": "logical_dns", - "lb_type": "round_robin", - "hosts": [{"url": "tcp://foo.bar.com:443"}, {"url": "tcp://foo2.bar.com:443"}] + std::string family_json(""); + Network::DnsLookupFamily family(Network::DnsLookupFamily::V4Only); + std::list dns_response{"127.0.0.1", "127.0.0.2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); } - )EOF"; - - EXPECT_THROW(setup(json), EnvoyException); + { + std::string family_json(R"EOF("dns_lookup_family": "v4_only",)EOF"); + Network::DnsLookupFamily family(Network::DnsLookupFamily::V4Only); + std::list dns_response{"127.0.0.1", "127.0.0.2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + { + std::string family_json(R"EOF("dns_lookup_family": "v6_only",)EOF"); + Network::DnsLookupFamily family(Network::DnsLookupFamily::V6Only); + std::list dns_response{"::1", "::2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + { + std::string family_json(R"EOF("dns_lookup_family": "auto",)EOF"); + Network::DnsLookupFamily family(Network::DnsLookupFamily::Auto); + std::list dns_response{"::1"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + return dns_config; } +class LogicalDnsParamTest : public LogicalDnsClusterTest, + public testing::WithParamInterface {}; + +INSTANTIATE_TEST_CASE_P(DnsParam, LogicalDnsParamTest, + testing::ValuesIn(generateLogicalDnsParams())); + // Validate that if the DNS resolves immediately, during the LogicalDnsCluster // constructor, we have the expected host state and initialization callback // invocation. -TEST_F(LogicalDnsClusterTest, ImmediateResolve) { +TEST_P(LogicalDnsParamTest, ImmediateResolve) { std::string json = R"EOF( { "name": "name", "connect_timeout_ms": 250, "type": "logical_dns", "lb_type": "round_robin", + )EOF"; + json += std::get<0>(GetParam()); + json += R"EOF( "hosts": [{"url": "tcp://foo.bar.com:443"}] } )EOF"; EXPECT_CALL(initialized_, ready()); - EXPECT_CALL(dns_resolver_, resolve("foo.bar.com", _)) - .WillOnce(Invoke([&](const std::string&, Network::DnsResolver::ResolveCb cb) - -> Network::ActiveDnsQuery* { - EXPECT_CALL(*resolve_timer_, enableTimer(_)); - cb(TestUtility::makeDnsResponse({"127.0.0.1", "127.0.0.2"})); - return nullptr; - })); + EXPECT_CALL(dns_resolver_, resolve("foo.bar.com", std::get<1>(GetParam()), _)) + .WillOnce(Invoke([&](const std::string&, Network::DnsLookupFamily, + Network::DnsResolver::ResolveCb cb) -> Network::ActiveDnsQuery* { + EXPECT_CALL(*resolve_timer_, enableTimer(_)); + cb(TestUtility::makeDnsResponse(std::get<2>(GetParam()))); + return nullptr; + })); setup(json); EXPECT_EQ(1UL, cluster_->hosts().size()); EXPECT_EQ(1UL, cluster_->healthyHosts().size()); @@ -102,6 +129,20 @@ TEST_F(LogicalDnsClusterTest, ImmediateResolve) { tls_.shutdownThread(); } +TEST_F(LogicalDnsClusterTest, BadConfig) { + std::string json = R"EOF( + { + "name": "name", + "connect_timeout_ms": 250, + "type": "logical_dns", + "lb_type": "round_robin", + "hosts": [{"url": "tcp://foo.bar.com:443"}, {"url": "tcp://foo2.bar.com:443"}] + } + )EOF"; + + EXPECT_THROW(setup(json), EnvoyException); +} + TEST_F(LogicalDnsClusterTest, Basic) { std::string json = R"EOF( { @@ -114,7 +155,7 @@ TEST_F(LogicalDnsClusterTest, Basic) { } )EOF"; - expectResolve(); + expectResolve(Network::DnsLookupFamily::V4Only); setup(json); EXPECT_CALL(membership_updated_, ready()); @@ -135,7 +176,7 @@ TEST_F(LogicalDnsClusterTest, Basic) { logical_host->createConnection(dispatcher_); logical_host->outlierDetector().putHttpResponseCode(200); - expectResolve(); + expectResolve(Network::DnsLookupFamily::V4Only); resolve_timer_->callback_(); // Should not cause any changes. @@ -155,7 +196,7 @@ TEST_F(LogicalDnsClusterTest, Basic) { EXPECT_EQ("foo.bar.com", data.host_description_->hostname()); data.host_description_->outlierDetector().putHttpResponseCode(200); - expectResolve(); + expectResolve(Network::DnsLookupFamily::V4Only); resolve_timer_->callback_(); // Should cause a change. @@ -168,7 +209,7 @@ TEST_F(LogicalDnsClusterTest, Basic) { .WillOnce(Return(new NiceMock())); logical_host->createConnection(dispatcher_); - expectResolve(); + expectResolve(Network::DnsLookupFamily::V4Only); resolve_timer_->callback_(); // Empty should not cause any change. @@ -183,7 +224,7 @@ TEST_F(LogicalDnsClusterTest, Basic) { // Make sure we cancel. EXPECT_CALL(active_dns_query_, cancel()); - expectResolve(); + expectResolve(Network::DnsLookupFamily::V4Only); resolve_timer_->callback_(); tls_.shutdownThread(); diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 8f4101751823b..0e5e42afab35c 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include "envoy/api/api.h" @@ -47,12 +48,12 @@ struct ResolverData { } void expectResolve(Network::MockDnsResolver& dns_resolver) { - EXPECT_CALL(dns_resolver, resolve(_, _)) - .WillOnce(Invoke([&](const std::string&, Network::DnsResolver::ResolveCb cb) - -> Network::ActiveDnsQuery* { - dns_callback_ = cb; - return &active_dns_query_; - })) + EXPECT_CALL(dns_resolver, resolve(_, _, _)) + .WillOnce(Invoke([&](const std::string&, Network::DnsLookupFamily, + Network::DnsResolver::ResolveCb cb) -> Network::ActiveDnsQuery* { + dns_callback_ = cb; + return &active_dns_query_; + })) .RetiresOnSaturation(); } @@ -61,7 +62,42 @@ struct ResolverData { Network::MockActiveDnsQuery active_dns_query_; }; -TEST(StrictDnsClusterImplTest, ImmediateResolve) { +typedef std::tuple> + StrictDnsConfigTuple; +std::vector generateStrictDnsParams() { + std::vector dns_config; + { + std::string family_json(""); + Network::DnsLookupFamily family(Network::DnsLookupFamily::V4Only); + std::list dns_response{"127.0.0.1", "127.0.0.2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + { + std::string family_json(R"EOF("dns_lookup_family": "v4_only",)EOF"); + Network::DnsLookupFamily family(Network::DnsLookupFamily::V4Only); + std::list dns_response{"127.0.0.1", "127.0.0.2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + { + std::string family_json(R"EOF("dns_lookup_family": "v6_only",)EOF"); + Network::DnsLookupFamily family(Network::DnsLookupFamily::V6Only); + std::list dns_response{"::1", "::2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + { + std::string family_json(R"EOF("dns_lookup_family": "auto",)EOF"); + Network::DnsLookupFamily family(Network::DnsLookupFamily::Auto); + std::list dns_response{"127.0.0.1", "127.0.0.2"}; + dns_config.push_back(std::make_tuple(family_json, family, dns_response)); + } + return dns_config; +} + +class StrictDnsParamTest : public testing::TestWithParam {}; + +INSTANTIATE_TEST_CASE_P(DnsParam, StrictDnsParamTest, testing::ValuesIn(generateStrictDnsParams())); + +TEST_P(StrictDnsParamTest, ImmediateResolve) { Stats::IsolatedStoreImpl stats; Ssl::MockContextManager ssl_context_manager; NiceMock dns_resolver; @@ -74,18 +110,20 @@ TEST(StrictDnsClusterImplTest, ImmediateResolve) { "name": "name", "connect_timeout_ms": 250, "type": "strict_dns", + )EOF"; + json += std::get<0>(GetParam()); + json += R"EOF( "lb_type": "round_robin", "hosts": [{"url": "tcp://foo.bar.com:443"}] } )EOF"; - EXPECT_CALL(initialized, ready()); - EXPECT_CALL(dns_resolver, resolve("foo.bar.com", _)) - .WillOnce(Invoke([&](const std::string&, Network::DnsResolver::ResolveCb cb) - -> Network::ActiveDnsQuery* { - cb(TestUtility::makeDnsResponse({"127.0.0.1", "127.0.0.2"})); - return nullptr; - })); + EXPECT_CALL(dns_resolver, resolve("foo.bar.com", std::get<1>(GetParam()), _)) + .WillOnce(Invoke([&](const std::string&, Network::DnsLookupFamily, + Network::DnsResolver::ResolveCb cb) -> Network::ActiveDnsQuery* { + cb(TestUtility::makeDnsResponse(std::get<2>(GetParam()))); + return nullptr; + })); Json::ObjectSharedPtr loader = Json::Factory::loadFromString(json); StrictDnsClusterImpl cluster(*loader, runtime, stats, ssl_context_manager, dns_resolver, dispatcher); @@ -473,5 +511,21 @@ TEST(ClusterDefinitionTest, BadClusterConfig) { EXPECT_THROW(loader->validateSchema(Json::Schema::CLUSTER_SCHEMA), Json::Exception); } +TEST(ClusterDefinitionTest, BadDnsClusterConfig) { + std::string json = R"EOF( + { + "name": "cluster_1", + "connect_timeout_ms": 250, + "type": "static", + "lb_type": "round_robin", + "hosts": [{"url": "tcp://127.0.0.1:11001"}], + "dns_lookup_family" : "foo" + } + )EOF"; + + Json::ObjectSharedPtr loader = Json::Factory::loadFromString(json); + EXPECT_THROW(loader->validateSchema(Json::Schema::CLUSTER_SCHEMA), Json::Exception); +} + } // Upstream } // Envoy diff --git a/test/mocks/network/mocks.cc b/test/mocks/network/mocks.cc index 119db644ee862..5bca23db36b65 100644 --- a/test/mocks/network/mocks.cc +++ b/test/mocks/network/mocks.cc @@ -76,7 +76,7 @@ MockActiveDnsQuery::MockActiveDnsQuery() {} MockActiveDnsQuery::~MockActiveDnsQuery() {} MockDnsResolver::MockDnsResolver() { - ON_CALL(*this, resolve(_, _)).WillByDefault(Return(&active_query_)); + ON_CALL(*this, resolve(_, _, _)).WillByDefault(Return(&active_query_)); } MockDnsResolver::~MockDnsResolver() {} diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 170b09896e968..fe78c71474f26 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -117,7 +117,8 @@ class MockDnsResolver : public DnsResolver { ~MockDnsResolver(); // Network::DnsResolver - MOCK_METHOD2(resolve, ActiveDnsQuery*(const std::string& dns_name, ResolveCb callback)); + MOCK_METHOD3(resolve, ActiveDnsQuery*(const std::string& dns_name, + DnsLookupFamily dns_lookup_family, ResolveCb callback)); testing::NiceMock active_query_; }; diff --git a/test/test_common/BUILD b/test/test_common/BUILD index c28731ba1ba23..f66fb67d466b8 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -63,5 +63,6 @@ envoy_cc_test_library( "//source/common/common:empty_string", "//source/common/http:header_map_lib", "//source/common/network:address_lib", + "//source/common/network:utility_lib", ], ) diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index f8d2b4934f909..013ab2b90d8fe 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -14,6 +14,7 @@ #include "common/common/empty_string.h" #include "common/network/address_impl.h" +#include "common/network/utility.h" #include "test/test_common/printers.h" @@ -65,7 +66,7 @@ std::list TestUtility::makeDnsResponse(const std::list& addresses) { std::list ret; for (auto address : addresses) { - ret.emplace_back(new Network::Address::Ipv4Instance(address)); + ret.emplace_back(Network::Utility::parseInternetAddress(address)); } return ret; }