You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ae...@apache.org on 2018/11/08 18:11:40 UTC
[28/29] hadoop git commit: HADOOP-15846. ABFS: fix mask related bugs
in setAcl, modifyAclEntries and removeAclEntries.
HADOOP-15846. ABFS: fix mask related bugs in setAcl, modifyAclEntries and removeAclEntries.
Contributed by Junhua Gu.
(cherry picked from commit 66715005f9e8f4f25faa352a06d142b75a029f0e)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/724c1500
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/724c1500
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/724c1500
Branch: refs/heads/HDDS-4
Commit: 724c15007beb9ca36f2f37c829f1daa366d95bbf
Parents: 9317a61
Author: Junhua Gu <Ju...@microsoft.com>
Authored: Thu Nov 8 17:21:40 2018 +0000
Committer: Steve Loughran <st...@apache.org>
Committed: Thu Nov 8 17:21:40 2018 +0000
----------------------------------------------------------------------
.../fs/azurebfs/AzureBlobFileSystemStore.java | 20 +--
.../fs/azurebfs/services/AbfsAclHelper.java | 89 +++++++++--
.../azurebfs/ITestAzureBlobFilesystemAcl.java | 147 ++++++++++++++++++-
3 files changed, 225 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/724c1500/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java
index 1ac1761..bfdbba8 100644
--- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java
+++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java
@@ -626,17 +626,7 @@ public class AzureBlobFileSystemStore {
final Map<String, String> aclEntries = AbfsAclHelper.deserializeAclSpec(op.getResult().getResponseHeader(HttpHeaderConfigurations.X_MS_ACL));
- for (Map.Entry<String, String> modifyAclEntry : modifyAclEntries.entrySet()) {
- aclEntries.put(modifyAclEntry.getKey(), modifyAclEntry.getValue());
- }
-
- if (!modifyAclEntries.containsKey(AbfsHttpConstants.ACCESS_MASK)) {
- aclEntries.remove(AbfsHttpConstants.ACCESS_MASK);
- }
-
- if (!modifyAclEntries.containsKey(AbfsHttpConstants.DEFAULT_MASK)) {
- aclEntries.remove(AbfsHttpConstants.DEFAULT_MASK);
- }
+ AbfsAclHelper.modifyAclEntriesInternal(aclEntries, modifyAclEntries);
client.setAcl(AbfsHttpConstants.FORWARD_SLASH + getRelativePath(path, true),
AbfsAclHelper.serializeAclSpec(aclEntries), eTag);
@@ -736,12 +726,8 @@ public class AzureBlobFileSystemStore {
final String eTag = op.getResult().getResponseHeader(HttpHeaderConfigurations.ETAG);
final Map<String, String> getAclEntries = AbfsAclHelper.deserializeAclSpec(op.getResult().getResponseHeader(HttpHeaderConfigurations.X_MS_ACL));
- for (Map.Entry<String, String> ace : getAclEntries.entrySet()) {
- if (ace.getKey().startsWith("default:") && (ace.getKey() != AbfsHttpConstants.DEFAULT_MASK)
- && !aclEntries.containsKey(ace.getKey())) {
- aclEntries.put(ace.getKey(), ace.getValue());
- }
- }
+
+ AbfsAclHelper.setAclEntriesInternal(aclEntries, getAclEntries);
client.setAcl(AbfsHttpConstants.FORWARD_SLASH + getRelativePath(path, true),
AbfsAclHelper.serializeAclSpec(aclEntries), eTag);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/724c1500/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAclHelper.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAclHelper.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAclHelper.java
index c28da2c..34959a6 100644
--- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAclHelper.java
+++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAclHelper.java
@@ -44,12 +44,17 @@ public final class AbfsAclHelper {
// not called
}
- public static Map<String, String> deserializeAclSpec(final String aclSpecString) {
+ public static Map<String, String> deserializeAclSpec(final String aclSpecString) throws AzureBlobFileSystemException {
final Map<String, String> aclEntries = new HashMap<>();
- final String[] aclArray = aclSpecString.split(AbfsHttpConstants.COMMA);
- for (String acl : aclArray) {
- int idx = acl.lastIndexOf(AbfsHttpConstants.COLON);
- aclEntries.put(acl.substring(0, idx), acl.substring(idx + 1));
+ final String[] aceArray = aclSpecString.split(AbfsHttpConstants.COMMA);
+ for (String ace : aceArray) {
+ int idx = ace.lastIndexOf(AbfsHttpConstants.COLON);
+ final String key = ace.substring(0, idx);
+ final String val = ace.substring(idx + 1);
+ if (aclEntries.containsKey(key)) {
+ throw new InvalidAclOperationException("Duplicate acl entries are not allowed.");
+ }
+ aclEntries.put(key, val);
}
return aclEntries;
}
@@ -104,12 +109,22 @@ public final class AbfsAclHelper {
}
}
}
+
+ if (removeIndicationSet.contains(AbfsHttpConstants.ACCESS_MASK) && containsNamedAce(aclEntries, false)) {
+ throw new InvalidAclOperationException("Access mask is required when a named access acl is present.");
+ }
+
if (accessAclTouched) {
if (removeIndicationSet.contains(AbfsHttpConstants.ACCESS_MASK)) {
aclEntries.remove(AbfsHttpConstants.ACCESS_MASK);
}
recalculateMask(aclEntries, false);
}
+
+ if (removeIndicationSet.contains(AbfsHttpConstants.DEFAULT_MASK) && containsNamedAce(aclEntries, true)) {
+ throw new InvalidAclOperationException("Default mask is required when a named default acl is present.");
+ }
+
if (defaultAclTouched) {
if (removeIndicationSet.contains(AbfsHttpConstants.DEFAULT_MASK)) {
aclEntries.remove(AbfsHttpConstants.DEFAULT_MASK);
@@ -127,6 +142,50 @@ public final class AbfsAclHelper {
}
}
+ public static void modifyAclEntriesInternal(Map<String, String> aclEntries, Map<String, String> toModifyEntries)
+ throws AzureBlobFileSystemException {
+ boolean namedAccessAclTouched = false;
+ boolean namedDefaultAclTouched = false;
+
+ for (Map.Entry<String, String> toModifyEntry : toModifyEntries.entrySet()) {
+ aclEntries.put(toModifyEntry.getKey(), toModifyEntry.getValue());
+ if (isNamedAce(toModifyEntry.getKey())) {
+ if (isDefaultAce(toModifyEntry.getKey())) {
+ namedDefaultAclTouched = true;
+ } else {
+ namedAccessAclTouched = true;
+ }
+ }
+ }
+
+ if (!toModifyEntries.containsKey(AbfsHttpConstants.ACCESS_MASK) && namedAccessAclTouched) {
+ aclEntries.remove(AbfsHttpConstants.ACCESS_MASK);
+ }
+
+ if (!toModifyEntries.containsKey(AbfsHttpConstants.DEFAULT_MASK) && namedDefaultAclTouched) {
+ aclEntries.remove(AbfsHttpConstants.DEFAULT_MASK);
+ }
+ }
+
+ public static void setAclEntriesInternal(Map<String, String> aclEntries, Map<String, String> getAclEntries)
+ throws AzureBlobFileSystemException {
+ boolean defaultAclTouched = false;
+
+ for (String entryKey : aclEntries.keySet()) {
+ if (isDefaultAce(entryKey)) {
+ defaultAclTouched = true;
+ break;
+ }
+ }
+
+ for (Map.Entry<String, String> ace : getAclEntries.entrySet()) {
+ if (AbfsAclHelper.isDefaultAce(ace.getKey()) && (ace.getKey() != AbfsHttpConstants.DEFAULT_MASK || !defaultAclTouched)
+ && !aclEntries.containsKey(ace.getKey())) {
+ aclEntries.put(ace.getKey(), ace.getValue());
+ }
+ }
+ }
+
private static boolean removeNamedAceAndUpdateSet(String entry, boolean isDefaultAcl, Set<String> removeIndicationSet,
Map<String, String> aclEntries)
throws AzureBlobFileSystemException {
@@ -136,8 +195,7 @@ public final class AbfsAclHelper {
: entryParts[startIndex] + AbfsHttpConstants.COLON;
if ((entry.equals(AbfsHttpConstants.ACCESS_USER) || entry.equals(AbfsHttpConstants.ACCESS_GROUP)
- || entry.equals(AbfsHttpConstants.ACCESS_OTHER))
- && !isNamedAce(entry)) {
+ || entry.equals(AbfsHttpConstants.ACCESS_OTHER))) {
throw new InvalidAclOperationException("Cannot remove user, group or other entry from access ACL.");
}
@@ -154,7 +212,7 @@ public final class AbfsAclHelper {
}
private static void recalculateMask(Map<String, String> aclEntries, boolean isDefaultMask) {
- FsAction umask = FsAction.NONE;
+ FsAction mask = FsAction.NONE;
if (!isExtendAcl(aclEntries, isDefaultMask)) {
return;
}
@@ -163,17 +221,17 @@ public final class AbfsAclHelper {
if (isDefaultMask) {
if ((isDefaultAce(aclEntry.getKey()) && isNamedAce(aclEntry.getKey()))
|| aclEntry.getKey().equals(AbfsHttpConstants.DEFAULT_GROUP)) {
- umask = umask.or(FsAction.getFsAction(aclEntry.getValue()));
+ mask = mask.or(FsAction.getFsAction(aclEntry.getValue()));
}
} else {
if ((!isDefaultAce(aclEntry.getKey()) && isNamedAce(aclEntry.getKey()))
|| aclEntry.getKey().equals(AbfsHttpConstants.ACCESS_GROUP)) {
- umask = umask.or(FsAction.getFsAction(aclEntry.getValue()));
+ mask = mask.or(FsAction.getFsAction(aclEntry.getValue()));
}
}
}
- aclEntries.put(isDefaultMask ? AbfsHttpConstants.DEFAULT_MASK : AbfsHttpConstants.ACCESS_MASK, umask.SYMBOL);
+ aclEntries.put(isDefaultMask ? AbfsHttpConstants.DEFAULT_MASK : AbfsHttpConstants.ACCESS_MASK, mask.SYMBOL);
}
private static boolean isExtendAcl(Map<String, String> aclEntries, boolean checkDefault) {
@@ -192,6 +250,15 @@ public final class AbfsAclHelper {
return false;
}
+ private static boolean containsNamedAce(Map<String, String> aclEntries, boolean checkDefault) {
+ for (String entryKey : aclEntries.keySet()) {
+ if (isNamedAce(entryKey) && (checkDefault == isDefaultAce(entryKey))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private static boolean isDefaultAce(String entry) {
return entry.startsWith(AbfsHttpConstants.DEFAULT_SCOPE);
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/724c1500/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFilesystemAcl.java
----------------------------------------------------------------------
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFilesystemAcl.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFilesystemAcl.java
index 7377132..05f9856 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFilesystemAcl.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFilesystemAcl.java
@@ -30,6 +30,7 @@ import org.junit.Test;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.PathIOException;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.azurebfs.utils.AclTestHelpers;
import org.apache.hadoop.fs.permission.AclEntry;
@@ -54,6 +55,7 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
private static final FsAction ALL = FsAction.ALL;
private static final FsAction NONE = FsAction.NONE;
private static final FsAction READ = FsAction.READ;
+ private static final FsAction EXECUTE = FsAction.EXECUTE;
private static final FsAction READ_EXECUTE = FsAction.READ_EXECUTE;
private static final FsAction READ_WRITE = FsAction.READ_WRITE;
@@ -65,6 +67,7 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
private static final short RWX_RX = 0750;
private static final short RWX_RX_RX = 0755;
private static final short RW_R = 0640;
+ private static final short RW_X = 0610;
private static final short RW_RW = 0660;
private static final short RW_RWX = 0670;
private static final short RW_R_R = 0644;
@@ -271,6 +274,67 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
}
@Test
+ public void testModifyAclEntriesWithDefaultMask() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(DEFAULT, MASK, EXECUTE));
+ fs.setAcl(path, aclSpec);
+
+ List<AclEntry> modifyAclSpec = Lists.newArrayList(
+ aclEntry(DEFAULT, USER, READ_WRITE));
+ fs.modifyAclEntries(path, modifyAclSpec);
+
+ AclStatus s = fs.getAclStatus(path);
+ AclEntry[] returned = s.getEntries().toArray(new AclEntry[0]);
+ assertArrayEquals(new AclEntry[] {
+ aclEntry(DEFAULT, USER, READ_WRITE),
+ aclEntry(DEFAULT, GROUP, READ_EXECUTE),
+ aclEntry(DEFAULT, MASK, EXECUTE),
+ aclEntry(DEFAULT, OTHER, NONE)}, returned);
+ assertPermission(fs, (short) RWX_RX);
+ }
+
+ @Test
+ public void testModifyAclEntriesWithAccessMask() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(ACCESS, MASK, EXECUTE));
+ fs.setAcl(path, aclSpec);
+
+ List<AclEntry> modifyAclSpec = Lists.newArrayList(
+ aclEntry(ACCESS, USER, READ_WRITE));
+ fs.modifyAclEntries(path, modifyAclSpec);
+
+ AclStatus s = fs.getAclStatus(path);
+ AclEntry[] returned = s.getEntries().toArray(new AclEntry[0]);
+ assertArrayEquals(new AclEntry[] {
+ aclEntry(ACCESS, GROUP, READ_EXECUTE)}, returned);
+ assertPermission(fs, (short) RW_X);
+ }
+
+ @Test(expected=PathIOException.class)
+ public void testModifyAclEntriesWithDuplicateEntries() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(ACCESS, MASK, EXECUTE));
+ fs.setAcl(path, aclSpec);
+
+ List<AclEntry> modifyAclSpec = Lists.newArrayList(
+ aclEntry(ACCESS, USER, READ_WRITE),
+ aclEntry(ACCESS, USER, READ_WRITE));
+ fs.modifyAclEntries(path, modifyAclSpec);
+ }
+
+ @Test
public void testRemoveAclEntries() throws Exception {
final AzureBlobFileSystem fs = this.getFileSystem();
assumeTrue(fs.getIsNamespaceEnabled());
@@ -440,6 +504,50 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
fs.removeAclEntries(path, aclSpec);
}
+ @Test(expected=PathIOException.class)
+ public void testRemoveAclEntriesAccessMask() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(ACCESS, MASK, EXECUTE),
+ aclEntry(ACCESS, USER, "foo", ALL));
+ fs.setAcl(path, aclSpec);
+
+ fs.removeAclEntries(path, Lists.newArrayList(aclEntry(ACCESS, MASK, NONE)));
+ }
+
+ @Test(expected=PathIOException.class)
+ public void testRemoveAclEntriesDefaultMask() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(DEFAULT, MASK, EXECUTE),
+ aclEntry(DEFAULT, USER, "foo", ALL));
+ fs.setAcl(path, aclSpec);
+
+ fs.removeAclEntries(path, Lists.newArrayList(aclEntry(DEFAULT, MASK, NONE)));
+ }
+
+ @Test(expected=PathIOException.class)
+ public void testRemoveAclEntriesWithDuplicateEntries() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(DEFAULT, MASK, EXECUTE));
+ fs.setAcl(path, aclSpec);
+
+ List<AclEntry> removeAclSpec = Lists.newArrayList(
+ aclEntry(DEFAULT, USER, READ_WRITE),
+ aclEntry(DEFAULT, USER, READ_WRITE));
+ fs.removeAclEntries(path, removeAclSpec);
+ }
+
@Test
public void testRemoveDefaultAcl() throws Exception {
final AzureBlobFileSystem fs = this.getFileSystem();
@@ -814,6 +922,42 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
}
@Test
+ public void testSetAclDoesNotChangeDefaultMask() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(DEFAULT, MASK, EXECUTE));
+ fs.setAcl(path, aclSpec);
+ // only change access acl, and default mask should not change.
+ List<AclEntry> aclSpec2 = Lists.newArrayList(
+ aclEntry(ACCESS, OTHER, READ_EXECUTE));
+ fs.setAcl(path, aclSpec2);
+ // get acl status and check result.
+ AclStatus s = fs.getAclStatus(path);
+ AclEntry[] returned = s.getEntries().toArray(new AclEntry[0]);
+ assertArrayEquals(new AclEntry[] {
+ aclEntry(DEFAULT, USER, ALL),
+ aclEntry(DEFAULT, GROUP, READ_EXECUTE),
+ aclEntry(DEFAULT, MASK, EXECUTE),
+ aclEntry(DEFAULT, OTHER, NONE) }, returned);
+ assertPermission(fs, (short) RWX_RX_RX);
+ }
+
+ @Test(expected=PathIOException.class)
+ public void testSetAclWithDuplicateEntries() throws Exception {
+ final AzureBlobFileSystem fs = this.getFileSystem();
+ assumeTrue(fs.getIsNamespaceEnabled());
+ path = new Path(testRoot, UUID.randomUUID().toString());
+ FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short) RWX_RX));
+ List<AclEntry> aclSpec = Lists.newArrayList(
+ aclEntry(ACCESS, MASK, EXECUTE),
+ aclEntry(ACCESS, MASK, EXECUTE));
+ fs.setAcl(path, aclSpec);
+ }
+
+ @Test
public void testSetPermission() throws Exception {
final AzureBlobFileSystem fs = this.getFileSystem();
assumeTrue(fs.getIsNamespaceEnabled());
@@ -924,7 +1068,6 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
}
@Test
- @Ignore // wait investigation in service
public void testDefaultMinimalAclNewFile() throws Exception {
final AzureBlobFileSystem fs = this.getFileSystem();
assumeTrue(fs.getIsNamespaceEnabled());
@@ -970,7 +1113,6 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
}
@Test
- @Ignore // wait umask fix to be deployed
public void testOnlyAccessAclNewDir() throws Exception {
final AzureBlobFileSystem fs = this.getFileSystem();
assumeTrue(fs.getIsNamespaceEnabled());
@@ -988,7 +1130,6 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
}
@Test
- @Ignore // wait investigation in service
public void testDefaultMinimalAclNewDir() throws Exception {
final AzureBlobFileSystem fs = this.getFileSystem();
assumeTrue(fs.getIsNamespaceEnabled());
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org