Skip to content

Conversation

@WangzJi
Copy link
Contributor

@WangzJi WangzJi commented Jun 21, 2025

Fix Azure Blob Storage authentication to prioritize account_key over DefaultAzureCredential

Problem Description

The WasbHook and WasbAsyncHook had an authentication priority issue where the account_key field in connection extras was being skipped when no password was provided. The hooks would directly fall back to DefaultAzureCredential without checking for account_key, causing authentication failures for users who configured their connections with account keys in the extra fields.

Solution

  • Modified the authentication flow in both WasbHook.get_conn() and WasbAsyncHook.get_async_conn() to check for account_key in connection extras before falling back to DefaultAzureCredential
  • Ensured proper authentication precedence following the documented behavior
  • Added comprehensive test coverage for the account_key authentication method

Tests

  • Added test_account_key_connection test case to verify account_key authentication works correctly
  • Updated existing test parameterization to include the new connection type
  • All existing tests continue to pass, ensuring backward compatibility

Changes

  • hooks/wasb.py: Updated authentication logic in both sync and async hooks
  • tests/test_wasb.py: Added test cases and connection setup for account_key authentication
  • newsfragments/51944.bugfix.rst: Added changelog entry for the fix

Verification

This fix ensures that users can successfully authenticate using account_key in connection extras as documented in the hook's class docstring: "These parameters have to be passed in Airflow Data Base: account_name and account_key."

closes: #51944


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg
Copy link

boring-cyborg bot commented Jun 21, 2025

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

- Check account_key in connection extra before falling back to DefaultAzureCredential
- Ensure proper authentication precedence for WasbHook and WasbAsyncHook
- Add test case for account_key authentication
- Update documentation to clarify account_key authentication method and priority
- Update hook docstring and UI placeholders to reflect account_key support
- Fixes apache#51944
@potiuk potiuk merged commit 1d4c1ae into apache:main Jun 21, 2025
70 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 21, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Azure BlobStorage for logging

2 participants