iOS: Add includeUnroutableDNSResults to engine builder#2128
Closed
jpsim wants to merge 8 commits intoenvoyproxy:mainfrom
Closed
iOS: Add includeUnroutableDNSResults to engine builder#2128jpsim wants to merge 8 commits intoenvoyproxy:mainfrom
includeUnroutableDNSResults to engine builder#2128jpsim wants to merge 8 commits intoenvoyproxy:mainfrom
Conversation
If set, 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 may be a useful setting to specify if the addresses considered unroutable by the system's heuristics may in practice be routable. Signed-off-by: JP Simard <[email protected]>
Signed-off-by: JP Simard <[email protected]>
Signed-off-by: JP Simard <[email protected]>
Signed-off-by: JP Simard <[email protected]>
Signed-off-by: JP Simard <[email protected]>
jpsim
commented
Mar 25, 2022
.gitmodules
Outdated
| [submodule "envoy"] | ||
| path = envoy | ||
| url = https://github.com/envoyproxy/envoy.git | ||
| url = https://github.com/jpsim/envoy.git |
Contributor
Author
There was a problem hiding this comment.
Just for testing purposes, will revert before merging.
To match Android Signed-off-by: JP Simard <[email protected]>
jpsim
changed the title
includeUnroutableFamilies to engine builderenableDNSFilterUnroutableFamilies to engine builder
goaway
reviewed
Mar 30, 2022
| enableHappyEyeballs:(BOOL)enableHappyEyeballs | ||
| enableInterfaceBinding:(BOOL)enableInterfaceBinding | ||
| enforceTrustChainVerification:(BOOL)enforceTrustChainVerification | ||
| enableDNSFilterUnroutableFamilies:(BOOL)enableDNSFilterUnroutableFamilies |
Contributor
There was a problem hiding this comment.
nit: how about filterUnroutableDNSResults (or perhaps, filterUnroutableDNSResultIPs)?
Contributor
Author
There was a problem hiding this comment.
Filter is ambiguous if we're keeping or rejecting unroutable results.
Perhaps excludeUnroutableDNSResults since we can share that setting across platforms since we also have the option to do that on the c-ares side.
goaway
reviewed
Mar 30, 2022
| @"envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig\"}\n"]; | ||
| @"envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig\", " | ||
| @"\"include_unroutable_families\": %@}\n", | ||
| self.enableDNSFilterUnroutableFamilies ? @"false" : @"true"]; |
Contributor
Author
|
Thanks for the review, @goaway although the most important part of this to review is the Envoy side: envoyproxy/envoy#20527 |
…unroutable-families * origin/main: (25 commits) Update Envoy to c96f711 (envoyproxy#2168) Bump Lyft Support Rotation (envoyproxy#2162) Update Envoy to 0e8899c (envoyproxy#2166) Update rules_apple & rules_swift (envoyproxy#2167) bazel: set inmemory remote exec flags globally http client: add cancel log and limit callback to open streams (envoyproxy#2165) bump envoy to 5181d2355f208061688922572727fe06ba8b3a07 (envoyproxy#2157) pin maven dependencies (envoyproxy#2161) Fix iOS termination crash in `ProvisionalDispatcher` (envoyproxy#2059) bazel: Allow multiple definitions for armeabi android links internal: pass engine handles throughout API (envoyproxy#2149) Bump Lyft Support Rotation (envoyproxy#2156) add specifying more maven deps (envoyproxy#2151) update envoy@e4eaf1b97 (envoyproxy#2146) bazel: create symbol mapping file (envoyproxy#2126) Bump Lyft Support Rotation (envoyproxy#2148) bazel: remove sandbox disable build: export flatbuffer jvm dep (envoyproxy#2147) Bump Lyft Support Rotation (envoyproxy#2143) bazel: Add flatbuffers Swift hack ... Signed-off-by: JP Simard <[email protected]>
* Rename `enableDNSFilterUnroutableFamilies` to `includeUnroutableDNSResults` * Flip default behavior on Android * Update docs Signed-off-by: JP Simard <[email protected]>
jpsim
changed the title
enableDNSFilterUnroutableFamilies to engine builderincludeUnroutableDNSResults to engine builder
Contributor
Author
|
Closing as stale now that we've migrated to the Envoy repo: https://github.com/envoyproxy/envoy/blob/main/mobile/README.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If set, 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 may be a useful setting to specify if the addresses considered unroutable by the system's heuristics may in practice be routable.
We may want to enable this in combination with happy eyeballs.
Description: iOS: Add
includeUnroutableFamiliesto engine builderRisk Level: Low, Apple-only, off by default
Testing: I've confirmed both branches of the code paths are hit when the flag is on and off on a physical iPhone. I will validate the behavioral changes next before merging. Unit tests in progress...
Docs Changes: In progress...
Release Notes: In progress...
To Do: