Apache
/
hadoop
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HADOOP-15846. ABFS: fix mask related bugs in setAcl, modifyAclEntries and removeAclEntries. 

Contributed by Junhua Gu.

(cherry picked from commit 66715005f9)
This commit is contained in:
Junhua Gu 2018-11-08 17:21:40 +00:00 committed by Steve Loughran
parent 9317a61f3c
commit 724c15007b
No known key found for this signature in database
GPG Key ID: D22CF846DBB162A0
3 changed files with 225 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java
View File

@ -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);

89
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAclHelper.java
View File

@ -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);
}

147
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFilesystemAcl.java
View File

@ -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;
@ -270,6 +273,67 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
fs.modifyAclEntries(path, aclSpec);
}
@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();
@ -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();
@ -813,6 +921,42 @@ public class ITestAzureBlobFilesystemAcl extends AbstractAbfsIntegrationTest {
fs.setAcl(path, aclSpec);
}
@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();
@ -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());