You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org> on 2026/02/01 08:27:57 UTC
[PR] Make auth manager initialization thread-safe [airflow]
Alok-kumar-priyadarshi opened a new pull request, #61310:
URL: https://github.com/apache/airflow/pull/61310
Summary
This PR fixes a race condition in FastAPI auth manager initialization that could cause intermittent 500 Internal Server Error responses under concurrent authentication requests.
Fix
Make auth manager creation thread-safe
Ensure the manager is initialized exactly once
Prevent concurrent requests from accessing an uninitialized instance
Notes
Internal change only
No user-facing or configuration changes
Verified with concurrent /auth/token requests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3833977966
Added a focused unit test covering concurrent auth manager initialization and singleton behavior.
This ensures thread safety and guards against regression. Thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3850874389
Please let me know if the failures are related to my changes — I’m happy to investigate and fix them if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3899310089
@jason810496
I’ve rebased on the latest `main`, resolved the CI issues, and all checks are now passing.
Could you please take another look when you have time? Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 closed pull request #61310: Make auth manager initialization thread-safe
URL: https://github.com/apache/airflow/pull/61310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3876261560
Rebased on latest main and re-applied the thread-safe auth manager fix.
The unrelated ERD changes have been removed.
@jason810496 could you please take another look when you have a moment? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2786526112
##########
scripts/ci/prek/common_prek_utils.py:
##########
@@ -401,7 +401,8 @@ def inner():
When only_top_level = False then returns
['os', 'collections.defaultdict', 'numpy', 'pandas.DataFrame', 'json', 'pathlib.Path', 'pathlib.PurePath']
"""
- root = ast.parse(file_path.read_text(), file_path.name)
+ root = ast.parse(file_path.read_text(encoding="utf-8"), file_path.name)
Review Comment:
Good question.
I added encoding="utf-8" to avoid relying on the system’s default encoding, which can differ across environments (for example, Windows vs Linux in CI). In some cases, the default encoding isn’t UTF-8, and that can lead to unexpected parsing issues if a file contains non-ASCII characters.
By specifying UTF-8 explicitly, we make the behavior consistent and predictable across platforms and CI environments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3917049484
Hi,
I have updated the PR to:
Make AuthManager initialization thread-safe using a guarded instance check with RLock.
Fix unintended dev-mode activation in api_server_command by replacing should_enable_hot_reload() with an explicit args.dev check (this was affecting uvicorn invocation arguments in tests).
All relevant unit tests pass locally.
CI is currently awaiting approval to run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2751276702
##########
airflow-core/src/airflow/api_fastapi/app.py:
##########
@@ -57,6 +58,7 @@
class _AuthManagerState:
instance: BaseAuthManager | None = None
+ lock = threading.RLock()
Review Comment:
How about exporting a method for getting the instance and ensure the thread safety within the method context.
```suggestion
lock = threading.RLock()
@classmethod
def get_instance(cls) -> BaseAuthManager:
if cls._instance is None:
with cls._lock:
if cls._instance is None:
auth_manager_cls = get_auth_manager_cls()
cls._instance = auth_manager_cls()
return cls._instance
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3831331996
Thanks for the review!
I've cleaned up the implementation and pushed an updated commit focusing solely on the thread-safe auth manager initialization.
Happy to add targeted tests if you'd like me to include them in a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2789063002
##########
airflow-core/src/airflow/api_fastapi/app.py:
##########
@@ -137,8 +139,11 @@ def get_auth_manager_cls() -> type[BaseAuthManager]:
def create_auth_manager() -> BaseAuthManager:
"""Create the auth manager."""
- auth_manager_cls = get_auth_manager_cls()
- _AuthManagerState.instance = auth_manager_cls()
+ if _AuthManagerState.instance is None:
+ with _AuthManagerState.lock:
+ if _AuthManagerState.instance is None:
Review Comment:
Those checks are reundant. We have `if _AuthManagerState.instance is None:` two times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3855991634
Conflicts have been resolved against the latest main.
While setting up the local development environment, running Docker/Breeze consumed a significant amount of disk space (≈40 GB+), which exceeded my local resource limits. Because of this, I wasn’t able to complete the full test suite locally.
I’ve relied on the project CI for validation and will promptly address any CI failures or reviewer feedback.
Please let me know if any additional verification is required from my side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2771340384
##########
airflow-core/docs/img/airflow_erd.sha256:
##########
Review Comment:
Why is the ERDiagram changed? The PR changes look unrelated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3901854427
@jason810496,
I’ve rebased onto the latest main and all CI checks are now passing.
Could you please take another look when you have time?
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3842095288
Tests are failing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2789257453
##########
airflow-core/src/airflow/api_fastapi/app.py:
##########
@@ -137,8 +139,11 @@ def get_auth_manager_cls() -> type[BaseAuthManager]:
def create_auth_manager() -> BaseAuthManager:
"""Create the auth manager."""
- auth_manager_cls = get_auth_manager_cls()
- _AuthManagerState.instance = auth_manager_cls()
+ if _AuthManagerState.instance is None:
+ with _AuthManagerState.lock:
+ if _AuthManagerState.instance is None:
Review Comment:
I indent to add two `if _AuthManagerState.instance is None` check.
The outer check is optimization to avoid taking lock on every call after initialization.
If we have 2 parallel threads that call `create_auth_manager` at the same time, the inner check will prevent the second thread from re-initializing once the first thread has created the instance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3897317211
Can you rebase? The CI was broken yesterday
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3930540934
There are plenty of issues in the tests and lots of discussions that take a lot of time. I suggest you open a new PR which actually has green PR and solves the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on PR #61310:
URL: https://github.com/apache/airflow/pull/61310#issuecomment-3837144543
Thanks for the guidance ! @vincbeck
I’ve moved the thread-safety test into test_app.py to keep a 1:1 mapping with the implementation and removed the standalone test file.
Please let me know if you’d like any additional coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2771336867
##########
airflow-core/src/airflow/api_fastapi/app.py:
##########
@@ -70,16 +68,70 @@ async def lifespan(app: FastAPI):
app.state.lifespan_called = True
yield
+def get_auth_manager_cls() -> type[BaseAuthManager]:
+ auth_manager_cls = conf.getimport(section="core", key="auth_manager")
+
+ if not auth_manager_cls:
+ raise AirflowConfigException(
+ "No auth manager defined in the config. Please specify one using section/key [core/auth_manager]."
+ )
+
+ return auth_manager_cls
+
+
+def _ensure_auth_lock(app: FastAPI) -> None:
+ if not hasattr(app.state, "auth_manager_lock"):
+ app.state.auth_manager_lock = threading.RLock()
+
+
+def _create_auth_manager_if_missing(app: FastAPI) -> BaseAuthManager:
+ _ensure_auth_lock(app)
+
+ with app.state.auth_manager_lock:
+ if not hasattr(app.state, "auth_manager"):
+ auth_manager_cls = get_auth_manager_cls()
+ app.state.auth_manager = auth_manager_cls()
+ app.state.auth_manager_initialized = False
+
+ return app.state.auth_manager
+
+
+def init_auth_manager(app: FastAPI | None = None) -> BaseAuthManager:
+ """
+ Initialize and attach the auth manager to FastAPI app.
+ Thread-safe and app-lifecycle scoped.
+ """
+ if app is None:
+ raise RuntimeError("init_auth_manager requires FastAPI app instance")
+
+ am = _create_auth_manager_if_missing(app)
+
+ with app.state.auth_manager_lock:
+ if not app.state.auth_manager_initialized:
+ am.init()
+ app.state.auth_manager_initialized = True
+
+ auth_manager_fastapi_app = am.get_fastapi_app()
+ if auth_manager_fastapi_app:
+ app.mount("/auth", auth_manager_fastapi_app)
+
+ return am
+
+
+def get_auth_manager(app: FastAPI) -> BaseAuthManager:
+ """
+ Return app-scoped auth manager.
+ """
+ return _create_auth_manager_if_missing(app)
+
@providers_configuration_loaded
def create_app(apps: str = "all") -> FastAPI:
apps_list = apps.split(",") if apps else ["all"]
app = FastAPI(
title="Airflow API",
- description="Airflow API. All endpoints located under ``/api/v2`` can be used safely, are stable and backward compatible. "
- "Endpoints located under ``/ui`` are dedicated to the UI and are subject to breaking change "
- "depending on the need of the frontend. Users should not rely on those but use the public ones instead.",
+ description="Airflow API...",
Review Comment:
Why dio you change the descriptions in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "Alok-kumar-priyadarshi (via GitHub)" <gi...@apache.org>.
Alok-kumar-priyadarshi commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2751295984
##########
airflow-core/src/airflow/api_fastapi/app.py:
##########
@@ -57,6 +58,7 @@
class _AuthManagerState:
instance: BaseAuthManager | None = None
+ lock = threading.RLock()
Review Comment:
Thanks for the review and the suggestion!
That makes sense — I’ll refactor this to centralize the thread-safe initialization behind a single accessor and add a unit test covering concurrent auth/token requests.
I’ll push an update shortly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2755050125
##########
airflow-core/tests/unit/api_fastapi/test_auth_manager_thread_safety.py:
##########
@@ -0,0 +1,34 @@
+import threading
Review Comment:
We usually try to have 1:1 mapping implementation and tests. Can you please move this test in `test_app.py`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2822356369
##########
airflow-core/src/airflow/cli/commands/api_server_command.py:
##########
@@ -233,7 +233,7 @@ def api_server(args: Namespace):
get_signing_args()
- if cli_utils.should_enable_hot_reload(args):
+ if getattr(args, "dev", False):
Review Comment:
Why change that? It seems unrelated to the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #61310: Make auth manager initialization thread-safe
URL: https://github.com/apache/airflow/pull/61310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Make auth manager initialization thread-safe [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #61310:
URL: https://github.com/apache/airflow/pull/61310#discussion_r2774674799
##########
scripts/ci/prek/common_prek_utils.py:
##########
@@ -401,7 +401,8 @@ def inner():
When only_top_level = False then returns
['os', 'collections.defaultdict', 'numpy', 'pandas.DataFrame', 'json', 'pathlib.Path', 'pathlib.PurePath']
"""
- root = ast.parse(file_path.read_text(), file_path.name)
+ root = ast.parse(file_path.read_text(encoding="utf-8"), file_path.name)
Review Comment:
Why is that needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org