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 2016/07/22 09:43:30 UTC

[6/6] mesos git commit: Refactored common HTTP authenticator initialize into helper function.

Refactored common HTTP authenticator initialize into helper function.

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


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

Branch: refs/heads/1.0.x
Commit: 71f8bc38c198b539fe9a0dd3fa2cb3990664c002
Parents: 67369c0
Author: Zhitao Li <zh...@gmail.com>
Authored: Fri Jul 22 01:19:51 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Fri Jul 22 02:02:25 2016 -0700

----------------------------------------------------------------------
 src/common/http.cpp      |  74 +++++++++++++++++++++++++++
 src/common/http.hpp      |  20 ++++++++
 src/master/constants.hpp |   3 --
 src/master/flags.cpp     |   1 +
 src/master/master.cpp    | 115 +++++++++---------------------------------
 src/master/master.hpp    |   7 ---
 src/slave/constants.hpp  |   3 --
 src/slave/flags.cpp      |   1 +
 src/slave/slave.cpp      | 101 ++++++-------------------------------
 src/slave/slave.hpp      |   7 ---
 10 files changed, 136 insertions(+), 196 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index d73170d..65e7323 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -25,7 +25,9 @@
 #include <mesos/http.hpp>
 #include <mesos/resources.hpp>
 
+#include <mesos/authentication/http/basic_authenticator_factory.hpp>
 #include <mesos/authorizer/authorizer.hpp>
+#include <mesos/module/http_authenticator.hpp>
 
 #include <process/dispatch.hpp>
 #include <process/future.hpp>
@@ -43,6 +45,7 @@
 #include "common/http.hpp"
 
 #include "messages/messages.hpp"
+#include "module/manager.hpp"
 
 using std::map;
 using std::ostream;
@@ -53,8 +56,11 @@ using std::vector;
 using process::Failure;
 using process::Owned;
 
+using process::http::authentication::Authenticator;
 using process::http::authorization::AuthorizationCallbacks;
 
+using mesos::http::authentication::BasicAuthenticatorFactory;
+
 namespace mesos {
 
 ostream& operator<<(ostream& stream, ContentType contentType)
@@ -851,4 +857,72 @@ bool approveViewRole(
   return approved.get();
 }
 
+
+Try<Nothing> initializeHttpAuthenticators(
+    const string& realm,
+    const vector<string>& httpAuthenticatorNames,
+    const Option<Credentials>& credentials)
+{
+  if (httpAuthenticatorNames.empty()) {
+    return Error("No HTTP authenticator specified for realm '" + realm + "'");
+  }
+
+  if (httpAuthenticatorNames.size() > 1) {
+    return Error("Multiple HTTP authenticators not supported");
+  }
+
+  Option<Authenticator*> httpAuthenticator;
+  if (httpAuthenticatorNames[0] == internal::DEFAULT_HTTP_AUTHENTICATOR) {
+    if (credentials.isNone()) {
+      return Error(
+          "No credentials provided for the default '" +
+          string(internal::DEFAULT_HTTP_AUTHENTICATOR) +
+          "' HTTP authenticator for realm '" + realm + "'");
+    }
+
+    LOG(INFO) << "Using default '" << internal::DEFAULT_HTTP_AUTHENTICATOR
+              << "' HTTP authenticator for realm '" << realm << "'";
+
+    Try<Authenticator*> authenticator =
+      BasicAuthenticatorFactory::create(realm, credentials.get());
+    if (authenticator.isError()) {
+      return Error(
+          "Could not create HTTP authenticator module '" +
+          httpAuthenticatorNames[0] + "': " + authenticator.error());
+    }
+
+    httpAuthenticator = authenticator.get();
+  } else {
+    if (!modules::ModuleManager::contains<Authenticator>(
+          httpAuthenticatorNames[0])) {
+      return Error(
+          "HTTP authenticator '" + httpAuthenticatorNames[0] +
+          "' not found. Check the spelling (compare to '" +
+          string(internal::DEFAULT_HTTP_AUTHENTICATOR) +
+          "') or verify that the authenticator was loaded "
+          "successfully (see --modules)");
+    }
+
+    Try<Authenticator*> module =
+      modules::ModuleManager::create<Authenticator>(httpAuthenticatorNames[0]);
+    if (module.isError()) {
+      return Error(
+          "Could not create HTTP authenticator module '" +
+          httpAuthenticatorNames[0] + "': " + module.error());
+    }
+    LOG(INFO) << "Using '" << httpAuthenticatorNames[0]
+              << "' HTTP authenticator for realm '" << realm << "'";
+    httpAuthenticator = module.get();
+  }
+
+  CHECK(httpAuthenticator.isSome());
+
+  // Ownership of the `httpAuthenticator` is passed to libprocess.
+  process::http::authentication::setAuthenticator(
+    realm, Owned<Authenticator>(httpAuthenticator.get()));
+
+  return Nothing();
+}
+
+
 }  // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index 2dfa789..145f64b 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -42,6 +42,9 @@ class Task;
 
 namespace internal {
 
+// Name of the default, basic authenticator.
+constexpr char DEFAULT_HTTP_AUTHENTICATOR[] = "basic";
+
 extern hashset<std::string> AUTHORIZABLE_ENDPOINTS;
 
 // Serializes a protobuf message for transmission
@@ -162,6 +165,23 @@ bool approveViewRole(
     const process::Owned<ObjectApprover>& rolesApprover,
     const std::string& role);
 
+
+/**
+ * Helper function to create HTTP authenticators
+ * for a given realm and register in libprocess.
+ *
+ * @param realm name of the realm.
+ * @param authenticatorNames a vector of authenticator names.
+ * @param credentials optional credentials for BasicAuthenticator only.
+ * @return nothing if authenticators are initialized and registered to
+ *         libprocess successfully, or error if authenticators cannot
+ *         be initialized.
+ */
+Try<Nothing> initializeHttpAuthenticators(
+    const std::string& realm,
+    const std::vector<std::string>& authenticatorNames,
+    const Option<Credentials>& credentials);
+
 } // namespace mesos {
 
 #endif // __COMMON_HTTP_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/master/constants.hpp
----------------------------------------------------------------------
diff --git a/src/master/constants.hpp b/src/master/constants.hpp
index e353b63..cd80dac 100644
--- a/src/master/constants.hpp
+++ b/src/master/constants.hpp
@@ -121,9 +121,6 @@ constexpr Duration DEFAULT_ALLOCATION_INTERVAL = Seconds(1);
 // Name of the default, local authorizer.
 constexpr char DEFAULT_AUTHORIZER[] = "local";
 
-// Name of the default, basic authenticator.
-constexpr char DEFAULT_HTTP_AUTHENTICATOR[] = "basic";
-
 // Name of the master HTTP authentication realm for read-only endpoints.
 constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] =
   "mesos-master-readonly";

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/master/flags.cpp
----------------------------------------------------------------------
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index 93ddf6b..176133f 100644
--- a/src/master/flags.cpp
+++ b/src/master/flags.cpp
@@ -19,6 +19,7 @@
 
 #include <stout/flags.hpp>
 
+#include "common/http.hpp"
 #include "common/parse.hpp"
 #include "master/constants.hpp"
 #include "master/flags.hpp"

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 219e850..cf2cb58 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -29,8 +29,6 @@
 
 #include <mesos/authentication/authenticator.hpp>
 
-#include <mesos/authentication/http/basic_authenticator_factory.hpp>
-
 #include <mesos/authorizer/authorizer.hpp>
 
 #include <mesos/allocator/allocator.hpp>
@@ -38,7 +36,6 @@
 #include <mesos/master/detector.hpp>
 
 #include <mesos/module/authenticator.hpp>
-#include <mesos/module/http_authenticator.hpp>
 
 #include <mesos/scheduler/scheduler.hpp>
 
@@ -122,16 +119,12 @@ namespace mesos {
 namespace internal {
 namespace master {
 
-namespace authentication = process::http::authentication;
-
 using mesos::allocator::Allocator;
 
 using mesos::master::contender::MasterContender;
 
 using mesos::master::detector::MasterDetector;
 
-using mesos::http::authentication::BasicAuthenticatorFactory;
-
 static bool isValidFailoverTimeout(const FrameworkInfo& frameworkinfo);
 
 class SlaveObserver : public ProtobufProcess<SlaveObserver>
@@ -377,78 +370,6 @@ struct BoundedRateLimiter
 };
 
 
-void Master::registerHttpAuthenticator(
-    const string& realm,
-    const Option<Credentials>& credentials,
-    const vector<string>& httpAuthenticatorNames)
-{
-  // At least the default authenticator is always specified.
-  // Passing an empty string into the `http_authenticators`
-  // flag is considered an error.
-  if (httpAuthenticatorNames.empty()) {
-    EXIT(EXIT_FAILURE)
-      << "No HTTP authenticator specified for realm '" << realm << "'";
-  }
-
-  if (httpAuthenticatorNames.size() > 1) {
-    EXIT(EXIT_FAILURE) << "Multiple HTTP authenticators not supported";
-  }
-
-  if (httpAuthenticatorNames[0] != DEFAULT_HTTP_AUTHENTICATOR &&
-      !modules::ModuleManager::contains<authentication::Authenticator>(
-          httpAuthenticatorNames[0])) {
-    EXIT(EXIT_FAILURE)
-      << "HTTP authenticator '" << httpAuthenticatorNames[0] << "' not"
-      << " found. Check the spelling (compare to '"
-      << DEFAULT_HTTP_AUTHENTICATOR << "') or verify that the"
-      << " authenticator was loaded successfully (see --modules)";
-  }
-
-  Option<authentication::Authenticator*> httpAuthenticator;
-  if (httpAuthenticatorNames[0] == DEFAULT_HTTP_AUTHENTICATOR) {
-    if (credentials.isNone()) {
-      EXIT(EXIT_FAILURE)
-        << "No credentials provided for the default '"
-        << DEFAULT_HTTP_AUTHENTICATOR << "' HTTP authenticator for realm '"
-        << realm << "'";
-    }
-
-    LOG(INFO) << "Using default '" << DEFAULT_HTTP_AUTHENTICATOR
-              << "' HTTP authenticator for realm '" << realm << "'";
-
-    Try<authentication::Authenticator*> authenticator =
-      BasicAuthenticatorFactory::create(realm, credentials.get());
-    if (authenticator.isError()) {
-      EXIT(EXIT_FAILURE)
-        << "Could not create HTTP authenticator module '"
-        << httpAuthenticatorNames[0] << "': " << authenticator.error();
-    }
-
-    httpAuthenticator = authenticator.get();
-  } else {
-    Try<authentication::Authenticator*> module =
-      modules::ModuleManager::create<authentication::Authenticator>(
-          httpAuthenticatorNames[0]);
-    if (module.isError()) {
-      EXIT(EXIT_FAILURE)
-        << "Could not create HTTP authenticator module '"
-        << httpAuthenticatorNames[0] << "': " << module.error();
-    }
-    LOG(INFO) << "Using '" << httpAuthenticatorNames[0]
-              << "' HTTP authenticator for realm '" << realm << "'";
-    httpAuthenticator = module.get();
-  }
-
-  if (httpAuthenticator.isSome() && httpAuthenticator.get() != nullptr) {
-    // Ownership of the `httpAuthenticator` is passed to libprocess.
-    process::http::authentication::setAuthenticator(
-        realm,
-        Owned<authentication::Authenticator>(httpAuthenticator.get()));
-    httpAuthenticator = None();
-  }
-}
-
-
 void Master::initialize()
 {
   LOG(INFO) << "Master " << info_.id() << " (" << info_.hostname() << ")"
@@ -614,17 +535,25 @@ void Master::initialize()
 #endif // HAS_AUTHENTICATION
 
   if (flags.authenticate_http_readonly) {
-    registerHttpAuthenticator(
-      READONLY_HTTP_AUTHENTICATION_REALM,
-      credentials,
-      strings::split(flags.http_authenticators, ","));
+    Try<Nothing> result = initializeHttpAuthenticators(
+        READONLY_HTTP_AUTHENTICATION_REALM,
+        strings::split(flags.http_authenticators, ","),
+        credentials);
+
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE) << result.error();
+    }
   }
 
   if (flags.authenticate_http_readwrite) {
-    registerHttpAuthenticator(
-      READWRITE_HTTP_AUTHENTICATION_REALM,
-      credentials,
-      strings::split(flags.http_authenticators, ","));
+    Try<Nothing> result = initializeHttpAuthenticators(
+        READWRITE_HTTP_AUTHENTICATION_REALM,
+        strings::split(flags.http_authenticators, ","),
+        credentials);
+
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE) << result.error();
+    }
   }
 
   if (flags.authenticate_http_frameworks) {
@@ -636,10 +565,14 @@ void Master::initialize()
         << "in conjunction with `--authenticate_http_frameworks`";
     }
 
