YARN-9884. Make container-executor mount logic modular

           Contributed by Eric Badger

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/CMakeLists.txt

@@ -137,6 +137,7 @@ add_library(container
     main/native/container-executor/impl/modules/fpga/fpga-module.c
     main/native/container-executor/impl/modules/devices/devices-module.c
     main/native/container-executor/impl/utils/docker-util.c
+    main/native/container-executor/impl/utils/mount-utils.c
 )
 
 add_executable(container-executor

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c

@@ -1367,7 +1367,7 @@ char **construct_docker_command(const char *command_file) {
   ret = get_docker_command(command_file, &CFG, &buffer);
   if (ret != 0) {
     fprintf(ERRORFILE, "Error constructing docker command, docker error code=%d, error message='%s'\n", ret,
-            get_docker_error_message(ret));
+            get_error_message(ret));
     exit(DOCKER_RUN_FAILED);
   }
 
@@ -1414,7 +1414,7 @@ int exec_container(const char *command_file) {
   if (ret != 0) {
     free_configuration(&command_config);
     free(docker_binary);
-    return INVALID_COMMAND_FILE;
+    return INVALID_DOCKER_COMMAND_FILE;
   }
 
   char *value = get_configuration_value("docker-command", DOCKER_COMMAND_FILE_SECTION, &command_config);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c

@@ -788,6 +788,11 @@ int main(int argc, char **argv) {
     break;
   }
 
+  if (exit_code) {
+    fprintf(ERRORFILE, "Nonzero exit code=%d, error message='%s'\n", exit_code,
+            get_error_message(exit_code));
+  }
+
   flush_and_close_log_files();
   return exit_code;
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.c

@@ -195,3 +195,140 @@ void quote_and_append_arg(char **str, size_t *size, const char* param, const cha
   sprintf(cur_ptr, append_format, param, tmp);
   free(tmp);
 }
+
+
+const char *get_error_message(const int error_code) {
+    switch (error_code) {
+      case INVALID_ARGUMENT_NUMBER:
+        return "Invalid argument number";
+      case INVALID_COMMAND_PROVIDED:
+        return "Invalid command provided";
+      case INVALID_NM_ROOT_DIRS:
+        return "Invalid NM root dirs";
+      case SETUID_OPER_FAILED:
+        return "setuid operation failed";
+      case UNABLE_TO_EXECUTE_CONTAINER_SCRIPT:
+        return "Unable to execute container script";
+      case UNABLE_TO_SIGNAL_CONTAINER:
+        return "Unable to signal container";
+      case INVALID_CONTAINER_PID:
+        return "Invalid container PID";
+      case OUT_OF_MEMORY:
+        return "Out of memory";
+      case INITIALIZE_USER_FAILED:
+        return "Initialize user failed";
+      case PATH_TO_DELETE_IS_NULL:
+        return "Path to delete is null";
+      case INVALID_CONTAINER_EXEC_PERMISSIONS:
+        return "Invalid container-executor permissions";
+      case INVALID_CONFIG_FILE:
+        return "Invalid config file";
+      case SETSID_OPER_FAILED:
+        return "setsid operation failed";
+      case WRITE_PIDFILE_FAILED:
+        return "Write to pidfile failed";
+      case WRITE_CGROUP_FAILED:
+        return "Write to cgroup failed";
+      case TRAFFIC_CONTROL_EXECUTION_FAILED:
+        return "Traffic control execution failed";
+      case DOCKER_RUN_FAILED:
+        return "Docker run failed";
+      case ERROR_OPENING_DOCKER_FILE:
+        return "Error opening Docker file";
+      case ERROR_READING_DOCKER_FILE:
+        return "Error reading Docker file";
+      case FEATURE_DISABLED:
+        return "Feature disabled";
+      case COULD_NOT_CREATE_SCRIPT_COPY:
+        return "Could not create script copy";
+      case COULD_NOT_CREATE_CREDENTIALS_COPY:
+        return "Could not create credentials copy";
+      case COULD_NOT_CREATE_WORK_DIRECTORIES:
+        return "Could not create work dirs";
+      case COULD_NOT_CREATE_APP_LOG_DIRECTORIES:
+        return "Could not create app log dirs";
+      case COULD_NOT_CREATE_TMP_DIRECTORIES:
+        return "Could not create tmp dirs";
+      case ERROR_CREATE_CONTAINER_DIRECTORIES_ARGUMENTS:
+        return "Error in create container directories arguments";
+      case ERROR_SANITIZING_DOCKER_COMMAND:
+        return "Error sanitizing Docker command";
+      case DOCKER_IMAGE_INVALID:
+        return "Docker image invalid";
+      case ERROR_COMPILING_REGEX:
+        return "Error compiling regex";
+      case INVALID_CONTAINER_ID:
+        return "Invalid container id";
+      case DOCKER_EXEC_FAILED:
+        return "Docker exec failed";
+      case COULD_NOT_CREATE_KEYSTORE_COPY:
+        return "Could not create keystore copy";
+      case COULD_NOT_CREATE_TRUSTSTORE_COPY:
+        return "Could not create truststore copy";
+      case ERROR_CALLING_SETVBUF:
+        return "Error calling setvbuf";
+      case BUFFER_TOO_SMALL:
+        return "Buffer too small";
+      case INVALID_MOUNT:
+        return "Invalid mount";
+      case INVALID_RO_MOUNT:
+        return "Invalid read-only mount";
+      case INVALID_RW_MOUNT:
+        return "Invalid read-write mount";
+      case MOUNT_ACCESS_ERROR:
+        return "Mount access error";
+      case INVALID_DOCKER_COMMAND_FILE:
+        return "Invalid docker command file passed";
+      case INCORRECT_DOCKER_COMMAND:
+        return "Incorrect command";
+      case INVALID_DOCKER_CONTAINER_NAME:
+        return "Invalid docker container name";
+      case INVALID_DOCKER_IMAGE_NAME:
+        return "Invalid docker image name";
+      case INVALID_DOCKER_USER_NAME:
+        return "Invalid docker user name";
+      case INVALID_DOCKER_INSPECT_FORMAT:
+        return "Invalid docker inspect format";
+      case UNKNOWN_DOCKER_COMMAND:
+        return "Unknown docker command";
+      case INVALID_DOCKER_NETWORK:
+        return "Invalid docker network";
+      case INVALID_DOCKER_PORTS_MAPPING:
+        return "Invalid docker ports mapping";
+      case INVALID_DOCKER_CAPABILITY:
+        return "Invalid docker capability";
+      case PRIVILEGED_DOCKER_CONTAINERS_DISABLED:
+        return "Privileged docker containers are disabled";
+      case INVALID_DOCKER_DEVICE:
+        return "Invalid docker device";
+      case INVALID_DOCKER_STOP_COMMAND:
+        return "Invalid docker stop command";
+      case INVALID_DOCKER_KILL_COMMAND:
+        return "Invalid docker kill command";
+      case INVALID_DOCKER_VOLUME_DRIVER:
+        return "Invalid docker volume-driver";
+      case INVALID_DOCKER_VOLUME_NAME:
+        return "Invalid docker volume name";
+      case INVALID_DOCKER_VOLUME_COMMAND:
+        return "Invalid docker volume command";
+      case DOCKER_PID_HOST_DISABLED:
+        return "Docker host pid namespace is disabled";
+      case INVALID_DOCKER_PID_NAMESPACE:
+        return "Invalid docker pid namespace";
+      case INVALID_DOCKER_IMAGE_TRUST:
+        return "Docker image is not trusted";
+      case INVALID_DOCKER_TMPFS_MOUNT:
+        return "Invalid docker tmpfs mount";
+      case INVALID_DOCKER_RUNTIME:
+        return "Invalid docker runtime";
+      case DOCKER_SERVICE_MODE_DISABLED:
+        return "Docker service mode disabled";
+      default:
+        return "Unknown error code";
+    }
+}
+
+int is_regex(const char *str) {
+    // regex should begin with prefix "regex:"
+    return (strncmp(str, "regex:", 6) == 0);
+}

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.h

@@ -31,10 +31,10 @@ enum errorcodes {
   INVALID_COMMAND_PROVIDED = 3,
   // SUPER_USER_NOT_ALLOWED_TO_RUN_TASKS (NOT USED) 4
   INVALID_NM_ROOT_DIRS = 5,
-  SETUID_OPER_FAILED, //6
-  UNABLE_TO_EXECUTE_CONTAINER_SCRIPT, //7
-  UNABLE_TO_SIGNAL_CONTAINER, //8
-  INVALID_CONTAINER_PID, //9
+  SETUID_OPER_FAILED = 6,
+  UNABLE_TO_EXECUTE_CONTAINER_SCRIPT = 7,
+  UNABLE_TO_SIGNAL_CONTAINER = 8,
+  INVALID_CONTAINER_PID = 9,
   // ERROR_RESOLVING_FILE_PATH (NOT_USED) 10
   // RELATIVE_PATH_COMPONENTS_IN_FILE_PATH (NOT USED) 11
   // UNABLE_TO_STAT_FILE (NOT USED) 12
@@ -46,8 +46,8 @@ enum errorcodes {
   OUT_OF_MEMORY = 18,
   // INITIALIZE_DISTCACHEFILE_FAILED (NOT USED) 19
   INITIALIZE_USER_FAILED = 20,
-  PATH_TO_DELETE_IS_NULL, //21
-  INVALID_CONTAINER_EXEC_PERMISSIONS, //22
+  PATH_TO_DELETE_IS_NULL = 21,
+  INVALID_CONTAINER_EXEC_PERMISSIONS = 22,
   // PREPARE_JOB_LOGS_FAILED (NOT USED) 23
   INVALID_CONFIG_FILE = 24,
   SETSID_OPER_FAILED = 25,
@@ -72,7 +72,35 @@ enum errorcodes {
   DOCKER_EXEC_FAILED = 44,
   COULD_NOT_CREATE_KEYSTORE_COPY = 45,
   COULD_NOT_CREATE_TRUSTSTORE_COPY = 46,
-  ERROR_CALLING_SETVBUF = 47
+  ERROR_CALLING_SETVBUF = 47,
+  BUFFER_TOO_SMALL = 48,
+  INVALID_MOUNT = 49,
+  INVALID_RO_MOUNT = 50,
+  INVALID_RW_MOUNT = 51,
+  MOUNT_ACCESS_ERROR = 52,
+  INVALID_DOCKER_COMMAND_FILE = 53,
+  INCORRECT_DOCKER_COMMAND = 54,
+  INVALID_DOCKER_CONTAINER_NAME = 55,
+  INVALID_DOCKER_IMAGE_NAME = 56,
+  INVALID_DOCKER_USER_NAME = 57,
+  INVALID_DOCKER_INSPECT_FORMAT = 58,
+  UNKNOWN_DOCKER_COMMAND = 59,
+  INVALID_DOCKER_NETWORK = 60,
+  INVALID_DOCKER_PORTS_MAPPING = 61,
+  INVALID_DOCKER_CAPABILITY = 62,
+  PRIVILEGED_DOCKER_CONTAINERS_DISABLED = 63,
+  INVALID_DOCKER_DEVICE = 64,
+  INVALID_DOCKER_STOP_COMMAND = 65,
+  INVALID_DOCKER_KILL_COMMAND = 66,
+  INVALID_DOCKER_VOLUME_DRIVER = 67,
+  INVALID_DOCKER_VOLUME_NAME = 68,
+  INVALID_DOCKER_VOLUME_COMMAND = 69,
+  DOCKER_PID_HOST_DISABLED = 70,
+  INVALID_DOCKER_PID_NAMESPACE = 71,
+  INVALID_DOCKER_IMAGE_TRUST = 72,
+  INVALID_DOCKER_TMPFS_MOUNT = 73,
+  INVALID_DOCKER_RUNTIME = 74,
+  DOCKER_SERVICE_MODE_DISABLED = 75
 };
 
 /* Macros for min/max. */
@@ -169,4 +197,8 @@ inline void* alloc_and_clear_memory(size_t num, size_t size) {
   return ret;
 }
 
+const char *get_error_message(const int error_code);
+
+int is_regex(const char *str);
+
 #endif

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c

@@ -32,6 +32,7 @@
 #include <grp.h>
 #include <pwd.h>
 #include <errno.h>
+#include "mount-utils.h"
 
 int entry_point = 0;
 
@@ -40,11 +41,11 @@ static int read_and_verify_command_file(const char *command_file, const char *do
   int ret = 0;
   ret = read_config(command_file, command_config);
   if (ret != 0) {
-    return INVALID_COMMAND_FILE;
+    return INVALID_DOCKER_COMMAND_FILE;
   }
   char *command = get_configuration_value("docker-command", DOCKER_COMMAND_FILE_SECTION, command_config);
   if (command == NULL || (strcmp(command, docker_command) != 0)) {
-    ret = INCORRECT_COMMAND;
+    ret = INCORRECT_DOCKER_COMMAND;
   }
   free(command);
   return ret;
@@ -173,23 +174,12 @@ free_and_exit:
   return ret;
 }
 
-static int is_regex(const char *str) {
-  // regex should begin with prefix "regex:"
-  return (strncmp(str, "regex:", 6) == 0);
-}
-
 static int is_valid_tmpfs_mount(const char *mount) {
   const char *regex_str = "^/[^:]+$";
   // execute_regex_match return 0 is matched success
   return execute_regex_match(regex_str, mount) == 0;
 }
 
-static int is_volume_name(const char *volume_name) {
-  const char *regex_str = "^[a-zA-Z0-9]([a-zA-Z0-9_.-]*)$";
-  // execute_regex_match return 0 is matched success
-  return execute_regex_match(regex_str, volume_name) == 0;
-}
-
 static int is_valid_ports_mapping(const char *ports_mapping) {
   const char *regex_str = "^:[0-9]+|^[0-9]+:[0-9]+|^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.)"
                           "{3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]):[0-9]+:[0-9]+$";
@@ -197,11 +187,6 @@ static int is_valid_ports_mapping(const char *ports_mapping) {
   return execute_regex_match(regex_str, ports_mapping) == 0;
 }
 
-static int is_volume_name_matched_by_regex(const char* requested, const char* pattern) {
-  // execute_regex_match return 0 is matched success
-  return is_volume_name(requested) && (execute_regex_match(pattern + sizeof("regex:"), requested) == 0);
-}
-
 static int add_param_to_command_if_allowed(const struct configuration *command_config,
                                            const struct configuration *executor_cfg,
                                            const char *key, const char *allowed_key, const char *param,
@@ -322,66 +307,6 @@ static int validate_container_name(const char *container_name) {
   return INVALID_DOCKER_CONTAINER_NAME;
 }
 
-const char *get_docker_error_message(const int error_code) {
-
-  switch (error_code) {
-    case INVALID_COMMAND_FILE:
-      return "Invalid command file passed";
-    case INCORRECT_COMMAND:
-      return "Incorrect command";
-    case BUFFER_TOO_SMALL:
-      return "Command buffer too small";
-    case INVALID_DOCKER_CONTAINER_NAME:
-      return "Invalid docker container name";
-    case INVALID_DOCKER_IMAGE_NAME:
-      return "Invalid docker image name";
-    case INVALID_DOCKER_USER_NAME:
-      return "Invalid docker user name";
-    case INVALID_DOCKER_INSPECT_FORMAT:
-      return "Invalid docker inspect format";
-    case UNKNOWN_DOCKER_COMMAND:
-      return "Unknown docker command";
-    case INVALID_DOCKER_NETWORK:
-      return "Invalid docker network";
-    case INVALID_DOCKER_PORTS_MAPPING:
-      return "Invalid docker ports mapping";
-    case INVALID_DOCKER_CAPABILITY:
-      return "Invalid docker capability";
-    case PRIVILEGED_CONTAINERS_DISABLED:
-      return "Privileged containers are disabled";
-    case INVALID_DOCKER_MOUNT:
-      return "Invalid docker mount";
-    case INVALID_DOCKER_RO_MOUNT:
-      return "Invalid docker read-only mount";
-    case INVALID_DOCKER_RW_MOUNT:
-      return "Invalid docker read-write mount";
-    case MOUNT_ACCESS_ERROR:
-      return "Mount access error";
-    case INVALID_DOCKER_DEVICE:
-      return "Invalid docker device";
-    case INVALID_DOCKER_VOLUME_DRIVER:
-      return "Invalid docker volume-driver";
-    case INVALID_DOCKER_VOLUME_NAME:
-      return "Invalid docker volume name";
-    case INVALID_DOCKER_VOLUME_COMMAND:
-      return "Invalid docker volume command";
-    case PID_HOST_DISABLED:
-      return "Host pid namespace is disabled";
-    case INVALID_PID_NAMESPACE:
-      return "Invalid pid namespace";
-    case INVALID_DOCKER_IMAGE_TRUST:
-      return "Docker image is not trusted";
-    case INVALID_DOCKER_TMPFS_MOUNT:
-      return "Invalid docker tmpfs mount";
-    case INVALID_DOCKER_RUNTIME:
-      return "Invalid docker runtime";
-    case SERVICE_MODE_DISABLED:
-      return "Service mode disabled";
-    default:
-      return "Unknown error";
-  }
-}
-
 int get_max_retries(const struct configuration *conf) {
   int retries = 10;
   char *max_retries = get_configuration_value(DOCKER_INSPECT_MAX_RETRIES_KEY,
@@ -424,7 +349,7 @@ int get_docker_command(const char *command_file, const struct configuration *con
   ret = read_config(command_file, &command_config);
   if (ret != 0) {
     free_configuration(&command_config);
-    return INVALID_COMMAND_FILE;
+    return INVALID_DOCKER_COMMAND_FILE;
   }
 
   char *docker = get_docker_binary(conf);
@@ -905,7 +830,7 @@ int get_docker_exec_command(const char *command_file, const struct configuration
       }
     }
   } else {
-    ret = INVALID_COMMAND_FILE;
+    ret = INVALID_DOCKER_COMMAND_FILE;
   }
 free_and_exit:
   free(container_name);
@@ -999,7 +924,7 @@ int is_service_mode_enabled(const struct configuration *command_config,
       if (is_feature_enabled(DOCKER_SERVICE_MODE_ENABLED_KEY, ret, section)) {
         ret = 1;
       } else {
-        ret = SERVICE_MODE_DISABLED;
+        ret = DOCKER_SERVICE_MODE_DISABLED;
       }
      }
     free(value);
@@ -1083,17 +1008,17 @@ static int set_pid_namespace(const struct configuration *command_config,
           }
         } else {
           fprintf(ERRORFILE, "Host pid namespace is disabled\n");
-          ret = PID_HOST_DISABLED;
+          ret = DOCKER_PID_HOST_DISABLED;
           goto free_and_exit;
         }
       } else {
         fprintf(ERRORFILE, "Host pid namespace is disabled\n");
-        ret = PID_HOST_DISABLED;
+        ret = DOCKER_PID_HOST_DISABLED;
         goto free_and_exit;
       }
     } else {
       fprintf(ERRORFILE, "Invalid pid namespace\n");
-      ret = INVALID_PID_NAMESPACE;
+      ret = INVALID_DOCKER_PID_NAMESPACE;
     }
   }
 
@@ -1161,164 +1086,6 @@ static int set_env(const struct configuration *command_config, struct args *args
   return ret;
 }
 
-/**
- * Helper function to help normalize mounts for checking if mounts are
- * permitted. The function does the following -
- * 1. Find the canonical path for mount using realpath
- * 2. If the path is a directory, add a '/' at the end (if not present)
- * 3. Return a copy of the canonicalised path(to be freed by the caller)
- * @param mount path to be canonicalised
- * @param isRegexAllowed whether regex matching is allowed for normalize mount
- * @return pointer to canonicalised path, NULL on error
- */
-static char* normalize_mount(const char* mount, int isRegexAllowed) {
-  int ret = 0;
-  struct stat buff;
-  char *ret_ptr = NULL, *real_mount = NULL;
-  if (mount == NULL) {
-    return NULL;
-  }
-  real_mount = realpath(mount, NULL);
-  if (real_mount == NULL) {
-    // If mount is a valid named volume, just return it and let docker decide
-    if (is_volume_name(mount)) {
-      return strdup(mount);
-    }
-    // we only allow permitted mount to be REGEX, for permitted mount, we check
-    // if it's a valid REGEX return; for user mount, we need to strictly check
-    if (isRegexAllowed) {
-      if (is_regex(mount)) {
-        return strdup(mount);
-      }
-    }
-    fprintf(ERRORFILE, "Could not determine real path of mount '%s'\n", mount);
-    return NULL;
-  }
-  ret = stat(real_mount, &buff);
-  if (ret == 0) {
-    if (S_ISDIR(buff.st_mode)) {
-      size_t len = strlen(real_mount);
-      if (len <= 0) {
-        free(real_mount);
-        return NULL;
-      }
-      if (real_mount[len - 1] != '/') {
-        ret_ptr = (char *) alloc_and_clear_memory(len + 2, sizeof(char));
-        strncpy(ret_ptr, real_mount, len);
-        ret_ptr[len] = '/';
-        ret_ptr[len + 1] = '\0';
-      } else {
-        ret_ptr = strdup(real_mount);
-      }
-    } else {
-      ret_ptr = strdup(real_mount);
-    }
-  } else {
-    fprintf(ERRORFILE, "Could not stat path '%s'\n", real_mount);
-    ret_ptr = NULL;
-  }
-  free(real_mount);
-  return ret_ptr;
-}
-
-static int normalize_mounts(char **mounts, int isRegexAllowed) {
-  int i = 0;
-  char *tmp = NULL;
-  if (mounts == NULL) {
-    return 0;
-  }
-  for (i = 0; mounts[i] != NULL; ++i) {
-    tmp = normalize_mount(mounts[i], isRegexAllowed);
-    if (tmp == NULL) {
-      return -1;
-    }
-    free(mounts[i]);
-    mounts[i] = tmp;
-  }
-  return 0;
-}
-
-static int check_mount_permitted(const char **permitted_mounts, const char *requested) {
-  int i = 0, ret = 0;
-  size_t permitted_mount_len = 0;
-  if (permitted_mounts == NULL) {
-    return 0;
-  }
-  char *normalized_path = normalize_mount(requested, 0);
-  if (normalized_path == NULL) {
-    return -1;
-  }
-  for (i = 0; permitted_mounts[i] != NULL; ++i) {
-    if (strcmp(normalized_path, permitted_mounts[i]) == 0) {
-      ret = 1;
-      break;
-    }
-    // if (permitted_mounts[i] is a REGEX): use REGEX to compare; return
-    if (is_regex(permitted_mounts[i]) &&
-    is_volume_name_matched_by_regex(normalized_path, permitted_mounts[i])) {
-      ret = 1;
-      break;
-    }
-
-    // directory check
-    permitted_mount_len = strlen(permitted_mounts[i]);
-    struct stat path_stat;
-    stat(permitted_mounts[i], &path_stat);
-    if(S_ISDIR(path_stat.st_mode)) {
-      if (strncmp(normalized_path, permitted_mounts[i], permitted_mount_len) == 0) {
-        ret = 1;
-        break;
-      }
-    }
-  }
-  free(normalized_path);
-  return ret;
-}
-
-static char* get_mount_source(const char *mount) {
-  const char *tmp = strchr(mount, ':');
-  if (tmp == NULL) {
-    fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount);
-    return NULL;
-  }
-  size_t len = tmp - mount;
-  return strndup(mount, len);
-}
-
-static char* get_mount_type(const char *mount) {
-  const char *tmp = strrchr(mount, ':');
-  if (tmp == NULL) {
-    fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount);
-    return NULL;
-  }
-  if (strlen(tmp) < 2) {
-    fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount);
-    return NULL;
-  }
-  char *mount_type = strdup(&tmp[1]);
-  if (strncmp("ro", mount_type, 2) != 0 &&
-      strncmp("rw", mount_type, 2) != 0) {
-    fprintf(ERRORFILE, "Invalid docker mount type '%s'\n", mount_type);
-    free(mount_type);
-    return NULL;
-  }
-  if (strlen(mount_type) > 2) {
-    if (strlen(mount_type) < 8 ||
-        (strcmp("shared", mount_type + 3) != 0 &&
-        strcmp("rshared", mount_type + 3) != 0 &&
-        strcmp("slave", mount_type + 3) != 0 &&
-        strcmp("rslave", mount_type + 3) != 0 &&
-        strcmp("private", mount_type + 3) != 0 &&
-        strcmp("rprivate", mount_type + 3) != 0)) {
-      fprintf(ERRORFILE, "Invalid docker mount type '%s'\n", mount_type);
-      free(mount_type);
-      return NULL;
-    }
-    mount_type[2] = ',';
-  }
-  return mount_type;
-}
-
 static int add_tmpfs_mounts(const struct configuration *command_config, args *args) {
   char **values = get_configuration_values_delimiter("tmpfs", DOCKER_COMMAND_FILE_SECTION, command_config, ",");
   int i = 0, ret = 0;
@@ -1348,122 +1115,7 @@ free_and_exit:
   return ret;
 }
 
