xunliu commented on code in PR #5786:
URL: https://github.com/apache/gravitino/pull/5786#discussion_r1896771312


##########
authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java:
##########
@@ -212,27 +287,77 @@ public AuthorizationMetadataObject 
translateMetadataObject(MetadataObject metada
     Preconditions.checkArgument(
         nsMetadataObject.size() > 0, "The metadata object must have at least 
one name.");
 
-    if (metadataObject.type() == MetadataObject.Type.FILESET) {
-      RangerPathBaseMetadataObject rangerHDFSMetadataObject =
-          new RangerPathBaseMetadataObject(
-              getFileSetPath(metadataObject), 
RangerPathBaseMetadataObject.Type.PATH);
-      rangerHDFSMetadataObject.validateAuthorizationMetadataObject();
-      return rangerHDFSMetadataObject;
-    } else {
-      return new RangerPathBaseMetadataObject("", 
RangerPathBaseMetadataObject.Type.PATH);
+    RangerHDFSMetadataObject rangerHDFSMetadataObject;
+    switch (metadataObject.type()) {
+      case METALAKE:
+      case CATALOG:
+        rangerHDFSMetadataObject =
+            new RangerHDFSMetadataObject("", 
RangerHDFSMetadataObject.Type.PATH);
+        break;
+      case SCHEMA:
+        rangerHDFSMetadataObject =
+            new RangerHDFSMetadataObject(
+                metadataObject.fullName(), RangerHDFSMetadataObject.Type.PATH);
+        break;
+      case FILESET:
+        rangerHDFSMetadataObject =
+            new RangerHDFSMetadataObject(
+                getLocationPath(metadataObject), 
RangerHDFSMetadataObject.Type.PATH);
+        break;
+      default:
+        throw new AuthorizationPluginException(
+            "The metadata object type %s is not supported in the 
RangerAuthorizationHDFSPlugin",
+            metadataObject.type());
     }
+    rangerHDFSMetadataObject.validateAuthorizationMetadataObject();
+    return rangerHDFSMetadataObject;
   }
 
-  public String getFileSetPath(MetadataObject metadataObject) {
-    FilesetDispatcher filesetDispatcher = 
GravitinoEnv.getInstance().filesetDispatcher();
-    NameIdentifier identifier =
-        NameIdentifier.parse(String.format("%s.%s", metalake, 
metadataObject.fullName()));
-    Fileset fileset = filesetDispatcher.loadFileset(identifier);
-    Preconditions.checkArgument(
-        fileset != null, String.format("Fileset %s is not found", identifier));
-    String filesetLocation = fileset.storageLocation();
-    Preconditions.checkArgument(
-        filesetLocation != null, String.format("Fileset %s location is not 
found", identifier));
-    return pattern.matcher(filesetLocation).replaceAll("");
+  private NameIdentifier getObjectNameIdentifier(MetadataObject 
metadataObject) {
+    return NameIdentifier.parse(String.format("%s.%s", metalake, 
metadataObject.fullName()));
+  }
+
+  @VisibleForTesting
+  public String getLocationPath(MetadataObject metadataObject) throws 
NoSuchEntityException {
+    String locationPath = null;
+    switch (metadataObject.type()) {
+      case METALAKE:
+      case SCHEMA:
+      case TABLE:

Review Comment:
   I already add todo integration test in the `TestChainedAuthorizationIT`.



##########
authorizations/authorization-chain/src/main/java/org/apache/gravitino/authorization/chain/ChainAuthorizationPlugin.java:
##########
@@ -0,0 +1,283 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.authorization.chain;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import org.apache.gravitino.Catalog;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.authorization.AuthorizationProperties;
+import org.apache.gravitino.authorization.ChainAuthorizationProperties;
+import org.apache.gravitino.authorization.Group;
+import org.apache.gravitino.authorization.MetadataObjectChange;
+import org.apache.gravitino.authorization.Owner;
+import org.apache.gravitino.authorization.Role;
+import org.apache.gravitino.authorization.RoleChange;
+import org.apache.gravitino.authorization.User;
+import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
+import org.apache.gravitino.connector.authorization.BaseAuthorization;
+import org.apache.gravitino.exceptions.AuthorizationPluginException;
+import org.apache.gravitino.utils.IsolatedClassLoader;
+
+/** Chain authorization operations plugin class. <br> */
+public class ChainAuthorizationPlugin implements AuthorizationPlugin {
+  private List<AuthorizationPlugin> plugins = Lists.newArrayList();
+  private final String metalake;
+
+  public ChainAuthorizationPlugin(
+      String metalake, String catalogProvider, Map<String, String> config) {
+    this.metalake = metalake;
+    initPlugins(catalogProvider, config);
+  }
+
+  private void initPlugins(String catalogProvider, Map<String, String> 
properties) {
+    ChainAuthorizationProperties chainAuthorizationProperties =
+        new ChainAuthorizationProperties(properties);
+    chainAuthorizationProperties.validate();
+    // Validate the properties for each plugin
+    chainAuthorizationProperties
+        .plugins()
+        .forEach(
+            pluginName -> {
+              Map<String, String> pluginProperties =
+                  
chainAuthorizationProperties.fetchAuthPluginProperties(pluginName);
+              String authProvider = 
chainAuthorizationProperties.getPluginProvider(pluginName);
+              AuthorizationProperties.validate(authProvider, pluginProperties);
+            });
+    // Create the plugins
+    chainAuthorizationProperties
+        .plugins()
+        .forEach(
+            pluginName -> {
+              String authProvider = 
chainAuthorizationProperties.getPluginProvider(pluginName);
+              Map<String, String> pluginConfig =
+                  
chainAuthorizationProperties.fetchAuthPluginProperties(pluginName);
+
+              ArrayList<String> libAndResourcesPaths = Lists.newArrayList();
+              BaseAuthorization.buildAuthorizationPkgPath(
+                      ImmutableMap.of(Catalog.AUTHORIZATION_PROVIDER, 
authProvider))
+                  .ifPresent(libAndResourcesPaths::add);
+              IsolatedClassLoader classLoader =
+                  IsolatedClassLoader.buildClassLoader(libAndResourcesPaths);
+              try {
+                BaseAuthorization<?> authorization =
+                    BaseAuthorization.createAuthorization(classLoader, 
authProvider);
+                AuthorizationPlugin authorizationPlugin =
+                    authorization.newPlugin(metalake, catalogProvider, 
pluginConfig);
+                plugins.add(authorizationPlugin);
+              } catch (Exception e) {
+                throw new RuntimeException(e);
+              }
+            });
+  }
+
+  @Override
+  public void close() throws IOException {
+    for (AuthorizationPlugin plugin : plugins) {
+      plugin.close();
+    }
+  }
+
+  @Override
+  public Boolean onMetadataUpdated(MetadataObjectChange... changes)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onMetadataUpdated(changes);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onRoleCreated(Role role) throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onRoleCreated(role);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onRoleAcquired(Role role) throws AuthorizationPluginException 
{
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onRoleAcquired(role);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onRoleDeleted(role);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onRoleUpdated(Role role, RoleChange... changes)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onRoleUpdated(role, changes);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onGrantedRolesToUser(List<Role> roles, User user)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onGrantedRolesToUser(roles, user);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onRevokedRolesFromUser(List<Role> roles, User user)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onRevokedRolesFromUser(roles, user);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onGrantedRolesToGroup(List<Role> roles, Group group)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onGrantedRolesToGroup(roles, group);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onRevokedRolesFromGroup(List<Role> roles, Group group)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onRevokedRolesFromGroup(roles, group);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onUserAdded(User user) throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onUserAdded(user);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onUserRemoved(User user) throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onUserRemoved(user);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onUserAcquired(User user) throws AuthorizationPluginException 
{
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onUserAcquired(user);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onGroupAdded(Group group) throws AuthorizationPluginException 
{
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onGroupAdded(group);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onGroupRemoved(Group group) throws 
AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onGroupRemoved(group);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onGroupAcquired(Group group) throws 
AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onGroupAcquired(group);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+
+  @Override
+  public Boolean onOwnerSet(MetadataObject metadataObject, Owner preOwner, 
Owner newOwner)
+      throws AuthorizationPluginException {
+    for (AuthorizationPlugin plugin : plugins) {
+      Boolean result = plugin.onOwnerSet(metadataObject, preOwner, newOwner);
+      if (Boolean.FALSE.equals(result)) {
+        return Boolean.FALSE;
+      }
+    }
+    return Boolean.TRUE;
+  }
+}

Review Comment:
   DONE



##########
authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/AuthorizationProperties.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.authorization;

Review Comment:
   DONE



##########
authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHDFSSecurableObject.java:
##########
@@ -25,12 +25,12 @@
 import org.apache.gravitino.authorization.AuthorizationPrivilege;
 import org.apache.gravitino.authorization.AuthorizationSecurableObject;
 
-public class RangerPathBaseSecurableObject extends RangerPathBaseMetadataObject
+public class RangerHDFSSecurableObject extends RangerHDFSMetadataObject

Review Comment:
   OK, I rollback this change.



##########
authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerAuthorizationHDFSPlugin.java:
##########
@@ -137,10 +171,52 @@ public List<AuthorizationSecurableObject> 
translatePrivilege(SecurableObject sec
                   .forEach(
                       rangerPrivilege ->
                           rangerPrivileges.add(
-                              new RangerPrivileges.RangerHivePrivilegeImpl(
+                              new RangerPrivileges.RangerHDFSPrivilegeImpl(
                                   rangerPrivilege, 
gravitinoPrivilege.condition())));
-
               switch (gravitinoPrivilege.name()) {
+                case USE_CATALOG:
+                case CREATE_CATALOG:
+                  // When HDFS is used as the Hive storage layer, Hive does 
not support the
+                  // `USE_CATALOG` and `CREATE_CATALOG` privileges. So, we 
ignore these
+                  // in the RangerAuthorizationHDFSPlugin.
+                  break;
+                case USE_SCHEMA:
+                  break;
+                case CREATE_SCHEMA:
+                  switch (securableObject.type()) {
+                    case METALAKE:
+                    case CATALOG:
+                      {
+                        String locationPath = getLocationPath(securableObject);
+                        if (locationPath != null && !locationPath.isEmpty()) {
+                          RangerHDFSMetadataObject rangerHDFSMetadataObject =
+                              new RangerHDFSMetadataObject(
+                                  locationPath, 
RangerHDFSMetadataObject.Type.PATH);
+                          rangerSecurableObjects.add(
+                              generateAuthorizationSecurableObject(
+                                  rangerHDFSMetadataObject.names(),
+                                  RangerHDFSMetadataObject.Type.PATH,
+                                  rangerPrivileges));
+                        }
+                      }
+                      break;
+                    case FILESET:
+                      rangerSecurableObjects.add(
+                          generateAuthorizationSecurableObject(
+                              translateMetadataObject(securableObject).names(),
+                              RangerHDFSMetadataObject.Type.PATH,
+                              rangerPrivileges));
+                      break;
+                    default:
+                      throw new AuthorizationPluginException(
+                          "The privilege %s is not supported for the securable 
object: %s",
+                          gravitinoPrivilege.name(), securableObject.type());
+                  }
+                  break;
+                case SELECT_TABLE:

Review Comment:
   I already add todo integration test in the `TestChainedAuthorizationIT`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to