service-profiles: Eliminate the HasDestination trait (#643)

The `HasDestination` trait isn't particularly useful, as it's basically
just `AsRef<Addr>`.

This change updates the `GetRoutes` signature to support this; and it
updates the inbound target type to store an `Addr` instead of an
`Option<NameAddr>` (so `Target` will be suitable for this in an upcoming
change).

Author: olix0r

linkerd/app/gateway/src/lib.rs

@@ -147,9 +147,12 @@ mod test {
                 ))
             };
 
+            let socket_addr = SocketAddr::from(([127, 0, 0, 1], 4143));
             let target = inbound::Target {
-                addr: SocketAddr::from(([127, 0, 0, 1], 4143)),
-                dst_name: dst_name.map(|n| NameAddr::from_str(n).unwrap()),
+                socket_addr,
+                logical: dst_name
+                    .map(|n| NameAddr::from_str(n).unwrap().into())
+                    .unwrap_or_else(|| socket_addr.into()),
                 http_settings: http::Settings::Http2,
                 tls_client_id: peer_id,
             };

linkerd/app/gateway/src/make.rs

@@ -72,7 +72,7 @@ where
 
     fn call(&mut self, target: inbound::Target) -> Self::Future {
         let inbound::Target {
-            dst_name,
+            logical,
             http_settings,
             tls_client_id,
             ..
@@ -85,7 +85,7 @@ where
             }
         };
 
