Expand SRV resolver to account for IPv6

Given the limitations of the dns resolver lib, to retrieve a service IP we currently take the SRV response target, which has the form `<host|IP>.svc.ns.svc.cluster-domain.`, and extract the first segment replacing dashes with dots. This fails for IPv6 address, so in this change we attempt parsing for IPv4 and if that fails fallback to IPv6 by instead replacing dots with colons. The `trust-dns-resolver` has been renamed to `hickory-resolver` upstream, so we follow suit and also upgrade the version.
linkerd · Apr 4, 2024 · a09cc25 · a09cc25
1 parent cf8ecb0
commit a09cc25
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 57 deletions.
diff --git a/Cargo.lock b/Cargo.lock
@@ -754,6 +754,51 @@ version = "0.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70"
 
+[[package]]
+name = "hickory-proto"
+version = "0.24.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "091a6fbccf4860009355e3efc52ff4acf37a63489aad7435372d44ceeb6fbbcf"
+dependencies = [
+ "async-trait",
+ "cfg-if",
+ "data-encoding",
+ "enum-as-inner",
+ "futures-channel",
+ "futures-io",
+ "futures-util",
+ "idna 0.4.0",
+ "ipnet",
+ "once_cell",
+ "rand",
+ "thiserror",
+ "tinyvec",
+ "tokio",
+ "tracing",
+ "url",
+]
+
+[[package]]
+name = "hickory-resolver"
+version = "0.24.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "35b8f021164e6a984c9030023544c57789c51760065cd510572fedcfb04164e8"
+dependencies = [
+ "cfg-if",
+ "futures-util",
+ "hickory-proto",
+ "ipconfig",
+ "lru-cache",
+ "once_cell",
+ "parking_lot",
+ "rand",
+ "resolv-conf",
+ "smallvec",
+ "thiserror",
+ "tokio",
+ "tracing",
+]
+
 [[package]]
 name = "home"
 version = "0.5.9"
@@ -1327,12 +1372,12 @@ name = "linkerd-dns"
 version = "0.1.0"
 dependencies = [
  "futures",
+ "hickory-resolver",
  "linkerd-dns-name",
  "linkerd-error",
  "thiserror",
  "tokio",
  "tracing",
- "trust-dns-resolver",
 ]
 
 [[package]]
@@ -3498,52 +3543,6 @@ dependencies = [
  "tracing-serde",
 ]
 
-[[package]]
-name = "trust-dns-proto"
-version = "0.23.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3119112651c157f4488931a01e586aa459736e9d6046d3bd9105ffb69352d374"
-dependencies = [
- "async-trait",
- "cfg-if",
- "data-encoding",
- "enum-as-inner",
- "futures-channel",
- "futures-io",
- "futures-util",
- "idna 0.4.0",
- "ipnet",
- "once_cell",
- "rand",
- "smallvec",
- "thiserror",
- "tinyvec",
- "tokio",
- "tracing",
- "url",
-]
-
-[[package]]
-name = "trust-dns-resolver"
-version = "0.23.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "10a3e6c3aff1718b3c73e395d1f35202ba2ffa847c6a62eea0db8fb4cfe30be6"
-dependencies = [
- "cfg-if",
- "futures-util",
- "ipconfig",
- "lru-cache",
- "once_cell",
- "parking_lot",
- "rand",
- "resolv-conf",
- "smallvec",
- "thiserror",
- "tokio",
- "tracing",
- "trust-dns-proto",
-]
-
 [[package]]
 name = "try-lock"
 version = "0.2.5"

diff --git a/linkerd/dns/Cargo.toml b/linkerd/dns/Cargo.toml
@@ -12,5 +12,5 @@ linkerd-dns-name = { path = "./name" }
 linkerd-error = { path = "../error" }
 thiserror = "1"
 tracing = "0.1"
-trust-dns-resolver = "0.23.2"
+hickory-resolver = "0.24.0"
 tokio = { version = "1", features = ["rt", "sync", "time"] }
diff --git a/linkerd/dns/src/lib.rs b/linkerd/dns/src/lib.rs
@@ -1,16 +1,16 @@
 #![deny(rust_2018_idioms, clippy::disallowed_methods, clippy::disallowed_types)]
 #![forbid(unsafe_code)]
 
