diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 03964522c6..4bcdd6e5b6 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -384,6 +384,9 @@ Release 0.23.2 - UNRELEASED HADOOP-8176. Disambiguate the destination of FsShell copies (Daryn Sharp via bobby) + HADOOP-8088. User-group mapping cache incorrectly does negative caching on + transient failures (Khiwal Lee via bobby) + Release 0.23.1 - 2012-02-17 INCOMPATIBLE CHANGES 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 e8be64477f..aea06f1fd2 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 @@ -86,6 +86,9 @@ public List getGroups(String user) throws IOException { // Create and cache user's groups groups = new CachedGroups(impl.getGroups(user)); + if (groups.getGroups().isEmpty()) { + throw new IOException("No groups found for user " + user); + } userToGroupsMap.put(user, groups); if(LOG.isDebugEnabled()) { LOG.debug("Returning fetched groups for '" + user + "'"); 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 new file mode 100644 index 0000000000..b284fe0c6a --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java @@ -0,0 +1,117 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.security; + +import java.io.IOException; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Set; + +import org.junit.Test; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.hadoop.security.Groups; +import org.apache.hadoop.security.ShellBasedUnixGroupsMapping; + + +public class TestGroupsCaching { + public static final Log LOG = LogFactory.getLog(TestGroupsCaching.class); + private static Configuration conf = new Configuration(); + private static String[] myGroups = {"grp1", "grp2"}; + + static { + conf.setClass(CommonConfigurationKeys.HADOOP_SECURITY_GROUP_MAPPING, + FakeGroupMapping.class, + ShellBasedUnixGroupsMapping.class); + } + + public static class FakeGroupMapping extends ShellBasedUnixGroupsMapping { + // any to n mapping + private static Set allGroups = new HashSet(); + private static Set blackList = new HashSet(); + + public List getGroups(String user) throws IOException { + LOG.info("Getting groups for " + user); + if (blackList.contains(user)) { + return new LinkedList(); + } + return new LinkedList(allGroups); + } + + public void cacheGroupsRefresh() throws IOException { + LOG.info("Cache is being refreshed."); + clearBlackList(); + return; + } + + public static void clearBlackList() throws IOException { + LOG.info("Clearing the blacklist"); + blackList.clear(); + } + + public void cacheGroupsAdd(List groups) throws IOException { + LOG.info("Adding " + groups + " to groups."); + allGroups.addAll(groups); + } + + public static void addToBlackList(String user) throws IOException { + LOG.info("Adding " + user + " to the blacklist"); + blackList.add(user); + } + } + + @Test + public void TestGroupsCaching() throws Exception { + Groups groups = new Groups(conf); + groups.cacheGroupsAdd(Arrays.asList(myGroups)); + groups.refresh(); + FakeGroupMapping.clearBlackList(); + FakeGroupMapping.addToBlackList("user1"); + + // regular entry + assertTrue(groups.getGroups("me").size() == 2); + + // this must be cached. blacklisting should have no effect. + FakeGroupMapping.addToBlackList("me"); + assertTrue(groups.getGroups("me").size() == 2); + + // ask for a negative entry + try { + LOG.error("We are not supposed to get here." + groups.getGroups("user1").toString()); + fail(); + } catch (IOException ioe) { + if(!ioe.getMessage().startsWith("No groups found")) { + LOG.error("Got unexpected exception: " + ioe.getMessage()); + fail(); + } + } + + // this shouldn't be cached. remove from the black list and retry. + FakeGroupMapping.clearBlackList(); + assertTrue(groups.getGroups("user1").size() == 2); + } +}