Add a FromStr for dns::Name (#746)

We use `TryFrom<&[u8]>` in many cases we can use `FromStr`.
This is a minor cleanup of these type coersions.

Author: olix0r

linkerd/addr/src/lib.rs

@@ -2,10 +2,11 @@
 
 use http;
 use linkerd2_dns_name::Name;
-use std::convert::TryFrom;
-use std::fmt;
-use std::net::{IpAddr, SocketAddr};
-use std::str::FromStr;
+use std::{
+    fmt,
+    net::{IpAddr, SocketAddr},
+    str::FromStr,
+};
 
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub enum Addr {
@@ -165,7 +166,7 @@ impl NameAddr {
             return Err(Error::InvalidHost);
         }
 
-        Name::try_from(host.as_bytes())
+        Name::from_str(host)
             .map(|name| NameAddr { name, port })
             .map_err(|_| Error::InvalidHost)
     }

linkerd/app/gateway/src/lib.rs

@@ -19,7 +19,7 @@ mod test {
         Error, NameAddr, NameMatch, Never,
     };
     use linkerd2_app_inbound::endpoint as inbound;
-    use std::{convert::TryFrom, net::SocketAddr};
+    use std::{net::SocketAddr, str::FromStr};
     use tower::util::{service_fn, ServiceExt};
     use tower_test::mock;
 
@@ -111,7 +111,7 @@ mod test {
                 suffix: "test.example.com",
                 dst_name: Some("dst.test.example.com:4321"),
                 peer_id: tls::PeerIdentity::Some(identity::Name::from(
-                    dns::Name::try_from("client.id.test".as_bytes()).unwrap(),
+                    dns::Name::from_str("client.id.test").unwrap(),
                 )),
                 orig_fwd: None,
             }
@@ -140,12 +140,12 @@ mod test {
                     tokio::spawn(async move { tx.closed().await });
                     Ok::<_, Never>(Some(rx))
                 });
-                let allow_discovery = NameMatch::new(Some(dns::Suffix::try_from(suffix).unwrap()));
+                let allow_discovery = NameMatch::new(Some(dns::Suffix::from_str(suffix).unwrap()));
                 Config { allow_discovery }.build(
                     move |_: _| outbound.clone(),
                     profiles,
                     tls::PeerIdentity::Some(identity::Name::from(
-                        dns::Name::try_from("gateway.id.test".as_bytes()).unwrap(),
+                        dns::Name::from_str("gateway.id.test").unwrap(),
                     )),
                 )
             };

linkerd/app/outbound/src/tcp/tests.rs

@@ -103,10 +103,9 @@ async fn tls_when_hinted() {
     };
 
     let cfg = default_config(plain_addr);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is valid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is valid");
     let mut srv_io = support::io();
     srv_io.write(b"hello").read(b"world");
     let id_name2 = id_name.clone();