-    registerHttpAuthenticator(
-      DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM,
-      credentials,
-      strings::split(flags.http_framework_authenticators.get(), ","));
+    Try<Nothing> result = initializeHttpAuthenticators(
+        DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM,
+        strings::split(flags.http_framework_authenticators.get(), ","),
+        credentials);
+
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE) << result.error();
+    }
   }
 
   if (authorizer.isSome()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index bed6b8a..46621b8 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1870,13 +1870,6 @@ private:
   process::Future<Option<Error>> validate(
       const FrameworkInfo& frameworkInfo,
       const process::UPID& from);
-
-  // Helper function to create HTTP authenticator
-  // for a given realm and register in libprocess.
-  void registerHttpAuthenticator(
-      const std::string& realm,
-      const Option<Credentials>& credentials,
-      const std::vector<std::string>& authenticatorNames);
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index 46ef54e..360dd05 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -124,9 +124,6 @@ constexpr char DEFAULT_AUTHENTICATEE[] = "crammd5";
 // Name of the default, local authorizer.
 constexpr char DEFAULT_AUTHORIZER[] = "local";
 
-// Name of the default HTTP authenticator.
-constexpr char DEFAULT_HTTP_AUTHENTICATOR[] = "basic";
-
 // Name of the agent HTTP authentication realm for read-only endpoints.
 constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] = "mesos-agent-readonly";
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index d7714de..b0c02ae 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -25,6 +25,7 @@
 
 #include <mesos/type_utils.hpp>
 
+#include "common/http.hpp"
 #include "common/parse.hpp"
 
 #include "slave/constants.hpp"

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 0a0bc70..708f945 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -30,10 +30,7 @@
 
 #include <mesos/type_utils.hpp>
 
-#include <mesos/authentication/http/basic_authenticator_factory.hpp>
-
 #include <mesos/module/authenticatee.hpp>
-#include <mesos/module/http_authenticator.hpp>
 
 #include <process/async.hpp>
 #include <process/check.hpp>
@@ -133,10 +130,6 @@ namespace slave {
 
 using namespace state;
 
-namespace authentication = process::http::authentication;
-
-using mesos::http::authentication::BasicAuthenticatorFactory;
-
 Slave::Slave(const std::string& id,
              const slave::Flags& _flags,
              MasterDetector* _detector,
@@ -361,17 +354,25 @@ void Slave::initialize()
   }
 
   if (flags.authenticate_http_readonly) {
-    registerHttpAuthenticator(
-      READONLY_HTTP_AUTHENTICATION_REALM,
-      httpCredentials,
-      strings::split(flags.http_authenticators, ","));
+    Try<Nothing> result = initializeHttpAuthenticators(
+        READONLY_HTTP_AUTHENTICATION_REALM,
+        strings::split(flags.http_authenticators, ","),
+        httpCredentials);
+
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE) << result.error();
+    }
   }
 
   if (flags.authenticate_http_readwrite) {
-    registerHttpAuthenticator(
-      READWRITE_HTTP_AUTHENTICATION_REALM,
-      httpCredentials,
-      strings::split(flags.http_authenticators, ","));
+    Try<Nothing> result = initializeHttpAuthenticators(
+        READWRITE_HTTP_AUTHENTICATION_REALM,
+        strings::split(flags.http_authenticators, ","),
+        httpCredentials);
+
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE) << result.error();
+    }
   }
 
   if ((flags.gc_disk_headroom < 0) || (flags.gc_disk_headroom > 1)) {
@@ -761,76 +762,6 @@ void Slave::initialize()
 }
 
 
-void Slave::registerHttpAuthenticator(
-    const string& realm,
-    const Option<Credentials>& credentials,
-    const vector<string>& httpAuthenticatorNames)
-{
-  // At least the default authenticator is always specified.
-  // Passing an empty string into the `http_authenticators`
-  // flag is considered an error.
-  if (httpAuthenticatorNames.empty()) {
-    EXIT(EXIT_FAILURE)
-      << "No HTTP authenticator specified for realm '" << realm << "'";
-  }
-  if (httpAuthenticatorNames.size() > 1) {
-    EXIT(EXIT_FAILURE) << "Multiple HTTP authenticators not supported";
-  }
-  if (httpAuthenticatorNames[0] != DEFAULT_HTTP_AUTHENTICATOR &&
-      !modules::ModuleManager::contains<authentication::Authenticator>(
-          httpAuthenticatorNames[0])) {
-    EXIT(EXIT_FAILURE)
-      << "HTTP authenticator '" << httpAuthenticatorNames[0] << "' not"
-      << " found. Check the spelling (compare to '"
-      << DEFAULT_HTTP_AUTHENTICATOR << "') or verify that the"
-      << " authenticator was loaded successfully (see --modules)";
-  }
-
-  Option<authentication::Authenticator*> httpAuthenticator;
-  if (httpAuthenticatorNames[0] == DEFAULT_HTTP_AUTHENTICATOR) {
-    if (credentials.isNone()) {
-      EXIT(EXIT_FAILURE)
-        << "No credentials provided for the default '"
-        << DEFAULT_HTTP_AUTHENTICATOR << "' HTTP authenticator for realm '"
-        << realm << "'";
-    }
-
-    LOG(INFO) << "Using default '" << DEFAULT_HTTP_AUTHENTICATOR
-              << "' HTTP authenticator for realm '" << realm << "'";
-
-    Try<authentication::Authenticator*> authenticator =
-      BasicAuthenticatorFactory::create(realm, credentials.get());
-    if (authenticator.isError()) {
-      EXIT(EXIT_FAILURE)
-        << "Could not create HTTP authenticator module '"
-        << httpAuthenticatorNames[0] << "': " << authenticator.error();
-    }
-
-    httpAuthenticator = authenticator.get();
-  } else {
-    Try<authentication::Authenticator*> module =
-      modules::ModuleManager::create<authentication::Authenticator>(
-          httpAuthenticatorNames[0]);
-    if (module.isError()) {
-      EXIT(EXIT_FAILURE)
-        << "Could not create HTTP authenticator module '"
-        << httpAuthenticatorNames[0] << "': " << module.error();
-    }
-    LOG(INFO) << "Using '" << httpAuthenticatorNames[0]
-              << "' HTTP authenticator for realm '" << realm << "'";
-    httpAuthenticator = module.get();
-  }
-
-  if (httpAuthenticator.isSome() && httpAuthenticator.get() != nullptr) {
-    // Ownership of the `httpAuthenticator` is passed to libprocess.
-    process::http::authentication::setAuthenticator(
-        realm,
-        Owned<authentication::Authenticator>(httpAuthenticator.get()));
-    httpAuthenticator = None();
-  }
-}
-
-
 void Slave::finalize()
 {
   LOG(INFO) << "Agent terminating";

http://git-wip-us.apache.org/repos/asf/mesos/blob/71f8bc38/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index f9f725b..a8952f0 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -411,13 +411,6 @@ public:
       const ContainerID& containerId);
 
 private:
-  // Helper function to create HTTP authenticator
-  // for a given realm and register in libprocess.
-  void registerHttpAuthenticator(
-      const std::string& realm,
-      const Option<Credentials>& credentials,
-      const std::vector<std::string>& authenticatorNames);
-
   void _authenticate();
   void authenticationTimeout(process::Future<bool> future);