YARN-7321. Backport container-executor changes from YARN-6852 to branch-2. (Varun Vasudev via wangda)

Change-Id: Ib6434b5a5ebe591050c3949cac12ff9e2a2bbaa5

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

@@ -101,6 +101,8 @@ add_library(container
     main/native/container-executor/impl/container-executor.c
     main/native/container-executor/impl/get_executable.c
     main/native/container-executor/impl/utils/string-utils.c
+    main/native/container-executor/impl/utils/path-utils.c
+    main/native/container-executor/impl/modules/common/module-configs.c
 )
 
 add_executable(container-executor
@@ -113,12 +115,14 @@ target_link_libraries(container-executor
 
 output_directory(container-executor target/usr/local/bin)
 
+# Test cases
 add_executable(test-container-executor
     main/native/container-executor/test/test-container-executor.c
 )
 target_link_libraries(test-container-executor
     container ${EXTRA_LIBS}
 )
+
 output_directory(test-container-executor target/usr/local/bin)
 
 # unit tests for container executor
@@ -126,6 +130,8 @@ add_executable(cetest
         main/native/container-executor/impl/util.c
         main/native/container-executor/test/test_configuration.cc
         main/native/container-executor/test/test_main.cc
+        main/native/container-executor/test/utils/test-string-utils.cc
+        main/native/container-executor/test/utils/test-path-utils.cc
         main/native/container-executor/test/test_util.cc)
-target_link_libraries(cetest gtest)
+target_link_libraries(cetest gtest container)
 output_directory(cetest test)

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

@@ -521,7 +521,7 @@ int main(int argc, char **argv) {
   open_log_files();
   assert_valid_setup(argv[0]);
 
-  int operation;
+  int operation = -1;
   int ret = validate_arguments(argc, argv, &operation);
 
   if (ret != 0) {

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

@@ -0,0 +1,29 @@
+/**
+ * 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.
+ */
+
+/* FreeBSD protects the getline() prototype. See getline(3) for more */
+#ifdef __FreeBSD__
+#define _WITH_GETLINE
+#endif
+
+#ifndef _MODULES_COMMON_CONSTANTS_H_
+#define _MODULES_COMMON_CONSTANTS_H_
+
+#define CONFIGS_MODULES_PREFIX "yarn.container-executor.modules."
+
+#endif

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/modules/common/module-configs.c

@@ -0,0 +1,41 @@
+/**
+ * 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 "util.h"
+#include "configuration.h"
+#include "container-executor.h"
+#include "modules/common/constants.h"
+
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#define ENABLED_CONFIG_KEY "module.enabled"
+
+int module_enabled(const struct section* section_cfg, const char* module_name) {
+  char* enabled_str = get_section_value(ENABLED_CONFIG_KEY, section_cfg);
+  int enabled = 0;
+  if (enabled_str && 0 == strcmp(enabled_str, "true")) {
+    enabled = 1;
+  } else {
+    fprintf(LOGFILE, "Module %s is disabled\n", module_name);
+  }
+
+  free(enabled_str);
+  return enabled;
+}

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/modules/common/module-configs.h

@@ -0,0 +1,33 @@
+/**
+ * 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.
+ */
+
+#ifdef __FreeBSD__
+#define _WITH_GETLINE
+#endif
+
+#ifndef _MODULES_COMMON_MODULE_CONFIGS_H_
+#define _MODULES_COMMON_MODULE_CONFIGS_H_
+
+
+/**
+ * check if module enabled given name of module.
+ * return 0 if disabled
+ */
+int module_enabled(const struct section* section_cfg, const char* module_name);
+
+#endif

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

@@ -0,0 +1,52 @@
+/**
+ * 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 "util.h"
+
+#include <strings.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int verify_path_safety(const char* path) {
+  if (!path || path[0] == 0) {
+    return 1;
+  }
+
+  char* dup = strdup(path);
+  if (!dup) {
+    fprintf(ERRORFILE, "%s: Failed to allocate memory for path.\n", __func__);
+    return 0;
+  }
+
+  char* p = strtok(dup, "/");
+  int succeeded = 1;
+
+  while (p != NULL) {
+    if (0 == strcmp(p, "..")) {
+      fprintf(ERRORFILE, "%s: Path included \"..\", path=%s.\n", __func__, path);
+      succeeded = 0;
+      break;
+    }
+
+    p = strtok(NULL, "/");
+  }
+  free(dup);
+
+  return succeeded;
+}

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

@@ -0,0 +1,35 @@
+/**
+ * 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.
+ */
+
+#ifdef __FreeBSD__
+#define _WITH_GETLINE
+#endif
+
+#ifndef _UTILS_PATH_UTILS_H_
+#define _UTILS_PATH_UTILS_H_
+
+/*
+ * Verify if a given path is safe or not. For example, we don't want a path
+ * include ".." which can do things like:
+ * - "/cgroups/cpu,cpuacct/container/../../../etc/passwd"
+ *
+ * return false/true
+ */
+int verify_path_safety(const char* path);
+
+#endif

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

@@ -15,7 +15,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#include "util.h"
 
+#include <limits.h>
+#include <errno.h>
 #include <strings.h>
 #include <string.h>
 #include <stdio.h>
@@ -26,60 +29,131 @@
  * return true/false
  */
 static int all_numbers(char* input) {
-  if (0 == strlen(input)) {
+  for (; input[0] != 0; input++) {
-    return 0;
+    if (input[0] < '0' || input[0] > '9') {
-  }
-
-  for (int i = 0; i < strlen(input); i++) {
-    if (input[i] < '0' || input[i] > '9') {
       return 0;
     }
   }
   return 1;
 }
 
+int get_numbers_split_by_comma(const char* input, int** numbers,
+                               size_t* ret_n_numbers) {
+  size_t allocation_size = 1;
+  int i = 0;
+  while (input[i] != 0) {
+    if (input[i] == ',') {
+      allocation_size++;
+    }
+    i++;
+  }
+
+  (*numbers) = malloc(sizeof(int) * allocation_size);
+  if (!(*numbers)) {
+    fprintf(ERRORFILE, "Failed to allocating memory for *numbers: %s\n",
+            __func__);
+    exit(OUT_OF_MEMORY);
+  }
+  memset(*numbers, 0, sizeof(int) * allocation_size);
+
+  char* input_cpy = strdup(input);
+  if (!input_cpy) {
+    fprintf(ERRORFILE, "Failed to allocating memory for input_cpy: %s\n",
+            __func__);
+    exit(OUT_OF_MEMORY);
+  }
+
+  char* p = strtok(input_cpy, ",");
+  int idx = 0;
+  size_t n_numbers = 0;
+  while (p != NULL) {
+    char *temp;
+    long n = strtol(p, &temp, 0);
+    // According to answer:
+    // https://stackoverflow.com/questions/14176123/correct-usage-of-strtol
+    // We need to properly check errno and overflows
+    if (temp == p || *temp != '\0' ||
+        ((n == LONG_MIN || n == LONG_MAX) && errno == ERANGE)) {
+      fprintf(stderr,
+              "Could not convert '%s' to long and leftover string is: '%s'\n",
+              p, temp);
+      free(input_cpy);
+      return -1;
+    }
+
+    n_numbers++;
+    (*numbers)[idx] = n;
+    p = strtok(NULL, ",");
+    idx++;
+  }
+
+  free(input_cpy);
+  *ret_n_numbers = n_numbers;
+
+  return 0;
+}
+
 int validate_container_id(const char* input) {
+  int is_container_id = 1;
+
   /*
    * Two different forms of container_id
    * container_e17_1410901177871_0001_01_000005
    * container_1410901177871_0001_01_000005
    */
+  if (!input) {
+    return 0;
+  }
+
   char* input_cpy = strdup(input);
+  if (!input_cpy) {
+    return 0;
+  }
+
   char* p = strtok(input_cpy, "_");
   int idx = 0;
   while (p != NULL) {
     if (0 == idx) {
       if (0 != strcmp("container", p)) {
-        return 0;
+        is_container_id = 0;
+        goto cleanup;
       }
     } else if (1 == idx) {
       // this could be e[n][n], or [n][n]...
       if (!all_numbers(p)) {
-        if (strlen(p) == 0) {
+        if (p[0] == 0) {
-          return 0;
+          is_container_id = 0;
+          goto cleanup;
         }
         if (p[0] != 'e') {
-          return 0;
+          is_container_id = 0;
+          goto cleanup;
         }
         if (!all_numbers(p + 1)) {
-          return 0;
+          is_container_id = 0;
+          goto cleanup;
         }
       }
     } else {
       // otherwise, should be all numbers
       if (!all_numbers(p)) {
-        return 0;
+        is_container_id = 0;
+        goto cleanup;
       }
     }
 
     p = strtok(NULL, "_");
     idx++;
   }
-  free(input_cpy);
+
+cleanup:
+  if (input_cpy) {
+    free(input_cpy);
+  }
 
   // We should have [5,6] elements split by '_'
   if (idx > 6 || idx < 5) {
-    return 0;
+    is_container_id = 0;
   }
-  return 1;
+  return is_container_id;
 }

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

@@ -29,4 +29,9 @@
  */
 int validate_container_id(const char* input);
 
+/*
+ * return 0 if succeeded
+ */
+int get_numbers_split_by_comma(const char* input, int** numbers, size_t* n_numbers);
+
 #endif

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

@@ -0,0 +1,36 @@
+/**
+ * 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.
+ */
+
+ #ifdef __APPLE__
+ #include <CoreFoundation/CFString.h>
+ #include <CoreFoundation/CFPreferences.h>
+
+ #define TMPDIR "/private/tmp"
+ #define RELTMPDIR "../.."
+ #else
+ #define RELTMPDIR ".."
+ #define TMPDIR "/tmp"
+ #endif
+
+ #define TEST_ROOT TMPDIR "/test-container-executor"
+
+ #define DONT_TOUCH_FILE "dont-touch-me"
+ #define NM_LOCAL_DIRS       TEST_ROOT "/local-1%" TEST_ROOT "/local-2%" \
+                TEST_ROOT "/local-3%" TEST_ROOT "/local-4%" TEST_ROOT "/local-5"
+ #define NM_LOG_DIRS         TEST_ROOT "/logs/userlogs"
+ #define ARRAY_SIZE 1000

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

@@ -19,6 +19,7 @@
 #include "container-executor.h"
 #include "utils/string-utils.h"
 #include "util.h"
+#include "test/test-container-executor-common.h"
 
 #include <inttypes.h>
 #include <errno.h>
@@ -31,25 +32,6 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 
-#ifdef __APPLE__
-#include <CoreFoundation/CFString.h>
-#include <CoreFoundation/CFPreferences.h>
-
-#define TMPDIR "/private/tmp"
-#define RELTMPDIR "../.."
-#else
-#define RELTMPDIR ".."
-#define TMPDIR "/tmp"
-#endif
-
-#define TEST_ROOT TMPDIR "/test-container-executor"
-
-#define DONT_TOUCH_FILE "dont-touch-me"
-#define NM_LOCAL_DIRS       TEST_ROOT "/local-1%" TEST_ROOT "/local-2%" \
-               TEST_ROOT "/local-3%" TEST_ROOT "/local-4%" TEST_ROOT "/local-5"
-#define NM_LOG_DIRS         TEST_ROOT "/logs/userlogs"
-#define ARRAY_SIZE 1000
-
 static char* username = NULL;
 static char* yarn_username = NULL;
 static char** local_dirs = NULL;

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

@@ -20,10 +20,13 @@
 #include <main/native/container-executor/impl/util.h>
 #include <cstdio>
 
-FILE* ERRORFILE = stderr;
+extern "C" {
-FILE* LOGFILE = stdout;
+#include "util.h"
+}
 
 int main(int argc, char **argv) {
-    testing::InitGoogleTest(&argc, argv);
+  ERRORFILE = stderr;
-    return RUN_ALL_TESTS();
+  LOGFILE = stdout;
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
 }

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

@@ -0,0 +1,67 @@
+/**
+ * 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 <errno.h>
+ #include <fcntl.h>
+ #include <inttypes.h>
+ #include <signal.h>
+ #include <stdio.h>
+ #include <stdlib.h>
+ #include <string.h>
+ #include <sys/stat.h>
+ #include <sys/wait.h>
+ #include <unistd.h>
+
+ #include <gtest/gtest.h>
+ #include <sstream>
+
+ extern "C" {
+ #include "utils/path-utils.h"
+ }
+
+ namespace ContainerExecutor {
+
+ class TestPathUtils : public ::testing::Test {
+ protected:
+   virtual void SetUp() {
+
+   }
+
+   virtual void TearDown() {
+
+   }
+ };
+
+ TEST_F(TestPathUtils, test_path_safety) {
+   const char* input = "./../abc/";
+   int flag = verify_path_safety(input);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_FALSE(flag) << "Should failed\n";
+
+   input = "abc/./cde";
+   flag = verify_path_safety(input);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_TRUE(flag) << "Should succeeded\n";
+
+   input = "/etc/abc/cde/./x/./y";
+   flag = verify_path_safety(input);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_TRUE(flag) << "Should succeeded\n";
+}
+
+} // namespace ContainerExecutor

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

@@ -0,0 +1,93 @@
+/**
+ * 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 <errno.h>
+ #include <fcntl.h>
+ #include <inttypes.h>
+ #include <signal.h>
+ #include <stdio.h>
+ #include <stdlib.h>
+ #include <string.h>
+ #include <sys/stat.h>
+ #include <sys/wait.h>
+ #include <unistd.h>
+
+ #include <gtest/gtest.h>
+ #include <sstream>
+
+ extern "C" {
+ #include "utils/string-utils.h"
+ }
+
+ namespace ContainerExecutor {
+
+ class TestStringUtils : public ::testing::Test {
+ protected:
+   virtual void SetUp() {
+
+   }
+
+   virtual void TearDown() {
+
+   }
+ };
+
+ TEST_F(TestStringUtils, test_get_numbers_split_by_comma) {
+   const char* input = ",1,2,3,-1,,1,,0,";
+   int* numbers;
+   size_t n_numbers;
+   int rc = get_numbers_split_by_comma(input, &numbers, &n_numbers);
+
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_EQ(0, rc) << "Should succeeded\n";
+   ASSERT_EQ(6, n_numbers);
+   ASSERT_EQ(1, numbers[0]);
+   ASSERT_EQ(-1, numbers[3]);
+   ASSERT_EQ(0, numbers[5]);
+
+   input = "3";
+   rc = get_numbers_split_by_comma(input, &numbers, &n_numbers);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_EQ(0, rc) << "Should succeeded\n";
+   ASSERT_EQ(1, n_numbers);
+   ASSERT_EQ(3, numbers[0]);
+
+   input = "";
+   rc = get_numbers_split_by_comma(input, &numbers, &n_numbers);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_EQ(0, rc) << "Should succeeded\n";
+   ASSERT_EQ(0, n_numbers);
+
+   input = ",,";
+   rc = get_numbers_split_by_comma(input, &numbers, &n_numbers);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_EQ(0, rc) << "Should succeeded\n";
+   ASSERT_EQ(0, n_numbers);
+
+   input = "1,2,aa,bb";
+   rc = get_numbers_split_by_comma(input, &numbers, &n_numbers);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_TRUE(0 != rc) << "Should failed\n";
+
+   input = "1,2,3,-12312312312312312312321311231231231";
+   rc = get_numbers_split_by_comma(input, &numbers, &n_numbers);
+   std::cout << "Testing input=" << input << "\n";
+   ASSERT_TRUE(0 != rc) << "Should failed\n";
+}
+
+} // namespace ContainerExecutor