refactor(proxy/http): extricate `Body` middleware types #3382

cratelyn · 2024-11-21T22:42:20Z

this branch pulls a number of components providing http_body::Body middleware out of linkerd-proxy-http and into new libraries: linkerd-http-classify, linkerd-http-stream-timeouts, linkerd-http-retain, and linkerd-http-override-authority.

these are each performed in distinct commits, to facilitate review.

sfleen

This is awesome! A few minor style nits, nothing substantial.

sfleen · 2024-11-22T13:39:51Z

Cargo.toml

    "opentelemetry-proto",
    "spiffe-proto",
-    "tools",
+    "tools", "linkerd/http/override-authority",


format nit: sort and put on it's own line

thank you for spotting this!

this is moved onto its own sorted line in the amended commit 5f9c146

sfleen · 2024-11-22T13:41:16Z

linkerd/http/override-authority/src/lib.rs

    inner: S,
 }

+/// Sets the [`Authority`][uri::Authority] of the given URI.


optional: This can just be [`Authority`] since it's already imported, but I'm not sure what our style guidelines are around this.

good catch, thanks! i appreciate the close eye. 🦅

this is fixed in the amended commit 5f9c146

cratelyn · 2024-11-22T14:56:58Z

rebased to address a minor typo in a commit message 👼

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 <[email protected]>

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 <[email protected]>

) 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 <[email protected]>

…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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

) 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 <[email protected]>

…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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

cratelyn · 2024-11-25T16:48:26Z

rebased onto the latest main, and pulled in the HTTP/1 upgrade facilities originally outlined in #3386.

olix0r

Yep, this is looking good to me.

note: this commit will not compile, code changes are intentionally elided from this commit. this commit upgrades hyper, http, tonic, prost, related dependencies, and their assorted cargo features. see <linkerd/linkerd2#8733>. see also: * #3379 * #3380 * #3382 * #3405 * hyperium/hyper#3796 * #3411 * #3421 * #3427 * #3428 * #3432 * #3433 * #3444 * #3445 * #3454 * #3455 * #3456 * #3457 * #3461 * #3459 * #3465 * #3466 * #3467 * #3468 * linkerd/linkerd2-proxy-api#421 * linkerd/linkerd2#13492 * linkerd/linkerd2#13493 * hyperium/hyper#3816 * #3472 * #3473 * #3479 * tokio-rs/tokio#7059 * #3509 * hyperium/http-body#140 * #3515 * hyperium/http-body#141 * #3530 * #3531 * #3540 * #3556 * #3558 * #3559 * #3564 * #3567 * #3573 * #3583 * hyperium/http-body#144 * #3585 * #3586 * #3597 * #3598 * #3611 * #3614 * #3615 * #3616 * #3647 * #3651 * #3653 * #3654 * #3655 * #3656 * #3657 * #3660 * #3671 * #3672 * #3673 * #3676 * hyperium/http-body#147 * #3692 * #3699 * #3700 * #3701 * #3708 * linkerd/drain-rs#36 * #3715 * #3717 * eminence/procfs#340 --- squash: chore(deps): add hyper-util workspace dependency chore(deps): add http-body-util workspace dependency chore(deps): upgrade linkerd2-proxy-api this commit represents main as of linkerd/linkerd2-proxy-api#421. Signed-off-by: katelyn martin <[email protected]>