+pub use hickory_resolver::config::ResolverOpts;
+use hickory_resolver::{
+    config::ResolverConfig, error, proto::rr::rdata, system_conf, AsyncResolver, TokioAsyncResolver,
+};
 use linkerd_dns_name::NameRef;
 pub use linkerd_dns_name::{InvalidName, Name, Suffix};
 use std::{fmt, net};
 use thiserror::Error;
 use tokio::time::{self, Instant};
 use tracing::{debug, trace};
-pub use trust_dns_resolver::config::ResolverOpts;
-use trust_dns_resolver::{
-    config::ResolverConfig, error, proto::rr::rdata, system_conf, AsyncResolver, TokioAsyncResolver,
-};
 
 #[derive(Clone)]
 pub struct Resolver {
@@ -127,16 +127,20 @@ impl Resolver {
     }
 
     // XXX We need to convert the SRV records to an IP addr manually,
-    // because of: https://github.com/bluejekyll/trust-dns/issues/872
+    // because of: https://github.com/hickory-dns/hickory-dns/issues/872
     // Here we rely in on the fact that the first label of the SRV
     // record's target will be the ip of the pod delimited by dashes
-    // instead of dots. We can alternatively do another lookup
+    // instead of dots/colons. We can alternatively do another lookup
     // on the pod's DNS but it seems unnecessary since the pod's
     // ip is in the target of the SRV record.
     fn srv_to_socket_addr(srv: rdata::SRV) -> Result<net::SocketAddr, InvalidSrv> {
         if let Some(first_label) = srv.target().iter().next() {
             if let Ok(utf8) = std::str::from_utf8(first_label) {
-                if let Ok(ip) = utf8.replace('-', ".").parse::<std::net::IpAddr>() {
+                let mut res = utf8.replace('-', ".").parse::<std::net::IpAddr>();
+                if res.is_err() {
+                    res = utf8.replace('-', ":").parse::<std::net::IpAddr>();
+                }
+                if let Ok(ip) = res {
                     return Ok(net::SocketAddr::new(ip, srv.port()));
                 }
             }
@@ -185,8 +189,9 @@ impl ResolveError {
 
 #[cfg(test)]
 mod tests {
-    use super::{Name, Suffix};
-    use std::str::FromStr;
+    use super::{Name, Resolver, Suffix};
+    use hickory_resolver::proto::rr::{domain, rdata};
+    use std::{net, str::FromStr};
 
     #[test]
     fn test_dns_name_parsing() {
@@ -289,6 +294,42 @@ mod tests {
 
         assert!(Suffix::from_str("").is_err(), "suffix must not be empty");
     }
+
+    #[test]
+    fn srv_to_socket_addr_invalid() {
+        let name = "foobar.linkerd-dst-headless.linkerd.svc.cluster.local.";
+        let target = domain::Name::from_str(name).unwrap();
+        let srv = rdata::SRV::new(1, 1, 8086, target);
+        assert!(Resolver::srv_to_socket_addr(srv).is_err());
+    }
+
+    #[test]
+    fn srv_to_socket_addr_valid() {
+        struct Case {
+            input: &'static str,
+            output: &'static str,
+        }
+
+        for case in &[
+            Case {
+                input: "10-42-0-15.linkerd-dst-headless.linkerd.svc.cluster.local.",
+                output: "10.42.0.15",
+            },
+            Case {
+                input: "2001-0db8-0000-0000-0000-ff00-0042-8329.linkerd-dst-headless.linkerd.svc.cluster.local.",
+                output: "2001:0db8:0000:0000:0000:ff00:0042:8329",
+            },
+            Case {
+                input: "2001-0db8--0042-8329.linkerd-dst-headless.linkerd.svc.cluster.local.",
+                output: "2001:0db8::0042:8329",
+            },
+        ] {
+            let target = domain::Name::from_str(case.input).unwrap();
+            let srv = rdata::SRV::new(1, 1, 8086, target);
+            let socket = Resolver::srv_to_socket_addr(srv).unwrap();
+            assert_eq!(socket.ip(), net::IpAddr::from_str(case.output).unwrap());
+        }
+    }
 }
 
 #[cfg(fuzzing)]