-static int add_mounts(const struct configuration *command_config, const struct configuration *conf, args *args) {
-  const char *tmp_path_buffer[2] = {NULL, NULL};
-  char *mount_src = NULL;
-  char *mount_type = NULL;
-  char **permitted_ro_mounts = get_configuration_values_delimiter("docker.allowed.ro-mounts",
-                                                                  CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ",");
-  char **permitted_rw_mounts = get_configuration_values_delimiter("docker.allowed.rw-mounts",
-                                                                  CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ",");
-  char **values = get_configuration_values_delimiter("mounts", DOCKER_COMMAND_FILE_SECTION, command_config, ",");
-  char *config_path = get_config_path("");
-  const char *container_executor_cfg_path = normalize_mount(config_path, 0);
-  free(config_path);
-  int i = 0, permitted_rw = 0, permitted_ro = 0, ret = 0;
-  if (values == NULL) {
-    goto free_and_exit;
-  }
-  // Disable mount volumes if image is not trusted.
-  if (check_trusted_image(command_config, conf) != 0) {
-    fprintf(ERRORFILE, "Disable mount volume for untrusted image\n");
-    // YARN will implicitly bind node manager local directory to
-    // docker image.  This can create file system security holes,
-    // if docker container has binary to escalate privileges.
-    // For untrusted image, we drop mounting without reporting
-    // INVALID_DOCKER_MOUNT messages to allow running untrusted
-    // image in a sandbox.
-    ret = 0;
-    goto free_and_exit;
-  }
-  ret = normalize_mounts(permitted_ro_mounts, 1);
-  ret |= normalize_mounts(permitted_rw_mounts, 1);
-  if (ret != 0) {
-    fprintf(ERRORFILE, "Unable to find permitted docker mounts on disk\n");
-    ret = MOUNT_ACCESS_ERROR;
-    goto free_and_exit;
-  }
-  for (i = 0; values[i] != NULL; i++) {
-    mount_src = get_mount_source(values[i]);
-    if (mount_src == NULL) {
-      fprintf(ERRORFILE, "Invalid docker mount '%s'\n", values[i]);
-      ret = INVALID_DOCKER_MOUNT;
-      goto free_and_exit;
-    }
-    mount_type = get_mount_type(values[i]);
-    if (mount_type == NULL) {
-      fprintf(ERRORFILE, "Invalid docker mount '%s'\n", values[i]);
-      ret = INVALID_DOCKER_MOUNT;
-      goto free_and_exit;
-    }
-    permitted_rw = check_mount_permitted((const char **) permitted_rw_mounts, mount_src);
-    permitted_ro = check_mount_permitted((const char **) permitted_ro_mounts, mount_src);
-    if (permitted_ro == -1 || permitted_rw == -1) {
-      fprintf(ERRORFILE, "Invalid docker mount '%s', realpath=%s\n", values[i], mount_src);
-      ret = INVALID_DOCKER_MOUNT;
-      goto free_and_exit;
-    }
-    if (strncmp("rw", mount_type, 2) == 0) {
-      // rw mount
-      if (permitted_rw == 0) {
-        fprintf(ERRORFILE, "Configuration does not allow docker mount '%s', realpath=%s\n", values[i], mount_src);
-        ret = INVALID_DOCKER_RW_MOUNT;
-        goto free_and_exit;
-      } else {
-        // determine if the user can modify the container-executor.cfg file
-        tmp_path_buffer[0] = normalize_mount(mount_src, 0);
-        // just re-use the function, flip the args to check if the container-executor path is in the requested
-        // mount point
-        ret = check_mount_permitted(tmp_path_buffer, container_executor_cfg_path);
-        free((void *) tmp_path_buffer[0]);
-        if (ret == 1) {
-          fprintf(ERRORFILE, "Attempting to mount a parent directory '%s' of container-executor.cfg as read-write\n",
-                  values[i]);
-          ret = INVALID_DOCKER_RW_MOUNT;
-          goto free_and_exit;
-        }
-      }
-    } else {
-      // ro mount
-      if (permitted_ro == 0 && permitted_rw == 0) {
-        fprintf(ERRORFILE, "Configuration does not allow docker mount '%s', realpath=%s\n", values[i], mount_src);
-        ret = INVALID_DOCKER_RO_MOUNT;
-        goto free_and_exit;
-      }
-    }
-
-    if (strlen(mount_type) > 2) {
-      // overwrite separator between read mode and propagation option with ','
-      int mount_type_index = strlen(values[i]) - strlen(mount_type);
-      values[i][mount_type_index + 2] = ',';
-    }
 
-    ret = add_to_args(args, "-v");
-    if (ret != 0) {
-      ret = BUFFER_TOO_SMALL;
-      goto free_and_exit;
-    }
-
-    ret = add_to_args(args, values[i]);
-    if (ret != 0) {
-      ret = BUFFER_TOO_SMALL;
-      goto free_and_exit;
-    }
-    free(mount_src);
-    free(mount_type);
-    mount_src = NULL;
-    mount_type = NULL;
-  }
-
-free_and_exit:
-  free(mount_src);
-  free(mount_type);
-  free_values(permitted_ro_mounts);
-  free_values(permitted_rw_mounts);
-  free_values(values);
-  free((void *) container_executor_cfg_path);
-  return ret;
-}
 
 static int check_privileges(const char *user) {
   int ngroups = 0;
@@ -1567,13 +1219,13 @@ static int set_privileged(const struct configuration *command_config, const stru
         // Disable set privileged if entry point mode is disabled
         if (get_use_entry_point_flag() != 1) {
           fprintf(ERRORFILE, "Privileged containers are disabled for non-entry-point mode\n");
-          ret = PRIVILEGED_CONTAINERS_DISABLED;
+          ret = PRIVILEGED_DOCKER_CONTAINERS_DISABLED;
           goto free_and_exit;
         }
         // Disable set privileged if image is not trusted.
         if (check_trusted_image(command_config, conf) != 0) {
           fprintf(ERRORFILE, "Privileged containers are disabled from untrusted source\n");
-          ret = PRIVILEGED_CONTAINERS_DISABLED;
+          ret = PRIVILEGED_DOCKER_CONTAINERS_DISABLED;
           goto free_and_exit;
         }
         allowed = check_privileges(user);
@@ -1584,17 +1236,17 @@ static int set_privileged(const struct configuration *command_config, const stru
           }
         } else {
           fprintf(ERRORFILE, "Privileged containers are disabled for user: %s\n", user);
-          ret = PRIVILEGED_CONTAINERS_DISABLED;
+          ret = PRIVILEGED_DOCKER_CONTAINERS_DISABLED;
           goto free_and_exit;
         }
       } else {
         fprintf(ERRORFILE, "Privileged containers are disabled\n");
-        ret = PRIVILEGED_CONTAINERS_DISABLED;
+        ret = PRIVILEGED_DOCKER_CONTAINERS_DISABLED;
         goto free_and_exit;
       }
     } else {
       fprintf(ERRORFILE, "Privileged containers are disabled\n");
-      ret = PRIVILEGED_CONTAINERS_DISABLED;
+      ret = PRIVILEGED_DOCKER_CONTAINERS_DISABLED;
       goto free_and_exit;
     }
   }
@@ -1606,6 +1258,287 @@ free_and_exit:
   return ret;
 }
 
+
+
+static char* get_docker_mount_source(const char *mount) {
+    const char *tmp = strchr(mount, ':');
+    if (tmp == NULL) {
+        fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount);
+        return NULL;
+    }
+    size_t len = tmp - mount;
+    return strndup(mount, len);
+}
+
+static char* get_docker_mount_dest(const char *mount) {
+    size_t len;
+    const char *start = strchr(mount, ':') + 1;
+    if (start == NULL) {
+        fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount);
+        return NULL;
+    }
+
+    const char *end = strchr(start, ':');
+    if (end == NULL) {
+        len = strlen(mount) - (start - mount);
+    } else {
+        len = end - start;
+    }
+    return strndup(start, len);
+}
+
+static mount_options* get_docker_mount_options(const char *mount_string) {
+    char **opts = NULL;
+    mount_options *options = NULL;
+    unsigned int num_opts = 0;
+    char *option = NULL;
+    int len = 0;
+
+    //+1 because we don't care about the ':'
+    const char *option_string = strrchr(mount_string, ':') + 1;
+    if (option_string == NULL) {
+        fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount_string);
+        return NULL;
+    }
+
+    options = (mount_options *) calloc(1, sizeof(*options));
+    if (options == NULL) {
+        fprintf(ERRORFILE, "Unable to allocate %ld bytes\n", sizeof(*options));
+        return NULL;
+    }
+
+    len = strlen(option_string);
+
+    if (len == 2) {
+        opts = (char **) calloc(1, sizeof(*opts));
+        if (opts == NULL) {
+            fprintf(ERRORFILE, "Unable to allocate %ld bytes\n", sizeof(*opts));
+            free(options);
+            return NULL;
+        }
+        num_opts = 1;
+    } else if (len < 8) {
+        fprintf(ERRORFILE, "Invalid docker mount. Too many options. '%s'\n", mount_string);
+        free(options);
+        return NULL;
+    } else {
+        opts = (char **) calloc(2, sizeof(*opts));
+        if (opts == NULL) {
+          fprintf(ERRORFILE, "Unable to allocate %ld bytes\n", 2 * sizeof(*opts));
+          free(options);
+          return NULL;
+        }
+        num_opts = 2;
+    }
+
+    options->opts = opts;
+    options->num_opts = num_opts;
+
+    char *option_string_token = strdup(option_string);
+    option = strtok(option_string_token, "+");
+    for (unsigned int i = 0; i < options->num_opts; i++) {
+        if (option == NULL) {
+            fprintf(ERRORFILE, "Invalid docker mount options '%s'\n", mount_string);
+            free(option_string_token);
+            free_mount_options(options);
+            return NULL;
+        }
+
+        if (strcmp("rw", option) == 0) {
+            options->rw = 1;
+        } else if (strcmp("ro", option) == 0) {
+            options->rw = 0;
+        } else if (strcmp("shared", option) != 0 &&
+          (strcmp("rshared", option) != 0) &&
+          (strcmp("slave", option) != 0) &&
+          (strcmp("rslave", option) != 0) &&
+          (strcmp("private", option) != 0) &&
+          (strcmp("rprivate", option) != 0)) {
+            fprintf(ERRORFILE, "Invalid docker mount options '%s'\n", mount_string);
+            free(option_string_token);
+            free_mount_options(options);
+            return NULL;
+        }
+        options->opts[i] = strdup(option);
+        option = strtok(NULL, "+");
+    }
+
+    free(option_string_token);
+    return options;
+}
+
+static char* get_docker_mount_options_string(mount_options *options) {
+    char *options_string = NULL;
+    int len = 0;
+    int idx = 0;
+    unsigned int i;
+
+    for (i = 0; i < options->num_opts; i++) {
+        len += strlen(options->opts[i]);
+    }
+    len += i; // i-1 commas plus 1 for NUL termination
+
+    options_string = (char *) calloc(len, sizeof(*options_string));
+    if (options_string == NULL) {
+      fputs("Unable to allocate memory\n", ERRORFILE);
+      return NULL;
+    }
+
+    idx += sprintf(options_string, "%s", options->opts[0]);
+    for (i = 1; i < options->num_opts; i++) {
+        idx += sprintf(options_string + idx, ",%s", options->opts[i]);
+    }
+
+  return options_string;
+}
+
+static int add_mounts_to_docker_args(mount *mounts, unsigned int num_mounts, args *args) {
+    int ret = 0, len;
+    unsigned int i;
+    char *mount_string = NULL;
+    char *options_string = NULL;
+
+    if (mounts == NULL) {
+        return ret;
+    }
+
+    for (i = 0; i < num_mounts; i++) {
+        ret = add_to_args(args, "-v");
+        if (ret != 0) {
+            ret = BUFFER_TOO_SMALL;
+            return ret;
+        }
+        options_string = get_docker_mount_options_string(mounts[i].options);
+
+        //magic number '+3': +1 for both ':', and +1 for NULL termination
+        len = strlen(mounts[i].src) + strlen(mounts[i].dest) + strlen(options_string) + 3;
+        mount_string = (char *) calloc(len, sizeof(*mount_string));
+
+        snprintf(mount_string, len, "%s:%s:%s", mounts[i].src, mounts[i].dest, options_string);
+
+        ret = add_to_args(args, mount_string);
+        if (ret != 0) {
+            ret = BUFFER_TOO_SMALL;
+            goto free_and_exit;
+        }
+        free(mount_string);
+        free(options_string);
+    }
+    return ret;
+
+free_and_exit:
+    free(mount_string);
+    free(options_string);
+    return ret;
+}
+
+static int get_docker_mounts(mount *mounts, char **mounts_string, unsigned int num_mounts) {
+    unsigned int i;
+    int ret = 0;
+    char *src, *dest;
+    mount_options *options = NULL;
+
+    if (mounts_string == NULL) {
+        fprintf(ERRORFILE, "Unable to normalize container-executor.cfg path\n");
+        ret = MOUNT_ACCESS_ERROR;
+        goto free_and_exit;
+    }
+
+    for (i = 0; i < num_mounts; i++) {
+        src = get_docker_mount_source(mounts_string[i]);
+        if (src == NULL) {
+            fprintf(ERRORFILE, "Invalid mount '%s'\n", mounts_string[i]);
+            ret = INVALID_MOUNT;
+            goto free_and_exit;
+        }
+        mounts[i].src = src;
+
+        dest = get_docker_mount_dest(mounts_string[i]);
+        if (dest == NULL) {
+            fprintf(ERRORFILE, "Invalid mount '%s'\n", mounts_string[i]);
+            ret = INVALID_MOUNT;
+            goto free_and_exit;
+        }
+        mounts[i].dest = dest;
+
+        options = get_docker_mount_options(mounts_string[i]);
+        if (options == NULL) {
+            fprintf(ERRORFILE, "Invalid mount '%s'\n", mounts_string[i]);
+            ret = INVALID_MOUNT;
+            goto free_and_exit;
+        }
+        mounts[i].options = options;
+    }
+
+free_and_exit:
+    return ret;
+}
+
+static int add_docker_mounts(const struct configuration *command_config, const struct configuration *conf, args *args) {
+    mount *mounts = NULL;
+    unsigned int num_mounts = 0;
+    int ret;
+    char **permitted_ro_mounts = NULL;
+    char **permitted_rw_mounts = NULL;
+    char **mounts_string = NULL;
+    permitted_ro_mounts = get_configuration_values_delimiter("docker.allowed.ro-mounts",
+                                                             CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ",");
+    permitted_rw_mounts = get_configuration_values_delimiter("docker.allowed.rw-mounts",
+                                                             CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ",");
+    mounts_string = get_configuration_values_delimiter("mounts", DOCKER_COMMAND_FILE_SECTION, command_config, ",");
+
+    if (mounts_string == NULL) {
+        ret = 0;
+        goto free_and_exit;
+    }
+
+    while (mounts_string[num_mounts] != NULL) {
+        num_mounts++;
+    }
+
+    // Disable mount volumes if image is not trusted.
+    if (check_trusted_image(command_config, conf) != 0) {
+        fprintf(ERRORFILE, "Disable mount volume for untrusted image\n");
+        // YARN will implicitly bind node manager local directory to
+        // docker image.  This can create file system security holes,
+        // if docker container has binary to escalate privileges.
+        // For untrusted image, we drop mounting without reporting
+        // INVALID_MOUNT messages to allow running untrusted
+        // image in a sandbox.
+        ret = 0;
+        goto free_and_exit;
+    }
+
+    mounts = (mount *) calloc(num_mounts, sizeof(*mounts));
+    if (mounts == NULL) {
+        fprintf(ERRORFILE, "Unable to allocate %ld bytes\n", num_mounts * sizeof(*mounts));
+        ret = OUT_OF_MEMORY;
+        goto free_and_exit;
+    }
+
+    ret = get_docker_mounts(mounts, mounts_string, num_mounts);
+    if (ret != 0) {
+        goto free_and_exit;
+    }
+
+    ret = validate_mounts(permitted_ro_mounts, permitted_rw_mounts, mounts, num_mounts);
+    if (ret != 0) {
+        goto free_and_exit;
+    }
+
+    ret = add_mounts_to_docker_args(mounts, num_mounts, args);
+    if (ret != 0) {
+        goto free_and_exit;
+    }
+
+free_and_exit:
+    free_values(permitted_ro_mounts);
+    free_values(permitted_rw_mounts);
+    free_values(mounts_string);
+    free_mounts(mounts, num_mounts);
+    return ret;
+}
+
 int get_docker_run_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0, i = 0;
   char *container_name = NULL, *user = NULL, *image = NULL;