## ✨ chore(deps): upgrade to hyperium 1.x crates this branch performs an exciting upgrade for our proxy. this branch upgrades a number of our dependencies so that we use the 1.0 release family of the `hyper` http framework, and its ecosystem. see the [v1.0 announcement][hyper-v1] for more information. this branch upgrades the following dependencies: * `h2`: 0.3 -> 0.4 * `http`: 0.2 -> 1 * `http-body`: 0.4 -> 1 * `hyper`: 0.14.32 -> 1 * `prost`: 0.12 -> 0.13 * `prost-build`: 0.12 -> 0.13 * `prost-types`: 0.12 -> 0.13 * `tonic`: 0.10 -> 0.12 * `tonic-build`: 0.10 -> 0.12 a `hyper-util` dependency is added, which provides among other things, legacy-compatible interfaces such as `hyper_util::client::legacy::Client`, or glue to use `hyper` with the tokio runtime. see <https://docs.rs/hyper-util/latest/hyper_util/> for more information. a `http-body-util` dependency is added, which provides a `BodyExt` trait and a channel-backed body for use in unit tests. the `deprecated` feature flag that was active on our `0.14` hyper dependency has been removed, along with the `stream` and `runtime` feature flags. the `linkerd2-proxy-api` dependency is updated. see: <linkerd/linkerd2-proxy-api#421> ### 📝 notes for review bear particular attention to changes involving `http_body::Body` middleware. the change from two separate `poll_data()` and `poll_trailers()` functions, to a single `poll_frame()` method, induces some subtle changes to various pieces of middleware. also bear in mind that failing to set a timer, in our case `hyper_util::rt::TokioTimer`, can cause http/2 clients, or http/1 and http/2 servers, to panic. make sure that any uses of `hyper::server::conn::http1::Builder`, `hyper::client::conn::http1::Builder`, or `hyper::client::conn::http2::Builder` install a timer. ### ❗ breaking change: `l5d-proxy-error` values the `l5d-proxy-error` header can be examined to observe the cause of proxy errors encountered when sending meshed traffic. by virtue of this using a newer `hyper` client in the proxy, some error messages may in turn look different. for example, an error like `"connect timed out after 1s"` may now appear as `"client error (Connect)"`. ### 📚 other notes this work, by virtue of touching so many parts of the system, is carried out in distinct commits. an initial commit upgrades the dependencies at th workspace level. subsequent commits will not compile if the `--workspace` flag is provided, but the intent of this branch is to update each crate individually. use commands like, e.g. `cargo check --tests -p linkerd-proxy-http` to build particular crates at intermediate commits within this branch. this commit is also only the final leaf in an _extended_ line of work. this has been done to mitigate the effort of reviewing this change, and the risk of churn in the event of any unanticipated errors. see the top-level comment in linkerd/linkerd2#8733 for an overview of all of the work that brought us to this juncture. [hyper-v1]: https://seanmonstar.com/blog/hyper-v1/ --- * chore(deps): upgrade to hyper 1.x note: this commit will not compile, code changes are intentionally elided from this commit. this commit upgrades hyper, http, tonic, prost, related dependencies, and their assorted cargo features. see <linkerd/linkerd2#8733>. see also: * #3379 * #3380 * #3382 * #3405 * hyperium/hyper#3796 * #3411 * #3421 * #3427 * #3428 * #3432 * #3433 * #3444 * #3445 * #3454 * #3455 * #3456 * #3457 * #3461 * #3459 * #3465 * #3466 * #3467 * #3468 * linkerd/linkerd2-proxy-api#421 * linkerd/linkerd2#13492 * linkerd/linkerd2#13493 * hyperium/hyper#3816 * #3472 * #3473 * #3479 * tokio-rs/tokio#7059 * #3509 * hyperium/http-body#140 * #3515 * hyperium/http-body#141 * #3530 * #3531 * #3540 * #3556 * #3558 * #3559 * #3564 * #3567 * #3573 * #3583 * hyperium/http-body#144 * #3585 * #3586 * #3597 * #3598 * #3611 * #3614 * #3615 * #3616 * #3647 * #3651 * #3653 * #3654 * #3655 * #3656 * #3657 * #3660 * #3671 * #3672 * #3673 * #3676 * hyperium/http-body#147 * #3692 * #3699 * #3700 * #3701 * #3708 * linkerd/drain-rs#36 * #3715 * #3717 * eminence/procfs#340 --- squash: chore(deps): add hyper-util workspace dependency chore(deps): add http-body-util workspace dependency chore(deps): upgrade linkerd2-proxy-api this commit represents main as of linkerd/linkerd2-proxy-api#421. Signed-off-by: katelyn martin <[email protected]> * chore(http/box): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(hyper-balance): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/retain): ugrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/stream-timeouts): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/classify): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/upgrade): upgrade to hyper 1.x NOTE: there is a comment noting that the upgrade middleware does not expect to be cloneable. it is unfortunately, however, at odds with the new bounds expected of extensions. so, `Http11Upgrade` is now Clone'able, but a comment is left in place noting this weakened invariant. it's worth investigating how upgrades have changed since, in more detail, but for the current moment we are interested in being especially conservative about changing behavior, and focusing on api changes like `Body::poll_frame(..)`. Signed-off-by: katelyn martin <[email protected]> * chore(metrics): upgrade to hyper 1.x a brief note; this commit happened to tickle an unfortunate sharp edge in `BoxBody` and `Full`'s respective constructors. type inference could not figure out how to construct the body, so we refrain from boxing the response body now. Signed-off-by: katelyn martin <[email protected]> * chore(http/metrics): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/prom): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/insert): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/retry): deprecate linkerd-http-body-compat Signed-off-by: katelyn martin <[email protected]> * chore(mock/http-body): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(http/retry): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(proxy/tap): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(proxy/http): update to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/core): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/test): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/admin): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/outbound): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/inbound): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app/integration): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(app): upgrade to hyper 1.x Signed-off-by: katelyn martin <[email protected]> * chore(transport-header): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(spiffe-proto): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(opencensus-proto): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(opentelemetry-proto): update generated code Signed-off-by: katelyn martin <[email protected]> * chore(deny.toml): update cargo-deny directives this commit updates the contents of `deny.toml`. Signed-off-by: katelyn martin <[email protected]> * chore: `compile` has been renamed to `compile_protos` this addresses deprecation warnings, updating calls to a function that has since been renamed. Signed-off-by: katelyn martin <[email protected]> * chore(deps): remove `linkerd-http-body-compat` dependencies this commit removes this crate, which we added to future proof code for this upgrade, from its dependents. Signed-off-by: katelyn martin <[email protected]> * chore(http/body-compat): remove `linkerd-http-body-compat` crate Signed-off-by: katelyn martin <[email protected]> * chore(deps): update to drain 0.2.1 see linkerd/drain-rs#41. Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>

