diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedIdMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedIdMapping.java index 28ab2484229..fd362d038d3 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedIdMapping.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedIdMapping.java @@ -75,6 +75,10 @@ public class ShellBasedIdMapping implements IdMappingServiceProvider { private final File staticMappingFile; private StaticMapping staticMapping = null; + // Last time the static map was modified, measured time difference in + // milliseconds since midnight, January 1, 1970 UTC + private long lastModificationTimeStaticMap = 0; + private boolean constructFullMapAtInit = false; // Used for parsing the static mapping file. @@ -88,7 +92,6 @@ public class ShellBasedIdMapping implements IdMappingServiceProvider { // Maps for id to name map. Guarded by this object monitor lock private BiMap uidNameMap = HashBiMap.create(); private BiMap gidNameMap = HashBiMap.create(); - private long lastUpdateTime = 0; // Last time maps were updated /* @@ -118,7 +121,7 @@ public ShellBasedIdMapping(Configuration conf, conf.get(IdMappingConstant.STATIC_ID_MAPPING_FILE_KEY, IdMappingConstant.STATIC_ID_MAPPING_FILE_DEFAULT); staticMappingFile = new File(staticFilePath); - + updateStaticMapping(); updateMaps(); } @@ -290,20 +293,42 @@ private static boolean isInteger(final String s) { return true; } - private synchronized void initStaticMapping() throws IOException { - staticMapping = new StaticMapping( - new HashMap(), new HashMap()); + private synchronized void updateStaticMapping() throws IOException { + final boolean init = (staticMapping == null); + // + // if the static mapping file + // - was modified after last update, load the map again; + // - did not exist but was added since last update, load the map; + // - existed before but deleted since last update, clear the map + // if (staticMappingFile.exists()) { - LOG.info("Using '" + staticMappingFile + "' for static UID/GID mapping..."); - staticMapping = parseStaticMap(staticMappingFile); + // check modification time, reload the file if the last modification + // time changed since prior load. + long lmTime = staticMappingFile.lastModified(); + if (lmTime != lastModificationTimeStaticMap) { + LOG.info(init? "Using " : "Reloading " + "'" + staticMappingFile + + "' for static UID/GID mapping..."); + lastModificationTimeStaticMap = lmTime; + staticMapping = parseStaticMap(staticMappingFile); + } } else { - LOG.info("Not doing static UID/GID mapping because '" + staticMappingFile - + "' does not exist."); + if (init) { + staticMapping = new StaticMapping(new HashMap(), + new HashMap()); + } + if (lastModificationTimeStaticMap != 0 || init) { + // print the following log at initialization or when the static + // mapping file was deleted after prior load + LOG.info("Not doing static UID/GID mapping because '" + + staticMappingFile + "' does not exist."); + } + lastModificationTimeStaticMap = 0; + staticMapping.clear(); } - } + } /* - * Reset the maps to empty. + * Refresh static map, and reset the other maps to empty. * For testing code, a full map may be re-constructed here when the object * was created with constructFullMapAtInit being set to true. */ @@ -314,15 +339,16 @@ synchronized public void updateMaps() throws IOException { if (constructFullMapAtInit) { loadFullMaps(); + // set constructFullMapAtInit to false to allow testing code to + // do incremental update to maps after initial construction + constructFullMapAtInit = false; } else { + updateStaticMapping(); clearNameMaps(); } } synchronized private void loadFullUserMap() throws IOException { - if (staticMapping == null) { - initStaticMapping(); - } BiMap uMap = HashBiMap.create(); if (OS.startsWith("Mac")) { updateMapInternal(uMap, "user", MAC_GET_ALL_USERS_CMD, "\\s+", @@ -336,9 +362,6 @@ synchronized private void loadFullUserMap() throws IOException { } synchronized private void loadFullGroupMap() throws IOException { - if (staticMapping == null) { - initStaticMapping(); - } BiMap gMap = HashBiMap.create(); if (OS.startsWith("Mac")) { @@ -353,7 +376,6 @@ synchronized private void loadFullGroupMap() throws IOException { } synchronized private void loadFullMaps() throws IOException { - initStaticMapping(); loadFullUserMap(); loadFullGroupMap(); } @@ -443,9 +465,7 @@ synchronized private void updateMapIncr(final String name, } boolean updated = false; - if (staticMapping == null) { - initStaticMapping(); - } + updateStaticMapping(); if (OS.startsWith("Linux")) { if (isGrp) { @@ -481,9 +501,7 @@ synchronized private void updateMapIncr(final int id, } boolean updated = false; - if (staticMapping == null) { - initStaticMapping(); - } + updateStaticMapping(); if (OS.startsWith("Linux")) { if (isGrp) { @@ -547,6 +565,15 @@ public StaticMapping(Map uidMapping, this.uidMapping = new PassThroughMap(uidMapping); this.gidMapping = new PassThroughMap(gidMapping); } + + public void clear() { + uidMapping.clear(); + gidMapping.clear(); + } + + public boolean isNonEmpty() { + return uidMapping.size() > 0 || gidMapping.size() > 0; + } } static StaticMapping parseStaticMap(File staticMapFile) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedIdMapping.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedIdMapping.java index 857c7068673..e6e1d73fb98 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedIdMapping.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestShellBasedIdMapping.java @@ -41,6 +41,13 @@ public class TestShellBasedIdMapping { private static final Map EMPTY_PASS_THROUGH_MAP = new PassThroughMap(); + private void createStaticMapFile(final File smapFile, final String smapStr) + throws IOException { + OutputStream out = new FileOutputStream(smapFile); + out.write(smapStr.getBytes()); + out.close(); + } + @Test public void testStaticMapParsing() throws IOException { File tempStaticMapFile = File.createTempFile("nfs-", ".map"); @@ -58,11 +65,10 @@ public void testStaticMapParsing() throws IOException { "gid 12 202\n" + "uid 4294967294 123\n" + "gid 4294967295 321"; - OutputStream out = new FileOutputStream(tempStaticMapFile); - out.write(staticMapFileContents.getBytes()); - out.close(); - StaticMapping parsedMap = ShellBasedIdMapping.parseStaticMap(tempStaticMapFile); - + createStaticMapFile(tempStaticMapFile, staticMapFileContents); + + StaticMapping parsedMap = + ShellBasedIdMapping.parseStaticMap(tempStaticMapFile); assertEquals(10, (int)parsedMap.uidMapping.get(100)); assertEquals(11, (int)parsedMap.uidMapping.get(201)); assertEquals(12, (int)parsedMap.uidMapping.get(301)); @@ -120,6 +126,78 @@ public void testStaticMapping() throws IOException { assertEquals(498, (int)gMap.inverse().get("mapred2")); } + // Test staticMap refreshing + @Test + public void testStaticMapUpdate() throws IOException { + File tempStaticMapFile = File.createTempFile("nfs-", ".map"); + tempStaticMapFile.delete(); + Configuration conf = new Configuration(); + conf.setLong(IdMappingConstant.USERGROUPID_UPDATE_MILLIS_KEY, 1000); + conf.set(IdMappingConstant.STATIC_ID_MAPPING_FILE_KEY, + tempStaticMapFile.getPath()); + + ShellBasedIdMapping refIdMapping = + new ShellBasedIdMapping(conf, true); + ShellBasedIdMapping incrIdMapping = new ShellBasedIdMapping(conf); + + BiMap uidNameMap = refIdMapping.getUidNameMap(); + BiMap gidNameMap = refIdMapping.getGidNameMap(); + + // Force empty map, to see effect of incremental map update of calling + // getUid() + incrIdMapping.clearNameMaps(); + uidNameMap = refIdMapping.getUidNameMap(); + { + BiMap.Entry me = uidNameMap.entrySet().iterator().next(); + Integer id = me.getKey(); + String name = me.getValue(); + + // The static map is empty, so the id found for "name" would be + // the same as "id" + Integer nid = incrIdMapping.getUid(name); + assertEquals(id, nid); + + // Clear map and update staticMap file + incrIdMapping.clearNameMaps(); + Integer rid = id + 10000; + String smapStr = "uid " + rid + " " + id; + createStaticMapFile(tempStaticMapFile, smapStr); + + // Now the id found for "name" should be the id specified by + // the staticMap + nid = incrIdMapping.getUid(name); + assertEquals(rid, nid); + } + + // Force empty map, to see effect of incremental map update of calling + // getGid() + incrIdMapping.clearNameMaps(); + gidNameMap = refIdMapping.getGidNameMap(); + { + BiMap.Entry me = gidNameMap.entrySet().iterator().next(); + Integer id = me.getKey(); + String name = me.getValue(); + + // The static map is empty, so the id found for "name" would be + // the same as "id" + Integer nid = incrIdMapping.getGid(name); + assertEquals(id, nid); + + // Clear map and update staticMap file + incrIdMapping.clearNameMaps(); + Integer rid = id + 10000; + String smapStr = "gid " + rid + " " + id; + // Sleep a bit to avoid that two changes have the same modification time + try {Thread.sleep(1000);} catch (InterruptedException e) {} + createStaticMapFile(tempStaticMapFile, smapStr); + + // Now the id found for "name" should be the id specified by + // the staticMap + nid = incrIdMapping.getGid(name); + assertEquals(rid, nid); + } + } + @Test public void testDuplicates() throws IOException { assumeTrue(!Shell.WINDOWS); diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index e9d6ad2db99..a76f2a8544a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -644,6 +644,9 @@ Release 2.7.0 - UNRELEASED HDFS-7583. Fix findbug in TransferFsImage.java (vinayakumarb) + HDFS-7564. NFS gateway dynamically reload UID/GID mapping file /etc/nfs.map + (Yongjun Zhang via brandonli) + Release 2.6.1 - UNRELEASED INCOMPATIBLE CHANGES