Skip to content

Commit 1796118

Browse files
authored
Allowing skipping TACLs migration during table migration. (#3384)
closes #3042
1 parent ac57c46 commit 1796118

File tree

7 files changed

+51
-0
lines changed

7 files changed

+51
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,6 +1313,7 @@ access the configuration file from the command line. Here's the description of c
13131313
* `num_threads`: An optional integer representing the number of threads to use for migration.
13141314
* `database_to_catalog_mapping`: An optional dictionary mapping source database names to target catalog names.
13151315
* `default_catalog`: An optional string representing the default catalog name.
1316+
* `skip_tacl_migration`: Optional flag, allow skipping TACL migration when migrating tables or creating catalogs and schemas.
13161317
* `default_owner_group`: Assigns this group to all migrated objects (catalogs, databases, tables, views, etc.). The group has to be an account group and the user running the migration has to be a member of this group.
13171318
* `log_level`: An optional string representing the log level.
13181319
* `workspace_start_path`: A string representing the starting path for notebooks and directories crawler in the workspace.

src/databricks/labs/ucx/config.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ class WorkspaceConfig: # pylint: disable=too-many-instance-attributes
8686
# Default table owner, assigned to a group
8787
default_owner_group: str | None = None
8888

89+
# Skip TACL migration during table migration
90+
skip_tacl_migration: bool = False
91+
8992
def replace_inventory_variable(self, text: str) -> str:
9093
return text.replace("$inventory", f"hive_metastore.{self.inventory_database}")
9194

src/databricks/labs/ucx/contexts/application.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ def migrate_grants(self) -> MigrateGrants:
349349
self.sql_backend,
350350
self.group_manager,
351351
grant_loaders,
352+
skip_tacl_migration=self.config.skip_tacl_migration,
352353
)
353354

354355
@cached_property

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,13 +777,20 @@ def __init__(
777777
sql_backend: SqlBackend,
778778
group_manager: GroupManager,
779779
grant_loaders: list[Callable[[], Iterable[Grant]]],
780+
*,
781+
skip_tacl_migration: bool = False,
780782
):
781783
self._sql_backend = sql_backend
782784
self._group_manager = group_manager
783785
# List of grant loaders, the assumption that the first one is a loader for ownership grants
784786
self._grant_loaders = grant_loaders
787+
if skip_tacl_migration:
788+
logger.warning("Skipping TACL migration")
789+
self._skip_tacl_migration = skip_tacl_migration
785790

786791
def apply(self, src: SecurableObject, dst: SecurableObject) -> bool:
792+
if self._skip_tacl_migration:
793+
return True
787794
grants = self._match_grants(src)
788795
for grant in grants:
789796
acl_migrate_sql = grant.uc_grant_sql(dst.kind, dst.full_name)

src/databricks/labs/ucx/install.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,8 @@ def _prompt_for_new_installation(self) -> WorkspaceConfig:
241241
configure_groups.run()
242242
include_databases = self._select_databases()
243243

244+
skip_tacl_migration = self.prompts.confirm("Do you want to skip TACL migration when migrating tables?")
245+
244246
# Checking if the user wants to define a default owner group.
245247
default_owner_group = None
246248
if self.prompts.confirm("Do you want to define a default owner group for all tables and schemas? "):
@@ -262,6 +264,7 @@ def _prompt_for_new_installation(self) -> WorkspaceConfig:
262264
group_match_by_external_id=configure_groups.group_match_by_external_id, # type: ignore[arg-type]
263265
include_group_names=configure_groups.include_group_names,
264266
renamed_group_prefix=configure_groups.renamed_group_prefix,
267+
skip_tacl_migration=skip_tacl_migration,
265268
log_level=log_level,
266269
num_threads=num_threads,
267270
include_databases=include_databases,

tests/unit/hive_metastore/test_grants.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -953,6 +953,41 @@ def test_grant_supports_history(mock_backend, grant_record: Grant, history_recor
953953
assert rows == [history_record]
954954

955955

956+
def test_migrate_grants_skip():
957+
group_manager = create_autospec(GroupManager)
958+
backend = MockBackend()
959+
960+
src = Table("hive_metastore", "database", "table", "MANAGED", "DELTA")
961+
grant = Grant("user", "SELECT")
962+
dst = Table("catalog", "database", "table", "MANAGED", "DELTA")
963+
964+
def grant_loader() -> list[Grant]:
965+
catalog = src.catalog
966+
database = src.database
967+
table = src.name
968+
return [
969+
dataclasses.replace(
970+
grant,
971+
catalog=catalog,
972+
database=database,
973+
table=table,
974+
),
975+
]
976+
977+
migrate_grants = MigrateGrants(
978+
backend,
979+
group_manager,
980+
[grant_loader],
981+
skip_tacl_migration=True,
982+
)
983+
984+
migrate_grants.apply(src, dst)
985+
986+
for query in backend.queries:
987+
assert not query.startswith("GRANT")
988+
group_manager.assert_not_called()
989+
990+
956991
# Testing the validation in retrival of the default owner group. 666 is the current_user user_id.
957992
@pytest.mark.parametrize("user_id, expected", [("666", True), ("777", False)])
958993
def test_default_owner(user_id, expected) -> None:

tests/unit/install/test_install.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ def result():
352352
),
353353
(r"Min workers for auto-scale.*", "2", {"min_workers": 2}),
354354
(r"Max workers for auto-scale.*", "20", {"max_workers": 20}),
355+
(r"Do you want to skip TACL.*", "yes", {"skip_tacl_migration": True}),
355356
],
356357
)
357358
def test_configure_sets_expected_workspace_configuration_values(

0 commit comments

Comments
 (0)