From 9796b407ac102d79cc4dc237bc2c2911475d7d83 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Sun, 1 Dec 2024 00:00:00 +0000 Subject: [PATCH 1/2] refactor(app/inbound): remove unused `Http` parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- linkerd/app/inbound/src/http/tests.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/linkerd/app/inbound/src/http/tests.rs b/linkerd/app/inbound/src/http/tests.rs index 77cc109ff6..69534fc15f 100644 --- a/linkerd/app/inbound/src/http/tests.rs +++ b/linkerd/app/inbound/src/http/tests.rs @@ -262,9 +262,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 +307,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. @@ -675,10 +671,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( From 717b678ee97ae16c362bb71f74f16bc7cc3d7234 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Sun, 1 Dec 2024 00:00:00 +0000 Subject: [PATCH 2/2] 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 --- linkerd/app/inbound/src/http/tests.rs | 70 +++++++++++++-------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/linkerd/app/inbound/src/http/tests.rs b/linkerd/app/inbound/src/http/tests.rs index 69534fc15f..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); @@ -523,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), @@ -602,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); @@ -616,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)) @@ -626,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| { @@ -635,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)) }