Skip to content

Commit c6d79c9

Browse files
committed
outbound: Preserve opaqueness on unknown endpoints (#1617)
The outbound stack only honors opaqueness when the profile response clearly indicates that the target is a known endpoint or logical service. This ignores the case when the target is unknown but the target port is in the default opaque ports list, in which case the profile response has no metadata except for the opaqueness setting. This change handles this case explicitly and adds a test for the `switch_logical` stack to ensure that these profile responses are honored. Fixes linkerd/linkerd2#8273 Signed-off-by: Oliver Gould <[email protected]>
1 parent d4c9fb2 commit c6d79c9

File tree

3 files changed

+64
-10
lines changed

3 files changed

+64
-10
lines changed

linkerd/app/outbound/src/endpoint.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,17 @@ pub struct FromMetadata {
3333
// === impl Endpoint ===
3434

3535
impl Endpoint<()> {
36-
pub(crate) fn forward(addr: OrigDstAddr, reason: tls::NoClientTls) -> Self {
36+
pub(crate) fn forward(
37+
addr: OrigDstAddr,
38+
reason: tls::NoClientTls,
39+
opaque_protocol: bool,
40+
) -> Self {
3741
Self {
3842
addr: Remote(ServerAddr(addr.into())),
3943
metadata: Metadata::default(),
4044
tls: Conditional::None(reason),
4145
logical_addr: None,
42-
opaque_protocol: false,
46+
opaque_protocol,
4347
protocol: (),
4448
}
4549
}
@@ -269,6 +273,7 @@ pub mod tests {
269273
.new_service(tcp::Endpoint::forward(
270274
OrigDstAddr(addr),
271275
tls::NoClientTls::Disabled,
276+
false,
272277
));
273278

274279
let (client_io, server_io) = support::io::duplex(4096);

linkerd/app/outbound/src/switch_logical.rs

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,11 @@ impl<S> Outbound<S> {
3232
.push_switch(
3333
move |(profile, target): (Option<profiles::Receiver>, T)| -> Result<_, Infallible> {
3434
if let Some(rx) = profile {
35-
// If the profile provides an endpoint, then the target is single endpoint and
36-
// not a logical/load-balanced service.
35+
let is_opaque = rx.is_opaque_protocol();
36+
37+
// If the profile provides an endpoint, then the target is single
38+
// endpoint and not a logical/load-balanced service.
3739
if let Some((addr, metadata)) = rx.endpoint() {
38-
let is_opaque = rx.is_opaque_protocol();
3940
tracing::debug!(%is_opaque, "Profile describes an endpoint");
4041
return Ok(svc::Either::A(Endpoint::from_metadata(
4142
addr,
@@ -46,20 +47,33 @@ impl<S> Outbound<S> {
4647
)));
4748
}
4849

49-
// Otherwise, if the profile provides a (named) logical address, then we build a
50+
// If the profile provides a (named) logical address, then we build a
5051
// logical stack so we apply routes, traffic splits, and load balancing.
5152
if let Some(logical_addr) = rx.logical_addr() {
5253
tracing::debug!("Profile describes a logical service");
5354
return Ok(svc::Either::B(Logical::new(logical_addr, rx)));
5455
}
56+
57+
// Otherwise, if there was a profile but it didn't include an endpoint or logical
58+
// address, create a bare endpoint from the original destination address
59+
// using the profile-provided opaqueness. This applies for targets that
60+
// aren't known by the destination controller that may target ports
61+
// included in the cluster-wide default opaque list.
62+
tracing::debug!("Unknown endpoint");
63+
return Ok(svc::Either::A(Endpoint::forward(
64+
target.param(),
65+
no_tls_reason,
66+
is_opaque,
67+
)));
5568
}
5669

57-
// If there was no profile or it didn't include any useful metadata, create a bare
58-
// endpoint from the original destination address.
59-
tracing::debug!("No profile; forwarding to the original destination");
70+
// If there was no profile, create a bare endpoint from the original
71+
// destination address.
72+
tracing::debug!("No profile");
6073
Ok(svc::Either::A(Endpoint::forward(
6174
target.param(),
6275
no_tls_reason,
76+
false,
6377
)))
6478
},
6579
logical,
@@ -175,4 +189,37 @@ mod tests {
175189
let (server_io, _client_io) = io::duplex(1);
176190
svc.oneshot(server_io).await.expect("service must succeed");
177191
}
192+
193+
#[tokio::test(flavor = "current_thread")]
194+
async fn profile_neither() {
195+
let _trace = linkerd_tracing::test::trace_init();
196+
197+
let endpoint_addr = SocketAddr::new([192, 0, 2, 20].into(), 2020);
198+
let endpoint = {
199+
let endpoint_addr = endpoint_addr.clone();
200+
move |ep: tcp::Endpoint| {
201+
assert_eq!(ep.addr.as_ref(), &endpoint_addr);
202+
assert!(ep.opaque_protocol, "protocol must be marked opaque");
203+
svc::mk(|_: io::DuplexStream| future::ok::<(), Error>(()))
204+
}
205+
};
206+
207+
let (rt, _shutdown) = runtime();
208+
let stack = Outbound::new(default_config(), rt)
209+
.with_stack(endpoint)
210+
.push_switch_logical(svc::Fail::<_, WrongStack>::default())
211+
.into_inner();
212+
213+
let (_tx, profile) = tokio::sync::watch::channel(profiles::Profile {
214+
endpoint: None,
215+
opaque_protocol: true,
216+
addr: None,
217+
..Default::default()
218+
});
219+
220+
let orig_dst = OrigDstAddr(endpoint_addr);
221+
let svc = stack.new_service((Some(profile.into()), orig_dst));
222+
let (server_io, _client_io) = io::duplex(1);
223+
svc.oneshot(server_io).await.expect("service must succeed");
224+
}
178225
}

linkerd/service-profiles/src/client.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::{
1010
task::{Context, Poll},
1111
};
1212
use tonic::{body::BoxBody, client::GrpcService};
13-
use tracing::debug;
13+
use tracing::{debug, trace};
1414

1515
/// Creates watches on service profiles.
1616
#[derive(Clone, Debug)]
@@ -73,7 +73,9 @@ where
7373
Box::pin(async move {
7474
match w.spawn_watch(addr).await {
7575
Ok(rsp) => {
76+
debug!("Resolved profile");
7677
let rx = rsp.into_inner();
78+
trace!(profile = ?rx.borrow());
7779
Ok(Some(rx.into()))
7880
}
7981
Err(status) => {

0 commit comments

Comments
 (0)