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

olix0r · 2022-06-23T20:53:20Z

Hyper is planning a major 1.0 milestone that will impact many of their public APIs and, therefore, the proxy. We should get a better understanding of the planned changes so that we can begin to scope and plan the required proxy changes (and so that we can provide meaningful feedback before the APIs are finalized).

"here's some links!" -kate 💐

The text was updated successfully, but these errors were encountered:

hawkw · 2022-06-23T21:05:13Z

Some of the things that are definitely worth keeping an eye on:

The future of hyper's use of the Service trait: whether hyper will use tower::Service in 1.0 is currently up in the air. If tower-service isn't 1.0 in time, it definitely won't. The question then becomes whether hyper will provide its own Service trait, or whether the interface will be changed significantly, potentially to one where user code calls into hyper.

If hyper ends up moving away from tower-service, there will definitely need to be some kind of glue layer (which could be a generic tower-hyper crate or similar); the complexity of this adapter will depend on how different the interfaces end up being.
I/O traits: It's unclear whether hyper 1.0 is going to use Tokio's AsyncRead/AsyncWrite traits (as it does currently), or something else.
- It seems like the use of the Tokio traits is more likely than the version from futures, but not set in stone, especially if there's an eventual plan for AsyncRead/AsyncWrite in std.
- There are also questions about whether the style of async I/O traits currently in tokio and in futures-io can ever efficiently support io_uring-style async IO, where ownership of the buffer is transferred to the I/O resource (and then to the OS). This is a broader ecosystem-level question, but it's worth keeping an eye on.
A bunch of stuff currently in hyper will move to hyper-util. This one is not a huge deal as that should generally be a pretty mechanical transformation.

stale · 2022-09-21T23:01:52Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

risingspiral · 2023-09-12T22:12:26Z

https://docs.rs/hyper/1.0.0-rc.4/hyper/

this is a prepatory chore, laying more ground to facilitate upgrading our hyper dependency to the 1.0 major release. this commit adds the `deprecated` cargo feature to each of the hyper dependencies in the cargo workspace. to prevent this from introducing a large number of compiler warnings to the build, the `#[allow(deprecated)]` attribute is added to relevant expressions. these uses of deprecated interfaces will be updated in subsequent commits. broadly, these fall into a few common cases: * `hyper::client::conn::SendRequest` now has protocol specific `h1` and `h2` variants. * `hyper::server::conn::Builder` now has protocol specific `h1` and `h2` variants. * `hyper::server::conn::Http` is deprecated. * functions like `hyper::body::aggregate(..)` and `hyper::body::to_bytes(..)` are deprecated. see linkerd/linkerd2#8733 for more information on the hyper 1.0 upgrade process. Signed-off-by: katelyn martin <kate@buoyant.io>

cratelyn · 2024-12-02T19:57:06Z

i've been driving this work forward recently, and was kindly pointed at this issue by @olix0r. i thought i'd lay down some thoughts on the work done thus far:

first, i drove a few small exploratory spikes into upgrading the proxy here, here, and here.

most of the friction related to this upgrade will be found in the linkerd-proxy-http crate, which provides a number of core facilities wrapping hyper's client and server interfaces used throughout the rest of the proxy.

i've landed a few preparatory changes, in linkerd/linkerd2-proxy#3379, linkerd/linkerd2-proxy#3380, and linkerd/linkerd2-proxy#3382. these pulled assorted standalone bits of http/hyper infrastructure that define http_body::Body middleware out of the linkerd-proxy-http library, and into their own distinct crates.

this will let us bump linkerd-http-classify, linkerd-http-stream-timeouts, linkerd-http-retain, linkerd-http-override-authority, [..] and other linkerd-http-* crates up to hyper 1.0 in separate steps, which should both facilitate review and mitigate the risk of bugs slipping in while addressing the changes to the Body trait in particular.

🥚 🔜 🐣 next steps

next, we can use the deprecated and backports feature flags provided in the 0.14 minor release to further prepare for the upgrade.

linkerd/linkerd2-proxy#3405 adds the deprecated flag to the hyper dependencies in the cargo workspace, and #[allow(deprecated)] attributes to code using deprecated hyper interfaces.

next, we'll address those deprecations, making use of the backports feature. i've landed hyperium/hyper#3796 upstream to backport a 1.0 interface that we'll need before we can use the connection builder backported to 0.14.

