Skip to content

Commit 6c8d715

Browse files
authored
profiles: Do not rely on tuples as stack targets (#650)
Using tuples for stack targets (particularly, the input targets of the route_request and split modules) is a bit brittle. Instead, we can use `AsRef` on the target type to access the profile receiver. This change introduces new target types to be used to satisfy these traits, and generally cleans up stack construction.
1 parent 67e99b7 commit 6c8d715

File tree

8 files changed

+100
-35
lines changed

8 files changed

+100
-35
lines changed

linkerd/app/gateway/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ mod test {
150150
let socket_addr = SocketAddr::from(([127, 0, 0, 1], 4143));
151151
let target = inbound::Target {
152152
socket_addr,
153-
logical: dst_name
153+
dst: dst_name
154154
.map(|n| NameAddr::from_str(n).unwrap().into())
155155
.unwrap_or_else(|| socket_addr.into()),
156156
http_settings: http::Settings::Http2,

linkerd/app/gateway/src/make.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ where
7272

7373
fn call(&mut self, target: inbound::Target) -> Self::Future {
7474
let inbound::Target {
75-
logical,
75+
dst,
7676
http_settings,
7777
tls_client_id,
7878
..
@@ -85,7 +85,7 @@ where
8585
}
8686
};
8787

88-
let orig_dst = match logical.into_name_addr() {
88+
let orig_dst = match dst.into_name_addr() {
8989
Some(n) => n,
9090
None => {
9191
return Box::pin(future::ok(Gateway::NoAuthority));

linkerd/app/inbound/src/endpoint.rs

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,18 @@ use tracing::debug;
1414

1515
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
1616
pub struct Target {
17-
pub logical: Addr,
17+
pub dst: Addr,
1818
pub socket_addr: SocketAddr,
1919
pub http_settings: http::Settings,
2020
pub tls_client_id: tls::PeerIdentity,
2121
}
2222

23+
#[derive(Clone, Debug)]
24+
pub struct Logical {
25+
target: Target,
26+
profiles: profiles::Receiver,
27+
}
28+
2329
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
2430
pub struct HttpEndpoint {
2531
pub port: u16,
@@ -100,10 +106,10 @@ impl tls::HasPeerIdentity for TcpEndpoint {
100106

101107
// === impl Profile ===
102108

103-
pub(super) fn route(route: profiles::http::Route, target: Target) -> dst::Route {
109+
pub(super) fn route((route, logical): (profiles::http::Route, Logical)) -> dst::Route {
104110
dst::Route {
105111
route,
106-
target: target.logical,
112+
target: logical.target.dst,
107113
direction: metric_labels::Direction::In,
108114
}
109115
}
@@ -112,7 +118,7 @@ pub(super) fn route(route: profiles::http::Route, target: Target) -> dst::Route
112118

113119
impl AsRef<Addr> for Target {
114120
fn as_ref(&self) -> &Addr {
115-
&self.logical
121+
&self.dst
116122
}
117123
}
118124

@@ -123,7 +129,7 @@ impl http::normalize_uri::ShouldNormalizeUri for Target {
123129
..
124130
} = self.http_settings
125131
{
126-
return Some(self.logical.to_http_authority());
132+
return Some(self.dst.to_http_authority());
127133
}
128134
None
129135
}
@@ -144,7 +150,7 @@ impl tls::HasPeerIdentity for Target {
144150
impl Into<metric_labels::EndpointLabels> for Target {
145151
fn into(self) -> metric_labels::EndpointLabels {
146152
metric_labels::EndpointLabels {
147-
authority: self.logical.name_addr().map(|d| d.as_http_authority()),
153+
authority: self.dst.name_addr().map(|d| d.as_http_authority()),
148154
direction: metric_labels::Direction::In,
149155
tls_id: self.tls_client_id.map(metric_labels::TlsId::ClientId),
150156
labels: None,
@@ -205,7 +211,7 @@ impl tap::Inspect for Target {
205211

206212
impl fmt::Display for Target {
207213
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
208-
self.logical.fmt(f)
214+
self.dst.fmt(f)
209215
}
210216
}
211217

@@ -214,7 +220,7 @@ impl stack_tracing::GetSpan<()> for Target {
214220
use tracing::info_span;
215221

216222
match self.http_settings {
217-
http::Settings::Http2 => match self.logical.name_addr() {
223+
http::Settings::Http2 => match self.dst.name_addr() {
218224
None => info_span!(
219225
"http2",
220226
port = %self.socket_addr.port(),
@@ -229,7 +235,7 @@ impl stack_tracing::GetSpan<()> for Target {
229235
keep_alive,
230236
wants_h1_upgrade,
231237
was_absolute_form,
232-
} => match self.logical.name_addr() {
238+
} => match self.dst.name_addr() {
233239
None => info_span!(
234240
"http1",
235241
port = %self.socket_addr.port(),
@@ -262,7 +268,7 @@ impl<A> router::Recognize<http::Request<A>> for RequestTarget {
262268
type Key = Target;
263269

264270
fn recognize(&self, req: &http::Request<A>) -> Self::Key {
265-
let logical = req
271+
let dst = req
266272
.headers()
267273
.get(CANONICAL_DST_HEADER)
268274
.and_then(|dst| {
@@ -286,10 +292,30 @@ impl<A> router::Recognize<http::Request<A>> for RequestTarget {
286292
.unwrap_or_else(|| self.accept.addrs.target_addr().into());
287293

288294
Target {
289-
logical,
295+
dst,
290296
socket_addr: self.accept.addrs.target_addr(),
291297
tls_client_id: self.accept.peer_identity.clone(),
292298
http_settings: http::Settings::from_request(req),
293299
}
294300
}
295301
}
302+
303+
impl From<Logical> for Target {
304+
fn from(Logical { target, .. }: Logical) -> Self {
305+
target
306+
}
307+
}
308+
309+
// === impl Logical ===
310+
311+
impl From<(profiles::Receiver, Target)> for Logical {
312+
fn from((profiles, target): (profiles::Receiver, Target)) -> Self {
313+
Self { profiles, target }
314+
}
315+
}
316+
317+
impl AsRef<profiles::Receiver> for Logical {
318+
fn as_ref(&self) -> &profiles::Receiver {
319+
&self.profiles
320+
}
321+
}

linkerd/app/inbound/src/lib.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ impl Config {
214214
.check_new_service::<Target, http::Request<http::boxed::Payload>>()
215215
.push_on_response(svc::layers().box_http_request())
216216
// The target stack doesn't use the profile resolution, so drop it.
217-
.push_map_target(|(_, target): (profiles::Receiver, Target)| target)
217+
.push_map_target(endpoint::Target::from)
218218
.push(profiles::http::route_request::layer(
219219
svc::proxies()
220220
// Sets the route as a request extension so that it can be used
@@ -226,11 +226,10 @@ impl Config {
226226
// extension.
227227
.push(classify::Layer::new())
228228
.check_new_clone::<dst::Route>()
229-
.push_map_target(|(r, t): (profiles::http::Route, Target)| {
230-
endpoint::route(r, t)
231-
})
229+
.push_map_target(endpoint::route)
232230
.into_inner(),
233231
))
232+
.push_map_target(endpoint::Logical::from)
234233
.push(profiles::discover::layer(profiles_client))
235234
.into_new_service()
236235
.cache(

linkerd/app/outbound/src/endpoint.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ pub struct Target<T> {
3535
pub inner: T,
3636
}
3737

38+
#[derive(Clone, Debug)]
39+
pub struct Profile {
40+
pub rx: profiles::Receiver,
41+
pub target: Target<HttpEndpoint>,
42+
}
43+
3844
#[derive(Clone, Debug, Eq, PartialEq)]
3945
pub struct HttpEndpoint {
4046
pub addr: SocketAddr,
@@ -49,6 +55,15 @@ pub struct TcpEndpoint {
4955
pub identity: tls::PeerIdentity,
5056
}
5157

58+
impl From<(Addr, Profile)> for Concrete<http::Settings> {
59+
fn from((addr, Profile { target, .. }): (Addr, Profile)) -> Self {
60+
Self {
61+
addr,
62+
inner: target.map(|e| e.settings),
63+
}
64+
}
65+
}
66+
5267
// === impl Target ===
5368

5469
impl<T> Target<T> {
@@ -406,10 +421,36 @@ impl<B> router::Recognize<http::Request<B>> for LogicalPerRequest {
406421
}
407422
}
408423

409-
pub fn route<T>(route: profiles::http::Route, target: Logical<T>) -> dst::Route {
424+
pub fn route((route, profile): (profiles::http::Route, Profile)) -> dst::Route {
410425
dst::Route {
411426
route,
412-
target: target.addr,
427+
target: profile.target.addr,
413428
direction: metric_labels::Direction::Out,
414429
}
415430
}
431+
432+
// === impl Profile ===
433+
434+
impl From<(profiles::Receiver, Target<HttpEndpoint>)> for Profile {
435+
fn from((rx, target): (profiles::Receiver, Target<HttpEndpoint>)) -> Self {
436+
Self { rx, target }
437+
}
438+
}
439+
440+
impl AsRef<Addr> for Profile {
441+
fn as_ref(&self) -> &Addr {
442+
&self.target.addr
443+
}
444+
}
445+
446+
impl AsRef<profiles::Receiver> for Profile {
447+
fn as_ref(&self) -> &profiles::Receiver {
448+
&self.rx
449+
}
450+
}
451+
452+
impl From<Profile> for Target<HttpEndpoint> {
453+
fn from(Profile { target, .. }: Profile) -> Self {
454+
target
455+
}
456+
}

linkerd/app/outbound/src/lib.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use linkerd2_app_core::{
2222
spans::SpanConverter,
2323
svc::{self, NewService},
2424
transport::{self, listen, tls},
25-
Addr, Conditional, DiscoveryRejected, Error, ProxyMetrics, StackMetrics, TraceContextLayer,
25+
Conditional, DiscoveryRejected, Error, ProxyMetrics, StackMetrics, TraceContextLayer,
2626
CANONICAL_DST_HEADER, DST_OVERRIDE_HEADER, L5D_REQUIRE_ID,
2727
};
2828
use std::{collections::HashMap, net::IpAddr, time::Duration};
@@ -291,10 +291,7 @@ impl Config {
291291
// processed.
292292
let logical = concrete
293293
// Uses the split-provided target `Addr` to build a concrete target.
294-
.push_map_target(|(addr, l): (Addr, Logical<HttpEndpoint>)| Concrete {
295-
addr,
296-
inner: l.map(|e| e.settings),
297-
})
294+
.push_map_target(Concrete::<http::Settings>::from)
298295
.push(profiles::split::layer())
299296
// Drives concrete stacks to readiness and makes the split
300297
// cloneable, as required by the retry middleware.
@@ -311,11 +308,10 @@ impl Config {
311308
// Sets the per-route response classifier as a request
312309
// extension.
313310
.push(classify::Layer::new())
314-
.push_map_target(|(r, l): (profiles::http::Route, Logical<HttpEndpoint>)| {
315-
endpoint::route(r, l)
316-
})
311+
.push_map_target(endpoint::route)
317312
.into_inner(),
318313
))
314+
.push_map_target(endpoint::Profile::from)
319315
// Discovers the service profile from the control plane and passes
320316
// it to inner stack to build the router and traffic split.
321317
.push(profiles::discover::layer(profiles_client))

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,17 @@ impl<M: Clone, N: Clone, R> Clone for NewRouteRequest<M, N, R> {
5252
}
5353
}
5454

55-
impl<T, M, N> NewService<(Receiver, T)> for NewRouteRequest<M, N, N::Service>
55+
impl<T, M, N> NewService<T> for NewRouteRequest<M, N, N::Service>
5656
where
57-
T: Clone,
58-
M: NewService<(Receiver, T)>,
57+
T: AsRef<Receiver> + Clone,
58+
M: NewService<T>,
5959
N: NewService<(Route, T)> + Clone,
6060
{
6161
type Service = RouteRequest<T, M::Service, N, N::Service>;
6262

63-
fn new_service(&self, (rx, target): (Receiver, T)) -> Self::Service {
64-
let inner = self.inner.new_service((rx.clone(), target.clone()));
63+
fn new_service(&self, target: T) -> Self::Service {
64+
let rx = target.as_ref().clone();
65+
let inner = self.inner.new_service(target.clone());
6566
let default = self
6667
.new_route
6768
.new_service((self.default.clone(), target.clone()));

linkerd/service-profiles/src/split.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,15 @@ impl<N: Clone, S, Req> Clone for NewSplit<N, S, Req> {
5858
}
5959
}
6060

61-
impl<T, N: Clone, S, Req> NewService<(Receiver, T)> for NewSplit<N, S, Req>
61+
impl<T, N: Clone, S, Req> NewService<T> for NewSplit<N, S, Req>
6262
where
63+
T: AsRef<Receiver>,
6364
S: tower::Service<Req>,
6465
{
6566
type Service = Split<T, N, S, Req>;
6667

67-
fn new_service(&self, (rx, target): (Receiver, T)) -> Self::Service {
68+
fn new_service(&self, target: T) -> Self::Service {
69+
let rx = target.as_ref().clone();
6870
Split {
6971
rx,
7072
target,

0 commit comments

Comments
 (0)