Skip to content

Commit

Permalink
Pass router::Config directly to router::Layer (linkerd#253)
Browse files Browse the repository at this point in the history 
Currently, router `layer`s are constructed with a single argument, a
type implementing `Recognize`. Then, the entire router stack is built
with a `router::Config`. However, in linkerd#248, it became necessary to
provide the config up front when constructing the `router::layer`, as
the layer is used in a fallback layer. Rather than providing a separate
type for a preconfigured layer, @olix0r suggested we simply change all
router layers to accept the `Config` when they're constructed (see
linkerd#248 (comment)).

This branch changes `router::Layer` to accept the config up front. The
`router::Stack` types `make` function now requires no arguments, and the
implementation of `Service` for `Stack` can be called with any `T` (as
the target is now ignored).

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
@hawkw
hawkw authored May 14, 2019
1 parent b27dfb2 commit 16441c2
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 53 deletions.
104 changes: 58 additions & 46 deletions src/app/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,17 +528,20 @@ where
// This is shared across addr-stacks so that multiple addrs that
// canonicalize to the same DstAddr use the same dst-stack service.
let dst_router = svc::builder()
.layer(router::layer(|req: &http::Request<_>| {
let addr = req.extensions().get::<Addr>().cloned().map(|addr| {
let settings = settings::Settings::from_request(req);
DstAddr::outbound(addr, settings)
});
debug!("outbound dst={:?}", addr);
addr
}))
.layer(router::layer(
router::Config::new("out dst", capacity, max_idle_age),
|req: &http::Request<_>| {
let addr = req.extensions().get::<Addr>().cloned().map(|addr| {
let settings = settings::Settings::from_request(req);
DstAddr::outbound(addr, settings)
});
debug!("outbound dst={:?}", addr);
addr
},
))
.buffer_pending(max_in_flight, DispatchDeadline::extract)
.service(dst_stack)
.make(&router::Config::new("out dst", capacity, max_idle_age));
.make();

// Canonicalizes the request-specified `Addr` via DNS, and
// annotates each request with a refined `Addr` so that it may be
Expand All @@ -560,27 +563,30 @@ where
// 4. Finally, if the Source had an SO_ORIGINAL_DST, this TCP
// address is used.
let addr_router = svc::builder()
.layer(router::layer(|req: &http::Request<_>| {
super::http_request_l5d_override_dst_addr(req)
.map(|override_addr| {
debug!("outbound addr={:?}; dst-override", override_addr);
override_addr
})
.or_else(|_| {
let addr = super::http_request_authority_addr(req)
.or_else(|_| super::http_request_host_addr(req))
.or_else(|_| super::http_request_orig_dst_addr(req));
debug!("outbound addr={:?}", addr);
addr
})
.ok()
}))
.layer(router::layer(
router::Config::new("out addr", capacity, max_idle_age),
|req: &http::Request<_>| {
super::http_request_l5d_override_dst_addr(req)
.map(|override_addr| {
debug!("outbound addr={:?}; dst-override", override_addr);
override_addr
})
.or_else(|_| {
let addr = super::http_request_authority_addr(req)
.or_else(|_| super::http_request_host_addr(req))
.or_else(|_| super::http_request_orig_dst_addr(req));
debug!("outbound addr={:?}", addr);
addr
})
.ok()
},
))
.buffer_pending(max_in_flight, DispatchDeadline::extract)
.layer(insert::target::layer())
.layer(strip_header::request::layer(super::DST_OVERRIDE_HEADER))
.layer(strip_header::request::layer(super::L5D_CLIENT_ID))
.service(addr_stack)
.make(&router::Config::new("out addr", capacity, max_idle_age));
.make();

// Share a single semaphore across all requests to signal when
// the proxy is overloaded.
Expand Down Expand Up @@ -657,14 +663,17 @@ where
// If there is no `SO_ORIGINAL_DST` for an inbound socket,
// `default_fwd_addr` may be used.
let endpoint_router = svc::builder()
.layer(router::layer(RecognizeEndpoint::new(default_fwd_addr)))
.layer(router::layer(
router::Config::new("in endpoint", capacity, max_idle_age),
RecognizeEndpoint::new(default_fwd_addr),
))
.buffer_pending(max_in_flight, DispatchDeadline::extract)
.layer(http_metrics::layer::<_, classify::Response>(
endpoint_http_metrics,
))
.layer(tap_layer)
.service(client_stack)
.make(&router::Config::new("in endpoint", capacity, max_idle_age));
.make();

// A per-`dst::Route` layer that uses profile data to configure
// a per-route layer.
Expand Down Expand Up @@ -712,27 +721,30 @@ where
// 5. Finally, if the Source had an SO_ORIGINAL_DST, this TCP
// address is used.
let dst_router = svc::builder()
.layer(router::layer(|req: &http::Request<_>| {
let canonical = req
.headers()
.get(super::CANONICAL_DST_HEADER)
.and_then(|dst| dst.to_str().ok())
.and_then(|d| Addr::from_str(d).ok());
debug!("inbound canonical={:?}", canonical);

let dst = canonical
.or_else(|| super::http_request_authority_addr(req).ok())
.or_else(|| super::http_request_host_addr(req).ok())
.or_else(|| super::http_request_orig_dst_addr(req).ok());
debug!("inbound dst={:?}", dst);
dst.map(|addr| {
let settings = settings::Settings::from_request(req);
DstAddr::inbound(addr, settings)
})
}))
.layer(router::layer(
router::Config::new("in dst", capacity, max_idle_age),
|req: &http::Request<_>| {
let canonical = req
.headers()
.get(super::CANONICAL_DST_HEADER)
.and_then(|dst| dst.to_str().ok())
.and_then(|d| Addr::from_str(d).ok());
debug!("inbound canonical={:?}", canonical);

let dst = canonical
.or_else(|| super::http_request_authority_addr(req).ok())
.or_else(|| super::http_request_host_addr(req).ok())
.or_else(|| super::http_request_orig_dst_addr(req).ok());
debug!("inbound dst={:?}", dst);
dst.map(|addr| {
let settings = settings::Settings::from_request(req);
DstAddr::inbound(addr, settings)
})
},
))
.buffer_pending(max_in_flight, DispatchDeadline::extract)
.service(dst_stack)
.make(&router::Config::new("in dst", capacity, max_idle_age));
.make();

// Share a single semaphore across all requests to signal when
// the proxy is overloaded.
Expand Down
18 changes: 11 additions & 7 deletions src/proxy/http/router.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ pub struct Config {
/// target, it uses a `Mk`-typed `Service` stack.
#[derive(Clone, Debug)]
pub struct Layer<Req, Rec: Recognize<Req>> {
config: Config,
recognize: Rec,
_p: PhantomData<fn() -> Req>,
}

#[derive(Clone, Debug)]
pub struct Stack<Req, Rec: Recognize<Req>, Mk> {
config: Config,
recognize: Rec,
inner: Mk,
_p: PhantomData<fn() -> Req>,
Expand Down Expand Up @@ -68,11 +70,12 @@ impl fmt::Display for Config {

// === impl Layer ===

pub fn layer<Rec, Req>(recognize: Rec) -> Layer<Req, Rec>
pub fn layer<Rec, Req>(config: Config, recognize: Rec) -> Layer<Req, Rec>
where
Rec: Recognize<Req> + Clone + Send + Sync + 'static,
{
Layer {
config,
recognize,
_p: PhantomData,
}
Expand All @@ -91,6 +94,7 @@ where
fn layer(&self, inner: Mk) -> Self::Service {
Stack {
inner,
config: self.config.clone(),
recognize: self.recognize.clone(),
_p: PhantomData,
}
Expand All @@ -107,18 +111,18 @@ where
<Mk::Value as svc::Service<Req>>::Error: Into<Error>,
B: Default + Send + 'static,
{
pub fn make(&self, config: &Config) -> Service<Req, Rec, Mk> {
pub fn make(&self) -> Service<Req, Rec, Mk> {
let inner = Router::new(
self.recognize.clone(),
self.inner.clone(),
config.capacity,
config.max_idle_age,
self.config.capacity,
self.config.max_idle_age,
);
Service { inner }
}
}

impl<Req, Rec, Mk, B> svc::Service<Config> for Stack<Req, Rec, Mk>
impl<Req, Rec, Mk, B, T> svc::Service<T> for Stack<Req, Rec, Mk>
where
Rec: Recognize<Req> + Clone + Send + Sync + 'static,
Mk: rt::Make<Rec::Target> + Clone + Send + Sync + 'static,
Expand All @@ -134,8 +138,8 @@ where
Ok(().into()) // always ready to make a Router
}

fn call(&mut self, config: Config) -> Self::Future {
futures::future::ok(self.make(&config))
fn call(&mut self, _: T) -> Self::Future {
futures::future::ok(self.make())
}
}

Expand Down

0 comments on commit 16441c2

Please sign in to comment.