this is a prepatory chore, laying more ground to facilitate upgrading our hyper dependency to the 1.0 major release. this commit adds the `deprecated` cargo feature to each of the hyper dependencies in the cargo workspace. to prevent this from introducing a large number of compiler warnings to the build, the `#[allow(deprecated)]` attribute is added to relevant expressions. these uses of deprecated interfaces will be updated in subsequent commits. broadly, these fall into a few common cases: * `hyper::client::conn::SendRequest` now has protocol specific `h1` and `h2` variants. * `hyper::server::conn::Builder` now has protocol specific `h1` and `h2` variants. * `hyper::server::conn::Http` is deprecated. * functions like `hyper::body::aggregate(..)` and `hyper::body::to_bytes(..)` are deprecated. see linkerd/linkerd2#8733 for more information on the hyper 1.0 upgrade process. Signed-off-by: katelyn martin <kate@buoyant.io>

cratelyn · 2024-12-03T20:03:43Z

Some of the things that are definitely worth keeping an eye on:

* **The future of `hyper`'s use of the `Service` trait**: [whether `hyper` will use `tower::Service` in 1.0](https://github.com/hyperium/hyper/blob/master/docs/ROADMAP.md#service) is currently up in the air. If `tower-service` isn't 1.0 in time, it definitely won't. The question then becomes whether `hyper` will provide its own `Service` trait, or whether the interface will be changed significantly, [potentially to one where user code calls into `hyper`](https://github.com/hyperium/hyper/blob/master/docs/ROADMAP.md#you-call-hyper-or-hyper-calls-you).
  If hyper ends up moving away from `tower-service`, there will definitely need to be some kind of glue layer (which could be a generic `tower-hyper` crate or similar); the complexity of this adapter will depend on how different the interfaces end up being.

