Skip to content

feat!: Add a Resolver trait to abstract over DNS resolvers #3326

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Frando
Copy link
Member

@Frando Frando commented May 27, 2025

Description

This adds a Resolver trait to abstract over DNS resolution. It contains methods to resolve IPv4 or IPv6 addresses, and TXT records. Because the trait needs to be dyn-compatible for our usage (we don't want to have a generic for the DNS resolver on the Endpoint), all methods return boxed futures that contain boxed iterators.

The DnsResolver struct is changed to contain an Arc<dyn Resolver> instead of being hardcoded to hickory_resolver::TokioResolver. Iroh ships an implementation of Resolver for hickory_resolver::TokioResolver, but does not have any hickory types in the public API anymore.

Users can implement the Resolver trait on whatever struct to use a completely custom DNS resolver.

Breaking Changes

Changes in iroh_relay::dns (reexported from iroh::dns):

  • DnsResolver::new now takes an impl Resolver. To create a default resolver, use DnsResolver::default or the equivalent DnsResolver::new_with_system_defaults
  • DnsResolver::lookup_txt now returns impl Iterator<Item = TxtRecord>
  • TxtLookup and TXT are removed.

Changes in iroh_relay::node_info (reexported from iroh::discovery):

  • NodeInfo::from_txt_lookup now takes name: String, lookup: impl Iterator<Item = crate::dns::TxtRecord>

Notes & open questions

We could now put the implementation of Resolver for hickory_resolver::TokioResolver behind a (default) feature flag to not have to hard-depend on a specific version of hickory_resolver anymore during iroh 1.0 - I think? Or maybe we'd still have to newtype it. Will need to think about this some more.

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.
    • List all breaking changes in the above "Breaking Changes" section.
    • Open an issue or PR on any number0 repos that are affected by this breaking change. Give guidance on how the updates should be handled or do the actual updates themselves. The major ones are:

@Frando Frando added this to the v0.99 milestone May 27, 2025
Copy link

github-actions bot commented May 27, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3326/docs/iroh/

Last updated: 2025-05-27T11:31:00Z

Copy link

github-actions bot commented May 27, 2025

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: ecb8767

@n0bot n0bot bot added this to iroh May 27, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh May 27, 2025
@Frando Frando force-pushed the Frando/resolver-trait branch from d9d6f7e to 5168b02 Compare May 27, 2025 11:12
@Frando Frando marked this pull request as ready for review May 27, 2025 11:29
@dignifiedquire
Copy link
Contributor

I am not convinced this amount of boxing is really worth it, I'd rather not have a trait, and simply wrap hickory types for 1.0 stability

@Frando
Copy link
Member Author

Frando commented May 27, 2025

I am not convinced this amount of boxing is really worth it, I'd rather not have a trait, and simply wrap hickory types for 1.0 stability

OK, then we'd need to add a builder to allow setting more options, no? Right now we don't allow configuring apart from with_single_nameserver (or, the way we do offer is that you can convert a hickory TokioResolver into our DNS resolver, but that means that hickory is in our public API).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

2 participants