From 9689568974ef88253932e9b2f942bda3315ae7cc Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 29 Feb 2024 18:42:15 +0100 Subject: [PATCH] refactor Implementation branching into its own module --- .../dns-test/src/docker/hickory.Dockerfile | 3 +- packages/dns-test/src/implementation.rs | 190 ++++++++++++++++++ packages/dns-test/src/lib.rs | 53 +---- packages/dns-test/src/name_server.rs | 81 ++------ packages/dns-test/src/resolver.rs | 63 ++---- .../src/templates/hickory.resolver.toml.jinja | 2 +- .../src/templates/named.resolver.conf.jinja | 2 +- .../dns-test/src/templates/nsd.conf.jinja | 3 + .../dns-test/src/templates/unbound.conf.jinja | 3 +- 9 files changed, 233 insertions(+), 167 deletions(-) create mode 100644 packages/dns-test/src/implementation.rs diff --git a/packages/dns-test/src/docker/hickory.Dockerfile b/packages/dns-test/src/docker/hickory.Dockerfile index 358b7a93..77206244 100644 --- a/packages/dns-test/src/docker/hickory.Dockerfile +++ b/packages/dns-test/src/docker/hickory.Dockerfile @@ -8,5 +8,6 @@ RUN apt-get update && \ # a clone of the hickory repository. `./src` here refers to that clone; not to # any directory inside the `dns-test` repository COPY ./src /usr/src/hickory -RUN cargo install --path /usr/src/hickory/bin --features recursor --debug +RUN cargo install --path /usr/src/hickory/bin --features recursor --debug && \ + mkdir /etc/hickory env RUST_LOG=debug diff --git a/packages/dns-test/src/implementation.rs b/packages/dns-test/src/implementation.rs new file mode 100644 index 00000000..256ad247 --- /dev/null +++ b/packages/dns-test/src/implementation.rs @@ -0,0 +1,190 @@ +use core::fmt; +use std::borrow::Cow; +use std::path::Path; + +use url::Url; + +use crate::FQDN; + +#[derive(Clone, Copy)] +pub enum Config<'a> { + NameServer { origin: &'a FQDN }, + Resolver { use_dnssec: bool, netmask: &'a str }, +} + +impl Config<'_> { + pub fn role(&self) -> Role { + match self { + Config::NameServer { .. } => Role::NameServer, + Config::Resolver { .. } => Role::Resolver, + } + } +} + +#[derive(Clone, Copy)] +pub enum Role { + NameServer, + Resolver, +} + +impl Role { + #[must_use] + pub fn is_resolver(&self) -> bool { + matches!(self, Self::Resolver) + } +} + +#[derive(Clone)] +pub enum Implementation { + Bind, + Hickory(Repository<'static>), + Unbound, +} + +impl Implementation { + #[must_use] + pub fn is_bind(&self) -> bool { + matches!(self, Self::Bind) + } + + pub(crate) fn format_config(&self, config: Config) -> String { + match config { + Config::Resolver { + use_dnssec, + netmask, + } => match self { + Self::Bind => { + minijinja::render!( + include_str!("templates/named.resolver.conf.jinja"), + use_dnssec => use_dnssec, + netmask => netmask, + ) + } + + Self::Hickory(_) => { + minijinja::render!( + include_str!("templates/hickory.resolver.toml.jinja"), + use_dnssec => use_dnssec, + ) + } + + Self::Unbound => { + minijinja::render!( + include_str!("templates/unbound.conf.jinja"), + use_dnssec => use_dnssec, + netmask => netmask, + ) + } + }, + + Config::NameServer { origin } => match self { + Self::Bind => { + minijinja::render!( + include_str!("templates/named.name-server.conf.jinja"), + fqdn => origin.as_str() + ) + } + + Self::Unbound => { + minijinja::render!( + include_str!("templates/nsd.conf.jinja"), + fqdn => origin.as_str() + ) + } + + Self::Hickory(_) => unimplemented!(), + }, + } + } + + pub(crate) fn conf_file_path(&self, role: Role) -> &'static str { + match self { + Self::Bind => "/etc/bind/named.conf", + + Self::Hickory(_) => "/etc/named.toml", + + Self::Unbound => match role { + Role::NameServer => "/etc/nsd/nsd.conf", + Role::Resolver => "/etc/unbound/unbound.conf", + }, + } + } + + pub(crate) fn cmd_args(&self, role: Role) -> &[&'static str] { + match self { + Implementation::Bind => &["named", "-g", "-d5"], + + Implementation::Hickory(_) => { + assert!( + role.is_resolver(), + "hickory acting in `NameServer` role is currently not supported" + ); + + &["hickory-dns", "-d"] + } + + Implementation::Unbound => match role { + Role::NameServer => &["nsd", "-d"], + + Role::Resolver => &["unbound", "-d"], + }, + } + } + + pub(crate) fn pidfile(&self, role: Role) -> &'static str { + match self { + Implementation::Bind => "/tmp/named.pid", + + Implementation::Hickory(_) => unimplemented!(), + + Implementation::Unbound => match role { + Role::NameServer => "/tmp/nsd.pid", + Role::Resolver => "/tmp/unbound.pid", + }, + } + } +} + +impl fmt::Display for Implementation { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let s = match self { + Implementation::Bind => "bind", + Implementation::Hickory(_) => "hickory", + Implementation::Unbound => "unbound", + }; + + f.write_str(s) + } +} + +#[derive(Clone)] +pub struct Repository<'a> { + inner: Cow<'a, str>, +} + +impl Repository<'_> { + pub(crate) fn as_str(&self) -> &str { + &self.inner + } +} + +/// checks that `input` looks like a valid repository which can be either local or remote +/// +/// # Panics +/// +/// this function panics if `input` is not a local `Path` that exists or a well-formed URL +#[allow(non_snake_case)] +pub fn Repository(input: impl Into>) -> Repository<'static> { + let input = input.into(); + assert!( + Path::new(&*input).exists() || Url::parse(&input).is_ok(), + "{input} is not a valid repository" + ); + Repository { inner: input } +} + +impl Default for Implementation { + fn default() -> Self { + Self::Unbound + } +} diff --git a/packages/dns-test/src/lib.rs b/packages/dns-test/src/lib.rs index eb7fe63a..cfbcb96f 100644 --- a/packages/dns-test/src/lib.rs +++ b/packages/dns-test/src/lib.rs @@ -1,18 +1,15 @@ //! A test framework for all things DNS -use std::borrow::Cow; -use std::path::Path; - -use url::Url; - pub use crate::container::Network; pub use crate::fqdn::FQDN; +pub use crate::implementation::{Implementation, Repository}; pub use crate::resolver::Resolver; pub use crate::trust_anchor::TrustAnchor; pub mod client; mod container; mod fqdn; +mod implementation; pub mod name_server; pub mod record; mod resolver; @@ -26,52 +23,6 @@ pub type Result = core::result::Result; // TODO maybe this should be a TLS variable that each unit test (thread) can override const DEFAULT_TTL: u32 = 24 * 60 * 60; // 1 day -#[derive(Clone)] -pub enum Implementation { - Bind, - Hickory(Repository<'static>), - Unbound, -} - -impl Implementation { - #[must_use] - pub fn is_bind(&self) -> bool { - matches!(self, Self::Bind) - } -} - -#[derive(Clone)] -pub struct Repository<'a> { - inner: Cow<'a, str>, -} - -impl Repository<'_> { - fn as_str(&self) -> &str { - &self.inner - } -} - -/// checks that `input` looks like a valid repository which can be either local or remote -/// -/// # Panics -/// -/// this function panics if `input` is not a local `Path` that exists or a well-formed URL -#[allow(non_snake_case)] -pub fn Repository(input: impl Into>) -> Repository<'static> { - let input = input.into(); - assert!( - Path::new(&*input).exists() || Url::parse(&input).is_ok(), - "{input} is not a valid repository" - ); - Repository { inner: input } -} - -impl Default for Implementation { - fn default() -> Self { - Self::Unbound - } -} - pub fn subject() -> Implementation { if let Ok(subject) = std::env::var("DNS_TEST_SUBJECT") { if subject == "unbound" { diff --git a/packages/dns-test/src/name_server.rs b/packages/dns-test/src/name_server.rs index 81ce4d70..eea06b21 100644 --- a/packages/dns-test/src/name_server.rs +++ b/packages/dns-test/src/name_server.rs @@ -2,6 +2,7 @@ use core::sync::atomic::{self, AtomicUsize}; use std::net::Ipv4Addr; use crate::container::{Child, Container, Network}; +use crate::implementation::{Config, Role}; use crate::record::{self, Record, SoaSettings, DS, SOA}; use crate::tshark::Tshark; use crate::zone_file::{self, ZoneFile}; @@ -147,30 +148,19 @@ impl NameServer { state: _, } = self; - let origin = zone_file.origin(); - let (path, contents, cmd_args) = match &implementation { - Implementation::Bind => ( - "/etc/bind/named.conf", - named_conf(origin), - &["named", "-g", "-d5"][..], - ), - - Implementation::Unbound => { - // for PID file - container.status_ok(&["mkdir", "-p", "/run/nsd/"])?; - - ("/etc/nsd/nsd.conf", nsd_conf(origin), &["nsd", "-d"][..]) - } - - Implementation::Hickory(_) => unreachable!(), + let config = Config::NameServer { + origin: zone_file.origin(), }; - container.cp(path, &contents)?; + container.cp( + implementation.conf_file_path(config.role()), + &implementation.format_config(config), + )?; container.status_ok(&["mkdir", "-p", ZONES_DIR])?; container.cp(&zone_file_path(), &zone_file.to_string())?; - let child = container.spawn(cmd_args)?; + let child = container.spawn(implementation.cmd_args(config.role()))?; Ok(NameServer { container, @@ -203,32 +193,16 @@ impl NameServer { state, } = self; - let (conf_path, conf_contents, cmd_args) = match implementation { - Implementation::Bind => ( - "/etc/bind/named.conf", - named_conf(zone_file.origin()), - &["named", "-g", "-d5"][..], - ), - - Implementation::Unbound => { - // for PID file - container.status_ok(&["mkdir", "-p", "/run/nsd/"])?; - - ( - "/etc/nsd/nsd.conf", - nsd_conf(zone_file.origin()), - &["nsd", "-d"][..], - ) - } - - Implementation::Hickory(..) => unreachable!(), + let config = Config::NameServer { + origin: zone_file.origin(), }; - - container.cp(conf_path, &conf_contents)?; - + container.cp( + implementation.conf_file_path(config.role()), + &implementation.format_config(config), + )?; container.cp(&zone_file_path(), &state.signed.to_string())?; - let child = container.spawn(cmd_args)?; + let child = container.spawn(implementation.cmd_args(config.role()))?; Ok(NameServer { container, @@ -267,13 +241,8 @@ impl NameServer { /// gracefully terminates the name server collecting all logs pub fn terminate(self) -> Result { - let pidfile = match &self.implementation { - Implementation::Bind => "/tmp/named.pid", + let pidfile = self.implementation.pidfile(Role::NameServer); - Implementation::Unbound => "/run/nsd/nsd.pid", - - Implementation::Hickory(_) => unreachable!(), - }; // if `terminate` is called right after `start` NSD may not have had the chance to create // the PID file so if it doesn't exist wait for a bit before invoking `kill` let kill = format!( @@ -284,7 +253,9 @@ kill -TERM $(cat {pidfile})" let output = self.state.child.wait()?; if !output.status.success() { - return Err("could not terminate the `unbound` process".into()); + return Err( + format!("could not terminate the `{}` process", self.implementation).into(), + ); } assert!( @@ -339,20 +310,6 @@ fn admin_ns(ns_count: usize) -> FQDN { FQDN(format!("admin{ns_count}.nameservers.com.")).unwrap() } -fn named_conf(fqdn: &FQDN) -> String { - minijinja::render!( - include_str!("templates/named.name-server.conf.jinja"), - fqdn => fqdn.as_str() - ) -} - -fn nsd_conf(fqdn: &FQDN) -> String { - minijinja::render!( - include_str!("templates/nsd.conf.jinja"), - fqdn => fqdn.as_str() - ) -} - #[cfg(test)] mod tests { use crate::client::{Client, DigSettings}; diff --git a/packages/dns-test/src/resolver.rs b/packages/dns-test/src/resolver.rs index efa6da29..4460a198 100644 --- a/packages/dns-test/src/resolver.rs +++ b/packages/dns-test/src/resolver.rs @@ -2,6 +2,7 @@ use core::fmt::Write; use std::net::Ipv4Addr; use crate::container::{Child, Container, Network}; +use crate::implementation::{Config, Role}; use crate::trust_anchor::TrustAnchor; use crate::tshark::Tshark; use crate::zone_file::Root; @@ -40,34 +41,17 @@ impl Resolver { writeln!(hints, "{root}").unwrap(); } + container.cp("/etc/root.hints", &hints)?; + let use_dnssec = !trust_anchor.is_empty(); - match implementation { - Implementation::Bind => { - container.cp("/etc/bind/root.hints", &hints)?; - - container.cp( - "/etc/bind/named.conf", - &named_conf(use_dnssec, network.netmask()), - )?; - } - - Implementation::Unbound => { - container.cp("/etc/unbound/root.hints", &hints)?; - - container.cp( - "/etc/unbound/unbound.conf", - &unbound_conf(use_dnssec, network.netmask()), - )?; - } - - Implementation::Hickory { .. } => { - container.status_ok(&["mkdir", "-p", "/etc/hickory"])?; - - container.cp("/etc/hickory/root.hints", &hints)?; - - container.cp("/etc/named.toml", &hickory_conf(use_dnssec))?; - } - } + let config = Config::Resolver { + use_dnssec, + netmask: network.netmask(), + }; + container.cp( + implementation.conf_file_path(config.role()), + &implementation.format_config(config), + )?; if use_dnssec { let path = if implementation.is_bind() { @@ -85,12 +69,7 @@ impl Resolver { container.cp(path, &contents)?; } - let command: &[_] = match implementation { - Implementation::Bind => &["named", "-g", "-d5"], - Implementation::Unbound => &["unbound", "-d"], - Implementation::Hickory { .. } => &["hickory-dns", "-d"], - }; - let child = container.spawn(command)?; + let child = container.spawn(implementation.cmd_args(config.role()))?; Ok(Self { child, @@ -113,11 +92,7 @@ impl Resolver { /// gracefully terminates the name server collecting all logs pub fn terminate(self) -> Result { - let pidfile = match self.implementation { - Implementation::Bind => "/tmp/named.pid", - Implementation::Unbound => "/run/unbound.pid", - Implementation::Hickory(..) => unimplemented!(), - }; + let pidfile = self.implementation.pidfile(Role::Resolver); let kill = format!( "test -f {pidfile} || sleep 1 kill -TERM $(cat {pidfile})" @@ -137,18 +112,6 @@ kill -TERM $(cat {pidfile})" } } -fn named_conf(use_dnssec: bool, netmask: &str) -> String { - minijinja::render!(include_str!("templates/named.resolver.conf.jinja"), use_dnssec => use_dnssec, netmask => netmask) -} - -fn unbound_conf(use_dnssec: bool, netmask: &str) -> String { - minijinja::render!(include_str!("templates/unbound.conf.jinja"), use_dnssec => use_dnssec, netmask => netmask) -} - -fn hickory_conf(use_dnssec: bool) -> String { - minijinja::render!(include_str!("templates/hickory.resolver.toml.jinja"), use_dnssec => use_dnssec) -} - #[cfg(test)] mod tests { use crate::{name_server::NameServer, FQDN}; diff --git a/packages/dns-test/src/templates/hickory.resolver.toml.jinja b/packages/dns-test/src/templates/hickory.resolver.toml.jinja index d3da6496..418db8fd 100644 --- a/packages/dns-test/src/templates/hickory.resolver.toml.jinja +++ b/packages/dns-test/src/templates/hickory.resolver.toml.jinja @@ -1,5 +1,5 @@ [[zones]] zone = "." zone_type = "Hint" -stores = { type = "recursor", roots = "/etc/hickory/root.hints", ns_cache_size = 1024, record_cache_size = 1048576 } +stores = { type = "recursor", roots = "/etc/root.hints", ns_cache_size = 1024, record_cache_size = 1048576 } enable_dnssec = {{ use_dnssec }} diff --git a/packages/dns-test/src/templates/named.resolver.conf.jinja b/packages/dns-test/src/templates/named.resolver.conf.jinja index 4b57908d..cc609492 100644 --- a/packages/dns-test/src/templates/named.resolver.conf.jinja +++ b/packages/dns-test/src/templates/named.resolver.conf.jinja @@ -10,5 +10,5 @@ options { zone "." { type hint; - file "/etc/bind/root.hints"; + file "/etc/root.hints"; }; diff --git a/packages/dns-test/src/templates/nsd.conf.jinja b/packages/dns-test/src/templates/nsd.conf.jinja index 36dea4fc..7a5b0ec3 100644 --- a/packages/dns-test/src/templates/nsd.conf.jinja +++ b/packages/dns-test/src/templates/nsd.conf.jinja @@ -1,3 +1,6 @@ +server: + pidfile: /tmp/nsd.pid + remote-control: control-enable: no diff --git a/packages/dns-test/src/templates/unbound.conf.jinja b/packages/dns-test/src/templates/unbound.conf.jinja index ca5e54d3..212078aa 100644 --- a/packages/dns-test/src/templates/unbound.conf.jinja +++ b/packages/dns-test/src/templates/unbound.conf.jinja @@ -3,7 +3,8 @@ server: use-syslog: no interface: 0.0.0.0 access-control: {{ netmask }} allow - root-hints: /etc/unbound/root.hints + root-hints: /etc/root.hints + pidfile: /tmp/unbound.pid {% if use_dnssec %} trust-anchor-file: /etc/trusted-key.key {% endif %}