sumitagrawl commented on code in PR #7455:
URL: https://github.com/apache/ozone/pull/7455#discussion_r1867175476


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java:
##########
@@ -47,24 +49,44 @@ public final class OzoneAclUtil {
   private OzoneAclUtil() {
   }
 
+  private static ACLType[] userRights;

Review Comment:
   this static initialization need to be protected, else can be initalized 
multiple time in multi-threaded env. There is no much impact as its re-init 
with same value only.
   If not planned protect, order of groupRights init and userRights init should 
be controlled, else one of member can be null for other threads as per if-check.



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java:
##########
@@ -47,24 +49,44 @@ public final class OzoneAclUtil {
   private OzoneAclUtil() {
   }
 
+  private static ACLType[] userRights;
+  private static ACLType[] groupRights;
+
   /**
-   * Helper function to get access acl list for current user.
+   * Helper function to get default access acl list for current user.
    *
-   * @param userName
-   * @param userGroups
+   * @param ugi current login user
+   * @param conf current configuration
    * @return list of OzoneAcls
    * */
-  public static List<OzoneAcl> getAclList(String userName,
-      String[] userGroups, ACLType userRights, ACLType groupRights) {
-
+  public static List<OzoneAcl> getDefaultAclList(UserGroupInformation ugi, 
OzoneConfiguration conf) {
+    // Get default acl rights for user and group.
+    if (userRights == null || groupRights == null) {
+      OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
+      userRights = aclConfig.getUserDefaultRights();
+      groupRights = aclConfig.getGroupDefaultRights();
+    }
     List<OzoneAcl> listOfAcls = new ArrayList<>();
+    // User ACL.
+    listOfAcls.add(new OzoneAcl(USER, ugi.getShortUserName(), ACCESS, 
userRights));
+    try {
+      String groupName = ugi.getPrimaryGroupName();
+      listOfAcls.add(new OzoneAcl(GROUP, groupName, ACCESS, groupRights));
+    } catch (IOException e) {
+      // do nothing, since user has the permission, user can add ACL for 
selected groups later.
+    }
+    return listOfAcls;
+  }
 
+  public static List<OzoneAcl> getAclList(UserGroupInformation ugi, ACLType 
userPrivilege, ACLType groupPrivilege) {
+    List<OzoneAcl> listOfAcls = new ArrayList<>();
     // User ACL.
-    listOfAcls.add(new OzoneAcl(USER, userName, ACCESS, userRights));
-    if (userGroups != null) {
-      // Group ACLs of the User.
-      Arrays.asList(userGroups).forEach((group) -> listOfAcls.add(
-          new OzoneAcl(GROUP, group, ACCESS, groupRights)));
+    listOfAcls.add(new OzoneAcl(USER, ugi.getShortUserName(), ACCESS, 
userPrivilege));
+    try {
+      String groupName = ugi.getPrimaryGroupName();
+      listOfAcls.add(new OzoneAcl(GROUP, groupName, ACCESS, groupPrivilege));
+    } catch (IOException e) {
+      // do nothing, since user has the permission, user can add ACL for 
selected groups later.

Review Comment:
   can add warn log in exception case



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java:
##########
@@ -47,24 +49,44 @@ public final class OzoneAclUtil {
   private OzoneAclUtil() {
   }
 
+  private static ACLType[] userRights;
+  private static ACLType[] groupRights;
+
   /**
-   * Helper function to get access acl list for current user.
+   * Helper function to get default access acl list for current user.
    *
-   * @param userName
-   * @param userGroups
+   * @param ugi current login user
+   * @param conf current configuration
    * @return list of OzoneAcls
    * */
-  public static List<OzoneAcl> getAclList(String userName,
-      String[] userGroups, ACLType userRights, ACLType groupRights) {
-
+  public static List<OzoneAcl> getDefaultAclList(UserGroupInformation ugi, 
OzoneConfiguration conf) {
+    // Get default acl rights for user and group.
+    if (userRights == null || groupRights == null) {
+      OzoneAclConfig aclConfig = conf.getObject(OzoneAclConfig.class);
+      userRights = aclConfig.getUserDefaultRights();
+      groupRights = aclConfig.getGroupDefaultRights();
+    }
     List<OzoneAcl> listOfAcls = new ArrayList<>();
+    // User ACL.
+    listOfAcls.add(new OzoneAcl(USER, ugi.getShortUserName(), ACCESS, 
userRights));
+    try {
+      String groupName = ugi.getPrimaryGroupName();
+      listOfAcls.add(new OzoneAcl(GROUP, groupName, ACCESS, groupRights));
+    } catch (IOException e) {
+      // do nothing, since user has the permission, user can add ACL for 
selected groups later.

Review Comment:
   can have warn log to identify issue if there.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java:
##########
@@ -322,16 +324,25 @@ private boolean isECBucket(BucketInfo bucketInfo) {
    * @param omVolumeArgs
    */
   private void addDefaultAcls(OmBucketInfo omBucketInfo,
-      OmVolumeArgs omVolumeArgs) {
-    // Add default acls for bucket creator.
+      OmVolumeArgs omVolumeArgs, OzoneManager ozoneManager) throws OMException 
{
     List<OzoneAcl> acls = new ArrayList<>();
+    // Add default acls
+    acls.addAll(getDefaultAclList(createUGIForApi(), 
ozoneManager.getConfiguration()));
     if (omBucketInfo.getAcls() != null) {
+      // Add acls for bucket creator.
       acls.addAll(omBucketInfo.getAcls());
     }
 
+    // Link bucket default acl
+    if (omBucketInfo.getSourceBucket() != null && 
omBucketInfo.getSourceVolume() != null) {
+      acls.add(LINK_BUCKET_DEFAULT_ACL);

Review Comment:
   RpcClient also added, and same added here also, plz check if de-duplicated 
this from this point.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to