Skip to content

Commit 16939ca

Browse files
olix0rolix0r
committed
Use the connection's HTTP version in transport header (#1533)
Currently, when the outbound proxy communicates with a meshed endpoint that uses opaque transport (i.e. multi-cluster gateways), it *always* sets the gateway header's session protocol to HTTP/2, since the meshed endpoint supports HTTP/2 protocol upgrading. But the HTTP client may choose not to use HTTP/2 if the request includes the `Upgrade` header, as it does for WebSocket connections. In these cases, the transport header should indicate that the connection is HTTP/1. This change modifies the HTTP client to pass the used protocol version when building a connection. This value is then used to set the session protocol header when it is required.. Signed-off-by: Oliver Gould <ver@buoyant.io> (cherry picked from commit cbb3390) Signed-off-by: Oliver Gould <ver@buoyant.io>
1 parent c6000b5 commit 16939ca

File tree

12 files changed

+164
-48
lines changed

12 files changed

+164
-48
lines changed

‎linkerd/app/core/src/control.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ impl Config {
8686
svc::stack(ConnectTcp::new(self.connect.keepalive))
8787
.push(tls::Client::layer(identity))
8888
.push_connect_timeout(self.connect.timeout)
89+
.push_map_target(|(_version, target)| target)
8990
.push(self::client::layer())
9091
.push_on_service(svc::MapErr::layer(Into::into))
9192
.into_new_service()

‎linkerd/app/inbound/src/http/router.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ impl<C> Inbound<C> {
9696
let http = connect
9797
.push(svc::stack::BoxFuture::layer())
9898
.push(transport::metrics::Client::layer(rt.metrics.proxy.transport.clone()))
99+
.check_service::<Http>()
100+
.push_map_target(|(_version, target)| target)
99101
.push(http::client::layer(
100102
config.proxy.connect.h1_settings,
101103
config.proxy.connect.h2_settings,

‎linkerd/app/outbound/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,5 @@ linkerd-app-test = { path = "../test" }
3636
linkerd-io = { path = "../../io", features = ["tokio-test"] }
3737
linkerd-tracing = { path = "../../tracing", features = ["ansi"] }
3838
parking_lot = "0.12"
39-
tokio = { version = "1", features = ["time", "macros"] }
39+
tokio = { version = "1", features = ["macros", "sync", "time"] }
4040
tokio-test = "0.4"

‎linkerd/app/outbound/src/endpoint.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ impl<S> Outbound<S> {
224224
{
225225
let http = self
226226
.clone()
227-
.push_tcp_endpoint::<http::Endpoint>()
227+
.push_tcp_endpoint::<http::Connect>()
228228
.push_http_endpoint()
229229
.push_http_server()
230230
.into_inner();

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

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ use linkerd_app_core::{
1919
profiles::{self, LogicalAddr},
2020
proxy::{api_resolve::ProtocolHint, tap},
2121
svc::Param,
22-
tls,
23-
transport_header::SessionProtocol,
24-
Addr, Conditional, CANONICAL_DST_HEADER,
22+
tls, Addr, Conditional, CANONICAL_DST_HEADER,
2523
};
2624
use std::{net::SocketAddr, str::FromStr};
2725

@@ -30,6 +28,8 @@ pub type Logical = crate::logical::Logical<Version>;
3028
pub type Concrete = crate::logical::Concrete<Version>;
3129
pub type Endpoint = crate::endpoint::Endpoint<Version>;
3230

31+
pub type Connect = self::endpoint::Connect<Endpoint>;
32+
3333
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
3434
struct Route {
3535
logical: Logical,
@@ -150,18 +150,6 @@ impl Param<client::Settings> for Endpoint {
150150
}
151151
}
152152

153-
impl Param<Option<SessionProtocol>> for Endpoint {
154-
fn param(&self) -> Option<SessionProtocol> {
155-
match self.protocol {
156-
Version::H2 => Some(SessionProtocol::Http2),
157-
Version::Http1 => match self.metadata.protocol_hint() {
158-
ProtocolHint::Http2 => Some(SessionProtocol::Http2),
159-
ProtocolHint::Unknown => Some(SessionProtocol::Http1),
160-
},
161-
}
162-
}
163-
}
164-
165153
impl tap::Inspect for Endpoint {
166154
fn src_addr<B>(&self, req: &Request<B>) -> Option<SocketAddr> {
167155
req.extensions().get::<ClientHandle>().map(|c| c.addr)

‎linkerd/app/outbound/src/http/endpoint.rs‎

Lines changed: 134 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
use super::{NewRequireIdentity, NewStripProxyError, ProxyConnectionClose};
2-
use crate::Outbound;
2+
use crate::{tcp::opaque_transport, Outbound};
33
use linkerd_app_core::{
44
classify, config, errors, http_tracing, metrics,
55
proxy::{http, tap},
66
svc::{self, ExtractParam},
7-
tls, Error, Result, CANONICAL_DST_HEADER,
7+
tls,
8+
transport::{self, Remote, ServerAddr},
9+
transport_header::SessionProtocol,
10+
Error, Result, CANONICAL_DST_HEADER,
811
};
912
use tokio::io;
1013

@@ -13,6 +16,12 @@ struct ClientRescue {
1316
emit_headers: bool,
1417
}
1518

19+
#[derive(Clone, Debug)]
20+
pub struct Connect<T> {
21+
version: http::Version,
22+
inner: T,
23+
}
24+
1625
impl<C> Outbound<C> {
1726
pub fn push_http_endpoint<T, B>(self) -> Outbound<svc::ArcNewHttp<T, B>>
1827
where
@@ -24,7 +33,7 @@ impl<C> Outbound<C> {
2433
+ tap::Inspect,
2534
B: http::HttpBody<Error = Error> + std::fmt::Debug + Default + Send + 'static,
2635
B::Data: Send + 'static,
27-
C: svc::Service<T> + Clone + Send + Sync + Unpin + 'static,
36+
C: svc::Service<Connect<T>> + Clone + Send + Sync + Unpin + 'static,
2837
C::Response: io::AsyncRead + io::AsyncWrite + Send + Unpin,
2938
C::Error: Into<Error>,
3039
C::Future: Send + Unpin + 'static,
@@ -40,7 +49,9 @@ impl<C> Outbound<C> {
4049
// Initiates an HTTP client on the underlying transport. Prior-knowledge HTTP/2
4150
// is typically used (i.e. when communicating with other proxies); though
4251
// HTTP/1.x fallback is supported as needed.
43-
connect
52+
svc::stack(connect.into_inner().into_service())
53+
.check_service::<Connect<T>>()
54+
.push_map_target(|(version, inner)| Connect { version, inner })
4455
.push(http::client::layer(h1_settings, h2_settings))
4556
.push_on_service(svc::MapErr::layer(Into::<Error>::into))
4657
.check_service::<T>()
@@ -133,17 +144,72 @@ impl errors::HttpRescue<Error> for ClientRescue {
133144
}
134145
}
135146

147+
// === impl Connect ===
148+
149+
impl<T> svc::Param<Option<SessionProtocol>> for Connect<T> {
150+
#[inline]
151+
fn param(&self) -> Option<SessionProtocol> {
152+
match self.version {
153+
http::Version::Http1 => Some(SessionProtocol::Http1),
154+
http::Version::H2 => Some(SessionProtocol::Http2),
155+
}
156+
}
157+
}
158+
159+
impl<T: svc::Param<Remote<ServerAddr>>> svc::Param<Remote<ServerAddr>> for Connect<T> {
160+
#[inline]
161+
fn param(&self) -> Remote<ServerAddr> {
162+
self.inner.param()
163+
}
164+
}
165+
166+
impl<T: svc::Param<tls::ConditionalClientTls>> svc::Param<tls::ConditionalClientTls>
167+
for Connect<T>
168+
{
169+
#[inline]
170+
fn param(&self) -> tls::ConditionalClientTls {
171+
self.inner.param()
172+
}
173+
}
174+
175+
impl<T: svc::Param<Option<opaque_transport::PortOverride>>>
176+
svc::Param<Option<opaque_transport::PortOverride>> for Connect<T>
177+
{
178+
#[inline]
179+
fn param(&self) -> Option<opaque_transport::PortOverride> {
180+
self.inner.param()
181+
}
182+
}
183+
184+
impl<T: svc::Param<Option<http::AuthorityOverride>>> svc::Param<Option<http::AuthorityOverride>>
185+
for Connect<T>
186+
{
187+
#[inline]
188+
fn param(&self) -> Option<http::AuthorityOverride> {
189+
self.inner.param()
190+
}
191+
}
192+
193+
impl<T: svc::Param<transport::labels::Key>> svc::Param<transport::labels::Key> for Connect<T> {
194+
#[inline]
195+
fn param(&self) -> transport::labels::Key {
196+
self.inner.param()
197+
}
198+
}
199+
136200
#[cfg(test)]
137201
mod test {
138202
use super::*;
139-
use crate::{http, test_util::*, transport::addrs::*};
203+
use crate::{http, test_util::*};
204+
use ::http::header::{CONNECTION, UPGRADE};
140205
use linkerd_app_core::{
141206
io,
142207
proxy::api_resolve::Metadata,
143208
svc::{NewService, ServiceExt},
144209
Infallible,
145210
};
146211
use std::net::SocketAddr;
212+
use support::resolver::ProtocolHint;
147213

148214
static WAS_ORIG_PROTO: &str = "request-orig-proto";
149215

@@ -155,7 +221,7 @@ mod test {
155221
let addr = SocketAddr::new([192, 0, 2, 41].into(), 2041);
156222

157223
let connect = support::connect()
158-
.endpoint_fn_boxed(addr, |_: http::Endpoint| serve(::http::Version::HTTP_11));
224+
.endpoint_fn_boxed(addr, |_: http::Connect| serve(::http::Version::HTTP_11));
159225

160226
// Build the outbound server
161227
let (rt, _shutdown) = runtime();
@@ -192,7 +258,7 @@ mod test {
192258
let addr = SocketAddr::new([192, 0, 2, 41].into(), 2042);
193259

194260
let connect = support::connect()
195-
.endpoint_fn_boxed(addr, |_: http::Endpoint| serve(::http::Version::HTTP_2));
261+
.endpoint_fn_boxed(addr, |_: http::Connect| serve(::http::Version::HTTP_2));
196262

197263
// Build the outbound server
198264
let (rt, _shutdown) = runtime();
@@ -231,7 +297,7 @@ mod test {
231297

232298
// Pretend the upstream is a proxy that supports proto upgrades...
233299
let connect = support::connect()
234-
.endpoint_fn_boxed(addr, |_: http::Endpoint| serve(::http::Version::HTTP_2));
300+
.endpoint_fn_boxed(addr, |_: http::Connect| serve(::http::Version::HTTP_2));
235301

236302
// Build the outbound server
237303
let (rt, _shutdown) = runtime();
@@ -246,13 +312,7 @@ mod test {
246312
logical_addr: None,
247313
opaque_protocol: false,
248314
tls: tls::ConditionalClientTls::None(tls::NoClientTls::Disabled),
249-
metadata: Metadata::new(
250-
None,
251-
support::resolver::ProtocolHint::Http2,
252-
None,
253-
None,
254-
None,
255-
),
315+
metadata: Metadata::new(None, ProtocolHint::Http2, None, None, None),
256316
});
257317

258318
let req = http::Request::builder()
@@ -271,6 +331,63 @@ mod test {
271331
);
272332
}
273333

334+
#[tokio::test(flavor = "current_thread")]
335+
async fn orig_proto_skipped_on_http_upgrade() {
336+
let _trace = linkerd_tracing::test::trace_init();
337+
338+
let addr = SocketAddr::new([192, 0, 2, 41].into(), 2041);
339+
340+
// Pretend the upstream is a proxy that supports proto upgrades. The service needs to
341+
// support both HTTP/1 and HTTP/2 because an HTTP/2 connection is maintained by default and
342+
// HTTP/1 connections are created as-needed.
343+
let connect = support::connect().endpoint_fn_boxed(addr, |c: http::Connect| {
344+
serve(match svc::Param::param(&c) {
345+
Some(SessionProtocol::Http1) => ::http::Version::HTTP_11,
346+
Some(SessionProtocol::Http2) => ::http::Version::HTTP_2,
347+
None => unreachable!(),
348+
})
349+
});
350+
351+
// Build the outbound server
352+
let (rt, _shutdown) = runtime();
353+
let drain = rt.drain.clone();
354+
let stack = Outbound::new(default_config(), rt)
355+
.with_stack(connect)
356+
.push_http_endpoint::<_, http::BoxBody>()
357+
.into_stack()
358+
.push_on_service(http::BoxRequest::layer())
359+
// We need the server-side upgrade layer to annotate the request so that the client
360+
// knows that an HTTP upgrade is in progress.
361+
.push_on_service(svc::layer::mk(|svc| {
362+
http::upgrade::Service::new(svc, drain.clone())
363+
}))
364+
.into_inner();
365+
366+
let svc = stack.new_service(http::Endpoint {
367+
addr: Remote(ServerAddr(addr)),
368+
protocol: http::Version::Http1,
369+
logical_addr: None,
370+
opaque_protocol: false,
371+
tls: tls::ConditionalClientTls::None(tls::NoClientTls::Disabled),
372+
metadata: Metadata::new(None, ProtocolHint::Http2, None, None, None),
373+
});
374+
375+
let req = http::Request::builder()
376+
.version(::http::Version::HTTP_11)
377+
.uri("http://foo.example.com")
378+
.extension(http::ClientHandle::new(([192, 0, 2, 101], 40200).into()).0)
379+
// The request has upgrade headers
380+
.header(CONNECTION, "upgrade")
381+
.header(UPGRADE, "linkerdrocks")
382+
.body(hyper::Body::default())
383+
.unwrap();
384+
let rsp = svc.oneshot(req).await.unwrap();
385+
assert_eq!(rsp.status(), http::StatusCode::NO_CONTENT);
386+
// The request did NOT get a linkerd upgrade header.
387+
assert!(rsp.headers().get(WAS_ORIG_PROTO).is_none());
388+
assert_eq!(rsp.version(), ::http::Version::HTTP_11);
389+
}
390+
274391
/// Tests that the the HTTP endpoint stack ignores protocol upgrade hinting for HTTP/2 traffic.
275392
#[tokio::test(flavor = "current_thread")]
276393
async fn orig_proto_http2_noop() {
@@ -280,7 +397,7 @@ mod test {
280397

281398
// Pretend the upstream is a proxy that supports proto upgrades...
282399
let connect = support::connect()
283-
.endpoint_fn_boxed(addr, |_: http::Endpoint| serve(::http::Version::HTTP_2));
400+
.endpoint_fn_boxed(addr, |_: http::Connect| serve(::http::Version::HTTP_2));
284401

285402
// Build the outbound server
286403
let (rt, _shutdown) = runtime();
@@ -295,13 +412,7 @@ mod test {
295412
logical_addr: None,
296413
opaque_protocol: false,
297414
tls: tls::ConditionalClientTls::None(tls::NoClientTls::Disabled),
298-
metadata: Metadata::new(
299-
None,
300-
support::resolver::ProtocolHint::Http2,
301-
None,
302-
None,
303-
None,
304-
),
415+
metadata: Metadata::new(None, ProtocolHint::Http2, None, None, None),
305416
});
306417

307418
let req = http::Request::builder()

‎linkerd/proxy/http/src/client.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl<C, T, B> tower::Service<T> for MakeClient<C, B>
6868
where
6969
T: Clone + Send + Sync + 'static,
7070
T: Param<Settings>,
71-
C: tower::make::MakeConnection<T> + Clone + Unpin + Send + Sync + 'static,
71+
C: tower::make::MakeConnection<(crate::Version, T)> + Clone + Unpin + Send + Sync + 'static,
7272
C::Future: Unpin + Send + 'static,
7373
C::Error: Into<Error>,
7474
C::Connection: Unpin + Send + 'static,
@@ -134,8 +134,16 @@ type RspFuture = Pin<Box<dyn Future<Output = Result<http::Response<BoxBody>>> +
134134
impl<C, T, B> tower::Service<http::Request<B>> for Client<C, T, B>
135135
where
136136
T: Clone + Send + Sync + 'static,
137+
<<<<<<< HEAD
137138
C: tower::make::MakeConnection<T> + Clone + Send + Sync + 'static,
138139
C::Connection: Unpin + Send + 'static,
140+
||||||| parent of cbb3390f (Use the connection's HTTP version in transport header (#1533))
141+
C: MakeConnection<T> + Clone + Send + Sync + 'static,
142+
C::Connection: Unpin + Send,
143+
=======
144+
C: MakeConnection<(crate::Version, T)> + Clone + Send + Sync + 'static,
145+
C::Connection: Unpin + Send,
146+
>>>>>>> cbb3390f (Use the connection's HTTP version in transport header (#1533))
139147
C::Future: Unpin + Send + 'static,
140148
C::Error: Into<Error>,
141149
B: hyper::body::HttpBody + Send + 'static,

‎linkerd/proxy/http/src/glue.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,15 @@ impl<C, T> HyperConnect<C, T> {
160160
pub(super) fn new(connect: C, target: T, absolute_form: bool) -> Self {
161161
HyperConnect {
162162
connect,
163-
absolute_form,
164163
target,
164+
absolute_form,
165165
}
166166
}
167167
}
168168

169169
impl<C, T> tower::Service<hyper::Uri> for HyperConnect<C, T>
170170
where
171-
C: tower::make::MakeConnection<T> + Clone + Send + Sync,
171+
C: MakeConnection<(crate::Version, T)> + Clone + Send + Sync,
172172
C::Error: Into<Error>,
173173
C::Future: TryFuture<Ok = C::Connection> + Unpin + Send + 'static,
174174
<C::Future as TryFuture>::Error: Into<Error>,
@@ -185,7 +185,9 @@ where
185185

186186
fn call(&mut self, _dst: hyper::Uri) -> Self::Future {
187187
HyperConnectFuture {
188-
inner: self.connect.make_connection(self.target.clone()),
188+
inner: self
189+
.connect
190+
.make_connection((crate::Version::Http1, self.target.clone())),
189191
absolute_form: self.absolute_form,
190192
}
191193
}

‎linkerd/proxy/http/src/h1.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ type RspFuture = Pin<Box<dyn Future<Output = Result<http::Response<BoxBody>>> +
6565
impl<C, T, B> Client<C, T, B>
6666
where
6767
T: Clone + Send + Sync + 'static,
68-
C: tower::make::MakeConnection<T> + Clone + Send + Sync + 'static,
68+
C: MakeConnection<(crate::Version, T)> + Clone + Send + Sync + 'static,
6969
C::Connection: Unpin + Send + 'static,
7070
C::Future: Unpin + Send + 'static,
7171
C::Error: Into<Error>,

0 commit comments

Comments
 (0)