apple dns: resolve IP addresses without calling Apple APIs

source/common/network/apple_dns_impl.cc

@@ -113,22 +113,55 @@ ActiveDnsQuery* AppleDnsResolverImpl::resolve(const std::string& dns_name,
                                               DnsLookupFamily dns_lookup_family,
                                               ResolveCb callback) {
   ENVOY_LOG(debug, "DNS resolver resolve={}", dns_name);
-  std::unique_ptr<PendingResolution> pending_resolution(
-      new PendingResolution(*this, callback, dispatcher_, main_sd_ref_, dns_name));
 
-  DNSServiceErrorType error = pending_resolution->dnsServiceGetAddrInfo(dns_lookup_family);
-  if (error != kDNSServiceErr_NoError) {
-    ENVOY_LOG(warn, "DNS resolver error ({}) in dnsServiceGetAddrInfo for {}", error, dns_name);
-    return nullptr;
+  Address::InstanceConstSharedPtr address{};
+  try {
+    // After production testing with this resolver implementation it became empirically demonstrable
+    // that Apple's API issues queries to the DNS server for dns_name that might already be an IP
+    // address. In contrast, c-ares synchronously resolves such cases. Moreover, some DNS servers
+    // might not resolve IP addresses and thus result in callback targets that never get a
+    // resolution. Therefore, we short circuit here by trying to parse the dns_name into an internet
+    // address and synchronously issue the ResolveCb; only if the parsing throws an exception the
+    // resolver issues a call to Apple's API.
+    address = Utility::parseInternetAddress(dns_name);
+    ENVOY_LOG(debug, "DNS resolver resolved ({}) to ({}) without issuing call to Apple API",
+              dns_name, address->asString());
+  } catch (const EnvoyException& e) {
+    // Resolution via Apple APIs
+    ENVOY_LOG(debug, "DNS resolver local resolution failed with: {}", e.what());
+    std::unique_ptr<PendingResolution> pending_resolution(
+        new PendingResolution(*this, callback, dispatcher_, main_sd_ref_, dns_name));
+
+    DNSServiceErrorType error = pending_resolution->dnsServiceGetAddrInfo(dns_lookup_family);
+    if (error != kDNSServiceErr_NoError) {
+      ENVOY_LOG(warn, "DNS resolver error ({}) in dnsServiceGetAddrInfo for {}", error, dns_name);
+      return nullptr;
+    }
+
+    // If the query was synchronously resolved in the Apple API call, there is no need to return the
+    // query.
+    if (pending_resolution->synchronously_completed_) {
+      return nullptr;
+    }
+
+    pending_resolution->owned_ = true;
+    return pending_resolution.release();
   }
 
-  // If the query was synchronously resolved, there is no need to return the query.
-  if (pending_resolution->synchronously_completed_) {
+  ASSERT(address != nullptr);
+  // Finish local, synchronous resolution. This needs to happen outside of the exception block above
+  // as the callback itself can throw.
+  try {
+    callback(DnsResolver::ResolutionStatus::Success,
+             {DnsResponse(address, std::chrono::seconds(60))});
     return nullptr;
+  } catch (const std::exception& e) {
+    ENVOY_LOG(warn, "std::exception in callback with local resolution: {}", e.what());
+    throw EnvoyException(e.what());
+  } catch (...) {
+    ENVOY_LOG(warn, "Unknown exception in callback with local resolution");
+    throw EnvoyException("unknown");
   }
-
-  pending_resolution->owned_ = true;
-  return pending_resolution.release();
 }
 
 void AppleDnsResolverImpl::addPendingQuery(PendingResolution* query) {

source/common/network/utility.h

@@ -72,6 +72,7 @@ class Utility {
    * Resolve a URL.
    * @param url supplies the url to resolve.
    * @return Address::InstanceConstSharedPtr the resolved address.
+   * @throw EnvoyException if url is invalid.
    */
   static Address::InstanceConstSharedPtr resolveUrl(const std::string& url);
 

test/common/network/apple_dns_impl_test.cc

@@ -154,26 +154,45 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersion) {
 }
 
 TEST_F(AppleDnsImplTest, CallbackException) {
-  EXPECT_NE(nullptr, resolveWithException<EnvoyException>("1.2.3.4", DnsLookupFamily::V4Only,
+  EXPECT_NE(nullptr, resolveWithException<EnvoyException>("google.com", DnsLookupFamily::V4Only,
                                                           EnvoyException("Envoy exception")));
   EXPECT_THROW_WITH_MESSAGE(dispatcher_->run(Event::Dispatcher::RunType::Block), EnvoyException,
                             "Envoy exception");
 }
 
 TEST_F(AppleDnsImplTest, CallbackException2) {
-  EXPECT_NE(nullptr, resolveWithException<std::runtime_error>("1.2.3.4", DnsLookupFamily::V4Only,
+  EXPECT_NE(nullptr, resolveWithException<std::runtime_error>("google.com", DnsLookupFamily::V4Only,
                                                               std::runtime_error("runtime error")));
   EXPECT_THROW_WITH_MESSAGE(dispatcher_->run(Event::Dispatcher::RunType::Block), EnvoyException,
                             "runtime error");
 }
 
 TEST_F(AppleDnsImplTest, CallbackException3) {
-  EXPECT_NE(nullptr,
-            resolveWithException<std::string>("1.2.3.4", DnsLookupFamily::V4Only, std::string()));
+  EXPECT_NE(nullptr, resolveWithException<std::string>("google.com", DnsLookupFamily::V4Only,
+                                                       std::string()));
   EXPECT_THROW_WITH_MESSAGE(dispatcher_->run(Event::Dispatcher::RunType::Block), EnvoyException,
                             "unknown");
 }
 
+TEST_F(AppleDnsImplTest, CallbackExceptionLocalResolution) {
+  EXPECT_THROW_WITH_MESSAGE(resolveWithException<EnvoyException>("1.2.3.4", DnsLookupFamily::V4Only,
+                                                                 EnvoyException("Envoy exception")),
+                            EnvoyException, "Envoy exception");
+}
+
+TEST_F(AppleDnsImplTest, CallbackExceptionLocalResolution2) {
+  EXPECT_THROW_WITH_MESSAGE(
+      resolveWithException<std::runtime_error>("1.2.3.4", DnsLookupFamily::V4Only,
+                                               std::runtime_error("runtime error")),
+      EnvoyException, "runtime error");
+}
+
+TEST_F(AppleDnsImplTest, CallbackExceptionLocalResolution3) {
+  EXPECT_THROW_WITH_MESSAGE(
+      resolveWithException<std::string>("1.2.3.4", DnsLookupFamily::V4Only, std::string()),
+      EnvoyException, "unknown");
+}
+
 // Validate working of cancellation provided by ActiveDnsQuery return.
 TEST_F(AppleDnsImplTest, Cancel) {
   ActiveDnsQuery* query =
@@ -194,6 +213,19 @@ TEST_F(AppleDnsImplTest, Timeout) {
   dispatcher_->run(Event::Dispatcher::RunType::Block);
 }
 
+TEST_F(AppleDnsImplTest, LocalResolution) {
+  auto pending_resolution = resolver_->resolve(
+      "0.0.0.0", DnsLookupFamily::Auto,
+      [](DnsResolver::ResolutionStatus status, std::list<DnsResponse>&& results) -> void {
+        EXPECT_EQ(DnsResolver::ResolutionStatus::Success, status);
+        EXPECT_EQ(1, results.size());
+        EXPECT_EQ("0.0.0.0:0", results.front().address_->asString());
+        EXPECT_EQ(std::chrono::seconds(60), results.front().ttl_);
+      });
+  EXPECT_EQ(nullptr, pending_resolution);
+  // Note that the dispatcher does NOT have to run because resolution is synchronous.
+}
+
 // This class compliments the tests above by using a mocked Apple API that allows finer control over
 // error conditions, and callback firing.
 class AppleDnsImplFakeApiTest : public testing::Test {