[gobblin] branch master updated: [GOBBLIN-1892] replace setAcl with modifyEntries while preserving acls during distcp (#3755)

[zihanli58]

This is an automated email from the ASF dual-hosted git repository.

zihanli58 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/gobblin.git


The following commit(s) were added to refs/heads/master by this push:
     new f4a9966da [GOBBLIN-1892] replace setAcl with modifyEntries while 
preserving acls during distcp (#3755)
f4a9966da is described below

commit f4a9966da33b69a6e95d308fc7beb105452b0bdb
Author: meethngala <meethgala...@gmail.com>
AuthorDate: Tue Sep 5 14:52:46 2023 -0700

    [GOBBLIN-1892] replace setAcl with modifyEntries while preserving acls 
during distcp (#3755)
    
    * add AclEntryType.OTHER if absent while preserving acls during distcp
    
    * replace method setAcl with modifyAclEntries while preserving acls
    
    ---------
    
    Co-authored-by: Meeth Gala <mg...@linkedin.com>
---
 .../management/copy/writer/FileAwareInputStreamDataWriter.java | 10 +++++++---
 .../copy/writer/FileAwareInputStreamDataWriterTest.java        |  2 +-
 .../apache/gobblin/util/filesystem/FileSystemDecorator.java    |  3 +++
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git 
a/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
 
b/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
index 2712cc67e..a13384227 100644
--- 
a/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
+++ 
b/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
@@ -361,8 +361,10 @@ public class FileAwareInputStreamDataWriter extends 
InstrumentedDataWriter<FileA
       if (targetOwnerAndPermission.getFsPermission() != null) {
         fs.setPermission(path, targetOwnerAndPermission.getFsPermission());
       }
-      if (!ownerAndPermission.getAclEntries().isEmpty()) {
-        fs.setAcl(path, ownerAndPermission.getAclEntries());
+      if (!targetOwnerAndPermission.getAclEntries().isEmpty()) {
+        // use modify acls instead of setAcl since latter requires all three 
acl entry types: user, group and others
+        // while overwriting the acls for a given path. If anyone is absent it 
fails acl transformation validation.
+        fs.modifyAclEntries(path, targetOwnerAndPermission.getAclEntries());
       }
     } catch (IOException ioe) {
       log.warn("Failed to set permission for directory " + path, ioe);
@@ -519,7 +521,9 @@ public class FileAwareInputStreamDataWriter extends 
InstrumentedDataWriter<FileA
         log.warn("Failed to set owner and/or group for path " + path + " to " 
+ owner + ":" + group, ioe);
       }
       if (!aclEntries.isEmpty()) {
-        fs.setAcl(path, aclEntries);
+        // use modify acls instead of setAcl since latter requires all three 
acl entry types: user, group and others
+        // while overwriting the acls for a given path. If anyone is absent it 
fails acl transformation validation.
+        fs.modifyAclEntries(path, aclEntries);
       }
     } else {
       fs.mkdirs(path);
diff --git 
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriterTest.java
 
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriterTest.java
index 8807de3f8..35b91b766 100644
--- 
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriterTest.java
+++ 
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriterTest.java
@@ -647,7 +647,7 @@ public class FileAwareInputStreamDataWriterTest {
   protected class TestLocalFileSystem extends LocalFileSystem {
     private final ConcurrentHashMap<Path, List<AclEntry>> pathToAclEntries = 
new ConcurrentHashMap<>();
     @Override
-    public void setAcl(Path path, List<AclEntry> aclEntries) {
+    public void modifyAclEntries(Path path, List<AclEntry> aclEntries) {
       pathToAclEntries.put(path, aclEntries);
     }
     public ImmutableMap<Path, List<AclEntry>> getPathToAclEntries() {
diff --git 
a/gobblin-utility/src/main/java/org/apache/gobblin/util/filesystem/FileSystemDecorator.java
 
b/gobblin-utility/src/main/java/org/apache/gobblin/util/filesystem/FileSystemDecorator.java
index 6ae706baa..02e03c767 100644
--- 
a/gobblin-utility/src/main/java/org/apache/gobblin/util/filesystem/FileSystemDecorator.java
+++ 
b/gobblin-utility/src/main/java/org/apache/gobblin/util/filesystem/FileSystemDecorator.java
@@ -110,6 +110,9 @@ class FileSystemDecorator extends FileSystem implements 
Decorator {
   public void setAcl(Path path, List<AclEntry> aclSpec) throws IOException {
     this.underlyingFs.setAcl(path, aclSpec);
   }
+  public void modifyAclEntries(Path path, List<AclEntry> aclSpec) throws 
IOException {
+    this.underlyingFs.modifyAclEntries(path, aclSpec);
+  }
 
   public FsStatus getStatus() throws java.io.IOException {
     return this.underlyingFs.getStatus();