This is an automated email from the ASF dual-hosted git repository.
liuxun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 31a60e544 [#6042] refactor: Delete the privilege of catalog after
dropping the catalogs (#6045)
31a60e544 is described below
commit 31a60e544be8e0c514303e43e43ea220e79cba96
Author: roryqi <ror...@apache.org>
AuthorDate: Thu Jan 9 11:23:11 2025 +0800
[#6042] refactor: Delete the privilege of catalog after dropping the
catalogs (#6045)
### What changes were proposed in this pull request?
Delete the privilege of catalog after dropping the catalogs
### Why are the changes needed?
Fix: #6042
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI passed.
---------
Co-authored-by: Qiming Teng <ten...@outlook.com>
---
.../ranger/integration/test/RangerBaseE2EIT.java | 2 +
.../authorization/AuthorizationUtils.java | 57 ++++++++++++++++------
.../gravitino/hook/CatalogHookDispatcher.java | 21 ++++++--
3 files changed, 61 insertions(+), 19 deletions(-)
diff --git
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
index 730b426e3..8a502ce5e 100644
---
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
+++
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerBaseE2EIT.java
@@ -143,6 +143,8 @@ public abstract class RangerBaseE2EIT extends BaseIT {
(schema -> {
catalog.asSchemas().dropSchema(schema, false);
}));
+
+ // The `dropCatalog` call will invoke the catalog metadata object to
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
index c7096c765..499ba5cbf 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java
@@ -33,6 +33,7 @@ import org.apache.gravitino.Catalog;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
import org.apache.gravitino.Schema;
@@ -186,19 +187,8 @@ public class AuthorizationUtils {
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);
}
}
@@ -266,9 +256,12 @@ public class AuthorizationUtils {
// authorization plugin.
if (GravitinoEnv.getInstance().accessControlDispatcher() != null) {
MetadataObject metadataObject =
NameIdentifierUtil.toMetadataObject(ident, type);
+ String metalake =
+ type == Entity.EntityType.METALAKE ? ident.name() :
ident.namespace().level(0);
+
MetadataObjectChange removeObject =
MetadataObjectChange.remove(metadataObject, locations);
callAuthorizationPluginForMetadataObject(
- ident.namespace().level(0),
+ metalake,
metadataObject,
authorizationPlugin -> {
authorizationPlugin.onMetadataUpdated(removeObject);
@@ -276,6 +269,20 @@ public class AuthorizationUtils {
}
}
+ public static void removeCatalogPrivileges(Catalog catalog, List<String>
locations) {
+ // If we enable authorization, we should remove the privileges about the
entity in the
+ // authorization plugin.
+ MetadataObject metadataObject =
+ MetadataObjects.of(null, catalog.name(), MetadataObject.Type.CATALOG);
+ MetadataObjectChange removeObject =
MetadataObjectChange.remove(metadataObject, locations);
+
+ callAuthorizationPluginImpl(
+ authorizationPlugin -> {
+ authorizationPlugin.onMetadataUpdated(removeObject);
+ },
+ catalog);
+ }
+
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
@@ -377,6 +384,28 @@ public class AuthorizationUtils {
}
}
+ 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;
+ }
+
// The Hive default schema location is Hive warehouse directory
private static String getHiveDefaultLocation(String metalakeName, String
catalogName) {
NameIdentifier defaultSchemaIdent =
diff --git
a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
index 65b722fdf..cc350a15c 100644
--- a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
@@ -127,11 +127,22 @@ public class CatalogHookDispatcher implements
CatalogDispatcher {
@Override
public boolean dropCatalog(NameIdentifier ident, boolean force)
throws NonEmptyEntityException, CatalogInUseException {
- List<String> locations =
- AuthorizationUtils.getMetadataObjectLocation(ident,
Entity.EntityType.CATALOG);
- AuthorizationUtils.authorizationPluginRemovePrivileges(
- ident, Entity.EntityType.CATALOG, locations);
- return dispatcher.dropCatalog(ident, force);
+ if (!dispatcher.catalogExists(ident)) {
+ return false;
+ }
+
+ // If we call the authorization plugin after dropping catalog, we can't
load the plugin of the
+ // catalog
+ Catalog catalog = dispatcher.loadCatalog(ident);
+ boolean dropped = dispatcher.dropCatalog(ident, force);
+
+ if (dropped && catalog != null) {
+ List<String> locations =
+ AuthorizationUtils.getMetadataObjectLocation(ident,
Entity.EntityType.CATALOG);
+ AuthorizationUtils.removeCatalogPrivileges(catalog, locations);
+ }
+
+ return dropped;
}
@Override
