Skip to content
30 changes: 0 additions & 30 deletions docs/root/api/starting_envoy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,6 @@ The configuration is expected as a JSON list.
// Swift
builder.addDNSPreresolveHostnames("[{\"address\": \"foo.com", \"port_value\": 443}]")

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``addDNSFallbackNameservers``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. attention::

This API is only available for Kotlin.

Add a list of IP addresses to use as fallback DNS name servers.
See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/network/dns_resolver/cares/v3/cares_dns_resolver.proto#extensions-network-dns-resolver-cares-v3-caresdnsresolverconfig>`__
for further information.

// Kotlin
builder.addDNSFallbackNameservers(listOf<String>("8.8.8.8"))

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``enableDNSFilterUnroutableFamilies``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. attention::

This API is only available for Kotlin.

Specify whether to filter unroutable IP families during DNS resolution or not.
See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/network/dns_resolver/cares/v3/cares_dns_resolver.proto#extensions-network-dns-resolver-cares-v3-caresdnsresolverconfig>`__
for further information.

// Kotlin
builder.enableDNSFilterUnroutableFamilies(true)

~~~~~~~~~~~~~~~
``addLogLevel``
~~~~~~~~~~~~~~~
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Breaking changes:

- ios/android: remove ``addH2RawDomains`` method. (:issue: `#2590 <2590>`)
- build: building on macOS now requires Xcode 14.0. (:issue:`#2544 <2544>`)
- kotlin: always use ``getaddrinfo`` DNS resolver. Remove ``addDNSFallbackNameservers``, ``enableDNSFilterUnroutableFamilies``, and ``enableDNSUseSystemResolver`` methods from the Kotlin engine builder. (:issue:`#2618 <2618>`)

Bugfixes:

Expand Down
22 changes: 0 additions & 22 deletions library/cc/engine_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,27 +202,6 @@ EngineBuilder::enablePlatformCertificatesValidation(bool platform_certificates_v
}

std::string EngineBuilder::generateConfigStr() const {
#if defined(__APPLE__)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have high confidence of consistently using the apple resolver without this block - I'd think we'd want to replace cares here with getaddrinfo. Do we have any e2e validation for that? Alternately, can you get a secondary Lyft reviewer?
cc @RyanTheOptimist because perhaps it won't take effect until we switch to the c++ builder by defualt

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm following the PR and your comment correctly, I think the reason we'll use the Apple resolver without this block regardless of the Builder type (after this PR lands) is because of the change to config.cc:

https://github.com/envoyproxy/envoy-mobile/pull/2618/files#diff-1af53ae37f8c227f37b80bcb6e2335f55b2f830aa658ac77e099f2549f157712

Unless there is code to specifically override dns_resolver_config (which there shouldn't be since @Augustyniak seems to have removed it in this PR), it will use the Apple resolver on iOS and getaddrinfo elsewhere. Does that sound right to you?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough. the one other thing i'd ask for is a follow-up tracking issue on removing the cares dependency, which may be an upstream thing

Copy link
Copy Markdown
Contributor Author

@Augustyniak Augustyniak Oct 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there is code to specifically override dns_resolver_config (which there shouldn't be since @Augustyniak seems to have removed it in this PR), it will use the Apple resolver on iOS and getaddrinfo elsewhere. Does that sound right to you?

  1. iOS engine builder always uses apple dns resolver
  2. Kotlin engine builder uses apple dns resolver on mac and getaddrinfo everywhere else (linux/Android). I can modify it so that it always uses getaddtinfo - let me know.
  3. c++ engine builder uses apple dns resolver on mac/OS and getaddrinfo resolver everywhere else.

I would appreciate your thoughts on 2). Will look into @alyssawilk 's comment more and open GH issue if I cannot figure it out.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened #2629

std::string dns_resolver_name = "envoy.network.dns_resolver.apple";
std::string dns_resolver_config =
"{\"@type\":\"type.googleapis.com/"
"envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig\"}";
#else
std::string dns_resolver_name = "";
std::string dns_resolver_config = "";
if (this->use_system_resolver_) {
dns_resolver_name = "envoy.network.dns_resolver.getaddrinfo";
dns_resolver_config =
"{\"@type\":\"type.googleapis.com/"
"envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig\"}";
} else {
dns_resolver_name = "envoy.network.dns_resolver.cares";
dns_resolver_config =
"{\"@type\":\"type.googleapis.com/"
"envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig\"}";
}
#endif

std::vector<std::pair<std::string, std::string>> replacements {
{"connect_timeout", fmt::format("{}s", this->connect_timeout_seconds_)},
{"dns_fail_base_interval", fmt::format("{}s", this->dns_failure_refresh_seconds_base_)},
Expand All @@ -233,7 +212,6 @@ std::string EngineBuilder::generateConfigStr() const {
{"dns_preresolve_hostnames", this->dns_preresolve_hostnames_},
{"dns_refresh_rate", fmt::format("{}s", this->dns_refresh_seconds_)},
{"dns_query_timeout", fmt::format("{}s", this->dns_query_timeout_seconds_)},
{"dns_resolver_name", dns_resolver_name}, {"dns_resolver_config", dns_resolver_config},
{"enable_drain_post_dns_refresh", enable_drain_post_dns_refresh_ ? "true" : "false"},
{"enable_interface_binding", enable_interface_binding_ ? "true" : "false"},
{"h2_connection_keepalive_idle_interval",
Expand Down
4 changes: 2 additions & 2 deletions library/common/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ R"(- &dns_resolver_name envoy.network.dns_resolver.apple
- &dns_resolver_config {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig"}
)"
#else
R"(- &dns_resolver_name envoy.network.dns_resolver.cares
- &dns_resolver_config {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig"}
R"(- &dns_resolver_name envoy.network.dns_resolver.getaddrinfo
- &dns_resolver_config {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig"}
)"
#endif
R"(- &enable_drain_post_dns_refresh false
Expand Down
19 changes: 2 additions & 17 deletions library/common/jni/android_jni_interface.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#include <ares.h>

#include "library/common/jni/android_network_utility.h"
#include "library/common/jni/import/jni_import.h"
#include "library/common/jni/jni_support.h"
Expand All @@ -13,21 +11,8 @@
extern "C" JNIEXPORT jint JNICALL
Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_initialize(JNIEnv* env,
jclass, // class
jobject class_loader,
jobject connectivity_manager) {
jobject class_loader) {
set_class_loader(env->NewGlobalRef(class_loader));
// At this point, we know Android APIs are available. Register cert chain validation JNI calls.
envoy_status_t result =
register_platform_api(cert_validator_name, get_android_cert_validator_api());
if (result == ENVOY_FAILURE) {
return ENVOY_FAILURE;
}

// See note above about c-ares.
// c-ares jvm init is necessary in order to let c-ares perform DNS resolution in Envoy.
// More information can be found at:
// https://c-ares.haxx.se/ares_library_init_android.html
ares_library_init_jvm(get_vm());

return ares_library_init_android(connectivity_manager);
return register_platform_api(cert_validator_name, get_android_cert_validator_api());
}
3 changes: 1 addition & 2 deletions library/common/jni/android_test_jni_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
extern "C" JNIEXPORT jint JNICALL
Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_initialize(JNIEnv* env,
jclass, // class
jobject class_loader,
jobject connectivity_manager) {
jobject class_loader) {
set_class_loader(env->NewGlobalRef(class_loader));
// At this point, we know Android APIs are available. Register cert chain validation JNI calls.
return register_platform_api(cert_validator_name, get_android_cert_validator_api());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,15 @@ public static void load(Context context) {
// its dependencies are loaded and initialized at most once.
private static class AndroidLoader {
private AndroidLoader(Context context) {
AndroidJniLibrary.initialize(
context.getClassLoader(),
(ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE));
AndroidJniLibrary.initialize(context.getClassLoader());
}
}

/**
* Native binding to register the ConnectivityManager to C-Ares.
*
* @param classLoader Application's class loader.
* @param connectivityManager Android's ConnectivityManager.
* @return The resulting status of the initialization.
*/
protected static native int initialize(ClassLoader classLoader,
ConnectivityManager connectivityManager);
protected static native int initialize(ClassLoader classLoader);
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ public enum TrustChainVerification {
public final Integer dnsQueryTimeoutSeconds;
public final Integer dnsMinRefreshSeconds;
public final String dnsPreresolveHostnames;
public final List<String> dnsFallbackNameservers;
public final Boolean dnsFilterUnroutableFamilies;
public final Boolean dnsUseSystemResolver;
public final Boolean enableDrainPostDnsRefresh;
public final Boolean enableHttp3;
public final Boolean enableGzip;
Expand Down Expand Up @@ -85,13 +82,6 @@ public enum TrustChainVerification {
* refresh DNS.
* @param dnsPreresolveHostnames hostnames to preresolve on Envoy Client
* construction.
* @param dnsFallbackNameservers addresses to use as DNS name server
* fallback.
* @param dnsFilterUnroutableFamilies whether to filter unroutable IP families
* or not.
* @param dnsUseSystemResolver whether to use the getaddrinfo-based
* system resolver or
* c-ares.
* @param enableDrainPostDnsRefresh whether to drain connections after soft
* DNS refresh.
* @param enableHttp3 whether to enable experimental support for
Expand Down Expand Up @@ -133,13 +123,12 @@ public EnvoyConfiguration(
boolean adminInterfaceEnabled, String grpcStatsDomain, int connectTimeoutSeconds,
int dnsRefreshSeconds, int dnsFailureRefreshSecondsBase, int dnsFailureRefreshSecondsMax,
int dnsQueryTimeoutSeconds, int dnsMinRefreshSeconds, String dnsPreresolveHostnames,
List<String> dnsFallbackNameservers, boolean dnsFilterUnroutableFamilies,
boolean dnsUseSystemResolver, boolean enableDrainPostDnsRefresh, boolean enableHttp3,
boolean enableGzip, boolean enableBrotli, boolean enableSocketTagging,
boolean enableHappyEyeballs, boolean enableInterfaceBinding,
int h2ConnectionKeepaliveIdleIntervalMilliseconds, int h2ConnectionKeepaliveTimeoutSeconds,
boolean h2ExtendKeepaliveTimeout, int maxConnectionsPerHost, int statsFlushSeconds,
int streamIdleTimeoutSeconds, int perTryIdleTimeoutSeconds, String appVersion, String appId,
boolean enableDrainPostDnsRefresh, boolean enableHttp3, boolean enableGzip,
boolean enableBrotli, boolean enableSocketTagging, boolean enableHappyEyeballs,
boolean enableInterfaceBinding, int h2ConnectionKeepaliveIdleIntervalMilliseconds,
int h2ConnectionKeepaliveTimeoutSeconds, boolean h2ExtendKeepaliveTimeout,
int maxConnectionsPerHost, int statsFlushSeconds, int streamIdleTimeoutSeconds,
int perTryIdleTimeoutSeconds, String appVersion, String appId,
TrustChainVerification trustChainVerification, String virtualClusters,
List<EnvoyNativeFilterConfig> nativeFilterChain,
List<EnvoyHTTPFilterFactory> httpPlatformFilterFactories,
Expand All @@ -155,9 +144,6 @@ public EnvoyConfiguration(
this.dnsQueryTimeoutSeconds = dnsQueryTimeoutSeconds;
this.dnsMinRefreshSeconds = dnsMinRefreshSeconds;
this.dnsPreresolveHostnames = dnsPreresolveHostnames;
this.dnsFallbackNameservers = dnsFallbackNameservers;
this.dnsFilterUnroutableFamilies = dnsFilterUnroutableFamilies;
this.dnsUseSystemResolver = dnsUseSystemResolver;
this.enableDrainPostDnsRefresh = enableDrainPostDnsRefresh;
this.enableHttp3 = enableHttp3;
this.enableGzip = enableGzip;
Expand Down Expand Up @@ -235,33 +221,6 @@ String resolveTemplate(final String configTemplate, final String platformFilterT
String processedTemplate =
configTemplate.replace("#{custom_filters}", customFiltersBuilder.toString());

String dnsFallbackNameserversAsString = "[]";
if (!dnsFallbackNameservers.isEmpty()) {
StringBuilder sb = new StringBuilder("[");
String separator = "";
for (String nameserver : dnsFallbackNameservers) {
sb.append(separator);
separator = ",";
sb.append(String.format("{\"socket_address\":{\"address\":\"%s\"}}", nameserver));
}
sb.append("]");
dnsFallbackNameserversAsString = sb.toString();
}

String dnsResolverName = "";
String dnsResolverConfig = "";
if (dnsUseSystemResolver) {
dnsResolverName = "envoy.network.dns_resolver.getaddrinfo";
dnsResolverConfig =
"{\"@type\":\"type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig\"}";
} else {
dnsResolverName = "envoy.network.dns_resolver.cares";
dnsResolverConfig = String.format(
"{\"@type\":\"type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig\",\"resolvers\":%s,\"use_resolvers_as_fallback\": %s, \"filter_unroutable_families\": %s}",
dnsFallbackNameserversAsString, !dnsFallbackNameservers.isEmpty() ? "true" : "false",
dnsFilterUnroutableFamilies ? "true" : "false");
}

StringBuilder configBuilder = new StringBuilder("!ignore platform_defs:\n");
configBuilder.append(String.format("- &connect_timeout %ss\n", connectTimeoutSeconds))
.append(String.format("- &dns_fail_base_interval %ss\n", dnsFailureRefreshSecondsBase))
Expand All @@ -275,9 +234,7 @@ String resolveTemplate(final String configTemplate, final String platformFilterT
String.format("- &dns_multiple_addresses %s\n", enableHappyEyeballs ? "true" : "false"))
.append(String.format("- &h2_delay_keepalive_timeout %s\n",
h2ExtendKeepaliveTimeout ? "true" : "false"))
.append(String.format("- &dns_resolver_name %s\n", dnsResolverName))
.append(String.format("- &dns_refresh_rate %ss\n", dnsRefreshSeconds))
.append(String.format("- &dns_resolver_config %s\n", dnsResolverConfig))
.append(String.format("- &enable_drain_post_dns_refresh %s\n",
enableDrainPostDnsRefresh ? "true" : "false"))
.append(String.format("- &enable_interface_binding %s\n",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,8 @@ private EnvoyConfiguration createEnvoyConfiguration() {
return new EnvoyConfiguration(
mAdminInterfaceEnabled, mGrpcStatsDomain, mConnectTimeoutSeconds, mDnsRefreshSeconds,
mDnsFailureRefreshSecondsBase, mDnsFailureRefreshSecondsMax, mDnsQueryTimeoutSeconds,
mDnsMinRefreshSeconds, mDnsPreresolveHostnames, mDnsFallbackNameservers,
mEnableDnsFilterUnroutableFamilies, mDnsUseSystemResolver, mEnableDrainPostDnsRefresh,
quicEnabled(), mEnableGzip, brotliEnabled(), mEnableSocketTag, mEnableHappyEyeballs,
mDnsMinRefreshSeconds, mDnsPreresolveHostnames, mEnableDrainPostDnsRefresh, quicEnabled(),
mEnableGzip, brotliEnabled(), mEnableSocketTag, mEnableHappyEyeballs,
mEnableInterfaceBinding, mH2ConnectionKeepaliveIdleIntervalMilliseconds,
mH2ConnectionKeepaliveTimeoutSeconds, mH2ExtendKeepaliveTimeout, mMaxConnectionsPerHost,
mStatsFlushSeconds, mStreamIdleTimeoutSeconds, mPerTryIdleTimeoutSeconds, mAppVersion,
Expand Down
47 changes: 0 additions & 47 deletions library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ open class EngineBuilder(
private var dnsRefreshSeconds = 60
private var dnsFailureRefreshSecondsBase = 2
private var dnsFailureRefreshSecondsMax = 10
private var dnsFallbackNameservers = listOf<String>()
private var dnsFilterUnroutableFamilies = true
private var dnsUseSystemResolver = true
private var dnsQueryTimeoutSeconds = 25
private var dnsMinRefreshSeconds = 60
private var dnsPreresolveHostnames = "[]"
Expand Down Expand Up @@ -196,47 +193,6 @@ open class EngineBuilder(
return this
}

/**
* Add a list of IP addresses to use as fallback DNS name servers.
*
* @param dnsFallbackNameservers addresses to use.
*
* @return this builder.
*/
fun addDNSFallbackNameservers(dnsFallbackNameservers: List<String>): EngineBuilder {
this.dnsFallbackNameservers = dnsFallbackNameservers
return this
}

/**
* Specify whether to filter unroutable IP families during DNS resolution or not.
* Defaults to true.
*
* @param dnsFilterUnroutableFamilies whether to filter or not.
*
* @return this builder.
*/
fun enableDNSFilterUnroutableFamilies(dnsFilterUnroutableFamilies: Boolean): EngineBuilder {
this.dnsFilterUnroutableFamilies = dnsFilterUnroutableFamilies
return this
}

/**
* Specify whether to use the getaddrinfo-based system DNS resolver or the c-ares resolver.
* Defaults to true.
*
* Note that if this is set, the values of `dnsFallbackNameservers` and
* `dnsFilterUnroutableFamilies` will be ignored.
*
* @param dnsUseSystemResolver whether to use the system DNS resolver.
*
* @return this builder.
*/
fun enableDNSUseSystemResolver(dnsUseSystemResolver: Boolean): EngineBuilder {
this.dnsUseSystemResolver = dnsUseSystemResolver
return this
}

/**
* Specify whether to drain connections after the resolution of a soft DNS refresh. A refresh may
* be triggered directly via the Engine API, or as a result of a network status update provided by
Expand Down Expand Up @@ -628,9 +584,6 @@ open class EngineBuilder(
dnsQueryTimeoutSeconds,
dnsMinRefreshSeconds,
dnsPreresolveHostnames,
dnsFallbackNameservers,
dnsFilterUnroutableFamilies,
dnsUseSystemResolver,
enableDrainPostDnsRefresh,
enableHttp3,
enableGzip,
Expand Down
5 changes: 0 additions & 5 deletions library/objective-c/EnvoyConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,6 @@ - (nullable NSString *)resolveTemplate:(NSString *)templateYAML {
[definitions appendFormat:@"- &dns_refresh_rate %lus\n", (unsigned long)self.dnsRefreshSeconds];
[definitions appendFormat:@"- &enable_drain_post_dns_refresh %@\n",
self.enableDrainPostDnsRefresh ? @"true" : @"false"];
// No additional values are currently needed for Apple-based DNS resolver.
[definitions
appendFormat:@"- &dns_resolver_config "
@"{\"@type\":\"type.googleapis.com/"
@"envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig\"}\n"];
[definitions appendFormat:@"- &enable_interface_binding %@\n",
self.enableInterfaceBinding ? @"true" : @"false"];
[definitions appendFormat:@"- &trust_chain_verification %@\n", self.enforceTrustChainVerification
Expand Down
13 changes: 0 additions & 13 deletions test/cc/unit/envoy_config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,6 @@ TEST(TestConfig, ConfigIsValid) {
#endif
}

#if !defined(__APPLE__)
TEST(TestConfig, SetUseDnsCAresResolver) {
EngineBuilder engine_builder;
engine_builder.useDnsSystemResolver(false);
std::string config_str = engine_builder.generateConfigStr();
envoy::config::bootstrap::v3::Bootstrap bootstrap;
TestUtility::loadFromYaml(absl::StrCat(config_header, config_str), bootstrap);

ASSERT_THAT(bootstrap.DebugString(), HasSubstr("envoy.network.dns_resolver.cares"));
ASSERT_THAT(bootstrap.DebugString(), Not(HasSubstr("envoy.network.dns_resolver.getaddrinfo")));
}
#endif

TEST(TestConfig, SetGzip) {
EngineBuilder engine_builder;

Expand Down
Loading