Improve logging when skipping grant a in create-catalogs-schemas (#2917)

Improve logging when skipping grant a in `create-catalogs-schemas`

Author: JCZuurmond

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

@@ -77,7 +77,9 @@ def _update_principal_acl(self):
         for grant in grants:
             acl_migrate_sql = grant.uc_grant_sql()
             if acl_migrate_sql is None:
-                logger.warning(f"Cannot identify UC grant for {grant.this_type_and_key()}. Skipping.")
+                logger.warning(
+                    f"Skipping legacy grant that is not supported in UC: {grant.action_type} on {grant.this_type_and_key()}"
+                )
                 continue
             logger.debug(f"Migrating acls on {grant.this_type_and_key()} using SQL query: {acl_migrate_sql}")
             self._backend.execute(acl_migrate_sql)

tests/unit/hive_metastore/test_catalog_schema.py

@@ -106,7 +106,7 @@ def raise_catalog_exists(catalog: str, *_, **__) -> None:
         Grant('user1', 'MODIFY', 'catalog2', 'schema2', 'table'),
         Grant('user1', 'SELECT', 'catalog2', 'schema3', 'table2'),
         Grant('user1', 'USAGE', 'hive_metastore', 'schema3'),
-        Grant('user1', 'USAGE', 'hive_metastore', 'schema2'),
+        Grant('user1', 'DENY', 'hive_metastore', 'schema2'),
     ]
     hive_grants = [
         Grant(principal="princ1", catalog="hive_metastore", action_type="USE"),
@@ -244,7 +244,7 @@ def test_no_catalog_storage():
     ws.catalogs.create.assert_has_calls(calls, any_order=True)
 
 
-def test_catalog_schema_acl():
+def test_catalog_schema_acl() -> None:
     ws = create_autospec(WorkspaceClient)
     backend = MockBackend()
     mock_prompts = MockPrompts({"Please provide storage location url for catalog: *": ""})
@@ -260,10 +260,7 @@ def test_catalog_schema_acl():
     ws.schemas.create.assert_any_call("schema2", "catalog2", comment="Created by UCX")
     queries = [
         'GRANT USE SCHEMA ON DATABASE `catalog1`.`schema3` TO `user1`',
-        'GRANT USE SCHEMA ON DATABASE `catalog2`.`schema2` TO `user1`',
-        'GRANT USE SCHEMA ON DATABASE `catalog2`.`schema3` TO `user1`',
         'GRANT USE CATALOG ON CATALOG `catalog1` TO `user1`',
-        'GRANT USE CATALOG ON CATALOG `catalog2` TO `user1`',
         'GRANT USE CATALOG ON CATALOG `catalog1` TO `princ2`',
         'GRANT USE SCHEMA ON DATABASE `catalog1`.`schema3` TO `princ2`',
         'GRANT USE SCHEMA ON DATABASE `catalog2`.`schema2` TO `princ5`',
@@ -273,3 +270,21 @@ def test_catalog_schema_acl():
     assert len(backend.queries) == len(queries)
     for query in queries:
         assert query in backend.queries
+
+
+def test_create_all_catalogs_schemas_logs_untranslatable_grant(caplog) -> None:
+    ws = create_autospec(WorkspaceClient)
+    backend = MockBackend()
+    mock_prompts = MockPrompts({"Please provide storage location url for catalog: *": ""})
+    catalog_schema = prepare_test(ws, backend)
+
+    with caplog.at_level(logging.WARNING, logger="databricks.labs.ucx.hive_metastore.catalog_schema"):
+        catalog_schema.create_all_catalogs_schemas(mock_prompts)
+    assert "Skipping legacy grant that is not supported in UC: DENY on ('CATALOG', 'catalog2')" in caplog.messages
+    assert (
+        "Skipping legacy grant that is not supported in UC: DENY on ('DATABASE', 'catalog2.schema2')" in caplog.messages
+    )
+    assert (
+        "Skipping legacy grant that is not supported in UC: DENY on ('DATABASE', 'catalog2.schema3')" in caplog.messages
+    )
+    ws.assert_not_called()