From 54407b94a95f1ab12d1c63c806b1b28c85de536f Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Mon, 9 Dec 2024 11:55:29 -0500 Subject: [PATCH] chore(app/inbound): address `server::conn::Http` deprecations (#3432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this addresses hyper 1.0 deprecations in the server side of the inbound proxy's http unit test suite logic. see linkerd/linkerd2#8733 for more information. the client end of this change ends up being slightly involved, due to changes that will need to be made in linkerd_app_test::http_util. accordingly, those deprecations will be addressed in a subsequent commit. --- * refactor(app/inbound): remove unused `Http` parameter this was not being flagged as an unused variable, due to the `#[instrument]` attribute. (😉 _it's used as a field in the generated span!_) `connect_timeout(..)` doesn't use its parameter. to address some deprecations, and avoid the need for polymorphism / refactoring related to http/1 and http/2 connections being represented as distinct types in the hyper 1.0 api, we remove it. Signed-off-by: katelyn martin * chore(app/inbound): address `server::conn::Http` deprecations this addresses hyper 1.0 deprecations in the server side of the inbound proxy's http unit test suite logic. see for more information. the client end of this change ends up being slightly involved, due to changes that will need to be made in `linkerd_app_test::http_util`. accordingly, those deprecations will be addressed in a subsequent commit. Signed-off-by: katelyn martin --------- Signed-off-by: katelyn martin --- linkerd/app/inbound/src/http/tests.rs | 83 ++++++++++++--------------- 1 file changed, 36 insertions(+), 47 deletions(-) diff --git a/linkerd/app/inbound/src/http/tests.rs b/linkerd/app/inbound/src/http/tests.rs index 77cc109ff6..7f49abedc5 100644 --- a/linkerd/app/inbound/src/http/tests.rs +++ b/linkerd/app/inbound/src/http/tests.rs @@ -12,7 +12,7 @@ use linkerd_app_core::{ errors::respond::L5D_PROXY_ERROR, identity, io, metrics, proxy::http, - svc::{self, NewService, Param}, + svc::{self, http::TracingExecutor, NewService, Param}, tls, transport::{ClientAddr, OrigDstAddr, Remote, ServerAddr}, NameAddr, ProxyRuntime, @@ -47,9 +47,7 @@ where #[tokio::test(flavor = "current_thread")] async fn unmeshed_http1_hello_world() { - #[allow(deprecated)] // linkerd/linkerd2#8733 - let mut server = hyper::server::conn::Http::new(); - server.http1_only(true); + let server = hyper::server::conn::http1::Builder::new(); #[allow(deprecated)] // linkerd/linkerd2#8733 let mut client = hyper::client::conn::Builder::new(); let _trace = trace_init(); @@ -88,9 +86,7 @@ async fn unmeshed_http1_hello_world() { #[tokio::test(flavor = "current_thread")] async fn downgrade_origin_form() { // Reproduces https://github.com/linkerd/linkerd2/issues/5298 - #[allow(deprecated)] // linkerd/linkerd2#8733 - let mut server = hyper::server::conn::Http::new(); - server.http1_only(true); + let server = hyper::server::conn::http1::Builder::new(); #[allow(deprecated)] // linkerd/linkerd2#8733 let mut client = hyper::client::conn::Builder::new(); client.http2_only(true); @@ -131,9 +127,7 @@ async fn downgrade_origin_form() { #[tokio::test(flavor = "current_thread")] async fn downgrade_absolute_form() { - #[allow(deprecated)] // linkerd/linkerd2#8733 - let mut server = hyper::server::conn::Http::new(); - server.http1_only(true); + let server = hyper::server::conn::http1::Builder::new(); #[allow(deprecated)] // linkerd/linkerd2#8733 let mut client = hyper::client::conn::Builder::new(); client.http2_only(true); @@ -262,9 +256,7 @@ async fn http1_connect_timeout_meshed_response_error_header() { // Build a mock connect that sleeps longer than the default inbound // connect timeout. - #[allow(deprecated)] // linkerd/linkerd2#8733 - let server = hyper::server::conn::Http::new(); - let connect = support::connect().endpoint(Target::addr(), connect_timeout(server)); + let connect = support::connect().endpoint(Target::addr(), connect_timeout()); // Build a client using the connect that always sleeps so that responses // are GATEWAY_TIMEOUT. @@ -309,9 +301,7 @@ async fn http1_connect_timeout_unmeshed_response_error_header() { // Build a mock connect that sleeps longer than the default inbound // connect timeout. - #[allow(deprecated)] // linkerd/linkerd2#8733 - let server = hyper::server::conn::Http::new(); - let connect = support::connect().endpoint(Target::addr(), connect_timeout(server)); + let connect = support::connect().endpoint(Target::addr(), connect_timeout()); // Build a client using the connect that always sleeps so that responses // are GATEWAY_TIMEOUT. @@ -527,9 +517,7 @@ async fn grpc_response_class() { // Build a mock connector serves a gRPC server that returns errors. let connect = { - #[allow(deprecated)] // linkerd/linkerd2#8733 - let mut server = hyper::server::conn::Http::new(); - server.http2_only(true); + let server = hyper::server::conn::http2::Builder::new(TracingExecutor); support::connect().endpoint_fn_boxed( Target::addr(), grpc_status_server(server, tonic::Code::Unknown), @@ -606,9 +594,8 @@ async fn grpc_response_class() { } #[tracing::instrument] -#[allow(deprecated)] // linkerd/linkerd2#8733 fn hello_server( - http: hyper::server::conn::Http, + server: hyper::server::conn::http1::Builder, ) -> impl Fn(Remote) -> io::Result { move |endpoint| { let span = tracing::info_span!("hello_server", ?endpoint); @@ -620,7 +607,8 @@ fn hello_server( Ok::<_, io::Error>(Response::new(Body::from("Hello world!"))) }); tokio::spawn( - http.serve_connection(server_io, hello_svc) + server + .serve_connection(server_io, hello_svc) .in_current_span(), ); Ok(io::BoxedIo::new(client_io)) @@ -630,7 +618,7 @@ fn hello_server( #[tracing::instrument] #[allow(deprecated)] // linkerd/linkerd2#8733 fn grpc_status_server( - http: hyper::server::conn::Http, + server: hyper::server::conn::http2::Builder, status: tonic::Code, ) -> impl Fn(Remote) -> io::Result { move |endpoint| { @@ -639,26 +627,30 @@ fn grpc_status_server( tracing::info!("mock connecting"); let (client_io, server_io) = support::io::duplex(4096); tokio::spawn( - http.serve_connection( - server_io, - hyper::service::service_fn(move |request: Request| async move { - tracing::info!(?request); - let (mut tx, rx) = Body::channel(); - tokio::spawn(async move { - let mut trls = ::http::HeaderMap::new(); - trls.insert("grpc-status", (status as u32).to_string().parse().unwrap()); - tx.send_trailers(trls).await - }); - Ok::<_, io::Error>( - http::Response::builder() - .version(::http::Version::HTTP_2) - .header("content-type", "application/grpc") - .body(rx) - .unwrap(), - ) - }), - ) - .in_current_span(), + server + .serve_connection( + server_io, + hyper::service::service_fn(move |request: Request| async move { + tracing::info!(?request); + let (mut tx, rx) = Body::channel(); + tokio::spawn(async move { + let mut trls = ::http::HeaderMap::new(); + trls.insert( + "grpc-status", + (status as u32).to_string().parse().unwrap(), + ); + tx.send_trailers(trls).await + }); + Ok::<_, io::Error>( + http::Response::builder() + .version(::http::Version::HTTP_2) + .header("content-type", "application/grpc") + .body(rx) + .unwrap(), + ) + }), + ) + .in_current_span(), ); Ok(io::BoxedIo::new(client_io)) } @@ -675,10 +667,7 @@ fn connect_error() -> impl Fn(Remote) -> io::Result { } #[tracing::instrument] -#[allow(deprecated)] // linkerd/linkerd2#8733 -fn connect_timeout( - http: hyper::server::conn::Http, -) -> Box) -> ConnectFuture + Send> { +fn connect_timeout() -> Box) -> ConnectFuture + Send> { Box::new(move |endpoint| { let span = tracing::info_span!("connect_timeout", ?endpoint); Box::pin(