YARN-4245. Generalize config file handling in container-executor. Contributed by Sidharta Seethana.

(cherry picked from commit 8ed2e060e80c0def3fcb7604e0bd27c1c24d291e)
(cherry picked from commit 78919f8c341ec645cf9134991e3ae89a929b9184)

hadoop-yarn-project/CHANGES.txt

@@ -1,6 +1,21 @@
 Hadoop YARN Change Log
 
-elease 2.7.7 - 2018-07-18
+Release 2.7.8 - UNRELEASED
+
+  INCOMPATIBLE CHANGES
+
+  NEW FEATURES
+
+  IMPROVEMENTS
+
+    YARN-4245. Generalize config file handling in container-executor.
+    (Sidharta Seethana, Andrew Purtell via shv).
+
+  OPTIMIZATIONS
+
+  BUG FIXES
+
+Release 2.7.7 - 2018-07-18
 
   INCOMPATIBLE CHANGES
 

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

@@ -33,34 +33,22 @@
 
 #define MAX_SIZE 10
 
-struct confentry {
-  const char *key;
-  const char *value;
-};
-
-struct configuration {
-  int size;
-  struct confentry **confdetails;
-};
-
-struct configuration config={.size=0, .confdetails=NULL};
-
 //clean up method for freeing configuration
-void free_configurations() {
+void free_configurations(struct configuration *cfg) {
   int i = 0;
-  for (i = 0; i < config.size; i++) {
-    if (config.confdetails[i]->key != NULL) {
-      free((void *)config.confdetails[i]->key);
+  for (i = 0; i < cfg->size; i++) {
+    if (cfg->confdetails[i]->key != NULL) {
+      free((void *)cfg->confdetails[i]->key);
     }
-    if (config.confdetails[i]->value != NULL) {
-      free((void *)config.confdetails[i]->value);
+    if (cfg->confdetails[i]->value != NULL) {
+      free((void *)cfg->confdetails[i]->value);
     }
-    free(config.confdetails[i]);
+    free(cfg->confdetails[i]);
   }
-  if (config.size > 0) {
-    free(config.confdetails);
+  if (cfg->size > 0) {
+    free(cfg->confdetails);
   }
-  config.size = 0;
+  cfg->size = 0;
 }
 
 /**
@@ -137,8 +125,8 @@ int check_configuration_permissions(const char* file_name) {
   return 0;
 }
 
-//function used to load the configurations present in the secure config
-void read_config(const char* file_name) {
+
+void read_config(const char* file_name, struct configuration *cfg) {
   FILE *conf_file;
   char *line;
   char *equaltok;
@@ -156,9 +144,9 @@ void read_config(const char* file_name) {
   #endif
 
   //allocate space for ten configuration items.
-  config.confdetails = (struct confentry **) malloc(sizeof(struct confentry *)
+  cfg->confdetails = (struct confentry **) malloc(sizeof(struct confentry *)
       * MAX_SIZE);
-  config.size = 0;
+  cfg->size = 0;
   conf_file = fopen(file_name, "r");
   if (conf_file == NULL) {
     fprintf(ERRORFILE, "Invalid conf file provided : %s \n", file_name);
@@ -200,9 +188,9 @@ void read_config(const char* file_name) {
       free(line);
       continue;
     }
-    config.confdetails[config.size] = (struct confentry *) malloc(
+    cfg->confdetails[cfg->size] = (struct confentry *) malloc(
             sizeof(struct confentry));
-    if(config.confdetails[config.size] == NULL) {
+    if(cfg->confdetails[cfg->size] == NULL) {
       fprintf(LOGFILE,
           "Failed allocating memory for single configuration item\n");
       goto cleanup;
@@ -212,10 +200,10 @@ void read_config(const char* file_name) {
       fprintf(LOGFILE, "read_config : Adding conf key : %s \n", equaltok);
     #endif
 
-    memset(config.confdetails[config.size], 0, sizeof(struct confentry));
-    config.confdetails[config.size]->key = (char *) malloc(
+    memset(cfg->confdetails[cfg->size], 0, sizeof(struct confentry));
+    cfg->confdetails[cfg->size]->key = (char *) malloc(
             sizeof(char) * (strlen(equaltok)+1));
-    strcpy((char *)config.confdetails[config.size]->key, equaltok);
+    strcpy((char *)cfg->confdetails[cfg->size]->key, equaltok);
     equaltok = strtok_r(NULL, "=", &temp_equaltok);
     if (equaltok == NULL) {
       fprintf(LOGFILE, "configuration tokenization failed \n");
@@ -224,8 +212,8 @@ void read_config(const char* file_name) {
     //means value is commented so don't store the key
     if(equaltok[0] == '#') {
       free(line);
-      free((void *)config.confdetails[config.size]->key);
-      free(config.confdetails[config.size]);
+      free((void *)cfg->confdetails[cfg->size]->key);
+      free(cfg->confdetails[cfg->size]);
       continue;
     }
 
@@ -233,27 +221,29 @@ void read_config(const char* file_name) {
       fprintf(LOGFILE, "read_config : Adding conf value : %s \n", equaltok);
     #endif
 
-    config.confdetails[config.size]->value = (char *) malloc(
+    cfg->confdetails[cfg->size]->value = (char *) malloc(
             sizeof(char) * (strlen(equaltok)+1));
-    strcpy((char *)config.confdetails[config.size]->value, equaltok);
-    if((config.size + 1) % MAX_SIZE  == 0) {
-      config.confdetails = (struct confentry **) realloc(config.confdetails,
-          sizeof(struct confentry **) * (MAX_SIZE + config.size));
-      if (config.confdetails == NULL) {
+    strcpy((char *)cfg->confdetails[cfg->size]->value, equaltok);
+    if((cfg->size + 1) % MAX_SIZE  == 0) {
+      cfg->confdetails = (struct confentry **) realloc(cfg->confdetails,
+          sizeof(struct confentry **) * (MAX_SIZE + cfg->size));
+      if (cfg->confdetails == NULL) {
         fprintf(LOGFILE,
             "Failed re-allocating memory for configuration items\n");
         goto cleanup;
       }
     }
-    if(config.confdetails[config.size] )
-    config.size++;
+    if(cfg->confdetails[cfg->size]) {
+        cfg->size++;
+    }
+
     free(line);
   }
 
   //close the file
   fclose(conf_file);
 
-  if (config.size == 0) {
+  if (cfg->size == 0) {
     fprintf(ERRORFILE, "Invalid configuration provided in %s\n", file_name);
     exit(INVALID_CONFIG_FILE);
   }
@@ -266,7 +256,7 @@ void read_config(const char* file_name) {
     free(line);
   }
   fclose(conf_file);
-  free_configurations();
+  free_configurations(cfg);
   return;
 }
 
@@ -276,29 +266,17 @@ void read_config(const char* file_name) {
  * array, next time onwards used the populated array.
  *
  */
-char * get_value(const char* key) {
+char * get_value(const char* key, struct configuration *cfg) {
   int count;
-  for (count = 0; count < config.size; count++) {
-    if (strcmp(config.confdetails[count]->key, key) == 0) {
-      return strdup(config.confdetails[count]->value);
+  for (count = 0; count < cfg->size; count++) {
+    if (strcmp(cfg->confdetails[count]->key, key) == 0) {
+      return strdup(cfg->confdetails[count]->value);
     }
   }
   return NULL;
 }
 
-/**
- * Function to return an array of values for a key.
- * Value delimiter is assumed to be a comma.
- */
-char ** get_values(const char * key) {
-  char *value = get_value(key);
-  return extract_values(value);
-}
-
-/**
- * Extracts array of values from the comma separated list of values.
- */
-char ** extract_values(char *value) {
+char ** extract_values_delim(char *value, const char *delim) {
   char ** toPass = NULL;
   char *tempTok = NULL;
   char *tempstr = NULL;
@@ -324,6 +302,32 @@ char ** extract_values(char *value) {
   return toPass;
 }
 
+/**
+ * Extracts array of values from the comma separated list of values.
+ */
+char ** extract_values(char *value) {
+  return extract_values_delim(value, ",");
+}
+
+/**
+ * Function to return an array of values for a key.
+ * Value delimiter is assumed to be a comma.
+ */
+char ** get_values(const char * key, struct configuration *cfg) {
+  char *value = get_value(key, cfg);
+  return extract_values_delim(value, ",");
+}
+
+/**
+ * Function to return an array of values for a key, using the specified
+ delimiter.
+ */
+char ** get_values_delim(const char * key, struct configuration *cfg,
+    const char *delim) {
+  char *value = get_value(key, cfg);
+  return extract_values_delim(value, delim);
+}
+
 // free an entry set of values
 void free_values(char** values) {
   if (*values != NULL) {

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

@@ -37,15 +37,33 @@ int check_configuration_permissions(const char* file_name);
  */
 char *resolve_config_path(const char* file_name, const char *root);
 
-// read the given configuration file
-void read_config(const char* config_file);
+// Config data structures.
+struct confentry {
+  const char *key;
+  const char *value;
+};
+
+struct configuration {
+  int size;
+  struct confentry **confdetails;
+};
+
+// read the given configuration file into the specified config struct.
+void read_config(const char* config_file, struct configuration *cfg);
 
 //method exposed to get the configurations
-char *get_value(const char* key);
+char *get_value(const char* key, struct configuration *cfg);
 
 //function to return array of values pointing to the key. Values are
 //comma seperated strings.
-char ** get_values(const char* key);
+char ** get_values(const char* key, struct configuration *cfg);
+
+/**
+ * Function to return an array of values for a key, using the specified
+ delimiter.
+ */
+char ** get_values_delim(const char * key, struct configuration *cfg,
+    const char *delim);
 
 // Extracts array of values from the comma separated list of values.
 char ** extract_values(char *value);
@@ -54,7 +72,7 @@ char ** extract_values(char *value);
 void free_values(char** values);
 
 //method to free allocated configuration
-void free_configurations();
+void free_configurations(struct configuration *cfg);
 
 /**
  * If str is a string of the form key=val, find 'key'

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

@@ -71,6 +71,8 @@ FILE* ERRORFILE = NULL;
 static uid_t nm_uid = -1;
 static gid_t nm_gid = -1;
 
+struct configuration executor_cfg = {.size=0, .confdetails=NULL};
+
 char *concatenate(char *concat_pattern, char *return_path_name,
    int numArgs, ...);
 
@@ -79,6 +81,21 @@ void set_nm_uid(uid_t user, gid_t group) {
   nm_gid = group;
 }
 
+//function used to load the configurations present in the secure config
+void read_executor_config(const char* file_name) {
+    read_config(file_name, &executor_cfg);
+}
+
+//function used to free executor configuration data
+void free_executor_configurations() {
+    free_configurations(&executor_cfg);
+}
+
+//Lookup nodemanager group from container executor configuration.
+char *get_nodemanager_group() {
+    return get_value(NM_GROUP_KEY, &executor_cfg);
+}
+
 int check_executor_permissions(char *executable_file) {
 
   errno = 0;
@@ -678,7 +695,7 @@ static struct passwd* get_user_info(const char* user) {
 }
 
 int is_whitelisted(const char *user) {
-  char **whitelist = get_values(ALLOWED_SYSTEM_USERS_KEY);
+  char **whitelist = get_values(ALLOWED_SYSTEM_USERS_KEY, &executor_cfg);
   char **users = whitelist;
   if (whitelist != NULL) {
     for(; *users; ++users) {
@@ -706,7 +723,7 @@ struct passwd* check_user(const char *user) {
     fflush(LOGFILE);
     return NULL;
   }
-  char *min_uid_str = get_value(MIN_USERID_KEY);
+  char *min_uid_str = get_value(MIN_USERID_KEY, &executor_cfg);
   int min_uid = DEFAULT_MIN_USERID;
   if (min_uid_str != NULL) {
     char *end_ptr = NULL;
@@ -733,7 +750,7 @@ struct passwd* check_user(const char *user) {
     free(user_info);
     return NULL;
   }
-  char **banned_users = get_values(BANNED_USERS_KEY);
+  char **banned_users = get_values(BANNED_USERS_KEY, &executor_cfg);
   banned_users = banned_users == NULL ?
     (char**) DEFAULT_BANNED_USERS : banned_users;
   char **banned_user = banned_users;

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

@@ -75,10 +75,15 @@ extern FILE *LOGFILE;
 // the log file for error messages
 extern FILE *ERRORFILE;
 
-
 // get the executable's filename
 char* get_executable();
 
+//function used to load the configurations present in the secure config
+void read_executor_config(const char* file_name);
+
+//Lookup nodemanager group from container executor configuration.
+char *get_nodemanager_group();
+
 /**
  * Check the permissions on the container-executor to make sure that security is
  * permissible. For this, we need container-executor binary to
@@ -91,6 +96,12 @@ char* get_executable();
  */
 int check_executor_permissions(char *executable_file);
 
+//function used to load the configurations present in the secure config.
+void read_executor_config(const char* file_name);
+
+//function used to free executor configuration data
+void free_executor_configurations();
+
 // initialize the application directory
 int initialize_app(const char *user, const char *app_id,
                    const char *credentials, char* const* local_dirs,

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

@@ -144,11 +144,11 @@ int main(int argc, char **argv) {
     flush_and_close_log_files();
     exit(INVALID_CONFIG_FILE);
   }
-  read_config(conf_file);
+  read_executor_config(conf_file);
   free(conf_file);
 
   // look up the node manager group in the config file
-  char *nm_group = get_value(NM_GROUP_KEY);
+  char *nm_group = get_nodemanager_group();
   if (nm_group == NULL) {
     free(executable_file);
     fprintf(ERRORFILE, "Can't get configured value for %s.\n", NM_GROUP_KEY);

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

@@ -769,7 +769,7 @@ int main(int argc, char **argv) {
   if (write_config_file(TEST_ROOT "/test.cfg", 1) != 0) {
     exit(1);
   }
-  read_config(TEST_ROOT "/test.cfg");
+  read_executor_config(TEST_ROOT "/test.cfg");
 
   local_dirs = extract_values(strdup(NM_LOCAL_DIRS));
   log_dirs = extract_values(strdup(NM_LOG_DIRS));
@@ -872,14 +872,14 @@ int main(int argc, char **argv) {
   ret++;
   // test_delete_user must run as root since that's how we use the delete_as_user
   test_delete_user();
-  free_configurations();
+  free_executor_configurations();
 
   printf("\nTrying banned default user()\n");
   if (write_config_file(TEST_ROOT "/test.cfg", 0) != 0) {
     exit(1);
   }
 
-  read_config(TEST_ROOT "/test.cfg");
+  read_executor_config(TEST_ROOT "/test.cfg");
 #ifdef __APPLE__
   username = "_uucp";
   test_check_user(1);
@@ -898,6 +898,6 @@ int main(int argc, char **argv) {
   printf("\nFinished tests\n");
 
   free(current_username);
-  free_configurations();
+  free_executor_configurations();
   return 0;
 }