Skip to content

Commit

Permalink
Skip endpoint resolution when profile lookup is rejected (#691)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
olix0r authored Oct 2, 2020
1 parent 9870faa commit 580cb28
Show file tree
Hide file tree
Showing 15 changed files with 403 additions and 410 deletions.
27 changes: 8 additions & 19 deletions linkerd/app/inbound/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)]
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()
}
}
5 changes: 0 additions & 5 deletions linkerd/app/inbound/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
123 changes: 39 additions & 84 deletions linkerd/app/outbound/src/endpoint.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -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};

Expand All @@ -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,
}

Expand All @@ -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,
}

Expand Down Expand Up @@ -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())
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -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 }
}
}
Expand All @@ -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()
}
}
Loading

0 comments on commit 580cb28

Please sign in to comment.