From 016fbbe193d8b21908ef45be570a3dc6fb4300f1 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 1 Nov 2024 00:00:00 +0000 Subject: [PATCH 1/6] refactor(proxy/http): move types to `linkerd-http-classify` (#3382) in this case we already had a crate defining the classify traits, but the http body and other assorted middleware were defined in `linkerd-proxy-http`. this commit moves those types to the `linkerd-http-classify` crate, which astute readers may notice, is a concrete step towards simplifying the `linkerd-proxy-http` crate's upgrade process. one small detail worth calling out: we implement `http_body::Body` directly, to avoid taking on a `hyper` dependency. otherwise, nothing has changed in the `channel`, `gate`, and `insert` middleware. Signed-off-by: katelyn martin --- Cargo.lock | 10 ++++++++++ linkerd/http/classify/Cargo.toml | 12 ++++++++++++ .../src/classify => http/classify/src}/channel.rs | 4 ++-- .../http/src/classify => http/classify/src}/gate.rs | 2 +- .../src/classify => http/classify/src}/insert.rs | 0 linkerd/http/classify/src/lib.rs | 10 ++++++++++ linkerd/proxy/http/src/classify.rs | 10 ---------- linkerd/proxy/http/src/lib.rs | 2 +- 8 files changed, 36 insertions(+), 14 deletions(-) rename linkerd/{proxy/http/src/classify => http/classify/src}/channel.rs (98%) rename linkerd/{proxy/http/src/classify => http/classify/src}/gate.rs (98%) rename linkerd/{proxy/http/src/classify => http/classify/src}/insert.rs (100%) delete mode 100644 linkerd/proxy/http/src/classify.rs diff --git a/Cargo.lock b/Cargo.lock index 87cf75f982..cdb54a66e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1705,8 +1705,18 @@ dependencies = [ name = "linkerd-http-classify" version = "0.1.0" dependencies = [ + "futures", "http", + "http-body", "linkerd-error", + "linkerd-http-box", + "linkerd-stack", + "linkerd-tracing", + "pin-project", + "tokio", + "tokio-test", + "tower-test", + "tracing", ] [[package]] diff --git a/linkerd/http/classify/Cargo.toml b/linkerd/http/classify/Cargo.toml index 051f80c32f..8eee4380ca 100644 --- a/linkerd/http/classify/Cargo.toml +++ b/linkerd/http/classify/Cargo.toml @@ -7,6 +7,18 @@ edition = "2021" publish = false [dependencies] +futures = { version = "0.3", default-features = false } http = "0.2" +http-body = "0.4" +pin-project = "1" +tokio = { version = "1", default-features = false } +tracing = "0.1" linkerd-error = { path = "../../error" } +linkerd-http-box = { path = "../../http/box" } +linkerd-stack = { path = "../../stack" } + +[dev-dependencies] +tokio-test = "0.4" +tower-test = "0.4" +linkerd-tracing = { path = "../../tracing", features = ["ansi"] } diff --git a/linkerd/proxy/http/src/classify/channel.rs b/linkerd/http/classify/src/channel.rs similarity index 98% rename from linkerd/proxy/http/src/classify/channel.rs rename to linkerd/http/classify/src/channel.rs index 413887345b..247e7b282b 100644 --- a/linkerd/proxy/http/src/classify/channel.rs +++ b/linkerd/http/classify/src/channel.rs @@ -207,10 +207,10 @@ where // === impl ResponseBody === -impl hyper::body::HttpBody for ResponseBody +impl http_body::Body for ResponseBody where C: ClassifyEos + Unpin, - B: hyper::body::HttpBody, + B: http_body::Body, { type Data = B::Data; type Error = B::Error; diff --git a/linkerd/proxy/http/src/classify/gate.rs b/linkerd/http/classify/src/gate.rs similarity index 98% rename from linkerd/proxy/http/src/classify/gate.rs rename to linkerd/http/classify/src/gate.rs index 709a0a135f..726d8bf493 100644 --- a/linkerd/proxy/http/src/classify/gate.rs +++ b/linkerd/http/classify/src/gate.rs @@ -1,4 +1,4 @@ -use crate::classify::{BroadcastClassification, ClassifyResponse}; +use crate::{channel::BroadcastClassification, ClassifyResponse}; use linkerd_stack::{gate, layer, ExtractParam, Gate, NewService}; use std::marker::PhantomData; use tokio::sync::mpsc; diff --git a/linkerd/proxy/http/src/classify/insert.rs b/linkerd/http/classify/src/insert.rs similarity index 100% rename from linkerd/proxy/http/src/classify/insert.rs rename to linkerd/http/classify/src/insert.rs diff --git a/linkerd/http/classify/src/lib.rs b/linkerd/http/classify/src/lib.rs index 5003776444..5640fe43a9 100644 --- a/linkerd/http/classify/src/lib.rs +++ b/linkerd/http/classify/src/lib.rs @@ -3,6 +3,16 @@ use linkerd_error::Error; +pub use self::{ + channel::{BroadcastClassification, NewBroadcastClassification, Tx}, + gate::{NewClassifyGate, NewClassifyGateSet}, + insert::{InsertClassifyResponse, NewInsertClassifyResponse}, +}; + +pub mod channel; +pub mod gate; +mod insert; + /// Determines how a request's response should be classified. pub trait Classify { type Class: Clone + Send + Sync + 'static; diff --git a/linkerd/proxy/http/src/classify.rs b/linkerd/proxy/http/src/classify.rs deleted file mode 100644 index 487e047981..0000000000 --- a/linkerd/proxy/http/src/classify.rs +++ /dev/null @@ -1,10 +0,0 @@ -pub mod channel; -pub mod gate; -mod insert; - -pub use self::{ - channel::{BroadcastClassification, NewBroadcastClassification, Tx}, - gate::{NewClassifyGate, NewClassifyGateSet}, - insert::{InsertClassifyResponse, NewInsertClassifyResponse}, -}; -pub use linkerd_http_classify::*; diff --git a/linkerd/proxy/http/src/lib.rs b/linkerd/proxy/http/src/lib.rs index 6f889183ce..2e46e77a7e 100644 --- a/linkerd/proxy/http/src/lib.rs +++ b/linkerd/proxy/http/src/lib.rs @@ -5,7 +5,6 @@ use http::{header::AsHeaderName, uri::Authority}; use linkerd_error::Error; pub mod balance; -pub mod classify; pub mod client; pub mod client_handle; pub mod detect; @@ -46,6 +45,7 @@ pub use http::{ }; pub use hyper::body::HttpBody; pub use linkerd_http_box::{BoxBody, BoxRequest, BoxResponse, EraseResponse}; +pub use linkerd_http_classify as classify; pub use linkerd_http_executor::TracingExecutor; pub use linkerd_http_insert as insert; pub use linkerd_http_version::{self as version, Version}; From 63bcbe2167c49cfbbb77a04a0ba07f32cf548765 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 1 Nov 2024 00:00:00 +0000 Subject: [PATCH 2/6] refactor(proxy/http): create `linkerd-http-retain` crate (#3382) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this moves the `Retain` middleware from `linkerd-proxy-http` into a new `linkerd-http-retain` crate. as previously, reƫxports are added to make this a backwards compatible change. this moves another http body middleware out of the proxy's core http crate. great news. Signed-off-by: katelyn martin --- Cargo.lock | 12 +++++++++++ Cargo.toml | 1 + linkerd/http/retain/Cargo.toml | 20 +++++++++++++++++++ .../src/retain.rs => http/retain/src/lib.rs} | 6 ++++-- linkerd/proxy/http/Cargo.toml | 1 + linkerd/proxy/http/src/lib.rs | 3 +-- 6 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 linkerd/http/retain/Cargo.toml rename linkerd/{proxy/http/src/retain.rs => http/retain/src/lib.rs} (93%) diff --git a/Cargo.lock b/Cargo.lock index cdb54a66e8..feab09b5ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1782,6 +1782,17 @@ dependencies = [ "tokio", ] +[[package]] +name = "linkerd-http-retain" +version = "0.1.0" +dependencies = [ + "http", + "http-body", + "linkerd-stack", + "pin-project", + "tower", +] + [[package]] name = "linkerd-http-retry" version = "0.1.0" @@ -2188,6 +2199,7 @@ dependencies = [ "linkerd-http-executor", "linkerd-http-h2", "linkerd-http-insert", + "linkerd-http-retain", "linkerd-http-version", "linkerd-io", "linkerd-proxy-balance", diff --git a/Cargo.toml b/Cargo.toml index fa2e89ba4b..0e1d82e3dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "linkerd/http/insert", "linkerd/http/metrics", "linkerd/http/prom", + "linkerd/http/retain", "linkerd/http/retry", "linkerd/http/route", "linkerd/http/version", diff --git a/linkerd/http/retain/Cargo.toml b/linkerd/http/retain/Cargo.toml new file mode 100644 index 0000000000..dfbdda108c --- /dev/null +++ b/linkerd/http/retain/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "linkerd-http-retain" +version = "0.1.0" +authors = ["Linkerd Developers "] +license = "Apache-2.0" +edition = "2021" +publish = false +description = """ +Tower middleware to manage service lifecycles. + +This is mostly intended to support cache eviction. +""" + +[dependencies] +http = "0.2" +http-body = "0.4" +pin-project = "1" +tower = { version = "0.4", default-features = false } + +linkerd-stack = { path = "../../stack" } diff --git a/linkerd/proxy/http/src/retain.rs b/linkerd/http/retain/src/lib.rs similarity index 93% rename from linkerd/proxy/http/src/retain.rs rename to linkerd/http/retain/src/lib.rs index e5ed0f9deb..fe60f2c9ea 100644 --- a/linkerd/proxy/http/src/retain.rs +++ b/linkerd/http/retain/src/lib.rs @@ -1,5 +1,7 @@ -//! Provides a middleware that holds an inner service as long as responses are -//! being processed. +//! Tower middleware to manage service lifecycles. +//! +//! Provides a [`Retain`] middleware that holds an inner service as long as responses are +//! being processed. This is mostly intended to support cache eviction. use linkerd_stack::layer; use pin_project::pin_project; diff --git a/linkerd/proxy/http/Cargo.toml b/linkerd/proxy/http/Cargo.toml index 57f5be3f3d..de0703f0be 100644 --- a/linkerd/proxy/http/Cargo.toml +++ b/linkerd/proxy/http/Cargo.toml @@ -46,6 +46,7 @@ linkerd-http-classify = { path = "../../http/classify" } linkerd-http-executor = { path = "../../http/executor" } linkerd-http-h2 = { path = "../../http/h2" } linkerd-http-insert = { path = "../../http/insert" } +linkerd-http-retain = { path = "../../http/retain" } linkerd-http-version = { path = "../../http/version" } linkerd-io = { path = "../../io" } linkerd-proxy-balance = { path = "../balance" } diff --git a/linkerd/proxy/http/src/lib.rs b/linkerd/proxy/http/src/lib.rs index 2e46e77a7e..72a10d3b4a 100644 --- a/linkerd/proxy/http/src/lib.rs +++ b/linkerd/proxy/http/src/lib.rs @@ -15,7 +15,6 @@ mod header_from_target; pub mod normalize_uri; pub mod orig_proto; mod override_authority; -mod retain; mod server; pub mod stream_timeouts; pub mod strip_header; @@ -33,7 +32,6 @@ pub use self::{ header_from_target::NewHeaderFromTarget, normalize_uri::{MarkAbsoluteForm, NewNormalizeUri}, override_authority::{AuthorityOverride, NewOverrideAuthority}, - retain::Retain, server::{NewServeHttp, Params as ServerParams, ServeHttp}, stream_timeouts::{EnforceTimeouts, StreamTimeouts}, strip_header::StripHeader, @@ -48,6 +46,7 @@ pub use linkerd_http_box::{BoxBody, BoxRequest, BoxResponse, EraseResponse}; pub use linkerd_http_classify as classify; pub use linkerd_http_executor::TracingExecutor; pub use linkerd_http_insert as insert; +pub use linkerd_http_retain::{self as retain, Retain}; pub use linkerd_http_version::{self as version, Version}; #[derive(Clone, Debug)] From 6660f71ff43ca0574644ead32d72f2630f2fbbc2 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 1 Nov 2024 00:00:00 +0000 Subject: [PATCH 3/6] refactor(proxy/http): create `linkerd-http-stream-timeouts` crate (#3382) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this commit outlines the stream timeout middleware, pulling it out of `linkerd-proxy-http` and into a standalone crate. again, reƫxports are added to make this a backwards compatible change. Signed-off-by: katelyn martin --- Cargo.lock | 17 ++++++++++++++ Cargo.toml | 1 + linkerd/http/stream-timeouts/Cargo.toml | 23 +++++++++++++++++++ .../stream-timeouts/src/lib.rs} | 12 ++++++---- linkerd/proxy/http/Cargo.toml | 1 + linkerd/proxy/http/src/lib.rs | 3 +-- 6 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 linkerd/http/stream-timeouts/Cargo.toml rename linkerd/{proxy/http/src/stream_timeouts.rs => http/stream-timeouts/src/lib.rs} (98%) diff --git a/Cargo.lock b/Cargo.lock index feab09b5ec..e175418597 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1829,6 +1829,22 @@ dependencies = [ "url", ] +[[package]] +name = "linkerd-http-stream-timeouts" +version = "0.1.0" +dependencies = [ + "futures", + "http", + "http-body", + "linkerd-error", + "linkerd-stack", + "parking_lot", + "pin-project", + "thiserror", + "tokio", + "tracing", +] + [[package]] name = "linkerd-http-version" version = "0.1.0" @@ -2200,6 +2216,7 @@ dependencies = [ "linkerd-http-h2", "linkerd-http-insert", "linkerd-http-retain", + "linkerd-http-stream-timeouts", "linkerd-http-version", "linkerd-io", "linkerd-proxy-balance", diff --git a/Cargo.toml b/Cargo.toml index 0e1d82e3dc..aeb07230c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,7 @@ members = [ "linkerd/http/retain", "linkerd/http/retry", "linkerd/http/route", + "linkerd/http/stream-timeouts", "linkerd/http/version", "linkerd/identity", "linkerd/idle-cache", diff --git a/linkerd/http/stream-timeouts/Cargo.toml b/linkerd/http/stream-timeouts/Cargo.toml new file mode 100644 index 0000000000..318dcca831 --- /dev/null +++ b/linkerd/http/stream-timeouts/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "linkerd-http-stream-timeouts" +version = "0.1.0" +authors = ["Linkerd Developers "] +license = "Apache-2.0" +edition = "2021" +publish = false +description = """ +Tower middleware to express deadlines on streams. +""" + +[dependencies] +futures = { version = "0.3", default-features = false } +http = "0.2" +http-body = "0.4" +parking_lot = "0.12" +pin-project = "1" +thiserror = "1" +tokio = { version = "1", default-features = false } +tracing = "0.1" + +linkerd-error = { path = "../../error" } +linkerd-stack = { path = "../../stack" } diff --git a/linkerd/proxy/http/src/stream_timeouts.rs b/linkerd/http/stream-timeouts/src/lib.rs similarity index 98% rename from linkerd/proxy/http/src/stream_timeouts.rs rename to linkerd/http/stream-timeouts/src/lib.rs index 46c074afa7..90ffc99ea9 100644 --- a/linkerd/proxy/http/src/stream_timeouts.rs +++ b/linkerd/http/stream-timeouts/src/lib.rs @@ -1,3 +1,7 @@ +//! Tower middleware to express deadlines on streams. +//! +//! See [`EnforceTimeouts`]. + use futures::FutureExt; use linkerd_error::{Error, Result}; use linkerd_stack as svc; @@ -345,9 +349,9 @@ where // === impl RequestBody === -impl crate::HttpBody for RequestBody +impl http_body::Body for RequestBody where - B: crate::HttpBody, + B: http_body::Body, { type Data = B::Data; type Error = Error; @@ -405,9 +409,9 @@ where // === impl ResponseBody === -impl crate::HttpBody for ResponseBody +impl http_body::Body for ResponseBody where - B: crate::HttpBody, + B: http_body::Body, { type Data = B::Data; type Error = Error; diff --git a/linkerd/proxy/http/Cargo.toml b/linkerd/proxy/http/Cargo.toml index de0703f0be..5ab41220a4 100644 --- a/linkerd/proxy/http/Cargo.toml +++ b/linkerd/proxy/http/Cargo.toml @@ -47,6 +47,7 @@ linkerd-http-executor = { path = "../../http/executor" } linkerd-http-h2 = { path = "../../http/h2" } linkerd-http-insert = { path = "../../http/insert" } linkerd-http-retain = { path = "../../http/retain" } +linkerd-http-stream-timeouts = { path = "../../http/stream-timeouts" } linkerd-http-version = { path = "../../http/version" } linkerd-io = { path = "../../io" } linkerd-proxy-balance = { path = "../balance" } diff --git a/linkerd/proxy/http/src/lib.rs b/linkerd/proxy/http/src/lib.rs index 72a10d3b4a..2a1cb1da3b 100644 --- a/linkerd/proxy/http/src/lib.rs +++ b/linkerd/proxy/http/src/lib.rs @@ -16,7 +16,6 @@ pub mod normalize_uri; pub mod orig_proto; mod override_authority; mod server; -pub mod stream_timeouts; pub mod strip_header; pub mod timeout; pub mod upgrade; @@ -33,7 +32,6 @@ pub use self::{ normalize_uri::{MarkAbsoluteForm, NewNormalizeUri}, override_authority::{AuthorityOverride, NewOverrideAuthority}, server::{NewServeHttp, Params as ServerParams, ServeHttp}, - stream_timeouts::{EnforceTimeouts, StreamTimeouts}, strip_header::StripHeader, timeout::{NewTimeout, ResponseTimeout, ResponseTimeoutError}, }; @@ -47,6 +45,7 @@ pub use linkerd_http_classify as classify; pub use linkerd_http_executor::TracingExecutor; pub use linkerd_http_insert as insert; pub use linkerd_http_retain::{self as retain, Retain}; +pub use linkerd_http_stream_timeouts::{self as stream_timeouts, EnforceTimeouts, StreamTimeouts}; pub use linkerd_http_version::{self as version, Version}; #[derive(Clone, Debug)] From 269d72bd602820341d985e704a3c0e65045d1d97 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 1 Nov 2024 00:00:00 +0000 Subject: [PATCH 4/6] refactor(proxy/http): create `linkerd-http-override-authority` crate (#3382) NB: based on #3379 and #3380. this pull the `override_authority` submodule out of `linkerd-http-proxy` and into a standalone crate. Signed-off-by: katelyn martin --- Cargo.lock | 11 ++++++++ Cargo.toml | 1 + linkerd/http/override-authority/Cargo.toml | 17 ++++++++++++ .../override-authority/src/lib.rs} | 26 ++++++++++++++++++- linkerd/proxy/http/Cargo.toml | 1 + linkerd/proxy/http/src/lib.rs | 23 +--------------- linkerd/proxy/http/src/normalize_uri.rs | 2 +- 7 files changed, 57 insertions(+), 24 deletions(-) create mode 100644 linkerd/http/override-authority/Cargo.toml rename linkerd/{proxy/http/src/override_authority.rs => http/override-authority/src/lib.rs} (75%) diff --git a/Cargo.lock b/Cargo.lock index e175418597..856f80176c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1763,6 +1763,16 @@ dependencies = [ "tracing", ] +[[package]] +name = "linkerd-http-override-authority" +version = "0.1.0" +dependencies = [ + "http", + "linkerd-stack", + "tower", + "tracing", +] + [[package]] name = "linkerd-http-prom" version = "0.1.0" @@ -2215,6 +2225,7 @@ dependencies = [ "linkerd-http-executor", "linkerd-http-h2", "linkerd-http-insert", + "linkerd-http-override-authority", "linkerd-http-retain", "linkerd-http-stream-timeouts", "linkerd-http-version", diff --git a/Cargo.toml b/Cargo.toml index aeb07230c3..d55108c730 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ members = [ "linkerd/http/h2", "linkerd/http/insert", "linkerd/http/metrics", + "linkerd/http/override-authority", "linkerd/http/prom", "linkerd/http/retain", "linkerd/http/retry", diff --git a/linkerd/http/override-authority/Cargo.toml b/linkerd/http/override-authority/Cargo.toml new file mode 100644 index 0000000000..368f1bab03 --- /dev/null +++ b/linkerd/http/override-authority/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "linkerd-http-override-authority" +version = "0.1.0" +authors = ["Linkerd Developers "] +license = "Apache-2.0" +edition = "2021" +publish = false +description = """ +Tower middleware to override request authorities. +""" + +[dependencies] +http = "0.2" +tower = { version = "0.4", default-features = false } +tracing = "0.1" + +linkerd-stack = { path = "../../stack" } diff --git a/linkerd/proxy/http/src/override_authority.rs b/linkerd/http/override-authority/src/lib.rs similarity index 75% rename from linkerd/proxy/http/src/override_authority.rs rename to linkerd/http/override-authority/src/lib.rs index dc9983d6ac..e4efd3216b 100644 --- a/linkerd/proxy/http/src/override_authority.rs +++ b/linkerd/http/override-authority/src/lib.rs @@ -1,4 +1,7 @@ -use http::{header::AsHeaderName, uri::Authority}; +use http::{ + header::AsHeaderName, + uri::{self, Authority}, +}; use linkerd_stack::{layer, NewService, Param}; use std::{ fmt, @@ -23,6 +26,27 @@ pub struct OverrideAuthority { inner: S, } +/// Sets the [`Authority`] of the given URI. +pub fn set_authority(uri: &mut uri::Uri, auth: Authority) { + let mut parts = uri::Parts::from(std::mem::take(uri)); + + parts.authority = Some(auth); + + // If this was an origin-form target (path only), + // then we can't *only* set the authority, as that's + // an illegal target (such as `example.com/docs`). + // + // But don't set a scheme if this was authority-form (CONNECT), + // since that would change its meaning (like `https://example.com`). + if parts.path_and_query.is_some() { + parts.scheme = Some(http::uri::Scheme::HTTP); + } + + let new = http::uri::Uri::from_parts(parts).expect("absolute uri"); + + *uri = new; +} + // === impl NewOverrideAuthority === impl NewOverrideAuthority { diff --git a/linkerd/proxy/http/Cargo.toml b/linkerd/proxy/http/Cargo.toml index 5ab41220a4..785740e1c3 100644 --- a/linkerd/proxy/http/Cargo.toml +++ b/linkerd/proxy/http/Cargo.toml @@ -46,6 +46,7 @@ linkerd-http-classify = { path = "../../http/classify" } linkerd-http-executor = { path = "../../http/executor" } linkerd-http-h2 = { path = "../../http/h2" } linkerd-http-insert = { path = "../../http/insert" } +linkerd-http-override-authority = { path = "../../http/override-authority" } linkerd-http-retain = { path = "../../http/retain" } linkerd-http-stream-timeouts = { path = "../../http/stream-timeouts" } linkerd-http-version = { path = "../../http/version" } diff --git a/linkerd/proxy/http/src/lib.rs b/linkerd/proxy/http/src/lib.rs index 2a1cb1da3b..63e6bf8cae 100644 --- a/linkerd/proxy/http/src/lib.rs +++ b/linkerd/proxy/http/src/lib.rs @@ -14,7 +14,6 @@ pub mod h2; mod header_from_target; pub mod normalize_uri; pub mod orig_proto; -mod override_authority; mod server; pub mod strip_header; pub mod timeout; @@ -30,7 +29,6 @@ pub use self::{ detect::DetectHttp, header_from_target::NewHeaderFromTarget, normalize_uri::{MarkAbsoluteForm, NewNormalizeUri}, - override_authority::{AuthorityOverride, NewOverrideAuthority}, server::{NewServeHttp, Params as ServerParams, ServeHttp}, strip_header::StripHeader, timeout::{NewTimeout, ResponseTimeout, ResponseTimeoutError}, @@ -44,6 +42,7 @@ pub use linkerd_http_box::{BoxBody, BoxRequest, BoxResponse, EraseResponse}; pub use linkerd_http_classify as classify; pub use linkerd_http_executor::TracingExecutor; pub use linkerd_http_insert as insert; +pub use linkerd_http_override_authority::{AuthorityOverride, NewOverrideAuthority}; pub use linkerd_http_retain::{self as retain, Retain}; pub use linkerd_http_stream_timeouts::{self as stream_timeouts, EnforceTimeouts, StreamTimeouts}; pub use linkerd_http_version::{self as version, Version}; @@ -92,26 +91,6 @@ where v.to_str().ok()?.parse().ok() } -fn set_authority(uri: &mut uri::Uri, auth: uri::Authority) { - let mut parts = uri::Parts::from(std::mem::take(uri)); - - parts.authority = Some(auth); - - // If this was an origin-form target (path only), - // then we can't *only* set the authority, as that's - // an illegal target (such as `example.com/docs`). - // - // But don't set a scheme if this was authority-form (CONNECT), - // since that would change its meaning (like `https://example.com`). - if parts.path_and_query.is_some() { - parts.scheme = Some(http::uri::Scheme::HTTP); - } - - let new = http::uri::Uri::from_parts(parts).expect("absolute uri"); - - *uri = new; -} - fn strip_connection_headers(headers: &mut http::HeaderMap) { if let Some(val) = headers.remove(header::CONNECTION) { if let Ok(conn_header) = val.to_str() { diff --git a/linkerd/proxy/http/src/normalize_uri.rs b/linkerd/proxy/http/src/normalize_uri.rs index 830b004d23..818adcd48d 100644 --- a/linkerd/proxy/http/src/normalize_uri.rs +++ b/linkerd/proxy/http/src/normalize_uri.rs @@ -123,7 +123,7 @@ where }; trace!(%authority, "Normalizing URI"); - crate::set_authority(req.uri_mut(), authority); + linkerd_http_override_authority::set_authority(req.uri_mut(), authority); } } From 81c569ac24c8fa5eb7c2bd672fbe2f3a130e878f Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 1 Nov 2024 00:00:00 +0000 Subject: [PATCH 5/6] refactor(proxy/http): move `is_bad_request()` into `upgrade` (#3382) this is only used in once place, so as a brief chore before we move the upgrade submodule out into its own crate, we pull `is_bad_request()` next to its call site. Signed-off-by: katelyn martin --- linkerd/proxy/http/src/h1.rs | 35 --------------------------- linkerd/proxy/http/src/upgrade.rs | 39 +++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/linkerd/proxy/http/src/h1.rs b/linkerd/proxy/http/src/h1.rs index d897baf592..2941cb2ad5 100644 --- a/linkerd/proxy/http/src/h1.rs +++ b/linkerd/proxy/http/src/h1.rs @@ -222,38 +222,3 @@ pub(crate) fn is_absolute_form(uri: &Uri) -> bool { uri.scheme().is_some() } - -/// Returns if the request target is in `origin-form`. -/// -/// This is `origin-form`: `example.com` -fn is_origin_form(uri: &Uri) -> bool { - uri.scheme().is_none() && uri.path_and_query().is_none() -} - -/// Returns if the received request is definitely bad. -/// -/// Just because a request parses doesn't mean it's correct. For examples: -/// -/// - `GET example.com` -/// - `CONNECT /just-a-path -pub(crate) fn is_bad_request(req: &http::Request) -> bool { - if req.method() == http::Method::CONNECT { - // CONNECT is only valid over HTTP/1.1 - if req.version() != http::Version::HTTP_11 { - debug!("CONNECT request not valid for HTTP/1.0: {:?}", req.uri()); - return true; - } - - // CONNECT requests are only valid in authority-form. - if !is_origin_form(req.uri()) { - debug!("CONNECT request with illegal URI: {:?}", req.uri()); - return true; - } - } else if is_origin_form(req.uri()) { - // If not CONNECT, refuse any origin-form URIs - debug!("{} request with illegal URI: {:?}", req.method(), req.uri()); - return true; - } - - false -} diff --git a/linkerd/proxy/http/src/upgrade.rs b/linkerd/proxy/http/src/upgrade.rs index 088a3b7c4f..7b79b10b9d 100644 --- a/linkerd/proxy/http/src/upgrade.rs +++ b/linkerd/proxy/http/src/upgrade.rs @@ -1,6 +1,6 @@ //! HTTP/1.1 Upgrades -use crate::{glue::UpgradeBody, h1}; +use crate::glue::UpgradeBody; use futures::{ future::{self, Either}, TryFutureExt, @@ -193,7 +193,7 @@ where // // At the same time, this stuff is specifically HTTP1, so it feels // proper to not have the HTTP2 requests going through it... - if h1::is_bad_request(&req) { + if is_bad_request(&req) { let mut res = http::Response::default(); *res.status_mut() = http::StatusCode::BAD_REQUEST; return Either::Right(future::ok(res)); @@ -221,6 +221,41 @@ where } } +/// Returns if the received request is definitely bad. +/// +/// Just because a request parses doesn't mean it's correct. For examples: +/// +/// - `GET example.com` +/// - `CONNECT /just-a-path +pub(crate) fn is_bad_request(req: &http::Request) -> bool { + if req.method() == http::Method::CONNECT { + // CONNECT is only valid over HTTP/1.1 + if req.version() != http::Version::HTTP_11 { + debug!("CONNECT request not valid for HTTP/1.0: {:?}", req.uri()); + return true; + } + + // CONNECT requests are only valid in authority-form. + if !is_origin_form(req.uri()) { + debug!("CONNECT request with illegal URI: {:?}", req.uri()); + return true; + } + } else if is_origin_form(req.uri()) { + // If not CONNECT, refuse any origin-form URIs + debug!("{} request with illegal URI: {:?}", req.method(), req.uri()); + return true; + } + + false +} + +/// Returns if the request target is in `origin-form`. +/// +/// This is `origin-form`: `example.com` +fn is_origin_form(uri: &http::uri::Uri) -> bool { + uri.scheme().is_none() && uri.path_and_query().is_none() +} + /// Checks requests to determine if they want to perform an HTTP upgrade. fn wants_upgrade(req: &http::Request) -> bool { // HTTP upgrades were added in 1.1, not 1.0. From 61d2a6305d3b972a300ade3a948d441da527459a Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 1 Nov 2024 00:00:00 +0000 Subject: [PATCH 6/6] refactor(proxy/http): create `linkerd-http-upgrade` crate (#3382) this moves the inter-related `upgrade` and `glue` submodules out of the `linkerd-proxy-http` library and into a new standalone crate. Signed-off-by: katelyn martin --- Cargo.lock | 23 +++++++++++++++ Cargo.toml | 1 + linkerd/http/upgrade/Cargo.toml | 29 +++++++++++++++++++ .../{proxy/http => http/upgrade}/src/glue.rs | 9 +++--- linkerd/http/upgrade/src/lib.rs | 29 +++++++++++++++++++ .../http => http/upgrade}/src/upgrade.rs | 1 + linkerd/proxy/http/Cargo.toml | 1 + linkerd/proxy/http/src/h1.rs | 12 ++++---- linkerd/proxy/http/src/lib.rs | 25 +--------------- linkerd/proxy/http/src/orig_proto.rs | 8 +++-- linkerd/proxy/http/src/server.rs | 8 +++-- 11 files changed, 107 insertions(+), 39 deletions(-) create mode 100644 linkerd/http/upgrade/Cargo.toml rename linkerd/{proxy/http => http/upgrade}/src/glue.rs (95%) create mode 100644 linkerd/http/upgrade/src/lib.rs rename linkerd/{proxy/http => http/upgrade}/src/upgrade.rs (99%) diff --git a/Cargo.lock b/Cargo.lock index 856f80176c..10fd510b21 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1855,6 +1855,28 @@ dependencies = [ "tracing", ] +[[package]] +name = "linkerd-http-upgrade" +version = "0.1.0" +dependencies = [ + "bytes", + "drain", + "futures", + "http", + "http-body", + "hyper", + "linkerd-duplex", + "linkerd-error", + "linkerd-http-version", + "linkerd-io", + "linkerd-stack", + "pin-project", + "tokio", + "tower", + "tracing", + "try-lock", +] + [[package]] name = "linkerd-http-version" version = "0.1.0" @@ -2228,6 +2250,7 @@ dependencies = [ "linkerd-http-override-authority", "linkerd-http-retain", "linkerd-http-stream-timeouts", + "linkerd-http-upgrade", "linkerd-http-version", "linkerd-io", "linkerd-proxy-balance", diff --git a/Cargo.toml b/Cargo.toml index d55108c730..ff1dfef78a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ members = [ "linkerd/http/retry", "linkerd/http/route", "linkerd/http/stream-timeouts", + "linkerd/http/upgrade", "linkerd/http/version", "linkerd/identity", "linkerd/idle-cache", diff --git a/linkerd/http/upgrade/Cargo.toml b/linkerd/http/upgrade/Cargo.toml new file mode 100644 index 0000000000..de3dbb427d --- /dev/null +++ b/linkerd/http/upgrade/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "linkerd-http-upgrade" +version = "0.1.0" +authors = ["Linkerd Developers "] +license = "Apache-2.0" +edition = "2021" +publish = false +description = """ +Facilities for HTTP/1 upgrades. +""" + +[dependencies] +bytes = "1" +drain = "0.1" +futures = { version = "0.3", default-features = false } +http = "0.2" +http-body = "0.4" +hyper = { version = "0.14", default-features = false, features = ["client"] } +pin-project = "1" +tokio = { version = "1", default-features = false } +tower = { version = "0.4", default-features = false } +tracing = "0.1" +try-lock = "0.2" + +linkerd-duplex = { path = "../../duplex" } +linkerd-error = { path = "../../error" } +linkerd-http-version = { path = "../version" } +linkerd-io = { path = "../../io" } +linkerd-stack = { path = "../../stack" } diff --git a/linkerd/proxy/http/src/glue.rs b/linkerd/http/upgrade/src/glue.rs similarity index 95% rename from linkerd/proxy/http/src/glue.rs rename to linkerd/http/upgrade/src/glue.rs index 16c4dde464..2c262f3e61 100644 --- a/linkerd/proxy/http/src/glue.rs +++ b/linkerd/http/upgrade/src/glue.rs @@ -47,6 +47,7 @@ pub struct HyperConnectFuture { inner: F, absolute_form: bool, } + // === impl UpgradeBody === impl HttpBody for UpgradeBody { @@ -107,7 +108,7 @@ impl From for UpgradeBody { } impl UpgradeBody { - pub(crate) fn new( + pub fn new( body: hyper::Body, upgrade: Option<(Http11Upgrade, hyper::upgrade::OnUpgrade)>, ) -> Self { @@ -129,7 +130,7 @@ impl PinnedDrop for UpgradeBody { // === impl HyperConnect === impl HyperConnect { - pub(super) fn new(connect: C, target: T, absolute_form: bool) -> Self { + pub fn new(connect: C, target: T, absolute_form: bool) -> Self { HyperConnect { connect, target, @@ -140,7 +141,7 @@ impl HyperConnect { impl Service for HyperConnect where - C: MakeConnection<(crate::Version, T)> + Clone + Send + Sync, + C: MakeConnection<(linkerd_http_version::Version, T)> + Clone + Send + Sync, C::Connection: Unpin + Send, C::Future: Unpin + Send + 'static, T: Clone + Send + Sync, @@ -157,7 +158,7 @@ where HyperConnectFuture { inner: self .connect - .connect((crate::Version::Http1, self.target.clone())), + .connect((linkerd_http_version::Version::Http1, self.target.clone())), absolute_form: self.absolute_form, } } diff --git a/linkerd/http/upgrade/src/lib.rs b/linkerd/http/upgrade/src/lib.rs new file mode 100644 index 0000000000..851d176294 --- /dev/null +++ b/linkerd/http/upgrade/src/lib.rs @@ -0,0 +1,29 @@ +//! Facilities for HTTP/1 upgrades. + +pub use self::upgrade::Service; + +pub mod glue; +pub mod upgrade; + +pub fn strip_connection_headers(headers: &mut http::HeaderMap) { + use http::header; + if let Some(val) = headers.remove(header::CONNECTION) { + if let Ok(conn_header) = val.to_str() { + // A `Connection` header may have a comma-separated list of + // names of other headers that are meant for only this specific + // connection. + // + // Iterate these names and remove them as headers. + for name in conn_header.split(',') { + let name = name.trim(); + headers.remove(name); + } + } + } + + // Additionally, strip these "connection-level" headers always, since + // they are otherwise illegal if upgraded to HTTP2. + headers.remove(header::UPGRADE); + headers.remove("proxy-connection"); + headers.remove("keep-alive"); +} diff --git a/linkerd/proxy/http/src/upgrade.rs b/linkerd/http/upgrade/src/upgrade.rs similarity index 99% rename from linkerd/proxy/http/src/upgrade.rs rename to linkerd/http/upgrade/src/upgrade.rs index 7b79b10b9d..1094584f83 100644 --- a/linkerd/proxy/http/src/upgrade.rs +++ b/linkerd/http/upgrade/src/upgrade.rs @@ -162,6 +162,7 @@ impl Drop for Inner { } // === impl Service === + impl Service { pub fn new(service: S, upgrade_drain_signal: drain::Watch) -> Self { Self { diff --git a/linkerd/proxy/http/Cargo.toml b/linkerd/proxy/http/Cargo.toml index 785740e1c3..6de7ebac88 100644 --- a/linkerd/proxy/http/Cargo.toml +++ b/linkerd/proxy/http/Cargo.toml @@ -49,6 +49,7 @@ linkerd-http-insert = { path = "../../http/insert" } linkerd-http-override-authority = { path = "../../http/override-authority" } linkerd-http-retain = { path = "../../http/retain" } linkerd-http-stream-timeouts = { path = "../../http/stream-timeouts" } +linkerd-http-upgrade = { path = "../../http/upgrade" } linkerd-http-version = { path = "../../http/version" } linkerd-io = { path = "../../io" } linkerd-proxy-balance = { path = "../balance" } diff --git a/linkerd/proxy/http/src/h1.rs b/linkerd/proxy/http/src/h1.rs index 2941cb2ad5..6b99139c96 100644 --- a/linkerd/proxy/http/src/h1.rs +++ b/linkerd/proxy/http/src/h1.rs @@ -1,8 +1,4 @@ -use crate::{ - glue::HyperConnect, - upgrade::{Http11Upgrade, HttpConnect}, - TracingExecutor, -}; +use crate::TracingExecutor; use futures::prelude::*; use http::{ header::{CONTENT_LENGTH, TRANSFER_ENCODING}, @@ -10,6 +6,10 @@ use http::{ }; use linkerd_error::{Error, Result}; use linkerd_http_box::BoxBody; +use linkerd_http_upgrade::{ + glue::HyperConnect, + upgrade::{Http11Upgrade, HttpConnect}, +}; use linkerd_stack::MakeConnection; use std::{pin::Pin, time::Duration}; use tracing::{debug, trace}; @@ -161,7 +161,7 @@ where upgrade.insert_half(hyper::upgrade::on(&mut rsp)); } } else { - crate::strip_connection_headers(rsp.headers_mut()); + linkerd_http_upgrade::strip_connection_headers(rsp.headers_mut()); } rsp.map(BoxBody::new) diff --git a/linkerd/proxy/http/src/lib.rs b/linkerd/proxy/http/src/lib.rs index 63e6bf8cae..7973721a7b 100644 --- a/linkerd/proxy/http/src/lib.rs +++ b/linkerd/proxy/http/src/lib.rs @@ -8,7 +8,6 @@ pub mod balance; pub mod client; pub mod client_handle; pub mod detect; -mod glue; pub mod h1; pub mod h2; mod header_from_target; @@ -17,7 +16,6 @@ pub mod orig_proto; mod server; pub mod strip_header; pub mod timeout; -pub mod upgrade; pub use self::{ balance::NewBalance, @@ -45,6 +43,7 @@ pub use linkerd_http_insert as insert; pub use linkerd_http_override_authority::{AuthorityOverride, NewOverrideAuthority}; pub use linkerd_http_retain::{self as retain, Retain}; pub use linkerd_http_stream_timeouts::{self as stream_timeouts, EnforceTimeouts, StreamTimeouts}; +pub use linkerd_http_upgrade as upgrade; pub use linkerd_http_version::{self as version, Version}; #[derive(Clone, Debug)] @@ -90,25 +89,3 @@ where let v = req.headers().get(header)?; v.to_str().ok()?.parse().ok() } - -fn strip_connection_headers(headers: &mut http::HeaderMap) { - if let Some(val) = headers.remove(header::CONNECTION) { - if let Ok(conn_header) = val.to_str() { - // A `Connection` header may have a comma-separated list of - // names of other headers that are meant for only this specific - // connection. - // - // Iterate these names and remove them as headers. - for name in conn_header.split(',') { - let name = name.trim(); - headers.remove(name); - } - } - } - - // Additionally, strip these "connection-level" headers always, since - // they are otherwise illegal if upgraded to HTTP2. - headers.remove(header::UPGRADE); - headers.remove("proxy-connection"); - headers.remove("keep-alive"); -} diff --git a/linkerd/proxy/http/src/orig_proto.rs b/linkerd/proxy/http/src/orig_proto.rs index 858b7a8431..d8d9f1f2fd 100644 --- a/linkerd/proxy/http/src/orig_proto.rs +++ b/linkerd/proxy/http/src/orig_proto.rs @@ -1,4 +1,4 @@ -use super::{h1, h2, upgrade}; +use super::{h1, h2}; use futures::prelude::*; use http::header::{HeaderValue, TRANSFER_ENCODING}; use hyper::body::HttpBody; @@ -71,7 +71,11 @@ where fn call(&mut self, mut req: http::Request) -> Self::Future { debug_assert!(req.version() != http::Version::HTTP_2); - if req.extensions().get::().is_some() { + if req + .extensions() + .get::() + .is_some() + { debug!("Skipping orig-proto upgrade due to HTTP/1.1 upgrade"); return Box::pin(self.http1.request(req).map_ok(|rsp| rsp.map(BoxBody::new))); } diff --git a/linkerd/proxy/http/src/server.rs b/linkerd/proxy/http/src/server.rs index 168ae61c7e..5cdb10adf9 100644 --- a/linkerd/proxy/http/src/server.rs +++ b/linkerd/proxy/http/src/server.rs @@ -1,6 +1,5 @@ use crate::{ - client_handle::SetClientHandle, h2, upgrade, BoxBody, BoxRequest, ClientHandle, - TracingExecutor, Version, + client_handle::SetClientHandle, h2, BoxBody, BoxRequest, ClientHandle, TracingExecutor, Version, }; use linkerd_error::Error; use linkerd_io::{self as io, PeerAddr}; @@ -157,7 +156,10 @@ where match version { Version::Http1 => { // Enable support for HTTP upgrades (CONNECT and websockets). - let svc = upgrade::Service::new(BoxRequest::new(svc), drain.clone()); + let svc = linkerd_http_upgrade::upgrade::Service::new( + BoxRequest::new(svc), + drain.clone(), + ); let mut conn = server .http1_only(true) .serve_connection(io, svc)