-
Notifications
You must be signed in to change notification settings - Fork 16.3k
AIP-84 Add Auth for backfill #47482
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
AIP-84 Add Auth for backfill #47482
Conversation
|
Let me know when this is ready for review. |
@vatsrahul1001 has mark it for ready to review. I'll also take a look later today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rebase the branch and regenerate the openapi spec.
Some endpoints are missing the permissions (list_backfills, create_backfill, get_backfill etc.)
Also I think we might want an extra Entity for the AuthManager instead of going through dagrun every time. (That might be good for creating a backfill, to have both backfill + create dag run permission maybe, but for listing backfills, we need a dedicated entity I guess). You can take a look at this PR that also does that for AssetAliases:
#47241
|
cc: @vincbeck |
3ceaf3d to
c40a5ef
Compare
I agree. I think #47241 is a great example, please follow it and tag me on the PR. I'll be happy to review it |
c40a5ef to
7151649
Compare
providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
Show resolved
Hide resolved
providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py
Show resolved
Hide resolved
providers/amazon/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py
Show resolved
Hide resolved
27c07f7 to
8784fe4
Compare
2ccf6c0 to
dce6fd5
Compare
dce6fd5 to
de4ccdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I'll let vincent double check.
providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
Outdated
Show resolved
Hide resolved
de4ccdf to
0119c4f
Compare
* add auth to backfills endpoints * feat(security): add is_authorized_backfill * feat(api_fastapi): add required permision for backfills * feat(AIP-84): add backfill operation permission to aws provider * test(AIP-84): add test cases for backfill auth * fixup! Merge branch 'main' into AIP84-auth-backfills * fixup! Merge branch 'main' into AIP84-auth-backfills * fixup! fixup! Merge branch 'main' into AIP84-auth-backfills * fixup! Merge branch 'main' into AIP84-auth-backfills * fixup! Merge branch 'main' into AIP84-auth-backfills --------- Co-authored-by: Wei Lee <weilee.rx@gmail.com>
related to #42360
^ 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.rstor{issue_number}.significant.rst, in newsfragments.