This is an automated email from the ASF dual-hosted git repository.
henry3260 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 8e283dcb3ac Fix airflow roles import crash on a role without
permissions (#72796)
8e283dcb3ac is described below
commit 8e283dcb3ac323745ac0d700a1e23b8cf4f64689
Author: Y-C <[email protected]>
AuthorDate: Tue Sep 15 23:54:17 2026 +0800
Fix airflow roles import crash on a role without permissions (#72796)
* Fix airflow roles import crash on a role without permissions
`airflow roles export` writes a role that holds no permissions as an entry
with
an empty resource and action, so the command cannot always read back its own
output. Importing such a file aborted outright when that entry came first,
and
silently re-applied the preceding entry's permissions when it came later —
both
of which break the export/import round trip these two commands exist to
serve.
The existing coverage missed this because its permissionless entry sits
last in
the fixture, where the stale binding from an earlier iteration papers over
the
missing one.
* Update test_role_command.py
Co-authored-by: rjgoyln <[email protected]>
* Apply suggestion from @rjgoyln
Co-authored-by: rjgoyln <[email protected]>
---------
Co-authored-by: Eason09053360
<[email protected]>
Co-authored-by: Henry Chen <[email protected]>
Co-authored-by: rjgoyln <[email protected]>
---
.../fab/auth_manager/cli_commands/role_command.py | 37 ++++++++--------------
.../auth_manager/cli_commands/test_role_command.py | 22 +++++++++++++
2 files changed, 35 insertions(+), 24 deletions(-)
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 6bbebcb5a0a..5f065a3bd0f 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
@@ -205,28 +205,17 @@ def roles_import(args):
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]
for role_dict in roles_to_import:
- if role_dict["name"] not in appbuilder.sm.get_all_roles():
- if role_dict["action"] == "" or role_dict["resource"] == "":
- appbuilder.sm.add_role(role_dict["name"])
- else:
- appbuilder.sm.add_role(role_dict["name"])
- role_args = Namespace(
- subcommand="add-perms",
- role=[role_dict["name"]],
- resource=[role_dict["resource"]],
- action=role_dict["action"].split(","),
- )
- __roles_add_or_remove_permissions(role_args)
-
- if role_dict["name"] in appbuilder.sm.get_all_roles():
- if role_dict["action"] == "" or role_dict["resource"] == "":
- pass
- else:
- role_args = Namespace(
- subcommand="add-perms",
- role=[role_dict["name"]],
- resource=[role_dict["resource"]],
- action=role_dict["action"].split(","),
- )
- __roles_add_or_remove_permissions(role_args)
+ # ``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.
+ appbuilder.sm.add_role(role_dict["name"])
+ if not role_dict.get("action") or not role_dict.get("resource"):
+ continue
+ __roles_add_or_remove_permissions(
+ Namespace(
+ subcommand="add-perms",
+ role=[role_dict["name"]],
+ resource=[role_dict["resource"]],
+ action=role_dict["action"].split(","),
+ )
+ )
print("roles and permissions successfully imported")
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 49281b6c9d2..f79ce5eba45 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
@@ -226,3 +226,25 @@ class TestCliRoles:
and permission.action.name == permissions.ACTION_CAN_ACCESS_MENU
for permission in fakeTeamA.permissions
)
+
+ def
test_cli_import_roles_when_a_role_without_permissions_comes_first(self,
tmp_path):
+ fn = tmp_path / "import_roles.json"
+ roles_list = [
+ {"name": "FakeTeamB", "resource": "", "action": ""},
+ {"name": "FakeTeamA", "resource": "Pools", "action":
"can_edit,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(fakeTeamB.permissions) == 0
+ assert {
+ (permission.resource.name, permission.action.name) for permission
in fakeTeamA.permissions
+ } == {
+ (permissions.RESOURCE_POOL, permissions.ACTION_CAN_EDIT),
+ (permissions.RESOURCE_POOL, permissions.ACTION_CAN_READ),
+ }
