[#5364] feat(auth-ranger): Throw AuthorizationPluginException in auth…

…orization plugin (#5365) ### What changes were proposed in this pull request? Currently, the Authorization plugin throws RuntimePluginException, We needs change it to AuthorizationPluginExceptionto ### Why are the changes needed? Fix: #5364 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Added ITs.
apache · Oct 31, 2024 · d04a17c · d04a17c
1 parent 0232b3c
commit d04a17c
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 75 deletions.
diff --git a/api/src/main/java/org/apache/gravitino/exceptions/AuthorizationPluginException.java b/api/src/main/java/org/apache/gravitino/exceptions/AuthorizationPluginException.java
@@ -22,7 +22,7 @@
 import com.google.errorprone.annotations.FormatString;
 
 /** An exception thrown when an authorization plugin operation failed. */
-public class AuthorizationPluginException extends IllegalArgumentException {
+public class AuthorizationPluginException extends GravitinoRuntimeException {
 
   /**
    * Constructs a new exception with the specified detail message.
@@ -36,11 +36,15 @@ public AuthorizationPluginException(@FormatString String message, Object... args
   }
 
   /**
-   * Constructs a new exception with the specified cause.
+   * Constructs a new exception with the specified detail message and cause.
    *
    * @param cause the cause.
+   * @param message the detail message.
+   * @param args the arguments to the message.
    */
-  public AuthorizationPluginException(Throwable cause) {
-    super(cause);
+  @FormatMethod
+  public AuthorizationPluginException(
+      Throwable cause, @FormatString String message, Object... args) {
+    super(cause, message, args);
   }
 }
diff --git a/...er/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java b/...er/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationPlugin.java
@@ -113,7 +113,7 @@ protected RangerAuthorizationPlugin(Map<String, String> config) {
    * 2. Save role name in the Policy items. <br>
    */
   @Override
-  public Boolean onRoleCreated(Role role) throws RuntimeException {
+  public Boolean onRoleCreated(Role role) throws AuthorizationPluginException {
     if (!validAuthorizationOperation(role.securableObjects())) {
       return false;
     }
@@ -127,7 +127,7 @@ public Boolean onRoleCreated(Role role) throws RuntimeException {
   }
 
   @Override
-  public Boolean onRoleAcquired(Role role) throws RuntimeException {
+  public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException {
     if (!validAuthorizationOperation(role.securableObjects())) {
       return false;
     }
@@ -136,7 +136,7 @@ public Boolean onRoleAcquired(Role role) throws RuntimeException {
 
   /** Remove the role name from the Ranger policy item, and delete this Role in the Ranger. <br> */
   @Override
-  public Boolean onRoleDeleted(Role role) throws RuntimeException {
+  public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
     if (!validAuthorizationOperation(role.securableObjects())) {
       return false;
     }
@@ -157,7 +157,8 @@ public Boolean onRoleDeleted(Role role) throws RuntimeException {
   }
 
   @Override
-  public Boolean onRoleUpdated(Role role, RoleChange... changes) throws RuntimeException {
+  public Boolean onRoleUpdated(Role role, RoleChange... changes)
+      throws AuthorizationPluginException {
     for (RoleChange change : changes) {
       if (change instanceof RoleChange.AddSecurableObject) {
         SecurableObject securableObject =
@@ -171,7 +172,7 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes) throws RuntimeExc
             .forEach(
                 rangerSecurableObject -> {
                   if (!doAddSecurableObject(role.name(), rangerSecurableObject)) {
-                    throw new RuntimeException(
+                    throw new AuthorizationPluginException(
                         "Failed to add the securable object to the Ranger policy!");
                   }
                 });
@@ -187,7 +188,7 @@ public Boolean onRoleUpdated(Role role, RoleChange... changes) throws RuntimeExc
             .forEach(
                 rangerSecurableObject -> {
                   if (!doRemoveSecurableObject(role.name(), rangerSecurableObject)) {
-                    throw new RuntimeException(
+                    throw new AuthorizationPluginException(
                         "Failed to add the securable object to the Ranger policy!");
                   }
                 });
@@ -276,7 +277,7 @@ public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws Runtime
    */
   @Override
   public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner newOwner)
-      throws RuntimeException {
+      throws AuthorizationPluginException {
     Preconditions.checkArgument(newOwner != null, "The newOwner must be not null");
 
     // Add the user or group to the Ranger
@@ -365,7 +366,8 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
                       rangerClient.updatePolicy(policy.getId(), policy);
                     }
                   } catch (RangerServiceException e) {
-                    throw new RuntimeException(e);
+                    throw new AuthorizationPluginException(
+                        e, "Failed to add the owner to the Ranger!");
                   }
                 });
         break;
@@ -385,7 +387,8 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
                       rangerClient.updatePolicy(policy.getId(), policy);
                     }
                   } catch (RangerServiceException e) {
-                    throw new RuntimeException(e);
+                    throw new AuthorizationPluginException(
+                        e, "Failed to add the owner to the Ranger!");
                   }
                 });
         break;
@@ -408,7 +411,8 @@ public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, Owner n
    * @param user The user to grant the roles.
    */
   @Override
-  public Boolean onGrantedRolesToUser(List<Role> roles, User user) throws RuntimeException {
+  public Boolean onGrantedRolesToUser(List<Role> roles, User user)
+      throws AuthorizationPluginException {
     if (roles.stream().anyMatch(role -> !validAuthorizationOperation(role.securableObjects()))) {
       return false;
     }
@@ -443,7 +447,8 @@ public Boolean onGrantedRolesToUser(List<Role> roles, User user) throws RuntimeE
    * @param user The user to revoke the roles.
    */
   @Override
-  public Boolean onRevokedRolesFromUser(List<Role> roles, User user) throws RuntimeException {
+  public Boolean onRevokedRolesFromUser(List<Role> roles, User user)
+      throws AuthorizationPluginException {
     if (roles.stream().anyMatch(role -> !validAuthorizationOperation(role.securableObjects()))) {
       return false;
     }
@@ -477,7 +482,8 @@ public Boolean onRevokedRolesFromUser(List<Role> roles, User user) throws Runtim
    * @param group The group to grant the roles.
    */
   @Override
-  public Boolean onGrantedRolesToGroup(List<Role> roles, Group group) throws RuntimeException {
+  public Boolean onGrantedRolesToGroup(List<Role> roles, Group group)
+      throws AuthorizationPluginException {
     if (roles.stream().anyMatch(role -> !validAuthorizationOperation(role.securableObjects()))) {
       return false;
     }
@@ -510,7 +516,8 @@ public Boolean onGrantedRolesToGroup(List<Role> roles, Group group) throws Runti
    * @param group The group to revoke the roles.
    */
   @Override
-  public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group) throws RuntimeException {
+  public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group)
+      throws AuthorizationPluginException {
     if (roles.stream().anyMatch(role -> !validAuthorizationOperation(role.securableObjects()))) {
       return false;
     }
@@ -533,7 +540,7 @@ public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group) throws Run
   }
 
   @Override
-  public Boolean onUserAdded(User user) throws RuntimeException {
+  public Boolean onUserAdded(User user) throws AuthorizationPluginException {
     VXUserList list = rangerClient.searchUser(ImmutableMap.of("name", user.name()));
     if (list.getListSize() > 0) {
       LOG.warn("The user({}) already exists in the Ranger!", user.name());
@@ -545,7 +552,7 @@ public Boolean onUserAdded(User user) throws RuntimeException {
   }
 
   @Override
-  public Boolean onUserRemoved(User user) throws RuntimeException {
+  public Boolean onUserRemoved(User user) throws AuthorizationPluginException {
     VXUserList list = rangerClient.searchUser(ImmutableMap.of("name", user.name()));
     if (list.getListSize() == 0) {
       LOG.warn("The user({}) doesn't exist in the Ranger!", user);
@@ -556,7 +563,7 @@ public Boolean onUserRemoved(User user) throws RuntimeException {
   }
 
   @Override
-  public Boolean onUserAcquired(User user) throws RuntimeException {
+  public Boolean onUserAcquired(User user) throws AuthorizationPluginException {
     VXUserList list = rangerClient.searchUser(ImmutableMap.of("name", user.name()));
     if (list.getListSize() == 0) {
       LOG.warn("The user({}) doesn't exist in the Ranger!", user);
@@ -566,13 +573,13 @@ public Boolean onUserAcquired(User user) throws RuntimeException {
   }
 
   @Override
-  public Boolean onGroupAdded(Group group) throws RuntimeException {
+  public Boolean onGroupAdded(Group group) throws AuthorizationPluginException {
     return rangerClient.createGroup(
         VXGroup.builder().withName(group.name()).withDescription(group.name()).build());
   }
 
   @Override
-  public Boolean onGroupRemoved(Group group) throws RuntimeException {
+  public Boolean onGroupRemoved(Group group) throws AuthorizationPluginException {
     VXGroupList list = rangerClient.searchGroup(ImmutableMap.of("name", group.name()));
     if (list.getListSize() == 0) {
       LOG.warn("The group({}) doesn't exist in the Ranger!", group);
@@ -650,7 +657,8 @@ private boolean doAddSecurableObject(String roleName, RangerSecurableObject secu
         rangerClient.updatePolicy(policy.getId(), policy);
       }
     } catch (RangerServiceException e) {
-      throw new RuntimeException(e);
+      throw new AuthorizationPluginException(
+          e, "Failed to add the securable object to the Ranger!");
     }
 
     return true;
@@ -720,7 +728,8 @@ private boolean doRemoveSecurableObject(
       }
     } catch (RangerServiceException e) {
       LOG.error("Failed to remove the policy item from the Ranger policy {}!", policy);
-      throw new RuntimeException(e);
+      throw new AuthorizationPluginException(
+          e, "Failed to remove the securable object from Ranger!");
     }
     return true;
   }

diff --git a/...rization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java b/...rization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java
@@ -182,7 +182,7 @@ public List<RangerPolicy> wildcardSearchPolies(List<String> metadataNames)
       List<RangerPolicy> policies = rangerClient.findPolicies(searchFilters);
       return policies;
     } catch (RangerServiceException e) {
-      throw new AuthorizationPluginException(e);
+      throw new AuthorizationPluginException(e, "Failed to find the policies in the Ranger");
     }
   }
 
@@ -246,7 +246,8 @@ protected boolean checkRangerRole(String roleName) throws AuthorizationPluginExc
     try {
       rangerClient.getRole(roleName, rangerAdminName, rangerServiceName);
     } catch (RangerServiceException e) {
-      throw new AuthorizationPluginException(e);
+      throw new AuthorizationPluginException(
+          e, "Failed to check the role(%s) in the Ranger", roleName);
     }
     return true;
   }
@@ -302,7 +303,8 @@ protected RangerRole createRangerRoleIfNotExists(String roleName, boolean isOwne
         rangerClient.createRole(rangerServiceName, rangerRole);
       }
     } catch (RangerServiceException e) {
-      throw new RuntimeException(e);
+      throw new AuthorizationPluginException(
+          e, "Failed to create the role(%s) in the Ranger", roleName);
     }
     return rangerRole;
   }

diff --git a/...rc/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java b/...rc/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java
@@ -22,6 +22,7 @@
 import static org.apache.gravitino.authorization.ranger.integration.test.RangerITEnv.rangerClient;
 import static org.apache.gravitino.authorization.ranger.integration.test.RangerITEnv.verifyRoleInRanger;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
@@ -49,6 +50,7 @@
 import org.apache.gravitino.authorization.ranger.RangerPrivileges;
 import org.apache.gravitino.authorization.ranger.RangerSecurableObject;
 import org.apache.gravitino.authorization.ranger.reference.RangerDefines;
+import org.apache.gravitino.exceptions.AuthorizationPluginException;
 import org.apache.gravitino.integration.test.util.GravitinoITUtils;
 import org.apache.gravitino.meta.AuditInfo;
 import org.apache.gravitino.meta.GroupEntity;
@@ -325,16 +327,20 @@ public void testFindManagedPolicy() {
     String dbName = currentFunName();
     createHivePolicy(
         Lists.newArrayList(String.format("%s*", dbName), "*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     createHivePolicy(
         Lists.newArrayList(String.format("%s*", dbName), "tab*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     createHivePolicy(
         Lists.newArrayList(String.format("%s3", dbName), "*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     createHivePolicy(
         Lists.newArrayList(String.format("%s3", dbName), "tab*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     // findManagedPolicy function use precise search, so return null
     RangerSecurableObject rangerSecurableObject =
         rangerAuthHivePlugin.generateRangerSecurableObject(
@@ -348,12 +354,33 @@ public void testFindManagedPolicy() {
     // Add a policy for `db3.tab1`
     createHivePolicy(
         Lists.newArrayList(String.format("%s3", dbName), "tab1"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     // findManagedPolicy function use precise search, so return not null
     Assertions.assertNotNull(rangerHelper.findManagedPolicy(rangerSecurableObject));
   }
 
-  static void createHivePolicy(List<String> metaObjects, String roleName) {
+  @Test
+  public void testManagedByGravitinoLabel() {
+    RoleEntity role = mock3TableRole(currentFunName());
+    role.securableObjects().stream()
+        .forEach(
+            securableObject -> {
+              Joiner DOT_JOINER = Joiner.on('.');
+              List<String> names =
+                  Lists.newArrayList(
+                      SecurableObjects.DOT_SPLITTER.splitToList(securableObject.fullName()));
+              names.remove(0); // remove catalog node
+              // Manual create the Ranger Policy
+              createHivePolicy(Lists.newArrayList(names), DOT_JOINER.join(names), false);
+            });
+    // Use role to create Ranger Policy
+    Assertions.assertThrows(
+        AuthorizationPluginException.class, () -> rangerAuthHivePlugin.onRoleCreated(role));
+  }
+
+  static void createHivePolicy(
+      List<String> metaObjects, String roleName, boolean labelManagedByGravitino) {
     Assertions.assertTrue(metaObjects.size() < 4);
     Map<String, RangerPolicy.RangerPolicyResource> policyResourceMap = new HashMap<>();
     for (int i = 0; i < metaObjects.size(); i++) {
@@ -377,7 +404,8 @@ static void createHivePolicy(List<String> metaObjects, String roleName) {
         RangerITEnv.RANGER_HIVE_REPO_NAME,
         roleName,
         policyResourceMap,
-        Collections.singletonList(policyItem));
+        Collections.singletonList(policyItem),
+        labelManagedByGravitino);
   }
 
   static boolean deleteHivePolicy(RangerSecurableObject rangerSecurableObject) {
@@ -780,16 +808,20 @@ void metadataObjectChangeRemoveMetalakeOrCatalog(String funcName, MetadataObject
       throws RangerServiceException {
     createHivePolicy(
         Lists.newArrayList(String.format("%s*", funcName), "*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     createHivePolicy(
         Lists.newArrayList(String.format("%s*", funcName), "tab*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     createHivePolicy(
         Lists.newArrayList(String.format("%s3", funcName), "*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     createHivePolicy(
         Lists.newArrayList(String.format("%s3", funcName), "tab*"),
-        GravitinoITUtils.genRandomName(currentFunName()));
+        GravitinoITUtils.genRandomName(currentFunName()),
+        true);
     Assertions.assertEquals(
         4, rangerClient.getPoliciesInService(RangerITEnv.RANGER_HIVE_REPO_NAME).size());
 

diff --git a/...src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java b/...src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java
@@ -180,7 +180,8 @@ static void allowAnyoneAccessHDFS() {
         RANGER_HDFS_REPO_NAME,
         policyName,
         policyResourceMap,
-        Collections.singletonList(policyItem));
+        Collections.singletonList(policyItem),
+        false);
   }
 
   /**
@@ -217,7 +218,8 @@ static void allowAnyoneAccessInformationSchema() {
         RANGER_HIVE_REPO_NAME,
         policyName,
         policyResourceMap,
-        Collections.singletonList(policyItem));
+        Collections.singletonList(policyItem),
+        false);
   }
 
   public void createRangerTrinoRepository(String trinoIp) {
@@ -513,7 +515,8 @@ protected static void updateOrCreateRangerPolicy(
       String serviceName,
       String policyName,
       Map<String, RangerPolicy.RangerPolicyResource> policyResourceMap,
-      List<RangerPolicy.RangerPolicyItem> policyItems) {
+      List<RangerPolicy.RangerPolicyItem> policyItems,
+      boolean labelManagedByGravitino) {
 
     Map<String, String> resourceFilter = new HashMap<>(); // use to match the precise policy
     Map<String, String> policyFilter = new HashMap<>();
@@ -572,7 +575,9 @@ protected static void updateOrCreateRangerPolicy(
         policy.setServiceType(type);
         policy.setService(serviceName);
         policy.setName(policyName);
-        policy.setPolicyLabels(Lists.newArrayList(RangerHelper.MANAGED_BY_GRAVITINO));
+        if (labelManagedByGravitino) {
+          policy.setPolicyLabels(Lists.newArrayList(RangerHelper.MANAGED_BY_GRAVITINO));
+        }
         policy.setResources(policyResourceMap);
         policy.setPolicyItems(policyItems);
         rangerClient.createPolicy(policy);