Repository navigation
reqwest: re-apply 0.13 bump at 0.13.4 with explicit TLS backend pin - #37469
Merged
Merged
Conversation
def-
approved these changes
Jul 7, 2026
def-
left a comment
Contributor
There was a problem hiding this comment.
Let's hope it's green now. Where is this flake playbook btw?
jasonhernandez
added a commit
that referenced
this pull request
Jul 7, 2026
… 8.x) (#37470) ## Summary Re-enables the Kafka auth test suite, disabled since #36405 (SS-115), and fixes the two breakages that accumulated while it was dark. ## What broke while the suite was disabled The Confluent Platform 7.9.4 → 8.2.0 bump (#36683) landed three weeks after the suite was skipped, and broke its setup in two ways nothing could catch: 1. **Schema registry basic auth returned 401 for everyone.** CP 8.x ships Jetty 12, which moved the JAAS login modules from `org.eclipse.jetty.jaas.spi` to `org.eclipse.jetty.security.jaas.spi`. Our `schema-registry.jaas.config` referenced the old class, so the login module never loaded and every request, correct credentials included, got 401. Verified directly: `curl -u materialize:sekurity` against the container → 401 before, 200 after the one-line class-path fix (wrong/missing creds still 401). 2. **Kafka mssl negative tests asserted TLS 1.2 alert text.** CP 8.x brokers negotiate TLS 1.3, where a missing/untrusted client certificate fails with `tlsv13 alert certificate required` (alert 116) instead of `ssl/tls alert bad certificate` (alert 42). The assertions now check only that the broker rejected the connection with a TLS alert. The exact wording is a function of TLS version and OpenSSL build that we do not control. This is the third exact-text loosening in this suite's history (#30501, #36232), so the loosening deliberately goes all the way rather than chasing the new strings. ## On the original SS-115 flake The `PEM routines:get_name:no start line` flake that got the suite disabled did not reproduce in local full-suite runs against current main images (reqwest 0.12) or against the reqwest 0.13.4 branch (#37469). Timeline evidence from SS-115 itself shows the flake continued after the reqwest 0.13 revert (#36241), so reqwest was likely misattributed. If it resurfaces, the `ci-regexp` in SS-115 will link it; the suite being enabled is what gives us the data to root-cause it. ## Why now Two in-flight workstreams change exactly the surface this suite covers: the reqwest 0.13 re-application (#37469) and the rdkafka → AWS-LC switch (#35941). Landing those with this suite dark is how auth/TLS regressions reach production unobserved. ## Test plan - [x] Full suite green locally against main images (all fixes applied) - [x] Repeated local runs to probe for the SS-115 flake: 8 consecutive local full-suite passes (5-run loop + 3 individual), plus 2 clean runs on the reqwest-0.12 main baseline. The PEM flake did not reproduce. - [ ] Kafka auth 1–3 green in this PR's CI (the un-skip makes them run here) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Jason Hernandez <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>
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 <[email protected]> Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
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 <[email protected]>
jasonhernandez
force-pushed
the
jason/reqwest-013-retry
branch
from
July 7, 2026 13:25
9d33029 to
a8106d9
Compare
Contributor
Author
The flake playbook is a reference to the mz-test skill. I'm going to rebase this on the kafka-auth tests quickly. |
Contributor
|
I'm quite concerned that this was merged. It failed multiple times even in the PR, now main is unusable: https://buildkite.com/materialize/test/builds/127623 |
def-
added a commit
that referenced
this pull request
Jul 7, 2026
…nd pin" (#37491) Failed again immediately: [Kafka auth 3](https://buildkite.com/materialize/test/builds/127633#019f3cdf-4fda-4523-bcb5-0d80fd378764) Reverts #37469
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.

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
reqwestto 0.13.4,reqwest-middlewareto 0.5.2, andreqwest-retryto 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 lineerror at connection time on theccsr(schema registry) client, on requests whose TLS handshake should have succeeded. Three things are different now:rustls-platform-verifierupgrade (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 viasentry0.48, so reqwest 0.13 is not itself new to production images.ccsrpinstls_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 carriesrustls-platform-verifier/quinn), whileccsr'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.reqwest_0_12workspace alias, keeping theBlobKnobstimeout plumbing intact on both the emulator and production paths (azure_core0.21 implementsHttpClientonly for reqwest 0.12; April dropped the emulator-path timeouts to work around this). The alias also covers iceberg'sRequestAuthenticator, 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-testskill'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.cargo check --workspace --all-targetscargo --locked deny check licenses bans sources,bin/lint-cargo,bin/fmtmz-testskill)🤖 Generated with Claude Code