You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2014/06/29 21:50:24 UTC

[1/4] git commit: HTTP authentication flag for basic/digest HTTP authentication.

Repository: mesos
Updated Branches:
  refs/heads/master 294337466 -> d4d253df8


HTTP authentication flag for basic/digest HTTP authentication.

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


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

Branch: refs/heads/master
Commit: 2cb3761c6bfa80b956eaafde9c69eafaeac3deae
Parents: 2943374
Author: Isabel Jimenez <co...@isabeljimenez.com>
Authored: Sun Jun 29 09:30:15 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 29 09:30:15 2014 -0700

----------------------------------------------------------------------
 include/mesos/mesos.proto       |  10 ++++
 src/Makefile.am                 |   1 +
 src/credentials/credentials.hpp |  73 +++++++++++++++++++---
 src/master/flags.hpp            |  28 ++++++++-
 src/master/master.cpp           |  14 +++--
 src/master/master.hpp           |   2 +
 src/sasl/authenticator.hpp      |  11 +---
 src/slave/flags.hpp             |  15 ++++-
 src/slave/slave.cpp             |  12 ++--
 src/tests/credentials_tests.cpp | 113 +++++++++++++++++++++++++++++++++++
 src/tests/mesos.cpp             |  26 ++++----
 11 files changed, 260 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index b41dc7f..ddbf55c 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -676,6 +676,16 @@ message Credential {
 
 
 /**
+ * Credentials used for authentication
+ *
+ */
+message Credentials {
+  repeated Credential registration = 1;
+  repeated Credential http = 2;
+}
+
+
+/**
  * ACLs used for authorization.
  */
 message ACL {

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index bc1603a..12d84bf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -996,6 +996,7 @@ mesos_tests_SOURCES =				\
   tests/authorization_tests.cpp		        \
   tests/containerizer.cpp			\
   tests/containerizer_tests.cpp			\
+  tests/credentials_tests.cpp			\
   tests/environment.cpp				\
   tests/examples_tests.cpp			\
   tests/exception_tests.cpp			\

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/credentials/credentials.hpp
----------------------------------------------------------------------
diff --git a/src/credentials/credentials.hpp b/src/credentials/credentials.hpp
index 98b9088..cb26da9 100644
--- a/src/credentials/credentials.hpp
+++ b/src/credentials/credentials.hpp
@@ -24,13 +24,14 @@
 
 #include <stout/option.hpp>
 #include <stout/os.hpp>
+#include <stout/protobuf.hpp>
 #include <stout/try.hpp>
 
 namespace mesos {
 namespace internal {
 namespace credentials {
 
-inline Result<std::vector<Credential> > read(const std::string& path)
+inline Result<Credentials> read(const std::string& path)
 {
   LOG(INFO) << "Loading credentials for authentication from '" << path << "'";
 
@@ -52,24 +53,78 @@ inline Result<std::vector<Credential> > read(const std::string& path)
                  << "credentials file is NOT accessible by others.";
   }
 
-  std::vector<Credential> credentials;
+  // TODO(ijimenez) deprecate text support only JSON like acls
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+  if (!json.isError()) {
+    Try<Credentials> credentials = ::protobuf::parse<Credentials>(json.get());
+    if (!credentials.isError()) {
+      return credentials.get();
+    }
+  }
+
+  Credentials credentials;
   foreach (const std::string& line, strings::tokenize(read.get(), "\n")) {
     const std::vector<std::string>& pairs = strings::tokenize(line, " ");
     if (pairs.size() != 2) {
-      return Error("Invalid credential format at line: " +
-                   stringify(credentials.size() + 1));
+        return Error("Invalid credential format at line " +
+                     credentials.registration().size() + 1);
     }
 
     // Add the credential.
-    Credential credential;
-    credential.set_principal(pairs[0]);
-    credential.set_secret(pairs[1]);
-    credentials.push_back(credential);
+    Credential *credential = credentials.add_registration();
+    credential->set_principal(pairs[0]);
+    credential->set_secret(pairs[1]);
   }
-
   return credentials;
 }
 
+
+inline Result<Credential> readCredential(const std::string& path)
+{
+  LOG(INFO) << "Loading credential for authentication from '" << path << "'";
+
+  Try<std::string> read = os::read(path);
+  if (read.isError()) {
+    return Error("Failed to read credential file '" + path +
+                 "': " + read.error());
+  } else if (read.get().empty()) {
+    return None();
+  }
+
+  Try<os::Permissions> permissions = os::permissions(path);
+  if (permissions.isError()) {
+    LOG(WARNING) << "Failed to stat credential file '" << path
+                 << "': " << permissions.error();
+  } else if (permissions.get().others.rwx) {
+    LOG(WARNING) << "Permissions on credential file '" << path
+                 << "' are too open. It is recommended that your "
+                 << "credential file is NOT accessible by others.";
+  }
+
+  // TODO(ijimenez) deprecate text support only JSON like acls
+  Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
+  if (!json.isError()) {
+    Try<Credential> credential = ::protobuf::parse<Credential>(json.get());
+    if (!credential.isError()) {
+      return credential.get();
+    }
+  }
+
+  Credential credential;
+  const std::vector<std::string>& line = strings::tokenize(read.get(), "\n");
+  if (line.size() != 1) {
+    return Error("Expecting only one credential");
+  }
+  const std::vector<std::string>& pairs = strings::tokenize(line[0], " ");
+  if (pairs.size() != 2) {
+    return Error("Invalid credential format");
+  }
+  // Add the credential.
+  credential.set_principal(pairs[0]);
+  credential.set_secret(pairs[1]);
+  return credential;
+}
+
 } // namespace credentials {
 } // namespace internal {
 } // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/master/flags.hpp
----------------------------------------------------------------------
diff --git a/src/master/flags.hpp b/src/master/flags.hpp
index cd2a70e..12c1b33 100644
--- a/src/master/flags.hpp
+++ b/src/master/flags.hpp
@@ -209,9 +209,31 @@ public:
 
     add(&Flags::credentials,
         "credentials",
-        "Path to a file with a list of credentials.\n"
-        "Each line contains 'principal' and 'secret' separated by whitespace.\n"
-        "Path could be of the form 'file:///path/to/file' or '/path/to/file'.");
+        "Either a path to a text file with a list of credentials,\n"
+        "each line containing 'principal' and 'secret' separated by "
+        "whitespace,\n"
+        "or, a path to a JSON-formatted file containing credentials\n"
+        "for identification/registration and http authentication."
+        "Path could be of the form 'file:///path/to/file' or '/path/to/file'."
+        "\n"
+        "JSON file Example:\n"
+        "{\n"
+        "  \"http\": [\n"
+        "             {\n"
+        "                \"principal\": \"username\",\n"
+        "                \"secret\": \"secret\",\n"
+        "             }\n"
+        "            ],\n"
+        "  \"identification\": [\n"
+        "                       {\n"
+        "                          \"principal\": \"username\",\n"
+        "                          \"secret\": \"secret\",\n"
+        "                       }\n"
+        "                      ]\n"
+        "}\n"
+        "Text file Example:\n"
+        "username:secret\n"
+        );
 
     add(&Flags::acls,
         "acls",

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 21b07c7..a549986 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -337,16 +337,20 @@ void Master::initialize()
     const string& path =
       strings::remove(flags.credentials.get(), "file://", strings::PREFIX);
 
-    Result<vector<Credential> > credentials = credentials::read(path);
-    if (credentials.isError()) {
-      EXIT(1) << credentials.error() << " (see --credentials flag)";
-    } else if (credentials.isNone()) {
+    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)";
     }
 
     // Give Authenticator access to credentials.
-    sasl::secrets::load(credentials.get());
+    sasl::secrets::load(_credentials.get());
+
+    // Store credentials in master
+    this->credentials = _credentials.get();
+
   } else if (flags.authenticate_frameworks || flags.authenticate_slaves) {
     EXIT(1) << "Authentication requires a credentials file"
             << " (see --credentials flag)";

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 5fef354..830b538 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -370,6 +370,8 @@ protected:
   OfferID newOfferId();
   SlaveID newSlaveId();
 
+  Option<Credentials> credentials;
+
 private:
   // Inner class used to namespace HTTP route handlers (see
   // master/http.cpp for implementations).

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/sasl/authenticator.hpp
----------------------------------------------------------------------
diff --git a/src/sasl/authenticator.hpp b/src/sasl/authenticator.hpp
index 365db5f..aa222d3 100644
--- a/src/sasl/authenticator.hpp
+++ b/src/sasl/authenticator.hpp
@@ -466,17 +466,12 @@ void load(const std::map<std::string, std::string>& secrets)
   InMemoryAuxiliaryPropertyPlugin::load(properties);
 }
 
-
-// Load credentials into the in-memory auxiliary propery plugin
-// that is used by the authenticators.
-void load(const std::vector<Credential>& credentials)
+void load(const Credentials& c)
 {
   std::map<std::string, std::string> secrets;
-
-  foreach (const Credential& credential, credentials) {
-    secrets[credential.principal()] = credential.secret();
+  foreach(const Credential& registration, c.registration()) {
+    secrets[registration.principal()] = registration.secret();
   }
-
   load(secrets);
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index c6a970f..1fe7b7d 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -243,9 +243,18 @@ public:
 
     add(&Flags::credential,
         "credential",
-        "Path to a file containing a single line with\n"
-        "the 'principal' and 'secret' separated by whitespace.\n"
-        "Path could be of the form 'file:///path/to/file' or '/path/to/file'");
+        "Either a path to a text with a single line\n"
+        "containing 'principal' and 'secret' separated by "
+        "whitespace.\n"
+        "Or a path containing the JSON "
+        "formatted information used for one credential.\n"
+        "Path could be of the form 'file:///path/to/file' or '/path/to/file'."
+        "\n"
+        "Example:\n"
+        "{\n"
+        "    \"principal\": \"username\",\n"
+        "    \"secret\": \"secret\",\n"
+        "}");
 
     add(&Flags::containerizer_path,
         "containerizer_path",

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 24eb8fd..f42ab60 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -255,16 +255,14 @@ void Slave::initialize()
     const string& path =
       strings::remove(flags.credential.get(), "file://", strings::PREFIX);
 
-    Result<vector<Credential> > credentials = credentials::read(path);
-    if (credentials.isError()) {
-      EXIT(1) << credentials.error() << " (see --credential flag)";
-    } else if (credentials.isNone()) {
+    Result<Credential> _credential = credentials::readCredential(path);
+    if (_credential.isError()) {
+      EXIT(1) << _credential.error() << " (see --credential flag)";
+    } else if (_credential.isNone()) {
       EXIT(1) << "Empty credential file '" << path
               << "' (see --credential flag)";
-    } else if (credentials.get().size() != 1) {
-      EXIT(1) << "Not expecting multiple credentials (see --credential flag)";
     } else {
-      credential = credentials.get()[0];
+      credential = _credential.get();
       LOG(INFO) << "Slave using credential for: "
                 << credential.get().principal();
     }

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/tests/credentials_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/credentials_tests.cpp b/src/tests/credentials_tests.cpp
new file mode 100644
index 0000000..418e02c
--- /dev/null
+++ b/src/tests/credentials_tests.cpp
@@ -0,0 +1,113 @@
+/**
+ * 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 <gmock/gmock.h>
+
+#include <string>
+
+#include <process/gmock.hpp>
+#include <process/pid.hpp>
+
+#include "master/flags.hpp"
+#include "master/master.hpp"
+#include "tests/mesos.hpp"
+#include "tests/utils.hpp"
+
+using std::map;
+using std::string;
+using std::vector;
+
+using namespace mesos;
+using namespace mesos::internal;
+using namespace mesos::internal::slave;
+using namespace mesos::internal::tests;
+
+using mesos::internal::master::Master;
+using mesos::internal::slave::Slave;
+
+using process::PID;
+
+using testing::_;
+using testing::Eq;
+using testing::Return;
+
+class CredentialsTest : public MesosTest {};
+
+
+// This test verifies that an authenticated slave is
+// granted registration by the master.
+TEST_F(CredentialsTest, authenticatedSlave)
+{
+  Try<PID<Master> > master = StartMaster();
+  ASSERT_SOME(master);
+
+  Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+  Try<PID<Slave> > slave = StartSlave();
+  ASSERT_SOME(slave);
+
+  AWAIT_READY(slaveRegisteredMessage);
+  ASSERT_NE("", slaveRegisteredMessage.get().slave_id().value());
+
+  Shutdown();
+}
+
+
+// Test verifing well executed credential authentication
+// using text formatted credentials so as to test
+// backwards compatibility
+TEST_F(CredentialsTest, authenticatedSlaveText)
+{
+  master::Flags flags = CreateMasterFlags();
+
+  const string& path =  path::join(os::getcwd(), "credentials");
+
+  Try<int> fd = os::open(
+      path,
+      O_WRONLY | O_CREAT | O_TRUNC,
+      S_IRUSR | S_IWUSR | S_IRGRP);
+
+  CHECK_SOME(fd);
+
+  const std::string& credentials =
+    DEFAULT_CREDENTIAL.principal() + " " + DEFAULT_CREDENTIAL.secret();
+  CHECK_SOME(os::write(fd.get(), credentials))
+     << "Failed to write credentials to '" << path << "'";
+  CHECK_SOME(os::close(fd.get()));
+
+  flags.credentials = "file://" + path;
+
+  Try<PID<Master> > master = StartMaster(flags);
+  ASSERT_SOME(master);
+
+  Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+  slave::Flags slaveFlags = CreateSlaveFlags();
+
+  slaveFlags.credential = "file://" + path;
+
+  Try<PID<Slave> > slave = StartSlave(slaveFlags);
+  ASSERT_SOME(slave);
+
+  AWAIT_READY(slaveRegisteredMessage);
+  ASSERT_NE("", slaveRegisteredMessage.get().slave_id().value());
+
+  Shutdown();
+}

http://git-wip-us.apache.org/repos/asf/mesos/blob/2cb3761c/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 0bed925..131d18b 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -91,12 +91,17 @@ master::Flags MesosTest::CreateMasterFlags()
 
   CHECK_SOME(fd);
 
-  const string& credentials =
-    DEFAULT_CREDENTIAL.principal() + " " + DEFAULT_CREDENTIAL.secret();
-
-  CHECK_SOME(os::write(fd.get(), credentials))
-    << "Failed to write credentials to '" << path << "'";
-
+  // JSON default format for credentials
+  Credentials credentials;
+  Credential *credential = credentials.add_registration();
+  credential->set_principal(DEFAULT_CREDENTIAL.principal());
+  credential->set_secret(DEFAULT_CREDENTIAL.secret());
+  credential = credentials.add_http();
+  credential->set_principal(DEFAULT_CREDENTIAL.principal());
+  credential->set_secret(DEFAULT_CREDENTIAL.secret());
+
+  CHECK_SOME(os::write(fd.get(), stringify(JSON::Protobuf(credentials))))
+     << "Failed to write credentials to '" << path << "'";
   CHECK_SOME(os::close(fd.get()));
 
   flags.credentials = "file://" + path;
@@ -137,11 +142,12 @@ slave::Flags MesosTest::CreateSlaveFlags()
 
   CHECK_SOME(fd);
 
-  const string& credential =
-    DEFAULT_CREDENTIAL.principal() + " " + DEFAULT_CREDENTIAL.secret();
+  Credential credential;
+  credential.set_principal(DEFAULT_CREDENTIAL.principal());
+  credential.set_secret(DEFAULT_CREDENTIAL.secret());
 
-  CHECK_SOME(os::write(fd.get(), credential))
-    << "Failed to write slave credential to '" << path << "'";
+  CHECK_SOME(os::write(fd.get(), stringify(JSON::Protobuf(credential))))
+     << "Failed to write slave credential to '" << path << "'";
 
   CHECK_SOME(os::close(fd.get()));
 


[3/4] git commit: Split version where N tokens can be specified.

Posted by be...@apache.org.
Split version where N tokens can be specified.

Updated 'split' function in stout/strings so we can choose how many
tokens we want to retreive.

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


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

Branch: refs/heads/master
Commit: 7124e5db9dc8e7578dfcb31d39a13e90c559679c
Parents: 58cbed1
Author: Isabel Jimenez <co...@isabeljimenez.com>
Authored: Sun Jun 29 09:50:06 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 29 09:50:07 2014 -0700

----------------------------------------------------------------------
 .../3rdparty/stout/include/stout/strings.hpp    | 26 ++++++-
 .../3rdparty/stout/tests/strings_tests.cpp      | 74 ++++++++++++++++++++
 2 files changed, 97 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7124e5db/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
index 08428b8..43ae7be 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
@@ -21,6 +21,7 @@
 
 #include "foreach.hpp"
 #include "format.hpp"
+#include "option.hpp"
 #include "stringify.hpp"
 
 namespace strings {
@@ -96,6 +97,8 @@ inline std::string replace(
 
 // Tokenizes the string using the delimiters.
 // Empty tokens will not be included in the result.
+// TODO(ijimenez) support maximum number of tokens
+// to be returned
 inline std::vector<std::string> tokenize(
     const std::string& s,
     const std::string& delims)
@@ -124,24 +127,41 @@ inline std::vector<std::string> tokenize(
 
 
 // Splits the string using the provided delimiters.
+// The string is split each time at the first character
+// that matches any of the characters specified in delims.
 // Empty tokens are allowed in the result.
+// Optinally, maximum number of tokens to be returned
+// can be specified.
 inline std::vector<std::string> split(
     const std::string& s,
-    const std::string& delims)
+    const std::string& delims,
+    Option<int> n = None())
 {
   std::vector<std::string> tokens;
   size_t offset = 0;
   size_t next = 0;
+  int _n = 0;
+  bool some = n.isSome();
 
-  while (true) {
+  if (some) {
+     _n =  n.get();
+  }
+
+  while (!some || _n > 0) {
+    if (_n == 1) {
+      tokens.push_back(s.substr(offset));
+      break;
+    }
     next = s.find_first_of(delims, offset);
     if (next == std::string::npos) {
       tokens.push_back(s.substr(offset));
       break;
     }
-
     tokens.push_back(s.substr(offset, next - offset));
     offset = next + 1;
+    if (some) {
+      --_n;
+    }
   }
   return tokens;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/7124e5db/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp
index c83156e..51008e5 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp
@@ -263,6 +263,80 @@ TEST(StringsTest, SplitWithMultipleDelims)
 }
 
 
+TEST(StringsTest, SplitNZero)
+{
+  vector<string> tokens = strings::split("foo,bar,,,", ",", 0);
+  ASSERT_EQ(0u, tokens.size());
+}
+
+
+TEST(StringsTest, SplitNDelimOnlyString)
+{
+  vector<string> tokens = strings::split(",,,", ",", 2);
+  ASSERT_EQ(2u, tokens.size());
+  EXPECT_EQ("",   tokens[0]);
+  EXPECT_EQ(",,", tokens[1]);
+}
+
+
+TEST(StringsTest, SplitN)
+{
+  vector<string> tokens = strings::split("foo,bar,,baz", ",", 2);
+  ASSERT_EQ(2u, tokens.size());
+  EXPECT_EQ("foo",      tokens[0]);
+  EXPECT_EQ("bar,,baz", tokens[1]);
+}
+
+
+TEST(StringsTest, SplitNStringWithDelimsAtStart)
+{
+  vector<string> tokens = strings::split(",,foo,bar,,baz", ",", 5);
+  ASSERT_EQ(5u, tokens.size());
+  EXPECT_EQ("",     tokens[0]);
+  EXPECT_EQ("",     tokens[1]);
+  EXPECT_EQ("foo",  tokens[2]);
+  EXPECT_EQ("bar",  tokens[3]);
+  EXPECT_EQ(",baz", tokens[4]);
+}
+
+
+TEST(StringsTest, SplitNStringWithDelimsAtEnd)
+{
+  vector<string> tokens = strings::split("foo,bar,,baz,,", ",", 4);
+  ASSERT_EQ(4u, tokens.size());
+  EXPECT_EQ("foo",   tokens[0]);
+  EXPECT_EQ("bar",   tokens[1]);
+  EXPECT_EQ("",      tokens[2]);
+  EXPECT_EQ("baz,,", tokens[3]);
+}
+
+
+TEST(StringsTest, SplitNStringWithDelimsAtStartAndEnd)
+{
+  vector<string> tokens = strings::split(",,foo,bar,,", ",", 6);
+  ASSERT_EQ(6u, tokens.size());
+  EXPECT_EQ("",    tokens[0]);
+  EXPECT_EQ("",    tokens[1]);
+  EXPECT_EQ("foo", tokens[2]);
+  EXPECT_EQ("bar", tokens[3]);
+  EXPECT_EQ("",    tokens[4]);
+  EXPECT_EQ("",    tokens[5]);
+}
+
+
+TEST(StringsTest, SplitNWithMultipleDelims)
+{
+  vector<string> tokens = strings::split("foo.bar,.,.baz.", ",.", 6);
+  ASSERT_EQ(6u, tokens.size());
+  EXPECT_EQ("foo",  tokens[0]);
+  EXPECT_EQ("bar",  tokens[1]);
+  EXPECT_EQ("",     tokens[2]);
+  EXPECT_EQ("",     tokens[3]);
+  EXPECT_EQ("",     tokens[4]);
+  EXPECT_EQ("baz.", tokens[5]);
+}
+
+
 TEST(StringsTest, Pairs)
 {
   map<string, vector<string> > pairs = strings::pairs("one=1,two=2", ",", "=");


[2/4] git commit: Minor style cleanups and some additional comments.

Posted by be...@apache.org.
Minor style cleanups and some additional comments.


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

Branch: refs/heads/master
Commit: 58cbed14cdbfa4293f4679b2fe5b1613eb193549
Parents: 2cb3761
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Jun 29 09:48:58 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 29 09:48:58 2014 -0700

----------------------------------------------------------------------
 include/mesos/mesos.proto       |  8 +++++++-
 src/credentials/credentials.hpp |  4 ++--
 src/master/flags.hpp            |  2 +-
 src/master/master.cpp           | 15 ++++++---------
 src/master/master.hpp           |  2 --
 src/tests/credentials_tests.cpp |  2 --
 src/tests/mesos.cpp             |  2 +-
 7 files changed, 17 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index ddbf55c..6d4fd14 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -676,11 +676,17 @@ message Credential {
 
 
 /**
- * Credentials used for authentication
+ * Credentials used for authentication.
  *
  */
 message Credentials {
+  // Collection of credentials used to authenticate "registration" of
+  // either frameworks or slaves.
   repeated Credential registration = 1;
+
+  // Collection of credentails used to authenticate HTTP endpoints
+  // (where the common "username" and "password" are captured as
+  // 'principal' and 'secret' respectively).
   repeated Credential http = 2;
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/src/credentials/credentials.hpp
----------------------------------------------------------------------
diff --git a/src/credentials/credentials.hpp b/src/credentials/credentials.hpp
index cb26da9..c446aec 100644
--- a/src/credentials/credentials.hpp
+++ b/src/credentials/credentials.hpp
@@ -71,7 +71,7 @@ inline Result<Credentials> read(const std::string& path)
     }
 
     // Add the credential.
-    Credential *credential = credentials.add_registration();
+    Credential* credential = credentials.add_registration();
     credential->set_principal(pairs[0]);
     credential->set_secret(pairs[1]);
   }
@@ -101,7 +101,7 @@ inline Result<Credential> readCredential(const std::string& path)
                  << "credential file is NOT accessible by others.";
   }
 
-  // TODO(ijimenez) deprecate text support only JSON like acls
+  // TODO(ijimenez): Deprecate text support for only JSON ACLs.
   Try<JSON::Object> json = JSON::parse<JSON::Object>(read.get());
   if (!json.isError()) {
     Try<Credential> credential = ::protobuf::parse<Credential>(json.get());

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/src/master/flags.hpp
----------------------------------------------------------------------
diff --git a/src/master/flags.hpp b/src/master/flags.hpp
index 12c1b33..32704ce 100644
--- a/src/master/flags.hpp
+++ b/src/master/flags.hpp
@@ -232,7 +232,7 @@ public:
         "                      ]\n"
         "}\n"
         "Text file Example:\n"
-        "username:secret\n"
+        "username secret\n"
         );
 
     add(&Flags::acls,

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index a549986..474014b 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -337,19 +337,16 @@ void Master::initialize()
     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()) {
+    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)";
     }
 
-    // Give Authenticator access to credentials.
-    sasl::secrets::load(_credentials.get());
-
-    // Store credentials in master
-    this->credentials = _credentials.get();
+    // Load "registration" credentials into SASL based Authenticator.
+    sasl::secrets::load(credentials.get());
 
   } else if (flags.authenticate_frameworks || flags.authenticate_slaves) {
     EXIT(1) << "Authentication requires a credentials file"

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 830b538..5fef354 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -370,8 +370,6 @@ protected:
   OfferID newOfferId();
   SlaveID newSlaveId();
 
-  Option<Credentials> credentials;
-
 private:
   // Inner class used to namespace HTTP route handlers (see
   // master/http.cpp for implementations).

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/src/tests/credentials_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/credentials_tests.cpp b/src/tests/credentials_tests.cpp
index 418e02c..c2e2ee7 100644
--- a/src/tests/credentials_tests.cpp
+++ b/src/tests/credentials_tests.cpp
@@ -43,8 +43,6 @@ using mesos::internal::slave::Slave;
 using process::PID;
 
 using testing::_;
-using testing::Eq;
-using testing::Return;
 
 class CredentialsTest : public MesosTest {};
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/58cbed14/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 131d18b..0d7f335 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -93,7 +93,7 @@ master::Flags MesosTest::CreateMasterFlags()
 
   // JSON default format for credentials
   Credentials credentials;
-  Credential *credential = credentials.add_registration();
+  Credential* credential = credentials.add_registration();
   credential->set_principal(DEFAULT_CREDENTIAL.principal());
   credential->set_secret(DEFAULT_CREDENTIAL.secret());
   credential = credentials.add_http();


[4/4] git commit: Simplified strings::split when 'n' != None.

Posted by be...@apache.org.
Simplified strings::split when 'n' != None.


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

Branch: refs/heads/master
Commit: d4d253df8bd233c6333bb11d3fb81c57f494420b
Parents: 7124e5d
Author: Benjamin Hindman <be...@gmail.com>
Authored: Sun Jun 29 12:42:37 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 29 12:42:37 2014 -0700

----------------------------------------------------------------------
 .../3rdparty/stout/include/stout/strings.hpp    | 28 ++++++++------------
 1 file changed, 11 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d4d253df/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
index 43ae7be..c7c1ed4 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp
@@ -97,8 +97,8 @@ inline std::string replace(
 
 // Tokenizes the string using the delimiters.
 // Empty tokens will not be included in the result.
-// TODO(ijimenez) support maximum number of tokens
-// to be returned
+// TODO(ijimenez): Support maximum number of tokens
+// to be returned.
 inline std::vector<std::string> tokenize(
     const std::string& s,
     const std::string& delims)
@@ -130,37 +130,31 @@ inline std::vector<std::string> tokenize(
 // The string is split each time at the first character
 // that matches any of the characters specified in delims.
 // Empty tokens are allowed in the result.
-// Optinally, maximum number of tokens to be returned
+// Optionally, maximum number of tokens to be returned
 // can be specified.
 inline std::vector<std::string> split(
     const std::string& s,
     const std::string& delims,
-    Option<int> n = None())
+    const Option<unsigned int>& n = None())
 {
   std::vector<std::string> tokens;
   size_t offset = 0;
   size_t next = 0;
-  int _n = 0;
-  bool some = n.isSome();
 
-  if (some) {
-     _n =  n.get();
-  }
-
-  while (!some || _n > 0) {
-    if (_n == 1) {
-      tokens.push_back(s.substr(offset));
-      break;
-    }
+  while (n.isNone() || n.get() > 0) {
     next = s.find_first_of(delims, offset);
     if (next == std::string::npos) {
       tokens.push_back(s.substr(offset));
       break;
     }
+
     tokens.push_back(s.substr(offset, next - offset));
     offset = next + 1;
-    if (some) {
-      --_n;
+
+    // Finish splitting if we've found enough tokens.
+    if (n.isSome() && tokens.size() == n.get() - 1) {
+      tokens.push_back(s.substr(offset));
+      break;
     }
   }
   return tokens;