You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mapreduce-commits@hadoop.apache.org by yh...@apache.org on 2009/08/27 12:17:35 UTC

svn commit: r808351 - in /hadoop/mapreduce/trunk: ./ src/c++/task-controller/ src/test/mapred/org/apache/hadoop/mapred/

Author: yhemanth
Date: Thu Aug 27 10:17:34 2009
New Revision: 808351

URL: http://svn.apache.org/viewvc?rev=808351&view=rev
Log:
MAPREDUCE-871. Fix ownership of Job/Task local files to have correct group ownership according to the egid of the tasktracker. Contributed by Vinod Kumar Vavilapalli.

Modified:
    hadoop/mapreduce/trunk/CHANGES.txt
    hadoop/mapreduce/trunk/src/c++/task-controller/task-controller.c
    hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestLocalizationWithLinuxTaskController.java
    hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java

Modified: hadoop/mapreduce/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/CHANGES.txt?rev=808351&r1=808350&r2=808351&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/CHANGES.txt (original)
+++ hadoop/mapreduce/trunk/CHANGES.txt Thu Aug 27 10:17:34 2009
@@ -497,3 +497,6 @@
     MAPREDUCE-430. Fix a bug related to task getting stuck in case of 
     OOM error. (Amar Kamat via ddas)
 
+    MAPREDUCE-871. Fix ownership of Job/Task local files to have correct 
+    group ownership according to the egid of the tasktracker.
+    (Vinod Kumar Vavilapalli via yhemanth) 

Modified: hadoop/mapreduce/trunk/src/c++/task-controller/task-controller.c
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/c%2B%2B/task-controller/task-controller.c?rev=808351&r1=808350&r2=808351&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/c++/task-controller/task-controller.c (original)
+++ hadoop/mapreduce/trunk/src/c++/task-controller/task-controller.c Thu Aug 27 10:17:34 2009
@@ -333,7 +333,7 @@
       continue;
     }
 
-    if (check_ownership(getuid(), getgid(), entry->fts_path) != 0) {
+    if (check_ownership(entry->fts_path) != 0) {
       fprintf(LOGFILE,
           "Invalid file path. %s not user/group owned by the tasktracker.\n",
           entry->fts_path);
@@ -369,13 +369,13 @@
     return INVALID_ARGUMENT_NUMBER;
   }
 
+  gid_t tasktracker_gid = getegid(); // the group permissions of the binary.
+
   if (get_user_details(user) < 0) {
     fprintf(LOGFILE, "Couldn't get the user details of %s.\n", user);
     return INVALID_USER_NAME;
   }
 
-  int tasktracker_gid = getgid();
-
   char **local_dir = (char **) get_values(TT_SYS_DIR_KEY);
 
   if (local_dir == NULL) {
@@ -485,7 +485,7 @@
     }
   }
 
-  int tasktracker_gid = getgid();
+  gid_t tasktracker_gid = getegid(); // the group permissions of the binary.
   if (secure_path(task_log_dir, user_detail->pw_uid, tasktracker_gid, S_IRWXU
       | S_IRWXG, S_ISGID | S_IRWXU | S_IRWXG) != 0) {
     // setgid on dirs but not files, 770. As of now, there are no files though
@@ -508,15 +508,18 @@
 }
 
 /*
- * Function to check if a user/group actually owns the file.
+ * Function to check if the TaskTracker actually owns the file.
   */
