chore(app/inbound): address hyper deprecations in http/2 tests #3445

cratelyn · 2024-12-11T06:17:55Z

this addresses deprecations in inbound proxy tests that should migrate to hyper's new http/2 client connection builder.

http/1 tests will be upgraded in a follow-on commit. the connect_and_accept(..) helper function is duplicated into an http/2 version.

see linkerd/linkerd2#8733 for more information on upgrading to hyper 1.0.

this addresses deprecations in inbound proxy tests that should migrate to hyper's new http/2 client connection builder. http/1 tests will be upgraded in a follow-on commit. the `connect_and_accept(..)` helper function is copied, and duplicated into an http/2 version. see <linkerd/linkerd2#8733> for more information on upgrading to hyper 1.0. Signed-off-by: katelyn martin <[email protected]>

cratelyn · 2024-12-11T06:20:38Z

linkerd/app/test/src/http_util.rs

+pub async fn connect_and_accept_http2<B>(
+    client_settings: &mut hyper::client::conn::http2::Builder,
+    server: BoxServer,
+) -> (
+    hyper::client::conn::http2::SendRequest<B>,
+    JoinSet<Result<(), Error>>,
+)


the diff presents this unfortunately, but the body of connect_and_accept is not changed, this is the function added in this branch.

cratelyn · 2024-12-11T06:21:18Z

linkerd/app/inbound/src/http/tests.rs

-    #[allow(deprecated)] // linkerd/linkerd2#8733
-    let mut client = hyper::client::conn::Builder::new();
-    client.http2_only(true);
+    let mut client = hyper::client::conn::http2::Builder::new(TracingExecutor);


this is the change we're applying to these inbound proxy tests.

this is a follow-up commit related to 24dc5d8 (#3445). see <linkerd/linkerd2#8733> for more information on upgrading to hyper 1.0. --- this addresses hyper deprecations in the http/1 tests for the inbound proxy. prior, we made use of `tower::ServiceExt::oneshot`, which consumes a service and drops it after sending a request and polling the response future to completion. <https://docs.rs/tower/0.5.2/src/tower/util/oneshot.rs.html#96-100> tower is not a 1.0 library yet, so `SendRequest` does not provide an implementation of `tower::Service` in hyper's 1.0 interface: - <https://docs.rs/hyper/0.14.31/hyper/client/conn/struct.SendRequest.html#impl-Service%3CRequest%3CB%3E%3E-for-SendRequest%3CB%3E> - <https://docs.rs/hyper/1.5.1/hyper/client/conn/http1/struct.SendRequest.html#trait-implementations> consequentially, we must drop the sender ourselves after receiving a response now. --- this commit *also* addresses hyper deprecations in the http/1 downgrade tests for the inbound proxy. because these tests involve a http/2 client and an http/1 server, we take the choice of inlining the body of `http_util::connect_and_accept()` rather than introducing a new, third `http_util::connect_and_accept_http_downgrade()` function. we will refactor these helper functions in follow-on commits. NB: because `ContextError` is internal to the `linkerd-app-test` crate, we do not wrap the errors. these are allegedly used by the fuzzing tests (_see f.ex #986 and #989_), but for our purposes with respect to the inbound proxy we can elide them rather than making `ctx()` a public method. --- Signed-off-by: katelyn martin <[email protected]>

* chore(app/inbound): address hyper deprecations in http/1 tests this is a follow-up commit related to 24dc5d8 (#3445). see <linkerd/linkerd2#8733> for more information on upgrading to hyper 1.0. --- this addresses hyper deprecations in the http/1 tests for the inbound proxy. prior, we made use of `tower::ServiceExt::oneshot`, which consumes a service and drops it after sending a request and polling the response future to completion. <https://docs.rs/tower/0.5.2/src/tower/util/oneshot.rs.html#96-100> tower is not a 1.0 library yet, so `SendRequest` does not provide an implementation of `tower::Service` in hyper's 1.0 interface: - <https://docs.rs/hyper/0.14.31/hyper/client/conn/struct.SendRequest.html#impl-Service%3CRequest%3CB%3E%3E-for-SendRequest%3CB%3E> - <https://docs.rs/hyper/1.5.1/hyper/client/conn/http1/struct.SendRequest.html#trait-implementations> consequentially, we must drop the sender ourselves after receiving a response now. --- this commit *also* addresses hyper deprecations in the http/1 downgrade tests for the inbound proxy. because these tests involve a http/2 client and an http/1 server, we take the choice of inlining the body of `http_util::connect_and_accept()` rather than introducing a new, third `http_util::connect_and_accept_http_downgrade()` function. we will refactor these helper functions in follow-on commits. NB: because `ContextError` is internal to the `linkerd-app-test` crate, we do not wrap the errors. these are allegedly used by the fuzzing tests (_see f.ex #986 and #989_), but for our purposes with respect to the inbound proxy we can elide them rather than making `ctx()` a public method. --- Signed-off-by: katelyn martin <[email protected]> * refactor(app/test): remove unused `http_util::connect_and_accept(..)` this removes `connect_and_accept(..)`. this will break fuzzing builds, but it is not used elsewhere. Signed-off-by: katelyn martin <[email protected]> * chore(fuzz): address hyper deprecation in inbound fuzz tests Signed-off-by: katelyn martin <[email protected]> * chore(fuzz): address preëxisting fuzz breakage this commit addresses other breakage found in the fuzz tests, tied to other previous work. after these changes, one can observe that the fuzz tests build and run once more by running the following: ```sh cargo +nightly fuzz run --fuzz-dir=linkerd/app/inbound/fuzz/ fuzz_target_1 ``` Signed-off-by: katelyn martin <[email protected]> * nit(fuzz): remove stray newline from manifest Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>

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 commented Dec 11, 2024

View reviewed changes

cratelyn marked this pull request as ready for review December 11, 2024 06:36

cratelyn requested a review from a team as a code owner December 11, 2024 06:36

olix0r approved these changes Dec 11, 2024

View reviewed changes

cratelyn merged commit 24dc5d8 into main Dec 11, 2024
15 checks passed

cratelyn deleted the kate/hyper-1.x-address-inbound-http-tests-deprecation branch December 11, 2024 16:37

cratelyn mentioned this pull request Dec 11, 2024

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

Closed

cratelyn mentioned this pull request Dec 13, 2024

chore(app/inbound): address hyper deprecations in http/1 tests #3454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(app/inbound): address hyper deprecations in http/2 tests #3445

chore(app/inbound): address hyper deprecations in http/2 tests #3445

Uh oh!

cratelyn commented Dec 11, 2024 •

edited

Loading

Uh oh!

cratelyn Dec 11, 2024

Uh oh!

cratelyn Dec 11, 2024

Uh oh!

Uh oh!

Uh oh!

chore(app/inbound): address hyper deprecations in http/2 tests #3445

chore(app/inbound): address hyper deprecations in http/2 tests #3445

Uh oh!

Conversation

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

Uh oh!

Uh oh!

cratelyn Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

cratelyn Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cratelyn commented Dec 11, 2024 •

edited

Loading