Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DNS resolver as an extension point #14310

Closed
htuch opened this issue Dec 7, 2020 · 7 comments · Fixed by #17272 or #17479
Closed

DNS resolver as an extension point #14310

htuch opened this issue Dec 7, 2020 · 7 comments · Fixed by #17272 or #17479
Assignees
Labels
area/dns design proposal Needs design doc/proposal before implementation help wanted Needs help!

Comments

@htuch
Copy link
Member

htuch commented Dec 7, 2020

We currently have c-ares and Apple iOS DNS resolver support as compile time options. We might also be interested in a site-specific DNS resolver library. I think it's reasonable to make this a standard extension point.

@junr03 since you were last digging into this, any thoughts on whether it would be feasible to accommodate the various DNS resolvers this way?

CC @yanavlasov @antoniovicente @asraa @adisuissa @KBaichoo @akonradi

@htuch htuch added triage Issue requires triage area/dns design proposal Needs design doc/proposal before implementation and removed triage Issue requires triage labels Dec 7, 2020
@junr03
Copy link
Member

junr03 commented Dec 8, 2020

It would certainly be feasible. @mattklein123 and I discussed it somewhere (I am trying to find where) when I was adding the Apple Resolver, and decided for the current setup for expediency. However, there aren't any major technical reasons why the resolvers couldn't be formalized as an extension point. In fact, it would help clean up configuration that doesn't apply to all resolvers.

Resolvers are created via the dispatcher, so there is already a small surface for resolver creation. In terms of configuration consumption there are a few places where the extension would have to be accessible: some clusters, dns cache, udp dns filter; and additionally the "default" server resolver.

So overall, my assessment is that while there is some cleaning up to do it would actually be nice to elevate resolvers as an extension point, and something I would have wanted to do if I had had more time in the past. Happy to help whomever wants to take this on.

@mattklein123
Copy link
Member

Yes I agree making the DNS resolver a full extension would be good.

@github-actions
Copy link

github-actions bot commented Jan 7, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 7, 2021
@htuch htuch added help wanted Needs help! and removed stale stalebot believes this issue/PR has not been touched recently labels Jan 8, 2021
@yanjunxiang-google
Copy link
Contributor

yanjunxiang-google commented Jun 30, 2021

I am looking at this issue now.

@yanjunxiang-google
Copy link
Contributor

@yanavlasov

@KBaichoo
Copy link
Contributor

/assign @yanjunxiang-google

yanjunxiang-google added a commit to yanjunxiang-google/envoy that referenced this issue Jul 8, 2021
…NS resolver

in API definition. This configuration is to support DNS resolution as a first
class Envoy extension. This API change is the first step for that task.

This configuration is to replace above dns_resolution_config.
By default envoy supports c-ares DNS or apple DNS resolvers. This extension can be used
to configure other DNS resolver types.
This configuration is optional. In case it is missing, the default behavior is in place,
which means Envoy will either use c-ares DNS library or apple DNS library based on
the compiling flag. When this configuration is in place, Envoy will use the configured
DNS resolver carried in the extension typed_config field.
In case both typed_dns_resolver_config and dns_resolution_config are configured,
typed_dns_resolver_config take precedence.

Testing:
The code change passed bazel test.

Release Notes:
N/A

Issues: envoyproxy#14310
Fix : N/A

Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
@htuch
Copy link
Member Author

htuch commented Jul 19, 2021

Reopening, since #17272 was just the API changes (@yanjunxiang-google best to put "Part of #xyzabc" instead of "Fixes #xyzabc" when working on a series of patches on an issue).

@htuch htuch reopened this Jul 19, 2021
yanjunxiang-google added a commit to yanjunxiang-google/envoy that referenced this issue Jul 25, 2021
… class extension for issue: envoyproxy#14310.

Signed-off-by: Yanjun Xiang <yanjunxiang@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dns design proposal Needs design doc/proposal before implementation help wanted Needs help!
Projects
None yet
5 participants