http: Parameterize header module (#941)

The module responsible for adding HTTP headers from target types expects
the target to be coerced to a `HeaderValue`. This means that a target
can only instrument a single header; and it creates some ambiguity.

This change updates the header insertion module to use a Param-type to
represent a key-value header pair, making uses clearer.

Author: olix0r

linkerd/app/outbound/src/http/logical.rs

@@ -1,9 +1,9 @@
-use super::{Concrete, Logical};
+use super::{CanonicalDstHeader, Concrete, Logical};
 use crate::{resolve, stack_labels, Outbound};
 use linkerd_app_core::{
     classify, config, profiles,
     proxy::{core::Resolve, http},
-    retry, svc, tls, Error, Never, CANONICAL_DST_HEADER, DST_OVERRIDE_HEADER,
+    retry, svc, tls, Error, Never, DST_OVERRIDE_HEADER,
 };
 use tracing::debug_span;
 
@@ -146,7 +146,7 @@ impl<E> Outbound<E> {
             // Strips headers that may be set by this proxy and add an outbound
             // canonical-dst-header. The response body is boxed unify the profile
             // stack's response type. withthat of to endpoint stack.
-            .push(http::NewHeaderFromTarget::layer(CANONICAL_DST_HEADER))
+            .push(http::NewHeaderFromTarget::<CanonicalDstHeader, _>::layer())
             .push_on_response(
                 svc::layers()
                     .push(http::strip_header::request::layer(DST_OVERRIDE_HEADER))

linkerd/app/outbound/src/http/mod.rs

@@ -16,7 +16,7 @@ use linkerd_app_core::{
     svc::Param,
     tls,
     transport_header::SessionProtocol,
-    Conditional,
+    Addr, Conditional, CANONICAL_DST_HEADER,
 };
 use std::{net::SocketAddr, str::FromStr, sync::Arc};
 
@@ -25,6 +25,22 @@ pub type Logical = crate::target::Logical<Version>;
 pub type Concrete = crate::target::Concrete<Version>;
 pub type Endpoint = crate::target::Endpoint<Version>;
 
+#[derive(Clone, Debug)]
+pub struct CanonicalDstHeader(pub Addr);
+
+// === impl CanonicalDstHeader ===
+
+impl Into<HeaderPair> for CanonicalDstHeader {
+    fn into(self) -> HeaderPair {
+        HeaderPair(
+            HeaderName::from_static(CANONICAL_DST_HEADER),
+            HeaderValue::from_str(&self.0.to_string()).expect("addr must be a valid header"),
+        )
+    }
+}
+
+// === impl Accept ===
+
 impl Param<Version> for Accept {
     fn param(&self) -> Version {
         self.protocol
@@ -40,6 +56,8 @@ impl Param<normalize_uri::DefaultAuthority> for Accept {
     }
 }
 
+// === impl Logical ===
+
 impl From<(Version, tcp::Logical)> for Logical {
     fn from((protocol, logical): (Version, tcp::Logical)) -> Self {
         Self {
@@ -50,6 +68,12 @@ impl From<(Version, tcp::Logical)> for Logical {
     }
 }
 
+impl Param<CanonicalDstHeader> for Logical {
+    fn param(&self) -> CanonicalDstHeader {
+        CanonicalDstHeader(self.addr())
+    }
+}
+
 impl Param<Version> for Logical {
     fn param(&self) -> Version {
         self.protocol
@@ -85,6 +109,8 @@ impl Param<normalize_uri::DefaultAuthority> for Logical {
     }
 }
 
+// === impl Endpoint ===
+
 impl Param<client::Settings> for Endpoint {
     fn param(&self) -> client::Settings {
         match self.protocol {
@@ -109,14 +135,6 @@ impl Param<Option<SessionProtocol>> for Endpoint {
     }
 }
 
-// Used to set the l5d-canonical-dst header.
-impl From<&'_ Logical> for header::HeaderValue {
-    fn from(target: &'_ Logical) -> Self {
-        header::HeaderValue::from_str(&target.addr().to_string())
-            .expect("addr must be a valid header")
-    }
-}
-
 impl CanOverrideAuthority for Endpoint {
     fn override_authority(&self) -> Option<uri::Authority> {
         self.metadata.authority_override().cloned()

linkerd/proxy/http/src/header_from_target.rs

@@ -1,56 +1,53 @@
-use http::header::{HeaderValue, IntoHeaderName};
-use linkerd_stack::{layer, NewService};
+use crate::HeaderPair;
+use http::header::{HeaderName, HeaderValue};
+use linkerd_stack::{layer, NewService, Param};
 use std::task::{Context, Poll};
 
 /// Wraps an HTTP `Service` so that the Stack's `T -typed target` is cloned into
 /// each request's headers.
 #[derive(Clone, Debug)]
-pub struct NewHeaderFromTarget<H, M> {
-    header: H,
-    inner: M,
+pub struct NewHeaderFromTarget<H, N> {
+    inner: N,
+    _marker: std::marker::PhantomData<fn() -> H>,
 }
 
 #[derive(Clone, Debug)]
-pub struct HeaderFromTarget<H, S> {
-    header: H,
+pub struct HeaderFromTarget<S> {
+    name: HeaderName,
     value: HeaderValue,
     inner: S,
 }
 
 // === impl NewHeaderFromTarget ===
 
-impl<H: Clone, N> NewHeaderFromTarget<H, N> {
-    pub fn layer(header: H) -> impl layer::Layer<N, Service = Self> + Clone {
+impl<H, N> NewHeaderFromTarget<H, N> {
+    pub fn layer() -> impl layer::Layer<N, Service = Self> + Clone {
         layer::mk(move |inner| Self {
             inner,
-            header: header.clone(),
+            _marker: std::marker::PhantomData,
         })
     }
 }
 
 impl<H, T, N> NewService<T> for NewHeaderFromTarget<H, N>
 where
-    H: IntoHeaderName + Clone,
-    T: Clone + Send + Sync + 'static,
-    HeaderValue: for<'t> From<&'t T>,
+    H: Into<HeaderPair>,
+    T: Param<H>,
     N: NewService<T>,
 {
-    type Service = HeaderFromTarget<H, N::Service>;
+    type Service = HeaderFromTarget<N::Service>;
 
     fn new_service(&mut self, t: T) -> Self::Service {
-        HeaderFromTarget {
-            value: (&t).into(),
-            inner: self.inner.new_service(t),
-            header: self.header.clone(),
-        }
+        let HeaderPair(name, value) = t.param().into();
+        let inner = self.inner.new_service(t);
+        HeaderFromTarget { name, value, inner }
     }
 }
 
 // === impl HeaderFromTarget ===
 
-impl<H, S, B> tower::Service<http::Request<B>> for HeaderFromTarget<H, S>
+impl<S, B> tower::Service<http::Request<B>> for HeaderFromTarget<S>
 where
-    H: IntoHeaderName + Clone,
     S: tower::Service<http::Request<B>>,
 {
     type Response = S::Response;
@@ -65,7 +62,7 @@ where
     #[inline]
     fn call(&mut self, mut req: http::Request<B>) -> Self::Future {
         req.headers_mut()
-            .insert(self.header.clone(), self.value.clone());
+            .insert(self.name.clone(), self.value.clone());
         self.inner.call(req)
     }
 }

linkerd/proxy/http/src/lib.rs

@@ -36,11 +36,17 @@ pub use self::{
     timeout::MakeTimeoutLayer,
     version::Version,
 };
-pub use http::{header, uri, Request, Response, StatusCode};
+pub use http::{
+    header::{self, HeaderName, HeaderValue},
+    uri, Request, Response, StatusCode,
+};
 pub use hyper::body::HttpBody;
 pub use linkerd_http_box::{BoxBody, BoxRequest, BoxResponse};
 use std::str::FromStr;
 
+#[derive(Clone, Debug)]
+pub struct HeaderPair(pub HeaderName, pub HeaderValue);
+
 pub trait HasH2Reason {
     fn h2_reason(&self) -> Option<::h2::Reason>;
 }