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 su...@apache.org on 2018/10/24 05:53:01 UTC
[41/50] [abbrv] hadoop git commit: Revert "HADOOP-15836. Review of
AccessControlList. Contributed by BELUGA BEHR."
Revert "HADOOP-15836. Review of AccessControlList. Contributed by BELUGA BEHR."
This reverts commit 00254d7b8c714ae2000d0934d260b23458033529.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/dd268a64
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/dd268a64
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/dd268a64
Branch: refs/heads/HDFS-12943
Commit: dd268a64d36d27dbeeb775b92cb3664f4ed4fbdf
Parents: 296818a
Author: Inigo Goiri <in...@apache.org>
Authored: Tue Oct 23 09:23:03 2018 -0700
Committer: Inigo Goiri <in...@apache.org>
Committed: Tue Oct 23 09:23:03 2018 -0700
----------------------------------------------------------------------
.../security/authorize/AccessControlList.java | 138 +++++++++++--------
.../authorize/TestAccessControlList.java | 135 +++++++++---------
2 files changed, 150 insertions(+), 123 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/dd268a64/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java
index f1ad287..a32a5fe 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java
@@ -20,13 +20,11 @@ package org.apache.hadoop.security.authorize;
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.Set;
-import java.util.TreeSet;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
-import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
@@ -36,6 +34,7 @@ import org.apache.hadoop.io.WritableFactories;
import org.apache.hadoop.io.WritableFactory;
import org.apache.hadoop.security.Groups;
import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.StringUtils;
/**
* Class representing a configured access control list.
@@ -44,7 +43,7 @@ import org.apache.hadoop.security.UserGroupInformation;
@InterfaceStability.Evolving
public class AccessControlList implements Writable {
- static { // register a ctor
+ static { // register a ctor
WritableFactories.setFactory
(AccessControlList.class,
new WritableFactory() {
@@ -58,9 +57,9 @@ public class AccessControlList implements Writable {
private static final int INITIAL_CAPACITY = 256;
// Set of users who are granted access.
- private final Set<String> users = new TreeSet<>();
+ private Collection<String> users;
// Set of groups which are granted access
- private final Set<String> groups = new TreeSet<>();
+ private Collection<String> groups;
// Whether all users are granted access.
private boolean allAllowed;
@@ -74,22 +73,22 @@ public class AccessControlList implements Writable {
/**
* Construct a new ACL from a String representation of the same.
- *
+ *
* The String is a a comma separated list of users and groups.
- * The user list comes first and is separated by a space followed
+ * The user list comes first and is separated by a space followed
* by the group list. For e.g. "user1,user2 group1,group2"
- *
+ *
* @param aclString String representation of the ACL
*/
public AccessControlList(String aclString) {
buildACL(aclString.split(" ", 2));
}
-
+
/**
- * Construct a new ACL from String representation of users and groups.
- *
+ * Construct a new ACL from String representation of users and groups
+ *
* The arguments are comma separated lists
- *
+ *
* @param users comma separated list of users
* @param groups comma separated list of groups
*/
@@ -104,49 +103,49 @@ public class AccessControlList implements Writable {
* @param aclString build ACL from array of Strings
*/
private void buildACL(String[] userGroupStrings) {
+ users = new HashSet<String>();
+ groups = new HashSet<String>();
for (String aclPart : userGroupStrings) {
if (aclPart != null && isWildCardACLValue(aclPart)) {
allAllowed = true;
- return;
- }
- }
- if (userGroupStrings.length >= 1 && userGroupStrings[0] != null) {
- String[] userList = userGroupStrings[0].split(",");
- for (String user : userList) {
- if (StringUtils.isNotBlank(user)) {
- users.add(user.trim());
- }
+ break;
}
}
- if (userGroupStrings.length == 2 && userGroupStrings[1] != null) {
- String[] groupList = userGroupStrings[1].split(",");
- for (String group : groupList) {
- if (StringUtils.isNotBlank(group)) {
- groups.add(group.trim());
- }
+ if (!allAllowed) {
+ if (userGroupStrings.length >= 1 && userGroupStrings[0] != null) {
+ users = StringUtils.getTrimmedStringCollection(userGroupStrings[0]);
+ }
+
+ if (userGroupStrings.length == 2 && userGroupStrings[1] != null) {
+ groups = StringUtils.getTrimmedStringCollection(userGroupStrings[1]);
+ groupsMapping.cacheGroupsAdd(new LinkedList<String>(groups));
}
- groupsMapping.cacheGroupsAdd(new ArrayList<>(groups));
}
}
-
+
/**
- * Checks whether ACL string contains wildcard.
+ * Checks whether ACL string contains wildcard
*
* @param aclString check this ACL string for wildcard
* @return true if ACL string contains wildcard false otherwise
*/
private boolean isWildCardACLValue(String aclString) {
- return WILDCARD_ACL_VALUE.equals(aclString.trim());
+ if (aclString.contains(WILDCARD_ACL_VALUE) &&
+ aclString.trim().equals(WILDCARD_ACL_VALUE)) {
+ return true;
+ }
+ return false;
}
public boolean isAllAllowed() {
return allAllowed;
}
-
+
/**
* Add user to the names of users allowed for this service.
- *
- * @param user The user name
+ *
+ * @param user
+ * The user name
*/
public void addUser(String user) {
if (isWildCardACLValue(user)) {
@@ -159,24 +158,27 @@ public class AccessControlList implements Writable {
/**
* Add group to the names of groups allowed for this service.
- *
- * @param group The group name
+ *
+ * @param group
+ * The group name
*/
public void addGroup(String group) {
if (isWildCardACLValue(group)) {
- throw new IllegalArgumentException(
- "Group " + group + " can not be added");
+ throw new IllegalArgumentException("Group " + group + " can not be added");
}
if (!isAllAllowed()) {
- groupsMapping.cacheGroupsAdd(Collections.singletonList(group));
+ List<String> groupsList = new LinkedList<String>();
+ groupsList.add(group);
+ groupsMapping.cacheGroupsAdd(groupsList);
groups.add(group);
}
}
/**
* Remove user from the names of users allowed for this service.
- *
- * @param user The user name
+ *
+ * @param user
+ * The user name
*/
public void removeUser(String user) {
if (isWildCardACLValue(user)) {
@@ -189,13 +191,14 @@ public class AccessControlList implements Writable {
/**
* Remove group from the names of groups allowed for this service.
- *
- * @param group The group name
+ *
+ * @param group
+ * The group name
*/
public void removeGroup(String group) {
if (isWildCardACLValue(group)) {
- throw new IllegalArgumentException(
- "Group " + group + " can not be removed");
+ throw new IllegalArgumentException("Group " + group
+ + " can not be removed");
}
if (!isAllAllowed()) {
groups.remove(group);
@@ -204,20 +207,18 @@ public class AccessControlList implements Writable {
/**
* Get the names of users allowed for this service.
- *
- * @return an unmodifiable set of user names in alphabetic order.
+ * @return the set of user names. the set must not be modified.
*/
public Collection<String> getUsers() {
- return Collections.unmodifiableSet(users);
+ return users;
}
-
+
/**
* Get the names of user groups allowed for this service.
- *
- * @return an unmodifiable set of group names in alphabetic order.
+ * @return the set of group names. the set must not be modified.
*/
public Collection<String> getGroups() {
- return Collections.unmodifiableSet(groups);
+ return groups;
}
/**
@@ -229,8 +230,7 @@ public class AccessControlList implements Writable {
public final boolean isUserInList(UserGroupInformation ugi) {
if (allAllowed || users.contains(ugi.getShortUserName())) {
return true;
- }
- if (!groups.isEmpty()) {
+ } else if (!groups.isEmpty()) {
for (String group : ugi.getGroups()) {
if (groups.contains(group)) {
return true;
@@ -326,7 +326,7 @@ public class AccessControlList implements Writable {
* @return comma separated list of users
*/
private String getUsersString() {
- return String.join(",", users);
+ return getString(users);
}
/**
@@ -335,6 +335,26 @@ public class AccessControlList implements Writable {
* @return comma separated list of groups
*/
private String getGroupsString() {
- return String.join(",", groups);
+ return getString(groups);
+ }
+
+ /**
+ * Returns comma-separated concatenated single String of all strings of
+ * the given set
+ *
+ * @param strings set of strings to concatenate
+ */
+ private String getString(Collection<String> strings) {
+ StringBuilder sb = new StringBuilder(INITIAL_CAPACITY);
+ boolean first = true;
+ for(String str: strings) {
+ if (!first) {
+ sb.append(",");
+ } else {
+ first = false;
+ }
+ sb.append(str);
+ }
+ return sb.toString();
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/dd268a64/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java
index b8e86fe..7039001 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestAccessControlList.java
@@ -21,11 +21,9 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.verify;
import java.util.Collection;
+import java.util.Iterator;
import java.util.List;
import org.apache.hadoop.classification.InterfaceAudience;
@@ -39,7 +37,9 @@ import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.collect.Iterables;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
@InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"})
@InterfaceStability.Evolving
@@ -148,17 +148,17 @@ public class TestAccessControlList {
assertTrue(jerryLeeLewisGroups.contains("@memphis"));
// allowed because his netgroup is in ACL
- UserGroupInformation elvis =
+ UserGroupInformation elvis =
UserGroupInformation.createRemoteUser("elvis");
assertUserAllowed(elvis, acl);
// allowed because he's in ACL
- UserGroupInformation carlPerkins =
+ UserGroupInformation carlPerkins =
UserGroupInformation.createRemoteUser("carlPerkins");
assertUserAllowed(carlPerkins, acl);
// not allowed because he's not in ACL and has no netgroups
- UserGroupInformation littleRichard =
+ UserGroupInformation littleRichard =
UserGroupInformation.createRemoteUser("littleRichard");
assertUserNotAllowed(littleRichard, acl);
}
@@ -166,16 +166,16 @@ public class TestAccessControlList {
@Test
public void testWildCardAccessControlList() throws Exception {
AccessControlList acl;
-
+
acl = new AccessControlList("*");
assertTrue(acl.isAllAllowed());
-
+
acl = new AccessControlList(" * ");
assertTrue(acl.isAllAllowed());
-
+
acl = new AccessControlList(" *");
assertTrue(acl.isAllAllowed());
-
+
acl = new AccessControlList("* ");
assertTrue(acl.isAllAllowed());
}
@@ -202,14 +202,14 @@ public class TestAccessControlList {
validateGetAclString(acl);
acl = new AccessControlList(" group1,group2");
- assertEquals("Members of the groups [group1, group2] are allowed",
- acl.toString());
+ assertTrue(acl.toString().equals(
+ "Members of the groups [group1, group2] are allowed"));
validateGetAclString(acl);
acl = new AccessControlList("user1,user2 group1,group2");
- assertEquals("Users [user1, user2] and members of the groups "
- + "[group1, group2] are allowed", acl.toString());
-
+ assertTrue(acl.toString().equals(
+ "Users [user1, user2] and " +
+ "members of the groups [group1, group2] are allowed"));
validateGetAclString(acl);
}
@@ -225,45 +225,48 @@ public class TestAccessControlList {
AccessControlList acl;
Collection<String> users;
Collection<String> groups;
-
+
acl = new AccessControlList("drwho tardis");
users = acl.getUsers();
- assertEquals(1, users.size());
- assertEquals("drwho", Iterables.getOnlyElement(users));
+ assertEquals(users.size(), 1);
+ assertEquals(users.iterator().next(), "drwho");
groups = acl.getGroups();
- assertEquals(1, groups.size());
- assertEquals("tardis", Iterables.getOnlyElement(groups));
-
+ assertEquals(groups.size(), 1);
+ assertEquals(groups.iterator().next(), "tardis");
+
acl = new AccessControlList("drwho");
users = acl.getUsers();
- assertEquals(1, users.size());
- assertEquals("drwho", Iterables.getOnlyElement(users));
+ assertEquals(users.size(), 1);
+ assertEquals(users.iterator().next(), "drwho");
groups = acl.getGroups();
- assertEquals(0, groups.size());
-
+ assertEquals(groups.size(), 0);
+
acl = new AccessControlList("drwho ");
users = acl.getUsers();
- assertEquals(1, users.size());
- assertEquals("drwho", Iterables.getOnlyElement(users));
+ assertEquals(users.size(), 1);
+ assertEquals(users.iterator().next(), "drwho");
groups = acl.getGroups();
- assertEquals(0, groups.size());
-
+ assertEquals(groups.size(), 0);
+
acl = new AccessControlList(" tardis");
users = acl.getUsers();
- assertEquals(0, users.size());
+ assertEquals(users.size(), 0);
groups = acl.getGroups();
- assertEquals(1, groups.size());
- assertEquals("tardis", Iterables.getOnlyElement(groups));
+ assertEquals(groups.size(), 1);
+ assertEquals(groups.iterator().next(), "tardis");
+ Iterator<String> iter;
acl = new AccessControlList("drwho,joe tardis, users");
users = acl.getUsers();
- assertEquals(2, users.size());
- assertTrue(users.contains("drwho"));
- assertTrue(users.contains("joe"));
+ assertEquals(users.size(), 2);
+ iter = users.iterator();
+ assertEquals(iter.next(), "drwho");
+ assertEquals(iter.next(), "joe");
groups = acl.getGroups();
- assertEquals(2, groups.size());
- assertTrue(groups.contains("tardis"));
- assertTrue(groups.contains("users"));
+ assertEquals(groups.size(), 2);
+ iter = groups.iterator();
+ assertEquals(iter.next(), "tardis");
+ assertEquals(iter.next(), "users");
}
/**
@@ -278,60 +281,64 @@ public class TestAccessControlList {
assertEquals(0, acl.getUsers().size());
assertEquals(0, acl.getGroups().size());
assertEquals(" ", acl.getAclString());
-
+
acl.addUser("drwho");
users = acl.getUsers();
- assertEquals(1, users.size());
- assertEquals("drwho", Iterables.getOnlyElement(users));
+ assertEquals(users.size(), 1);
+ assertEquals(users.iterator().next(), "drwho");
assertEquals("drwho ", acl.getAclString());
-
+
acl.addGroup("tardis");
groups = acl.getGroups();
- assertEquals(1, groups.size());
- assertEquals("tardis", Iterables.getOnlyElement(groups));
+ assertEquals(groups.size(), 1);
+ assertEquals(groups.iterator().next(), "tardis");
assertEquals("drwho tardis", acl.getAclString());
-
+
acl.addUser("joe");
acl.addGroup("users");
users = acl.getUsers();
- assertEquals(2, users.size());
- assertTrue(users.contains("drwho"));
- assertTrue(users.contains("joe"));
-
+ assertEquals(users.size(), 2);
+ Iterator<String> iter = users.iterator();
+ assertEquals(iter.next(), "drwho");
+ assertEquals(iter.next(), "joe");
groups = acl.getGroups();
- assertEquals(2, groups.size());
- assertTrue(groups.contains("tardis"));
- assertTrue(groups.contains("users"));
+ assertEquals(groups.size(), 2);
+ iter = groups.iterator();
+ assertEquals(iter.next(), "tardis");
+ assertEquals(iter.next(), "users");
+ assertEquals("drwho,joe tardis,users", acl.getAclString());
acl.removeUser("joe");
acl.removeGroup("users");
users = acl.getUsers();
- assertEquals(1, users.size());
- assertEquals("drwho", Iterables.getOnlyElement(users));
+ assertEquals(users.size(), 1);
+ assertFalse(users.contains("joe"));
groups = acl.getGroups();
- assertEquals(1, groups.size());
- assertEquals("tardis", Iterables.getOnlyElement(groups));
+ assertEquals(groups.size(), 1);
+ assertFalse(groups.contains("users"));
assertEquals("drwho tardis", acl.getAclString());
-
+
acl.removeGroup("tardis");
groups = acl.getGroups();
assertEquals(0, groups.size());
+ assertFalse(groups.contains("tardis"));
assertEquals("drwho ", acl.getAclString());
-
+
acl.removeUser("drwho");
assertEquals(0, users.size());
+ assertFalse(users.contains("drwho"));
assertEquals(0, acl.getGroups().size());
assertEquals(0, acl.getUsers().size());
assertEquals(" ", acl.getAclString());
}
-
+
/**
* Tests adding/removing wild card as the user/group.
*/
@Test
public void testAddRemoveWildCard() {
AccessControlList acl = new AccessControlList("drwho tardis");
-
+
Throwable th = null;
try {
acl.addUser(" * ");
@@ -340,7 +347,7 @@ public class TestAccessControlList {
}
assertNotNull(th);
assertTrue(th instanceof IllegalArgumentException);
-
+
th = null;
try {
acl.addGroup(" * ");
@@ -366,7 +373,7 @@ public class TestAccessControlList {
assertNotNull(th);
assertTrue(th instanceof IllegalArgumentException);
}
-
+
/**
* Tests adding user/group to an wild card acl.
*/
@@ -388,7 +395,7 @@ public class TestAccessControlList {
acl.addGroup("tardis");
assertTrue(acl.isAllAllowed());
assertFalse(acl.getAclString().contains("tardis"));
-
+
acl.removeUser("drwho");
assertTrue(acl.isAllAllowed());
assertUserAllowed(drwho, acl);
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org