diff --git a/api/envoy/extensions/network/dns_resolver/apple/v3/apple_dns_resolver.proto b/api/envoy/extensions/network/dns_resolver/apple/v3/apple_dns_resolver.proto index c27d1c78276a3..60588ca848a21 100644 --- a/api/envoy/extensions/network/dns_resolver/apple/v3/apple_dns_resolver.proto +++ b/api/envoy/extensions/network/dns_resolver/apple/v3/apple_dns_resolver.proto @@ -15,4 +15,12 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Configuration for apple DNS resolver. message AppleDnsResolverConfig { + // The resolver will avoid the system's heuristics to only return + // IPv4 or IPv6 addresses that it considers to be "routable", instead + // returning all possible IPv4 or IPv6 addresses. This setting is + // ignored if the DNS lookup family is set to v4-only or v6-only. + // This should remain false in the vast majority of cases, but may be + // useful when performing custom filtering of addresses, such as with + // Happy Eyeballs. + bool include_unroutable_families = 1; } diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index c809a807f674f..fff25b62f318e 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -23,6 +23,7 @@ Removed Config or Runtime New Features ------------ +* dns_resolver: added :ref:`include_unroutable_families` to the Apple DNS resolver. * ext_proc: added support for per-route :ref:`grpc_service `. Deprecated diff --git a/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc b/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc index 60476112f0eb9..35e84c186a014 100644 --- a/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc +++ b/source/extensions/network/dns_resolver/apple/apple_dns_impl.cc @@ -46,9 +46,12 @@ DNSServiceErrorType DnsService::dnsServiceGetAddrInfo(DNSServiceRef* sdRef, DNSS return DNSServiceGetAddrInfo(sdRef, flags, interfaceIndex, protocol, hostname, callBack, context); } -AppleDnsResolverImpl::AppleDnsResolverImpl(Event::Dispatcher& dispatcher, Stats::Scope& root_scope) +AppleDnsResolverImpl::AppleDnsResolverImpl( + const envoy::extensions::network::dns_resolver::apple::v3::AppleDnsResolverConfig& proto_config, + Event::Dispatcher& dispatcher, Stats::Scope& root_scope) : dispatcher_(dispatcher), scope_(root_scope.createScope("dns.apple.")), - stats_(generateAppleDnsResolverStats(*scope_)) {} + stats_(generateAppleDnsResolverStats(*scope_)), + include_unroutable_families_(proto_config.include_unroutable_families()) {} AppleDnsResolverStats AppleDnsResolverImpl::generateAppleDnsResolverStats(Stats::Scope& scope) { return {ALL_APPLE_DNS_RESOLVER_STATS(POOL_COUNTER(scope))}; @@ -78,7 +81,8 @@ AppleDnsResolverImpl::startResolution(const std::string& dns_name, auto pending_resolution = std::make_unique(*this, callback, dispatcher_, dns_name, dns_lookup_family); - DNSServiceErrorType error = pending_resolution->dnsServiceGetAddrInfo(); + DNSServiceErrorType error = + pending_resolution->dnsServiceGetAddrInfo(include_unroutable_families_); if (error != kDNSServiceErr_NoError) { ENVOY_LOG(warn, "DNS resolver error ({}) in dnsServiceGetAddrInfo for {}", error, dns_name); chargeGetAddrInfoErrorStats(error); @@ -238,7 +242,8 @@ void AppleDnsResolverImpl::PendingResolution::finishResolve() { } } -DNSServiceErrorType AppleDnsResolverImpl::PendingResolution::dnsServiceGetAddrInfo() { +DNSServiceErrorType +AppleDnsResolverImpl::PendingResolution::dnsServiceGetAddrInfo(bool include_unroutable_families) { DNSServiceProtocol protocol; switch (dns_lookup_family_) { case DnsLookupFamily::V4Only: @@ -250,6 +255,11 @@ DNSServiceErrorType AppleDnsResolverImpl::PendingResolution::dnsServiceGetAddrIn case DnsLookupFamily::Auto: case DnsLookupFamily::V4Preferred: case DnsLookupFamily::All: + if (include_unroutable_families) { + protocol = kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6; + break; + } + /* We want to make sure we don't get any address that is not routable. Passing 0 * to apple's `DNSServiceGetAddrInfo` will make a best attempt to filter out IPv6 * or IPv4 addresses depending on what's routable, per Apple's documentation: @@ -280,7 +290,7 @@ DNSServiceErrorType AppleDnsResolverImpl::PendingResolution::dnsServiceGetAddrIn * that DNSServiceRef. */ - // Therefore, much like the c-ares implementation All calls and callbacks to the API need to + // Therefore, much like the c-ares implementation, all calls and callbacks to the API need to // happen on the thread that owns the creating dispatcher. This is the case as callbacks are // driven by processing bytes in onEventCallback which run on the passed in dispatcher's event // loop. @@ -393,11 +403,14 @@ class AppleDnsResolverFactory : public DnsResolverFactory { return ProtobufTypes::MessagePtr{ new envoy::extensions::network::dns_resolver::apple::v3::AppleDnsResolverConfig()}; } - DnsResolverSharedPtr - createDnsResolver(Event::Dispatcher& dispatcher, Api::Api& api, - const envoy::config::core::v3::TypedExtensionConfig&) const override { + + DnsResolverSharedPtr createDnsResolver(Event::Dispatcher& dispatcher, Api::Api& api, + const envoy::config::core::v3::TypedExtensionConfig& + typed_dns_resolver_config) const override { ASSERT(dispatcher.isThreadSafe()); - return std::make_shared(dispatcher, api.rootScope()); + envoy::extensions::network::dns_resolver::apple::v3::AppleDnsResolverConfig apple; + Envoy::MessageUtil::unpackTo(typed_dns_resolver_config.typed_config(), apple); + return std::make_shared(apple, dispatcher, api.rootScope()); } }; diff --git a/source/extensions/network/dns_resolver/apple/apple_dns_impl.h b/source/extensions/network/dns_resolver/apple/apple_dns_impl.h index 95a47de8c0da9..a9ab53847effe 100644 --- a/source/extensions/network/dns_resolver/apple/apple_dns_impl.h +++ b/source/extensions/network/dns_resolver/apple/apple_dns_impl.h @@ -16,6 +16,7 @@ #include "source/common/common/linked_object.h" #include "source/common/common/logger.h" #include "source/common/common/utility.h" +#include "source/common/network/dns_resolver/dns_factory_util.h" #include "source/common/singleton/threadsafe_singleton.h" #include "absl/container/node_hash_map.h" @@ -64,7 +65,10 @@ struct AppleDnsResolverStats { */ class AppleDnsResolverImpl : public DnsResolver, protected Logger::Loggable { public: - AppleDnsResolverImpl(Event::Dispatcher& dispatcher, Stats::Scope& root_scope); + AppleDnsResolverImpl( + const envoy::extensions::network::dns_resolver::apple::v3::AppleDnsResolverConfig& + proto_config, + Event::Dispatcher& dispatcher, Stats::Scope& root_scope); static AppleDnsResolverStats generateAppleDnsResolverStats(Stats::Scope& scope); @@ -99,7 +103,7 @@ class AppleDnsResolverImpl : public DnsResolver, protected Logger::Loggable(dispatcher_, stats_store_); + config_.set_include_unroutable_families(false); + resolver_ = std::make_unique(config_, dispatcher_, stats_store_); } void checkErrorStat(DNSServiceErrorType error_code) { @@ -559,11 +560,22 @@ class AppleDnsImplFakeApiTest : public testing::Test { TestThreadsafeSingletonInjector dns_service_injector_{&dns_service_}; Stats::IsolatedStoreImpl stats_store_; std::unique_ptr resolver_{}; + envoy::extensions::network::dns_resolver::apple::v3::AppleDnsResolverConfig config_; NiceMock dispatcher_; NiceMock* file_event_; Event::FileReadyCb file_ready_cb_; }; +TEST_F(AppleDnsImplFakeApiTest, IncludeUnroutableFamiliesConfigFalse) { + config_.set_include_unroutable_families(false); + EXPECT_EQ(false, config_.include_unroutable_families()); +} + +TEST_F(AppleDnsImplFakeApiTest, IncludeUnroutableFamiliesConfigTrue) { + config_.set_include_unroutable_families(true); + EXPECT_EQ(true, config_.include_unroutable_families()); +} + TEST_F(AppleDnsImplFakeApiTest, ErrorInSocketAccess) { const std::string hostname = "foo.com"; sockaddr_in addr4; @@ -691,6 +703,47 @@ TEST_F(AppleDnsImplFakeApiTest, SynchronousTimeoutInGetAddrInfo) { synchronousWithError(kDNSServiceErr_Timeout); } +TEST_F(AppleDnsImplFakeApiTest, QuerySynchronousCompletionUnroutableFamilies) { + config_.set_include_unroutable_families(true); + resolver_ = std::make_unique(config_, dispatcher_, stats_store_); + + const std::string hostname = "foo.com"; + sockaddr_in addr4; + memset(&addr4, 0, sizeof(addr4)); + addr4.sin_family = AF_INET; + EXPECT_EQ(1, inet_pton(AF_INET, "1.2.3.4", &addr4.sin_addr)); + addr4.sin_port = htons(6502); + + Network::Address::Ipv4Instance address(&addr4); + absl::Notification dns_callback_executed; + + EXPECT_CALL(dns_service_, + dnsServiceGetAddrInfo(_, kDNSServiceFlagsTimeout, 0, + kDNSServiceProtocol_IPv4 | kDNSServiceProtocol_IPv6, + StrEq(hostname.c_str()), _, _)) + .WillOnce(DoAll( + // Have the API call synchronously call the provided callback. + WithArgs<5, 6>(Invoke([&](DNSServiceGetAddrInfoReply callback, void* context) -> void { + callback(nullptr, kDNSServiceFlagsAdd, 0, kDNSServiceErr_NoError, hostname.c_str(), + address.sockAddr(), 30, context); + })), + Return(kDNSServiceErr_NoError))); + + // The returned value is nullptr because the query has already been fulfilled. Verify that the + // callback ran via notification. + EXPECT_EQ(nullptr, resolver_->resolve( + hostname, Network::DnsLookupFamily::Auto, + [&dns_callback_executed](DnsResolver::ResolutionStatus status, + std::list&& response) -> void { + EXPECT_EQ(DnsResolver::ResolutionStatus::Success, status); + EXPECT_EQ(1, response.size()); + EXPECT_EQ("1.2.3.4:0", response.front().addrInfo().address_->asString()); + EXPECT_EQ(std::chrono::seconds(30), response.front().addrInfo().ttl_); + dns_callback_executed.Notify(); + })); + dns_callback_executed.WaitForNotification(); +} + TEST_F(AppleDnsImplFakeApiTest, QuerySynchronousCompletion) { const std::string hostname = "foo.com"; sockaddr_in addr4;