This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 144b0816760 Migrate FAB PATCH /roles/{name} to FastAPI (#58023)
144b0816760 is described below
commit 144b0816760d4010ee72120e2cb9c2369ea6ba1f
Author: kyounghoonJang <[email protected]>
AuthorDate: Mon Nov 17 07:20:10 2025 +0900
Migrate FAB PATCH /roles/{name} to FastAPI (#58023)
---
.../openapi/v2-fab-auth-manager-generated.yaml | 70 ++++++++++
.../fab/auth_manager/api_fastapi/routes/roles.py | 22 ++++
.../fab/auth_manager/api_fastapi/services/roles.py | 36 ++++++
.../auth_manager/api_fastapi/routes/test_roles.py | 142 ++++++++++++++++++++-
.../api_fastapi/services/test_roles.py | 113 +++++++++++++++-
5 files changed, 380 insertions(+), 3 deletions(-)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml
index f3f7fbfe90b..06403b34cfa 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml
@@ -324,6 +324,76 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HTTPValidationError'
+ patch:
+ tags:
+ - FabAuthManager
+ summary: Patch Role
+ description: Update an existing role.
+ operationId: patch_role
+ security:
+ - OAuth2PasswordBearer: []
+ - HTTPBearer: []
+ parameters:
+ - name: name
+ in: path
+ required: true
+ schema:
+ type: string
+ minLength: 1
+ title: Name
+ - name: update_mask
+ in: query
+ required: false
+ schema:
+ anyOf:
+ - type: string
+ - type: 'null'
+ description: Comma-separated list of fields to update
+ title: Update Mask
+ description: Comma-separated list of fields to update
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/RoleBody'
+ responses:
+ '200':
+ description: Successful Response
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/RoleResponse'
+ '400':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Bad Request
+ '401':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Unauthorized
+ '403':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Forbidden
+ '404':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Not Found
+ '422':
+ description: Validation Error
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPValidationError'
components:
schemas:
ActionResourceResponse:
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py
index 97eb1984758..2488adbc33d 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py
@@ -113,3 +113,25 @@ def get_role(name: str = Path(..., min_length=1)) ->
RoleResponse:
"""Get an existing role."""
with get_application_builder():
return FABAuthManagerRoles.get_role(name=name)
+
+
+@roles_router.patch(
+ "/roles/{name}",
+ responses=create_openapi_http_exception_doc(
+ [
+ status.HTTP_400_BAD_REQUEST,
+ status.HTTP_401_UNAUTHORIZED,
+ status.HTTP_403_FORBIDDEN,
+ status.HTTP_404_NOT_FOUND,
+ ]
+ ),
+ dependencies=[Depends(requires_fab_custom_view("PATCH",
permissions.RESOURCE_ROLE))],
+)
+def patch_role(
+ body: RoleBody,
+ name: str = Path(..., min_length=1),
+ update_mask: str | None = Query(None, description="Comma-separated list of
fields to update"),
+) -> RoleResponse:
+ """Update an existing role."""
+ with get_application_builder():
+ return FABAuthManagerRoles.patch_role(name=name, body=body,
update_mask=update_mask)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
index 68d8b85d254..03bfb2e6b9b 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py
@@ -120,3 +120,39 @@ class FABAuthManagerRoles:
detail=f"Role with name {name!r} does not exist.",
)
return RoleResponse.model_validate(existing)
+
+ @classmethod
+ def patch_role(cls, body: RoleBody, name: str, update_mask: str | None =
None) -> RoleResponse:
+ security_manager = get_fab_auth_manager().security_manager
+
+ existing = security_manager.find_role(name=name)
+ if not existing:
+ raise HTTPException(
+ status_code=status.HTTP_404_NOT_FOUND,
+ detail=f"Role with name {name!r} does not exist.",
+ )
+
+ if update_mask:
+ update_data = RoleResponse.model_validate(existing)
+
+ for field in update_mask:
+ if field == "actions":
+ update_data.permissions = body.permissions
+ elif hasattr(body, field):
+ setattr(update_data, field, getattr(body, field))
+ else:
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail=f"'{field}' in update_mask is unknown",
+ )
+ else:
+ update_data = RoleResponse(name=body.name,
permissions=body.permissions or [])
+
+ perms: list[tuple[str, str]] = [(ar.action.name, ar.resource.name) for
ar in (body.permissions or [])]
+ cls._check_action_and_resource(security_manager, perms)
+ security_manager.bulk_sync_roles([{"role": name, "perms": perms}])
+
+ new_name = update_data.name
+ if new_name and new_name != existing.name:
+ security_manager.update_role(role_id=existing.id, name=new_name)
+ return RoleResponse.model_validate(update_data)
diff --git
a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py
b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py
index f15e715ccd5..0d1e3b80842 100644
--- a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py
+++ b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py
@@ -18,7 +18,7 @@
from __future__ import annotations
from contextlib import nullcontext as _noop_cm
-from unittest.mock import MagicMock, patch
+from unittest.mock import ANY, MagicMock, patch
import pytest
from fastapi import HTTPException, status
@@ -412,3 +412,143 @@ class TestRoles:
resp = test_client.get("/fab/v1/roles/")
assert resp.status_code == 404
mock_roles.get_role.assert_not_called()
+
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles")
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager")
+ @patch(
+
"airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder",
+ return_value=_noop_cm(),
+ )
+ def test_patch_role(
+ self, mock_get_application_builder, mock_get_auth_manager, mock_roles,
test_client, as_user
+ ):
+ mgr = MagicMock()
+ mgr.is_authorized_custom_view.return_value = True
+ mock_get_auth_manager.return_value = mgr
+
+ dummy_out = RoleResponse(name="roleA", permissions=[])
+ mock_roles.patch_role.return_value = dummy_out
+
+ with as_user():
+ resp = test_client.patch("/fab/v1/roles/roleA", json={"name":
"roleA", "actions": []})
+ assert resp.status_code == 200
+ assert resp.json() == dummy_out.model_dump(by_alias=True)
+ mock_roles.patch_role.assert_called_once_with(name="roleA",
body=ANY, update_mask=None)
+
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles")
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager")
+ @patch(
+
"airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder",
+ return_value=_noop_cm(),
+ )
+ def test_patch_role_with_update_mask(
+ self, mock_get_application_builder, mock_get_auth_manager, mock_roles,
test_client, as_user
+ ):
+ mgr = MagicMock()
+ mgr.is_authorized_custom_view.return_value = True
+ mock_get_auth_manager.return_value = mgr
+
+ dummy_out = RoleResponse(name="roleA", permissions=[])
+ mock_roles.patch_role.return_value = dummy_out
+
+ with as_user():
+ resp = test_client.patch(
+ "/fab/v1/roles/roleA",
+ json={"name": "roleA", "actions": []},
+ params={"update_mask": "name,actions"},
+ )
+ assert resp.status_code == 200
+ assert resp.json() == dummy_out.model_dump(by_alias=True)
+ mock_roles.patch_role.assert_called_once_with(name="roleA",
body=ANY, update_mask="name,actions")
+
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles")
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager")
+ @patch(
+
"airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder",
+ return_value=_noop_cm(),
+ )
+ def test_path_role_forbidden(
+ self, mock_get_application_builder, mock_get_auth_manager, mock_roles,
test_client, as_user
+ ):
+ mgr = MagicMock()
+ mgr.is_authorized_custom_view.return_value = False
+ mock_get_auth_manager.return_value = mgr
+
+ with as_user():
+ resp = test_client.patch("/fab/v1/roles/roleA", json={"name":
"roleA", "actions": []})
+ assert resp.status_code == 403
+ mock_roles.patch_role.assert_not_called()
+
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles")
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager")
+ @patch(
+
"airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder",
+ return_value=_noop_cm(),
+ )
+ def test_patch_role_validation_404_not_found(
+ self, mock_get_application_builder, mock_get_auth_manager, mock_roles,
test_client, as_user
+ ):
+ mgr = MagicMock()
+ mgr.is_authorized_custom_view.return_value = True
+ mock_get_auth_manager.return_value = mgr
+ mock_roles.patch_role.side_effect = HTTPException(
+ status_code=status.HTTP_404_NOT_FOUND,
+ detail="Role with name 'non_existent_role' does not exist.",
+ )
+
+ with as_user():
+ resp = test_client.patch(
+ "/fab/v1/roles/non_existent_role", json={"name":
"non_existent_role", "actions": []}
+ )
+ assert resp.status_code == 404
+ mock_roles.patch_role.assert_called_once_with(
+ name="non_existent_role", body=ANY, update_mask=None
+ )
+
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles")
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager")
+ @patch(
+
"airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder",
+ return_value=_noop_cm(),
+ )
+ def test_patch_role_validation_404_empty_name(
+ self, mock_get_application_builder, mock_get_auth_manager, mock_roles,
test_client, as_user
+ ):
+ mgr = MagicMock()
+ mgr.is_authorized_custom_view.return_value = True
+ mock_get_auth_manager.return_value = mgr
+ mock_roles.patch_role.side_effect = HTTPException(
+ status_code=status.HTTP_404_NOT_FOUND,
+ detail="Role with name 'non_existent_role' does not exist.",
+ )
+
+ with as_user():
+ resp = test_client.patch("/fab/v1/roles/", json={"name":
"non_existent_role", "actions": []})
+ assert resp.status_code == 404
+
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles")
+
@patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager")
+ @patch(
+
"airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder",
+ return_value=_noop_cm(),
+ )
+ def test_path_role_unknown_update_mask(
+ self, mock_get_application_builder, mock_get_auth_manager, mock_roles,
test_client, as_user
+ ):
+ mgr = MagicMock()
+ mgr.is_authorized_custom_view.return_value = True
+ mock_get_auth_manager.return_value = mgr
+
+ mock_roles.patch_role.side_effect = HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="unknown_field in update_mask is unknown",
+ )
+
+ with as_user():
+ resp = test_client.patch(
+ "/fab/v1/roles/roleA",
+ json={"name": "roleA", "actions": []},
+ params={"update_mask": "unknown_field"},
+ )
+ assert resp.status_code == 400
+ mock_roles.patch_role.assert_called_once_with(name="roleA",
body=ANY, update_mask="unknown_field")
diff --git
a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
index 1c9aada09ec..c46acbaef63 100644
---
a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
+++
b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py
@@ -35,7 +35,7 @@ def fab_auth_manager():
@pytest.fixture
def security_manager():
sm = MagicMock()
- sm.get_action.side_effect = lambda n: object() if n in {"can_read"} else
None
+ sm.get_action.side_effect = lambda n: object() if n in {"can_read",
"can_edit"} else None
sm.get_resource.side_effect = lambda n: object() if n in {"DAG"} else None
return sm
@@ -48,7 +48,7 @@ def _make_role_obj(name: str, perms: list[tuple[str, str]]):
)
for (a, r) in perms
]
- return types.SimpleNamespace(name=name, permissions=perm_objs)
+ return types.SimpleNamespace(id=1, name=name, permissions=perm_objs)
class _FakeScalarCount:
@@ -252,3 +252,112 @@ class TestRolesService:
with pytest.raises(HTTPException) as ex:
FABAuthManagerRoles.get_role(name="roleA")
assert ex.value.status_code == 404
+
+ # PATCH /roles/{name}
+
+ def test_patch_role_success(self, get_fab_auth_manager, fab_auth_manager,
security_manager):
+ role = _make_role_obj("viewer", [("can_read", "DAG")])
+ security_manager.find_role.return_value = role
+ fab_auth_manager.security_manager = security_manager
+ get_fab_auth_manager.return_value = fab_auth_manager
+ body = types.SimpleNamespace(
+ name="viewer",
+ permissions=[
+ types.SimpleNamespace(
+ action=types.SimpleNamespace(name="can_edit"),
+ resource=types.SimpleNamespace(name="DAG"),
+ )
+ ],
+ )
+ out = FABAuthManagerRoles.patch_role(body=body, name="viewer")
+ assert out.name == "viewer"
+ assert out.permissions
+ assert out.permissions[0].action.name == "can_edit"
+ assert out.permissions[0].resource.name == "DAG"
+
+ def test_patch_role_rename_success(self, get_fab_auth_manager,
fab_auth_manager, security_manager):
+ role = _make_role_obj("viewer", [("can_edit", "DAG")])
+ security_manager.find_role.return_value = role
+ fab_auth_manager.security_manager = security_manager
+ get_fab_auth_manager.return_value = fab_auth_manager
+ body = types.SimpleNamespace(
+ name="editor",
+ permissions=[
+ types.SimpleNamespace(
+ action=types.SimpleNamespace(name="can_edit"),
+ resource=types.SimpleNamespace(name="DAG"),
+ )
+ ],
+ )
+ out = FABAuthManagerRoles.patch_role(body=body, name="viewer")
+ assert out.name == "editor"
+ assert out.permissions
+ assert out.permissions[0].action.name == "can_edit"
+ assert out.permissions[0].resource.name == "DAG"
+
+ def test_patch_role_with_update_mask(self, get_fab_auth_manager,
fab_auth_manager, security_manager):
+ role = _make_role_obj("viewer", [("can_read", "DAG")])
+ security_manager.find_role.return_value = role
+ fab_auth_manager.security_manager = security_manager
+ get_fab_auth_manager.return_value = fab_auth_manager
+ body = types.SimpleNamespace(
+ name="viewer1",
+ permissions=[
+ types.SimpleNamespace(
+ action=types.SimpleNamespace(name="can_edit"),
+ resource=types.SimpleNamespace(name="DAG"),
+ )
+ ],
+ )
+ out = FABAuthManagerRoles.patch_role(
+ body=body,
+ name="viewer",
+ update_mask=["actions"],
+ )
+ assert out.name == "viewer"
+ assert out.permissions
+ assert out.permissions[0].action.name == "can_edit"
+ assert out.permissions[0].resource.name == "DAG"
+
+ def test_patch_role_rename_with_update_mask(
+ self, get_fab_auth_manager, fab_auth_manager, security_manager
+ ):
+ role = _make_role_obj("viewer", [("can_read", "DAG")])
+ security_manager.find_role.return_value = role
+ fab_auth_manager.security_manager = security_manager
+ get_fab_auth_manager.return_value = fab_auth_manager
+ body = types.SimpleNamespace(
+ name="viewer1",
+ permissions=[
+ types.SimpleNamespace(
+ action=types.SimpleNamespace(name="can_edit"),
+ resource=types.SimpleNamespace(name="DAG"),
+ )
+ ],
+ )
+ out = FABAuthManagerRoles.patch_role(
+ body=body,
+ name="viewer",
+ update_mask=["name"],
+ )
+ assert out.name == "viewer1"
+ assert out.permissions
+ assert out.permissions[0].action.name == "can_read"
+ assert out.permissions[0].resource.name == "DAG"
+
+ def test_patch_role_not_found(self, get_fab_auth_manager,
fab_auth_manager, security_manager):
+ security_manager.find_role.return_value = None
+ fab_auth_manager.security_manager = security_manager
+ get_fab_auth_manager.return_value = fab_auth_manager
+ body = types.SimpleNamespace(
+ name="viewer",
+ permissions=[
+ types.SimpleNamespace(
+ action=types.SimpleNamespace(name="can_edit"),
+ resource=types.SimpleNamespace(name="DAG"),
+ )
+ ],
+ )
+ with pytest.raises(HTTPException) as ex:
+ FABAuthManagerRoles.patch_role(body=body, name="viewer")
+ assert ex.value.status_code == 404
