This is an automated email from the ASF dual-hosted git repository.
vincbeck 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 23f1772772f fix(fab): clean up FK associations before deleting role
(#65375) (#66686)
23f1772772f is described below
commit 23f1772772f18cf8b1abc236992d5d4975ec8330
Author: June <[email protected]>
AuthorDate: Mon May 11 07:04:19 2026 -0700
fix(fab): clean up FK associations before deleting role (#65375) (#66686)
The delete_role method used a raw DELETE on ab_role which fails with
IntegrityError on databases whose FK constraints lack CASCADE (common
in environments migrated from older Airflow versions).
Delete association rows from ab_permission_view_role, ab_user_role, and
ab_group_role before deleting the role itself.
---
.../fab/auth_manager/security_manager/override.py | 15 ++++++--
.../auth_manager/security_manager/test_override.py | 40 +++++++++++++++++++++-
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
index 9f54bb9761e..0466bc033e6 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
@@ -78,7 +78,9 @@ from airflow.providers.fab.auth_manager.models import (
Resource,
Role,
User,
+ assoc_group_role,
assoc_permission_role,
+ assoc_user_role,
)
from airflow.providers.fab.auth_manager.models.anonymous_user import
AnonymousUser
from airflow.providers.fab.auth_manager.security_manager.constants import
EXISTING_ROLES
@@ -1332,12 +1334,21 @@ class
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
"""
Delete the given Role.
+ Cleans up association table rows (permission-role, user-role,
group-role) before
+ deleting the role itself, so that databases whose FK constraints lack
CASCADE
+ (e.g. databases migrated from older Airflow versions) do not raise
IntegrityError.
+
:param role_name: the name of a role in the ab_role table
"""
- role = self.session.scalars(select(Role).where(Role.name ==
role_name)).first()
+ role =
self.session.scalars(select(self.role_model).where(self.role_model.name ==
role_name)).first()
if role:
log.info("Deleting role '%s'", role_name)
- self.session.execute(delete(Role).where(Role.name == role_name))
+ self.session.execute(
+
delete(assoc_permission_role).where(assoc_permission_role.c.role_id == role.id)
+ )
+
self.session.execute(delete(assoc_user_role).where(assoc_user_role.c.role_id ==
role.id))
+
self.session.execute(delete(assoc_group_role).where(assoc_group_role.c.role_id
== role.id))
+
self.session.execute(delete(self.role_model).where(self.role_model.id ==
role.id))
self.session.commit()
else:
raise FabException(f"Role named '{role_name}' does not exist")
diff --git
a/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
b/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
index 409bd4bb700..a7a3e620d24 100644
---
a/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
+++
b/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
@@ -32,7 +32,10 @@ from airflow.providers.fab.auth_manager.models import (
Role,
User,
)
-from airflow.providers.fab.auth_manager.security_manager.override import
FabAirflowSecurityManagerOverride
+from airflow.providers.fab.auth_manager.security_manager.override import (
+ FabAirflowSecurityManagerOverride,
+ FabException,
+)
class EmptySecurityManager(FabAirflowSecurityManagerOverride):
@@ -43,6 +46,41 @@ class
EmptySecurityManager(FabAirflowSecurityManagerOverride):
class TestFabAirflowSecurityManagerOverride:
+
@mock.patch("airflow.providers.fab.auth_manager.security_manager.override.log")
+ def test_delete_role_cleans_up_associations_before_delete(self, mock_log):
+ """delete_role must remove association rows before the role row
itself."""
+ sm = EmptySecurityManager()
+ role = Mock(spec=Role, id=42, name="TestRole")
+
+ mock_session = Mock(spec=Session)
+ mock_scalars = Mock()
+ mock_scalars.first.return_value = role
+ mock_session.scalars.return_value = mock_scalars
+
+ with mock.patch.object(EmptySecurityManager, "session", mock_session):
+ sm.delete_role("TestRole")
+
+ # 4 deletes: permission-role, user-role, group-role, then the role
itself
+ assert mock_session.execute.call_count == 4
+ mock_session.commit.assert_called_once()
+ mock_log.info.assert_called_once_with("Deleting role '%s'", "TestRole")
+
+ def test_delete_role_raises_for_missing_role(self):
+ """delete_role must raise FabException when the role does not exist."""
+ sm = EmptySecurityManager()
+
+ mock_session = Mock(spec=Session)
+ mock_scalars = Mock()
+ mock_scalars.first.return_value = None
+ mock_session.scalars.return_value = mock_scalars
+
+ with mock.patch.object(EmptySecurityManager, "session", mock_session):
+ with pytest.raises(FabException, match="Role named 'NoSuchRole'
does not exist"):
+ sm.delete_role("NoSuchRole")
+
+ mock_session.execute.assert_not_called()
+ mock_session.commit.assert_not_called()
+
@mock.patch("airflow.providers.fab.auth_manager.security_manager.override.log")
def
test_add_permission_to_role_ignores_duplicate_from_concurrent_worker(self,
mock_log):
sm = EmptySecurityManager()
