Low level send method in resolver about hickory-dns HOT 8 OPEN

Just exposing a send() method that can send any message on a Resolver or AsyncResolver feels like a bit of a layering violation? I feel like the resolver abstraction kind of requires that the message is actually a query, for example.

from hickory-dns.

I have started some work in this area specifically for recursive resolution. I'd be curious what other changes you think would be necessary, here's an example of how that's being used, here: https://github.com/hickory-dns/hickory-dns/blob/main/crates/recursor/src/recursor_pool.rs#L98C27-L98C27

That's based on the GenericNameServerPool, which is really just a NameServerPool with a predefined GenericNameServer: https://github.com/hickory-dns/hickory-dns/blob/main/crates/resolver/src/name_server/name_server_pool.rs#L51

The NameServerPool itself does implement DnsHandle, which has a send method for a Request: https://github.com/hickory-dns/hickory-dns/blob/main/crates/resolver/src/name_server/name_server_pool.rs#L232

That will return a single DnsResponse... In terms of advanced parameters, we have this type that can be used to pass information to the underlying protocol, though I've never been very happy with it, https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/xfer/dns_request.rs#L17

DnsRequest is really just a Message with the DnsRequestOptions, https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/src/xfer/dns_request.rs#L50

So I think this already exists?

from hickory-dns.

So I think this already exists?

Oh I didn't realize NameServerPool implemented DnsHandle, and yeah that looks like maybe it's exactly what I want? So is that basically Resolver but without the high level interface?

I'd be interested in trying it out in any case. It looks like it's pub(crate) though ATM and I'm not sure how many other internals would need to be opened up to use it.

from hickory-dns.

And yeah, didn't mean to imply that I specifically wanted this in Resolver.

from hickory-dns.

I’d have to review all the details here, but I think it primarily will not do secondary lookups, like CNAME chain resolution (if it’s not in the original response packet). So I wouldn’t call it the resolver, it’s really just a connection pool (with some extra logic to deal with UDP upgrades to TCP in certain cases).

Since I’m using it across crates, the recursor pool is marked crate only, but the NameServerPool which I think is what you’re interested in should be available publicly. NameServerPool could theoretically be moved into the proto crate I think, and then be used with the Client.

from hickory-dns.

Moving NameServerPool down into the proto crate seems like an interesting idea. 👍

from hickory-dns.

Oh awesome, I'll try that out! And thanks for the detailed replies!

So it looks like idiomatic usage (forwarding a request) is basically

let mut name_servers = NameServerConfigGroup::new();
name_servers.push(NameServerConfig::new(SocketAddr::new(...), Protocol::Udp));
let pool = NameServerPool::from_config(
            name_servers,
            ResolverOpts::default(),
            GenericConnector::new(TokioRuntimeProvider::new()),
);
...
let resp = self1.upstream.send(DnsRequest::new({
    let mut m = Message::new();
    m.add_query({
        let mut q =
            Query::query(
                Name::from(request.query().name()),
                request.query().query_type(),
            );
        q.set_query_class(request.query().query_class());
        q
    });
    m
}, DnsRequestOptions::default())).next().await;
match resp {
    Some(Ok(r)) => ...,
    Some(Err(e)) => ...,
    None => ...
}

from hickory-dns.

This is working for me! The only trick was empty responses get turned into errors, but I think transforming them back is pretty safe and it had all the info I needed.

Should I leave this open/rename it about moving the type?

Also if I may, I think the name NameServerPool is confusing led to me skipping over it. I assumed it was a pool that returned configured name server information or something. At the moment the docs just say "Abstract interface for mocking purpose".

I think ideally there'd be something saying "If you need a lower level interface for name resolution, you can use implementations of DnsHandle" and allow users to look for implementers, but unfortunately docs.rs doesn't have the ability to search for implementations (and it's harder cross-crate).

DnsHandle docs say "A trait for implementing high level functions of DNS" which suggests it's intended for internal use only by the wording. I think it's a very usable interface and promoting it would make me less nervous.

I'd be happy to make doc changes if you can suggest the best places to put the info.

from hickory-dns.