Skip endpoint resolution when profile lookup is rejected (#691)

When profile lookup is rejected, this error propagates to the stack and
is handled by falling back to an alternate stack.

In order to move profile discovery out of the HTTP stack and onto the
initial TCP accept stack, we want to avoid this sort of fallback
behavior. So this change modifies profile discovery to make the profile
optional. When a profile lookup is rejected/skipped, we now simply
return a `None` profile; and in these cases we avoid performing endpoint
resolution by omitting a concrete address.

Author: olix0r

linkerd/app/inbound/src/endpoint.rs

@@ -7,7 +7,7 @@ use linkerd2_app_core::{
     transport::{listen, tls},
     Addr, Conditional, CANONICAL_DST_HEADER, DST_OVERRIDE_HEADER,
 };
-use std::{convert::TryInto, fmt, net::SocketAddr, sync::Arc};
+use std::{convert::TryInto, net::SocketAddr, sync::Arc};
 use tracing::debug;
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -21,7 +21,7 @@ pub struct Target {
 #[derive(Clone, Debug)]
 pub struct Logical {
     target: Target,
-    profiles: profiles::Receiver,
+    profiles: Option<profiles::Receiver>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -114,18 +114,13 @@ pub(super) fn route((route, logical): (profiles::http::Route, Logical)) -> dst::
 
 // === impl Target ===
 
+/// Used for profile discovery.
 impl Into<Addr> for &'_ Target {
     fn into(self) -> Addr {
         self.dst.clone()
     }
 }
 
-impl AsRef<Addr> for Target {
-    fn as_ref(&self) -> &Addr {
-        &self.dst
-    }
-}
-
 impl tls::HasPeerIdentity for Target {
     fn peer_identity(&self) -> tls::PeerIdentity {
         Conditional::None(tls::ReasonForNoPeerName::Loopback.into())
@@ -197,12 +192,6 @@ impl tap::Inspect for Target {
     }
 }
 
-impl fmt::Display for Target {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.dst.fmt(f)
-    }
-}
-
 impl stack_tracing::GetSpan<()> for Target {
     fn get_span(&self, _: &()) -> tracing::Span {
         use tracing::info_span;
@@ -275,14 +264,14 @@ impl From<Logical> for Target {
 
 // === impl Logical ===
 
-impl From<(profiles::Receiver, Target)> for Logical {
-    fn from((profiles, target): (profiles::Receiver, Target)) -> Self {
+impl From<(Option<profiles::Receiver>, Target)> for Logical {
+    fn from((profiles, target): (Option<profiles::Receiver>, Target)) -> Self {
         Self { profiles, target }
     }
 }
 
-impl AsRef<profiles::Receiver> for Logical {
-    fn as_ref(&self) -> &profiles::Receiver {
-        &self.profiles
+impl Into<Option<profiles::Receiver>> for &'_ Logical {
+    fn into(self) -> Option<profiles::Receiver> {
+        self.profiles.clone()
     }
 }

linkerd/app/inbound/src/lib.rs

@@ -248,14 +248,9 @@ impl Config {
             .push_on_response(svc::layers().box_http_response())
             .check_new_service::<Target, http::Request<http::boxed::Payload>>();
 
-        let forward = target
-            .instrument(|_: &Target| debug_span!("forward"))
-            .check_new_service::<Target, http::Request<http::boxed::Payload>>();
-
         // Attempts to resolve the target as a service profile or, if that
         // fails, skips that stack to forward to the local endpoint.
         profile
-            .push_fallback(forward)
             .check_new_service::<Target, http::Request<http::boxed::Payload>>()
             // If the traffic is targeted at the inbound port, send it through
             // the loopback service (i.e. as a gateway).

linkerd/app/outbound/src/endpoint.rs

@@ -1,8 +1,7 @@
 use crate::http::uri::Authority;
 use indexmap::IndexMap;
 use linkerd2_app_core::{
-    dst,
-    metric_labels,
+    dst, metric_labels,
     metric_labels::{prefix_labels, EndpointLabels, TlsStatus},
     profiles,
     proxy::{
@@ -15,8 +14,7 @@ use linkerd2_app_core::{
     },
     router,
     transport::{listen, tls},
-    Addr,
-    Conditional, //L5D_REQUIRE_ID,
+    Addr, Conditional,
 };
 use std::{net::SocketAddr, sync::Arc};
 
@@ -38,7 +36,7 @@ pub struct HttpLogical {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct HttpConcrete {
-    pub dst: Addr,
+    pub resolve: Option<Addr>,
     pub logical: HttpLogical,
 }
 
@@ -47,7 +45,7 @@ pub struct LogicalPerRequest(HttpAccept);
 
 #[derive(Clone, Debug)]
 pub struct Profile {
-    pub rx: profiles::Receiver,
+    pub rx: Option<profiles::Receiver>,
     pub logical: HttpLogical,
 }
 
@@ -81,21 +79,17 @@ impl From<listen::Addrs> for TcpLogical {
     }
 }
 
+/// Used as a default destination when resolution is rejected.
 impl Into<SocketAddr> for &'_ TcpLogical {
     fn into(self) -> SocketAddr {
         self.addr
     }
 }
 
-impl Into<Addr> for &'_ TcpLogical {
-    fn into(self) -> Addr {
-        self.addr.into()
-    }
-}
-
-impl std::fmt::Display for TcpLogical {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.addr.fmt(f)
+/// Used to resolve endpoints.
+impl Into<Option<Addr>> for &'_ TcpLogical {
+    fn into(self) -> Option<Addr> {
+        Some(self.addr.into())
     }
 }
 
@@ -118,102 +112,63 @@ impl Into<SocketAddr> for &'_ HttpAccept {
 
 // === impl HttpConrete ===
 
-impl From<(Addr, Profile)> for HttpConcrete {
-    fn from((dst, Profile { logical, .. }): (Addr, Profile)) -> Self {
-        Self { dst, logical }
-    }
-}
-
-impl AsRef<Addr> for HttpConcrete {
-    fn as_ref(&self) -> &Addr {
-        &self.dst
+impl From<(Option<Addr>, Profile)> for HttpConcrete {
+    fn from((resolve, Profile { logical, .. }): (Option<Addr>, Profile)) -> Self {
+        Self { resolve, logical }
     }
 }
 
-impl Into<Addr> for &'_ HttpConcrete {
-    fn into(self) -> Addr {
-        self.dst.clone()
+/// Produces an address to resolve to individual endpoints. This address is only
+/// present if the initial profile resolution was not rejected.
+impl Into<Option<Addr>> for &'_ HttpConcrete {
+    fn into(self) -> Option<Addr> {
+        self.resolve.clone()
     }
 }
 
+/// Produces an address to be used if resolution is rejected.
 impl Into<SocketAddr> for &'_ HttpConcrete {
     fn into(self) -> SocketAddr {
-        self.dst
-            .socket_addr()
+        self.resolve
+            .as_ref()
+            .and_then(|a| a.socket_addr())
             .unwrap_or_else(|| self.logical.orig_dst)
     }
 }
 
-impl std::fmt::Display for HttpConcrete {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.dst.fmt(f)
-    }
-}
-
-impl From<HttpLogical> for HttpConcrete {
-    fn from(logical: HttpLogical) -> Self {
-        Self {
-            dst: logical.dst.clone(),
-            logical,
-        }
-    }
-}
-
 // === impl HttpLogical ===
 
-impl std::fmt::Display for HttpLogical {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.dst.fmt(f)
-    }
-}
-
-impl<'t> From<&'t HttpLogical> for http::header::HeaderValue {
-    fn from(target: &'t HttpLogical) -> Self {
-        http::header::HeaderValue::from_str(&target.dst.to_string())
-            .expect("addr must be a valid header")
-    }
-}
-
-impl Into<SocketAddr> for HttpLogical {
-    fn into(self) -> SocketAddr {
-        self.orig_dst
-    }
-}
-
+/// Produces an address for profile discovery.
 impl Into<Addr> for &'_ HttpLogical {
     fn into(self) -> Addr {
         self.dst.clone()
     }
 }
 
+/// Needed for canonicalization.
 impl AsRef<Addr> for HttpLogical {
     fn as_ref(&self) -> &Addr {
         &self.dst
     }
 }
 
+/// Needed for canonicalization.
 impl AsMut<Addr> for HttpLogical {
     fn as_mut(&mut self) -> &mut Addr {
         &mut self.dst
     }
 }
 
-// === impl HttpEndpoint ===
-
-impl From<HttpLogical> for HttpEndpoint {
-    fn from(logical: HttpLogical) -> Self {
-        Self {
-            addr: logical.orig_dst,
-            settings: logical.version.into(),
-            identity: Conditional::None(
-                tls::ReasonForNoPeerName::NotProvidedByServiceDiscovery.into(),
-            ),
-            concrete: logical.into(),
-            metadata: Metadata::default(),
-        }
+// Used to set the l5d-canonical-dst header.
+impl<'t> From<&'t HttpLogical> for http::header::HeaderValue {
+    fn from(target: &'t HttpLogical) -> Self {
+        http::header::HeaderValue::from_str(&target.dst.to_string())
+            .expect("addr must be a valid header")
     }
 }
 
+// === impl HttpEndpoint ===
+
 impl std::hash::Hash for HttpEndpoint {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.addr.hash(state);
@@ -461,8 +416,8 @@ pub fn route((route, profile): (profiles::http::Route, Profile)) -> dst::Route {
 
 // === impl Profile ===
 
-impl From<(profiles::Receiver, HttpLogical)> for Profile {
-    fn from((rx, logical): (profiles::Receiver, HttpLogical)) -> Self {
+impl From<(Option<profiles::Receiver>, HttpLogical)> for Profile {
+    fn from((rx, logical): (Option<profiles::Receiver>, HttpLogical)) -> Self {
         Self { rx, logical }
     }
 }
@@ -473,14 +428,14 @@ impl AsRef<Addr> for Profile {
     }
 }
 
-impl AsRef<profiles::Receiver> for Profile {
-    fn as_ref(&self) -> &profiles::Receiver {
-        &self.rx
+impl Into<Addr> for &'_ Profile {
+    fn into(self) -> Addr {
+        self.logical.dst.clone()
     }
 }
 
-impl From<Profile> for HttpLogical {
-    fn from(Profile { logical, .. }: Profile) -> Self {
-        logical
+impl Into<Option<profiles::Receiver>> for &'_ Profile {
+    fn into(self) -> Option<profiles::Receiver> {
+        self.rx.clone()
     }
 }