Skip to content

Commit

Permalink
live-tests: avoid relying on poorly-behaving DNS (#7267)
Browse files Browse the repository at this point in the history
1. Live tests don't currently run because the check that you are on an
underlay network asks internal DNS for A/AAAA records for internal DNS
(via `hickory_resolver::Resolver::lookup_ip`). We don't have those, but
prior to #6308 we responded with whatever records we had for a given
name regardless of what type the query asked for. Live tests aren't run
in CI so we missed it.
2. `internal_dns_resolver::Resolver::lookup_ip` is now only used here
and in a unit test that is arguably doing something similar, so we
remove the function altogether.
  • Loading branch information
iliana authored Dec 18, 2024
1 parent 4a4d6ca commit ad61b01
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 17 deletions.
18 changes: 2 additions & 16 deletions internal-dns/resolver/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use omicron_common::address::{
get_internal_dns_server_addresses, Ipv6Subnet, AZ_PREFIX, DNS_PORT,
};
use slog::{debug, error, info, trace};
use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV6};
use std::net::{Ipv6Addr, SocketAddr, SocketAddrV6};

#[derive(Debug, Clone, thiserror::Error)]
pub enum ResolveError {
Expand Down Expand Up @@ -323,20 +323,6 @@ impl Resolver {
}
}

pub async fn lookup_ip(
&self,
srv: ServiceName,
) -> Result<IpAddr, ResolveError> {
let name = srv.srv_name();
debug!(self.log, "lookup srv"; "dns_name" => &name);
let response = self.resolver.lookup_ip(&name).await?;
let address = response
.iter()
.next()
.ok_or_else(|| ResolveError::NotFound(srv))?;
Ok(address)
}

/// Returns an iterator of [`SocketAddrV6`]'s for the targets of the given
/// SRV lookup response.
// SRV records have a target, which is itself another DNS name that needs
Expand Down Expand Up @@ -534,7 +520,7 @@ mod test {
let resolver = dns_server.resolver().unwrap();

let err = resolver
.lookup_ip(ServiceName::Cockroach)
.lookup_srv(ServiceName::Cockroach)
.await
.expect_err("Looking up non-existent service should fail");

Expand Down
2 changes: 1 addition & 1 deletion live-tests/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ async fn check_execution_environment(
// The only real requirement for these tests is that they're run from a
// place with connectivity to the underlay network of a deployed control
// plane. The easiest way to tell is to look up something in internal DNS.
resolver.lookup_ip(ServiceName::InternalDns).await.map_err(|e| {
resolver.lookup_srv(ServiceName::InternalDns).await.map_err(|e| {
let text = format!(
"check_execution_environment(): failed to look up internal DNS \
in the internal DNS servers.\n\n \
Expand Down

0 comments on commit ad61b01

Please sign in to comment.