Skip to content

Update hickory-dns monorepo to 0.25.2 #8391

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

Merged
merged 7 commits into from
Jun 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
344 changes: 308 additions & 36 deletions Cargo.lock

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,10 @@ headers = "0.4.1"
heck = "0.5"
hex = "0.4.3"
hex-literal = "0.4.1"
hickory-client = "0.24.4"
hickory-proto = "0.24.4"
hickory-resolver = "0.24.4"
hickory-server = "0.24.4"
hickory-client = "0.25.2"
hickory-proto = "0.25.2"
hickory-resolver = "0.25.2"
hickory-server = "0.25.2"
highway = "1.3.0"
hkdf = "0.12.4"
hmac = "0.12.1"
Expand Down
34 changes: 19 additions & 15 deletions clients/oxide-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
use anyhow::Context;
use anyhow::anyhow;
use futures::FutureExt;
use hickory_resolver::TokioAsyncResolver;
use hickory_resolver::TokioResolver;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, i see, the synchronous Hickory resolver is just gone gone (PR 2515 over there) and names shifted accordingly. good for them!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everyone loves async

use hickory_resolver::config::{
NameServerConfig, Protocol, ResolverConfig, ResolverOpts,
NameServerConfig, ResolverConfig, ResolverOpts,
};
use hickory_resolver::name_server::TokioConnectionProvider;
use std::net::SocketAddr;
use std::sync::Arc;
use thiserror::Error;
Expand All @@ -29,32 +30,35 @@ progenitor::generate_api!(
/// for Nexus. This is often useful when trying to connect with Nexus using
/// TLS, since you need to come in via the DNS name to do that.
///
/// This is a thin wrapper around `TokioAsyncResolver`
/// This is a thin wrapper around `TokioResolver`
pub struct CustomDnsResolver {
dns_addr: SocketAddr,
// The lifetime constraints on the `Resolve` trait make it hard to avoid an
// Arc here.
resolver: Arc<TokioAsyncResolver>,
resolver: Arc<TokioResolver>,
}

impl CustomDnsResolver {
/// Make a new custom resolver that uses the DNS server at the specified
/// address
pub fn new(dns_addr: SocketAddr) -> anyhow::Result<CustomDnsResolver> {
let mut resolver_config = ResolverConfig::new();
resolver_config.add_name_server(NameServerConfig {
socket_addr: dns_addr,
protocol: Protocol::Udp,
tls_dns_name: None,
trust_negative_responses: false,
bind_addr: None,
});
resolver_config.add_name_server(NameServerConfig::new(
dns_addr,
hickory_resolver::proto::xfer::Protocol::Udp,
));
Comment on lines +46 to +49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops... that is a kinds of funny. Nice catch

let mut resolver_opts = ResolverOpts::default();
// Enable edns for potentially larger records
resolver_opts.edns0 = true;

let resolver =
Arc::new(TokioAsyncResolver::tokio(resolver_config, resolver_opts));
let resolver = Arc::new(
TokioResolver::builder_with_config(
resolver_config,
TokioConnectionProvider::default(),
)
.with_options(resolver_opts)
.build(),
);
Ok(CustomDnsResolver { dns_addr, resolver })
}

Expand All @@ -63,8 +67,8 @@ impl CustomDnsResolver {
self.dns_addr
}

/// Returns the underlying `TokioAsyncResolver
pub fn resolver(&self) -> &TokioAsyncResolver {
/// Returns the underlying `TokioResolver`
pub fn resolver(&self) -> &TokioResolver {
&self.resolver
}
}
Expand Down
8 changes: 4 additions & 4 deletions dev-tools/omdb/tests/successes.out
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ task: "nat_v4_garbage_collector"
currently executing: no
last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }

task: "blueprint_loader"
configured period: every <REDACTED_DURATION>m <REDACTED_DURATION>s
Expand Down Expand Up @@ -523,7 +523,7 @@ task: "bfd_manager"
currently executing: no
last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }

task: "blueprint_planner"
configured period: every <REDACTED_DURATION>m
Expand Down Expand Up @@ -1010,7 +1010,7 @@ task: "nat_v4_garbage_collector"
currently executing: no
last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }

task: "blueprint_loader"
configured period: every <REDACTED_DURATION>m <REDACTED_DURATION>s
Expand Down Expand Up @@ -1049,7 +1049,7 @@ task: "bfd_manager"
currently executing: no
last completed activation: <REDACTED ITERATIONS>, triggered by a periodic timer firing
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
last completion reported error: failed to resolve addresses for Dendrite services: no record found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }
last completion reported error: failed to resolve addresses for Dendrite services: proto error: no records found for Query { name: Name("_dendrite._tcp.control-plane.oxide.internal."), query_type: SRV, query_class: IN }