-        let orig_dst = match dst_name {
+        let orig_dst = match logical.into_name_addr() {
             Some(n) => n,
             None => {
                 return Box::pin(future::ok(Gateway::NoAuthority));

linkerd/app/inbound/src/endpoint.rs

@@ -5,7 +5,7 @@ use linkerd2_app_core::{
     proxy::{http, identity, tap},
     router, stack_tracing,
     transport::{connect, listen, tls},
-    Addr, Conditional, NameAddr, CANONICAL_DST_HEADER, DST_OVERRIDE_HEADER,
+    Addr, Conditional, CANONICAL_DST_HEADER, DST_OVERRIDE_HEADER,
 };
 use std::fmt;
 use std::net::SocketAddr;
@@ -14,8 +14,8 @@ use tracing::debug;
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct Target {
-    pub addr: SocketAddr,
-    pub dst_name: Option<NameAddr>,
+    pub logical: Addr,
+    pub socket_addr: SocketAddr,
     pub http_settings: http::Settings,
     pub tls_client_id: tls::PeerIdentity,
 }
@@ -59,7 +59,7 @@ impl http::settings::HasSettings for HttpEndpoint {
 impl From<Target> for HttpEndpoint {
     fn from(target: Target) -> Self {
         Self {
-            port: target.addr.port(),
+            port: target.socket_addr.port(),
             settings: target.http_settings,
         }
     }
@@ -105,27 +105,16 @@ impl tls::HasPeerIdentity for TcpEndpoint {
 
 impl From<Target> for Profile {
     fn from(t: Target) -> Self {
-        Profile(
-            t.dst_name
-                .clone()
-                .map(|d| d.into())
-                .unwrap_or_else(|| t.addr.clone().into()),
-        )
+        Profile(t.logical)
     }
 }
 
-impl Profile {
-    pub fn addr(&self) -> &Addr {
+impl AsRef<Addr> for Profile {
+    fn as_ref(&self) -> &Addr {
         &self.0
     }
 }
 
-impl profiles::HasDestination for Profile {
-    fn destination(&self) -> Addr {
-        self.0.clone()
-    }
-}
-
 impl profiles::WithRoute for Profile {
     type Route = dst::Route;
 
@@ -138,7 +127,7 @@ impl profiles::WithRoute for Profile {
     }
 }
 
-// // === impl Target ===
+// === impl Target ===
 
 impl http::normalize_uri::ShouldNormalizeUri for Target {
     fn should_normalize_uri(&self) -> Option<http::uri::Authority> {
@@ -147,12 +136,7 @@ impl http::normalize_uri::ShouldNormalizeUri for Target {
             ..
         } = self.http_settings
         {
-            return Some(
-                self.dst_name
-                    .as_ref()
-                    .map(|dst| dst.as_http_authority())
-                    .unwrap_or_else(|| Addr::from(self.addr).to_http_authority()),
-            );
+            return Some(self.logical.to_http_authority());
         }
         None
     }
@@ -173,7 +157,7 @@ impl tls::HasPeerIdentity for Target {
 impl Into<metric_labels::EndpointLabels> for Target {
     fn into(self) -> metric_labels::EndpointLabels {
         metric_labels::EndpointLabels {
-            authority: self.dst_name.map(|d| d.as_http_authority()),
+            authority: self.logical.name_addr().map(|d| d.as_http_authority()),
             direction: metric_labels::Direction::In,
             tls_id: self.tls_client_id.map(metric_labels::TlsId::ClientId),
             labels: None,
@@ -207,7 +191,7 @@ impl tap::Inspect for Target {
     }
 
     fn dst_addr<B>(&self, _: &http::Request<B>) -> Option<SocketAddr> {
-        Some(self.addr)
+        Some(self.socket_addr)
     }
 
     fn dst_labels<B>(&self, _: &http::Request<B>) -> Option<&IndexMap<String, String>> {
@@ -234,7 +218,7 @@ impl tap::Inspect for Target {
 
 impl fmt::Display for Target {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.addr.fmt(f)
+        self.logical.fmt(f)
     }
 }
 
@@ -243,33 +227,33 @@ impl stack_tracing::GetSpan<()> for Target {
         use tracing::info_span;
 
         match self.http_settings {
-            http::Settings::Http2 => match self.dst_name.as_ref() {
+            http::Settings::Http2 => match self.logical.name_addr() {
                 None => info_span!(
                     "http2",
-                    port = %self.addr.port(),
+                    port = %self.socket_addr.port(),
                 ),
                 Some(name) => info_span!(
                     "http2",
                     %name,
-                    port = %self.addr.port(),
+                    port = %self.socket_addr.port(),
                 ),
             },
             http::Settings::Http1 {
                 keep_alive,
                 wants_h1_upgrade,
                 was_absolute_form,
-            } => match self.dst_name.as_ref() {
+            } => match self.logical.name_addr() {
                 None => info_span!(
                     "http1",
-                    port = %self.addr.port(),
+                    port = %self.socket_addr.port(),
                     keep_alive,
                     wants_h1_upgrade,
                     was_absolute_form,
                 ),
                 Some(name) => info_span!(
                     "http1",
                     %name,
-                    port = %self.addr.port(),
+                    port = %self.socket_addr.port(),
                     keep_alive,
                     wants_h1_upgrade,
                     was_absolute_form,
@@ -291,7 +275,7 @@ impl<A> router::Recognize<http::Request<A>> for RequestTarget {
     type Key = Target;
 
     fn recognize(&self, req: &http::Request<A>) -> Self::Key {
-        let dst_name = req
+        let logical = req
             .headers()
             .get(CANONICAL_DST_HEADER)
             .and_then(|dst| {
@@ -312,12 +296,11 @@ impl<A> router::Recognize<http::Request<A>> for RequestTarget {
             })
             .or_else(|| http_request_authority_addr(req).ok())
             .or_else(|| http_request_host_addr(req).ok())
-            .or_else(|| self.accept.addrs.target_addr_if_not_local().map(Addr::from))
-            .and_then(|a| a.name_addr().cloned());
+            .unwrap_or_else(|| self.accept.addrs.target_addr().into());
 
         Target {
-            dst_name,
-            addr: self.accept.addrs.target_addr(),
+            logical,
+            socket_addr: self.accept.addrs.target_addr(),
             tls_client_id: self.accept.peer_identity.clone(),
             http_settings: http::Settings::from_request(req),
         }
@@ -330,11 +313,6 @@ impl router::Recognize<Target> for ProfileTarget {
     type Key = Profile;
 
     fn recognize(&self, t: &Target) -> Self::Key {
-        Profile(
-            t.dst_name
-                .clone()
-                .map(Into::into)
-                .unwrap_or_else(|| t.addr.into()),
-        )
+        Profile(t.logical.clone())
     }
 }

linkerd/app/inbound/src/lib.rs

@@ -258,7 +258,7 @@ impl Config {
                 ),
             )
             .spawn_buffer(buffer_capacity)
-            .instrument(|p: &Profile| info_span!("profile", addr = %p.addr()))
+            .instrument(|p: &Profile| info_span!("profile", addr = %p.as_ref()))
             .check_make_service::<Profile, Target>()
             .push(router::Layer::new(|()| ProfileTarget))
             .check_new_service::<(), Target>()

linkerd/app/inbound/src/prevent_loop.rs

@@ -24,8 +24,8 @@ impl admit::Admit<Target> for PreventLoop {
     type Error = LoopPrevented;
 
     fn admit(&mut self, ep: &Target) -> Result<(), Self::Error> {
-        tracing::debug!(addr = %ep.addr, self.port);
-        if ep.addr.port() == self.port {
+        tracing::debug!(addr = %ep.socket_addr, self.port);
+        if ep.socket_addr.port() == self.port {
             return Err(LoopPrevented { port: self.port });
         }
 

linkerd/app/outbound/src/endpoint.rs

@@ -430,9 +430,9 @@ impl<T> router::Recognize<Target<T>> for ProfilePerTarget {
 
 // === impl Profile ===
 
-impl profiles::HasDestination for Profile {
-    fn destination(&self) -> Addr {
-        self.0.clone()
+impl AsRef<Addr> for Profile {
+    fn as_ref(&self) -> &Addr {
+        &self.0
     }
 }
 

linkerd/service-profiles/src/client.rs

@@ -112,8 +112,9 @@ where
     }
 }
 
-impl<S, R> tower::Service<Addr> for Client<S, R>
+impl<T, S, R> tower::Service<T> for Client<S, R>
 where
+    T: AsRef<Addr>,
     S: GrpcService<BoxBody> + Clone + Send + 'static,
     S::ResponseBody: Send,
     <S::ResponseBody as Body>::Data: Send,
@@ -132,8 +133,8 @@ where
         Poll::Ready(Ok(()))
     }
 
-    fn call(&mut self, dst: Addr) -> Self::Future {
-        let path = dst.to_string();
+    fn call(&mut self, dst: T) -> Self::Future {
+        let path = dst.as_ref().to_string();
 
         let service = {
             // In case the ready service holds resources, pass it into the

linkerd/service-profiles/src/http/mod.rs

@@ -50,8 +50,7 @@ pub trait GetRoutes<T> {
 
 impl<T, S> GetRoutes<T> for S
 where
-    T: HasDestination,
-    S: tower::Service<Addr, Response = watch::Receiver<Routes>>,
+    S: tower::Service<T, Response = watch::Receiver<Routes>>,
     S::Error: Into<Error>,
 {
     type Error = S::Error;
@@ -62,7 +61,7 @@ where
     }
 
     fn get_routes(&mut self, target: T) -> Self::Future {
-        tower::Service::call(self, target.destination())
+        tower::Service::call(self, target)
     }
 }
 
@@ -79,12 +78,6 @@ pub trait OverrideDestination {
     fn dst_mut(&mut self) -> &mut Addr;
 }
 
-/// Implemented by target types that may have a `NameAddr` destination that
-/// can be discovered via `GetRoutes`.
-pub trait HasDestination {
-    fn destination(&self) -> Addr;
-}
-
 #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
 pub struct Route {
     labels: Labels,