From 9486b5a60519dde146a047f8b61ea9efc4092652 Mon Sep 17 00:00:00 2001 From: Meeth Gala Date: Fri, 1 Sep 2023 10:59:09 -0700 Subject: [PATCH] replace method setAcl with modifyAclEntries while preserving acls --- .../data/management/copy/CopyableFile.java | 31 +------------------ .../FileAwareInputStreamDataWriter.java | 8 +++-- .../FileAwareInputStreamDataWriterTest.java | 2 +- .../util/filesystem/FileSystemDecorator.java | 3 ++ 4 files changed, 11 insertions(+), 33 deletions(-) diff --git a/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java b/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java index fbbcf5b0f63..85fa80f0fe8 100644 --- a/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java +++ b/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java @@ -29,10 +29,7 @@ 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; @@ -444,33 +441,7 @@ public static Map resolveReplicatedAncestorOwnerAndP private static List getAclEntries(FileSystem srcFs, Path path) throws IOException { AclStatus aclStatus = srcFs.getAclStatus(path); - 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 addOthersEntryTypeToAclEntriesIfMissing(List 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; - + return aclStatus.getEntries(); } @Override 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 02b1ea6bd2c..a1338422782 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 @@ -362,7 +362,9 @@ public static void safeSetPathPermission(FileSystem fs, FileStatus file, OwnerAn fs.setPermission(path, targetOwnerAndPermission.getFsPermission()); } if (!targetOwnerAndPermission.getAclEntries().isEmpty()) { - fs.setAcl(path, targetOwnerAndPermission.getAclEntries()); + // 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 @@ private void ensureDirectoryExists(FileSystem fs, Path path, Iterator> pathToAclEntries = new ConcurrentHashMap<>(); @Override - public void setAcl(Path path, List aclEntries) { + public void modifyAclEntries(Path path, List aclEntries) { pathToAclEntries.put(path, aclEntries); } public ImmutableMap> 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 6ae706baa54..02e03c76765 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 @@ public AclStatus getAclStatus(Path path) throws IOException { public void setAcl(Path path, List aclSpec) throws IOException { this.underlyingFs.setAcl(path, aclSpec); } + public void modifyAclEntries(Path path, List aclSpec) throws IOException { + this.underlyingFs.modifyAclEntries(path, aclSpec); + } public FsStatus getStatus() throws java.io.IOException { return this.underlyingFs.getStatus();