task: "blueprint_planner"
configured period: every <REDACTED_DURATION>m
Expand Down
81 changes: 40 additions & 41 deletions dns-server/src/dns_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use hickory_proto::op::ResponseCode;
use hickory_proto::rr::RData;
use hickory_proto::rr::Record;
use hickory_proto::rr::RecordType;
use hickory_proto::rr::rdata::NS;
use hickory_proto::rr::rdata::SRV;
use hickory_proto::serialize::binary::BinDecodable;
use hickory_proto::serialize::binary::BinDecoder;
Expand Down Expand Up @@ -224,19 +225,11 @@ fn dns_record_to_record(
) -> Result<Record, RequestError> {
match record {
DnsRecord::A(addr) => {
let mut a = Record::new();
a.set_name(name.clone())
.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())))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

DnsRecord::Aaaa(addr) => {
let mut aaaa = Record::new();
aaaa.set_name(name.clone())
.set_rr_type(RecordType::AAAA)
.set_data(Some(RData::AAAA((*addr).into())));
Ok(aaaa)
Ok(Record::from_rdata(name.clone(), 0, RData::AAAA((*addr).into())))
}

DnsRecord::Srv(Srv { prio, weight, port, target }) => {
Expand All @@ -247,11 +240,11 @@ fn dns_record_to_record(
error
))
})?;
let mut srv = Record::new();
srv.set_name(name.clone()).set_rr_type(RecordType::SRV).set_data(
Some(RData::SRV(SRV::new(*prio, *weight, *port, tgt))),
);
Ok(srv)
Ok(Record::from_rdata(
name.clone(),
0,
RData::SRV(SRV::new(*prio, *weight, *port, tgt)),
))
}

