This is an automated email from the ASF dual-hosted git repository.
roryqi 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 87a18901c8 [#6716] improvement(authz): Delete catalogs if failing to
execute post hook actions (#6717)
87a18901c8 is described below
commit 87a18901c80338e6bb7594d5a74663ec6bf63433
Author: roryqi <[email protected]>
AuthorDate: Thu Mar 20 17:19:33 2025 +0800
[#6716] improvement(authz): Delete catalogs if failing to execute post hook
actions (#6717)
### What changes were proposed in this pull request?
Delete catalogs if failing to execute post hook actions
### Why are the changes needed?
Fix: #6716
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Add a new IT.
---
.../ranger/RangerAuthorizationPlugin.java | 9 +++--
.../ranger/integration/test/RangerHiveE2EIT.java | 39 ++++++++++++++++++++++
.../gravitino/hook/CatalogHookDispatcher.java | 36 ++++++++++++--------
3 files changed, 65 insertions(+), 19 deletions(-)
diff --git
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
index e1a8c21c4b..f4d19df233 100644
---
a/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
+++
b/authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
@@ -103,10 +103,7 @@ public abstract class RangerAuthorizationPlugin
rangerServiceName =
config.get(RangerAuthorizationProperties.RANGER_SERVICE_NAME);
rangerClient = new RangerClientExtension(rangerUrl, authType,
rangerAdminName, password);
- if (Boolean.parseBoolean(
-
config.get(RangerAuthorizationProperties.RANGER_SERVICE_CREATE_IF_ABSENT))) {
- createRangerServiceIfNecessary(config, rangerServiceName);
- }
+ createRangerServiceIfNecessary(config, rangerServiceName);
rangerHelper =
new RangerHelper(
@@ -786,7 +783,9 @@ public abstract class RangerAuthorizationPlugin
try {
rangerClient.getService(serviceName);
} catch (RangerServiceException rse) {
- if (rse.getStatus().equals(ClientResponse.Status.NOT_FOUND)) {
+ if (Boolean.parseBoolean(
+
config.get(RangerAuthorizationProperties.RANGER_SERVICE_CREATE_IF_ABSENT))
+ && ClientResponse.Status.NOT_FOUND.equals(rse.getStatus())) {
try {
RangerService rangerService = new RangerService();
rangerService.setType(getServiceType());
diff --git
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java
index a90b0d86bb..53c81bd0b9 100644
---
a/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java
+++
b/authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveE2EIT.java
@@ -237,6 +237,45 @@ public class RangerHiveE2EIT extends RangerBaseE2EIT {
} catch (Exception e) {
throw new RuntimeException(e);
}
+
+ // Test to create a catalog with wrong properties which lacks Ranger
service name,
+ // It will throw RuntimeException with message that Ranger service name is
required.
+ Map<String, String> wrongProperties =
+ ImmutableMap.of(
+ HiveConstants.METASTORE_URIS,
+ HIVE_METASTORE_URIS,
+ IMPERSONATION_ENABLE,
+ "true",
+ AUTHORIZATION_PROVIDER,
+ "ranger",
+ RangerAuthorizationProperties.RANGER_SERVICE_TYPE,
+ "HadoopSQL",
+ RangerAuthorizationProperties.RANGER_ADMIN_URL,
+ RangerITEnv.RANGER_ADMIN_URL,
+ RangerAuthorizationProperties.RANGER_AUTH_TYPE,
+ RangerContainer.authType,
+ RangerAuthorizationProperties.RANGER_USERNAME,
+ RangerContainer.rangerUserName,
+ RangerAuthorizationProperties.RANGER_PASSWORD,
+ RangerContainer.rangerPassword,
+ RangerAuthorizationProperties.RANGER_SERVICE_CREATE_IF_ABSENT,
+ "true");
+
+ int catalogSize = metalake.listCatalogs().length;
+ Exception exception =
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () ->
+ metalake.createCatalog(
+ "wrongTestProperties",
+ Catalog.Type.RELATIONAL,
+ provider,
+ "comment",
+ wrongProperties));
+ Assertions.assertTrue(
+ exception.getMessage().contains("authorization.ranger.service.name is
required"));
+
+ Assertions.assertEquals(catalogSize, metalake.listCatalogs().length);
}
protected void checkTableAllPrivilegesExceptForCreating() {
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 07dc4f079a..ba28bbbbd4 100644
--- a/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
+++ b/core/src/main/java/org/apache/gravitino/hook/CatalogHookDispatcher.java
@@ -40,6 +40,8 @@ import
org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NonEmptyEntityException;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* {@code CatalogHookDispatcher} is a decorator for {@link CatalogDispatcher}
that not only
@@ -47,6 +49,7 @@ import org.apache.gravitino.utils.PrincipalUtils;
* operations before or after the underlying operations.
*/
public class CatalogHookDispatcher implements CatalogDispatcher {
+ private static final Logger LOG =
LoggerFactory.getLogger(CatalogHookDispatcher.class);
private final CatalogDispatcher dispatcher;
public CatalogHookDispatcher(CatalogDispatcher dispatcher) {
@@ -82,21 +85,26 @@ public class CatalogHookDispatcher implements
CatalogDispatcher {
Catalog catalog = dispatcher.createCatalog(ident, type, provider, comment,
properties);
- // Set the creator as the owner of the catalog.
- OwnerManager ownerManager = GravitinoEnv.getInstance().ownerManager();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.CATALOG),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
- }
+ try {
+ // Set the creator as the owner of the catalog.
+ OwnerManager ownerManager = GravitinoEnv.getInstance().ownerManager();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.CATALOG),
+ PrincipalUtils.getCurrentUserName(),
+ Owner.Type.USER);
+ }
- // Apply the metalake securable object privileges to authorization plugin
- FutureGrantManager futureGrantManager =
GravitinoEnv.getInstance().futureGrantManager();
- if (futureGrantManager != null && catalog instanceof BaseCatalog) {
- futureGrantManager.grantNewlyCreatedCatalog(
- ident.namespace().level(0), (BaseCatalog) catalog);
+ // Apply the metalake securable object privileges to authorization plugin
+ FutureGrantManager futureGrantManager =
GravitinoEnv.getInstance().futureGrantManager();
+ if (futureGrantManager != null && catalog instanceof BaseCatalog) {
+ futureGrantManager.grantNewlyCreatedCatalog(
+ ident.namespace().level(0), (BaseCatalog) catalog);
+ }
+ } catch (Exception e) {
+ LOG.warn("Fail to execute the post hook operations, rollback the catalog
" + ident, e);
+ dispatcher.dropCatalog(ident, true);
}
return catalog;
