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.

Sign up for GitHub

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

Jump to bottom

Let auth managers provide their own API endpoints #34349

Merged
merged 25 commits into from
Oct 17, 2023
Merged

Let auth managers provide their own API endpoints #34349

merged 25 commits into from
Oct 17, 2023

Conversation

vandonr-amz
Copy link
Contributor

closes #32207

  • offer a method for auth managers to provide their own API in the form of a Flask Blueprint that is added to the main app
  • "move" (actually duplicate, for back-compat reasons) user & role related endpoints to the blueprint provided by FAB auth manager

I marked the existing endpoints as deprecated, but it only appears in the doc I think. IDK how to get the message across to current users so that they change the route they use, if we ever want to remove the user & role endpoints from the "main" API.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:webserver Webserver related Issues labels Sep 13, 2023
@vincbeck vincbeck added the AIP-56 Extensible user management label Sep 13, 2023
vincbeck
vincbeck reviewed Sep 13, 2023
View reviewed changes
airflow/auth/managers/base_auth_manager.py Outdated Show resolved Hide resolved
airflow/www/extensions/init_views.py Outdated Show resolved Hide resolved
vandonr-amz and others added 3 commits September 13, 2023 12:53
@vandonr-amz
test fixes
f67eac3 
 - fix paths
 - add error handling to all existing endpoints
@vandonr-amz
apply suggestion
32e67aa
@vandonr-amz @vincbeck
docstring change
8a8c738 
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
jedcunningham
jedcunningham reviewed Sep 14, 2023
View reviewed changes
airflow/auth/managers/fab/fab_auth_manager.py Outdated Show resolved Hide resolved
airflow/auth/managers/base_auth_manager.py Outdated Show resolved Hide resolved
airflow/api_connexion/openapi/v1.yaml Outdated Show resolved Hide resolved
@vandonr-amz @vincbeck
a bit more detail in http error
ec46600 
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
vincbeck
vincbeck approved these changes Sep 25, 2023
View reviewed changes
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

LGTM. Some static failures though

@vincbeck vincbeck requested a review from uranusjr October 4, 2023 17:51
jedcunningham
jedcunningham reviewed Oct 6, 2023
View reviewed changes
airflow/api_connexion/endpoints/forward_to_fab_endpoint.py Outdated Show resolved Hide resolved
airflow/auth/managers/base_auth_manager.py Outdated Show resolved Hide resolved
airflow/auth/managers/fab/openapi/v1.yaml Outdated Show resolved Hide resolved
airflow/www/extensions/init_views.py Show resolved Hide resolved
jedcunningham
jedcunningham reviewed Oct 17, 2023
View reviewed changes
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.

I'd actually rather we let the auth manager provide an arbitrary blueprint so we don't force the use of connexion, but I won't stand in the way on that change.

@vincbeck vincbeck merged commit 0201b1c into apache:main Oct 17, 2023
66 checks passed
@vincbeck vincbeck deleted the vandonr/fab branch October 17, 2023 22:10
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Oct 27, 2023
@potiuk potiuk mentioned this pull request Nov 4, 2023
Open
1 task
@ephraimbuddy ephraimbuddy added this to the Airflow 2.8.0 milestone Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincbeck vincbeck vincbeck approved these changes

@potiuk potiuk potiuk approved these changes

@jedcunningham jedcunningham jedcunningham approved these changes

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton ryanahamilton is a code owner

@ashb ashb Awaiting requested review from ashb ashb is a code owner

@bbovenzi bbovenzi Awaiting requested review from bbovenzi bbovenzi is a code owner

@pierrejeambrun pierrejeambrun Awaiting requested review from pierrejeambrun pierrejeambrun is a code owner

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

@uranusjr uranusjr Awaiting requested review from uranusjr

Assignees
No one assigned
Labels
AIP-56 Extensible user management area:API Airflow's REST/HTTP API area:webserver Webserver related Issues changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Milestone
Airflow 2.8.0
Development

Successfully merging this pull request may close these issues.

AIP-56 - FAB AM - Move User and Role Rest APIs to FAB AM
6 participants
@vandonr-amz @potiuk @uranusjr @jedcunningham @vincbeck @ephraimbuddy