@@ -173,10 +172,9 @@ async fn resolutions_are_reused() {
 
     let addr = SocketAddr::new([0, 0, 0, 0].into(), 5550);
     let cfg = default_config(addr);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is valid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is valid");
 
     // Build a mock "connector" that returns the upstream "server" IO.
     let connect = support::connect().endpoint(
@@ -254,10 +252,9 @@ async fn load_balances() {
     ];
 
     let cfg = default_config(svc_addr);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is valid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is valid");
 
     // Build a mock "connector" that returns the upstream "server" IO
     let mut connect = support::connect();
@@ -345,10 +342,9 @@ async fn load_balancer_add_endpoints() {
     ];
 
     let cfg = default_config(svc_addr);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is valid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is valid");
 
     let mut connect = support::connect();
     for &(addr, ref conns) in endpoints {
@@ -454,10 +450,9 @@ async fn load_balancer_remove_endpoints() {
     ];
 
     let cfg = default_config(svc_addr);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is valid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is valid");
 
     let mut connect = support::connect();
     for &(addr, ref enabled) in endpoints {
@@ -544,10 +539,9 @@ async fn no_profiles_when_outside_search_nets() {
         allow_discovery: IpMatch::new(Some(IpNet::from_str("10.0.0.0/8").unwrap())).into(),
         ..default_config(profile_addr)
     };
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is invalid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is invalid");
     let id_name2 = id_name.clone();
 
     // Build a mock "connector" that returns the upstream "server" IO.
@@ -617,10 +611,9 @@ async fn no_discovery_when_profile_has_an_endpoint() {
 
     let ep = SocketAddr::new([10, 0, 0, 41].into(), 5550);
     let cfg = default_config(ep);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is invalid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is invalid");
     let meta = support::resolver::Metadata::new(
         Default::default(),
         support::resolver::ProtocolHint::Unknown,
@@ -672,10 +665,9 @@ async fn profile_endpoint_propagates_conn_errors() {
     let ep2 = SocketAddr::new([10, 0, 0, 42].into(), 5550);
 
     let cfg = default_config(ep1);
-    let id_name = linkerd2_identity::Name::from_hostname(
-        b"foo.ns1.serviceaccount.identity.linkerd.cluster.local",
-    )
-    .expect("hostname is invalid");
+    let id_name =
+        linkerd2_identity::Name::from_str("foo.ns1.serviceaccount.identity.linkerd.cluster.local")
+            .expect("hostname is invalid");
     let meta = support::resolver::Metadata::new(
         Default::default(),
         support::resolver::ProtocolHint::Unknown,

linkerd/app/src/env.rs

@@ -9,8 +9,7 @@ use crate::core::{
 use crate::{dns, gateway, identity, inbound, oc_collector, outbound};
 use indexmap::IndexSet;
 use std::{
-    collections::HashMap, convert::TryFrom, fmt, fs, net::SocketAddr, path::PathBuf, str::FromStr,
-    time::Duration,
+    collections::HashMap, fmt, fs, net::SocketAddr, path::PathBuf, str::FromStr, time::Duration,
 };
 use tracing::{error, warn};
 
@@ -768,7 +767,7 @@ fn parse_port_set(s: &str) -> Result<IndexSet<u16>, ParseError> {
 }
 
 pub(super) fn parse_identity(s: &str) -> Result<identity::Name, ParseError> {
-    identity::Name::from_hostname(s.as_bytes()).map_err(|identity::InvalidName| {
+    identity::Name::from_str(s).map_err(|identity::InvalidName| {
         error!("Not a valid identity name: {}", s);
         ParseError::NameError
     })
@@ -835,9 +834,7 @@ fn parse_dns_suffix(s: &str) -> Result<dns::Suffix, ParseError> {
         return Ok(dns::Suffix::Root);
     }
 
-    dns::Name::try_from(s.as_bytes())
-        .map(dns::Suffix::Name)
-        .map_err(|_| ParseError::NotADomainSuffix)
+    dns::Suffix::from_str(s).map_err(|_| ParseError::NotADomainSuffix)
 }
 
 fn parse_networks(list: &str) -> Result<IndexSet<ipnet::IpNet>, ParseError> {

linkerd/app/test/src/profile.rs

@@ -8,8 +8,8 @@ where
 }
 
 pub fn with_name(name: &str) -> Profile {
-    use std::convert::TryFrom;
-    let name = dns::Name::try_from(name.as_bytes()).expect("non-ascii characters in DNS name! 😢");
+    use std::str::FromStr;
+    let name = dns::Name::from_str(name).expect("non-ascii characters in DNS name! 😢");
     Profile {
         name: Some(name),
         ..Default::default()

linkerd/dns/name/src/name.rs

@@ -53,6 +53,13 @@ impl<'a> TryFrom<&'a [u8]> for Name {
     }
 }
 
+impl<'a> std::str::FromStr for Name {
+    type Err = InvalidName;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Self::try_from(s.as_bytes())
+    }
+}
+
 impl Into<webpki::DNSName> for Name {
     fn into(self) -> webpki::DNSName {
         self.0

linkerd/dns/name/src/suffix.rs

@@ -1,6 +1,5 @@
 use crate::Name;
-use std::convert::TryFrom;
-use std::fmt;
+use std::{fmt, str::FromStr};
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub enum Suffix {
@@ -23,14 +22,14 @@ impl From<Name> for Suffix {
     }
 }
 
-impl<'s> TryFrom<&'s str> for Suffix {
-    type Error = <Name as TryFrom<&'s [u8]>>::Error;
+impl FromStr for Suffix {
+    type Err = <Name as FromStr>::Err;
 
-    fn try_from(s: &str) -> Result<Self, Self::Error> {
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         if s == "." {
             Ok(Suffix::Root)
         } else {
-            Name::try_from(s.as_bytes()).map(|n| n.into())
+            Name::from_str(s).map(Suffix::Name)
         }
     }
 }

linkerd/dns/src/lib.rs

@@ -173,7 +173,7 @@ impl std::error::Error for InvalidSrv {}
 #[cfg(test)]
 mod tests {
     use super::{Name, Suffix};
-    use std::convert::TryFrom;
+    use std::str::FromStr;
 
     #[test]
     fn test_dns_name_parsing() {
@@ -225,7 +225,7 @@ mod tests {
         ];
 
         for case in VALID {
-            let name = Name::try_from(case.input.as_bytes());
+            let name = Name::from_str(&case.input);
             assert_eq!(name.as_ref().map(|x| x.as_ref()), Ok(case.output));
         }
 
@@ -237,7 +237,7 @@ mod tests {
         ];
 
         for case in INVALID {
-            assert!(Name::try_from(case.as_bytes()).is_err());
+            assert!(Name::from_str(case).is_err());
         }
     }
 
@@ -255,8 +255,8 @@ mod tests {
             ("a.b.c.", "b.c"),
             ("hacker.example.com", "example.com"),
         ] {
-            let n = Name::try_from((*name).as_bytes()).unwrap();
-            let s = Suffix::try_from(*suffix).unwrap();
+            let n = Name::from_str(name).unwrap();
+            let s = Suffix::from_str(suffix).unwrap();
             assert!(
                 s.contains(&n),
                 format!("{} should contain {}", suffix, name)
@@ -272,14 +272,14 @@ mod tests {
             ("b.a", "b"),
             ("hackerexample.com", "example.com"),
         ] {
-            let n = Name::try_from((*name).as_bytes()).unwrap();
-            let s = Suffix::try_from(*suffix).unwrap();
+            let n = Name::from_str(name).unwrap();
+            let s = Suffix::from_str(suffix).unwrap();
             assert!(
                 !s.contains(&n),
                 format!("{} should not contain {}", suffix, name)
             );
         }
 
-        assert!(Suffix::try_from("").is_err(), "suffix must not be empty");
+        assert!(Suffix::from_str("").is_err(), "suffix must not be empty");
     }
 }

linkerd/identity/src/lib.rs

@@ -4,11 +4,7 @@ use linkerd2_dns_name;
 pub use ring::error::KeyRejected;
 use ring::rand;
 use ring::signature::EcdsaKeyPair;
-use std::convert::TryFrom;
-use std::error::Error;
-use std::sync::Arc;
-use std::time::SystemTime;
-use std::{fmt, fs, io};
+use std::{convert::TryFrom, error::Error, fmt, fs, io, str::FromStr, sync::Arc, time::SystemTime};
 use tracing::{debug, warn};
 
 #[cfg(any(test, feature = "test-util"))]
@@ -132,16 +128,32 @@ impl From<linkerd2_dns_name::Name> for Name {
 }
 
 impl Name {
-    pub fn from_hostname(hostname: &[u8]) -> Result<Self, InvalidName> {
-        if hostname.last() == Some(&b'.') {
+    pub fn as_dns_name_ref(&self) -> webpki::DNSNameRef<'_> {
+        self.0.as_dns_name_ref()
+    }
+}
+
+impl FromStr for Name {
+    type Err = InvalidName;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.as_bytes().last() == Some(&b'.') {
             return Err(InvalidName); // SNI hostnames are implicitly absolute.
         }
 
-        linkerd2_dns_name::Name::try_from(hostname).map(|n| Name(Arc::new(n)))
+        linkerd2_dns_name::Name::from_str(s).map(|n| Name(Arc::new(n)))
     }
+}
 
-    pub fn as_dns_name_ref(&self) -> webpki::DNSNameRef<'_> {
-        self.0.as_dns_name_ref()
+impl TryFrom<&[u8]> for Name {
+    type Error = InvalidName;
+
+    fn try_from(s: &[u8]) -> Result<Self, Self::Error> {
+        if s.last() == Some(&b'.') {
+            return Err(InvalidName); // SNI hostnames are implicitly absolute.
+        }
+
+        linkerd2_dns_name::Name::try_from(s).map(|n| Name(Arc::new(n)))
     }
 }
 

linkerd/identity/src/test_util.rs

@@ -35,7 +35,7 @@ impl Identity {
     pub fn crt(&self) -> Crt {
         const HOUR: Duration = Duration::from_secs(60 * 60);
 
-        let n = Name::from_hostname(self.name.as_bytes()).expect("name must be valid");
+        let n = Name::from_str(&self.name).expect("name must be valid");
         let der = self.crt.iter().map(|b| *b).collect();
         Crt::new(n, der, vec![], SystemTime::now() + HOUR)
     }