Repository navigation
CLO-190: persist: refresh Azure workload identity tokens from rotated file - #38178
Conversation
The azure_identity default credential chain resolves AKS workload identity via a WorkloadIdentityCredential that reads AZURE_FEDERATED_TOKEN_FILE once at construction and holds the contents for the life of the process. Kubernetes rotates that projected token, so once the last cached AAD access token expired, every refresh presented a stale client assertion and failed, permanently locking long-running processes out of persist blob storage. Add a RefreshingWorkloadIdentityCredential that re-reads the projected token file on every AAD access token refresh and caches the resulting access token until near expiry, mirroring azure_identity's token cache semantics. Use it when the workload identity environment variables are present, falling back to the default credential chain (whose remaining credential types, e.g. IMDS managed identity, refresh correctly) otherwise. The SAS-token-in-URL path is unchanged. Fixes CLO-190. Co-Authored-By: Claude Fable 5 <[email protected]>
A token provided directly via the AZURE_FEDERATED_TOKEN env var is static, so there is nothing to re-read on refresh. azure_identity's credential chain prefers it over AZURE_FEDERATED_TOKEN_FILE, so preserve that precedence when both are set. Part of CLO-190. Co-Authored-By: Claude Fable 5 <[email protected]>
Part of CLO-190. Co-Authored-By: Claude Fable 5 <[email protected]>
|
|
||
| // Refresh under the write lock so concurrent callers don't stampede | ||
| // AAD with token requests. | ||
| let mut cache = self.cache.write().await; |
There was a problem hiding this comment.
How often does get_token get called, is it once per blob store call? If yes we may want to do the refresh concurrently while the current token is still in use.
There was a problem hiding this comment.
Yes, the azure_storage authorization policy calls get_token on every storage request, so a blocking refresh stalled all in-flight requests. Took your suggestion in 51da79c: there are now two thresholds, a refresh starts 5m before expiry while the current token keeps being served (including when the refresh fails transiently), and callers only block once the token is within 20s of expiry.
| /// flight. Matches the margin in `azure_identity`'s internal token cache. | ||
| const TOKEN_EXPIRY_BUFFER: Duration = Duration::from_secs(20); | ||
|
|
||
| fn is_expired(token: &AccessToken) -> bool { |
There was a problem hiding this comment.
Let's name this needs_refresh since when this function returns true the token is not necessarily expired
There was a problem hiding this comment.
Renamed in 51da79c. With the concurrent-refresh change there are now two predicates, needs_refresh (within 5m of expiry, start refreshing) and is_usable (more than 20s from expiry, still safe to present to storage).
Address review feedback. The azure_storage authorization policy calls get_token on every storage request, so refreshing under the cache write lock stalled all in-flight requests for the duration of the AAD round trip. Split the single expiry check into two thresholds. A token within TOKEN_REFRESH_BUFFER (5m) of expiry triggers a refresh but keeps being served while the refresh is in flight, including when the refresh fails, e.g. AAD is transiently unreachable. Only within TOKEN_EXPIRY_BUFFER (20s) of expiry do callers block on the refresh. Also rename is_expired to needs_refresh accordingly. Part of CLO-190. Co-Authored-By: Claude Fable 5 <[email protected]>
| } | ||
| } | ||
|
|
||
| match self.refresh_token(&scopes_key).await { |
There was a problem hiding this comment.
If get_token is called on every blob request I think this needs to be as lightweight as possible. Doing the refresh inline will still stall the request that hits this condition. Also it looks like the tokens we issue are related to a list of scopes but we have only a single refresh mutex object so if there are multiple sets of scopes in play their refreshes will serialize.
I suggest that the make the cache hold a (ArcSwap<AccessToken>, AbortOnDropHandle) pair. The idea being that get_token reduces to something like:
match self.cache.lock().await.entry(&scopes_key) {
Entry::Occupied((token_slot, _)) => (*token_slot).clone(),
Entry::Vacant(entry) => {
let token_slot = ArcSwap::new(create_token());
let refresh_handle = tokio::spawn(refresh_task(token_slot.clone(), scopes_key)).abort_on_drop();
entry.insert((token_slot, refresh_handle));
(*token_slot).clone()
}
}This way there is an independent task for every active scope list that refreshes the contents of the corresponding token slot. get_token then only blocks the first time a slot combination is used.
There was a problem hiding this comment.
Took this design in ef9eb69: the cache now maps each scope set to a token slot plus an AbortOnDropHandle for a background task that keeps the slot fresh (fetches within 5m of expiry, retries failures every 10s while the slot serves the last good token). get_token is a map read-lock plus a slot read, and only the first use of a scope set blocks on the initial fetch. Refreshes for different scope sets no longer serialize.
One deviation from the sketch: the slot is an Arc<std::sync::RwLock> rather than ArcSwap, since arc-swap isn't in our workspace deps and an uncontended read lock is equivalent at this call rate. Happy to add the dep if you prefer ArcSwap.
Address review feedback. get_token runs on every storage request, so even an inline refresh of a still-usable token stalls the request that hits the refresh window, and a single refresh mutex serializes refreshes across scope sets. Each scope set now gets a token slot and a background task that fetches a replacement within TOKEN_REFRESH_BUFFER of expiry, retrying failures at TOKEN_REFRESH_RETRY_INTERVAL while the slot keeps serving the last good token. get_token reduces to a map lookup and a slot read, blocking only on the first use of a scope set. Dropping the credential or calling clear_cache aborts the tasks. Part of CLO-190. Co-Authored-By: Claude Fable 5 <[email protected]>
Adds the `## v26.38.1` section to the releases index. v26.38.1 is a single-fix hotfix train with no RC snapshots, so this one commit carries the whole release note: the Azure workload identity token refresh fix (#38178). No `Released to` lines: v26.38.1 is not being released, and the fix ships to customers in v26.38.2. No row is added to the Self-Managed operator compatibility table. Every row in that file carries a real release date, and a patch tag customers do not receive is absent from it entirely - v26.28.1, v26.29.1, v26.31.1, v26.33.1 and v26.33.2 all have no row. A row can be added by the usual `docs: Bump self-managed to ...` route once a version actually ships. Co-Authored-By: Claude Opus 5 <[email protected]>
Fixes CLO-190.
Seems to work on an Azure env I spun up.
Motivation
A customer environment on AKS lost persist auth into Azure Blob Storage after running for a while. Root cause:
azure_identity0.21's default credential chain resolves AKS workload identity viaWorkloadIdentityCredential, which reads the projected service account token fromAZURE_FEDERATED_TOKEN_FILEonce at construction and holds the contents for the life of the process. Kubernetes rotates that projected token, so once the last cached AAD access token expired, every refresh presented a stale client assertion to AAD and failed, permanently locking long-running processes out of persist blob storage.Changes
RefreshingWorkloadIdentityCredentialinsrc/persist/src/azure.rsthat re-reads the projected token file on every AAD access token refresh and exchanges it via the publicazure_identity::federated_credentials_flow::perform. Resulting access tokens are cached per scope until near expiry, mirroringazure_identity's internal token cache semantics (including its 20s expiry buffer), with refresh under a write lock to avoid stampeding AAD.AzureBlobConfig::newwhenAZURE_TENANT_ID,AZURE_CLIENT_ID, andAZURE_FEDERATED_TOKEN_FILEare all present (andAZURE_FEDERATED_TOKENis not, that static-token path keeps its upstream precedence). Otherwise fall back tocreate_default_credential(), whose remaining credential types (e.g. IMDS managed identity) refresh correctly.clear_cache, without contacting AAD (the exchange is injected).Out of scope
Migrating off the archived unofficial
azure_storage/azure_identity0.21 crates to the official Azure SDK for Rust is the long-term fix for this class of issue, but is a much larger change and not needed to resolve this incident class.Tips for reviewer
Compare with the vendored
azure_identity0.21 sources:WorkloadIdentityCredential::createreads the token file once (token_credentials/workload_identity_credentials.rs), and it enters the default chain viaEnvironmentCredential(token_credentials/environment_credentials.rs).Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.🤖 Generated with Claude Code