Update hickory-dns monorepo to 0.25.2 #8391
Conversation
![oxide-renovate[bot]](https://avatars.githubusercontent.com/u/54040662?s=60&v=4){kind=small}
oxide-renovate
bot
added
the
dependencies
dns-server/src/dns_server.rs
Outdated
| } | ||
|
|
||
| let response_records = name_records | ||
| let response_records = mr |
There was a problem hiding this comment.
@iximeow this is the area where I would particularly appreciate scrutiny. hickory changed from a single query (query()) to multiple queries (queries()). I think I've updated this in a reasonable fashion, but I'm a bit out of my depth here.
There was a problem hiding this comment.
i think this is reasonable for what it is, but i don't think we should try answering queries with multiple questions unless someone's got a good reason for external DNS to. RFC 9619 is hot off the presses and constrains this to zero or one questions anyway. we're definitely not going to query like this internally, and it seems really unlikely to get queries like this externally.
you may ask, as i did: "but RFC 9619 talks specifically about Opcode 0 (Query), what if the request was for something else? would we be here?": ah, right #6411
i'm going to adjust this to still use queries(), but FORMERR if we get a query with anything other than one question. now can i write a test to hang this comment off of, is the question.. :)
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. |
| resolver_config.add_name_server(NameServerConfig::new( | ||
| dns_addr, | ||
| hickory_resolver::proto::xfer::Protocol::Udp, | ||
| )); |
There was a problem hiding this comment.
kind of funny but fine: trust_negative_responses "defaults to false" while NameServerConfig::new makes it true. we're only going to query the one resolver so this doesn't change anything in practice.
now that our internal answers are authoritative it'd be a difference if we queried all internal name servers, but even then trusting negative answers seems fine.
There was a problem hiding this comment.
Ooops... that is a kinds of funny. Nice catch
clients/oxide-client/src/lib.rs
Outdated
| @@ -63,8 +67,8 @@ impl CustomDnsResolver { | |||
| self.dns_addr | |||
| } | |||
|
|
|||
| /// Returns the underlying `TokioAsyncResolver | |||
| pub fn resolver(&self) -> &TokioAsyncResolver { | |||
| /// Returns the underlying `TokioResolver | |||
| .set_rr_type(RecordType::A) | ||
| .set_data(Some(RData::A((*addr).into()))); | ||
| Ok(a) | ||
| Ok(Record::from_rdata(name.clone(), 0, RData::A((*addr).into()))) |
There was a problem hiding this comment.
nice. i'd wondered why .set_rr_type() was needed when .set_data() follows up with an RData that tells you the type too, and it looks like they solved this by moving around Record innards.
dns-server/src/dns_server.rs
Outdated
| } | ||
|
|
||
| let response_records = name_records | ||
| let response_records = mr |
There was a problem hiding this comment.
i think this is reasonable for what it is, but i don't think we should try answering queries with multiple questions unless someone's got a good reason for external DNS to. RFC 9619 is hot off the presses and constrains this to zero or one questions anyway. we're definitely not going to query like this internally, and it seems really unlikely to get queries like this externally.
you may ask, as i did: "but RFC 9619 talks specifically about Opcode 0 (Query), what if the request was for something else? would we be here?": ah, right #6411
i'm going to adjust this to still use queries(), but FORMERR if we get a query with anything other than one question. now can i write a test to hang this comment off of, is the question.. :)
| .map_err(|e| match e.kind() { | ||
| ResolveErrorKind::NoRecordsFound { .. } | ||
| | ResolveErrorKind::Timeout => CondCheckError::NotYet, | ||
| .map_err(|e| match resolve_error_proto_kind(&e) { |
There was a problem hiding this comment.
lookup_ip will error directly with a ProtoError instead of the ResolveErrorKind stuff once hickory-dns 0.26 happens, so we have that to look forward to.
| // Otherwise, `NoRecordsFound` means we should either switch to | ||
| // AAAA queries (if this was A) or we're done (and failed). |
There was a problem hiding this comment.
this is kind of an oddball with the other comments going away, i'll wordsmith.
There was a problem hiding this comment.
I meant to pull that other comment into the body of the if, but forgot
| @@ -7,10 +7,11 @@ | |||
| use anyhow::Context; | |||
| use anyhow::anyhow; | |||
| use futures::FutureExt; | |||
| use hickory_resolver::TokioAsyncResolver; | |||
| use hickory_resolver::TokioResolver; | |||
There was a problem hiding this comment.
ah, i see, the synchronous Hickory resolver is just gone gone (PR 2515 over there) and names shifted accordingly. good for them!
This PR contains the following updates:
0.24.4->0.25.20.24.4->0.25.20.24.4->0.25.20.24.4->0.25.2Release Notes
hickory-dns/hickory-dns (hickory-client)
v0.25.2: 0.25.2Compare Source
What's Changed
ns_pool_for_zone()by @divergentdave in https://github.com/hickory-dns/hickory-dns/pull/2888ResolverBuilder::with_options()method by @cratelyn in https://github.com/hickory-dns/hickory-dns/pull/2877tsharkoutput by @divergentdave in https://github.com/hickory-dns/hickory-dns/pull/2868TBS::new()by @divergentdave in https://github.com/hickory-dns/hickory-dns/pull/2942v0.25.1: 0.25.1Compare Source
This is a small patch release to address errors that prevented publication of version 0.25.0 of some crates.
What's Changed
Full Changelog: hickory-dns/hickory-dns@v0.25.0...v0.25.1
Configuration
📅 Schedule: Branch creation - "after 8pm,before 6am" in timezone America/Los_Angeles, Automerge - "after 8pm,before 6am" in timezone America/Los_Angeles.
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about these updates again.
This PR has been generated by Renovate Bot.