[#6042] refactor: Use callback to delete the privilege of catalog

apache · Dec 30, 2024 · 1704d13 · 1704d13
1 parent 061f24b
commit 1704d13
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 16 deletions.
diff --git a/...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java b/...test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
@@ -142,6 +142,8 @@ protected void cleanIT() {
               (schema -> {
                 catalog.asSchemas().dropSchema(schema, false);
               }));
+
+      // This code will call the catalog metadata object remove privileges
       Arrays.stream(metalake.listCatalogs())
           .forEach((catalogName -> metalake.dropCatalog(catalogName, true)));
       client.disableMetalake(metalakeName);

diff --git a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -174,19 +174,8 @@ public static void callAuthorizationPluginForSecurableObjects(
 
   public static void callAuthorizationPluginForMetadataObject(
       String metalake, MetadataObject metadataObject, Consumer<AuthorizationPlugin> consumer) {
-    CatalogManager catalogManager = GravitinoEnv.getInstance().catalogManager();
-    if (needApplyAuthorizationPluginAllCatalogs(metadataObject.type())) {
-      NameIdentifier[] catalogs = catalogManager.listCatalogs(Namespace.of(metalake));
-      // ListCatalogsInfo return `CatalogInfo` instead of `BaseCatalog`, we need `BaseCatalog` to
-      // call authorization plugin method.
-      for (NameIdentifier catalog : catalogs) {
-        callAuthorizationPluginImpl(consumer, catalogManager.loadCatalog(catalog));
-      }
-    } else if (needApplyAuthorization(metadataObject.type())) {
-      NameIdentifier catalogIdent =
-          NameIdentifierUtil.getCatalogIdentifier(
-              MetadataObjectUtil.toEntityIdent(metalake, metadataObject));
-      Catalog catalog = catalogManager.loadCatalog(catalogIdent);
+    List<Catalog> loadedCatalogs = loadMetadataObjectCatalog(metalake, metadataObject);
+    for (Catalog catalog : loadedCatalogs) {
       callAuthorizationPluginImpl(consumer, catalog);
     }
   }
@@ -255,15 +244,45 @@ public static void authorizationPluginRemovePrivileges(
     if (GravitinoEnv.getInstance().accessControlDispatcher() != null) {
       MetadataObject metadataObject = NameIdentifierUtil.toMetadataObject(ident, type);
       MetadataObjectChange removeObject = MetadataObjectChange.remove(metadataObject);
+
+      String metalake =
+          type == Entity.EntityType.METALAKE ? ident.name() : ident.namespace().level(0);
+
       callAuthorizationPluginForMetadataObject(
-          ident.namespace().level(0),
+          metalake,
           metadataObject,
           authorizationPlugin -> {
             authorizationPlugin.onMetadataUpdated(removeObject);
           });
     }
   }
 
+  public static Runnable createRemovePrivilegesCallback(
+      NameIdentifier ident, Entity.EntityType type) {
+    // If we enable authorization, we should remove the privileges about the entity in the
+    // authorization plugin.
+    if (GravitinoEnv.getInstance().accessControlDispatcher() != null) {
+      MetadataObject metadataObject = NameIdentifierUtil.toMetadataObject(ident, type);
+      MetadataObjectChange removeObject = MetadataObjectChange.remove(metadataObject);
+
+      String metalake =
+          type == Entity.EntityType.METALAKE ? ident.name() : ident.namespace().level(0);
+
+      List<Catalog> catalogs = loadMetadataObjectCatalog(metalake, metadataObject);
+
+      return () -> {
+        for (Catalog catalog : catalogs) {
+          callAuthorizationPluginImpl(
+              authorizationPlugin -> {
+                authorizationPlugin.onMetadataUpdated(removeObject);
+              },
+              catalog);
+        }
+      };
+    }
+    throw new IllegalStateException("Authorization is not enabled");
+  }
+
   public static void authorizationPluginRenamePrivileges(
       NameIdentifier ident, Entity.EntityType type, String newName) {
     // If we enable authorization, we should rename the privileges about the entity in the
@@ -364,4 +383,26 @@ private static void checkCatalogType(
           catalogIdent, catalog.type(), privilege);
     }
   }
+
+  private static List<Catalog> loadMetadataObjectCatalog(
+      String metalake, MetadataObject metadataObject) {
+    CatalogManager catalogManager = GravitinoEnv.getInstance().catalogManager();
+    List<Catalog> loadedCatalogs = Lists.newArrayList();
+    if (needApplyAuthorizationPluginAllCatalogs(metadataObject.type())) {
+      NameIdentifier[] catalogs = catalogManager.listCatalogs(Namespace.of(metalake));
+      // ListCatalogsInfo return `CatalogInfo` instead of `BaseCatalog`, we need `BaseCatalog` to
+      // call authorization plugin method.
+      for (NameIdentifier catalog : catalogs) {
+        loadedCatalogs.add(catalogManager.loadCatalog(catalog));
+      }
+    } else if (needApplyAuthorization(metadataObject.type())) {
+      NameIdentifier catalogIdent =
+          NameIdentifierUtil.getCatalogIdentifier(
+              MetadataObjectUtil.toEntityIdent(metalake, metadataObject));
+      Catalog catalog = catalogManager.loadCatalog(catalogIdent);
+      loadedCatalogs.add(catalog);
+    }
+
+    return loadedCatalogs;
+  }
 }
diff --git a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
@@ -126,8 +126,20 @@ public boolean dropCatalog(NameIdentifier ident) {
   @Override
   public boolean dropCatalog(NameIdentifier ident, boolean force)
       throws NonEmptyEntityException, CatalogInUseException {
-    AuthorizationUtils.authorizationPluginRemovePrivileges(ident, Entity.EntityType.CATALOG);
-    return dispatcher.dropCatalog(ident, force);
+    // If we call the authorization plugin after dropping catalog, we can't load the plugin of the
+    // catalog
+    Runnable removePrivilegesCallback = null;
+    if (dispatcher.catalogExists(ident)) {
+      removePrivilegesCallback =
+          AuthorizationUtils.createRemovePrivilegesCallback(ident, Entity.EntityType.CATALOG);
+    }
+
+    boolean dropped = dispatcher.dropCatalog(ident, force);
+
+    if (dropped && removePrivilegesCallback != null) {
+      removePrivilegesCallback.run();
+    }
+    return dropped;
   }
 
   @Override