@@ -1622,8 +1555,8 @@ int get_docker_run_command(const char *command_file, const struct configuration
   }
 
   service_mode_enabled = is_service_mode_enabled(&command_config, conf, args);
-  if (service_mode_enabled == SERVICE_MODE_DISABLED) {
-    ret = SERVICE_MODE_DISABLED;
+  if (service_mode_enabled == DOCKER_SERVICE_MODE_DISABLED) {
+    ret = DOCKER_SERVICE_MODE_DISABLED;
     goto free_and_exit;
   }
 
@@ -1721,7 +1654,7 @@ int get_docker_run_command(const char *command_file, const struct configuration
     goto free_and_exit;
   }
 
-  ret = add_mounts(&command_config, conf, args);
+  ret = add_docker_mounts(&command_config, conf, args);
   if (ret != 0) {
     goto free_and_exit;
   }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.h

@@ -45,37 +45,6 @@ typedef struct args {
     char *data[DOCKER_ARG_MAX];
 } args;
 
-enum docker_error_codes {
-    INVALID_COMMAND_FILE = 1,
-    INCORRECT_COMMAND,
-    BUFFER_TOO_SMALL,
-    INVALID_DOCKER_CONTAINER_NAME,
-    INVALID_DOCKER_IMAGE_NAME,
-    INVALID_DOCKER_USER_NAME,
-    INVALID_DOCKER_INSPECT_FORMAT,
-    UNKNOWN_DOCKER_COMMAND,
-    INVALID_DOCKER_NETWORK,
-    INVALID_DOCKER_PORTS_MAPPING,
-    INVALID_DOCKER_CAPABILITY,
-    PRIVILEGED_CONTAINERS_DISABLED,
-    INVALID_DOCKER_MOUNT,
-    INVALID_DOCKER_RO_MOUNT,
-    INVALID_DOCKER_RW_MOUNT,
-    MOUNT_ACCESS_ERROR,
-    INVALID_DOCKER_DEVICE,
-    INVALID_DOCKER_STOP_COMMAND,
-    INVALID_DOCKER_KILL_COMMAND,
-    INVALID_DOCKER_VOLUME_DRIVER,
-    INVALID_DOCKER_VOLUME_NAME,
-    INVALID_DOCKER_VOLUME_COMMAND,
-    PID_HOST_DISABLED,
-    INVALID_PID_NAMESPACE,
-    INVALID_DOCKER_IMAGE_TRUST,
-    INVALID_DOCKER_TMPFS_MOUNT,
-    INVALID_DOCKER_RUNTIME,
-    SERVICE_MODE_DISABLED
-};
-
 /**
  * Get the full path for the docker binary.
  * @param conf Configuration for the container-executor

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/mount-utils.c

@@ -0,0 +1,356 @@
+/**
+ * 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.
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "util.h"
+#include "mount-utils.h"
+#include "configuration.h"
+
+/**
+ * Function to free an options struct.
+ * @param options - options struct to be freed.
+ * @return void.
+ */
+void free_mount_options(mount_options *options) {
+  if (options == NULL) {
+      return;
+  }
+
+  if (options->opts != NULL) {
+      for (unsigned int i = 0; i < options->num_opts; i++) {
+          free(options->opts[i]);
+      }
+      free(options->opts);
+  }
+
+  free(options);
+}
+
+/**
+ * Function to free an array of mounts.
+ * @param mounts - Array of mounts to be freed.
+ * @param num_mounts - Number of mounts to be freed.
+ * @return void.
+ */
+void free_mounts(mount *mounts, const unsigned int num_mounts) {
+    if (mounts == NULL) {
+        return;
+    }
+
+    for (unsigned int i = 0; i < num_mounts; i++) {
+       free(mounts[i].src);
+       free(mounts[i].dest);
+       free_mount_options(mounts[i].options);
+    }
+    free(mounts);
+}
+
+/**
+ * Function to determine whether a string is a volume name.
+ * @param volume_name - string to check.
+ * @return 1 on match, 0 on no match.
+ */
+static int is_volume_name(const char *volume_name) {
+    const char *regex_str = "^[a-zA-Z0-9]([a-zA-Z0-9_.-]*)$";
+    // execute_regex_match return 0 is matched success
+    return execute_regex_match(regex_str, volume_name) == 0;
+}
+
+/**
+ * Function to determine whether a string is a valid volume name and if
+ * it matches a passed in regex.
+ * @param requested - string to check.
+ * @param pattern - regular expression to check against.
+ * @return 1 on match, 0 on no match.
+ */
+static int is_volume_name_matched_by_regex(const char* requested, const char* pattern) {
+    // execute_regex_match return 0 is matched success
+    return is_volume_name(requested) && (execute_regex_match(pattern + sizeof("regex:"), requested) == 0);
+}
+
+/**
+ * Helper function to help normalize mounts for checking if mounts are
+ * permitted. The function does the following -
+ * 1. Find the canonical path for mount using realpath
+ * 2. If the path is a directory, add a '/' at the end (if not present)
+ * 3. Return a copy of the canonicalised path(to be freed by the caller)
+ * @param mount path to be canonicalised.
+ * @param isRegexAllowed whether regex matching is allowed for normalize mount.
+ * @return pointer to canonicalised path, NULL on error.
+ */
+static char* normalize_mount(const char* mount, const int isRegexAllowed) {
+    int ret = 0;
+    struct stat buff;
+    char *ret_ptr = NULL, *real_mount = NULL;
+    if (mount == NULL) {
+        return NULL;
+    }
+    real_mount = realpath(mount, NULL);
+    if (real_mount == NULL) {
+        // If mount is a valid named volume, just return it and let the container runtime decide
+        if (is_volume_name(mount)) {
+            ret_ptr = strdup(mount);
+            goto free_and_exit;
+        }
+        // we only allow permitted mount to be REGEX, for permitted mount, we check
+        // if it's a valid REGEX return; for user mount, we need to strictly check
+        if (isRegexAllowed) {
+            if (is_regex(mount)) {
+                ret_ptr = strdup(mount);
+                goto free_and_exit;
+            }
+        }
+        fprintf(ERRORFILE, "Could not determine real path of mount '%s'\n", mount);
+        ret_ptr = NULL;
+        goto free_and_exit;
+    }
+    ret = stat(real_mount, &buff);
+    if (ret == 0) {
+        if (S_ISDIR(buff.st_mode)) {
+            size_t len = strlen(real_mount);
+            if (len <= 0) {
+                ret_ptr = NULL;
+                goto free_and_exit;
+            }
+            if (real_mount[len - 1] != '/') {
+                ret_ptr = (char *) alloc_and_clear_memory(len + 2, sizeof(char));
+                strncpy(ret_ptr, real_mount, len);
+                ret_ptr[len] = '/';
+                ret_ptr[len + 1] = '\0';
+            } else {
+                ret_ptr = strdup(real_mount);
+            }
+        } else {
+            ret_ptr = strdup(real_mount);
+        }
+    } else {
+        fprintf(ERRORFILE, "Could not stat path '%s'\n", real_mount);
+        ret_ptr = NULL;
+    }
+
+free_and_exit:
+    free(real_mount);
+    return ret_ptr;
+}
+
+/**
+ * Function to normalize an array of strings. Each string in the array will
+ * be normalized to its real path in the file system.
+ * @param mounts - An array of strings to normalize. The contents of this
+ * string array will be modified and replaced with their normalized equivalents.
+ * If a string is replaced, the original string will be freed. The caller is responsible
+ * for freeing the mounts string array.
+ * @param isRegexAllowed - Integer to determine whether or not regex is allowed.
+ * for any of the strings. 1 for allowed, 0 for not allowed.
+ * @return 0 on success, -1 on failure.
+ */
+static int normalize_mounts(char **mounts, const int isRegexAllowed) {
+    unsigned int i = 0;
+    char *tmp = NULL;
+    if (mounts == NULL) {
+        return 0;
+    }
+    for (i = 0; mounts[i] != NULL; ++i) {
+        tmp = normalize_mount(mounts[i], isRegexAllowed);
+        if (tmp == NULL) {
+            return -1;
+        }
+        free(mounts[i]);
+        mounts[i] = tmp;
+    }
+    return 0;
+}
+
+/**
+ * Function to get the normalized path of the container-executor config file.
+ * @param container_executor_cfg_path - A pointer to a string. This pointer will
+ * point to an allocated string containing the normalized path to the container-executor
+ * config file. The caller is responsible for freeing this memory.
+ * @return 0 on success, MOUNT_ACCESS_ERROR on failure.
+ */
+static int get_normalized_config_path(const char **container_executor_cfg_path) {
+    char *config_path = NULL;
+    int ret = 0;
+
+    config_path = get_config_path("");
+    *container_executor_cfg_path = normalize_mount(config_path, 0);
+    if (*container_executor_cfg_path == NULL) {
+        ret = MOUNT_ACCESS_ERROR;
+        goto free_and_exit;
+    }
+
+free_and_exit:
+    free(config_path);
+    return ret;
+}
+
+/**
+ * Function to determine whether or not a requested string path is allowed as a mount.
+ * The requested string will be normalized and checked against the normalized paths in the
+ * permitted_mounts string array. Volumes that match a regex in the permitted_mounts list
+ * will also be allowed.
+ * @param permitted_mounts - An array of strings that define the permitted list of paths
+ * for the requested string.
+ * @param requested - A string that is requested to be mounted.
+ * @return 0 on not permitted, 1 on permitted, -1 on error.
+ */
+static int check_mount_permitted(const char **permitted_mounts, const char *requested) {
+    int ret = 0;
+    unsigned int i;
+    size_t permitted_mount_len = 0;
+    if (permitted_mounts == NULL) {
+        return 0;
+    }
+    char *normalized_path = normalize_mount(requested, 0);
+    if (normalized_path == NULL) {
+        return -1;
+    }
+    for (i = 0; permitted_mounts[i] != NULL; ++i) {
+        if (strcmp(normalized_path, permitted_mounts[i]) == 0) {
+            ret = 1;
+            break;
+        }
+        // if (permitted_mounts[i] is a REGEX): use REGEX to compare; return
+        if (is_regex(permitted_mounts[i]) &&
+            is_volume_name_matched_by_regex(normalized_path, permitted_mounts[i])) {
+            ret = 1;
+            break;
+        }
+
+        // directory check
+        permitted_mount_len = strlen(permitted_mounts[i]);
+        struct stat path_stat;
+        stat(permitted_mounts[i], &path_stat);
+        if (S_ISDIR(path_stat.st_mode)) {
+            if (strncmp(normalized_path, permitted_mounts[i], permitted_mount_len) == 0) {
+                ret = 1;
+                break;
+            }
+        }
+    }
+    free(normalized_path);
+    return ret;
+}
+
+/**
+ * Function to validate whether a requested mount path is permitted or not. The normalized mount path
+ * must be in the correct permitted list based on the type of mount (ro or rw) and must not be a
+ * parent of the container-executor config file.
+ * @param permitted_ro_mounts - Array of permitted read-only mounts.
+ * @param permitted_rw_mounts - Array of permitted read-write mounts.
+ * @param requested  - Mount path to be validated
+ * @return 0 on valid mount, INVALID_MOUNT, INVALID_RW_MOUNT, INVALID_RO_MOUNT,
+ * or MOUNT_ACCESS_ERROR on error.
+ */
+static int validate_mount(const char **permitted_ro_mounts, const char **permitted_rw_mounts, const mount *requested) {
+    const char *container_executor_cfg_path = NULL;
+    const char *tmp_path_buffer[2] = {NULL, NULL};
+    int permitted_rw, permitted_ro;
+    int ret = 0;
+
+    if (requested == NULL) {
+        goto free_and_exit;
+    }
+
+    ret = get_normalized_config_path(&container_executor_cfg_path);
+    if (ret != 0) {
+        goto free_and_exit;
+    }
+
+    permitted_rw = check_mount_permitted(permitted_rw_mounts, requested->src);
+    permitted_ro = check_mount_permitted(permitted_ro_mounts, requested->src);
+
+    if (permitted_ro == -1 || permitted_rw == -1) {
+        fprintf(ERRORFILE, "Invalid mount src='%s', dest='%s'\n",
+                requested->src, requested->dest);
+        ret = INVALID_MOUNT;
+        goto free_and_exit;
+    }
+
+    if (requested->options != NULL && requested->options->rw == 1) {
+        // rw mount
+        if (permitted_rw == 0) {
+            fprintf(ERRORFILE, "Configuration does not allow mount src='%s', dest='%s'\n",
+                    requested->src, requested->dest);
+            ret = INVALID_RW_MOUNT;
+            goto free_and_exit;
+        } else {
+            // determine if the user can modify the container-executor.cfg file
+            tmp_path_buffer[0] = normalize_mount(requested->src, 0);
+            // just re-use the function, flip the args to check if the container-executor path is in the requested
+            // mount point
+            ret = check_mount_permitted(tmp_path_buffer, container_executor_cfg_path);
+            free((void *) tmp_path_buffer[0]);
+            if (ret == 1) {
+                fprintf(ERRORFILE, "Attempting to mount a parent directory of container-executor.cfg as read-write. src='%s', dest='%s'\n",
+                        requested->src, requested->dest);
+                ret = INVALID_RW_MOUNT;
+                goto free_and_exit;
+            }
+        }
+    } else {
+        // ro mount
+        if (permitted_ro == 0 && permitted_rw == 0) {
+            fprintf(ERRORFILE, "Configuration does not allow mount src='%s', dest='%s'\n",
+                    requested->src, requested->dest);
+            ret = INVALID_RO_MOUNT;
+            goto free_and_exit;
+        }
+    }
+
+free_and_exit:
+    free((void *) container_executor_cfg_path);
+    return ret;
+}
+
+/**
+ * Function to validate an array of mounts.
+ * @param permitted_ro_mounts - Array of permitted read-only mounts.
+ * @param permitted_rw_mounts - Array of permitted read-write mounts.
+ * @param mounts - Array of mounts to be validated.
+ * @param num_mounts - Number of mounts to be valildated.
+ * @return 0 on valid mounts, INVALID_MOUNT, INVALID_RW_MOUNT, INVALID_RO_MOUNT,
+ * or MOUNT_ACCESS_ERROR on error.
+ */
+int validate_mounts(char **permitted_ro_mounts, char **permitted_rw_mounts, mount *mounts, const unsigned int num_mounts) {
+    int ret = 0;
+    unsigned int i;
+
+    ret = normalize_mounts(permitted_ro_mounts, 1);
+    ret |= normalize_mounts(permitted_rw_mounts, 1);
+    if (ret != 0) {
+        fprintf(ERRORFILE, "Unable to find permitted mounts on disk\n");
+        ret = MOUNT_ACCESS_ERROR;
+        goto free_and_exit;
+    }
+
+    for (i = 0; i < num_mounts; i++) {
+        ret = validate_mount((const char **) permitted_ro_mounts, (const char **) permitted_rw_mounts, &mounts[i]);
+        if (ret != 0) {
+            goto free_and_exit;
+        }
+    }
+
+free_and_exit:
+    return ret;
+}

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/mount-utils.h

@@ -0,0 +1,40 @@
+/**
+ * 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.
+ */
+
+#ifndef __YARN_POSIX_CONTAINER_EXECUTOR_MOUNT_UTIL_H__
+#define __YARN_POSIX_CONTAINER_EXECUTOR_MOUNT_UTIL_H__
+
+typedef struct mount_options_struct {
+  char **opts;
+  unsigned int num_opts;
+  unsigned int rw; // 0 for read, 1 for write
+} mount_options;
+
+typedef struct mount_struct {
+    char *src;
+    char *dest;
+    mount_options *options;
+} mount;
+
+void free_mount_options(mount_options *options);
+
+void free_mounts(mount *mounts, const unsigned int num_mounts);
+
+int validate_mounts(char **permitted_ro_mounts, char **permitted_rw_mounts, mount *mounts, unsigned int num_mounts);
+
+#endif

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/utils/test_docker_util.cc

@@ -22,6 +22,7 @@
 
 extern "C" {
 #include "utils/docker-util.c"
+#include "utils/mount-utils.c"
 }
 
 namespace ContainerExecutor {
@@ -121,7 +122,7 @@ namespace ContainerExecutor {
       for (itr = file_cmd_vec.begin(); itr != file_cmd_vec.end(); ++itr) {
         write_command_file(itr->first);
         int ret = (*docker_func)(docker_command_file.c_str(), &container_executor_cfg, &tmp);
-        ASSERT_EQ(0, ret) << "error message: " << get_docker_error_message(ret) << " for input " << itr->first;
+        ASSERT_EQ(0, ret) << "error message: " << get_error_message(ret) << " for input " << itr->first;
         char *actual = flatten(&tmp);
         ASSERT_STREQ(itr->second.c_str(), actual);
         reset_args(&tmp);
@@ -136,7 +137,7 @@ namespace ContainerExecutor {
         reset_args(&tmp);
       }
       int ret = (*docker_func)("unknown-file", &container_executor_cfg, &tmp);
-      ASSERT_EQ(static_cast<int>(INVALID_COMMAND_FILE), ret);
+      ASSERT_EQ(static_cast<int>(INVALID_DOCKER_COMMAND_FILE), ret);
       reset_args(&tmp);
     }
 
@@ -181,10 +182,10 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  format='{{.State.Status}}'",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "docker-command=inspect\n  format='{{.State.Status}}'",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=inspect\n  format={{.State.Status}}\n  name=",
         static_cast<int>(INVALID_DOCKER_CONTAINER_NAME)));
@@ -218,9 +219,9 @@ namespace ContainerExecutor {
 
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "[docker-command-execution]\n  docker-command=run\n  image=image-id", static_cast<int>(INCORRECT_COMMAND)));
+        "[docker-command-execution]\n  docker-command=run\n  image=image-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "docker-command=load\n  image=image-id", static_cast<int>(INCORRECT_COMMAND)));
+        "docker-command=load\n  image=image-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=load\n  image=", static_cast<int>(INVALID_DOCKER_IMAGE_NAME)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>("[docker-command-execution]\n  docker-command=load",
@@ -288,9 +289,9 @@ namespace ContainerExecutor {
 
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "[docker-command-execution]\n  docker-command=run\n  image=image-id", static_cast<int>(INCORRECT_COMMAND)));
+        "[docker-command-execution]\n  docker-command=run\n  image=image-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "docker-command=pull\n  image=image-id", static_cast<int>(INCORRECT_COMMAND)));
+        "docker-command=pull\n  image=image-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=pull\n  image=", static_cast<int>(INVALID_DOCKER_IMAGE_NAME)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>("[docker-command-execution]\n  docker-command=pull",
@@ -309,9 +310,9 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  name=container_e1_12312_11111_02_000001",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "docker-command=rm\n  name=ctr-id", static_cast<int>(INCORRECT_COMMAND)));
+        "docker-command=rm\n  name=ctr-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=rm\n  name=", static_cast<int>(INVALID_DOCKER_CONTAINER_NAME)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -332,9 +333,9 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  name=container_e1_12312_11111_02_000001",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "docker-command=stop\n  name=ctr-id", static_cast<int>(INCORRECT_COMMAND)));
+        "docker-command=stop\n  name=ctr-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=stop\n  name=", static_cast<int>(INVALID_DOCKER_CONTAINER_NAME)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -359,9 +360,9 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  name=container_e1_12312_11111_02_000001",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "docker-command=kill\n  name=ctr-id", static_cast<int>(INCORRECT_COMMAND)));
+        "docker-command=kill\n  name=ctr-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=kill\n  name=", static_cast<int>(INVALID_DOCKER_CONTAINER_NAME)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -382,9 +383,9 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  name=container_e1_12312_11111_02_000001",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
-        "docker-command=start\n  name=ctr-id", static_cast<int>(INCORRECT_COMMAND)));
+        "docker-command=start\n  name=ctr-id", static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=start\n  name=", static_cast<int>(INVALID_DOCKER_CONTAINER_NAME)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -474,7 +475,7 @@ namespace ContainerExecutor {
       }
       ret = set_runtime(&cmd_cfg, &container_cfg, &buff);
       char *actual = flatten(&buff);