DnsRecord::Ns(nsdname) => {
Expand All @@ -262,12 +255,7 @@ fn dns_record_to_record(
error
))
})?;
let mut ns = Record::new();
use hickory_proto::rr::rdata::NS;
ns.set_name(name.clone())
.set_rr_type(RecordType::NS)
.set_data(Some(RData::NS(NS(nsdname))));
Ok(ns)
Ok(Record::from_rdata(name.clone(), 0, RData::NS(NS(nsdname))))
}
}
}
Expand All @@ -287,9 +275,27 @@ async fn handle_dns_message(
// have to decide if the error is authoritative.
header.set_authoritative(true);

let query = mr.query();
let query = match mr.queries() {
[] => {
// A request with no queries could be legitimate (such as RFC 7873's Server Cookie
// query), but we won't look enough to find out because we don't support that.
return Err(RequestError::ServFail(anyhow!(
"request with no query?"
)));
}
[query] => query,
_ => {
// A request that is a query (opcode 0) with more than one question
// is invalid according to RFC 9619. We don't currently check that
// the opcode is actually QUERY, but even for other opcodes we don't
// support more than one question.
return Err(RequestError::ServFail(anyhow!(
"request with too many queries"
)));
}
};
let name = query.original().name().clone();
let answer = store.query(mr)?;
let answer = store.query(query)?;
let rb = MessageResponseBuilder::from_message_request(mr);
let mut additional_records = vec![];

Expand All @@ -315,29 +321,22 @@ async fn handle_dns_message(
if name_records.is_empty() {
return Err(RequestError::NxDomain(answer.queried_fqdn()));
}

let response_records = name_records
.into_iter()
.filter(|record| {
let ty = query.query_type();
if ty == RecordType::ANY {
return true;
}

match (ty, record.data()) {
(RecordType::A, Some(RData::A(_))) => true,
(RecordType::AAAA, Some(RData::AAAA(_))) => true,
(RecordType::SRV, Some(RData::SRV(_))) => true,
(RecordType::NS, Some(RData::NS(_))) => true,
(RecordType::SOA, Some(RData::SOA(_))) => true,
_ => false,
}
.filter(|record| match (query.query_type(), record.data()) {
(RecordType::ANY, _) => true,
(RecordType::A, RData::A(_)) => true,
(RecordType::AAAA, RData::AAAA(_)) => true,
(RecordType::SRV, RData::SRV(_)) => true,
(RecordType::NS, RData::NS(_)) => true,
(RecordType::SOA, RData::SOA(_)) => true,
_ => false,
})
.map(|record| {
// DNS allows for the server to return additional records that
// weren't explicitly asked for by the client but that the server
// expects the client will want. SRV and NS records both use names
// for their referents (rather than IP addresses dierctly). If
// for their referents (rather than IP addresses directly). If
// someone has queried for one of those kinds of records, they'll
// almost certainly be needing the IP addresses that go with them as
// well. We opportunistically attempt to resolve the target here and
Expand All @@ -346,8 +345,8 @@ async fn handle_dns_message(
// NOTE: we only do this one-layer deep. If the target of a SRV or
// NS is a CNAME instead of A/AAAA directly, it will be lost here.
let additionals_target = match record.data() {
Some(RData::SRV(srv)) => Some(srv.target()),
Some(RData::NS(ns)) => Some(&ns.0),
RData::SRV(srv) => Some(srv.target()),
RData::NS(ns) => Some(&ns.0),
_ => None,
};

Expand Down
27 changes: 15 additions & 12 deletions dns-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ pub mod http_server;
pub mod storage;

use anyhow::{Context, anyhow};
use hickory_resolver::TokioAsyncResolver;
use hickory_resolver::TokioResolver;
use hickory_resolver::config::NameServerConfig;
use hickory_resolver::config::Protocol;
use hickory_resolver::config::ResolverConfig;
use hickory_resolver::config::ResolverOpts;
use hickory_resolver::name_server::TokioConnectionProvider;
use internal_dns_types::config::DnsConfigParams;
use slog::o;
use std::net::SocketAddr;
Expand Down Expand Up @@ -179,20 +179,23 @@ impl TransientServer {
Ok(())
}

pub async fn resolver(&self) -> Result<TokioAsyncResolver, anyhow::Error> {
pub async fn resolver(&self) -> Result<TokioResolver, anyhow::Error> {
let mut resolver_config = ResolverConfig::new();
resolver_config.add_name_server(NameServerConfig {
socket_addr: self.dns_server.local_address(),
protocol: Protocol::Udp,
tls_dns_name: None,
trust_negative_responses: false,
bind_addr: None,
});
resolver_config.add_name_server(NameServerConfig::new(
self.dns_server.local_address(),
hickory_proto::xfer::Protocol::Udp,
));
let mut resolver_opts = ResolverOpts::default();
// Enable edns for potentially larger records
resolver_opts.edns0 = true;
let resolver =
TokioAsyncResolver::tokio(resolver_config, resolver_opts);

let resolver = TokioResolver::builder_with_config(
resolver_config,
TokioConnectionProvider::default(),
)
.with_options(resolver_opts)
.build();

Ok(resolver)
}
}
38 changes: 19 additions & 19 deletions dns-server/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@

use anyhow::{Context, anyhow};
use camino::Utf8PathBuf;
use hickory_proto::rr::LowerName;
use hickory_proto::{op::LowerQuery, rr::LowerName};
use hickory_resolver::Name;
use internal_dns_types::{
config::{DnsConfig, DnsConfigParams, DnsConfigZone, DnsRecord},
Expand Down Expand Up @@ -439,23 +439,23 @@ impl Store {
})
.map(name_from_str)??;

let mut record = hickory_proto::rr::Record::new();
let soa_name = name_from_str(&answer.queried_fqdn())?;
let rname = name_from_str(format!("admin.{}", answer.zone.as_str()))?;
record
.set_name(soa_name)
.set_rr_type(hickory_proto::rr::RecordType::SOA)
.set_data(Some(hickory_proto::rr::RData::SOA(
hickory_proto::rr::rdata::SOA::new(
preferred_nameserver,
rname,
answer.serial,
3600,
600,
1800,
600,
),
)));

let record = hickory_proto::rr::Record::from_rdata(
soa_name,
0,
hickory_proto::rr::RData::SOA(hickory_proto::rr::rdata::SOA::new(
preferred_nameserver,
rname,
answer.serial,
3600,
600,
1800,
600,
)),
);

Ok(record)
}

Expand Down Expand Up @@ -759,10 +759,10 @@ impl Store {
/// If the name does not match any zone, returns `QueryError::NoZone`.
pub(crate) fn query(
&self,
mr: &hickory_server::authority::MessageRequest,
query: &LowerQuery,
) -> Result<Answer, QueryError> {
let name = mr.query().name();
let orig_name = mr.query().original().name();
let name = query.name();
let orig_name = query.original().name();
self.query_raw(name, orig_name)
}

Expand Down
Loading
Loading