mobile: Update EngineBuilder's addDnsPreresolveHostnames to add a port (

envoyproxy#30829)

Also, we add a test to verify the addDnsPreresolveHostnames behavior.

Signed-off-by: Ali Beyad <abeyad@google.com>

mobile/library/cc/engine_builder.cc

@@ -208,7 +208,12 @@ EngineBuilder& EngineBuilder::addDnsQueryTimeoutSeconds(int dns_query_timeout_se
 }
 
 EngineBuilder& EngineBuilder::addDnsPreresolveHostnames(const std::vector<std::string>& hostnames) {
-  dns_preresolve_hostnames_ = hostnames;
+  // Add a default port of 443 for all hosts. We'll eventually change this API so it takes a single
+  // {host, pair} and it can be called multiple times.
+  dns_preresolve_hostnames_.clear();
+  for (const std::string& hostname : hostnames) {
+    dns_preresolve_hostnames_.push_back({hostname /* host */, 443 /* port */});
+  }
   return *this;
 }
 
@@ -623,10 +628,10 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
   dns_cache_config->mutable_typed_dns_resolver_config()->mutable_typed_config()->PackFrom(
       resolver_config);
 
-  for (auto& hostname : dns_preresolve_hostnames_) {
+  for (const auto& [host, port] : dns_preresolve_hostnames_) {
     envoy::config::core::v3::SocketAddress* address = dns_cache_config->add_preresolve_hostnames();
-    address->set_address(hostname);
-    address->set_port_value(443);
+    address->set_address(host);
+    address->set_port_value(port);
   }
 
   auto* dfp_filter = hcm->add_http_filters();

mobile/library/cc/engine_builder.h

@@ -179,6 +179,11 @@ class EngineBuilder {
 #endif
   EngineBuilder& enableDnsCache(bool dns_cache_on, int save_interval_seconds = 1);
   EngineBuilder& setForceAlwaysUsev6(bool value);
+  // Adds the hostnames that should be pre-resolved by DNS prior to the first request issued for
+  // that host. When invoked, any previous preresolve hostname entries get cleared and only the ones
+  // provided in the hostnames argument get set.
+  // TODO(abeyad): change this method and the other language APIs to take a {host,port} pair.
+  // E.g. addDnsPreresolveHost(std::string host, uint32_t port);
   EngineBuilder& addDnsPreresolveHostnames(const std::vector<std::string>& hostnames);
   EngineBuilder& addNativeFilter(std::string name, std::string typed_config);
 
@@ -253,7 +258,7 @@ class EngineBuilder {
   std::vector<std::string> stats_sinks_;
 
   std::vector<NativeFilterConfig> native_filter_chain_;
-  std::vector<std::string> dns_preresolve_hostnames_;
+  std::vector<std::pair<std::string /* host */, uint32_t /* port */>> dns_preresolve_hostnames_;
 
   std::vector<std::pair<std::string, bool>> runtime_guards_;
   absl::flat_hash_map<std::string, StringAccessorSharedPtr> string_accessors_;

mobile/test/cc/unit/BUILD

@@ -15,6 +15,7 @@ envoy_cc_test(
     deps = [
         "//library/cc:engine_builder_lib",
         "//library/cc:envoy_engine_cc_lib_no_stamp",
+        "@envoy_api//envoy/extensions/clusters/dynamic_forward_proxy/v3:pkg_cc_proto",
         "@envoy_build_config//:extension_registry",
         "@envoy_build_config//:test_extensions",
     ],

mobile/test/cc/unit/envoy_config_test.cc

@@ -1,8 +1,11 @@
 #include <string>
 #include <vector>
 
+#include "envoy/extensions/clusters/dynamic_forward_proxy/v3/cluster.pb.h"
+
 #include "test/test_common/utility.h"
 
+#include "absl/strings/str_cat.h"
 #include "absl/strings/str_replace.h"
 #include "absl/synchronization/notification.h"
 #include "gtest/gtest.h"
@@ -15,16 +18,28 @@
 #include "source/extensions/network/dns_resolver/apple/apple_dns_impl.h"
 #endif
 
+namespace Envoy {
+namespace {
+
+using namespace Platform;
+
 using envoy::config::bootstrap::v3::Bootstrap;
+using DfpClusterConfig = ::envoy::extensions::clusters::dynamic_forward_proxy::v3::ClusterConfig;
 using testing::HasSubstr;
 using testing::IsEmpty;
 using testing::Not;
 using testing::SizeIs;
 
-namespace Envoy {
-namespace {
-
-using namespace Platform;
+DfpClusterConfig getDfpClusterConfig(const Bootstrap& bootstrap) {
+  DfpClusterConfig cluster_config;
+  const auto& clusters = bootstrap.static_resources().clusters();
+  for (const auto& cluster : clusters) {
+    if (cluster.name() == "base") {
+      MessageUtil::unpackTo(cluster.cluster_type().typed_config(), cluster_config);
+    }
+  }
+  return cluster_config;
+}
 
 TEST(TestConfig, ConfigIsApplied) {
   EngineBuilder engine_builder;
@@ -232,6 +247,35 @@ TEST(TestConfig, AddMaxConnectionsPerHost) {
   EXPECT_THAT(bootstrap->ShortDebugString(), HasSubstr("max_connections { value: 16 }"));
 }
 
+TEST(TestConfig, AddDnsPreresolveHostnames) {
+  EngineBuilder engine_builder;
+  engine_builder.addDnsPreresolveHostnames({"google.com", "lyft.com"});
+  std::unique_ptr<Bootstrap> bootstrap = engine_builder.generateBootstrap();
+
+  Protobuf::RepeatedPtrField<envoy::config::core::v3::SocketAddress>
+      expected_dns_preresolve_hostnames;
+  auto& host_addr1 = *expected_dns_preresolve_hostnames.Add();
+  host_addr1.set_address("google.com");
+  host_addr1.set_port_value(443);
+  auto& host_addr2 = *expected_dns_preresolve_hostnames.Add();
+  host_addr2.set_address("lyft.com");
+  host_addr2.set_port_value(443);
+  EXPECT_TRUE(TestUtility::repeatedPtrFieldEqual(
+      getDfpClusterConfig(*bootstrap).dns_cache_config().preresolve_hostnames(),
+      expected_dns_preresolve_hostnames));
+
+  // Resetting the DNS preresolve hostnames with just "google.com" now.
+  engine_builder.addDnsPreresolveHostnames({"google.com"});
+  bootstrap = engine_builder.generateBootstrap();
+  expected_dns_preresolve_hostnames.Clear();
+  auto& host_addr3 = *expected_dns_preresolve_hostnames.Add();
+  host_addr3.set_address("google.com");
+  host_addr3.set_port_value(443);
+  EXPECT_TRUE(TestUtility::repeatedPtrFieldEqual(
+      getDfpClusterConfig(*bootstrap).dns_cache_config().preresolve_hostnames(),
+      expected_dns_preresolve_hostnames));
+}
+
 #ifdef ENVOY_MOBILE_STATS_REPORTING
 std::string statsdSinkConfig(int port) {
   std::string config = R"({ name: envoy.stat_sinks.statsd,
@@ -280,13 +324,18 @@ TEST(TestConfig, DisableHttp3) {
 #ifdef ENVOY_GOOGLE_GRPC
 TEST(TestConfig, XdsConfig) {
   EngineBuilder engine_builder;
-  XdsBuilder xds_builder(/*xds_server_address=*/"fake-td.googleapis.com",
-                         /*xds_server_port=*/12345);
+  const std::string host = "fake-td.googleapis.com";
+  const uint32_t port = 12345;
+  const std::string authority = absl::StrCat(host, ":", port);
+
+  XdsBuilder xds_builder(/*xds_server_address=*/host,
+                         /*xds_server_port=*/port);
   engine_builder.setXds(std::move(xds_builder));
   std::unique_ptr<Bootstrap> bootstrap = engine_builder.generateBootstrap();
+
   auto& ads_config = bootstrap->dynamic_resources().ads_config();
   EXPECT_EQ(ads_config.api_type(), envoy::config::core::v3::ApiConfigSource::GRPC);
-  EXPECT_EQ(ads_config.grpc_services(0).google_grpc().target_uri(), "fake-td.googleapis.com:12345");
+  EXPECT_EQ(ads_config.grpc_services(0).google_grpc().target_uri(), authority);
   EXPECT_EQ(ads_config.grpc_services(0).google_grpc().stat_prefix(), "ads");
   EXPECT_THAT(ads_config.grpc_services(0)
                   .google_grpc()
@@ -298,19 +347,17 @@ TEST(TestConfig, XdsConfig) {
   EXPECT_THAT(ads_config.grpc_services(0).google_grpc().call_credentials(), SizeIs(0));
 
   // With initial gRPC metadata.
-  xds_builder =
-      XdsBuilder(/*xds_server_address=*/"fake-td.googleapis.com", /*xds_server_port=*/12345);
+  xds_builder = XdsBuilder(/*xds_server_address=*/host, /*xds_server_port=*/port);
   xds_builder.addInitialStreamHeader(/*header=*/"x-goog-api-key", /*value=*/"A1B2C3")
       .addInitialStreamHeader(/*header=*/"x-android-package",
                               /*value=*/"com.google.envoymobile.io.myapp");
   xds_builder.setSslRootCerts(/*root_certs=*/"my_root_cert");
-  xds_builder.setSni(/*sni=*/"fake-td.googleapis.com");
+  xds_builder.setSni(/*sni=*/host);
   engine_builder.setXds(std::move(xds_builder));
   bootstrap = engine_builder.generateBootstrap();
   auto& ads_config_with_metadata = bootstrap->dynamic_resources().ads_config();
   EXPECT_EQ(ads_config_with_metadata.api_type(), envoy::config::core::v3::ApiConfigSource::GRPC);
-  EXPECT_EQ(ads_config_with_metadata.grpc_services(0).google_grpc().target_uri(),
-            "fake-td.googleapis.com:12345");
+  EXPECT_EQ(ads_config_with_metadata.grpc_services(0).google_grpc().target_uri(), authority);
   EXPECT_EQ(ads_config_with_metadata.grpc_services(0).google_grpc().stat_prefix(), "ads");
   EXPECT_EQ(ads_config_with_metadata.grpc_services(0)
                 .google_grpc()