WIP: migrate Azure blob storage to azure_storage_blob 1.0.0 - #1
Closed
siva-abstract-security wants to merge 1 commit into
Closed
WIP: migrate Azure blob storage to azure_storage_blob 1.0.0#1siva-abstract-security wants to merge 1 commit into
siva-abstract-security wants to merge 1 commit into
Conversation
Move the workspace off the legacy Azure SDK and onto the 1.0 line: azure_core 1.1, azure_identity 1.0 and azure_storage_blob 1.0. `azure_storage` is dropped outright. The rewritten SDK has no successor for `StorageCredentials`, `CloudLocation` or `ConnectionString`, so the concepts it provided have to be rebuilt on top of the pipeline instead of renamed. Feature names changed with the rewrite: `enable_reqwest_rustls` is now `reqwest_rustls`, and the `azurite_workaround` features no longer exist, so they leave `integration-testsuite`. `hmac_rust` survives in `azure_core` 1.1, which matters because a shared key signing policy needs it. The new SDK resolves to a smaller graph: `Cargo.lock` loses 296 lines net. This commit only moves the dependencies. `quickwit-storage` does not build against them yet.
Owner
Author
|
Opened upstream instead: quickwit-oss#6693. Same branch, so this duplicate is closed. |
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.
Follow-up to quickwit-oss#6672, where guilload asked for an upgrade to azure_storage_blob 1.0.0 rather than a patch against azure_identity 0.21.
Status: the dependency layer is done and resolves. The backend port is not written. This branch does not compile. Do not read this as a finished change.
What is done
Workspace dependencies now point at azure_core 1.1.0, azure_identity 1.0.0 and azure_storage_blob 1.0.0. The azure_storage crate is dropped outright, because the rewritten SDK has no successor for StorageCredentials, CloudLocation or ConnectionString. Those concepts need rebuilding on the pipeline rather than renaming.
Cargo.lock loses 296 lines net, so the new SDK pulls a smaller dependency graph.
Feature renames: enable_reqwest_rustls becomes reqwest_rustls, and the azurite_workaround features no longer exist, so they leave integration-testsuite. hmac_rust survives in azure_core 1.1, which matters for a shared key signing policy. New SDK MSRV is 1.88 against the pinned toolchain of 1.96.
What the upgrade fixes
azure_identity 1.0.0 re-reads the federated token file whenever the cached copy is older than 600 seconds. See the ClientAssertion impl in workload_identity_credential.rs. Every token exchange presents a fresh assertion, so the 24 hour indexer failure in quickwit-oss#6672 stops occurring. No workaround needed once this lands.
Three findings from reading the 1.0.0 source
Two of these change the design of the port.
The default credential chain is gone. azure_identity 1.0.0 ships no DefaultAzureCredential and no create_credential. DeveloperToolsCredential chains only the Azure CLI and the azd CLI, so production use falls outside what the type covers. Quickwit currently calls azure_identity::create_credential() and lets the SDK inspect the environment. The port has to choose workload identity or managed identity from the environment explicitly.
Shared key auth is gone, and Microsoft has said they will not add support back. See Support shared client credentials / access key to be able to connect to azurite Azure/azure-sdk-for-rust#2975, comment dated 31 July 2026. Two places in Quickwit depend on shared key: the documented access_key config option, and ClientBuilder::emulator() behind integration-testsuite. Azurite speaks shared key.
The token scope is hardcoded. All six clients in azure_storage_blob 1.0.0 pass vec!["https://storage.azure.com/.default"] into BearerTokenAuthorizationPolicy, and BlobContainerClientOptions exposes no audience field. Sovereign clouds need a different scope, which is the subject of fix: sovereign Azure storage token scopes for managed identity quickwit-oss/quickwit#6666, so a naive port would regress sovereign cloud support.
A design suggestion covering all three
The credential parameter on every client constructor is an Option, and passing None skips the built-in bearer policy and also allows plain http. So Quickwit would pass None and install its own auth policy through ClientOptions per_retry_policies, selecting between a bearer policy carrying the correct sovereign scope and a shared key signing policy. One seam then covers access_key, Azurite and sovereign scopes.
Open questions for upstream
Do you want access_key kept working through a shared key signing policy carried in quickwit-storage, around 150 lines of HMAC canonicalization lifted from azure_storage 0.21? Or do you want account key auth dropped in favour of Entra only, with Azurite switched to oauth basic mode for tests?
fix: sovereign Azure storage token scopes for managed identity quickwit-oss/quickwit#6666 is open on the same call site. Should the migration wait for the merge, or absorb the sovereign scope work?
What I do not know
I do not know whether BlobClient::download preserves the incremental streaming Quickwit relies on for copy_to and get_slice_stream. The current code walks a Pageable of chunk responses and copies each chunk body as arrives. The new download runs internal parallel range fetches and returns one result, and I have not confirmed how to pull an incremental body stream out of BlobClientDownloadResult.
I do not know whether Azurite accepts the API version azure_storage_blob 1.0.0 sends by default.
I do not know how long the remaining port takes. The backend is 722 lines against an API I have not used before.