[mesos] 01/02: Allowed creating non-existing host paths for Mesos Containerizer.

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gilbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 281acd44a904d98c10b0bb9f57abb2b110fc6dce
Author: Jason Lai <ja...@jasonlai.net>
AuthorDate: Thu Feb 21 18:29:50 2019 -0800

    Allowed creating non-existing host paths for Mesos Containerizer.
    
    Added a new agent flag `--host_path_volume_force_creation` for
    the `volume/host_path` isolator. The flag takes a colon-separated
    whitelist of paths, under which non-existing host paths are allowed to
    be created.
    
    If the flag is not specified, the isolator behaves in the original way
    of prohibiting all non-existing host paths from being created.
    
    Review: https://reviews.apache.org/r/69286/
---
 src/CMakeLists.txt                                 |  1 +
 src/Makefile.am                                    |  2 +
 .../mesos/isolators/volume/host_path.cpp           | 48 +++++++++++++++++++--
 .../mesos/isolators/volume/host_path.hpp           |  6 +++
 .../isolators/volume/{host_path.hpp => utils.cpp}  | 49 ++++++++++++----------
 .../isolators/volume/{host_path.hpp => utils.hpp}  | 33 +++++++--------
 src/slave/flags.cpp                                |  6 +++
 src/slave/flags.hpp                                |  1 +
 8 files changed, 103 insertions(+), 43 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 9a200d8..687dc85 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -307,6 +307,7 @@ set(LINUX_SRC
   slave/containerizer/mesos/isolators/volume/host_path.cpp
   slave/containerizer/mesos/isolators/volume/image.cpp
   slave/containerizer/mesos/isolators/volume/secret.cpp
+  slave/containerizer/mesos/isolators/volume/utils.cpp
   slave/containerizer/mesos/provisioner/backends/aufs.cpp
   slave/containerizer/mesos/provisioner/backends/bind.cpp
   slave/containerizer/mesos/provisioner/backends/overlay.cpp)
diff --git a/src/Makefile.am b/src/Makefile.am
index e2de7ad..283d5ed 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1409,6 +1409,8 @@ MESOS_LINUX_FILES =									\
   slave/containerizer/mesos/isolators/volume/image.hpp					\
   slave/containerizer/mesos/isolators/volume/secret.cpp					\
   slave/containerizer/mesos/isolators/volume/secret.hpp					\
+  slave/containerizer/mesos/isolators/volume/utils.cpp					\
+  slave/containerizer/mesos/isolators/volume/utils.hpp					\
   slave/containerizer/mesos/provisioner/backends/aufs.cpp				\
   slave/containerizer/mesos/provisioner/backends/aufs.hpp				\
   slave/containerizer/mesos/provisioner/backends/bind.cpp				\
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.cpp b/src/slave/containerizer/mesos/isolators/volume/host_path.cpp
index 64abfc1..a20f7ac 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.cpp
+++ b/src/slave/containerizer/mesos/isolators/volume/host_path.cpp
@@ -38,6 +38,7 @@
 #include "linux/fs.hpp"
 
 #include "slave/containerizer/mesos/isolators/volume/host_path.hpp"
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
 
 using std::string;
 
@@ -53,6 +54,8 @@ using mesos::slave::ContainerLaunchInfo;
 using mesos::slave::ContainerMountInfo;
 using mesos::slave::Isolator;
 
+using mesos::internal::slave::volume::PathValidator;
+
 namespace mesos {
 namespace internal {
 namespace slave {
@@ -69,7 +72,11 @@ Try<Isolator*> VolumeHostPathIsolatorProcess::create(
   }
 
   Owned<MesosIsolatorProcess> process(
-      new VolumeHostPathIsolatorProcess(flags));
+      flags.host_path_volume_force_creation.isSome()
+      ? new VolumeHostPathIsolatorProcess(
+            flags,
+            PathValidator::parse(flags.host_path_volume_force_creation.get()))
+      : new VolumeHostPathIsolatorProcess(flags));
 
   return new MesosIsolator(process);
 }
@@ -78,7 +85,16 @@ Try<Isolator*> VolumeHostPathIsolatorProcess::create(
 VolumeHostPathIsolatorProcess::VolumeHostPathIsolatorProcess(
     const Flags& _flags)
   : ProcessBase(process::ID::generate("volume-host-path-isolator")),
-    flags(_flags) {}
+    flags(_flags),
+    pathValidator() {}
+
+
+VolumeHostPathIsolatorProcess::VolumeHostPathIsolatorProcess(
+    const Flags& _flags,
+    const PathValidator& _pathValidator)
+  : ProcessBase(process::ID::generate("volume-host-path-isolator")),
+    flags(_flags),
+    pathValidator(_pathValidator) {}
 
 
 VolumeHostPathIsolatorProcess::~VolumeHostPathIsolatorProcess() {}
@@ -164,8 +180,32 @@ Future<Option<ContainerLaunchInfo>> VolumeHostPathIsolatorProcess::prepare(
     }
 
     if (!os::exists(hostPath.get())) {
-      return Failure(
-          "Path '" + hostPath.get() + "' in HOST_PATH volume does not exist");
+      if (pathValidator.isNone()) {
+        return Failure(
+            "Path '" + hostPath.get() + "' in HOST_PATH volume does not exist");
+      }
+
+      Try<string> normalizedPath = path::normalize(hostPath.get());
+      if (normalizedPath.isError()) {
+        return Failure(
+            "Failed to normalized the host path '" + hostPath.get() + "': " +
+            normalizedPath.error());
+      }
+
+      Try<Nothing> validate = pathValidator->validate(normalizedPath.get());
+      if (validate.isError()) {
+        return Failure(
+            "Path '" + hostPath.get() + "' in HOST_PATH volume does not exist "
+            "and is not whitelisted for creation: " + validate.error());
+      }
+
+      // Always assume the non-existing host path as a directory.
+      Try<Nothing> mkdir = os::mkdir(normalizedPath.get());
+      if (mkdir.isError()) {
+        return Failure(
+            "Failed to create the host path at '" + hostPath.get() + "': " +
+            mkdir.error());
+      }
     }
 
     // Determine the mount point for the host volume.
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp b/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
index 4b509e9..08f9946 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
+++ b/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
@@ -21,6 +21,8 @@
 
 #include "slave/containerizer/mesos/isolator.hpp"
 
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
+
 namespace mesos {
 namespace internal {
 namespace slave {
@@ -41,8 +43,12 @@ public:
 
 private:
   VolumeHostPathIsolatorProcess(const Flags& flags);
+  VolumeHostPathIsolatorProcess(
+      const Flags& flags,
+      const volume::PathValidator& pathValidator);
 
   const Flags flags;
+  const Option<volume::PathValidator> pathValidator;
 };
 
 } // namespace slave {
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp b/src/slave/containerizer/mesos/isolators/volume/utils.cpp
similarity index 52%
copy from src/slave/containerizer/mesos/isolators/volume/host_path.hpp
copy to src/slave/containerizer/mesos/isolators/volume/utils.cpp
index 4b509e9..3b353f4 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
+++ b/src/slave/containerizer/mesos/isolators/volume/utils.cpp
@@ -14,39 +14,44 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef __VOLUME_HOST_PATH_ISOLATOR_HPP__
-#define __VOLUME_HOST_PATH_ISOLATOR_HPP__
+#include <stout/path.hpp>
+#include <stout/stringify.hpp>
+#include <stout/strings.hpp>
 
-#include "slave/flags.hpp"
+#include "slave/containerizer/mesos/isolators/volume/utils.hpp"
 
-#include "slave/containerizer/mesos/isolator.hpp"
+using std::string;
+using std::vector;
 
 namespace mesos {
 namespace internal {
 namespace slave {
+namespace volume {
 
-class VolumeHostPathIsolatorProcess : public MesosIsolatorProcess
-{
-public:
-  static Try<mesos::slave::Isolator*> create(const Flags& flags);
-
-  ~VolumeHostPathIsolatorProcess() override;
-
-  bool supportsNesting() override;
-  bool supportsStandalone() override;
+PathValidator::PathValidator(const vector<string>& _whitelist)
+  : whitelist(_whitelist) {}
 
-  process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
-      const ContainerID& containerId,
-      const mesos::slave::ContainerConfig& containerConfig) override;
 
-private:
-  VolumeHostPathIsolatorProcess(const Flags& flags);
+PathValidator PathValidator::parse(const string& whitelist)
+{
+  return PathValidator(strings::split(whitelist, HOST_PATH_WHITELIST_DELIM));
+}
 
-  const Flags flags;
-};
 
+Try<Nothing> PathValidator::validate(const string& path) const
+{
+  foreach (const string& allowedPath, whitelist) {
+    const string allowedDirectory = path::join(
+        allowedPath, stringify(os::PATH_SEPARATOR));
+    if (path == allowedPath || strings::startsWith(path, allowedDirectory)) {
+      return Nothing();
+    }
+  }
+
+  return Error("Path '" + path + "' is not whitelisted");
+}
+
+} // namespace volume {
 } // namespace slave {
 } // namespace internal {
 } // namespace mesos {
-
-#endif // __VOLUME_HOST_PATH_ISOLATOR_HPP__
diff --git a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp b/src/slave/containerizer/mesos/isolators/volume/utils.hpp
similarity index 57%
copy from src/slave/containerizer/mesos/isolators/volume/host_path.hpp
copy to src/slave/containerizer/mesos/isolators/volume/utils.hpp
index 4b509e9..9628e6d 100644
--- a/src/slave/containerizer/mesos/isolators/volume/host_path.hpp
+++ b/src/slave/containerizer/mesos/isolators/volume/utils.hpp
@@ -14,39 +14,38 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef __VOLUME_HOST_PATH_ISOLATOR_HPP__
-#define __VOLUME_HOST_PATH_ISOLATOR_HPP__
+#ifndef __VOLUME_UTILS_HPP__
+#define __VOLUME_UTILS_HPP__
 
-#include "slave/flags.hpp"
+#include <string>
+#include <vector>
 
-#include "slave/containerizer/mesos/isolator.hpp"
+#include <stout/nothing.hpp>
+#include <stout/try.hpp>
 
 namespace mesos {
 namespace internal {
 namespace slave {
+namespace volume {
 
-class VolumeHostPathIsolatorProcess : public MesosIsolatorProcess
+constexpr char HOST_PATH_WHITELIST_DELIM[] = ":";
+
+class PathValidator
 {
 public:
-  static Try<mesos::slave::Isolator*> create(const Flags& flags);
-
-  ~VolumeHostPathIsolatorProcess() override;
-
-  bool supportsNesting() override;
-  bool supportsStandalone() override;
+  static PathValidator parse(const std::string& whitelist);
 
-  process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
-      const ContainerID& containerId,
-      const mesos::slave::ContainerConfig& containerConfig) override;
+  Try<Nothing> validate(const std::string& path) const;
 
 private:
-  VolumeHostPathIsolatorProcess(const Flags& flags);
+  PathValidator(const std::vector<std::string>& whitelist);
 
-  const Flags flags;
+  const std::vector<std::string> whitelist;
 };
 
+} // namespace volume {
 } // namespace slave {
 } // namespace internal {
 } // namespace mesos {
 
-#endif // __VOLUME_HOST_PATH_ISOLATOR_HPP__
+#endif // __VOLUME_UTILS_HPP__
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index a5b7d90..2a1f3f9 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -650,6 +650,12 @@ mesos::internal::slave::Flags::Flags()
       "no cgroup limits are set, they are inherited from the root mesos\n"
       "cgroup.");
 
+  add(&Flags::host_path_volume_force_creation,
+      "host_path_volume_force_creation",
+      "A colon-separated list of directories where descendant directories\n"
+      "are allowed to be created by the `volume/host_path` isolator,\n"
+      "if the directories do not exist.");
+
   add(&Flags::nvidia_gpu_devices,
       "nvidia_gpu_devices",
       "A comma-separated list of Nvidia GPU devices. When `gpus` is\n"
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 7346ba5..c64edd0 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -112,6 +112,7 @@ public:
   Option<std::string> cgroups_net_cls_secondary_handles;
   Option<DeviceWhitelist> allowed_devices;
   Option<std::string> agent_subsystems;
+  Option<std::string> host_path_volume_force_creation;
   Option<std::vector<unsigned int>> nvidia_gpu_devices;
   Option<std::string> perf_events;
   Duration perf_interval;