-int check_ownership(uid_t uid, gid_t gid, char *path) {
+int check_ownership(char *path) {
   struct stat filestat;
   if (stat(path, &filestat) != 0) {
     return UNABLE_TO_STAT_FILE;
   }
-  // check user/group.
-  if (uid != filestat.st_uid || gid != filestat.st_gid) {
+  // check user/group. User should be TaskTracker user, group can either be
+  // TaskTracker's primary group or the special group to which binary's
+  // permissions are set.
+  if (getuid() != filestat.st_uid || (getgid() != filestat.st_gid && getegid()
+      != filestat.st_gid)) {
     return FILE_NOT_OWNED_BY_TASKTRACKER;
   }
   return 0;
@@ -540,7 +543,7 @@
     return INVALID_USER_NAME;
   }
 
-  gid_t tasktracker_gid = getgid(); // TaskTracker's group-id
+  gid_t tasktracker_gid = getegid(); // the group permissions of the binary.
 
   char **local_dir = (char **) get_values(TT_SYS_DIR_KEY);
   if (local_dir == NULL) {

Modified: hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestLocalizationWithLinuxTaskController.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestLocalizationWithLinuxTaskController.java?rev=808351&r1=808350&r2=808351&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestLocalizationWithLinuxTaskController.java (original)
+++ hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestLocalizationWithLinuxTaskController.java Thu Aug 27 10:17:34 2009
@@ -30,7 +30,6 @@
 import org.apache.hadoop.mapred.TaskController.JobInitializationContext;
 import org.apache.hadoop.mapred.TaskController.TaskControllerContext;
 import org.apache.hadoop.mapred.TaskTracker.TaskInProgress;
-import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.mapred.ClusterWithLinuxTaskController.MyLinuxTaskController;
 import org.apache.hadoop.mapred.JvmManager.JvmEnv;
 
@@ -48,6 +47,8 @@
   private File configFile;
   private MyLinuxTaskController taskController;
 
+  private static String taskTrackerSpecialGroup;
+
   @Override
   protected void setUp()
       throws Exception {
@@ -66,6 +67,7 @@
             localDirs);
     String execPath = path + "/task-controller";
     taskController.setTaskControllerExe(execPath);
+    taskTrackerSpecialGroup = getFilePermissionAttrs(execPath)[2];
     taskController.setConf(trackerFConf);
     taskController.setup();
   }
@@ -120,14 +122,12 @@
     taskController.initializeJob(context);
     // ///////////
 
-    UserGroupInformation taskTrackerugi =
-        UserGroupInformation.login(localizedJobConf);
     for (String localDir : trackerFConf.getStrings("mapred.local.dir")) {
       File jobDir =
           new File(localDir, TaskTracker.getLocalJobDir(jobId.toString()));
       // check the private permissions on the job directory
       checkFilePermissions(jobDir.getAbsolutePath(), "dr-xrws---",
-          localizedJobConf.getUser(), taskTrackerugi.getGroupNames()[0]);
+          localizedJobConf.getUser(), taskTrackerSpecialGroup);
     }
 
     // check the private permissions of various directories
@@ -139,7 +139,7 @@
     dirs.add(new Path(jarsDir, "lib"));
     for (Path dir : dirs) {
       checkFilePermissions(dir.toUri().getPath(), "dr-xrws---",
-          localizedJobConf.getUser(), taskTrackerugi.getGroupNames()[0]);
+          localizedJobConf.getUser(), taskTrackerSpecialGroup);
     }
 
     // job-work dir needs user writable permissions
@@ -147,7 +147,7 @@
         lDirAlloc.getLocalPathToRead(TaskTracker.getJobWorkDir(jobId
             .toString()), trackerFConf);
     checkFilePermissions(jobWorkDir.toUri().getPath(), "drwxrws---",
-        localizedJobConf.getUser(), taskTrackerugi.getGroupNames()[0]);
+        localizedJobConf.getUser(), taskTrackerSpecialGroup);
 
     // check the private permissions of various files
     List<Path> files = new ArrayList<Path>();
@@ -159,7 +159,7 @@
     files.add(new Path(jarsDir, "lib" + Path.SEPARATOR + "lib2.jar"));
     for (Path file : files) {
       checkFilePermissions(file.toUri().getPath(), "-r-xrwx---",
-          localizedJobConf.getUser(), taskTrackerugi.getGroupNames()[0]);
+          localizedJobConf.getUser(), taskTrackerSpecialGroup);
     }
   }
 
@@ -223,11 +223,9 @@
     dirs.add(workDir);
     dirs.add(new Path(workDir, "tmp"));
     dirs.add(new Path(logFiles[1].getParentFile().getAbsolutePath()));
-    UserGroupInformation taskTrackerugi =
-        UserGroupInformation.login(localizedJobConf);
     for (Path dir : dirs) {
       checkFilePermissions(dir.toUri().getPath(), "drwxrws---",
-          localizedJobConf.getUser(), taskTrackerugi.getGroupNames()[0]);
+          localizedJobConf.getUser(), taskTrackerSpecialGroup);
     }
 
     // check the private permissions of various files
@@ -237,7 +235,7 @@
         .isTaskCleanupTask()), trackerFConf));
     for (Path file : files) {
       checkFilePermissions(file.toUri().getPath(), "-rwxrwx---",
-          localizedJobConf.getUser(), taskTrackerugi.getGroupNames()[0]);
+          localizedJobConf.getUser(), taskTrackerSpecialGroup);
     }
   }
 }

Modified: hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java?rev=808351&r1=808350&r2=808351&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java (original)
+++ hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java Thu Aug 27 10:17:34 2009
@@ -131,7 +131,7 @@
     FileUtil.fullyDelete(TEST_ROOT_DIR);
   }
 
-  private static String[] getFilePermissionAttrs(String path)
+  protected static String[] getFilePermissionAttrs(String path)
       throws IOException {
     String output = Shell.execCommand("stat", path, "-c", "%A:%U:%G");
     return output.split(":|\n");