You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by me...@apache.org on 2015/01/14 09:15:31 UTC

[1/2] mesos git commit: Revised authenticator interface to allow for two fold implementations.

Repository: mesos
Updated Branches:
  refs/heads/master f65254957 -> 585e4acb6


Revised authenticator interface to allow for two fold implementations.

The initial design and implementation of the authenticator module
interface caused issues and was not optimal for heavy lifting setup of
external dependencies. By introducing a two fold design, this has been
decoupled from the authentication message processing. The new design
also gets us back on track to the goal of makeing SASL a soft
dependency of mesos.

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


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

Branch: refs/heads/master
Commit: 03ce326887edd1f8718b3b60e9c81b3bf063ef64
Parents: f652549
Author: Till Toenshoff <to...@me.com>
Authored: Tue Jan 13 23:03:28 2015 -0800
Committer: Adam B <ad...@mesosphere.io>
Committed: Tue Jan 13 23:03:28 2015 -0800

----------------------------------------------------------------------
 src/authentication/authenticator.hpp          |  30 +++-
 src/authentication/cram_md5/authenticator.hpp | 176 +++++++++++----------
 src/master/master.cpp                         | 131 ++++++++++-----
 src/master/master.hpp                         |   6 +-
 src/tests/cram_md5_authentication_tests.cpp   |  69 +++++---
 5 files changed, 258 insertions(+), 154 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/03ce3268/src/authentication/authenticator.hpp
----------------------------------------------------------------------
diff --git a/src/authentication/authenticator.hpp b/src/authentication/authenticator.hpp
index 460494a..2a224dc 100644
--- a/src/authentication/authenticator.hpp
+++ b/src/authentication/authenticator.hpp
@@ -30,15 +30,37 @@
 namespace mesos {
 namespace internal {
 
-// Note that this interface definition is not hardened yet and will
-// slightly change within the next release. See MESOS-2050.
+// Forward declaration.
+class AuthenticatorSession;
+
+// Authenticator poses as a stateful factory. It expects to be
+// initialized before any sessions are created. It should do
+// heavy lifting tasks like setting up a database connection.
 class Authenticator
 {
 public:
   Authenticator() {}
+
   virtual ~Authenticator() {}
 
-  virtual void initialize(const process::UPID& clientPid) = 0;
+  // Does any heavy lifting that is needed only once for the entire
+  // lifetime of this factory.
+  virtual Try<Nothing> initialize(const Option<Credentials>& credentials) = 0;
+
+  // Creates a new authenticator session for every authentication
+  // request.
+  virtual Try<AuthenticatorSession*> session(const process::UPID& pid) = 0;
+};
+
+
+// AuthenticatorSession is created by the Authenticator for every
+// authentication request received.
+class AuthenticatorSession
+{
+public:
+  explicit AuthenticatorSession(const process::UPID& pid) {}
+
+  virtual ~AuthenticatorSession() {}
 
   // Returns the principal of the Authenticatee if successfully
   // authenticated otherwise None or an error. Note that we
@@ -48,7 +70,7 @@ public:
   // will cause the future to fail if it hasn't already completed
   // since we have already started the authentication procedure and
   // can't reliably cancel.
-  virtual process::Future<Option<std::string>> authenticate(void) = 0;
+  virtual process::Future<Option<std::string>> authenticate() = 0;
 };
 
 } // namespace internal {

http://git-wip-us.apache.org/repos/asf/mesos/blob/03ce3268/src/authentication/cram_md5/authenticator.hpp
----------------------------------------------------------------------
diff --git a/src/authentication/cram_md5/authenticator.hpp b/src/authentication/cram_md5/authenticator.hpp
index d739a02..1819ef4 100644
--- a/src/authentication/cram_md5/authenticator.hpp
+++ b/src/authentication/cram_md5/authenticator.hpp
@@ -19,6 +19,8 @@
 #ifndef __AUTHENTICATION_CRAM_MD5_AUTHENTICATOR_HPP__
 #define __AUTHENTICATION_CRAM_MD5_AUTHENTICATOR_HPP__
 
+#include <stddef.h>   // For size_t needed by sasl.h
+
 #include <sasl/sasl.h>
 #include <sasl/saslplug.h>
 
@@ -47,7 +49,7 @@ namespace internal {
 namespace cram_md5 {
 
 // Forward declaration.
-class CRAMMD5AuthenticatorProcess;
+class CRAMMD5AuthenticatorSessionProcess;
 
 
 class CRAMMD5Authenticator : public Authenticator
@@ -58,36 +60,42 @@ public:
 
   CRAMMD5Authenticator();
 
-  virtual ~CRAMMD5Authenticator();
+  virtual ~CRAMMD5Authenticator() {}
+
+  virtual Try<Nothing> initialize(const Option<Credentials>& credentials);
+
+  virtual Try<AuthenticatorSession*> session(const process::UPID& pid);
+
+private:
+  bool initialized;
+};
+
+
+class CRAMMD5AuthenticatorSession : public AuthenticatorSession
+{
+public:
+  explicit CRAMMD5AuthenticatorSession(const process::UPID& pid);
 
-  virtual void initialize(const process::UPID& clientPid);
+  virtual ~CRAMMD5AuthenticatorSession();
 
-  // Returns the principal of the Authenticatee if successfully
-  // authenticated otherwise None or an error. Note that we
-  // distinguish authentication failure (None) from a failed future
-  // in the event the future failed due to a transient error and
-  // authentication can (should) be retried. Discarding the future
-  // will cause the future to fail if it hasn't already completed
-  // since we have already started the authentication procedure and
-  // can't reliably cancel.
   virtual process::Future<Option<std::string>> authenticate();
 
 private:
-  CRAMMD5AuthenticatorProcess* process;
+  CRAMMD5AuthenticatorSessionProcess* process;
 };
 
 
-class CRAMMD5AuthenticatorProcess
-  : public ProtobufProcess<CRAMMD5AuthenticatorProcess>
+class CRAMMD5AuthenticatorSessionProcess
+  : public ProtobufProcess<CRAMMD5AuthenticatorSessionProcess>
 {
 public:
-  explicit CRAMMD5AuthenticatorProcess(const process::UPID& _pid)
-    : ProcessBase(process::ID::generate("crammd5_authenticator")),
+  explicit CRAMMD5AuthenticatorSessionProcess(const process::UPID& _pid)
+    : ProcessBase(process::ID::generate("crammd5_authenticator_session")),
       status(READY),
       pid(_pid),
       connection(NULL) {}
 
-  virtual ~CRAMMD5AuthenticatorProcess()
+  virtual ~CRAMMD5AuthenticatorSessionProcess()
   {
     if (connection != NULL) {
       sasl_dispose(&connection);
@@ -101,55 +109,6 @@ public:
 
   process::Future<Option<std::string>> authenticate()
   {
-    static process::Once* initialize = new process::Once();
-    static bool initialized = false;
-
-    if (!initialize->once()) {
-      LOG(INFO) << "Initializing server SASL";
-
-      int result = sasl_server_init(NULL, "mesos");
-
-      if (result != SASL_OK) {
-        std::string error = "Failed to initialize SASL: ";
-        error += sasl_errstring(result, NULL, NULL);
-        LOG(ERROR) << error;
-        AuthenticationErrorMessage message;
-        message.set_error(error);
-        send(pid, message);
-        status = ERROR;
-        promise.fail(error);
-        initialize->done();
-        return promise.future();
-      }
-
-      result = sasl_auxprop_add_plugin(
-          InMemoryAuxiliaryPropertyPlugin::name(),
-          &InMemoryAuxiliaryPropertyPlugin::initialize);
-
-      if (result != SASL_OK) {
-        std::string error =
-          "Failed to add \"in-memory\" auxiliary property plugin: ";
-        error += sasl_errstring(result, NULL, NULL);
-        LOG(ERROR) << error;
-        AuthenticationErrorMessage message;
-        message.set_error(error);
-        send(pid, message);
-        status = ERROR;
-        promise.fail(error);
-        initialize->done();
-        return promise.future();
-      }
-
-      initialized = true;
-
-      initialize->done();
-    }
-
-    if (!initialized) {
-      promise.fail("Failed to initialize SASL");
-      return promise.future();
-    }
-
     if (status != READY) {
       return promise.future();
     }
@@ -245,12 +204,12 @@ protected:
 
     // Anticipate start and steps messages from the client.
     install<AuthenticationStartMessage>(
-        &CRAMMD5AuthenticatorProcess::start,
+        &CRAMMD5AuthenticatorSessionProcess::start,
         &AuthenticationStartMessage::mechanism,
         &AuthenticationStartMessage::data);
 
     install<AuthenticationStepMessage>(
-        &CRAMMD5AuthenticatorProcess::step,
+        &CRAMMD5AuthenticatorSessionProcess::step,
         &AuthenticationStepMessage::data);
   }
 
@@ -477,37 +436,84 @@ Try<Authenticator*> CRAMMD5Authenticator::create()
 }
 
 
-CRAMMD5Authenticator::CRAMMD5Authenticator() : process(NULL) {}
+CRAMMD5Authenticator::CRAMMD5Authenticator() : initialized(false) {}
 
 
-CRAMMD5Authenticator::~CRAMMD5Authenticator()
+Try<Nothing> CRAMMD5Authenticator::initialize(
+    const Option<Credentials>& credentials)
 {
-  if (process != NULL) {
-    // TODO(vinod): As a short term fix for the race condition #1 in
-    // MESOS-1866, we inject the 'terminate' event at the end of the
-    // CRAMMD5AuthenticatorProcess queue instead of at the front.
-    // The long term fix for this is https://reviews.apache.org/r/25945/.
-    process::terminate(process, false);
-
-    process::wait(process);
-    delete process;
+  if (initialized) {
+    return Error("Authenticator initialized already");
   }
+
+  if (credentials.isNone()) {
+    return Error("Authenticator requires credentials");
+  }
+
+  secrets::load(credentials.get());
+
+  LOG(INFO) << "Initializing server SASL";
+
+  int result = sasl_server_init(NULL, "mesos");
+
+  if (result != SASL_OK) {
+    std::string error = "Failed to initialize SASL: ";
+    error += sasl_errstring(result, NULL, NULL);
+    return Error(error);
+  }
+
+  result = sasl_auxprop_add_plugin(
+      InMemoryAuxiliaryPropertyPlugin::name(),
+      &InMemoryAuxiliaryPropertyPlugin::initialize);
+
+  if (result != SASL_OK) {
+    std::string error =
+      "Failed to add \"in-memory\" auxiliary property plugin: ";
+    error += sasl_errstring(result, NULL, NULL);
+    return Error(error);
+  }
+
+  initialized = true;
+
+  return Nothing();
+}
+
+
+Try<AuthenticatorSession*> CRAMMD5Authenticator::session(
+    const process::UPID& pid)
+{
+  if (!initialized) {
+    return Error("Authenticator not initialized");
+  }
+
+  return new CRAMMD5AuthenticatorSession(pid);
 }
 
 
-void CRAMMD5Authenticator::initialize(const process::UPID& pid)
+CRAMMD5AuthenticatorSession::CRAMMD5AuthenticatorSession(
+    const process::UPID& pid) : AuthenticatorSession(pid)
 {
-  CHECK(process == NULL) << "Authenticator has already been initialized";
-  process = new CRAMMD5AuthenticatorProcess(pid);
+  process = new CRAMMD5AuthenticatorSessionProcess(pid);
   process::spawn(process);
 }
 
 
-process::Future<Option<std::string>> CRAMMD5Authenticator::authenticate(void)
+CRAMMD5AuthenticatorSession::~CRAMMD5AuthenticatorSession()
+{
+  // TODO(vinod): As a short term fix for the race condition #1 in
+  // MESOS-1866, we inject the 'terminate' event at the end of the
+  // CRAMMD5AuthenticatorProcess queue instead of at the front.
+  // The long term fix for this is https://reviews.apache.org/r/25945/.
+  process::terminate(process, false);
+  process::wait(process);
+  delete process;
+}
+
+
+process::Future<Option<std::string>> CRAMMD5AuthenticatorSession::authenticate()
 {
-  CHECK(process != NULL) << "Authenticator has not been initialized";
   return process::dispatch(
-      process, &CRAMMD5AuthenticatorProcess::authenticate);
+      process, &CRAMMD5AuthenticatorSessionProcess::authenticate);
 }
 
 } // namespace cram_md5 {

http://git-wip-us.apache.org/repos/asf/mesos/blob/03ce3268/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 63ca19a..385df85 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -214,6 +214,7 @@ Master::Master(
     contender(_contender),
     detector(_detector),
     authorizer(_authorizer),
+    authenticator(NULL),
     metrics(*this),
     electedTime(None())
 {
@@ -313,6 +314,22 @@ void Master::initialize()
     LOG(INFO) << "Master allowing unauthenticated slaves to register";
   }
 
+  // Load credentials.
+  if (flags.credentials.isSome()) {
+    const string& path =
+      strings::remove(flags.credentials.get(), "file://", strings::PREFIX);
+
+    Result<Credentials> _credentials = credentials::read(path);
+    if (_credentials.isError()) {
+      EXIT(1) << _credentials.error() << " (see --credentials flag)";
+    } else if (_credentials.isNone()) {
+      EXIT(1) << "Credentials file must contain at least one credential"
+              << " (see --credentials flag)";
+    }
+    // Store credentials in master to use them in routes.
+    credentials = _credentials.get();
+  }
+
   // Extract authenticator names and validate them.
   authenticatorNames = strings::split(flags.authenticators, ",");
   if (authenticatorNames.empty()) {
@@ -330,25 +347,44 @@ void Master::initialize()
             << "(see --modules)";
   }
 
-  // Load credentials.
-  if (flags.credentials.isSome()) {
-    const string& path =
-      strings::remove(flags.credentials.get(), "file://", strings::PREFIX);
-
-    Result<Credentials> _credentials = credentials::read(path);
-    if (_credentials.isError()) {
-      EXIT(1) << _credentials.error() << " (see --credentials flag)";
-    } else if (_credentials.isNone()) {
-      EXIT(1) << "Credentials file must contain at least one credential"
-              << " (see --credentials flag)";
+  // TODO(tillt): Allow multiple authenticators to be loaded and enable
+  // the authenticatee to select the appropriate one. See MESOS-1939.
+  if (authenticatorNames[0] == DEFAULT_AUTHENTICATOR) {
+    LOG(INFO) << "Using default '" << DEFAULT_AUTHENTICATOR
+              << "' authenticator";
+    authenticator = new cram_md5::CRAMMD5Authenticator();
+  } else {
+    Try<Authenticator*> module =
+      modules::ModuleManager::create<Authenticator>(authenticatorNames[0]);
+    if (module.isError()) {
+      EXIT(1) << "Could not create authenticator module '"
+              << authenticatorNames[0] << "': " << module.error();
     }
-    // Store credentials in master to use them in routes.
-    credentials = _credentials.get();
+    LOG(INFO) << "Using '" << authenticatorNames[0] << "' authenticator";
+    authenticator = module.get();
+  }
 
-    // Give Authenticator access to credentials.
-    // TODO(tillt): Move this into a mechanism (module) specific
-    // Authenticator factory. See MESOS-2050.
-    cram_md5::secrets::load(credentials.get());
+  // Give Authenticator access to credentials when needed.
+  Try<Nothing> initialize = authenticator->initialize(credentials);
+  if (initialize.isError()) {
+    const string error =
+      "Could not initialize authenticator '" + authenticatorNames[0] +
+      "': " + initialize.error();
+    if (flags.authenticate_frameworks || flags.authenticate_slaves) {
+      EXIT(1) << "Cannot start master with authentication enabled: " << error;
+    } else {
+      // A failure to initialize the authenticator does lead to
+      // unusable authentication but still allows non authenticating
+      // frameworks and slaves to connect.
+      if (credentials.isSome() ||
+          (authenticatorNames[0] != DEFAULT_AUTHENTICATOR)) {
+        LOG(WARNING) << "Only non-authenticating frameworks and slaves are "
+                     << "allowed to connect. "
+                     << "Authentication is disabled: " << error;
+      }
+      delete authenticator;
+      authenticator = NULL;
+    }
   }
 
   if (authorizer.isSome()) {
@@ -731,6 +767,8 @@ void Master::finalize()
   terminate(whitelistWatcher);
   wait(whitelistWatcher);
   delete whitelistWatcher;
+
+  delete authenticator;
 }
 
 
@@ -3992,6 +4030,8 @@ void Master::offer(const FrameworkID& frameworkId,
 // authenticate with master they would be stepping on each other's
 // toes. Currently it is tricky to detect this case because the
 // 'authenticate' message doesn't contain the 'FrameworkID'.
+// 'from' is the authenticatee process with which to communicate.
+// 'pid' is the framework/slave process being authenticated.
 void Master::authenticate(const UPID& from, const UPID& pid)
 {
   ++metrics.messages_authenticate;
@@ -4028,10 +4068,10 @@ void Master::authenticate(const UPID& from, const UPID& pid)
               << " because authentication is still in progress";
 
     // Try to cancel the in progress authentication by deleting
-    // the authenticator.
-    authenticators.erase(pid);
+    // the authenticator session.
+    authenticatorSessions.erase(pid);
 
-    // Retry after the current authenticator finishes.
+    // Retry after the current authenticator session finishes.
     authenticating[pid]
       .onAny(defer(self(), &Self::authenticate, from, pid));
 
@@ -4044,32 +4084,39 @@ void Master::authenticate(const UPID& from, const UPID& pid)
   // procedure. We'll set this _after_ we finish _authenticate.
   Owned<Promise<Nothing>> promise(new Promise<Nothing>());
 
-  // Create and initialize the authenticator.
-  Authenticator* authenticator;
-  // TODO(tillt): Allow multiple authenticators to be loaded and enable
-  // the authenticatee to select the appropriate one. See MESOS-1939.
-  if (authenticatorNames[0] == DEFAULT_AUTHENTICATOR) {
-    LOG(INFO) << "Using default CRAM-MD5 authenticator";
-    authenticator = new cram_md5::CRAMMD5Authenticator();
-  } else {
-    Try<Authenticator*> module =
-      modules::ModuleManager::create<Authenticator>(authenticatorNames[0]);
-    if (module.isError()) {
-      EXIT(1) << "Could not create authenticator module '"
-              << authenticatorNames[0] << "': " << module.error();
-    }
-    LOG(INFO) << "Using '" << authenticatorNames[0] << "' authenticator";
-    authenticator = module.get();
+  if (authenticator == NULL) {
+    std::string error = "No authenticator loaded";
+    LOG(ERROR) << error;
+
+    AuthenticationErrorMessage message;
+    message.set_error(error);
+    send(from, message);
+
+    return;
+  }
+
+  Try<AuthenticatorSession*> session = authenticator->session(from);
+  if (session.isError()) {
+    const std::string error =
+      "Authenticator '" + authenticatorNames[0] + "' failed: " +
+      session.error();
+    LOG(ERROR) << error;
+
+    AuthenticationErrorMessage message;
+    message.set_error(error);
+    send(from, message);
+
+    return;
   }
-  Owned<Authenticator> authenticator_ = Owned<Authenticator>(authenticator);
 
-  authenticator_->initialize(from);
+  Owned<AuthenticatorSession> session_ =
+    Owned<AuthenticatorSession>(session.get());
 
   // Start authentication.
-  const Future<Option<string>>& future = authenticator_->authenticate()
+  const Future<Option<string>>& future = session_->authenticate()
      .onAny(defer(self(), &Self::_authenticate, pid, promise, lambda::_1));
 
-  // Don't wait for authentication to happen for ever.
+  // Don't wait for authentication to happen forever.
   delay(Seconds(5),
         self(),
         &Self::authenticationTimeout,
@@ -4077,7 +4124,7 @@ void Master::authenticate(const UPID& from, const UPID& pid)
 
   // Save our state.
   authenticating[pid] = promise->future();
-  authenticators.put(pid, authenticator_);
+  authenticatorSessions.put(pid, session_);
 }
 
 
@@ -4103,7 +4150,7 @@ void Master::_authenticate(
     authenticated.put(pid, future.get().get());
   }
 
-  authenticators.erase(pid);
+  authenticatorSessions.erase(pid);
   authenticating.erase(pid);
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/03ce3268/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 26116af..6ccfb79 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -70,6 +70,7 @@ class Slaves;
 }
 
 class Authenticator;
+class AuthenticatorSession;
 class Authorizer;
 class WhitelistWatcher;
 
@@ -585,12 +586,15 @@ private:
   // Authenticator names as supplied via flags.
   std::vector<std::string> authenticatorNames;
 
+  Authenticator* authenticator;
+
   // Frameworks/slaves that are currently in the process of authentication.
   // 'authenticating' future for an authenticatee is ready when it is
   // authenticated.
   hashmap<process::UPID, process::Future<Nothing>> authenticating;
 
-  hashmap<process::UPID, process::Owned<Authenticator>> authenticators;
+  hashmap<process::UPID, process::Owned<AuthenticatorSession>>
+    authenticatorSessions;
 
   // Principals of authenticated frameworks/slaves keyed by PID.
   hashmap<process::UPID, std::string> authenticated;

http://git-wip-us.apache.org/repos/asf/mesos/blob/03ce3268/src/tests/cram_md5_authentication_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/cram_md5_authentication_tests.cpp b/src/tests/cram_md5_authentication_tests.cpp
index a356aa1..5c201d4 100644
--- a/src/tests/cram_md5_authentication_tests.cpp
+++ b/src/tests/cram_md5_authentication_tests.cpp
@@ -92,8 +92,6 @@ TYPED_TEST(CRAMMD5Authentication, success)
   credential2->set_principal(credential1.principal());
   credential2->set_secret(credential1.secret());
 
-  secrets::load(credentials);
-
   Future<Message> message =
     FUTURE_MESSAGE(Eq(AuthenticateMessage().GetTypeName()), _, _);
 
@@ -108,15 +106,20 @@ TYPED_TEST(CRAMMD5Authentication, success)
   Try<Authenticator*> authenticator = TypeParam::TypeAuthenticator::create();
   CHECK_SOME(authenticator);
 
-  authenticator.get()->initialize(message.get().from);
+  EXPECT_SOME(authenticator.get()->initialize(credentials));
+
+  Try<AuthenticatorSession*> session =
+    authenticator.get()->session(message.get().from);
+  CHECK_SOME(session);
 
-  Future<Option<string>> principal = authenticator.get()->authenticate();
+  Future<Option<string>> principal = session.get()->authenticate();
 
   AWAIT_EQ(true, client);
   AWAIT_READY(principal);
   EXPECT_SOME_EQ("benh", principal.get());
 
   terminate(pid);
+  delete session.get();
   delete authenticator.get();
   delete authenticatee.get();
 }
@@ -137,8 +140,6 @@ TYPED_TEST(CRAMMD5Authentication, failed1)
   credential2->set_principal(credential1.principal());
   credential2->set_secret("secret2");
 
-  secrets::load(credentials);
-
   Future<Message> message =
     FUTURE_MESSAGE(Eq(AuthenticateMessage().GetTypeName()), _, _);
 
@@ -153,15 +154,20 @@ TYPED_TEST(CRAMMD5Authentication, failed1)
   Try<Authenticator*> authenticator = TypeParam::TypeAuthenticator::create();
   CHECK_SOME(authenticator);
 
-  authenticator.get()->initialize(message.get().from);
+  EXPECT_SOME(authenticator.get()->initialize(credentials));
 
-  Future<Option<string>> server = authenticator.get()->authenticate();
+  Try<AuthenticatorSession*> session =
+    authenticator.get()->session(message.get().from);
+  CHECK_SOME(session);
+
+  Future<Option<string>> server = session.get()->authenticate();
 
   AWAIT_EQ(false, client);
   AWAIT_READY(server);
   EXPECT_NONE(server.get());
 
   terminate(pid);
+  delete session.get();
   delete authenticator.get();
   delete authenticatee.get();
 }
@@ -182,8 +188,6 @@ TYPED_TEST(CRAMMD5Authentication, failed2)
   credential2->set_principal("vinod");
   credential2->set_secret(credential1.secret());
 
-  secrets::load(credentials);
-
   Future<Message> message =
     FUTURE_MESSAGE(Eq(AuthenticateMessage().GetTypeName()), _, _);
 
@@ -198,23 +202,28 @@ TYPED_TEST(CRAMMD5Authentication, failed2)
   Try<Authenticator*> authenticator = TypeParam::TypeAuthenticator::create();
   CHECK_SOME(authenticator);
 
-  authenticator.get()->initialize(message.get().from);
+  EXPECT_SOME(authenticator.get()->initialize(credentials));
 
-  Future<Option<string>> server = authenticator.get()->authenticate();
+  Try<AuthenticatorSession*> session =
+    authenticator.get()->session(message.get().from);
+  CHECK_SOME(session);
+
+  Future<Option<string>> server = session.get()->authenticate();
 
   AWAIT_EQ(false, client);
   AWAIT_READY(server);
   EXPECT_NONE(server.get());
 
   terminate(pid);
+  delete session.get();
   delete authenticator.get();
   delete authenticatee.get();
 }
 
 
 // This test verifies that the pending future returned by
-// 'Authenticator::authenticate()' is properly failed when the Authenticator is
-// destructed in the middle of authentication.
+// 'Authenticator::authenticate()' is properly failed when the
+// Authenticator Session is destroyed in the middle of authentication.
 TYPED_TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
 {
   // Launch a dummy process (somebody to send the AuthenticateMessage).
@@ -229,8 +238,6 @@ TYPED_TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
   credential2->set_principal(credential1.principal());
   credential2->set_secret(credential1.secret());
 
-  secrets::load(credentials);
-
   Future<Message> message =
     FUTURE_MESSAGE(Eq(AuthenticateMessage().GetTypeName()), _, _);
 
@@ -245,14 +252,18 @@ TYPED_TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
   Try<Authenticator*> authenticator = TypeParam::TypeAuthenticator::create();
   CHECK_SOME(authenticator);
 
-  authenticator.get()->initialize(message.get().from);
+  EXPECT_SOME(authenticator.get()->initialize(credentials));
+
+  Try<AuthenticatorSession*> session =
+    authenticator.get()->session(message.get().from);
+  CHECK_SOME(session);
 
-  // Drop the AuthenticationStepMessage from authenticator to keep
-  // the authentication from getting completed.
+  // Drop the AuthenticationStepMessage from authenticator session to
+  // keep the authentication from getting completed.
   Future<AuthenticationStepMessage> authenticationStepMessage =
     DROP_PROTOBUF(AuthenticationStepMessage(), _, _);
 
-  Future<Option<string>> principal = authenticator.get()->authenticate();
+  Future<Option<string>> principal = session.get()->authenticate();
 
   AWAIT_READY(authenticationStepMessage);
 
@@ -262,16 +273,30 @@ TYPED_TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
   // Authentication should be pending.
   ASSERT_TRUE(principal.isPending());
 
-  // Now delete the authenticator.
-  delete authenticator.get();
+  // Now delete the authenticator session.
+  delete session.get();
 
   // The future should be failed at this point.
   AWAIT_FAILED(principal);
 
   terminate(pid);
+  delete authenticator.get();
   delete authenticatee.get();
 }
 
+
+// Missing credentials should fail the default (CRAM-MD5)
+// authenticator initialization.
+TYPED_TEST(CRAMMD5Authentication, AuthenticatorCredentialsMissing)
+{
+  Try<Authenticator*> authenticator = TypeParam::TypeAuthenticator::create();
+  CHECK_SOME(authenticator);
+
+  EXPECT_ERROR(authenticator.get()->initialize(None()));
+
+  delete authenticator.get();
+}
+
 } // namespace cram_md5 {
 } // namespace internal {
 } // namespace mesos {


[2/2] mesos git commit: Moved allocator specific declaration into allocator tests.

Posted by me...@apache.org.
Moved allocator specific declaration into allocator tests.

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


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

Branch: refs/heads/master
Commit: 585e4acb6f833181216a9b7724a0f97bfc85da36
Parents: 03ce326
Author: Alexander Rukletsov <al...@mesosphere.io>
Authored: Tue Jan 13 23:05:59 2015 -0800
Committer: Adam B <ad...@mesosphere.io>
Committed: Tue Jan 13 23:05:59 2015 -0800

----------------------------------------------------------------------
 src/tests/master_allocator_tests.cpp | 3 +++
 src/tests/mesos.hpp                  | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/585e4acb/src/tests/master_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_allocator_tests.cpp b/src/tests/master_allocator_tests.cpp
index 0e2e5b5..2430622 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -84,6 +84,9 @@ protected:
 };
 
 
+typedef ::testing::Types<HierarchicalDRFAllocatorProcess> AllocatorTypes;
+
+
 // Causes all TYPED_TEST(MasterAllocatorTest, ...) to be run for
 // each of the specified Allocator classes.
 TYPED_TEST_CASE(MasterAllocatorTest, AllocatorTypes);

http://git-wip-us.apache.org/repos/asf/mesos/blob/585e4acb/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index f93c3f1..591134b 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -47,7 +47,6 @@
 
 #include "master/allocator.hpp"
 #include "master/detector.hpp"
-#include "master/hierarchical_allocator_process.hpp"
 #include "master/master.hpp"
 
 #include "slave/slave.hpp"
@@ -795,10 +794,6 @@ public:
 };
 
 
-typedef ::testing::Types<master::allocator::HierarchicalDRFAllocatorProcess>
-AllocatorTypes;
-
-
 // The following actions make up for the fact that DoDefault
 // cannot be used inside a DoAll, for example:
 // EXPECT_CALL(allocator, addFramework(_, _, _))