* **I/O traits**: It's unclear [whether `hyper` 1.0 is going to use Tokio's `AsyncRead`/`AsyncWrite` traits (as it does currently), or something else](https://github.com/hyperium/hyper/blob/master/docs/ROADMAP.md#runtime-woes).
  
  * It seems like the use of the Tokio traits is more likely than the version from `futures`, but not set in stone, especially if there's an eventual plan for `AsyncRead`/`AsyncWrite` in `std`.
  * There are also questions about whether the style of async I/O traits currently in `tokio` and in `futures-io` can ever efficiently support `io_uring`-style async IO, where ownership of the buffer is transferred to the I/O resource (and then to the OS).  This is a broader ecosystem-level question, but it's worth keeping an eye on.

* **A bunch of stuff currently in `hyper` will move to `hyper-util`.** This one is not a huge deal as that should _generally_ be a pretty mechanical transformation.

for the sake of closing the loop on the details above:

Service trait: hyper now provides its own Service trait. glue connecting tower to hyper lives in the hyper-util library: here.
I/O traits: hyper provides its own Read and Write traits. glue connecting this to tokio's traits lives in hyper-util::rt::tokio.
hyper-util motion: legacy client builder interfaces can be found in hyper_util::client::legacy. see hyper-util::server as well.

hyper 0.14.x provided a collection of interfaces related to collecting and aggregating request and response bodies, which were deprecated and removed in the 1.x major release. this commit updates calls to `hyper::body::to_bytes(..)` and `hyper::body::aggregate(..)`. for now, `http_body::Body` is used, but we can use `http_body_util::BodyExt` once we've bumped our hyper dependency to the 1.x major release. for more information, see: * linkerd/linkerd2#8733 * hyperium/hyper#2840 * hyperium/hyper#3020 Signed-off-by: katelyn martin <kate@buoyant.io>

* chore(app/admin): add `http-body` dependency before we address deprecated hyper interfaces related to `http_bodies`, we'll want to add this dependency so that we can call `Body::collect()`. Signed-off-by: katelyn martin <kate@buoyant.io> * refactor(app): update deprecated hyper body calls hyper 0.14.x provided a collection of interfaces related to collecting and aggregating request and response bodies, which were deprecated and removed in the 1.x major release. this commit updates calls to `hyper::body::to_bytes(..)` and `hyper::body::aggregate(..)`. for now, `http_body::Body` is used, but we can use `http_body_util::BodyExt` once we've bumped our hyper dependency to the 1.x major release. for more information, see: * linkerd/linkerd2#8733 * hyperium/hyper#2840 * hyperium/hyper#3020 Signed-off-by: katelyn martin <kate@buoyant.io> --------- Signed-off-by: katelyn martin <kate@buoyant.io>

cratelyn · 2024-12-04T19:51:18Z

with linkerd/linkerd2-proxy#3405 landed, we're now in a good position to begin addressing particular deprecations in the hyper 0.14.31 interface to prepare the proxy for the hyper 1.0 upgrade.

linkerd/linkerd2-proxy#3411 is an example of one such change, handling calls to deprecated hyper::body functions such as e.g. aggregate().

this `Server` type is not used by any tests. this commit removes it, to help facilitate the upgrade to the hyper 1.0 major release. see <linkerd/linkerd2#8733>. Signed-off-by: katelyn martin <kate@buoyant.io>

#3427) this branch contains a sequence of commits that focus on addressing deprecation warnings related to hyper::client::conn::Builder. this branch enables the backports feature, and then replaces some of these builders with the backported, http/2 specific, hyper::client::conn::http2::Builder type. relates to linkerd/linkerd2#8733. --- * chore(proxy/http): address `h2::Connection` deprecation this commit updates `Connection<B>` to use the backported, http/2 specific `SendRequest` type. this commit enables the `backports` feature flag in the `hyper` dependency. Signed-off-by: katelyn martin <kate@buoyant.io> * chore(proxy/http): address `server::tests` deprecations Signed-off-by: katelyn martin <kate@buoyant.io> * refactor(proxy/http): consolidate connect functions `connect()` is never called elsewhere. let's avoid the misdirection and move it into the body of `connect_h2()`. Signed-off-by: katelyn martin <kate@buoyant.io> --------- Signed-off-by: katelyn martin <kate@buoyant.io>

while handling the upgrade to hyper 1.0, i noticed some small changes that'd be nice to make. most importantly, with respect to linkerd/linkerd2#8733, this commit outlines `http_util::http_request()`. hyper 1.0 provides version specific `SendRequest` types for HTTP/1 and HTTP/2, so rather than try to be polymorphic across both senders, we send the request at the call site. two other commits remove `pub` attributes for functions that aren't called externally. we'll address `run_proxy()` and `connect_client()` in a subsequent PR, because the dependent tests use both HTTP/1 and HTTP/2. --- * refactor(app/test): remove `pub` from `http_util::connect_client()` this is not used elsewhere. it can be private. Signed-off-by: katelyn martin <kate@buoyant.io> * refactor(app/test): remove `pub` from `http_util::run_proxy()` Signed-off-by: katelyn martin <kate@buoyant.io> * refactor(app/test): remove `http_util::http_request()` this function abstracts over one statement, at the cost of needing to name the `SendRequest` type. because hyper's 1.0 interface provides multiple `SendRequest` types based on the HTTP version being used. to avoid needing to deal with polymorphism, and to implicitly address a function-level allowance of deprecated types, we remove this function and move this statement to the function's call sites. Signed-off-by: katelyn martin <kate@buoyant.io> * refactor(app/test): collect bodies with `String::from_utf8` this function previously called `std::str::from_utf8`, before calling `str::to_owned` on the result. because of this, there is a bit of extra gymnastics, passing a `&body[..]` slice along after collecting the body bytes. this is both more baroque and less performant. this commit updates the call, using `String::from_utf8` to collect the body, sparing a needless `to_owned()`/`clone()` call. Signed-off-by: katelyn martin <kate@buoyant.io> * docs(app/test): document `io` submodule Signed-off-by: katelyn martin <kate@buoyant.io> * refactor(app/test): remove duplicate `io` reëxport the same `pub use` bundle is found in the parent `lib.rs`. this removes it, and refers to the same `mod io {}` defined above. Signed-off-by: katelyn martin <kate@buoyant.io> * review(app/inbound): use `ServiceExt::oneshot(..)` #3428 (comment) we can simplify these statements sending requests, by using `ServiceExt::oneshot(..)`. Signed-off-by: katelyn martin <kate@buoyant.io> --------- Signed-off-by: katelyn martin <kate@buoyant.io>

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. Signed-off-by: katelyn martin <kate@buoyant.io>

this contains some small patches that we're interested in, before we address deprecations and upgrade to hyper 1.0. see #8733 for more information. Signed-off-by: katelyn martin <me+cratelyn@katelyn.world>

NB: this branch is based upon #13492. see #8733 for more information about migrating to hyper 1.0. this enables the `backports` and `deprecated` feature flags in the hyper dependencies in this project, and addresses warnings. see <https://hyper.rs/guides/1/upgrading/> for more information about these feature flags. largely, the control plane is unaffected by this upgrade, besides the following changes: * one usage of a deprecated `hyper::body::aggregate` function is updated. * a `hyper::rt::Executor<E>` implementation, which spawns tasks onto the tokio runtime, is provided. once we upgrade to hyper 1.0, we can replace this with the executor provided in [`hyper-util`](https://docs.rs/hyper-util/latest/hyper_util/rt/tokio/struct.TokioExecutor.html#impl-Executor%3CFut%3E-for-TokioExecutor). * the `hyper::service::Service<hyper::Request<tonic::body::BoxBody>>` implementation for `GrpcHttp` now boxes its returned future, on account of `SendRequest` returning an anonymous `impl Future<Output = ...>`. * the `policy-test` additionally depends on the `runtime` feature of hyper. this is an artifact of an internal config structure shared by the legacy connection builder and the backported connection builder containing two keep-alive fields that were feature gated prior to 1.0. Signed-off-by: katelyn martin <me+cratelyn@katelyn.world>

this contains some small patches that we're interested in, before we address deprecations and upgrade to hyper 1.0. see #8733 for more information. Signed-off-by: katelyn martin <kate@buoyant.io>

NB: this branch is based upon #13492. see #8733 for more information about migrating to hyper 1.0. this enables the `backports` and `deprecated` feature flags in the hyper dependencies in this project, and addresses warnings. see <https://hyper.rs/guides/1/upgrading/> for more information about these feature flags. largely, the control plane is unaffected by this upgrade, besides the following changes: * one usage of a deprecated `hyper::body::aggregate` function is updated. * a `hyper::rt::Executor<E>` implementation, which spawns tasks onto the tokio runtime, is provided. once we upgrade to hyper 1.0, we can replace this with the executor provided in [`hyper-util`](https://docs.rs/hyper-util/latest/hyper_util/rt/tokio/struct.TokioExecutor.html#impl-Executor%3CFut%3E-for-TokioExecutor). * the `hyper::service::Service<hyper::Request<tonic::body::BoxBody>>` implementation for `GrpcHttp` now boxes its returned future, on account of `SendRequest` returning an anonymous `impl Future<Output = ...>`. * the `policy-test` additionally depends on the `runtime` feature of hyper. this is an artifact of an internal config structure shared by the legacy connection builder and the backported connection builder containing two keep-alive fields that were feature gated prior to 1.0. Signed-off-by: katelyn martin <kate@buoyant.io>

this commit upgrades to hyper 0.14.32, removing the manifest's `[patch]` section. hyperium/hyper#3796 backported a method we use when building http/2 connections. #3457 patched the workspace to rely on a git dependency of hyper at commit `a24f0c0a`. this work has been released in version 0.14.32. this commit also changes `deny.toml`, removing the exception we carved out for hyper in #3457. for more information, see: * hyperium/hyper#3796 * linkerd/linkerd2#8733 * https://github.com/hyperium/hyper/commits/0.14.x * #3457 * 03f5577 Signed-off-by: katelyn martin <kate@buoyant.io>

hyper 1.0 migrated to a new definition of the `Body` trait, which exposes a single `poll_frame(..)` method, rather than the two `poll_data(..)` and `poll_trailers(..)` methods previously required of implementors. this commit introduces a new `linkerd-http-body` crate. this crate allows callers to wrap a "legacy" body that implements the 0.4 version of `http_body::Body` in a middleware to incrementally adopt the new 1.0 interface. see <linkerd/linkerd2#8733>. Signed-off-by: katelyn martin <kate@buoyant.io>

this commit modifies the workspace manifest, defining prost and prost-types as common workspace dependencies. no changes to the lockfile are made because this commit does not affect the dependency graph of the project. * linkerd/linkerd2#8733 Signed-off-by: katelyn martin <kate@buoyant.io>

this commit modifies the workspace manifest, defining tonic and tonic-build as common workspace dependencies. no changes to the lockfile are made because this commit does not affect the dependency graph of the project. * linkerd/linkerd2#8733 Signed-off-by: katelyn martin <kate@buoyant.io>

this commit modifies the workspace manifest, defining bytes as a workspace dependency. no changes to the lockfile are made because this commit does not affect the dependency graph of the project. * linkerd/linkerd2#8733 Signed-off-by: katelyn martin <kate@buoyant.io>

this commit modifies the workspace manifest, defining h2 as a workspace dependency. no changes to the lockfile are made because this commit does not affect the dependency graph of the project. * linkerd/linkerd2#8733 Signed-off-by: katelyn martin <kate@buoyant.io>