Skip to content

Commit 5423f58

Browse files
authored
Skip applying permissions for workspace system groups to Unity Catalog resources (#2997)
## Changes Skip applying permissions for workspace system groups because they do not exist on the account level. ### Linked issues Resolves #2986 ### Functionality ACL related code - [x] modified existing command: `databricks labs ucx create-catalog-schemas` - [x] modified existing workflow: `migrate-table-*` ### Tests - [x] added unit tests - [x] added integration tests
1 parent 15f7bca commit 5423f58

File tree

4 files changed

+40
-4
lines changed

4 files changed

+40
-4
lines changed

src/databricks/labs/ucx/hive_metastore/grants.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,8 @@ def _get_external_locations(
592592

593593

594594
class PrincipalACL:
595+
_workspace_system_groups = {"admins", "users"}
596+
595597
def __init__(
596598
self,
597599
ws: WorkspaceClient,
@@ -692,7 +694,7 @@ def _get_cluster_principal_mapping(self, cluster_id: str, object_type: str) -> l
692694
if acl.user_name is not None:
693695
principal_list.append(acl.user_name)
694696
if acl.group_name is not None:
695-
if acl.group_name == "admins":
697+
if acl.group_name in self._workspace_system_groups: # Do not exists on account level
696698
continue
697699
principal_list.append(acl.group_name)
698700
if acl.service_principal_name is not None:

tests/integration/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1228,7 +1228,7 @@ def prepare_tables_for_migration(
12281228

12291229

12301230
@pytest.fixture
1231-
def prepared_principal_acl(runtime_ctx, env_or_skip, make_mounted_location, make_catalog, make_schema):
1231+
def prepared_principal_acl(runtime_ctx, make_mounted_location, make_catalog, make_schema):
12321232
src_schema = make_schema(catalog_name="hive_metastore")
12331233
src_external_table = runtime_ctx.make_table(
12341234
catalog_name=src_schema.catalog_name,

tests/integration/hive_metastore/test_catalog_schema.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def test_create_catalog_schema_with_principal_acl_aws(
137137

138138

139139
@retried(on=[NotFound], timeout=timedelta(minutes=3))
140-
def test_create_catalog_schema_with_legacy_acls(
140+
def test_create_catalog_schema_with_legacy_hive_metastore_privileges(
141141
ws: WorkspaceClient,
142142
runtime_ctx,
143143
make_random,
@@ -146,7 +146,7 @@ def test_create_catalog_schema_with_legacy_acls(
146146
) -> None:
147147
src_schema = runtime_ctx.make_schema(catalog_name="hive_metastore")
148148
src_table = runtime_ctx.make_table(catalog_name=src_schema.catalog_name, schema_name=src_schema.name)
149-
dst_catalog_name = f"ucx-{make_random()}"
149+
dst_catalog_name = f"ucx_{make_random()}"
150150
dst_schema_name = "test"
151151
rules = [Rule("workspace", dst_catalog_name, src_schema.name, dst_schema_name, src_table.name, src_table.name)]
152152
runtime_ctx.with_table_mapping_rules(rules)
@@ -174,3 +174,32 @@ def get_schema_permissions_list(full_name: str) -> PermissionsList:
174174
schema_grants = get_schema_permissions_list(f"{dst_catalog_name}.{dst_schema_name}")
175175
assert schema_grants.privilege_assignments is not None
176176
assert PrivilegeAssignment(table_owner.user_name, [Privilege.USE_SCHEMA]) in schema_grants.privilege_assignments
177+
178+
179+
def test_create_catalog_schema_when_users_group_in_warehouse_acl(
180+
caplog,
181+
runtime_ctx,
182+
make_random,
183+
make_warehouse,
184+
make_warehouse_permissions,
185+
) -> None:
186+
"""Privileges inferred from being a member of the 'users' group are ignored."""
187+
src_schema = runtime_ctx.make_schema(catalog_name="hive_metastore")
188+
src_table = runtime_ctx.make_table(catalog_name=src_schema.catalog_name, schema_name=src_schema.name)
189+
dst_catalog_name = f"ucx_{make_random()}"
190+
dst_schema_name = "test"
191+
rule = Rule("workspace", dst_catalog_name, src_schema.name, dst_schema_name, src_table.name, src_table.name)
192+
runtime_ctx.with_table_mapping_rules([rule])
193+
runtime_ctx.with_dummy_resource_permission()
194+
runtime_ctx.make_group()
195+
warehouse = make_warehouse()
196+
make_warehouse_permissions(object_id=warehouse.id, permission_level=PermissionLevel.CAN_USE, group_name="users")
197+
mock_prompts = MockPrompts({"Please provide storage location url for catalog: *": ""})
198+
199+
with caplog.at_level(logging.WARNING, logger="databricks.labs.ucx.account.aggregate"):
200+
runtime_ctx.catalog_schema.create_all_catalogs_schemas(mock_prompts)
201+
202+
failed_to_migrate_message = (
203+
f"failed-to-migrate: Failed to migrate ACL for {src_schema.full_name} to {dst_catalog_name}"
204+
)
205+
assert failed_to_migrate_message not in caplog.messages

tests/unit/hive_metastore/test_principal_grants.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,18 @@ def ws():
7777
service_principal_name='spn1',
7878
all_permissions=[iam.Permission(permission_level=iam.PermissionLevel.CAN_USE)],
7979
),
80+
iam.AccessControlResponse(
81+
group_name='admins',
82+
all_permissions=[iam.Permission(permission_level=iam.PermissionLevel.CAN_USE)],
83+
),
8084
],
8185
),
8286
'warehouse1': iam.ObjectPermissions(
8387
object_id='warehouse1',
8488
object_type="warehouses",
8589
access_control_list=[
8690
iam.AccessControlResponse(group_name='group2', all_permissions=[iam.Permission(inherited=False)]),
91+
iam.AccessControlResponse(group_name='users', all_permissions=[iam.Permission(inherited=False)]),
8792
],
8893
),
8994
}

0 commit comments

Comments
 (0)