Bump reqwest to 0.13 - #36215
Merged
Merged
Conversation
jasonhernandez
marked this pull request as ready for review
April 22, 2026 21:15
jasonhernandez
force-pushed
the
jason/bump-reqwest-013
branch
4 times, most recently
from
April 23, 2026 21:55
fc787d3 to
c6913da
Compare
2 tasks
jasonhernandez
force-pushed
the
jason/bump-reqwest-013
branch
from
April 23, 2026 22:07
c6913da to
43e5cc1
Compare
reqwest 0.13 flipped the default TLS backend from native-tls to rustls and made `query` opt-in; we pin `native-tls`/`native-tls-vendored` explicitly to keep the previous behavior and enable `query` for the four call sites that use it (cloud-api, frontegg-client). The full rustls-with-aws-lc-rs migration is tracked separately in the crypto migration plan and is deliberately not in scope here. reqwest-middleware and reqwest-retry are bumped to the matching 0.5 / 0.9 lines. Two workarounds for code that can't use the new reqwest version yet: - `src/persist/src/azure.rs`: the custom `TransportOptions` plumbing passed a `reqwest::Client` to azure_core 0.21's `HttpClient` trait, which is pinned to reqwest 0.12 internally. Drop the custom transport for now; `operation_timeout` still applies via the retry policy. The per-attempt/read/connect timeout plumbing will be restored by the separate azure_sdk 0.35 migration. - `src/storage-types/src/connections.rs`: the `AwsCredentialLoad` trait comes from reqsign 0.16 (via our iceberg fork) and is defined against reqwest 0.12's `Client`. Add a versioned `reqwest_0_12` alias dep and use it for the single trait-impl parameter. Can be removed when the iceberg fork picks up a reqsign that supports reqwest 0.13. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- reqwest 0.13 changed its default features: `default-tls` now activates `rustls` (which is banned) and `system-proxy` is a new feature that was implicit in 0.12. Add `default-features = false` and list everything explicitly, including `system-proxy`. - Move the `reqwest_0_12` alias into `[workspace.dependencies]` — the Cargo lint forbids inlining versions in member crate manifests. - Allow the intentional duplicate reqwest 0.12 / 0.13 pair in `deny.toml`'s skip list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each reqwest_0_12 site was repeating the same reqsign 0.16 / iceberg fork story. Keep the full explanation once at the workspace declaration and leave one-line pointers at the consuming sites (deny.toml, the storage-types Cargo.toml, and the AwsCredentialLoad impl). Also trim the feature-flag comment on reqwest and the TransportOptions note in azure.rs to the essentials. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
def-
force-pushed
the
jason/bump-reqwest-013
branch
from
April 24, 2026 00:51
43e5cc1 to
18ae917
Compare
jasonhernandez
added a commit
that referenced
this pull request
Apr 24, 2026
## Summary
Two `!` assertions in
`test/kafka-auth/test-schema-registry-mssl{-basic,}.td` have been flaky
for over a year because the exact TLS error text varies with the TLS
backend, reqwest version, and OpenSSL build in use. The tests only
verify that a disallowed-client-cert connection fails — the wording is
"not under our control" (as the comment in the tests already says).
Switch both `contains:alert certificate unknown` to `contains:error
sending request for url` — the outer reqwest wrapper that every failure
mode we've observed shares.
## Precedent
- [#30501](#30501)
(Nov 2024) loosened the same test family for the same reason, citing 16
CI failures over an inconsistent error string. Dennis's commit message:
*"I'm not sure if this indicates something worse or is just an
inconsistent error message."*
## Variants observed
- `ssl/tls alert certificate unknown` (native-tls + OpenSSL, current
main)
- `alert bad certificate` (pre-2024 wording, fixed by #30501)
- `CertificateUnknown` (rustls; from the crypto migration branch)
- `PEM routines:get_name:no start
line:crypto/pem/pem_lib.c:773:Expecting: CERTIFICATE` (reqwest 0.13 on
Linux, intermittent)
## Why separate from the reqwest bump
The reqwest 0.13 bump
([#36215](#36215))
surfaces the OpenSSL PEM variant more often, which is how I noticed this
— but it's not the root cause. The flakiness is latent on `main` and
will bite any subsequent TLS-stack change (rustls migration, OpenSSL
updates). Hardening the assertion is independently justifiable.
## Test plan
- [ ] `bin/mzcompose --find kafka-auth run default` passes.
- [ ] No other `alert certificate unknown` assertions exist against
these endpoints (verified via `grep`).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Jason Hernandez <7144515+jasonhernandez@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
def-
approved these changes
Apr 24, 2026
jasonhernandez
enabled auto-merge (squash)
April 24, 2026 00:56
def-
added a commit
that referenced
this pull request
Apr 24, 2026
This reverts commit 6d7c3bb. Causing a lot of flakiness in CI: https://buildkite.com/materialize/test/builds/121629 and https://buildkite.com/materialize/test/builds/121626 @jasonhernandez Alternatively you could also fix the test failures, or we could disable them.
4 tasks
jasonhernandez
added a commit
that referenced
this pull request
Jul 7, 2026
One of several sequenced PRs in the crypto migration toward rustls + aws-lc-rs for everything. Specifically: - **This PR** — mechanical bump from reqwest 0.12 → 0.13. Keeps the existing `native-tls` backend for now to scope the change to just the version bump. Does not flip the TLS backend. - **Next in the reqwest chain** — the HTTP-clients-to-rustls PR (#35947 in the plan) swaps `native-tls` for `rustls` with `aws-lc-rs` across all reqwest consumers. That's where the real TLS backend change happens. - **In parallel** — the Azure SDK migration (#36216, stacked on this) adopts the new `azure_core` 0.35 series, which defaults to `reqwest_rustls` + aws-lc-rs on its own; that's a concrete step forward on the Azure path even before the workspace-wide flip. - Bump `reqwest` 0.12.28 → 0.13.2, `reqwest-middleware` 0.4.2 → 0.5.1, `reqwest-retry` 0.8.0 → 0.9.1. - `default-features = false` on reqwest so the 0.13 default changes (now `default-tls = rustls`, new `system-proxy` feature) don't ambush us mid-bump. - Explicitly list the feature set we had in 0.12 (including `native-tls` / `native-tls-vendored` for now) plus the two newly-opt-in features we need: `query` (used by `cloud-api` and `frontegg-client`) and `system-proxy` (was implicit in 0.12). The `native-tls` pin here is a temporary consequence of keeping this PR scoped to the bump — the crypto migration's HTTP-clients PR is where the switch to rustls actually lands. Two places still need reqwest 0.12 because their trait ecosystem is pinned there. Both are narrow and clearly labeled; both go away as the related migrations land. 1. **`src/persist/src/azure.rs` — Azurite test path only.** The old code plumbed a custom `reqwest::Client` through `TransportOptions` inside the `if account == EMULATOR_ACCOUNT` branch to apply short `BlobKnobs` per-attempt / read / connect timeouts (5s–10s) to the Azurite test client. `azure_core` 0.21 pins reqwest 0.12 internally, so our 0.13 `Client` no longer implements its `HttpClient` trait. - **Production impact: zero.** The `else` branch (real Azure Blob Storage) never set a custom transport — it uses `BlobServiceClient::new(...)` which gets the SDK's default reqwest client. That behavior is unchanged. - **Test impact: minimal.** Azurite runs on localhost, so falling back to the SDK's default timeouts has no observable effect on test reliability. - The outer `knobs.operation_timeout()` is preserved via the retry policy's `max_total_elapsed` in both branches. - The Azure SDK 0.35 migration (#36216, stacked) restores the emulator-path timeout plumbing against the new SDK. 2. **`src/storage-types/src/connections.rs` — `AwsCredentialLoad` impl.** The `reqsign::AwsCredentialLoad` trait (re-exported from iceberg) is defined against reqwest 0.12. Added a versioned `reqwest_0_12` alias dep in `[workspace.dependencies]` and use it only for the single trait-impl parameter. Removable once iceberg upstream bumps to a reqsign that supports reqwest 0.13 — tracked as a multi-hop ecosystem blocker (reqsign 0.17 was a breaking API rewrite; neither iceberg nor opendal have migrated). ~27 new packages (quinn for HTTP/3, rustls-platform-verifier, wasm-streams, newer windows-sys). Both reqwest 0.12 and 0.13 coexist in the tree — the 0.12 copy comes in via azure_core 0.21 and reqsign 0.16 and is needed only for the two workarounds above. Exempted in `deny.toml`'s skip list. - [x] `cargo check --workspace --all-targets` — clean - [x] `cargo --locked deny check bans sources` — `bans ok` - [x] `bin/lint-cargo` — exit 0 - [x] Full `cargo test` run / CI green - [ ] Smoke-test at least one TLS client path (`ccsr`, `frontegg-client`, or similar) against a real endpoint to confirm native-tls still works at 0.13 before the crypto migration's rustls switch lands 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Jason Hernandez <7144515+jasonhernandez@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jasonhernandez
added a commit
that referenced
this pull request
Jul 7, 2026
Re-application of #36215 (reverted in #36241 for flaky Kafka auth schema-registry TLS tests) with three changes: - reqwest 0.13.4 instead of 0.13.2. Upstream has since fixed several bugs in the TLS config layer, notably PEM parsing of CRL values and native-tls ALPN/TLS 1.3 handling. sentry 0.47 already ships reqwest 0.13.3 in our tree, so 0.13 itself is not new here. - ccsr pins its client to tls_backend_native(). reqwest 0.13 defaults to rustls whenever transitive deps compile the rustls backend in, and ccsr's certificates and identities are prepared for native-tls. The April flake surfaced OpenSSL PEM errors at connect time on exactly this client, consistent with TLS backend/verifier drift. - The Azure transport client comes from the reqwest_0_12 alias, keeping the BlobKnobs timeout plumbing on both the emulator and production paths. azure_core 0.21 implements HttpClient only for reqwest 0.12. The alias also covers iceberg's RequestAuthenticator, which gained a second impl (Sigv4Authenticator) since April. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jasonhernandez
added a commit
that referenced
this pull request
Jul 7, 2026
…37469) ## Summary Re-applies the reqwest 0.12 → 0.13 bump (originally #36215, reverted in #36241 when the Kafka auth schema-registry TLS tests started flaking on main). Bumps `reqwest` to 0.13.4, `reqwest-middleware` to 0.5.2, and `reqwest-retry` to 0.9.1. This is only the version bump — the TLS backend stays native-tls; the switch to rustls is owned separately by the crypto migration (#35947). ## Why it should stick this time The April flake was an OpenSSL `PEM routines:get_name:no start line` error at connection time on the `ccsr` (schema registry) client, on requests whose TLS handshake should have succeeded. Three things are different now: 1. **reqwest 0.13.4, not 0.13.2.** Upstream has since fixed several bugs in the 0.13 TLS config layer: PEM parsing of CRL values plus a `rustls-platform-verifier` upgrade (0.13.3), native-tls ALPN combinations (late 0.13.2), and TLS 1.3 for native-tls (0.13.4). Note the tree already ships reqwest 0.13.3 transitively via `sentry` 0.48, so reqwest 0.13 is not itself new to production images. 2. **`ccsr` pins `tls_backend_native()` explicitly.** In reqwest 0.13, rustls becomes the default backend whenever any transitive dependency compiles the rustls feature in (ours do — the lockfile carries `rustls-platform-verifier`/`quinn`), while `ccsr`'s certificates and identities are prepared for native-tls. Silent backend/verifier drift on this exact client is the most plausible mechanism for the April flake, and the explicit pin removes that degree of freedom regardless of which theory is correct. 3. **Cleaner handling of the reqwest-0.12-only ecosystem pins.** The Azure transport client now comes from the `reqwest_0_12` workspace alias, keeping the `BlobKnobs` timeout plumbing intact on both the emulator and production paths (`azure_core` 0.21 implements `HttpClient` only for reqwest 0.12; April dropped the emulator-path timeouts to work around this). The alias also covers iceberg's `RequestAuthenticator`, which gained a second impl (`Sigv4Authenticator`) since April. ## Validation plan The Kafka auth suite is the regression signal that matters here (it exercises the exact schema-registry TLS path that flaked). It was disabled in CI when this PR was first opened (SS-115), but #37470 re-enabled it and merged, and this branch is now rebased on top — so **Kafka auth 1–3 run on this PR's CI** and directly exercise the April failure mode. Because that failure was intermittent, a single green run isn't sufficient. The `mz-test` skill's flake-handling guidance applies: re-trigger the build 10–20 times (via https://ci.dev.materialize.com/trigger, clearing the target branch for parallel runs) and confirm Kafka auth stays green before merging. - [x] `cargo check --workspace --all-targets` - [x] `cargo --locked deny check licenses bans sources`, `bin/lint-cargo`, `bin/fmt` - [x] Kafka auth suite green locally (8/8 consecutive runs) on this stack - [ ] Kafka auth green across 10–20 CI re-triggers (now runs here post-rebase, per the `mz-test` skill) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Jason Hernandez <7144515+jasonhernandez@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One of several sequenced PRs in the crypto migration toward rustls + aws-lc-rs for everything.
Specifically:
native-tlsbackend for now to scope the change to just the version bump. Does not flip the TLS backend.native-tlsforrustlswithaws-lc-rsacross all reqwest consumers. That's where the real TLS backend change happens.azure_core0.35 series, which defaults toreqwest_rustls+ aws-lc-rs on its own; that's a concrete step forward on the Azure path even before the workspace-wide flip.Summary
reqwest0.12.28 → 0.13.2,reqwest-middleware0.4.2 → 0.5.1,reqwest-retry0.8.0 → 0.9.1.default-features = falseon reqwest so the 0.13 default changes (nowdefault-tls = rustls, newsystem-proxyfeature) don't ambush us mid-bump.native-tls/native-tls-vendoredfor now) plus the two newly-opt-in features we need:query(used bycloud-apiandfrontegg-client) andsystem-proxy(was implicit in 0.12).The
native-tlspin here is a temporary consequence of keeping this PR scoped to the bump — the crypto migration's HTTP-clients PR is where the switch to rustls actually lands.Known workarounds for incompatible call sites
Two places still need reqwest 0.12 because their trait ecosystem is pinned there. Both are narrow and clearly labeled; both go away as the related migrations land.
src/persist/src/azure.rs— Azurite test path only. The old code plumbed a customreqwest::ClientthroughTransportOptionsinside theif account == EMULATOR_ACCOUNTbranch to apply shortBlobKnobsper-attempt / read / connect timeouts (5s–10s) to the Azurite test client.azure_core0.21 pins reqwest 0.12 internally, so our 0.13Clientno longer implements itsHttpClienttrait.elsebranch (real Azure Blob Storage) never set a custom transport — it usesBlobServiceClient::new(...)which gets the SDK's default reqwest client. That behavior is unchanged.knobs.operation_timeout()is preserved via the retry policy'smax_total_elapsedin both branches.src/storage-types/src/connections.rs—AwsCredentialLoadimpl. Thereqsign::AwsCredentialLoadtrait (re-exported from iceberg) is defined against reqwest 0.12. Added a versionedreqwest_0_12alias dep in[workspace.dependencies]and use it only for the single trait-impl parameter. Removable once iceberg upstream bumps to a reqsign that supports reqwest 0.13 — tracked as a multi-hop ecosystem blocker (reqsign 0.17 was a breaking API rewrite; neither iceberg nor opendal have migrated).Cargo.lock
~27 new packages (quinn for HTTP/3, rustls-platform-verifier, wasm-streams, newer windows-sys). Both reqwest 0.12 and 0.13 coexist in the tree — the 0.12 copy comes in via azure_core 0.21 and reqsign 0.16 and is needed only for the two workarounds above. Exempted in
deny.toml's skip list.Test plan
cargo check --workspace --all-targets— cleancargo --locked deny check bans sources—bans okbin/lint-cargo— exit 0cargo testrun / CI greenccsr,frontegg-client, or similar) against a real endpoint to confirm native-tls still works at 0.13 before the crypto migration's rustls switch lands🤖 Generated with Claude Code