diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java index 528f6c270f4ee..df92ee089724c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java @@ -2747,7 +2747,7 @@ static void checkAccessPermissions(FileStatus stat, FsAction mode) if (perm.getUserAction().implies(mode)) { return; } - } else if (ugi.getGroups().contains(stat.getGroup())) { + } else if (ugi.getGroupsSet().contains(stat.getGroup())) { if (perm.getGroupAction().implies(mode)) { return; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java index 2f1eecd7a6320..e6bd81a0bc8d6 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java @@ -272,7 +272,7 @@ private static void checkStat(File f, String owner, String group, UserGroupInformation.createRemoteUser(expectedOwner); final String adminsGroupString = "Administrators"; success = owner.equals(adminsGroupString) - && ugi.getGroups().contains(adminsGroupString); + && ugi.getGroupsSet().contains(adminsGroupString); } else { success = false; } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java index b762df2acc022..807628aeff2eb 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -107,6 +108,29 @@ public void cacheGroupsAdd(List groups) throws IOException { // does nothing in this provider of user to groups mapping } + @Override + public synchronized Set getGroupsSet(String user) throws IOException { + Set groupSet = new HashSet(); + + Set groups = null; + for (GroupMappingServiceProvider provider : providersList) { + try { + groups = provider.getGroupsSet(user); + } catch (Exception e) { + LOG.warn("Unable to get groups for user {} via {} because: {}", + user, provider.getClass().getSimpleName(), e.toString()); + LOG.debug("Stacktrace: ", e); + } + if (groups != null && !groups.isEmpty()) { + groupSet.addAll(groups); + if (!combined) { + break; + } + } + } + return groupSet; + } + @Override public synchronized Configuration getConf() { return conf; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/GroupMappingServiceProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/GroupMappingServiceProvider.java index 8b90f5bc7af9e..3a9073bbffaba 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/GroupMappingServiceProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/GroupMappingServiceProvider.java @@ -18,7 +18,9 @@ package org.apache.hadoop.security; import java.io.IOException; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -52,4 +54,16 @@ public interface GroupMappingServiceProvider { * @throws IOException */ public void cacheGroupsAdd(List groups) throws IOException; + + /** + * Get all various group memberships of a given user. + * Returns EMPTY set in case of non-existing user + * @param user User's name + * @return set of group memberships of user + * @throws IOException + */ + default Set getGroupsSet(String user) throws IOException { + //Override to form the set directly to avoid another conversion + return new LinkedHashSet<>(getGroups(user)); + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java index 5ff57788a85cb..406d0d0e1507f 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; @@ -78,8 +77,8 @@ public class Groups { private final GroupMappingServiceProvider impl; - private final LoadingCache> cache; - private final AtomicReference>> staticMapRef = + private final LoadingCache> cache; + private final AtomicReference>> staticMapRef = new AtomicReference<>(); private final long cacheTimeout; private final long negativeCacheTimeout; @@ -168,8 +167,7 @@ private void parseStaticMapping(Configuration conf) { CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES_DEFAULT); Collection mappings = StringUtils.getStringCollection( staticMapping, ";"); - Map> staticUserToGroupsMap = - new HashMap>(); + Map> staticUserToGroupsMap = new HashMap<>(); for (String users : mappings) { Collection userToGroups = StringUtils.getStringCollection(users, "="); @@ -181,10 +179,10 @@ private void parseStaticMapping(Configuration conf) { String[] userToGroupsArray = userToGroups.toArray(new String[userToGroups .size()]); String user = userToGroupsArray[0]; - List groups = Collections.emptyList(); + Set groups = Collections.emptySet(); if (userToGroupsArray.length == 2) { - groups = (List) StringUtils - .getStringCollection(userToGroupsArray[1]); + groups = new LinkedHashSet(StringUtils + .getStringCollection(userToGroupsArray[1])); } staticUserToGroupsMap.put(user, groups); } @@ -203,15 +201,47 @@ private IOException noGroupsForUser(String user) { /** * Get the group memberships of a given user. * If the user's group is not cached, this method may block. + * Note this method can be expensive as it involves Set->List conversion. + * For user with large group membership (i.e., > 1000 groups), we recommend + * using getGroupSet to avoid the conversion and fast membership look up via + * contains(). * @param user User's name - * @return the group memberships of the user + * @return the group memberships of the user as list * @throws IOException if user does not exist + * @deprecated Use {@link #getGroupsSet(String user)} instead. */ + @Deprecated public List getGroups(final String user) throws IOException { + return Collections.unmodifiableList(new ArrayList<>( + getGroupInternal(user))); + } + + /** + * Get the group memberships of a given user. + * If the user's group is not cached, this method may block. + * This provide better performance when user has large group membership via + * 1) avoid set->list->set conversion for the caller UGI/PermissionCheck + * 2) fast lookup using contains() via Set instead of List + * @param user User's name + * @return the group memberships of the user as set + * @throws IOException if user does not exist + */ + public Set getGroupsSet(final String user) throws IOException { + return Collections.unmodifiableSet(getGroupInternal(user)); + } + + /** + * Get the group memberships of a given user. + * If the user's group is not cached, this method may block. + * @param user User's name + * @return the group memberships of the user as Set + * @throws IOException if user does not exist + */ + private Set getGroupInternal(final String user) throws IOException { // No need to lookup for groups of static users - Map> staticUserToGroupsMap = staticMapRef.get(); + Map> staticUserToGroupsMap = staticMapRef.get(); if (staticUserToGroupsMap != null) { - List staticMapping = staticUserToGroupsMap.get(user); + Set staticMapping = staticUserToGroupsMap.get(user); if (staticMapping != null) { return staticMapping; } @@ -267,7 +297,7 @@ public long read() { /** * Deals with loading data into the cache. */ - private class GroupCacheLoader extends CacheLoader> { + private class GroupCacheLoader extends CacheLoader> { private ListeningExecutorService executorService; @@ -308,7 +338,7 @@ private class GroupCacheLoader extends CacheLoader> { * @throws IOException to prevent caching negative entries */ @Override - public List load(String user) throws Exception { + public Set load(String user) throws Exception { LOG.debug("GroupCacheLoader - load."); TraceScope scope = null; Tracer tracer = Tracer.curThreadTracer(); @@ -316,9 +346,9 @@ public List load(String user) throws Exception { scope = tracer.newScope("Groups#fetchGroupList"); scope.addKVAnnotation("user", user); } - List groups = null; + Set groups = null; try { - groups = fetchGroupList(user); + groups = fetchGroupSet(user); } finally { if (scope != null) { scope.close(); @@ -334,9 +364,7 @@ public List load(String user) throws Exception { throw noGroupsForUser(user); } - // return immutable de-duped list - return Collections.unmodifiableList( - new ArrayList<>(new LinkedHashSet<>(groups))); + return groups; } /** @@ -345,8 +373,8 @@ public List load(String user) throws Exception { * implementation, otherwise is arranges for the cache to be updated later */ @Override - public ListenableFuture> reload(final String key, - List oldValue) + public ListenableFuture> reload(final String key, + Set oldValue) throws Exception { LOG.debug("GroupCacheLoader - reload (async)."); if (!reloadGroupsInBackground) { @@ -354,19 +382,16 @@ public ListenableFuture> reload(final String key, } backgroundRefreshQueued.incrementAndGet(); - ListenableFuture> listenableFuture = - executorService.submit(new Callable>() { - @Override - public List call() throws Exception { - backgroundRefreshQueued.decrementAndGet(); - backgroundRefreshRunning.incrementAndGet(); - List results = load(key); - return results; - } + ListenableFuture> listenableFuture = + executorService.submit(() -> { + backgroundRefreshQueued.decrementAndGet(); + backgroundRefreshRunning.incrementAndGet(); + Set results = load(key); + return results; }); - Futures.addCallback(listenableFuture, new FutureCallback>() { + Futures.addCallback(listenableFuture, new FutureCallback>() { @Override - public void onSuccess(List result) { + public void onSuccess(Set result) { backgroundRefreshSuccess.incrementAndGet(); backgroundRefreshRunning.decrementAndGet(); } @@ -380,11 +405,12 @@ public void onFailure(Throwable t) { } /** - * Queries impl for groups belonging to the user. This could involve I/O and take awhile. + * Queries impl for groups belonging to the user. + * This could involve I/O and take awhile. */ - private List fetchGroupList(String user) throws IOException { + private Set fetchGroupSet(String user) throws IOException { long startMs = timer.monotonicNow(); - List groupList = impl.getGroups(user); + Set groups = impl.getGroupsSet(user); long endMs = timer.monotonicNow(); long deltaMs = endMs - startMs ; UserGroupInformation.metrics.addGetGroups(deltaMs); @@ -392,8 +418,7 @@ private List fetchGroupList(String user) throws IOException { LOG.warn("Potential performance problem: getGroups(user=" + user +") " + "took " + deltaMs + " milliseconds."); } - - return groupList; + return groups; } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMapping.java index a0f6142a3c5c7..6c24427f3e50e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMapping.java @@ -20,8 +20,11 @@ import java.io.IOException; import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -75,6 +78,18 @@ static private void logError(int groupId, String error) { @Override public List getGroups(String user) throws IOException { + return Arrays.asList(getGroupsInternal(user)); + } + + @Override + public Set getGroupsSet(String user) throws IOException { + String[] groups = getGroupsInternal(user); + Set result = new LinkedHashSet(groups.length); + CollectionUtils.addAll(result, groups); + return result; + } + + private String[] getGroupsInternal(String user) throws IOException { String[] groups = new String[0]; try { groups = getGroupsForUser(user); @@ -85,7 +100,7 @@ public List getGroups(String user) throws IOException { LOG.info("Error getting groups for " + user + ": " + e.getMessage()); } } - return Arrays.asList(groups); + return groups; } @Override diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java index f1644305d917e..cc47df1462678 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import org.apache.hadoop.util.NativeCodeLoader; import org.apache.hadoop.util.PerformanceAdvisory; @@ -61,4 +62,9 @@ public void cacheGroupsAdd(List groups) throws IOException { impl.cacheGroupsAdd(groups); } + @Override + public Set getGroupsSet(String user) throws IOException { + return impl.getGroupsSet(user); + } + } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java index fcc47cb796f33..3d4bd588a5344 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import org.apache.hadoop.util.NativeCodeLoader; import org.slf4j.Logger; @@ -60,4 +61,9 @@ public void cacheGroupsAdd(List groups) throws IOException { impl.cacheGroupsAdd(groups); } + @Override + public Set getGroupsSet(String user) throws IOException { + return impl.getGroupsSet(user); + } + } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java index 3e89c27a78d59..98fd003cfc843 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.Hashtable; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.HashSet; import java.util.Collection; @@ -302,12 +303,12 @@ public class LdapGroupsMapping } private DirContext ctx; - private Configuration conf; + private volatile Configuration conf; - private Iterator ldapUrls; + private volatile Iterator ldapUrls; private String currentLdapUrl; - private boolean useSsl; + private volatile boolean useSsl; private String keystore; private String keystorePass; private String truststore; @@ -320,21 +321,21 @@ public class LdapGroupsMapping private Iterator bindUsers; private BindUserInfo currentBindUser; - private String userbaseDN; + private volatile String userbaseDN; private String groupbaseDN; private String groupSearchFilter; - private String userSearchFilter; - private String memberOfAttr; + private volatile String userSearchFilter; + private volatile String memberOfAttr; private String groupMemberAttr; - private String groupNameAttr; - private int groupHierarchyLevels; - private String posixUidAttr; - private String posixGidAttr; + private volatile String groupNameAttr; + private volatile int groupHierarchyLevels; + private volatile String posixUidAttr; + private volatile String posixGidAttr; private boolean isPosix; - private boolean useOneQuery; + private volatile boolean useOneQuery; private int numAttempts; - private int numAttemptsBeforeFailover; - private String ldapCtxFactoryClassName; + private volatile int numAttemptsBeforeFailover; + private volatile String ldapCtxFactoryClassName; /** * Returns list of groups for a user. @@ -348,38 +349,7 @@ public class LdapGroupsMapping */ @Override public synchronized List getGroups(String user) { - /* - * Normal garbage collection takes care of removing Context instances when - * they are no longer in use. Connections used by Context instances being - * garbage collected will be closed automatically. So in case connection is - * closed and gets CommunicationException, retry some times with new new - * DirContext/connection. - */ - - // Tracks the number of attempts made using the same LDAP server - int atemptsBeforeFailover = 1; - - for (int attempt = 1; attempt <= numAttempts; attempt++, - atemptsBeforeFailover++) { - try { - return doGetGroups(user, groupHierarchyLevels); - } catch (AuthenticationException e) { - switchBindUser(e); - } catch (NamingException e) { - LOG.warn("Failed to get groups for user {} (attempt={}/{}) using {}. " + - "Exception: ", user, attempt, numAttempts, currentLdapUrl, e); - LOG.trace("TRACE", e); - - if (failover(atemptsBeforeFailover, numAttemptsBeforeFailover)) { - atemptsBeforeFailover = 0; - } - } - - // Reset ctx so that new DirContext can be created with new connection - this.ctx = null; - } - - return Collections.emptyList(); + return new ArrayList<>(getGroupsSet(user)); } /** @@ -458,10 +428,10 @@ private NamingEnumeration lookupPosixGroup(SearchResult result, * @return a list of strings representing group names of the user. * @throws NamingException if unable to find group names */ - private List lookupGroup(SearchResult result, DirContext c, + private Set lookupGroup(SearchResult result, DirContext c, int goUpHierarchy) throws NamingException { - List groups = new ArrayList<>(); + Set groups = new LinkedHashSet<>(); Set groupDNs = new HashSet<>(); NamingEnumeration groupResults; @@ -484,11 +454,7 @@ private List lookupGroup(SearchResult result, DirContext c, getGroupNames(groupResult, groups, groupDNs, goUpHierarchy > 0); } if (goUpHierarchy > 0 && !isPosix) { - // convert groups to a set to ensure uniqueness - Set groupset = new HashSet<>(groups); - goUpGroupHierarchy(groupDNs, goUpHierarchy, groupset); - // convert set back to list for compatibility - groups = new ArrayList<>(groupset); + goUpGroupHierarchy(groupDNs, goUpHierarchy, groups); } } return groups; @@ -507,7 +473,7 @@ private List lookupGroup(SearchResult result, DirContext c, * return an empty string array. * @throws NamingException if unable to get group names */ - List doGetGroups(String user, int goUpHierarchy) + Set doGetGroups(String user, int goUpHierarchy) throws NamingException { DirContext c = getDirContext(); @@ -518,11 +484,11 @@ List doGetGroups(String user, int goUpHierarchy) if (!results.hasMoreElements()) { LOG.debug("doGetGroups({}) returned no groups because the " + "user is not found.", user); - return new ArrayList<>(); + return Collections.emptySet(); } SearchResult result = results.nextElement(); - List groups = null; + Set groups = Collections.emptySet(); if (useOneQuery) { try { /** @@ -536,7 +502,7 @@ List doGetGroups(String user, int goUpHierarchy) memberOfAttr + "' attribute." + "Returned user object: " + result.toString()); } - groups = new ArrayList<>(); + groups = new LinkedHashSet<>(); NamingEnumeration groupEnumeration = groupDNAttr.getAll(); while (groupEnumeration.hasMore()) { String groupDN = groupEnumeration.next().toString(); @@ -700,6 +666,42 @@ public void cacheGroupsAdd(List groups) { // does nothing in this provider of user to groups mapping } + @Override + public Set getGroupsSet(String user) { + /* + * Normal garbage collection takes care of removing Context instances when + * they are no longer in use. Connections used by Context instances being + * garbage collected will be closed automatically. So in case connection is + * closed and gets CommunicationException, retry some times with new new + * DirContext/connection. + */ + + // Tracks the number of attempts made using the same LDAP server + int atemptsBeforeFailover = 1; + + for (int attempt = 1; attempt <= numAttempts; attempt++, + atemptsBeforeFailover++) { + try { + return doGetGroups(user, groupHierarchyLevels); + } catch (AuthenticationException e) { + switchBindUser(e); + } catch (NamingException e) { + LOG.warn("Failed to get groups for user {} (attempt={}/{}) using {}. " + + "Exception: ", user, attempt, numAttempts, currentLdapUrl, e); + LOG.trace("TRACE", e); + + if (failover(atemptsBeforeFailover, numAttemptsBeforeFailover)) { + atemptsBeforeFailover = 0; + } + } + + // Reset ctx so that new DirContext can be created with new connection + this.ctx = null; + } + + return Collections.emptySet(); + } + @Override public synchronized Configuration getConf() { return conf; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NullGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NullGroupsMapping.java index f3d048daf990a..9592ecc32c012 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NullGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NullGroupsMapping.java @@ -15,8 +15,10 @@ */ package org.apache.hadoop.security; +import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.Set; /** * This class provides groups mapping for {@link UserGroupInformation} when the @@ -31,6 +33,19 @@ public class NullGroupsMapping implements GroupMappingServiceProvider { public void cacheGroupsAdd(List groups) { } + /** + * Get all various group memberships of a given user. + * Returns EMPTY set in case of non-existing user + * + * @param user User's name + * @return set of group memberships of user + * @throws IOException + */ + @Override + public Set getGroupsSet(String user) throws IOException { + return Collections.emptySet(); + } + /** * Returns an empty list. * @param user ignored diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/RuleBasedLdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/RuleBasedLdapGroupsMapping.java index 6af28f155c466..5fadcc3ced58b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/RuleBasedLdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/RuleBasedLdapGroupsMapping.java @@ -17,7 +17,6 @@ */ package org.apache.hadoop.security; -import org.apache.hadoop.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; @@ -25,7 +24,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; /** @@ -88,4 +89,18 @@ public synchronized List getGroups(String user) { } } + public synchronized Set getGroupsSet(String user) { + Set groups = super.getGroupsSet(user); + switch (rule) { + case TO_UPPER: + return groups.stream().map(StringUtils::toUpperCase).collect( + Collectors.toCollection(LinkedHashSet::new)); + case TO_LOWER: + return groups.stream().map(StringUtils::toLowerCase).collect( + Collectors.toCollection(LinkedHashSet::new)); + case NONE: + default: + return groups; + } + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java index 96e4402e5b9c0..ca9f474cb2670 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java @@ -18,8 +18,11 @@ package org.apache.hadoop.security; import java.io.IOException; -import java.util.LinkedList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.StringTokenizer; import java.util.concurrent.TimeUnit; @@ -53,7 +56,7 @@ public class ShellBasedUnixGroupsMapping extends Configured private long timeout = CommonConfigurationKeys. HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_DEFAULT; - private static final List EMPTY_GROUPS = new LinkedList<>(); + private static final Set EMPTY_GROUPS_SET = Collections.emptySet(); @Override public void setConf(Configuration conf) { @@ -94,7 +97,7 @@ public String toString() { */ @Override public List getGroups(String userName) throws IOException { - return getUnixGroups(userName); + return new ArrayList(getUnixGroups(userName)); } /** @@ -115,6 +118,11 @@ public void cacheGroupsAdd(List groups) throws IOException { // does nothing in this provider of user to groups mapping } + @Override + public Set getGroupsSet(String userName) throws IOException { + return getUnixGroups(userName); + } + /** * Create a ShellCommandExecutor object using the user's name. * @@ -192,44 +200,33 @@ private boolean handleExecutorTimeout( * group is returned first. * @throws IOException if encounter any error when running the command */ - private List getUnixGroups(String user) throws IOException { + private Set getUnixGroups(String user) throws IOException { ShellCommandExecutor executor = createGroupExecutor(user); - List groups; + Set groups; try { executor.execute(); groups = resolveFullGroupNames(executor.getOutput()); } catch (ExitCodeException e) { if (handleExecutorTimeout(executor, user)) { - return EMPTY_GROUPS; + return EMPTY_GROUPS_SET; } else { try { groups = resolvePartialGroupNames(user, e.getMessage(), executor.getOutput()); } catch (PartialGroupNameException pge) { LOG.warn("unable to return groups for user {}", user, pge); - return EMPTY_GROUPS; + return EMPTY_GROUPS_SET; } } } catch (IOException ioe) { if (handleExecutorTimeout(executor, user)) { - return EMPTY_GROUPS; + return EMPTY_GROUPS_SET; } else { // If its not an executor timeout, we should let the caller handle it throw ioe; } } - - // remove duplicated primary group - if (!Shell.WINDOWS) { - for (int i = 1; i < groups.size(); i++) { - if (groups.get(i).equals(groups.get(0))) { - groups.remove(i); - break; - } - } - } - return groups; } @@ -242,13 +239,13 @@ private List getUnixGroups(String user) throws IOException { * @return a linked list of group names * @throws PartialGroupNameException */ - private List parsePartialGroupNames(String groupNames, + private Set parsePartialGroupNames(String groupNames, String groupIDs) throws PartialGroupNameException { StringTokenizer nameTokenizer = new StringTokenizer(groupNames, Shell.TOKEN_SEPARATOR_REGEX); StringTokenizer idTokenizer = new StringTokenizer(groupIDs, Shell.TOKEN_SEPARATOR_REGEX); - List groups = new LinkedList(); + Set groups = new LinkedHashSet<>(); while (nameTokenizer.hasMoreTokens()) { // check for unresolvable group names. if (!idTokenizer.hasMoreTokens()) { @@ -277,10 +274,10 @@ private List parsePartialGroupNames(String groupNames, * @param userName the user's name * @param errMessage error message from the shell command * @param groupNames the incomplete list of group names - * @return a list of resolved group names + * @return a set of resolved group names * @throws PartialGroupNameException if the resolution fails or times out */ - private List resolvePartialGroupNames(String userName, + private Set resolvePartialGroupNames(String userName, String errMessage, String groupNames) throws PartialGroupNameException { // Exception may indicate that some group names are not resolvable. // Shell-based implementation should tolerate unresolvable groups names, @@ -322,16 +319,16 @@ private List resolvePartialGroupNames(String userName, } /** - * Split group names into a linked list. + * Split group names into a set. * * @param groupNames a string representing the user's group names - * @return a linked list of group names + * @return a set of group names */ @VisibleForTesting - protected List resolveFullGroupNames(String groupNames) { + protected Set resolveFullGroupNames(String groupNames) { StringTokenizer tokenizer = new StringTokenizer(groupNames, Shell.TOKEN_SEPARATOR_REGEX); - List groups = new LinkedList(); + Set groups = new LinkedHashSet<>(); while (tokenizer.hasMoreTokens()) { groups.add(tokenizer.nextToken()); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java index 67d151862be4d..4ee177c901835 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java @@ -40,7 +40,6 @@ import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EnumMap; @@ -1516,8 +1515,8 @@ public UserGroupInformation getRealUser() { * map that has the translation of usernames to groups. */ private static class TestingGroups extends Groups { - private final Map> userToGroupsMapping = - new HashMap>(); + private final Map> userToGroupsMapping = + new HashMap<>(); private Groups underlyingImplementation; private TestingGroups(Groups underlyingImplementation) { @@ -1527,17 +1526,22 @@ private TestingGroups(Groups underlyingImplementation) { @Override public List getGroups(String user) throws IOException { - List result = userToGroupsMapping.get(user); - + return new ArrayList<>(getGroupsSet(user)); + } + + @Override + public Set getGroupsSet(String user) throws IOException { + Set result = userToGroupsMapping.get(user); if (result == null) { - result = underlyingImplementation.getGroups(user); + result = underlyingImplementation.getGroupsSet(user); } - return result; } private void setUserGroups(String user, String[] groups) { - userToGroupsMapping.put(user, Arrays.asList(groups)); + Set groupsSet = new LinkedHashSet<>(); + Collections.addAll(groupsSet, groups); + userToGroupsMapping.put(user, groupsSet); } } @@ -1596,11 +1600,11 @@ public String getShortUserName() { } public String getPrimaryGroupName() throws IOException { - List groups = getGroups(); - if (groups.isEmpty()) { + Set groupsSet = getGroupsSet(); + if (groupsSet.isEmpty()) { throw new IOException("There is no primary group for UGI " + this); } - return groups.get(0); + return groupsSet.iterator().next(); } /** @@ -1713,21 +1717,24 @@ private synchronized Credentials getCredentialsInternal() { } /** - * Get the group names for this user. {@link #getGroups()} is less + * Get the group names for this user. {@link #getGroupsSet()} is less * expensive alternative when checking for a contained element. * @return the list of users with the primary group first. If the command * fails, it returns an empty list. */ public String[] getGroupNames() { - List groups = getGroups(); - return groups.toArray(new String[groups.size()]); + Collection groupsSet = getGroupsSet(); + return groupsSet.toArray(new String[groupsSet.size()]); } /** - * Get the group names for this user. + * Get the group names for this user. {@link #getGroupsSet()} is less + * expensive alternative when checking for a contained element. * @return the list of users with the primary group first. If the command * fails, it returns an empty list. + * @deprecated Use {@link #getGroupsSet()} instead. */ + @Deprecated public List getGroups() { ensureInitialized(); try { @@ -1738,6 +1745,21 @@ public List getGroups() { } } + /** + * Get the groups names for the user as a Set. + * @return the set of users with the primary group first. If the command + * fails, it returns an empty set. + */ + public Set getGroupsSet() { + ensureInitialized(); + try { + return groups.getGroupsSet(getShortUserName()); + } catch (IOException ie) { + LOG.debug("Failed to get groups for user {}", getShortUserName(), ie); + return Collections.emptySet(); + } + } + /** * Return the username. */ 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 8af47d6e9d5e9..e86d918b05504 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 @@ -24,6 +24,7 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Set; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -231,8 +232,9 @@ public final boolean isUserInList(UserGroupInformation ugi) { if (allAllowed || users.contains(ugi.getShortUserName())) { return true; } else if (!groups.isEmpty()) { - for (String group : ugi.getGroups()) { - if (groups.contains(group)) { + Set ugiGroups = ugi.getGroupsSet(); + for (String group : groups) { + if (ugiGroups.contains(group)) { return true; } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java index e0c87e93a9ac0..ad9617dca79de 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java @@ -62,8 +62,10 @@ import java.util.Arrays; import java.util.Enumeration; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.concurrent.CountDownLatch; @@ -410,6 +412,13 @@ static void clearMapping() { public List getGroups(String user) throws IOException { return mapping.get(user); } + + @Override + public Set getGroupsSet(String user) throws IOException { + Set result = new HashSet(); + result.addAll(mapping.get(user)); + return result; + } } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestCompositeGroupMapping.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestCompositeGroupMapping.java index 0a2d42c27329a..1803fb1a05806 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestCompositeGroupMapping.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestCompositeGroupMapping.java @@ -22,7 +22,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.hadoop.conf.Configurable; import org.apache.hadoop.conf.Configuration; @@ -87,13 +89,22 @@ public void cacheGroupsRefresh() throws IOException { public void cacheGroupsAdd(List groups) throws IOException { } - + protected List toList(String group) { if (group != null) { return Arrays.asList(new String[] {group}); } return new ArrayList(); } + + protected Set toSet(String group) { + if (group != null) { + Set result = new HashSet<>(); + result.add(group); + return result; + } + return new HashSet(); + } protected void checkTestConf(String expectedValue) { String configValue = getConf().get(PROVIDER_SPECIFIC_CONF_KEY); @@ -106,32 +117,49 @@ protected void checkTestConf(String expectedValue) { private static class UserProvider extends GroupMappingProviderBase { @Override public List getGroups(String user) throws IOException { + return toList(getGroupInternal(user)); + } + + @Override + public Set getGroupsSet(String user) throws IOException { + return toSet(getGroupInternal(user)); + } + + private String getGroupInternal(String user) throws IOException { checkTestConf(PROVIDER_SPECIFIC_CONF_VALUE_FOR_USER); - + String group = null; if (user.equals(john.name)) { group = john.group; } else if (user.equals(jack.name)) { group = jack.group; } - - return toList(group); + return group; } } private static class ClusterProvider extends GroupMappingProviderBase { @Override public List getGroups(String user) throws IOException { + return toList(getGroupsInternal(user)); + } + + @Override + public Set getGroupsSet(String user) throws IOException { + return toSet(getGroupsInternal(user)); + } + + private String getGroupsInternal(String user) throws IOException { checkTestConf(PROVIDER_SPECIFIC_CONF_VALUE_FOR_CLUSTER); - + String group = null; if (user.equals(hdfs.name)) { group = hdfs.group; } else if (user.equals(jack.name)) { // jack has another group from clusterProvider group = jack.group2; } - - return toList(group); + return group; + } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java index ebff93d50d5e1..87788691f6d1b 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java @@ -21,9 +21,9 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; -import java.util.LinkedList; import java.util.List; import java.util.Set; import java.util.concurrent.CountDownLatch; @@ -75,7 +75,7 @@ public static class FakeGroupMapping extends ShellBasedUnixGroupsMapping { private static volatile CountDownLatch latch = null; @Override - public List getGroups(String user) throws IOException { + public Set getGroupsSet(String user) throws IOException { TESTLOG.info("Getting groups for " + user); delayIfNecessary(); @@ -86,9 +86,14 @@ public List getGroups(String user) throws IOException { } if (blackList.contains(user)) { - return new LinkedList(); + return Collections.emptySet(); } - return new LinkedList(allGroups); + return new LinkedHashSet<>(allGroups); + } + + @Override + public List getGroups(String user) throws IOException { + return new ArrayList<>(getGroupsSet(user)); } /** @@ -129,7 +134,7 @@ public static void clearAll() throws IOException { TESTLOG.info("Resetting FakeGroupMapping"); blackList.clear(); allGroups.clear(); - requestCount = 0; + resetRequestCount(); getGroupsDelayMs = 0; throwException = false; latch = null; @@ -197,6 +202,12 @@ public List getGroups(String user) throws IOException { throw new IOException("For test"); } + @Override + public Set getGroupsSet(String user) throws IOException { + requestCount++; + throw new IOException("For test"); + } + public static int getRequestCount() { return requestCount; } @@ -550,7 +561,7 @@ public void testExceptionOnBackgroundRefreshHandled() throws Exception { FakeGroupMapping.clearBlackList(); // We make an initial request to populate the cache - groups.getGroups("me"); + List g1 = groups.getGroups("me"); // add another group groups.cacheGroupsAdd(Arrays.asList("grp3")); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRuleBasedLdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRuleBasedLdapGroupsMapping.java index cd04ae09e3148..8862fd7b60984 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRuleBasedLdapGroupsMapping.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRuleBasedLdapGroupsMapping.java @@ -24,7 +24,9 @@ import javax.naming.NamingException; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import static org.apache.hadoop.security.RuleBasedLdapGroupsMapping .CONVERSION_RULE_KEY; @@ -40,7 +42,7 @@ public class TestRuleBasedLdapGroupsMapping { public void testGetGroupsToUpper() throws NamingException { RuleBasedLdapGroupsMapping groupsMapping = Mockito.spy( new RuleBasedLdapGroupsMapping()); - List groups = new ArrayList<>(); + Set groups = new LinkedHashSet<>(); groups.add("group1"); groups.add("group2"); Mockito.doReturn(groups).when((LdapGroupsMapping) groupsMapping) @@ -61,7 +63,7 @@ public void testGetGroupsToUpper() throws NamingException { public void testGetGroupsToLower() throws NamingException { RuleBasedLdapGroupsMapping groupsMapping = Mockito.spy( new RuleBasedLdapGroupsMapping()); - List groups = new ArrayList<>(); + Set groups = new LinkedHashSet<>(); groups.add("GROUP1"); groups.add("GROUP2"); Mockito.doReturn(groups).when((LdapGroupsMapping) groupsMapping) @@ -82,7 +84,7 @@ public void testGetGroupsToLower() throws NamingException { public void testGetGroupsInvalidRule() throws NamingException { RuleBasedLdapGroupsMapping groupsMapping = Mockito.spy( new RuleBasedLdapGroupsMapping()); - List groups = new ArrayList<>(); + Set groups = new LinkedHashSet<>(); groups.add("group1"); groups.add("GROUP2"); Mockito.doReturn(groups).when((LdapGroupsMapping) groupsMapping) @@ -93,7 +95,7 @@ public void testGetGroupsInvalidRule() throws NamingException { conf.set(CONVERSION_RULE_KEY, "none"); groupsMapping.setConf(conf); - Assert.assertEquals(groups, groupsMapping.getGroups("admin")); + Assert.assertEquals(groups, groupsMapping.getGroupsSet("admin")); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java index 5965f7082fa73..91599227549d0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java @@ -96,6 +96,7 @@ import java.util.EnumSet; import java.util.List; import java.util.Map; +import java.util.Set; /** * Main class of HttpFSServer server. @@ -324,7 +325,7 @@ public InputStream run() throws Exception { case INSTRUMENTATION: { enforceRootPath(op.value(), path); Groups groups = HttpFSServerWebApp.get().get(Groups.class); - List userGroups = groups.getGroups(user.getShortUserName()); + Set userGroups = groups.getGroupsSet(user.getShortUserName()); if (!userGroups.contains(HttpFSServerWebApp.get().getAdminGroup())) { throw new AccessControlException( "User not in HttpFSServer admin group"); diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/Groups.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/Groups.java index 90733f9cdc7e4..2cc942f8e03e5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/Groups.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/Groups.java @@ -22,10 +22,13 @@ import java.io.IOException; import java.util.List; +import java.util.Set; @InterfaceAudience.Private public interface Groups { public List getGroups(String user) throws IOException; + Set getGroupsSet(String user) throws IOException; + } diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/security/GroupsService.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/security/GroupsService.java index 560a3ccf6ebe4..8de0630c9b11b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/security/GroupsService.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/security/GroupsService.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; @InterfaceAudience.Private public class GroupsService extends BaseService implements Groups { @@ -50,9 +51,18 @@ public Class getInterface() { return Groups.class; } + /** + * @deprecated use {@link #getGroupsSet(String user)} + */ + @Deprecated @Override public List getGroups(String user) throws IOException { return hGroups.getGroups(user); } + @Override + public Set getGroupsSet(String user) throws IOException { + return hGroups.getGroupsSet(user); + } + } diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java index 2f0ef9ab23ce7..a06bce3ed539e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java @@ -61,9 +61,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.commons.io.IOUtils; import org.apache.hadoop.conf.Configuration; @@ -189,6 +191,11 @@ public List getGroups(String user) throws IOException { return Arrays.asList(HadoopUsersConfTestHelper.getHadoopUserGroups(user)); } + @Override + public Set getGroupsSet(String user) throws IOException { + return new HashSet<>(getGroups(user)); + } + } private Configuration createHttpFSConf(boolean addDelegationTokenAuthHandler, diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/service/security/DummyGroupMapping.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/service/security/DummyGroupMapping.java index 9ef786db2d3c0..80a94b18d1e51 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/service/security/DummyGroupMapping.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/service/security/DummyGroupMapping.java @@ -21,7 +21,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; +import org.apache.hadoop.thirdparty.com.google.common.collect.Sets; import org.apache.hadoop.security.GroupMappingServiceProvider; import org.apache.hadoop.test.HadoopUsersConfTestHelper; @@ -47,4 +49,17 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + if (user.equals("root")) { + return Sets.newHashSet("admin"); + } else if (user.equals("nobody")) { + return Sets.newHashSet("nobody"); + } else { + String[] groups = HadoopUsersConfTestHelper.getHadoopUserGroups(user); + return (groups != null) ? Sets.newHashSet(groups) : + Collections.emptySet(); + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterPermissionChecker.java index 3d8ef21942110..23a3c6e759e7f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterPermissionChecker.java @@ -18,8 +18,6 @@ package org.apache.hadoop.hdfs.server.federation.router; import java.io.IOException; -import java.util.Arrays; -import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -126,8 +124,7 @@ public void checkSuperuserPrivilege() throws AccessControlException { } // Is the user a member of the super group? - List groups = Arrays.asList(ugi.getGroupNames()); - if (groups.contains(superGroup)) { + if (ugi.getGroupsSet().contains(superGroup)) { return; } diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java index 5d7d5c2966997..907a4055adb82 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java @@ -149,7 +149,7 @@ public static MountTable newInstance(final String src, // Set permission fields UserGroupInformation ugi = NameNode.getRemoteUser(); record.setOwnerName(ugi.getShortUserName()); - String group = ugi.getGroups().isEmpty() ? ugi.getShortUserName() + String group = ugi.getGroupsSet().isEmpty() ? ugi.getShortUserName() : ugi.getPrimaryGroupName(); record.setGroupName(group); record.setMode(new FsPermission( diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRefreshSuperUserGroupsConfiguration.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRefreshSuperUserGroupsConfiguration.java index fb88882243fed..62fcf31cee60d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRefreshSuperUserGroupsConfiguration.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRefreshSuperUserGroupsConfiguration.java @@ -45,6 +45,7 @@ import java.net.URLDecoder; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashSet; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; @@ -135,6 +136,8 @@ private void testRefreshSuperUserGroupsConfigurationInternal( when(ugi.getRealUser()).thenReturn(impersonator); when(ugi.getUserName()).thenReturn("victim"); when(ugi.getGroups()).thenReturn(Arrays.asList("groupVictim")); + when(ugi.getGroupsSet()).thenReturn(new LinkedHashSet<>(Arrays.asList( + "groupVictim"))); // Exception should be thrown before applying config LambdaTestUtils.intercept( diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterUserMappings.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterUserMappings.java index dc7ebbf0d3475..ba8c4639e4d13 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterUserMappings.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterUserMappings.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdfs.server.federation.router; +import org.apache.hadoop.thirdparty.com.google.common.collect.Sets; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.fs.FileSystem; @@ -56,7 +57,9 @@ import java.net.URL; import java.net.URLDecoder; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertArrayEquals; @@ -111,6 +114,16 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + LOG.info("Getting groups in MockUnixGroupsMapping"); + String g1 = user + (10 * i + 1); + String g2 = user + (10 * i + 2); + Set s = Sets.newHashSet(g1, g2); + i++; + return s; + } } @Before @@ -191,6 +204,10 @@ private void testRefreshSuperUserGroupsConfigurationInternal( final List groupNames2 = new ArrayList<>(); groupNames2.add("gr3"); groupNames2.add("gr4"); + final Set groupNamesSet1 = new LinkedHashSet<>(); + groupNamesSet1.addAll(groupNames1); + final Set groupNamesSet2 = new LinkedHashSet<>(); + groupNamesSet2.addAll(groupNames2); //keys in conf String userKeyGroups = DefaultImpersonationProvider.getTestProvider(). @@ -222,6 +239,8 @@ private void testRefreshSuperUserGroupsConfigurationInternal( // set groups for users when(ugi1.getGroups()).thenReturn(groupNames1); when(ugi2.getGroups()).thenReturn(groupNames2); + when(ugi1.getGroupsSet()).thenReturn(groupNamesSet1); + when(ugi2.getGroupsSet()).thenReturn(groupNamesSet2); // check before refresh LambdaTestUtils.intercept(AuthorizationException.class, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java index 079dda43b35c6..23f0a973f4e90 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java @@ -1082,8 +1082,7 @@ private void checkSuperuserPrivilege() throws IOException, AccessControlExceptio } // Is the user a member of the super group? - List groups = Arrays.asList(callerUgi.getGroupNames()); - if (groups.contains(supergroup)) { + if (callerUgi.getGroupsSet().contains(supergroup)) { return; } // Not a superuser. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java index a539bf6f17627..a83ec51529b50 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java @@ -103,7 +103,7 @@ protected FSPermissionChecker(String fsOwner, String supergroup, this.fsOwner = fsOwner; this.supergroup = supergroup; this.callerUgi = callerUgi; - this.groups = callerUgi.getGroups(); + this.groups = callerUgi.getGroupsSet(); user = callerUgi.getShortUserName(); isSuper = user.equals(fsOwner) || groups.contains(supergroup); this.attributeProvider = attributeProvider; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java index 2d7410a405cc9..890c6fb3ebc79 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java @@ -34,8 +34,11 @@ import java.net.URL; import java.net.URLDecoder; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; +import org.apache.hadoop.thirdparty.com.google.common.collect.Sets; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; @@ -84,6 +87,16 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) { + LOG.info("Getting groups in MockUnixGroupsMapping"); + String g1 = user + (10 * i + 1); + String g2 = user + (10 * i + 2); + Set s = Sets.newHashSet(g1, g2); + i++; + return s; + } } @Before @@ -196,6 +209,8 @@ public void testRefreshSuperUserGroupsConfiguration() throws Exception { // set groups for users when(ugi1.getGroups()).thenReturn(groupNames1); when(ugi2.getGroups()).thenReturn(groupNames2); + when(ugi1.getGroupsSet()).thenReturn(new LinkedHashSet<>(groupNames1)); + when(ugi2.getGroupsSet()).thenReturn(new LinkedHashSet<>(groupNames2)); // check before diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/server/TestHSAdminServer.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/server/TestHSAdminServer.java index 1eb1d1c58d369..b961a23c723d0 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/server/TestHSAdminServer.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/server/TestHSAdminServer.java @@ -26,7 +26,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.conf.Configuration; @@ -56,6 +58,7 @@ import org.apache.hadoop.security.authorize.AuthorizationException; import org.apache.hadoop.yarn.logaggregation.AggregatedLogDeletionService; +import org.mockito.internal.util.collections.Sets; @RunWith(Parameterized.class) public class TestHSAdminServer { @@ -91,6 +94,15 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + Set result = new LinkedHashSet<>(); + result.add(user + (10 * i + 1)); + result.add(user + (10 * i +2)); + i++; + return result; + } } @Parameters @@ -189,6 +201,9 @@ public void testRefreshSuperUserGroups() throws Exception { when(superUser.getUserName()).thenReturn("superuser"); when(ugi.getGroups()) .thenReturn(Arrays.asList(new String[] { "group3" })); + when(ugi.getGroupsSet()) + .thenReturn(Sets.newSet("group3")); + when(ugi.getUserName()).thenReturn("regularUser"); // Set super user groups not to include groups of regularUser diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/webapp/TestHsWebServicesAcls.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/webapp/TestHsWebServicesAcls.java index 960993ed7f706..8d4f635e11d68 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/webapp/TestHsWebServicesAcls.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/webapp/TestHsWebServicesAcls.java @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -276,6 +277,11 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + return Collections.emptySet(); + } } private static class MockJobForAcls implements Job { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/NetworkTagMappingJsonManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/NetworkTagMappingJsonManager.java index e59454b9394e2..9fd293c257933 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/NetworkTagMappingJsonManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/NetworkTagMappingJsonManager.java @@ -86,7 +86,7 @@ public String getNetworkTagHexID(Container container) { container.getUser()); List groups = this.networkTagMapping.getGroups(); for(Group group : groups) { - if (userUGI.getGroups().contains(group.getGroupName())) { + if (userUGI.getGroupsSet().contains(group.getGroupName())) { return group.getNetworkTagID(); } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/JavaSandboxLinuxContainerRuntime.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/JavaSandboxLinuxContainerRuntime.java index b4ea66dde2c09..0a25d105b10cb 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/JavaSandboxLinuxContainerRuntime.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/JavaSandboxLinuxContainerRuntime.java @@ -303,9 +303,9 @@ public boolean isRuntimeRequested(Map env) { private static List getGroupPolicyFiles(Configuration conf, String user) throws ContainerExecutionException { Groups groups = Groups.getUserToGroupsMappingService(conf); - List userGroups; + Set userGroups; try { - userGroups = groups.getGroups(user); + userGroups = groups.getGroupsSet(user); } catch (IOException e) { throw new ContainerExecutionException("Container user does not exist"); } @@ -330,11 +330,11 @@ private boolean isSandboxContainerWhitelisted(String username, String whitelistGroup = configuration.get( YarnConfiguration.YARN_CONTAINER_SANDBOX_WHITELIST_GROUP); Groups groups = Groups.getUserToGroupsMappingService(configuration); - List userGroups; + Set userGroups; boolean isWhitelisted = false; try { - userGroups = groups.getGroups(username); + userGroups = groups.getGroupsSet(username); } catch (IOException e) { throw new ContainerExecutionException("Container user does not exist"); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/PrimaryGroupPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/PrimaryGroupPlacementRule.java index 73e5cd0148473..948194f4dbb08 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/PrimaryGroupPlacementRule.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/PrimaryGroupPlacementRule.java @@ -30,7 +30,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.util.List; +import java.util.Set; import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.DOT; import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.assureRoot; @@ -62,19 +62,19 @@ public ApplicationPlacementContext getPlacementForApp( // All users should have at least one group the primary group. If no groups // are returned then there is a real issue. - final List groupList; + final Set groupSet; try { - groupList = groupProvider.getGroups(user); + groupSet = groupProvider.getGroupsSet(user); } catch (IOException ioe) { throw new YarnException("Group resolution failed", ioe); } - if (groupList.isEmpty()) { + if (groupSet.isEmpty()) { LOG.error("Group placement rule failed: No groups returned for user {}", user); throw new YarnException("No groups returned for user " + user); } - String cleanGroup = cleanName(groupList.get(0)); + String cleanGroup = cleanName(groupSet.iterator().next()); String queueName; PlacementRule parentRule = getParentRule(); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/SecondaryGroupExistingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/SecondaryGroupExistingPlacementRule.java index 9acdbccc32ef9..8e6ccb3413e78 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/SecondaryGroupExistingPlacementRule.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/SecondaryGroupExistingPlacementRule.java @@ -30,7 +30,8 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.util.List; +import java.util.Iterator; +import java.util.Set; import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.DOT; import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.assureRoot; @@ -65,9 +66,9 @@ public ApplicationPlacementContext getPlacementForApp( // All users should have at least one group the primary group. If no groups // are returned then there is a real issue. - final List groupList; + final Set groupSet; try { - groupList = groupProvider.getGroups(user); + groupSet = groupProvider.getGroupsSet(user); } catch (IOException ioe) { throw new YarnException("Group resolution failed", ioe); } @@ -90,8 +91,9 @@ public ApplicationPlacementContext getPlacementForApp( parentQueue); } // now check the groups inside the parent - for (int i = 1; i < groupList.size(); i++) { - String group = cleanName(groupList.get(i)); + Iterator it = groupSet.iterator(); + while (it.hasNext()) { + String group = cleanName(it.next()); String queueName = parentQueue == null ? assureRoot(group) : parentQueue + DOT + group; if (configuredQueue(queueName)) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java index 8eb912bca7c72..c793c764a3869 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java @@ -20,7 +20,9 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; +import java.util.Set; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.classification.InterfaceAudience.Private; @@ -74,18 +76,21 @@ public UserGroupMappingPlacementRule(){ } private String getPrimaryGroup(String user) throws IOException { - return groups.getGroups(user).get(0); + return groups.getGroupsSet(user).iterator().next(); } private String getSecondaryGroup(String user) throws IOException { - List groupsList = groups.getGroups(user); + Set groupsSet = groups.getGroupsSet(user); String secondaryGroup = null; // Traverse all secondary groups (as there could be more than one // and position is not guaranteed) and ensure there is queue with // the same name - for (int i = 1; i < groupsList.size(); i++) { - if (this.queueManager.getQueue(groupsList.get(i)) != null) { - secondaryGroup = groupsList.get(i); + Iterator it = groupsSet.iterator(); + it.next(); + while (it.hasNext()) { + String group = it.next(); + if (this.queueManager.getQueue(group) != null) { + secondaryGroup = group; break; } } @@ -180,7 +185,7 @@ private ApplicationPlacementContext getPlacementForUser(String user) } } if (mapping.getType().equals(MappingType.GROUP)) { - for (String userGroups : groups.getGroups(user)) { + for (String userGroups : groups.getGroupsSet(user)) { if (userGroups.equals(mapping.getSource())) { if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) { if (LOG.isDebugEnabled()) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java index 699d975eff09f..bc540b0ba7f32 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java @@ -1459,6 +1459,11 @@ public void cacheGroupsAdd(List groups) throws IOException { // Do nothing } + @Override + public Set getGroupsSet(String user) throws IOException { + return ImmutableSet.copyOf(group); + } + public static void updateGroups() { group.clear(); group.add("test_group_D"); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PeriodGroupsMapping.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PeriodGroupsMapping.java index 9586381d97b5a..cb24799240582 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PeriodGroupsMapping.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PeriodGroupsMapping.java @@ -18,17 +18,20 @@ package org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair; +import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableSet; import org.apache.hadoop.security.GroupMappingServiceProvider; import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Set; public class PeriodGroupsMapping implements GroupMappingServiceProvider { @Override public List getGroups(String user) { - return Arrays.asList(user + ".group", user + "subgroup1", user + "subgroup2"); + return Arrays.asList(user + ".group", user + "subgroup1", + user + "subgroup2"); } @Override @@ -41,4 +44,9 @@ public void cacheGroupsAdd(List groups) throws IOException { throw new UnsupportedOperationException(); } + @Override + public Set getGroupsSet(String user) throws IOException { + return ImmutableSet.of(user + ".group", user + "subgroup1", + user + "subgroup2"); + } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PrimaryGroupMapping.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PrimaryGroupMapping.java index 11415b0f7571f..a34ca8bb24ab8 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PrimaryGroupMapping.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PrimaryGroupMapping.java @@ -22,7 +22,9 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Set; /** * Group Mapping class used for test cases. Returns only primary group of the @@ -44,4 +46,9 @@ public void cacheGroupsRefresh() throws IOException { public void cacheGroupsAdd(List groups) throws IOException { throw new UnsupportedOperationException(); } + + @Override + public Set getGroupsSet(String user) throws IOException { + return Collections.singleton(user + "group"); + } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/SimpleGroupsMapping.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/SimpleGroupsMapping.java index 9c916e36418bd..129df0e41b2a4 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/SimpleGroupsMapping.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/SimpleGroupsMapping.java @@ -21,7 +21,9 @@ import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Set; +import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableSet; import org.apache.hadoop.security.GroupMappingServiceProvider; public class SimpleGroupsMapping implements GroupMappingServiceProvider { @@ -45,4 +47,10 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + return ImmutableSet.of(user + "group", user + "subgroup1", + user + "subgroup2"); + } }