From f5994e784c341041f1043f4d5e550a7daf019da3 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Sat, 20 Jun 2020 21:39:27 -0700 Subject: [PATCH 01/11] HADOOP-17079. Optimize UGI#getGroups by adding UGI#getGroupsSet. --- .../java/org/apache/hadoop/fs/FileSystem.java | 2 +- .../org/apache/hadoop/io/SecureIOUtils.java | 2 +- .../security/CompositeGroupsMapping.java | 22 +++++ .../security/GroupMappingServiceProvider.java | 10 +++ .../org/apache/hadoop/security/Groups.java | 90 ++++++++++++------- .../security/JniBasedUnixGroupsMapping.java | 18 +++- ...JniBasedUnixGroupsMappingWithFallback.java | 6 ++ ...UnixGroupsNetgroupMappingWithFallback.java | 6 ++ .../hadoop/security/LdapGroupsMapping.java | 57 +++++++++--- .../hadoop/security/NullGroupsMapping.java | 15 ++++ .../security/ShellBasedUnixGroupsMapping.java | 51 +++++------ .../hadoop/security/UserGroupInformation.java | 27 ++++-- .../security/authorize/AccessControlList.java | 10 +-- .../security/TestCompositeGroupMapping.java | 42 +++++++-- .../hadoop/fs/http/server/HttpFSServer.java | 3 +- .../org/apache/hadoop/lib/service/Groups.java | 3 + .../lib/service/security/GroupsService.java | 10 +++ .../fs/http/server/TestHttpFSServer.java | 7 ++ .../service/security/DummyGroupMapping.java | 6 ++ .../router/RouterPermissionChecker.java | 3 +- .../federation/store/records/MountTable.java | 2 +- .../router/TestRouterUserMappings.java | 6 ++ .../hadoop/hdfs/server/datanode/DataNode.java | 3 +- .../server/namenode/FSPermissionChecker.java | 3 +- .../security/TestRefreshUserMappings.java | 6 ++ .../v2/hs/server/TestHSAdminServer.java | 10 +++ .../v2/hs/webapp/TestHsWebServicesAcls.java | 6 ++ .../NetworkTagMappingJsonManager.java | 2 +- .../JavaSandboxLinuxContainerRuntime.java | 8 +- .../placement/PrimaryGroupPlacementRule.java | 10 +-- .../SecondaryGroupExistingPlacementRule.java | 12 +-- .../UserGroupMappingPlacementRule.java | 16 ++-- .../resourcemanager/TestRMAdminService.java | 5 ++ .../scheduler/fair/PeriodGroupsMapping.java | 10 ++- .../scheduler/fair/PrimaryGroupMapping.java | 7 ++ .../scheduler/fair/SimpleGroupsMapping.java | 8 ++ 36 files changed, 381 insertions(+), 123 deletions(-) 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 abb31ed869591..8136993b6c78a 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 @@ -2695,7 +2695,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 9d3c3c1ceeaa7..f14d99227c7cc 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 5040de1e65056..aace62fde5c84 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.Collections; import java.util.Iterator; import java.util.List; @@ -106,6 +107,27 @@ 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..1bae4b2de2bec 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 @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -52,4 +53,13 @@ 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 + */ + public Set getGroupsSet(String user) throws IOException; } 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 b29278bd20751..0d104a317af6c 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 @@ -78,8 +78,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 +168,8 @@ 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 +181,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,17 +203,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 */ 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; + return Collections.unmodifiableSet(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,19 @@ public ListenableFuture> reload(final String key, } backgroundRefreshQueued.incrementAndGet(); - ListenableFuture> listenableFuture = - executorService.submit(new Callable>() { + ListenableFuture> listenableFuture = + executorService.submit(new Callable>() { @Override - public List call() throws Exception { + public Set call() throws Exception { backgroundRefreshQueued.decrementAndGet(); backgroundRefreshRunning.incrementAndGet(); - List results = load(key); + 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 +408,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 +421,7 @@ private List fetchGroupList(String user) throws IOException { LOG.warn("Potential performance problem: getGroups(user=" + user +") " + "took " + deltaMs + " milliseconds."); } - - return groupList; + return Collections.unmodifiableSet(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..f86745a8db841 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 @@ -19,9 +19,9 @@ package org.apache.hadoop.security; import java.io.IOException; -import java.util.Arrays; -import java.util.List; +import java.util.*; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -75,6 +75,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 +97,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 7c53948cc1f98..0f198f3843911 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; @@ -362,7 +363,7 @@ public synchronized List getGroups(String user) { for (int attempt = 1; attempt <= numAttempts; attempt++, atemptsBeforeFailover++) { try { - return doGetGroups(user, groupHierarchyLevels); + return new ArrayList<>(doGetGroups(user, groupHierarchyLevels)); } catch (AuthenticationException e) { switchBindUser(e); } catch (NamingException e) { @@ -458,10 +459,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 +485,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 +504,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 +515,11 @@ List doGetGroups(String user, int goUpHierarchy) if (!results.hasMoreElements()) { LOG.debug("doGetGroups({}) returned no groups because the " + "user is not found.", user); - return Collections.emptyList(); + return Collections.emptySet(); } SearchResult result = results.nextElement(); - List groups = Collections.emptyList(); + Set groups = null; if (useOneQuery) { try { /** @@ -536,7 +533,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 +697,42 @@ public void cacheGroupsAdd(List groups) { // does nothing in this provider of user to groups mapping } + @Override + public Set getGroupsSet(String user) throws IOException { + /* + * 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..8431f11163793 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 null; + } + /** * Returns an empty list. * @param user ignored 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 31f43980552f2..4872318647565 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,13 +18,16 @@ package org.apache.hadoop.security; import java.io.IOException; -import java.util.LinkedList; +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; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; +import org.apache.commons.compress.utils.Lists; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -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 Lists.newArrayList(getUnixGroups(userName).iterator()); } /** @@ -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 8c84a8d31a063..1cb89c2a020f6 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 @@ -1563,11 +1563,11 @@ public String getShortUserName() { } public String getPrimaryGroupName() throws IOException { - List groups = getGroups(); + Collection groups = getGroupsSet(); if (groups.isEmpty()) { throw new IOException("There is no primary group for UGI " + this); } - return groups.get(0); + return groups.iterator().next(); } /** @@ -1680,20 +1680,22 @@ 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(); + Collection groups = getGroupsSet(); return groups.toArray(new String[groups.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. + * Use {@link #getGroupsSet()} */ public List getGroups() { ensureInitialized(); @@ -1705,6 +1707,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..94bb5656a1b4c 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,10 +20,7 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; -import java.util.Collection; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; +import java.util.*; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -231,8 +228,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/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-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 17be09ea1f331..bae9dd19b4053 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. @@ -288,7 +289,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..eacd4990f3652 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; + public 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..967685e625c3c 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)} + */ + @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 a5bbb92f2153b..6739393924e5e 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 @@ -60,9 +60,11 @@ import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; +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; @@ -170,6 +172,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..0db77a5a1c8a3 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,6 +21,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; import org.apache.hadoop.security.GroupMappingServiceProvider; import org.apache.hadoop.test.HadoopUsersConfTestHelper; @@ -47,4 +48,9 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + return null; + } } 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 b1e68b6de8324..46a0b7e665831 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 @@ -126,8 +126,7 @@ public void checkSuperuserPrivilege() throws AccessControlException { } // Is the user a member of the super group? - List groups = ugi.getGroups(); - 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 d1351a340c3cf..282fe6cbb53e2 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/TestRouterUserMappings.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterUserMappings.java index dc7ebbf0d3475..8a9ae161d6978 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 @@ -57,6 +57,7 @@ import java.net.URLDecoder; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; import static org.junit.Assert.assertArrayEquals; @@ -111,6 +112,11 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + return null; + } } @Before 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 2e498e47504e2..d605584d7cc86 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 = callerUgi.getGroups(); - 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 d60098273d738..aa18f1d04c5d3 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; @@ -549,7 +549,6 @@ private boolean hasPermission(INodeAttributes inode, FsAction access) { * - Default entries may be present, but they are ignored during enforcement. * * @param inode INodeAttributes accessed inode - * @param snapshotId int snapshot ID * @param access FsAction requested permission * @param mode FsPermission mode from inode * @param aclFeature AclFeature of inode 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..5761aa1698dbc 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 @@ -35,6 +35,7 @@ import java.net.URLDecoder; import java.util.ArrayList; import java.util.List; +import java.util.Set; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -84,6 +85,11 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + return null; + } } @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..fd3539dd3212a 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 @@ -27,7 +27,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Set; +import com.google.common.collect.ImmutableSet; import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; @@ -91,6 +93,14 @@ public void cacheGroupsRefresh() throws IOException { @Override public void cacheGroupsAdd(List groups) throws IOException { } + + @Override + public Set getGroupsSet(String user) throws IOException { + Set result = + ImmutableSet.of(user + (10 * i + 1) , user + (10 * i +2)); + i++; + return result; + } } @Parameters 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 eba0ce1deeb17..242300b9cbf6b 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 71d9bb78805d9..18f356c151519 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,20 @@ 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(); + while (it.hasNext()) { + String group = it.next(); + if (this.queueManager.getQueue(group) != null) { + secondaryGroup = group; break; } } @@ -180,7 +184,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 e235632f75cf9..32f1ab1d9bb1c 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..b6c50c0b56ecb 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 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 f7648c86d4bb5..0b69a75e920d6 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 com.google.common.collect.ImmutableSet; import org.apache.hadoop.security.GroupMappingServiceProvider; public class SimpleGroupsMapping implements GroupMappingServiceProvider { @@ -38,4 +40,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"); + } } From 68a4062707ae94c039c889db2e962e7c95ea26e3 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Wed, 24 Jun 2020 12:55:50 -0700 Subject: [PATCH 02/11] Address code review feedbacks. --- .../java/org/apache/hadoop/security/Groups.java | 13 +++++-------- .../hadoop/security/JniBasedUnixGroupsMapping.java | 5 ++++- .../apache/hadoop/security/NullGroupsMapping.java | 2 +- .../security/authorize/AccessControlList.java | 6 +++++- .../lib/service/security/DummyGroupMapping.java | 12 +++++++++++- .../federation/router/TestRouterUserMappings.java | 8 +++++++- .../hdfs/server/namenode/FSPermissionChecker.java | 1 + 7 files changed, 34 insertions(+), 13 deletions(-) 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 0d104a317af6c..8e150876a7958 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 @@ -383,14 +383,11 @@ public ListenableFuture> reload(final String key, backgroundRefreshQueued.incrementAndGet(); ListenableFuture> listenableFuture = - executorService.submit(new Callable>() { - @Override - public Set call() throws Exception { - backgroundRefreshQueued.decrementAndGet(); - backgroundRefreshRunning.incrementAndGet(); - Set results = load(key); - return results; - } + executorService.submit(() -> { + backgroundRefreshQueued.decrementAndGet(); + backgroundRefreshRunning.incrementAndGet(); + Set results = load(key); + return results; }); Futures.addCallback(listenableFuture, new FutureCallback>() { @Override 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 f86745a8db841..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 @@ -19,7 +19,10 @@ package org.apache.hadoop.security; import java.io.IOException; -import java.util.*; +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; 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 8431f11163793..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 @@ -43,7 +43,7 @@ public void cacheGroupsAdd(List groups) { */ @Override public Set getGroupsSet(String user) throws IOException { - return null; + return Collections.emptySet(); } /** 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 94bb5656a1b4c..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 @@ -20,7 +20,11 @@ import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; -import java.util.*; +import java.util.Collection; +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; 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 0db77a5a1c8a3..9df018b67ad5d 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 @@ -23,6 +23,7 @@ import java.util.List; import java.util.Set; +import com.google.common.collect.Sets; import org.apache.hadoop.security.GroupMappingServiceProvider; import org.apache.hadoop.test.HadoopUsersConfTestHelper; @@ -51,6 +52,15 @@ public void cacheGroupsAdd(List groups) throws IOException { @Override public Set getGroupsSet(String user) throws IOException { - return null; + 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/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 8a9ae161d6978..887e0a91155ca 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 com.google.common.collect.Sets; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.fs.FileSystem; @@ -115,7 +116,12 @@ public void cacheGroupsAdd(List groups) throws IOException { @Override public Set getGroupsSet(String user) throws IOException { - return null; + 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; } } 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 aa18f1d04c5d3..93fefeea317d0 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 @@ -549,6 +549,7 @@ private boolean hasPermission(INodeAttributes inode, FsAction access) { * - Default entries may be present, but they are ignored during enforcement. * * @param inode INodeAttributes accessed inode + * @param snapshotId int snapshot ID * @param access FsAction requested permission * @param mode FsPermission mode from inode * @param aclFeature AclFeature of inode From 7b2846431cdfd8e7209d84afb7f37955ad26c568 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Thu, 25 Jun 2020 07:47:45 -0700 Subject: [PATCH 03/11] fix checkstyle --- .../apache/hadoop/security/CompositeGroupsMapping.java | 6 ++++-- .../hadoop/security/GroupMappingServiceProvider.java | 2 +- .../main/java/org/apache/hadoop/security/Groups.java | 1 - .../apache/hadoop/security/UserGroupInformation.java | 10 +++++----- .../java/org/apache/hadoop/lib/service/Groups.java | 2 +- .../hadoop/lib/service/security/DummyGroupMapping.java | 3 +-- .../federation/router/RouterPermissionChecker.java | 2 -- .../mapreduce/v2/hs/server/TestHSAdminServer.java | 2 +- 8 files changed, 13 insertions(+), 15 deletions(-) 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 aace62fde5c84..6f799c1542095 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 @@ -120,9 +120,11 @@ public synchronized Set getGroupsSet(String user) throws IOException { user, provider.getClass().getSimpleName(), e.toString()); LOG.debug("Stacktrace: ", e); } - if (groups != null && ! groups.isEmpty()) { + if (groups != null && !groups.isEmpty()) { groupSet.addAll(groups); - if (!combined) break; + if (!combined) { + break; + } } } return groupSet; 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 1bae4b2de2bec..ff6c86d5febf3 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 @@ -61,5 +61,5 @@ public interface GroupMappingServiceProvider { * @return set of group memberships of user * @throws IOException */ - public Set getGroupsSet(String user) throws IOException; + Set getGroupsSet(String user) throws IOException; } 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 8e150876a7958..6343b0b3b781d 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; 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 1cb89c2a020f6..a92b152066fc8 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 @@ -1563,11 +1563,11 @@ public String getShortUserName() { } public String getPrimaryGroupName() throws IOException { - Collection groups = getGroupsSet(); - if (groups.isEmpty()) { + Set groupsSet = getGroupsSet(); + if (groupsSet.isEmpty()) { throw new IOException("There is no primary group for UGI " + this); } - return groups.iterator().next(); + return groupsSet.iterator().next(); } /** @@ -1686,8 +1686,8 @@ private synchronized Credentials getCredentialsInternal() { * fails, it returns an empty list. */ public String[] getGroupNames() { - Collection groups = getGroupsSet(); - return groups.toArray(new String[groups.size()]); + Collection groupsSet = getGroupsSet(); + return groupsSet.toArray(new String[groupsSet.size()]); } /** 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 eacd4990f3652..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 @@ -29,6 +29,6 @@ public interface Groups { public List getGroups(String user) throws IOException; - public Set getGroupsSet(String user) throws IOException; + Set getGroupsSet(String user) throws IOException; } 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 9df018b67ad5d..2693deff7d93a 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 @@ -54,8 +54,7 @@ public void cacheGroupsAdd(List groups) throws IOException { public Set getGroupsSet(String user) throws IOException { if (user.equals("root")) { return Sets.newHashSet("admin"); - } - else if (user.equals("nobody")) { + } else if (user.equals("nobody")) { return Sets.newHashSet("nobody"); } else { String[] groups = HadoopUsersConfTestHelper.getHadoopUserGroups(user); 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 46a0b7e665831..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; 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 fd3539dd3212a..f7bcfc9e506de 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 @@ -97,7 +97,7 @@ public void cacheGroupsAdd(List groups) throws IOException { @Override public Set getGroupsSet(String user) throws IOException { Set result = - ImmutableSet.of(user + (10 * i + 1) , user + (10 * i +2)); + ImmutableSet.of(user + (10 * i + 1), user + (10 * i +2)); i++; return result; } From 7c1bcaa8be2f2161fe6367484cf0e730dd492e30 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Thu, 25 Jun 2020 16:56:16 -0700 Subject: [PATCH 04/11] Fix unit tests --- .../hadoop/security/LdapGroupsMapping.java | 4 ++-- .../security/RuleBasedLdapGroupsMapping.java | 17 +++++++++++++- .../hadoop/security/UserGroupInformation.java | 20 ++++++++++------ .../apache/hadoop/http/TestHttpServer.java | 9 ++++++++ .../hadoop/security/TestGroupsCaching.java | 23 ++++++++++++++----- .../TestRuleBasedLdapGroupsMapping.java | 10 ++++---- ...erRefreshSuperUserGroupsConfiguration.java | 3 +++ .../router/TestRouterUserMappings.java | 7 ++++++ .../security/TestRefreshUserMappings.java | 14 +++++++++-- .../v2/hs/server/TestHSAdminServer.java | 11 ++++++--- 10 files changed, 93 insertions(+), 25 deletions(-) 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 0f198f3843911..c753eceaebd0f 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 @@ -519,7 +519,7 @@ Set doGetGroups(String user, int goUpHierarchy) } SearchResult result = results.nextElement(); - Set groups = null; + Set groups = Collections.emptySet(); if (useOneQuery) { try { /** @@ -698,7 +698,7 @@ public void cacheGroupsAdd(List groups) { } @Override - public Set getGroupsSet(String user) throws IOException { + 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 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 6accf2fdced02..3eb22673c5a31 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 com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; @@ -26,6 +25,7 @@ import org.slf4j.LoggerFactory; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; /** @@ -88,4 +88,19 @@ 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.toSet()); + case TO_LOWER: + return groups.stream().map(StringUtils::toLowerCase).collect( + Collectors.toSet()); + case NONE: + default: + return groups; + } + } + } 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 a92b152066fc8..646b620a06d24 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 @@ -70,6 +70,7 @@ import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; +import org.apache.commons.compress.utils.Lists; import org.apache.hadoop.io.retry.RetryPolicies; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -1483,8 +1484,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) { @@ -1494,17 +1495,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); } } 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/TestGroupsCaching.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java index 46e9f92258502..dbd0e54c72b72 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 Collections.unmodifiableSet(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-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 887e0a91155ca..19d1c436bc827 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 @@ -57,6 +57,7 @@ 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; @@ -203,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(). @@ -234,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/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java index 5761aa1698dbc..e5b7c119a112b 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,9 +34,12 @@ import java.net.URL; import java.net.URLDecoder; import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import com.google.common.collect.Sets; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; @@ -87,8 +90,13 @@ public void cacheGroupsAdd(List groups) throws IOException { } @Override - public Set getGroupsSet(String user) throws IOException { - return null; + 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; } } @@ -202,6 +210,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 f7bcfc9e506de..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,10 +26,10 @@ 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 com.google.common.collect.ImmutableSet; import org.apache.hadoop.HadoopIllegalArgumentException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; @@ -58,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 { @@ -96,8 +97,9 @@ public void cacheGroupsAdd(List groups) throws IOException { @Override public Set getGroupsSet(String user) throws IOException { - Set result = - ImmutableSet.of(user + (10 * i + 1), user + (10 * i +2)); + Set result = new LinkedHashSet<>(); + result.add(user + (10 * i + 1)); + result.add(user + (10 * i +2)); i++; return result; } @@ -199,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 From a0447676fef3d4de5615cc776e923083f55845c4 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Fri, 26 Jun 2020 11:21:41 -0700 Subject: [PATCH 05/11] Fix checkstyle --- .../java/org/apache/hadoop/security/UserGroupInformation.java | 2 -- .../org/apache/hadoop/security/TestRefreshUserMappings.java | 1 - 2 files changed, 3 deletions(-) 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 646b620a06d24..dce12f9a1a7ab 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; @@ -70,7 +69,6 @@ import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; -import org.apache.commons.compress.utils.Lists; import org.apache.hadoop.io.retry.RetryPolicies; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; 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 e5b7c119a112b..5c026d7d77bee 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,7 +34,6 @@ import java.net.URL; import java.net.URLDecoder; import java.util.ArrayList; -import java.util.Arrays; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; From ec7feede7d42f5c3698733f2fb4c9ad46842fd89 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Fri, 26 Jun 2020 14:45:18 -0700 Subject: [PATCH 06/11] findbugs and checkstyle --- .../org/apache/hadoop/security/Groups.java | 4 +- .../hadoop/security/LdapGroupsMapping.java | 59 +++++-------------- 2 files changed, 16 insertions(+), 47 deletions(-) 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 6343b0b3b781d..f28e0c4d3e260 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 @@ -242,7 +242,7 @@ private Set getGroupInternal(final String user) throws IOException { if (staticUserToGroupsMap != null) { Set staticMapping = staticUserToGroupsMap.get(user); if (staticMapping != null) { - return Collections.unmodifiableSet(staticMapping); + return staticMapping; } } @@ -417,7 +417,7 @@ private Set fetchGroupSet(String user) throws IOException { LOG.warn("Potential performance problem: getGroups(user=" + user +") " + "took " + deltaMs + " milliseconds."); } - return Collections.unmodifiableSet(groups); + return groups; } } 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 c753eceaebd0f..3f656990517af 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 @@ -303,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; @@ -321,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. @@ -349,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 new ArrayList<>(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)); } /** From 8dae651fd7b3834aa001a9f97384196ab91127d5 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Fri, 26 Jun 2020 16:45:24 -0700 Subject: [PATCH 07/11] additional unit test fix --- .../test/java/org/apache/hadoop/security/TestGroupsCaching.java | 2 +- .../placement/UserGroupMappingPlacementRule.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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 dbd0e54c72b72..7620068cf67e4 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 @@ -88,7 +88,7 @@ public Set getGroupsSet(String user) throws IOException { if (blackList.contains(user)) { return Collections.emptySet(); } - return Collections.unmodifiableSet(allGroups); + return new LinkedHashSet<>(allGroups); } @Override 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 18f356c151519..a5faefe796cc2 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 @@ -86,6 +86,7 @@ private String getSecondaryGroup(String user) throws IOException { // and position is not guaranteed) and ensure there is queue with // the same name Iterator it = groupsSet.iterator(); + it.next(); while (it.hasNext()) { String group = it.next(); if (this.queueManager.getQueue(group) != null) { From 214ee3da5abf89a6dad04a23d165cb47401b4f57 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Sat, 27 Jun 2020 17:16:02 -0700 Subject: [PATCH 08/11] Fix testGetGroupsToLower --- .../apache/hadoop/security/RuleBasedLdapGroupsMapping.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 3eb22673c5a31..7bcf711622590 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 @@ -24,8 +24,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -93,10 +95,10 @@ public synchronized Set getGroupsSet(String user) { switch (rule) { case TO_UPPER: return groups.stream().map(StringUtils::toUpperCase).collect( - Collectors.toSet()); + Collectors.toCollection(LinkedHashSet::new)); case TO_LOWER: return groups.stream().map(StringUtils::toLowerCase).collect( - Collectors.toSet()); + Collectors.toCollection(LinkedHashSet::new)); case NONE: default: return groups; From dc2a4e08172d9a274f41c44d504a019229b59640 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Sat, 27 Jun 2020 17:18:57 -0700 Subject: [PATCH 09/11] checkstyle --- .../security/RuleBasedLdapGroupsMapping.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) 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 7bcf711622590..e53c32f165980 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 @@ -93,16 +93,15 @@ 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; + 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; } } - } From dfb7c3cb26f0fad4b735b2c2c41c1be08da43848 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Sat, 27 Jun 2020 17:34:24 -0700 Subject: [PATCH 10/11] add deprecated annotation for getGroups() call --- .../src/main/java/org/apache/hadoop/security/Groups.java | 5 +++-- .../apache/hadoop/security/ShellBasedUnixGroupsMapping.java | 4 ++-- .../org/apache/hadoop/security/UserGroupInformation.java | 4 +++- .../apache/hadoop/lib/service/security/GroupsService.java | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) 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 f28e0c4d3e260..961ec7d591924 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 @@ -167,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, "="); @@ -209,7 +208,9 @@ private IOException noGroupsForUser(String user) { * @param user User's name * @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))); 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 4872318647565..f3432a6f91762 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,6 +18,7 @@ package org.apache.hadoop.security; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; @@ -27,7 +28,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; -import org.apache.commons.compress.utils.Lists; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -97,7 +97,7 @@ public String toString() { */ @Override public List getGroups(String userName) throws IOException { - return Lists.newArrayList(getUnixGroups(userName).iterator()); + return new ArrayList(getUnixGroups(userName)); } /** 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 dce12f9a1a7ab..170e6e1a83add 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 @@ -69,6 +69,7 @@ import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; +import org.apache.hadoop.io.SequenceFile; import org.apache.hadoop.io.retry.RetryPolicies; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -1699,8 +1700,9 @@ public String[] getGroupNames() { * 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. - * Use {@link #getGroupsSet()} + * @deprecated Use {@link #getGroupsSet()} instead. */ + @Deprecated public List getGroups() { ensureInitialized(); try { 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 967685e625c3c..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 @@ -52,7 +52,7 @@ public Class getInterface() { } /** - * @deprecated use {@link #getGroupsSet(String)} + * @deprecated use {@link #getGroupsSet(String user)} */ @Deprecated @Override From 2a88387ccf91531b42ac7eef1d2bfd8b2b81c5df Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Mon, 29 Jun 2020 17:07:31 -0700 Subject: [PATCH 11/11] Remove unused imports --- .../org/apache/hadoop/security/RuleBasedLdapGroupsMapping.java | 1 - .../java/org/apache/hadoop/security/UserGroupInformation.java | 1 - 2 files changed, 2 deletions(-) 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 e53c32f165980..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 @@ -27,7 +27,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import java.util.function.Function; import java.util.stream.Collectors; /** 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 170e6e1a83add..5269e5a33061a 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 @@ -69,7 +69,6 @@ import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; -import org.apache.hadoop.io.SequenceFile; import org.apache.hadoop.io.retry.RetryPolicies; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability;