You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ka...@apache.org on 2017/05/22 21:28:06 UTC

[4/5] mesos git commit: Added environment secret isolator.

Added environment secret isolator.

Review: https://reviews.apache.org/r/59000


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e9e9e739
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e9e9e739
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e9e9e739

Branch: refs/heads/master
Commit: e9e9e739d4d120c7c7dc9f366b0b1f7d2f1d3154
Parents: 5a630d8
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Mon May 1 18:51:22 2017 -0400
Committer: Kapil Arya <ka...@mesosphere.io>
Committed: Mon May 22 16:35:15 2017 -0400

----------------------------------------------------------------------
 src/CMakeLists.txt                              |   1 +
 src/Makefile.am                                 |   3 +
 src/launcher/executor.cpp                       |   8 ++
 src/slave/containerizer/mesos/containerizer.cpp |  21 ++-
 .../mesos/isolators/environment_secret.cpp      | 138 +++++++++++++++++++
 .../mesos/isolators/environment_secret.hpp      |  60 ++++++++
 src/tests/CMakeLists.txt                        |   1 +
 .../environment_secret_isolator_tests.cpp       | 130 +++++++++++++++++
 8 files changed, 360 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index d71f1c6..fd154e2 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -197,6 +197,7 @@ set(AGENT_SRC
 if (NOT WIN32)
   set(AGENT_SRC
     ${AGENT_SRC}
+    slave/containerizer/mesos/isolators/environment_secret.cpp
     slave/containerizer/mesos/isolators/docker/volume/driver.cpp
     slave/containerizer/mesos/isolators/docker/volume/paths.cpp
     slave/containerizer/mesos/isolators/filesystem/posix.cpp

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index e1fdda3..78158a3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -960,6 +960,7 @@ libmesos_no_3rdparty_la_SOURCES +=					\
   slave/containerizer/mesos/paths.cpp					\
   slave/containerizer/mesos/utils.cpp					\
   slave/containerizer/mesos/io/switchboard.cpp				\
+  slave/containerizer/mesos/isolators/environment_secret.cpp		\
   slave/containerizer/mesos/isolators/docker/volume/driver.cpp		\
   slave/containerizer/mesos/isolators/docker/volume/paths.cpp		\
   slave/containerizer/mesos/isolators/filesystem/posix.cpp		\
@@ -1090,6 +1091,7 @@ libmesos_no_3rdparty_la_SOURCES +=					\
   slave/containerizer/mesos/paths.hpp					\
   slave/containerizer/mesos/utils.hpp					\
   slave/containerizer/mesos/io/switchboard.hpp				\
+  slave/containerizer/mesos/isolators/environment_secret.hpp		\
   slave/containerizer/mesos/isolators/posix.hpp				\
   slave/containerizer/mesos/isolators/filesystem/posix.hpp		\
   slave/containerizer/mesos/isolators/filesystem/windows.hpp		\
@@ -2345,6 +2347,7 @@ mesos_tests_SOURCES =						\
   tests/containerizer/docker_containerizer_tests.cpp		\
   tests/containerizer/docker_spec_tests.cpp			\
   tests/containerizer/docker_tests.cpp				\
+  tests/containerizer/environment_secret_isolator_tests.cpp	\
   tests/containerizer/io_switchboard_tests.cpp			\
   tests/containerizer/isolator_tests.cpp			\
   tests/containerizer/launcher.cpp				\

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/launcher/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 9971a67..9ac3c3d 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -587,6 +587,10 @@ protected:
     if (taskEnvironment.isSome()) {
       foreach (const Environment::Variable& variable,
                taskEnvironment->variables()) {
+        // Skip overwriting if the variable is unresolved secret.
+        if (variable.type() == Environment::Variable::SECRET) {
+          continue;
+        }
         const string& name = variable.name();
         if (environment.contains(name) &&
             environment[name].value() != variable.value()) {
@@ -599,6 +603,10 @@ protected:
     if (command.has_environment()) {
       foreach (const Environment::Variable& variable,
                command.environment().variables()) {
+        // Skip overwriting if the variable is unresolved secret.
+        if (variable.type() == Environment::Variable::SECRET) {
+          continue;
+        }
         const string& name = variable.name();
         if (environment.contains(name) &&
             environment[name].value() != variable.value()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 2c9cf38..b513e68 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -65,6 +65,7 @@
 
 #include "slave/containerizer/mesos/io/switchboard.hpp"
 
+#include "slave/containerizer/mesos/isolators/environment_secret.hpp"
 #include "slave/containerizer/mesos/isolators/filesystem/posix.hpp"
 #include "slave/containerizer/mesos/isolators/posix.hpp"
 #include "slave/containerizer/mesos/isolators/posix/disk.hpp"
@@ -221,6 +222,11 @@ Try<MesosContainerizer*> MesosContainerizer::create(
   }
 #endif // __linux__
 
+  // Always add environment secret isolator.
+  if (!strings::contains(flags_.isolation, "environment_secret")) {
+    flags_.isolation += ",environment_secret";
+  }
+
   LOG(INFO) << "Using isolation: " << flags_.isolation;
 
   // Create the launcher for the MesosContainerizer.
@@ -353,6 +359,11 @@ Try<MesosContainerizer*> MesosContainerizer::create(
 #if !defined(__WINDOWS__) && defined(ENABLE_PORT_MAPPING_ISOLATOR)
     {"network/port_mapping", &PortMappingIsolatorProcess::create},
 #endif
+
+    {"environment_secret",
+      [secretResolver] (const Flags& flags) -> Try<Isolator*> {
+        return EnvironmentSecretIsolatorProcess::create(flags, secretResolver);
+      }},
   };
 
   vector<string> tokens = strings::tokenize(flags_.isolation, ",");
@@ -1483,9 +1494,15 @@ Future<bool> MesosContainerizerProcess::_launch(
   containerEnvironment.MergeFrom(launchInfo.environment());
 
   // Include user specified environment.
+  // Skip over any secrets as they should have been resolved by the
+  // environment_secret isolator.
   if (container->config.command_info().has_environment()) {
-    containerEnvironment.MergeFrom(
-        container->config.command_info().environment());
+    foreach (const Environment::Variable& variable,
+             container->config.command_info().environment().variables()) {
+      if (variable.type() != Environment::Variable::SECRET) {
+        containerEnvironment.add_variables()->CopyFrom(variable);
+      }
+    }
   }
 
   // Set the aggregated environment of the launch command.

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/slave/containerizer/mesos/isolators/environment_secret.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/environment_secret.cpp b/src/slave/containerizer/mesos/isolators/environment_secret.cpp
new file mode 100644
index 0000000..5b0b2fc
--- /dev/null
+++ b/src/slave/containerizer/mesos/isolators/environment_secret.cpp
@@ -0,0 +1,138 @@
+// 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 "slave/containerizer/mesos/isolators/environment_secret.hpp"
+
+#include <list>
+
+#include <mesos/secret/resolver.hpp>
+
+#include <process/collect.hpp>
+#include <process/future.hpp>
+#include <process/id.hpp>
+#include <process/owned.hpp>
+
+#include <stout/foreach.hpp>
+
+#include "common/validation.hpp"
+
+using std::list;
+
+using process::collect;
+using process::Failure;
+using process::Future;
+using process::Owned;
+
+using mesos::slave::ContainerConfig;
+using mesos::slave::ContainerLaunchInfo;
+using mesos::slave::ContainerState;
+using mesos::slave::Isolator;
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+Try<Isolator*> EnvironmentSecretIsolatorProcess::create(
+    const Flags& flags,
+    SecretResolver* secretResolver)
+{
+  Owned<MesosIsolatorProcess> process(new EnvironmentSecretIsolatorProcess(
+      flags,
+      secretResolver));;
+
+  return new MesosIsolator(process);
+}
+
+
+EnvironmentSecretIsolatorProcess::EnvironmentSecretIsolatorProcess(
+    const Flags& _flags,
+    SecretResolver* _secretResolver)
+  : ProcessBase(process::ID::generate("environment-secret-isolator")),
+    flags(_flags),
+    secretResolver(_secretResolver) {}
+
+
+EnvironmentSecretIsolatorProcess::~EnvironmentSecretIsolatorProcess() {}
+
+
+bool EnvironmentSecretIsolatorProcess::supportsNesting()
+{
+  return true;
+}
+
+
+Future<Option<ContainerLaunchInfo>> EnvironmentSecretIsolatorProcess::prepare(
+    const ContainerID& containerId,
+    const ContainerConfig& containerConfig)
+{
+  Option<Error> error = common::validation::validateEnvironment(
+      containerConfig.command_info().environment());
+  if (error.isSome()) {
+    return Failure( "Invalid environment specified: " + error->message);
+  }
+
+  Environment environment;
+
+  list<Future<Environment::Variable>> futures;
+  foreach (const Environment::Variable& variable,
+           containerConfig.command_info().environment().variables()) {
+    if (variable.type() != Environment::Variable::SECRET) {
+      continue;
+    }
+
+    const Secret& secret = variable.secret();
+
+    error = common::validation::validateSecret(secret);
+    if (error.isSome()) {
+      return Failure(
+          "Invalid secret specified in environment '" + variable.name() +
+          "': " + error->message);
+    }
+
+    if (secretResolver == nullptr) {
+      return Failure(
+          "Error: Environment variable '" + variable.name() +
+          "' contains secret but no secret resolver provided");
+    }
+
+    Future<Environment::Variable> future = secretResolver->resolve(secret)
+      .then([variable](const Secret::Value& secretValue)
+            -> Future<Environment::Variable> {
+          Environment::Variable result;
+          result.set_name(variable.name());
+          result.set_value(secretValue.data());
+          return result;
+        });
+
+    futures.push_back(future);
+  }
+
+  return collect(futures)
+    .then([](const list<Environment::Variable>& variables)
+        -> Future<Option<ContainerLaunchInfo>> {
+      ContainerLaunchInfo launchInfo;
+      Environment* environment = launchInfo.mutable_environment();
+      foreach (const Environment::Variable& variable, variables) {
+        environment->add_variables()->CopyFrom(variable);
+      }
+      launchInfo.mutable_task_environment()->CopyFrom(*environment);
+      return launchInfo;
+    });
+}
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/slave/containerizer/mesos/isolators/environment_secret.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/environment_secret.hpp b/src/slave/containerizer/mesos/isolators/environment_secret.hpp
new file mode 100644
index 0000000..b98e8fe
--- /dev/null
+++ b/src/slave/containerizer/mesos/isolators/environment_secret.hpp
@@ -0,0 +1,60 @@
+// 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 __ENVIRONMENT_SECRET_ISOLATOR_HPP__
+#define __ENVIRONMENT_SECRET_ISOLATOR_HPP__
+
+#include <list>
+
+#include <mesos/secret/resolver.hpp>
+
+#include "slave/flags.hpp"
+
+#include "slave/containerizer/mesos/isolator.hpp"
+
+namespace mesos {
+namespace internal {
+namespace slave {
+
+class EnvironmentSecretIsolatorProcess : public MesosIsolatorProcess
+{
+public:
+  static Try<mesos::slave::Isolator*> create(
+      const Flags& flags,
+      SecretResolver* secretResolver);
+
+  virtual ~EnvironmentSecretIsolatorProcess();
+
+  virtual bool supportsNesting();
+
+  virtual process::Future<Option<mesos::slave::ContainerLaunchInfo>> prepare(
+      const ContainerID& containerId,
+      const mesos::slave::ContainerConfig& containerConfig);
+
+private:
+  EnvironmentSecretIsolatorProcess(
+      const Flags& flags,
+      SecretResolver* secretResolver);
+
+  const Flags flags;
+  SecretResolver* secretResolver;
+};
+
+} // namespace slave {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __ENVIRONMENT_SECRET_ISOLATOR_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 9f2af9c..8cecb72 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -193,6 +193,7 @@ if (NOT WIN32)
     containerizer/docker_containerizer_tests.cpp
     containerizer/docker_spec_tests.cpp
     containerizer/docker_tests.cpp
+    containerizer/environment_secret_isolator_tests.cpp
     containerizer/io_switchboard_tests.cpp
     containerizer/isolator_tests.cpp
     containerizer/memory_isolator_tests.cpp

http://git-wip-us.apache.org/repos/asf/mesos/blob/e9e9e739/src/tests/containerizer/environment_secret_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/environment_secret_isolator_tests.cpp b/src/tests/containerizer/environment_secret_isolator_tests.cpp
new file mode 100644
index 0000000..f8c7719
--- /dev/null
+++ b/src/tests/containerizer/environment_secret_isolator_tests.cpp
@@ -0,0 +1,130 @@
+// 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 <string>
+
+#include <mesos/secret/resolver.hpp>
+
+#include <process/future.hpp>
+#include <process/gtest.hpp>
+
+#include <stout/gtest.hpp>
+
+#include "tests/mesos.hpp"
+
+using process::Future;
+using process::Owned;
+
+using mesos::internal::slave::Fetcher;
+using mesos::internal::slave::MesosContainerizer;
+
+using mesos::master::detector::MasterDetector;
+
+using std::string;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+const char SECRET_VALUE[] = "password";
+const char SECRET_ENV_NAME[] = "My_SeCrEt";
+
+class EnvironmentSecretIsolatorTest : public MesosTest {};
+
+
+// This test verifies that the environment secrets are resolved when launching a
+// task.
+TEST_F(EnvironmentSecretIsolatorTest, ResolveSecret)
+{
+  Try<Owned<cluster::Master>> master = StartMaster();
+  ASSERT_SOME(master);
+
+  mesos::internal::slave::Flags flags = CreateSlaveFlags();
+
+  Fetcher fetcher;
+  Try<SecretResolver*> secretResolver = SecretResolver::create();
+  EXPECT_SOME(secretResolver);
+
+  Try<MesosContainerizer*> containerizer =
+    MesosContainerizer::create(flags, false, &fetcher, secretResolver.get());
+  EXPECT_SOME(containerizer);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+  Try<Owned<cluster::Slave>> slave =
+    StartSlave(detector.get(), containerizer.get());
+  ASSERT_SOME(slave);
+
+  MockScheduler sched;
+  MesosSchedulerDriver driver(
+      &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+  EXPECT_CALL(sched, registered(&driver, _, _));
+
+  Future<std::vector<Offer>> offers;
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(FutureArg<1>(&offers))
+    .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+  driver.start();
+
+  AWAIT_READY(offers);
+  EXPECT_NE(0u, offers.get().size());
+
+  const string commandString = strings::format(
+      "env; test \"$%s\" = \"%s\"",
+      string(SECRET_ENV_NAME),
+      string(SECRET_VALUE));
+
+  CommandInfo command;
+  command.set_value(commandString);
+
+  // Request a secret.
+  // TODO(kapil): Update createEnvironment() to support secrets.
+  mesos::Environment::Variable *env =
+    command.mutable_environment()->add_variables();
+  env->set_name(SECRET_ENV_NAME);
+  env->set_type(mesos::Environment::Variable::SECRET);
+
+  mesos::Secret* secret = env->mutable_secret();
+  secret->set_type(Secret::VALUE);
+  secret->mutable_value()->set_data(SECRET_VALUE);
+
+  TaskInfo task = createTask(
+      offers.get()[0].slave_id(),
+      Resources::parse("cpus:0.1;mem:32").get(),
+      command);
+
+  // NOTE: Successful tasks will output two status updates.
+  Future<TaskStatus> statusRunning;
+  Future<TaskStatus> statusFinished;
+  EXPECT_CALL(sched, statusUpdate(&driver, _))
+    .WillOnce(FutureArg<1>(&statusRunning))
+    .WillOnce(FutureArg<1>(&statusFinished));
+
+  driver.launchTasks(offers.get()[0].id(), {task});
+
+  AWAIT_READY(statusRunning);
+  EXPECT_EQ(TASK_RUNNING, statusRunning.get().state());
+  AWAIT_READY(statusFinished);
+  EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
+
+  driver.stop();
+  driver.join();
+}
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {