YARN-88. DefaultContainerExecutor can fail to set proper permissions. (Contributed by Jason Lowe)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1388580 13f79535-47bb-0310-9956-ffa450edef68

hadoop-yarn-project/CHANGES.txt

@@ -103,6 +103,9 @@ Release 0.23.4 - UNRELEASED
 
   BUG FIXES
 
+    YARN-88. DefaultContainerExecutor can fail to set proper permissions.
+    (Jason Lowe via sseth)
+
 Release 0.23.3 - Unreleased 
 
   INCOMPATIBLE CHANGES

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DefaultContainerExecutor.java

@@ -109,6 +109,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       String userName, String appId, Path containerWorkDir,
       List<String> localDirs, List<String> logDirs) throws IOException {
 
+    FsPermission dirPerm = new FsPermission(APPDIR_PERM);
     ContainerId containerId = container.getContainerID();
 
     // create container dirs on all disks
@@ -123,7 +124,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       Path appCacheDir = new Path(userdir, ContainerLocalizer.APPCACHE);
       Path appDir = new Path(appCacheDir, appIdStr);
       Path containerDir = new Path(appDir, containerIdStr);
-      lfs.mkdir(containerDir, null, false);
+      createDir(containerDir, dirPerm, false);
     }
 
     // Create the container log-dirs on all disks
@@ -131,7 +132,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
 
     Path tmpDir = new Path(containerWorkDir,
         YarnConfiguration.DEFAULT_CONTAINER_TEMP_DIR);
-    lfs.mkdir(tmpDir, null, false);
+    createDir(tmpDir, dirPerm, false);
 
     // copy launch script to work dir
     Path launchDst =
@@ -286,20 +287,20 @@ public class DefaultContainerExecutor extends ContainerExecutor {
   }
 
   /** Permissions for user dir.
-   * $loaal.dir/usercache/$user */
+   * $local.dir/usercache/$user */
-  private static final short USER_PERM = (short)0750;
+  static final short USER_PERM = (short)0750;
   /** Permissions for user appcache dir.
-   * $loaal.dir/usercache/$user/appcache */
+   * $local.dir/usercache/$user/appcache */
-  private static final short APPCACHE_PERM = (short)0710;
+  static final short APPCACHE_PERM = (short)0710;
   /** Permissions for user filecache dir.
-   * $loaal.dir/usercache/$user/filecache */
+   * $local.dir/usercache/$user/filecache */
-  private static final short FILECACHE_PERM = (short)0710;
+  static final short FILECACHE_PERM = (short)0710;
   /** Permissions for user app dir.
-   * $loaal.dir/usercache/$user/filecache */
+   * $local.dir/usercache/$user/appcache/$appId */
-  private static final short APPDIR_PERM = (short)0710;
+  static final short APPDIR_PERM = (short)0710;
   /** Permissions for user log dir.
    * $logdir/$user/$appId */
-  private static final short LOGDIR_PERM = (short)0710;
+  static final short LOGDIR_PERM = (short)0710;
 
   private Path getFirstApplicationDir(List<String> localDirs, String user,
       String appId) {
@@ -324,20 +325,28 @@ public class DefaultContainerExecutor extends ContainerExecutor {
         ContainerLocalizer.FILECACHE);
   }
 
+  private void createDir(Path dirPath, FsPermission perms,
+      boolean createParent) throws IOException {
+    lfs.mkdir(dirPath, perms, createParent);
+    if (!perms.equals(perms.applyUMask(lfs.getUMask()))) {
+      lfs.setPermission(dirPath, perms);
+    }
+  }
+
   /**
    * Initialize the local directories for a particular user.
-   * <ul>
+   * <ul>.mkdir
    * <li>$local.dir/usercache/$user</li>
    * </ul>
    */
-  private void createUserLocalDirs(List<String> localDirs, String user)
+  void createUserLocalDirs(List<String> localDirs, String user)
       throws IOException {
     boolean userDirStatus = false;
     FsPermission userperms = new FsPermission(USER_PERM);
     for (String localDir : localDirs) {
       // create $local.dir/usercache/$user and its immediate parent
       try {
-        lfs.mkdir(getUserCacheDir(new Path(localDir), user), userperms, true);
+        createDir(getUserCacheDir(new Path(localDir), user), userperms, true);
       } catch (IOException e) {
         LOG.warn("Unable to create the user directory : " + localDir, e);
         continue;
@@ -359,7 +368,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
    * <li>$local.dir/usercache/$user/filecache</li>
    * </ul>
    */
-  private void createUserCacheDirs(List<String> localDirs, String user)
+  void createUserCacheDirs(List<String> localDirs, String user)
       throws IOException {
     LOG.info("Initializing user " + user);
 
@@ -373,7 +382,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       Path localDirPath = new Path(localDir);
       final Path appDir = getAppcacheDir(localDirPath, user);
       try {
-        lfs.mkdir(appDir, appCachePerms, true);
+        createDir(appDir, appCachePerms, true);
         appcacheDirStatus = true;
       } catch (IOException e) {
         LOG.warn("Unable to create app cache directory : " + appDir, e);
@@ -381,7 +390,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       // create $local.dir/usercache/$user/filecache
       final Path distDir = getFileCacheDir(localDirPath, user);
       try {
-        lfs.mkdir(distDir, fileperms, true);
+        createDir(distDir, fileperms, true);
         distributedCacheDirStatus = true;
       } catch (IOException e) {
         LOG.warn("Unable to create file cache directory : " + distDir, e);
@@ -406,7 +415,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
    * </ul>
    * @param localDirs 
    */
-  private void createAppDirs(List<String> localDirs, String user, String appId)
+  void createAppDirs(List<String> localDirs, String user, String appId)
       throws IOException {
     boolean initAppDirStatus = false;
     FsPermission appperms = new FsPermission(APPDIR_PERM);
@@ -414,7 +423,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       Path fullAppDir = getApplicationDir(new Path(localDir), user, appId);
       // create $local.dir/usercache/$user/appcache/$appId
       try {
-        lfs.mkdir(fullAppDir, appperms, true);
+        createDir(fullAppDir, appperms, true);
         initAppDirStatus = true;
       } catch (IOException e) {
         LOG.warn("Unable to create app directory " + fullAppDir.toString(), e);
@@ -430,7 +439,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
   /**
    * Create application log directories on all disks.
    */
-  private void createAppLogDirs(String appId, List<String> logDirs)
+  void createAppLogDirs(String appId, List<String> logDirs)
       throws IOException {
 
     boolean appLogDirStatus = false;
@@ -439,7 +448,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       // create $log.dir/$appid
       Path appLogDir = new Path(rootLogDir, appId);
       try {
-        lfs.mkdir(appLogDir, appLogDirPerms, true);
+        createDir(appLogDir, appLogDirPerms, true);
       } catch (IOException e) {
         LOG.warn("Unable to create the app-log directory : " + appLogDir, e);
         continue;
@@ -455,7 +464,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
   /**
    * Create application log directories on all disks.
    */
-  private void createContainerLogDirs(String appId, String containerId,
+  void createContainerLogDirs(String appId, String containerId,
       List<String> logDirs) throws IOException {
 
     boolean containerLogDirStatus = false;
@@ -465,7 +474,7 @@ public class DefaultContainerExecutor extends ContainerExecutor {
       Path appLogDir = new Path(rootLogDir, appId);
       Path containerLogDir = new Path(appLogDir, containerId);
       try {
-        lfs.mkdir(containerLogDir, containerLogDirPerms, true);
+        createDir(containerLogDir, containerLogDirPerms, true);
       } catch (IOException e) {
         LOG.warn("Unable to create the container-log directory : "
             + appLogDir, e);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDefaultContainerExecutor.java

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.yarn.server.nodemanager;
 
+import java.io.FileNotFoundException;
 import java.io.InputStream;
 import java.io.IOException;
 import java.net.InetSocketAddress;
@@ -26,8 +27,11 @@ import java.util.EnumSet;
 import java.util.List;
 import java.util.Random;
 
+import junit.framework.Assert;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.AbstractFileSystem;
+import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.fs.FSDataInputStream;
 import org.apache.hadoop.fs.FSDataOutputStream;
 import org.apache.hadoop.fs.FileContext;
@@ -38,6 +42,7 @@ import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.io.DataInputBuffer;
 import org.apache.hadoop.io.DataOutputBuffer;
 import org.apache.hadoop.util.Progressable;
+import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.FakeFSDataInputStream;
 
 import static org.apache.hadoop.fs.CreateFlag.*;
@@ -91,11 +96,16 @@ public class TestDefaultContainerExecutor {
   }
   */
 
+  private static final Path BASE_TMP_PATH = new Path("target",
+      TestDefaultContainerExecutor.class.getSimpleName());
+
   @AfterClass
   public static void deleteTmpFiles() throws IOException {
     FileContext lfs = FileContext.getLocalFSFileContext();
-    lfs.delete(new Path("target",
+    try {
-          TestDefaultContainerExecutor.class.getSimpleName()), true);
+      lfs.delete(BASE_TMP_PATH, true);
+    } catch (FileNotFoundException e) {
+    }
   }
 
   byte[] createTmpFile(Path dst, Random r, int len)
@@ -116,6 +126,71 @@ public class TestDefaultContainerExecutor {
     return bytes;
   }
 
+  @Test
+  public void testDirPermissions() throws Exception {
+    deleteTmpFiles();
+
+    final String user = "somebody";
+    final String appId = "app_12345_123";
+    final FsPermission userCachePerm = new FsPermission(
+        DefaultContainerExecutor.USER_PERM);
+    final FsPermission appCachePerm = new FsPermission(
+        DefaultContainerExecutor.APPCACHE_PERM);
+    final FsPermission fileCachePerm = new FsPermission(
+        DefaultContainerExecutor.FILECACHE_PERM);
+    final FsPermission appDirPerm = new FsPermission(
+        DefaultContainerExecutor.APPDIR_PERM);
+    final FsPermission logDirPerm = new FsPermission(
+        DefaultContainerExecutor.LOGDIR_PERM);
+    List<String> localDirs = new ArrayList<String>();
+    localDirs.add(new Path(BASE_TMP_PATH, "localDirA").toString());
+    localDirs.add(new Path(BASE_TMP_PATH, "localDirB").toString());
+    List<String> logDirs = new ArrayList<String>();
+    logDirs.add(new Path(BASE_TMP_PATH, "logDirA").toString());
+    logDirs.add(new Path(BASE_TMP_PATH, "logDirB").toString());
+
+    Configuration conf = new Configuration();
+    conf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "077");
+    FileContext lfs = FileContext.getLocalFSFileContext(conf);
+    DefaultContainerExecutor executor = new DefaultContainerExecutor(lfs);
+    executor.init();
+
+    try {
+      executor.createUserLocalDirs(localDirs, user);
+      executor.createUserCacheDirs(localDirs, user);
+      executor.createAppDirs(localDirs, user, appId);
+
+      for (String dir : localDirs) {
+        FileStatus stats = lfs.getFileStatus(
+            new Path(new Path(dir, ContainerLocalizer.USERCACHE), user));
+        Assert.assertEquals(userCachePerm, stats.getPermission());
+      }
+
+      for (String dir : localDirs) {
+        Path userCachePath = new Path(
+            new Path(dir, ContainerLocalizer.USERCACHE), user);
+        Path appCachePath = new Path(userCachePath,
+            ContainerLocalizer.APPCACHE);
+        FileStatus stats = lfs.getFileStatus(appCachePath);
+        Assert.assertEquals(appCachePerm, stats.getPermission());
+        stats = lfs.getFileStatus(
+            new Path(userCachePath, ContainerLocalizer.FILECACHE));
+        Assert.assertEquals(fileCachePerm, stats.getPermission());
+        stats = lfs.getFileStatus(new Path(appCachePath, appId));
+        Assert.assertEquals(appDirPerm, stats.getPermission());
+      }
+
+      executor.createAppLogDirs(appId, logDirs);
+
+      for (String dir : logDirs) {
+        FileStatus stats = lfs.getFileStatus(new Path(dir, appId));
+        Assert.assertEquals(logDirPerm, stats.getPermission());
+      }
+    } finally {
+      deleteTmpFiles();
+    }
+  }
+
 //  @Test
 //  public void testInit() throws IOException, InterruptedException {
 //    Configuration conf = new Configuration();