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 e288b73c85b Check all names before airflow roles import writes
anything (#74711)
e288b73c85b is described below
commit e288b73c85b90356d311ca71986b80ebcf9689e2
Author: CyberNative AI LLC <[email protected]>
AuthorDate: Wed Oct 14 03:59:07 2026 -0700
Check all names before airflow roles import writes anything (#74711)
---
.../fab/auth_manager/cli_commands/role_command.py | 26 ++++++++
.../auth_manager/cli_commands/test_role_command.py | 69 ++++++++++++++++++++++
2 files changed, 95 insertions(+)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/role_command.py
b/providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/role_command.py
index 5f065a3bd0f..130fa123827 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/role_command.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/role_command.py
@@ -204,6 +204,32 @@ def roles_import(args):
with get_application_builder() as appbuilder:
existing_roles = [role.name for role in appbuilder.sm.get_all_roles()]
roles_to_import = [role_dict for role_dict in role_list if
role_dict["name"] not in existing_roles]
+ # Each permission is committed as it is added, so check every name
before the first write;
+ # otherwise a failure leaves a partial role that a later import skips
as already existing.
+ permission_entries = [
+ role_dict
+ for role_dict in roles_to_import
+ if role_dict.get("action") and role_dict.get("resource")
+ ]
+ unknown_resources = [
+ name
+ for name in dict.fromkeys(role_dict["resource"] for role_dict in
permission_entries)
+ if not appbuilder.sm.get_resource(name)
+ ]
+ unknown_actions = [
+ name
+ for name in dict.fromkeys(
+ action for role_dict in permission_entries for action in
role_dict["action"].split(",")
+ )
+ if not appbuilder.sm.get_action(name)
+ ]
+ if unknown_resources or unknown_actions:
+ for name in unknown_resources:
+ print(f"Resource named '{name}' does not exist")
+ for name in unknown_actions:
+ print(f"Action named '{name}' does not exist")
+ print("No roles were imported")
+ exit(1)
for role_dict in roles_to_import:
# ``roles_export`` emits one entry per (role, resource) pair, so a
role name
# repeats across entries; ``add_role`` returns the existing role
after the first.
diff --git
a/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_role_command.py
b/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_role_command.py
index f79ce5eba45..4869a6944da 100644
---
a/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_role_command.py
+++
b/providers/fab/tests/unit/fab/auth_manager/cli_commands/test_role_command.py
@@ -248,3 +248,72 @@ class TestCliRoles:
(permissions.RESOURCE_POOL, permissions.ACTION_CAN_EDIT),
(permissions.RESOURCE_POOL, permissions.ACTION_CAN_READ),
}
+
+ @pytest.mark.parametrize(
+ ("roles_list", "messages"),
+ [
+ pytest.param(
+ [
+ {"name": "FakeTeamA", "resource": "Pools", "action":
"can_edit,can_read"},
+ {"name": "FakeTeamB", "resource": "NonExistentResource",
"action": "can_read"},
+ ],
+ ["Resource named 'NonExistentResource' does not exist"],
+ id="unknown-resource",
+ ),
+ pytest.param(
+ [
+ {"name": "FakeTeamA", "resource": "Pools", "action":
"can_edit,can_read"},
+ {"name": "FakeTeamB", "resource": "Pools", "action":
"can_read,can_nonexistent"},
+ ],
+ ["Action named 'can_nonexistent' does not exist"],
+ id="unknown-action",
+ ),
+ pytest.param(
+ [
+ {"name": "FakeTeamA", "resource": "Pools", "action":
"can_edit,can_read"},
+ {"name": "FakeTeamB", "resource": "NonExistentResource",
"action": "can_read"},
+ {"name": "FakeTeamB", "resource": "Pools", "action":
"can_nonexistent"},
+ {"name": "FakeTeamC", "resource": "NonExistentResource",
"action": "can_nonexistent"},
+ ],
+ [
+ "Resource named 'NonExistentResource' does not exist",
+ "Action named 'can_nonexistent' does not exist",
+ ],
+ id="several-unknown-names",
+ ),
+ ],
+ )
+ def test_cli_import_roles_with_unknown_names_changes_nothing(self,
tmp_path, roles_list, messages):
+ fn = tmp_path / "import_roles.json"
+ fn.write_text(json.dumps(roles_list))
+
+ with redirect_stdout(StringIO()) as stdout_io,
pytest.raises(SystemExit) as exc_info:
+ role_command.roles_import(self.parser.parse_args(["roles",
"import", str(fn)]))
+
+ assert exc_info.value.code == 1
+ stdout = stdout_io.getvalue()
+ for message in messages:
+ assert stdout.count(message) == 1
+ assert self.appbuilder.sm.find_role("FakeTeamA") is None
+ assert self.appbuilder.sm.find_role("FakeTeamB") is None
+ assert self.appbuilder.sm.find_role("FakeTeamC") is None
+
+ def
test_cli_import_roles_skips_entries_of_existing_roles_before_checking_names(self,
tmp_path):
+ self.appbuilder.sm.add_role("FakeTeamA")
+ fn = tmp_path / "import_roles.json"
+ roles_list = [
+ {"name": "FakeTeamA", "resource": "NonExistentResource", "action":
"can_nonexistent"},
+ {"name": "FakeTeamB", "resource": "Pools", "action": "can_read"},
+ ]
+ fn.write_text(json.dumps(roles_list))
+ role_command.roles_import(self.parser.parse_args(["roles", "import",
str(fn)]))
+
+ fakeTeamA: Role = self.appbuilder.sm.find_role("FakeTeamA")
+ fakeTeamB: Role = self.appbuilder.sm.find_role("FakeTeamB")
+
+ assert fakeTeamA is not None
+ assert fakeTeamB is not None
+ assert len(fakeTeamA.permissions) == 0
+ assert {
+ (permission.resource.name, permission.action.name) for permission
in fakeTeamB.permissions
+ } == {(permissions.RESOURCE_POOL, permissions.ACTION_CAN_READ)}
