Skip to content

persist: migrate to the GA Azure SDK crates - #36216

Draft
jasonhernandez wants to merge 1 commit into
mainfrom
jason/azure-sdk-new-crates
Draft

persist: migrate to the GA Azure SDK crates#36216
jasonhernandez wants to merge 1 commit into
mainfrom
jason/azure-sdk-new-crates

Conversation

@jasonhernandez

@jasonhernandez jasonhernandez commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Replaces azure_core / azure_identity / azure_storage / azure_storage_blobs 0.21 with the 1.x GA line: azure_core 1.1, azure_identity 1.0, azure_storage_blob 1.0.

This is a rewrite of the earlier version of this PR, which targeted the 0.35 preview series and was stacked on the reqwest 0.13 workspace bump. Both of those premises changed:

Net effect: -305 lines of our own code and -183 lines of Cargo.lock. The 0.21 series is unmaintained and drags an unmaintained HTTP stack with it (http-types, RustyXML, oauth2, async-std's executor pieces), which is also how one RUSTSEC exemption retires here.

The big win: CLO-190's workaround is now upstream

main carries a 426-line RefreshingWorkloadIdentityCredential (#38178, CLO-190). It exists because azure_identity 0.21's WorkloadIdentityCredential read AZURE_FEDERATED_TOKEN_FILE once at construction and held the contents for process lifetime. Kubernetes rotates that projected token, so once the last cached AAD access token expired, every refresh presented an expired client assertion and a long-running process was permanently locked out of blob storage.

azure_identity 1.0 fixes this upstream: WorkloadIdentityCredential now holds a ClientAssertion that re-reads the token file on a 10-minute interval (workload_identity_credential.rs, TIMEOUT = 600s). The whole module and its two tests are deleted.

The one behavior we lose is .trim() on the token file contents. Kubernetes writes the projected token without surrounding whitespace, so this only affected hand-provisioned files, which the CLO-190 comment already called out as the edge case.

Credential chain

create_default_credential() no longer exists in 1.x, so the chain is assembled in azure.rs: workload identity → managed identity → developer tools, tried in order on each token request.

That covers the same credential types 0.21's DefaultAzureCredential did (App Service MI + VM/IMDS MI collapse into ManagedIdentityCredential; Azure CLI + azd collapse into DeveloperToolsCredential) minus EnvironmentCredential, which 1.x dropped and which we never provisioned. Managed identity is in the chain, which resolves the open risk the previous version of this PR flagged.

Sources are re-tried per token request rather than latching onto the first that worked, matching 0.21's behavior — construction-time selection is not an option because ManagedIdentityCredential::new succeeds on any host and would shadow the developer-tools fallback on a laptop.

API migration in src/persist/src/azure.rs

  • ContainerClientBlobContainerClient, built from a single container URL rather than (account, container).
  • blob_client.get().into_stream()blob.download(None). 1.x issues parallel range requests internally and returns one body stream, so the hand-rolled FuturesOrdered over the SDK's chunk pager goes away.
  • blob.put_block_blob(value)blob.upload(value.into(), None).
  • get_properties / delete / create take None options; list_blobs yields BlobItems from a Pager.
  • Error::as_http_error().status()Error::http_status() (also in location.rs).

What to scrutinize

src/persist/src/azure/azurite.rs (new, 191 lines). The one part worth reading line by line, against the Shared Key spec. 1.x dropped the emulator credential and Azurite accepts only Shared Key, so a per-try policy signs each request. Notes:

  • The canonicalized resource repeats the account name (/devstoreaccount1/devstoreaccount1/container/...). That looks wrong and isn't: Azurite runs with --disableProductStyleUrl, so the account is addressed through the URL path and Azurite prepends the account it resolved. Signing it any other way returns AuthorizationFailure. There is a NOTE: on this.
  • The emulator client pins x-ms-version to 2025-01-05. The SDK's default is 2026-04-06, which our pinned Azurite 3.33.0 rejects outright with InvalidHeaderValue.
  • Signing uses azure_core::hmac::hmac_sha256 (the hmac_rust feature) rather than a direct hmac dependency.
  • It is not #[cfg(test)]-gated. mzcompose points persist at http://devstoreaccount1.azurite:10000/container using optimized builds, so gating this behind cfg(test) would break every mzcompose test that uses Azurite as the blob store. The Azurite key is a publicly documented constant and this path is only reachable for the equally well-known devstoreaccount1 account name.

The custom transport client. Still needed: the SDK's own client hardcodes 20s connect / 60s read timeouts and ignores BlobKnobs. Two deliberate choices:

  • .tls_backend_rustls() — explicit rather than relying on reqwest 0.13's default, so an unrelated dependency enabling native-tls cannot silently move Azure traffic onto it. This is the ccsr lesson from reqwest: re-apply 0.13 bump at 0.13.4 with explicit TLS backend pin #37469.
  • .no_gzip().no_deflate().no_brotli().no_zstd()required, not hygiene. reqwest enables auto-decompression by default for every codec whose feature is on, and the SDK turns on gzip and deflate. BlobClient::download reassembles a blob from range requests keyed by byte offset, which a transparently decompressed body invalidates. The SDK's own apply_client_defaults disables decompression for exactly this reason; a custom transport has to do it itself.

get's response-length metric. In 1.x, content_length on a download result describes only the initial range, not the whole blob, so comparing it against total bytes read would fire get_invalid_resp on every multi-part download. total_len() reads the total out of Content-Range when present and falls back to content_length for blobs served unranged.

deny.toml

  • Drops the exemptions the 0.21 chain needed: duplicate async-channel / event-listener / fastrand / futures-lite / getrandom 0.1 / rand 0.7 / rand_chacha 0.2 / rand_core 0.5, and the azure_svc_blobstorage log wrapper.
  • Retires RUSTSEC-2026-0174 (http-types), which only entered the tree through the azure_* crates.
  • Adds a log wrapper for rustls-platform-verifier, which arrives with reqwest 0.13's rustls backend.
  • Re-points the quick-xml 0.31 skip comment at junit-report, its actual source now.

Test plan

  • cargo check --workspace --all-targets — clean
  • cargo clippy -p mz-persist --all-targets — clean
  • bin/fmt, bin/lint-cargo, bin/unused-deps — clean (time dropped from mz-persist, it was only used by the deleted credential)
  • cargo deny check bans sources licenses — ok. advisories fails only on the pre-existing h2 RUSTSEC-2026-0258, verified identical on main.
  • cargo test -p mz-persist against a real Azurite — 18/18 pass, including azure::tests::azure_blob. This was the previous version's biggest open item: the Shared Key path had only ever been type-checked. Verified against mcr.microsoft.com/azure-storage/azurite:3.33.0 with the same flags misc/python/materialize/mzcompose/services/azurite.py uses, with Azurite debug logging on to confirm zero Validation failed lines across the full blob_impl_test suite (GET/PUT/HEAD/DELETE/list, including the empty-blob and ranged-read paths). Finding and fixing the canonicalized-resource bug came out of this.

doc/developer/generated/persist/azure.md still refers to azure_storage_blobs. That tree is maintained solely by the update-docs agent, so it is left alone here.

Part of SEC-619.

🤖 Generated with Claude Code

@jasonhernandez jasonhernandez mentioned this pull request Apr 22, 2026
5 tasks
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch 6 times, most recently from 5e0f2cf to dceb821 Compare April 23, 2026 21:01
@jasonhernandez
jasonhernandez force-pushed the jason/bump-reqwest-013 branch from fb85618 to cbbc3ff Compare April 23, 2026 21:09
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from dceb821 to 4be17ae Compare April 23, 2026 21:10
@jasonhernandez
jasonhernandez force-pushed the jason/bump-reqwest-013 branch from cbbc3ff to 7f950aa Compare April 23, 2026 21:15
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from 4be17ae to 456b931 Compare April 23, 2026 21:16
@jasonhernandez
jasonhernandez force-pushed the jason/bump-reqwest-013 branch from 7f950aa to fc787d3 Compare April 23, 2026 21:48
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from 456b931 to 847954d Compare April 23, 2026 21:48
@jasonhernandez
jasonhernandez force-pushed the jason/bump-reqwest-013 branch from fc787d3 to c6913da Compare April 23, 2026 21:55
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from 847954d to cd39544 Compare April 23, 2026 21:55
@jasonhernandez
jasonhernandez force-pushed the jason/bump-reqwest-013 branch from c6913da to 43e5cc1 Compare April 23, 2026 22:07
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from cd39544 to 058263c Compare April 23, 2026 22:07
@def-
def- force-pushed the jason/bump-reqwest-013 branch from 43e5cc1 to 18ae917 Compare April 24, 2026 00:51
Base automatically changed from jason/bump-reqwest-013 to main April 24, 2026 01:11
jasonhernandez added a commit that referenced this pull request Apr 24, 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.

## Summary
- 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.

## 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.

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).

## 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
- [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
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch 2 times, most recently from cbe57ef to 515447f Compare April 24, 2026 01:41
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from 515447f to 2db4ac5 Compare July 7, 2026 06:19
@jasonhernandez
jasonhernandez changed the base branch from main to jason/reqwest-013-retry July 7, 2026 06:19
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
jasonhernandez force-pushed the jason/reqwest-013-retry branch from 9d33029 to a8106d9 Compare July 7, 2026 13:25
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from 2db4ac5 to 1c8895f Compare July 7, 2026 13:44
Base automatically changed from jason/reqwest-013-retry to main July 7, 2026 13:58
Replaces azure_core / azure_identity / azure_storage / azure_storage_blobs
0.21 with the 1.x GA line: azure_core 1.1, azure_identity 1.0, and
azure_storage_blob 1.0. The 0.21 series is unmaintained and drags an
unmaintained HTTP stack (http-types, RustyXML, oauth2, async-std's
executor pieces) along with it. Net effect on the lock file is 180 fewer
lines and one retired RUSTSEC exemption.

API migration in src/persist/src/azure.rs:

- `ContainerClient` -> `BlobContainerClient`, constructed from a single
  container URL rather than an account plus a container name.
- `blob_client.get().into_stream()` -> `blob.download(None)`. 1.x
  downloads a blob as a set of parallel range requests and hands back one
  body stream, replacing the hand-rolled `FuturesOrdered` over the SDK's
  chunk pager.
- `blob.put_block_blob(value)` -> `blob.upload(value.into(), None)`.
- `get_properties` / `delete` / `create` take `None` options.
- `list_blobs` yields `BlobItem`s directly from a `Pager`.
- `Error::as_http_error().status()` -> `Error::http_status()`.

`RefreshingWorkloadIdentityCredential` is gone. It existed because
`azure_identity` 0.21's `WorkloadIdentityCredential` read
`AZURE_FEDERATED_TOKEN_FILE` once at construction, so a long-running
process was permanently locked out of blob storage after Kubernetes
rotated the projected token. 1.x re-reads the file on a 10 minute
interval behind its `ClientAssertion` trait, which is the same fix. The
one behavior we drop is trimming trailing whitespace from the token file;
Kubernetes writes the token without any.

`create_default_credential` is also gone from 1.x, so the credential
chain is assembled here: workload identity, then managed identity, then
the local developer tools. That covers the same credential types 0.21's
default chain did, minus `EnvironmentCredential`, which 1.x dropped and
which we never provisioned.

The custom transport client stays, since the SDK's own client hardcodes
20s connect and 60s read timeouts and ignores `BlobKnobs`. It has to be a
reqwest 0.13 client because that is the only major the SDK implements
`HttpClient` for, hence the `reqwest_0_13` alias in the workspace
manifest; the workspace `reqwest` stays at 0.12. The client explicitly
selects rustls (and so aws-lc-rs) and explicitly disables automatic
decompression, which reqwest would otherwise turn on and which would
corrupt the offset accounting in a partitioned download.

Azurite support moves into src/persist/src/azure/azurite.rs. 1.x dropped
the emulator's built-in credential, and the emulator only accepts the
Azure Storage Shared Key scheme, so a per-try policy signs each request
per
https://learn.microsoft.com/en-us/rest/api/storageservices/authorize-with-shared-key.
The emulator client is also pinned to the newest service API version our
pinned Azurite accepts, because the SDK's default is newer and Azurite
rejects it outright. The module is the one part of this change worth
reading line by line against the spec.

deny.toml drops the exemptions the 0.21 chain needed (its duplicate
async-channel / event-listener / fastrand / futures-lite / getrandom /
rand versions, the `azure_svc_blobstorage` log wrapper, and the
http-types RUSTSEC ignore) and adds a log wrapper for
rustls-platform-verifier, which arrives with reqwest 0.13's rustls
backend.

Part of SEC-619.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jasonhernandez
jasonhernandez force-pushed the jason/azure-sdk-new-crates branch from 1c8895f to 69f27a1 Compare August 19, 2026 18:59
@jasonhernandez jasonhernandez changed the title persist: migrate to the new Azure SDK crates persist: migrate to the GA Azure SDK crates Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant