add AclEntryType.OTHER if absent while preserving acls during distcp

apache · Aug 30, 2023 · e898825 · e898825
1 parent 0ab094a
commit e898825
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/...n-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java b/...n-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java
@@ -29,7 +29,10 @@
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.AclEntry;
+import org.apache.hadoop.fs.permission.AclEntryScope;
+import org.apache.hadoop.fs.permission.AclEntryType;
 import org.apache.hadoop.fs.permission.AclStatus;
+import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.fs.permission.FsPermission;
 
 import com.google.common.base.Optional;
@@ -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) {
+      AclEntry othersEntry = new AclEntry.Builder().setType(AclEntryType.OTHER).setScope(AclEntryScope.DEFAULT)
+          .setPermission(FsAction.READ_EXECUTE).build();
+
+      // Modify the ACL entries to include the new "others" entry
+      aclEntries.add(othersEntry);
+    }
+    return aclEntries;
+
   }
 
   @Override

diff --git a/...n/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java b/...n/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
@@ -361,8 +361,8 @@ public static void safeSetPathPermission(FileSystem fs, FileStatus file, OwnerAn
       if (targetOwnerAndPermission.getFsPermission() != null) {
         fs.setPermission(path, targetOwnerAndPermission.getFsPermission());
       }
-      if (!ownerAndPermission.getAclEntries().isEmpty()) {
-        fs.setAcl(path, ownerAndPermission.getAclEntries());
+      if (!targetOwnerAndPermission.getAclEntries().isEmpty()) {
+        fs.setAcl(path, targetOwnerAndPermission.getAclEntries());
       }
     } catch (IOException ioe) {
       log.warn("Failed to set permission for directory " + path, ioe);