cratelyn mentioned this pull request Nov 21, 2024

refactor(proxy/http): create linkerd-http-override-authority crate #3381

Closed

cratelyn marked this pull request as ready for review November 21, 2024 22:53

cratelyn requested a review from a team as a code owner November 21, 2024 22:53

sfleen approved these changes Nov 22, 2024

View reviewed changes

cratelyn force-pushed the kate/yoink-http-body-middleware branch 2 times, most recently from 5f9c146 to 1631681 Compare November 22, 2024 14:55

cratelyn force-pushed the kate/yoink-http-body-middleware branch from 1631681 to 7f564f9 Compare November 22, 2024 14:59

cratelyn mentioned this pull request Nov 22, 2024

refactor(proxy/http): create linkerd-http-upgrade crate #3386

Closed

cratelyn added 4 commits November 25, 2024 11:45

refactor(proxy/http): create linkerd-http-override-authority crate (#…

269d72b

…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 <[email protected]>

cratelyn force-pushed the kate/yoink-http-body-middleware branch from 7f564f9 to cb9f511 Compare November 25, 2024 16:46

cratelyn added 2 commits November 25, 2024 11:47

refactor(proxy/http): move is_bad_request() into upgrade (#3382)

81c569a

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 <[email protected]>

refactor(proxy/http): create linkerd-http-upgrade crate (#3382)

61d2a63

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 <[email protected]>

cratelyn force-pushed the kate/yoink-http-body-middleware branch from cb9f511 to 61d2a63 Compare November 25, 2024 16:48

olix0r self-assigned this Nov 25, 2024

olix0r approved these changes Nov 26, 2024

View reviewed changes

cratelyn merged commit aa1dbd7 into main Nov 26, 2024
20 checks passed

cratelyn deleted the kate/yoink-http-body-middleware branch November 26, 2024 20:01

cratelyn mentioned this pull request Dec 2, 2024

proxy: Figure out the plan for Hyper 1.0 migration linkerd/linkerd2#8733

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(proxy/http): extricate `Body` middleware types #3382

refactor(proxy/http): extricate `Body` middleware types #3382

Uh oh!

cratelyn commented Nov 21, 2024 •

edited

Loading

Uh oh!

sfleen left a comment

Uh oh!

sfleen Nov 22, 2024

Uh oh!

cratelyn Nov 22, 2024

Uh oh!

sfleen Nov 22, 2024

Uh oh!

cratelyn Nov 22, 2024

Uh oh!

cratelyn commented Nov 22, 2024

Uh oh!

cratelyn commented Nov 25, 2024

Uh oh!

olix0r left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

refactor(proxy/http): extricate Body middleware types #3382

refactor(proxy/http): extricate Body middleware types #3382

Uh oh!

Conversation

cratelyn commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sfleen left a comment

Choose a reason for hiding this comment

Uh oh!

sfleen Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

cratelyn Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

sfleen Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

cratelyn Nov 22, 2024

Choose a reason for hiding this comment

Uh oh!

cratelyn commented Nov 22, 2024

Uh oh!

cratelyn commented Nov 25, 2024

Uh oh!

olix0r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

refactor(proxy/http): extricate `Body` middleware types #3382

refactor(proxy/http): extricate `Body` middleware types #3382

cratelyn commented Nov 21, 2024 •

edited

Loading