Skip to content

refactor(services/azblob): use OsEnv instead of StaticEnv hack#7589

Merged
Xuanwo merged 1 commit into
apache:mainfrom
YuangGao:refactor/azblob-use-osenv
May 22, 2026
Merged

refactor(services/azblob): use OsEnv instead of StaticEnv hack#7589
Xuanwo merged 1 commit into
apache:mainfrom
YuangGao:refactor/azblob-use-osenv

Conversation

@YuangGao
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

azblob's builder was injecting user-supplied account_name / account_key / sas_token into reqsign by translating them into fake env vars stuffed into a StaticEnv. The same values were also pushed into the credential chain via StaticCredentialProvider::new_shared_key / new_sas_token (see lines 393-406 in the old code), so the fake-env path was redundant — only push_front actually fired, because it has higher priority in the chain.

The StaticEnv hack also previously masked OS env vars (see #7224 root cause for azdls); the same anti-pattern in azblob has no functional benefit.

What changes are included in this PR?

Switch to the s3 pattern:

  • .with_env(StaticEnv { ... }) -> .with_env(OsEnv).
  • Drop the envs.insert("AZBLOB_ACCOUNT_NAME", ...) / AZBLOB_ACCOUNT_KEY / AZURE_STORAGE_SAS_TOKEN translation block.
  • Keep the existing StaticCredentialProvider::new_shared_key / new_sas_token push_front chain (already the correct API).
  • Keep the base64 validation for account_key.
  • Drop now-unused reqsign_core::Env / StaticEnv imports.

Are there any user-facing changes?

No behavior change for users who set credentials via opendal config

@YuangGao YuangGao marked this pull request as ready for review May 22, 2026 04:52
@YuangGao YuangGao requested a review from Xuanwo as a code owner May 22, 2026 04:52
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" labels May 22, 2026
Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 22, 2026
@Xuanwo Xuanwo merged commit fc23e6b into apache:main May 22, 2026
108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants