[GitHub] [gobblin] meethngala commented on a diff in pull request #3755: [GOBBLIN-1892] replace setAcl with modifyEntries while preserving acls during distcp

["meethngala (via GitHub)" <gi...@apache.org>]

meethngala commented on code in PR #3755:
URL: https://github.com/apache/gobblin/pull/3755#discussion_r1313378858


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java:
##########
@@ -441,7 +444,33 @@ public static Map<String, OwnerAndPermission> resolveReplicatedAncestorOwnerAndP
 
   private static List<AclEntry> getAclEntries(FileSystem srcFs, Path path) throws IOException {
     AclStatus aclStatus = srcFs.getAclStatus(path);
-    return aclStatus.getEntries();
+    return addOthersEntryTypeToAclEntriesIfMissing(aclStatus.getEntries());
+  }
+  /*
+   * When we get AclEntry from org.apache.hadoop.fs.permission.AclStatus, it's missing AclEntryType.OTHER.
+   * This causes AclTransformation validation to fail on the list of AclEntries. As a result, adding this helper
+   * method to provide DEFAULT scope in cases where AclEntryType.OTHER is absent
+   */
+  private static List<AclEntry> addOthersEntryTypeToAclEntriesIfMissing(List<AclEntry> aclEntries) {
+    // Check if "others" entry is missing
+    boolean othersAclEntryTypeMissing = true;
+
+    for (AclEntry aclEntry : aclEntries) {
+      if (aclEntry.getType() == AclEntryType.OTHER) {
+        othersAclEntryTypeMissing = false;
+        break;
+      }
+    }
+    // If "others" entry is missing, add it
+    if (othersAclEntryTypeMissing) {

Review Comment:
   with the recent change of using `modifyEntries` instead of `setAcl`... we would not require to check this since we will always assume we get the right list of acl entries 



-- 
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: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org