From f73e34e0d807a3fe4821c18b4c55b7b0b4b61d25 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Jun 2018 14:36:37 -0700 Subject: [PATCH] proxy: Update `dns` module to use new Trust-DNS `AsyncResolver` API (#1032) Depends on #974. Closes #859. This PR updates the proxy's `dns` module to use the new `AsyncResolver` API I added to `trust-dns-resolver` in bluejekyll/trust-dns#487. This allows us to spawn one `Future` that will drive DNS resolution in the background, rather than having to repeatedly clone a heavyweight `ResolverFuture` for every lookup. It also means that we no longer have to clone the name to resolve in quite as many places. Signed-off-by: Eliza Weisman --- Cargo.lock | 42 ++++++++++++++++++++++++----- proxy/Cargo.toml | 2 +- proxy/src/dns.rs | 69 +++++++++++++++++++++++++++++++++--------------- proxy/src/lib.rs | 5 ++-- 4 files changed, 86 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be8011f02..0712dad4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -102,6 +102,14 @@ dependencies = [ "time 0.1.39 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "cloudabi" +version = "0.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "bitflags 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "codegen" version = "0.1.0" @@ -153,7 +161,7 @@ dependencies = [ "tower-reconnect 0.1.0 (git+https://github.com/tower-rs/tower)", "tower-service 0.1.0 (git+https://github.com/tower-rs/tower)", "tower-util 0.1.0 (git+https://github.com/tower-rs/tower)", - "trust-dns-resolver 0.9.0 (git+https://github.com/bluejekyll/trust-dns?rev=5c966b86e7d847ad14344e494237e162dc635de0)", + "trust-dns-resolver 0.9.0 (git+https://github.com/bluejekyll/trust-dns)", "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "webpki 0.18.0-alpha3 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -766,6 +774,23 @@ dependencies = [ "winapi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rand" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "fuchsia-zircon 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.40 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_core 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "rand_core" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "redox_syscall" version = "0.1.37" @@ -1222,7 +1247,7 @@ dependencies = [ [[package]] name = "trust-dns-proto" version = "0.4.0" -source = "git+https://github.com/bluejekyll/trust-dns?rev=5c966b86e7d847ad14344e494237e162dc635de0#5c966b86e7d847ad14344e494237e162dc635de0" +source = "git+https://github.com/bluejekyll/trust-dns#660ca00c9b9ff0cbd675d3ff594bf072d87eb223" dependencies = [ "byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1230,7 +1255,7 @@ dependencies = [ "idna 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", - "rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "socket2 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-executor 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1245,7 +1270,7 @@ dependencies = [ [[package]] name = "trust-dns-resolver" version = "0.9.0" -source = "git+https://github.com/bluejekyll/trust-dns?rev=5c966b86e7d847ad14344e494237e162dc635de0#5c966b86e7d847ad14344e494237e162dc635de0" +source = "git+https://github.com/bluejekyll/trust-dns#660ca00c9b9ff0cbd675d3ff594bf072d87eb223" dependencies = [ "cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1257,7 +1282,7 @@ dependencies = [ "resolv-conf 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "tokio 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", - "trust-dns-proto 0.4.0 (git+https://github.com/bluejekyll/trust-dns?rev=5c966b86e7d847ad14344e494237e162dc635de0)", + "trust-dns-proto 0.4.0 (git+https://github.com/bluejekyll/trust-dns)", ] [[package]] @@ -1432,6 +1457,7 @@ dependencies = [ "checksum cc 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)" = "0ebb87d1116151416c0cf66a0e3fb6430cccd120fd6300794b4dfaa050ac40ba" "checksum cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de" "checksum chrono 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7c20ebe0b2b08b0aeddba49c609fe7957ba2e33449882cb186a180bc60682fa9" +"checksum cloudabi 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ddfc5b9aa5d4507acaf872de71051dfd0e309860e88966e1051e462a077aac4f" "checksum codegen 0.1.0 (git+https://github.com/carllerche/codegen)" = "" "checksum crc 1.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bd5d02c0aac6bd68393ed69e00bbc2457f3e89075c6349db7189618dc4ddc1d7" "checksum crossbeam-deque 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "fe8153ef04a7594ded05b427ffad46ddeaf22e63fd48d42b3e1e3bb4db07cae7" @@ -1500,6 +1526,8 @@ dependencies = [ "checksum quote 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "7a6e920b65c65f10b2ae65c831a81a073a89edd28c7cce89475bff467ab4167a" "checksum quote 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1eca14c727ad12702eb4b6bfb5a232287dcf8385cb8ca83a3eeaf6519c44c408" "checksum rand 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "eba5f8cb59cc50ed56be8880a5c7b496bfd9bd26394e176bc67884094145c2c5" +"checksum rand 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7a89abf8d34faf9783692392dca7bcdc6e82fa84eca86ccb6301ec87f3497185" +"checksum rand_core 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1b7a5f27547c49e5ccf8a586db3f3782fd93cf849780b21853b9d981db203302" "checksum redox_syscall 0.1.37 (registry+https://github.com/rust-lang/crates.io-index)" = "0d92eecebad22b767915e4d529f89f28ee96dbbf5a4810d2b844373f136417fd" "checksum redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7e891cfe48e9100a70a3b6eb652fef28920c117d366339687bd5576160db0f76" "checksum regex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "75ecf88252dce580404a22444fc7d626c01815debba56a7f4f536772a5ff19d3" @@ -1547,8 +1575,8 @@ dependencies = [ "checksum tower-reconnect 0.1.0 (git+https://github.com/tower-rs/tower)" = "" "checksum tower-service 0.1.0 (git+https://github.com/tower-rs/tower)" = "" "checksum tower-util 0.1.0 (git+https://github.com/tower-rs/tower)" = "" -"checksum trust-dns-proto 0.4.0 (git+https://github.com/bluejekyll/trust-dns?rev=5c966b86e7d847ad14344e494237e162dc635de0)" = "" -"checksum trust-dns-resolver 0.9.0 (git+https://github.com/bluejekyll/trust-dns?rev=5c966b86e7d847ad14344e494237e162dc635de0)" = "" +"checksum trust-dns-proto 0.4.0 (git+https://github.com/bluejekyll/trust-dns)" = "" +"checksum trust-dns-resolver 0.9.0 (git+https://github.com/bluejekyll/trust-dns)" = "" "checksum try-lock 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee2aa4715743892880f70885373966c83d73ef1b0838a664ef0c76fffd35e7c2" "checksum ucd-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "fd2be2d6639d0f8fe6cdda291ad456e23629558d466e2789d2c3e9892bda285d" "checksum unicode-bidi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "49f2bd0c6468a8230e1db229cff8029217cf623c767ea5d60bfbd42729ea54d5" diff --git a/proxy/Cargo.toml b/proxy/Cargo.toml index 99b5fd936..b1bba0d70 100644 --- a/proxy/Cargo.toml +++ b/proxy/Cargo.toml @@ -37,7 +37,7 @@ tokio-signal = "0.2" prost = "0.3.0" prost-types = "0.3.0" -trust-dns-resolver = { default-features = false, git = "https://github.com/bluejekyll/trust-dns", rev = "5c966b86e7d847ad14344e494237e162dc635de0" } +trust-dns-resolver = { default-features = false, git = "https://github.com/bluejekyll/trust-dns" } tokio-connect = { git = "https://github.com/carllerche/tokio-connect" } tower-service = { git = "https://github.com/tower-rs/tower" } diff --git a/proxy/src/dns.rs b/proxy/src/dns.rs index 58d5725fc..1e2f6aeda 100644 --- a/proxy/src/dns.rs +++ b/proxy/src/dns.rs @@ -4,18 +4,19 @@ use std::net::IpAddr; use std::time::Instant; use tokio::timer::Delay; use transport; -use trust_dns_resolver; -use trust_dns_resolver::config::{ResolverConfig, ResolverOpts}; -use trust_dns_resolver::error::{ResolveError, ResolveErrorKind}; -use trust_dns_resolver::ResolverFuture; -use trust_dns_resolver::lookup_ip::LookupIp; +use trust_dns_resolver::{ + self, + config::{ResolverConfig, ResolverOpts}, + error::{ResolveError, ResolveErrorKind}, + lookup_ip::LookupIp, + AsyncResolver, +}; use config::Config; -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct Resolver { - config: ResolverConfig, - opts: ResolverOpts, + resolver: AsyncResolver, } pub enum IpAddrFuture { @@ -60,23 +61,42 @@ impl AsRef for Name { } } + impl Resolver { /// Construct a new `Resolver` from the system configuration and Conduit's /// environment variables. /// + /// # Returns + /// + /// Either a tuple containing a new `Resolver` and the background task to + /// drive that resolver's futures, or an error if the system configuration + /// could not be parsed. + /// /// TODO: Make this infallible, like it is in the `domain` crate. - pub fn new(env_config: &Config) -> Result { + pub fn from_system_config_and_env(env_config: &Config) + -> Result<(Self, impl Future + Send), ResolveError> { let (config, opts) = trust_dns_resolver::system_conf::read_system_conf()?; - let mut opts = env_config.configure_resolver_opts(opts); - // Disable Trust-DNS's caching. - opts.cache_size = 0; + let opts = env_config.configure_resolver_opts(opts); trace!("DNS config: {:?}", &config); trace!("DNS opts: {:?}", &opts); - Ok(Resolver { - config, - opts, - }) + Ok(Self::new(config, opts)) + } + + + /// NOTE: It would be nice to be able to return a named type rather than + /// `impl Future` for the background future; it would be called + /// `Background` or `ResolverBackground` if that were possible. + pub fn new(config: ResolverConfig, mut opts: ResolverOpts) + -> (Self, impl Future + Send) + { + // Disable Trust-DNS's caching. + opts.cache_size = 0; + let (resolver, background) = AsyncResolver::new(config, opts); + let resolver = Resolver { + resolver, + }; + (resolver, background) } pub fn resolve_one_ip(&self, host: &transport::Host) -> IpAddrFuture { @@ -120,12 +140,17 @@ impl Resolver { fn lookup_ip(self, &Name(ref name): &Name) -> impl Future { - let name = name.clone(); // TODO: ref-count names. - let resolver = ResolverFuture::new( - self.config, - self.opts - ); - resolver.and_then(move |r| r.lookup_ip(name.as_str())) + self.resolver.lookup_ip(name.as_str()) + } +} + +/// Note: `AsyncResolver` does not implement `Debug`, so we must manually +/// implement this. +impl fmt::Debug for Resolver { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("Resolver") + .field("resolver", &"...") + .finish() } } diff --git a/proxy/src/lib.rs b/proxy/src/lib.rs index e0ce2f33a..fc21a8a21 100644 --- a/proxy/src/lib.rs +++ b/proxy/src/lib.rs @@ -213,7 +213,7 @@ where &taps, ); - let dns_resolver = dns::Resolver::new(&config) + let (dns_resolver, dns_bg) = dns::Resolver::from_system_config_and_env(&config) .unwrap_or_else(|e| { // TODO: Make DNS configuration infallible. panic!("invalid DNS configuration: {:?}", e); @@ -313,10 +313,11 @@ where let metrics_server = telemetry.serve_metrics(metrics_listener); let fut = ::logging::admin().bg("resolver").future(resolver_bg) - .join4( + .join5( ::logging::admin().bg("telemetry").future(telemetry), tap.map_err(|_| {}), metrics_server.map_err(|_| {}), + ::logging::admin().bg("dns-resolver").future(dns_bg), ).map(|_| {}); rt.spawn(Box::new(fut));