Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option in auth manager interface to define FastAPI api #45009

Merged
merged 11 commits into from
Dec 20, 2024

Conversation

vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Dec 17, 2024

Resolves #44882 and #44847.

Today there is an option to extend the Flask Rest API in the auth manager interface. That allows, for instance, the FAB auth manager to extend the Rest API and define users, roles and permissions related APIs.

In Airflow 3, we are moving away from Flask and use FastApi as engine for APIs. As such, the auth manager interface should have an option to extend this API using FastAPI (instead of Flask).

In FAB auth manager, instead of converting all the APIs defined with Flask to FastAPI, we use WSGIMiddleware to embed a minimal Flask application that host the APIs.

With this PR: GET http://localhost:29091/auth/auth/fab/v1/roles return the list of roles. The duplicate auth/auth in the API path will be removed when the legacy Airflow 2 is gone.


^ 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 newsfragments.

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 19 changed files in this pull request and generated no comments.

Files not reviewed (14)
  • providers/src/airflow/providers/fab/auth_manager/security_manager/override.py: Evaluated as low risk
  • airflow/www/security_manager.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/auth_manager/api_endpoints/role_and_permission_endpoint.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/auth_manager/api_endpoints/user_endpoint.py: Evaluated as low risk
  • tests/auth/managers/test_base_auth_manager.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/auth_manager/decorators/auth.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/auth_manager/api/auth/backend/kerberos_auth.py: Evaluated as low risk
  • airflow/www/auth.py: Evaluated as low risk
  • airflow/www/decorators.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/www/extensions/init_appbuilder.py: Evaluated as low risk
  • airflow/api_connexion/security.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/www/extensions/init_jinja_globals.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/auth_manager/api/auth/backend/basic_auth.py: Evaluated as low risk
  • providers/src/airflow/providers/fab/www/app.py: Evaluated as low risk
@vincbeck vincbeck added the legacy api Whether legacy API changes should be allowed in PR label Dec 17, 2024
@vincbeck vincbeck closed this Dec 17, 2024
@vincbeck vincbeck reopened this Dec 17, 2024
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch from 164db0e to 7a0e355 Compare December 17, 2024 21:20
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch 5 times, most recently from 6bb131c to 1c029e5 Compare December 18, 2024 21:31
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch from 1c029e5 to 159afc4 Compare December 18, 2024 21:34
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch from 159afc4 to 432979c Compare December 18, 2024 22:14
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch from e4b4ae9 to edbf3f3 Compare December 19, 2024 15:37
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch 5 times, most recently from bdc611c to cd67389 Compare December 20, 2024 16:07
@vincbeck vincbeck force-pushed the vincbeck/auth_manager_router branch from cd67389 to 59c9447 Compare December 20, 2024 16:38
@vincbeck
Copy link
Contributor Author

Tests are passing 🥳

@vincbeck
Copy link
Contributor Author

Let me know if you have more questions/concerns @jedcunningham

Copy link
Member

@jedcunningham jedcunningham left a comment

Choose a reason for hiding this comment

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

Can you also add something into the amazon provider changelog to indicate that the aws auth manager is no longer compatible with af2?

tests_common/test_utils/db.py Outdated Show resolved Hide resolved
Copy link
Member

@jedcunningham jedcunningham left a comment

Choose a reason for hiding this comment

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

Good to go once we get an entry on the provider changelog. Thanks @vincbeck!

@vincbeck vincbeck merged commit a1db3ee into apache:main Dec 20, 2024
96 checks passed
@vincbeck vincbeck deleted the vincbeck/auth_manager_router branch December 20, 2024 23:19
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
agupta01 pushed a commit to agupta01/airflow that referenced this pull request Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:providers area:webserver Webserver related Issues legacy api Whether legacy API changes should be allowed in PR provider:fab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Register extra router from the authmanager into the FastAPI application
2 participants