-      ASSERT_EQ(0, ret) << "error message: " << get_docker_error_message(ret) << " for input " << itr->first;
+      ASSERT_EQ(0, ret) << "error message: " << get_error_message(ret) << " for input " << itr->first;
       ASSERT_STREQ(itr->second.c_str(), actual);
       reset_args(&buff);
       free(actual);
@@ -599,7 +600,7 @@ namespace ContainerExecutor {
       }
       ret = add_ports_mapping_to_command(&cmd_cfg, &buff);
       char *actual = flatten(&buff);
-      ASSERT_EQ(0, ret) << "error message: " << get_docker_error_message(ret) << " for input " << itr->first;
+      ASSERT_EQ(0, ret) << "error message: " << get_error_message(ret) << " for input " << itr->first;
       ASSERT_STREQ(itr->second.c_str(), actual);
       reset_args(&buff);
       free(actual);
@@ -655,7 +656,7 @@ namespace ContainerExecutor {
         "[docker-command-execution]\n  docker-command=run", ""));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n pid=other",
-        static_cast<int>(INVALID_PID_NAMESPACE)));
+        static_cast<int>(INVALID_DOCKER_PID_NAMESPACE)));
 
     for (int i = 1; i < 3; ++i) {
       write_container_executor_cfg(container_executor_cfg_contents[0]);
@@ -719,10 +720,10 @@ namespace ContainerExecutor {
       bad_file_cmd_vec.clear();
       bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n pid=other",
-        static_cast<int>(INVALID_PID_NAMESPACE)));
+        static_cast<int>(INVALID_DOCKER_PID_NAMESPACE)));
       bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n pid=host",
-        static_cast<int>(PID_HOST_DISABLED)));
+        static_cast<int>(DOCKER_PID_HOST_DISABLED)));
       for (itr2 = bad_file_cmd_vec.begin(); itr2 != bad_file_cmd_vec.end(); ++itr2) {
         write_command_file(itr2->first);
         ret = read_config(docker_command_file.c_str(), &cmd_cfg);
@@ -816,7 +817,7 @@ namespace ContainerExecutor {
           FAIL();
         }
         ret = set_privileged(&cmd_cfg, &container_cfg, &buff);
-        ASSERT_EQ(6, ret);
+        ASSERT_EQ(INVALID_DOCKER_USER_NAME, ret);
         ASSERT_EQ(0, buff.length);
         reset_args(&buff);
         free_configuration(&cmd_cfg);
@@ -827,7 +828,7 @@ namespace ContainerExecutor {
         FAIL();
       }
       ret = set_privileged(&cmd_cfg, &container_cfg, &buff);
-      ASSERT_EQ(PRIVILEGED_CONTAINERS_DISABLED, ret);
+      ASSERT_EQ(PRIVILEGED_DOCKER_CONTAINERS_DISABLED, ret);
       ASSERT_EQ(0, buff.length);
       reset_args(&buff);
       free_configuration(&cmd_cfg);
@@ -864,7 +865,7 @@ namespace ContainerExecutor {
         FAIL();
       }
       ret = set_privileged(&cmd_cfg, &container_cfg, &buff);
-      ASSERT_EQ(PRIVILEGED_CONTAINERS_DISABLED, ret);
+      ASSERT_EQ(PRIVILEGED_DOCKER_CONTAINERS_DISABLED, ret);
       ASSERT_EQ(0, buff.length);
       reset_args(&buff);
       free_configuration(&cmd_cfg);
@@ -1069,7 +1070,7 @@ namespace ContainerExecutor {
   }
 
 
-  TEST_F(TestDockerUtil, test_add_mounts) {
+  TEST_F(TestDockerUtil, test_add_docker_mounts) {
     struct configuration container_cfg, cmd_cfg;
     struct args buff = ARGS_INITIAL_VALUE;
     int ret = 0;
@@ -1125,7 +1126,7 @@ namespace ContainerExecutor {
       if (ret != 0) {
         FAIL();
       }
-      ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
+      ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
       char *actual = flatten(&buff);
       ASSERT_EQ(0, ret);
       ASSERT_STREQ(itr->second.c_str(), actual);
@@ -1137,22 +1138,22 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmds_vec;
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  image=hadoop/image\n  mounts=/lib:/lib:rw",
-        static_cast<int>(INVALID_DOCKER_RW_MOUNT)));
+        static_cast<int>(INVALID_RW_MOUNT)));
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  image=hadoop/image\n  mounts=/usr/bin/:/usr/bin:rw",
-        static_cast<int>(INVALID_DOCKER_RW_MOUNT)));
+        static_cast<int>(INVALID_RW_MOUNT)));
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  image=hadoop/image\n  mounts=/blah:/blah:rw",
-        static_cast<int>(INVALID_DOCKER_MOUNT)));
+        static_cast<int>(INVALID_MOUNT)));
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n image=hadoop/image\n mounts=/tmp:/tmp:shared",
-        static_cast<int>(INVALID_DOCKER_MOUNT)));
+        static_cast<int>(INVALID_MOUNT)));
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n image=hadoop/image\n mounts=/lib:/lib",
-        static_cast<int>(INVALID_DOCKER_MOUNT)));
+        static_cast<int>(INVALID_MOUNT)));
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n image=hadoop/image\n mounts=/lib:/lib:other",
-        static_cast<int>(INVALID_DOCKER_MOUNT)));
+        static_cast<int>(INVALID_MOUNT)));
 
     std::vector<std::pair<std::string, int> >::const_iterator itr2;
 
@@ -1162,7 +1163,7 @@ namespace ContainerExecutor {
       if (ret != 0) {
         FAIL();
       }
-      ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
+      ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
       char *actual = flatten(&buff);
       ASSERT_EQ(itr2->second, ret);
       ASSERT_STREQ("", actual);
@@ -1181,8 +1182,8 @@ namespace ContainerExecutor {
       if (ret != 0) {
         FAIL();
       }
-      ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
-      ASSERT_EQ(INVALID_DOCKER_RW_MOUNT, ret) << " for input " << cmd_file_contents;
+      ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
+      ASSERT_EQ(INVALID_RW_MOUNT, ret) << " for input " << cmd_file_contents;
       char *actual = flatten(&buff);
       ASSERT_STREQ("", actual);
       reset_args(&buff);
@@ -1196,7 +1197,7 @@ namespace ContainerExecutor {
     free(ce_path);
     free_configuration(&container_cfg);
 
-    // For untrusted image, container add_mounts will pass through
+    // For untrusted image, container add_docker_mounts will pass through
     // without mounting or report error code.
     container_executor_cfg_contents = "[docker]\n";
     write_container_executor_cfg(container_executor_cfg_contents);
@@ -1204,7 +1205,7 @@ namespace ContainerExecutor {
     if (ret != 0) {
       FAIL();
     }
-    ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
+    ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
     char *actual = flatten(&buff);
     ASSERT_EQ(0, ret);
     ASSERT_STREQ("", actual);
@@ -1266,7 +1267,7 @@ namespace ContainerExecutor {
       if (ret != 0) {
         FAIL();
       }
-      ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
+      ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
       char *actual = flatten(&buff);
       ASSERT_EQ(0, ret);
       ASSERT_STREQ(itr->second.c_str(), actual);
@@ -1278,10 +1279,10 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmds_vec;
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  image=hadoop/image\n  mounts=/etc:/etc:ro",
-        static_cast<int>(INVALID_DOCKER_RO_MOUNT)));
+        static_cast<int>(INVALID_RO_MOUNT)));
     bad_file_cmds_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  image=hadoop/image\n  mounts=/blah:/blah:ro",
-        static_cast<int>(INVALID_DOCKER_MOUNT)));
+        static_cast<int>(INVALID_MOUNT)));
 
     std::vector<std::pair<std::string, int> >::const_iterator itr2;
 
