Fix airflow users reset-password in FAB CLI (#63830)#64375
Fix airflow users reset-password in FAB CLI (#63830)#64375deepujain wants to merge 1 commit intoapache:v2-11-testfrom
Conversation
|
Pushed the fix for the FAB reset-password regression. The change keeps lookup and password reset in the same appbuilder context, and the targeted FAB reset-password tests plus ruff checks all passed locally. |
airflow users reset-password in FAB CLI (#63830)
kalluripradeep
left a comment
There was a problem hiding this comment.
Fix looks correct — keeping the lookup and reset in the same app context makes sense. Just noticed the celery integration test is failing, worth a quick check to confirm it's unrelated before merging.
a6bbe4f to
a382771
Compare
|
Validation update after rebasing this PR on
|
potiuk
left a comment
There was a problem hiding this comment.
This PR should be done with v2-11-test as target. You can test it there using breeze uv tool install -e ./dev/breeze --force followed by breeze start-airflow in that branch. Both airflow 2.11 and fab provider that support airflow 2.11.2 are built from that branch.
Please re-do it.
|
Replying to the 3.x question: I do not have a separate user reproduction on a released 3.x build, but the broken code path is present on current So this is partly observed and partly inference:
That is why I targeted |
a382771 to
bf52275
Compare
|
Redid this on What changed in the redo:
Validation note:
So the PR is now redone on the correct target branch |
Summary
airflow users reset-passwordin Airflow 2.11.2 can fail because the command looks up the user in one Flask app context and then resets the password in a different one. This change keeps the lookup and password reset inside the same application builder context so the security manager uses a fully initialized SQLAlchemy extension.Changes
providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/user_command.py-- refactored user lookup souser_reset_password()resolves the user and performs the password reset inside a singleget_application_builder()context.providers/fab/tests/unit/fab/auth_manager/cli_commands/test_user_command.py-- added a regression test that verifiesuser_reset_password()only opens one application builder context while resetting the password.Test plan
uv run --project /Users/dejain/nvidia/oss/airflow-63830/providers/fab pytest /Users/dejain/nvidia/oss/airflow-63830/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_user_command.py -k 'reset_user_password' -xvsruff check /Users/dejain/nvidia/oss/airflow-63830/providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/user_command.py /Users/dejain/nvidia/oss/airflow-63830/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_user_command.pyruff format --check /Users/dejain/nvidia/oss/airflow-63830/providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/user_command.py /Users/dejain/nvidia/oss/airflow-63830/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_user_command.pyEvidence it works
3 passed, 22 deselected.user_reset_password()uses a single application builder context, which matches the root cause described in the issue discussion.Fixes #63830