@@ -1291,7 +1292,7 @@ namespace ContainerExecutor {
       if (ret != 0) {
         FAIL();
       }
-      ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
+      ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
       char *actual = flatten(&buff);
       ASSERT_EQ(itr2->second, ret);
       ASSERT_STREQ("", actual);
@@ -1312,8 +1313,8 @@ namespace ContainerExecutor {
     if (ret != 0) {
       FAIL();
     }
-    ret = add_mounts(&cmd_cfg, &container_cfg, &buff);
-    ASSERT_EQ(INVALID_DOCKER_RO_MOUNT, ret);
+    ret = add_docker_mounts(&cmd_cfg, &container_cfg, &buff);
+    ASSERT_EQ(INVALID_RO_MOUNT, ret);
     ASSERT_EQ(0, buff.length);
     reset_args(&buff);
     free_configuration(&cmd_cfg);
@@ -1508,7 +1509,7 @@ namespace ContainerExecutor {
             "  network=bridge\n  privileged=true\n"
             "  cap-add=CHOWN,SETUID\n  cgroup-parent=ctr-cgroup\n  detach=true\n  rm=true\n  group-add=1000,1001\n"
             "  launch-command=bash,test_script.sh,arg1,arg2",
-        PRIVILEGED_CONTAINERS_DISABLED));
+        PRIVILEGED_DOCKER_CONTAINERS_DISABLED));
 
     // invalid rw mount
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -1518,7 +1519,7 @@ namespace ContainerExecutor {
             "  network=bridge\n  devices=/dev/test:/dev/test\n"
             "  cap-add=CHOWN,SETUID\n  cgroup-parent=ctr-cgroup\n  detach=true\n  rm=true\n"
             "  launch-command=bash,test_script.sh,arg1,arg2",
-        static_cast<int>(INVALID_DOCKER_RW_MOUNT)));
+        static_cast<int>(INVALID_RW_MOUNT)));
 
     // invalid ro mount
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -1528,7 +1529,7 @@ namespace ContainerExecutor {
             "  network=bridge\n  devices=/dev/test:/dev/test\n"
             "  cap-add=CHOWN,SETUID\n  cgroup-parent=ctr-cgroup\n  detach=true\n  rm=true\n"
             "  launch-command=bash,test_script.sh,arg1,arg2",
-        static_cast<int>(INVALID_DOCKER_RO_MOUNT)));
+        static_cast<int>(INVALID_RO_MOUNT)));
 
     // invalid capability
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -1548,7 +1549,7 @@ namespace ContainerExecutor {
             "  network=bridge\n  devices=/dev/dev1:/dev/dev1\n  privileged=true\n"
             "  cap-add=CHOWN,SETUID\n  cgroup-parent=ctr-cgroup\n  detach=true\n  rm=true\n"
             "  launch-command=bash,test_script.sh,arg1,arg2",
-        static_cast<int>(PRIVILEGED_CONTAINERS_DISABLED)));
+        static_cast<int>(PRIVILEGED_DOCKER_CONTAINERS_DISABLED)));
 
     // invalid network
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
@@ -1690,7 +1691,7 @@ namespace ContainerExecutor {
               "  network=bridge\n  devices=/dev/test:/dev/test\n  privileged=true\n"
               "  cap-add=CHOWN,SETUID\n  cgroup-parent=ctr-cgroup\n  detach=true\n  rm=true\n"
               "  launch-command=bash,test_script.sh,arg1,arg2",
-          static_cast<int>(PRIVILEGED_CONTAINERS_DISABLED)));
+          static_cast<int>(PRIVILEGED_DOCKER_CONTAINERS_DISABLED)));
 
       run_docker_command_test(file_cmd_vec, bad_file_cmd_vec, get_docker_run_command);
       free_configuration(&container_executor_cfg);
@@ -1978,10 +1979,10 @@ namespace ContainerExecutor {
     std::vector<std::pair<std::string, int> > bad_file_cmd_vec;
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "[docker-command-execution]\n  docker-command=run\n  image=image-id",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
     bad_file_cmd_vec.push_back(std::make_pair<std::string, int>(
         "docker-command=images\n  image=image-id",
-        static_cast<int>(INCORRECT_COMMAND)));
+        static_cast<int>(INCORRECT_DOCKER_COMMAND)));
 
     run_docker_command_test(file_cmd_vec, bad_file_cmd_vec,
       get_docker_images_command);