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/03/28 11:49:44 UTC
[1/3] mesos git commit: Added authentication to the '/files'
endpoints.
Repository: mesos
Updated Branches:
refs/heads/master f978d3ce9 -> 3fc9e4951
Added authentication to the '/files' endpoints.
The `Files` constructor was modified to accept an
`Option<string>& authenticationRealm`, allowing the master or agent to
set the authentication realm during initialization. The master/agent
test helpers were modified to pass the appropriate authentication realm
to `Files`, and the GC tests were modified to authenticate when hitting
the 'files/browse' endpoint.
Review: https://reviews.apache.org/r/45248/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/02ec71ec
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/02ec71ec
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/02ec71ec
Branch: refs/heads/master
Commit: 02ec71ec500d7eaacd9bc85844de8bd34bd5175f
Parents: f978d3c
Author: Greg Mann <gr...@mesosphere.io>
Authored: Mon Mar 28 00:39:23 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Mon Mar 28 01:00:15 2016 -0700
----------------------------------------------------------------------
src/files/files.cpp | 140 +++++++++++++++++++++++++++++++-------------
src/files/files.hpp | 2 +-
src/master/main.cpp | 2 +-
src/slave/main.cpp | 2 +-
src/tests/cluster.hpp | 5 +-
src/tests/gc_tests.cpp | 18 +++++-
src/tests/mesos.cpp | 3 +-
src/tests/mesos.hpp | 1 +
8 files changed, 124 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/files/files.cpp
----------------------------------------------------------------------
diff --git a/src/files/files.cpp b/src/files/files.cpp
index 3902167..a8b2751 100644
--- a/src/files/files.cpp
+++ b/src/files/files.cpp
@@ -80,7 +80,7 @@ namespace internal {
class FilesProcess : public Process<FilesProcess>
{
public:
- FilesProcess();
+ FilesProcess(const Option<string>& _authenticationRealm);
// Files implementation.
Future<Nothing> attach(const string& path, const string& name);
@@ -104,19 +104,27 @@ private:
// path: The directory to browse. Required.
// The response will contain a list of JSON files and directories contained
// in the path (see files::jsonFileInfo for the format).
- Future<Response> browse(const Request& request);
+ Future<Response> browse(
+ const Request& request,
+ const Option<string>& principal);
// Reads data from a file at a given offset and for a given length.
// See the jquery pailer for the expected behavior.
- Future<Response> read(const Request& request);
+ Future<Response> read(
+ const Request& request,
+ const Option<string>& principal);
// Returns the raw file contents for a given path.
// Requests have the following parameters:
// path: The directory to browse. Required.
- Future<Response> download(const Request& request);
+ Future<Response> download(
+ const Request& request,
+ const Option<string>& principal);
// Returns the internal virtual path mapping.
- Future<Response> debug(const Request& request);
+ Future<Response> debug(
+ const Request& request,
+ const Option<string>& principal);
const static string BROWSE_HELP;
const static string READ_HELP;
@@ -124,43 +132,87 @@ private:
const static string DEBUG_HELP;
hashmap<string, string> paths;
+
+ // The authentication realm, if any, into which this process'
+ // endpoints will be installed.
+ Option<string> authenticationRealm;
};
-FilesProcess::FilesProcess()
+FilesProcess::FilesProcess(const Option<string>& _authenticationRealm)
: ProcessBase("files")
-{}
+{
+ authenticationRealm = _authenticationRealm;
+}
void FilesProcess::initialize()
{
- // TODO(ijimenez): Remove these endpoints at the end of the
- // deprecation cycle on 0.26.
- route("/browse.json",
- FilesProcess::BROWSE_HELP,
- &FilesProcess::browse);
- route("/read.json",
- FilesProcess::READ_HELP,
- &FilesProcess::read);
- route("/download.json",
- FilesProcess::DOWNLOAD_HELP,
- &FilesProcess::download);
- route("/debug.json",
- FilesProcess::DEBUG_HELP,
- &FilesProcess::debug);
-
- route("/browse",
- FilesProcess::BROWSE_HELP,
- &FilesProcess::browse);
- route("/read",
- FilesProcess::READ_HELP,
- &FilesProcess::read);
- route("/download",
- FilesProcess::DOWNLOAD_HELP,
- &FilesProcess::download);
- route("/debug",
- FilesProcess::DEBUG_HELP,
- &FilesProcess::debug);
+ if (authenticationRealm.isSome()) {
+ // TODO(ijimenez): Remove these endpoints at the end of the
+ // deprecation cycle on 0.26.
+ route("/browse.json",
+ authenticationRealm.get(),
+ FilesProcess::BROWSE_HELP,
+ &FilesProcess::browse);
+ route("/read.json",
+ authenticationRealm.get(),
+ FilesProcess::READ_HELP,
+ &FilesProcess::read);
+ route("/download.json",
+ authenticationRealm.get(),
+ FilesProcess::DOWNLOAD_HELP,
+ &FilesProcess::download);
+ route("/debug.json",
+ authenticationRealm.get(),
+ FilesProcess::DEBUG_HELP,
+ &FilesProcess::debug);
+
+ route("/browse",
+ authenticationRealm.get(),
+ FilesProcess::BROWSE_HELP,
+ &FilesProcess::browse);
+ route("/read",
+ authenticationRealm.get(),
+ FilesProcess::READ_HELP,
+ &FilesProcess::read);
+ route("/download",
+ authenticationRealm.get(),
+ FilesProcess::DOWNLOAD_HELP,
+ &FilesProcess::download);
+ route("/debug",
+ authenticationRealm.get(),
+ FilesProcess::DEBUG_HELP,
+ &FilesProcess::debug);
+ } else {
+ // TODO(ijimenez): Remove these endpoints at the end of the
+ // deprecation cycle on 0.26.
+ route("/browse.json",
+ FilesProcess::BROWSE_HELP,
+ lambda::bind(&FilesProcess::browse, this, lambda::_1, None()));
+ route("/read.json",
+ FilesProcess::READ_HELP,
+ lambda::bind(&FilesProcess::read, this, lambda::_1, None()));
+ route("/download.json",
+ FilesProcess::DOWNLOAD_HELP,
+ lambda::bind(&FilesProcess::download, this, lambda::_1, None()));
+ route("/debug.json",
+ FilesProcess::DEBUG_HELP,
+ lambda::bind(&FilesProcess::debug, this, lambda::_1, None()));
+
+ route("/browse",
+ FilesProcess::BROWSE_HELP,
+ lambda::bind(&FilesProcess::browse, this, lambda::_1, None()));
+ route("/read",
+ FilesProcess::READ_HELP,
+ lambda::bind(&FilesProcess::read, this, lambda::_1, None()));
+ route("/download",
+ FilesProcess::DOWNLOAD_HELP,
+ lambda::bind(&FilesProcess::download, this, lambda::_1, None()));
+ route("/debug",
+ FilesProcess::DEBUG_HELP,
+ lambda::bind(&FilesProcess::debug, this, lambda::_1, None()));
+ }
}
@@ -212,7 +264,9 @@ const string FilesProcess::BROWSE_HELP = HELP(
"> path=VALUE The path of directory to browse."));
-Future<Response> FilesProcess::browse(const Request& request)
+Future<Response> FilesProcess::browse(
+ const Request& request,
+ const Option<string>& /* principal */)
{
Option<string> path = request.url.query.get("path");
@@ -285,7 +339,9 @@ const string FilesProcess::READ_HELP = HELP(
"> length=VALUE Length of file to read."));
-Future<Response> FilesProcess::read(const Request& request)
+Future<Response> FilesProcess::read(
+ const Request& request,
+ const Option<string>& /* principal */)
{
Option<string> path = request.url.query.get("path");
@@ -425,7 +481,9 @@ const string FilesProcess::DOWNLOAD_HELP = HELP(
"> path=VALUE The path of directory to browse."));
-Future<Response> FilesProcess::download(const Request& request)
+Future<Response> FilesProcess::download(
+ const Request& request,
+ const Option<string>& /* principal */)
{
Option<string> path = request.url.query.get("path");
@@ -474,7 +532,9 @@ const string FilesProcess::DEBUG_HELP = HELP(
"JSON object."));
-Future<Response> FilesProcess::debug(const Request& request)
+Future<Response> FilesProcess::debug(
+ const Request& request,
+ const Option<string>& /* principal */)
{
JSON::Object object;
foreachpair (const string& name, const string& path, paths) {
@@ -551,9 +611,9 @@ Result<string> FilesProcess::resolve(const string& path)
}
-Files::Files()
+Files::Files(const Option<string>& authenticationRealm)
{
- process = new FilesProcess();
+ process = new FilesProcess(authenticationRealm);
spawn(process);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/files/files.hpp
----------------------------------------------------------------------
diff --git a/src/files/files.hpp b/src/files/files.hpp
index 7b65a0a..90acb34 100644
--- a/src/files/files.hpp
+++ b/src/files/files.hpp
@@ -59,7 +59,7 @@ class FilesProcess;
class Files
{
public:
- Files();
+ Files(const Option<std::string>& authenticationRealm = None());
~Files();
// Returns the result of trying to attach the specified path
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index 61210d9..58561cf 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -329,7 +329,7 @@ int main(int argc, char** argv)
Registrar* registrar = new Registrar(flags, state);
Repairer* repairer = new Repairer();
- Files files;
+ Files files(DEFAULT_HTTP_AUTHENTICATION_REALM);
MasterContender* contender;
MasterDetector* detector;
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/slave/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index 33a1af8..f5f2142 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -302,7 +302,7 @@ int main(int argc, char** argv)
// terminating.
}
- Files files;
+ Files files(DEFAULT_HTTP_AUTHENTICATION_REALM);
GarbageCollector gc;
StatusUpdateManager statusUpdateManager(flags);
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/tests/cluster.hpp
----------------------------------------------------------------------
diff --git a/src/tests/cluster.hpp b/src/tests/cluster.hpp
index 06424dd..91d3998 100644
--- a/src/tests/cluster.hpp
+++ b/src/tests/cluster.hpp
@@ -49,6 +49,7 @@
#include "master/registrar.hpp"
#include "master/repairer.hpp"
+#include "slave/constants.hpp"
#include "slave/flags.hpp"
#include "slave/gc.hpp"
#include "slave/slave.hpp"
@@ -99,7 +100,7 @@ public:
process::PID<master::Master> pid;
private:
- Master() = default;
+ Master() : files(master::DEFAULT_HTTP_AUTHENTICATION_REALM) {};
// Not copyable, not assignable.
Master(const Master&) = delete;
@@ -165,7 +166,7 @@ public:
process::PID<slave::Slave> pid;
private:
- Slave() = default;
+ Slave() : files(slave::DEFAULT_HTTP_AUTHENTICATION_REALM) {};
// Not copyable, not assignable.
Slave(const Slave&) = delete;
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/tests/gc_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/gc_tests.cpp b/src/tests/gc_tests.cpp
index 42059b2..89a7f73 100644
--- a/src/tests/gc_tests.cpp
+++ b/src/tests/gc_tests.cpp
@@ -471,7 +471,11 @@ TEST_F(GarbageCollectorIntegrationTest, ExitedFramework)
process::UPID filesUpid("files", process::address());
AWAIT_EXPECT_RESPONSE_STATUS_EQ(
process::http::NotFound().status,
- process::http::get(filesUpid, "browse", "path=" + frameworkDir));
+ process::http::get(
+ filesUpid,
+ "browse",
+ "path=" + frameworkDir,
+ createBasicAuthHeaders(DEFAULT_CREDENTIAL)));
Clock::resume();
}
@@ -574,7 +578,11 @@ TEST_F(GarbageCollectorIntegrationTest, ExitedExecutor)
process::UPID files("files", process::address());
AWAIT_EXPECT_RESPONSE_STATUS_EQ(
process::http::NotFound().status,
- process::http::get(files, "browse", "path=" + executorDir));
+ process::http::get(
+ files,
+ "browse",
+ "path=" + executorDir,
+ createBasicAuthHeaders(DEFAULT_CREDENTIAL)));
Clock::resume();
@@ -691,7 +699,11 @@ TEST_F(GarbageCollectorIntegrationTest, DiskUsage)
process::UPID files("files", process::address());
AWAIT_EXPECT_RESPONSE_STATUS_EQ(
process::http::NotFound().status,
- process::http::get(files, "browse", "path=" + executorDir));
+ process::http::get(
+ files,
+ "browse",
+ "path=" + executorDir,
+ createBasicAuthHeaders(DEFAULT_CREDENTIAL)));
Clock::resume();
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 543b2a7..77d49cc 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -462,7 +462,8 @@ MockSlave::MockSlave(
&gc,
statusUpdateManager = new slave::StatusUpdateManager(flags),
&resourceEstimator,
- _qosController.isSome() ? _qosController.get() : &qosController)
+ _qosController.isSome() ? _qosController.get() : &qosController),
+ files(slave::DEFAULT_HTTP_AUTHENTICATION_REALM)
{
// Set up default behaviors, calling the original methods.
EXPECT_CALL(*this, runTask(_, _, _, _, _))
http://git-wip-us.apache.org/repos/asf/mesos/blob/02ec71ec/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index dc0e4dd..f2a29d0 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -72,6 +72,7 @@
#include "master/detector.hpp"
#include "master/master.hpp"
+#include "slave/constants.hpp"
#include "slave/slave.hpp"
#include "slave/containerizer/containerizer.hpp"
[3/3] mesos git commit: Added a test for the '/files/debug' endpoint.
Posted by me...@apache.org.
Added a test for the '/files/debug' endpoint.
This endpoint previously had no test coverage, so a test was added
that hits the endpoint and checks for the expected output.
Review: https://reviews.apache.org/r/45290/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3fc9e495
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3fc9e495
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3fc9e495
Branch: refs/heads/master
Commit: 3fc9e4951437c78b498957884f770b9f1eec2a78
Parents: e87201f
Author: Greg Mann <gr...@mesosphere.io>
Authored: Mon Mar 28 01:33:50 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Mon Mar 28 02:46:25 2016 -0700
----------------------------------------------------------------------
src/tests/files_tests.cpp | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/3fc9e495/src/tests/files_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/files_tests.cpp b/src/tests/files_tests.cpp
index bf050e7..2f0f84c 100644
--- a/src/tests/files_tests.cpp
+++ b/src/tests/files_tests.cpp
@@ -343,6 +343,31 @@ TEST_F(FilesTest, DownloadTest)
}
+// Tests that the '/files/debug' endpoint works as expected.
+TEST_F(FilesTest, DebugTest)
+{
+ Files files;
+ process::UPID upid("files", process::address());
+
+ ASSERT_SOME(os::mkdir("real-path-1"));
+ ASSERT_SOME(os::mkdir("real-path-2"));
+
+ AWAIT_EXPECT_READY(files.attach("real-path-1", "virtual-path-1"));
+ AWAIT_EXPECT_READY(files.attach("real-path-2", "virtual-path-2"));
+
+ // Construct the expected JSON output.
+ const string cwd = os::getcwd();
+ JSON::Object expected;
+ expected.values["virtual-path-1"] = path::join(cwd, "real-path-1");
+ expected.values["virtual-path-2"] = path::join(cwd, "real-path-2");
+
+ Future<Response> response = process::http::get(upid, "debug");
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+ AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
+}
+
+
// Tests that requests to the '/files/*' endpoints receive an `Unauthorized`
// response when HTTP authentication is enabled and an invalid credential is
// provided.
[2/3] mesos git commit: Added a new '/files' endpoints test using
authentication.
Posted by me...@apache.org.
Added a new '/files' endpoints test using authentication.
A new test was added (FilesTest.AuthenticationTest) to probe the
`/files/*` endpoints' behavior when HTTP authentication is enabled.
Review: https://reviews.apache.org/r/45249/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e87201fd
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e87201fd
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e87201fd
Branch: refs/heads/master
Commit: e87201fd9551a5ad086a6e6fc0a30f4ee22f99f4
Parents: 02ec71e
Author: Greg Mann <gr...@mesosphere.io>
Authored: Mon Mar 28 01:04:15 2016 -0700
Committer: Adam B <ad...@mesosphere.io>
Committed: Mon Mar 28 02:46:25 2016 -0700
----------------------------------------------------------------------
src/tests/files_tests.cpp | 96 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 95 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e87201fd/src/tests/files_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/files_tests.cpp b/src/tests/files_tests.cpp
index b389495..bf050e7 100644
--- a/src/tests/files_tests.cpp
+++ b/src/tests/files_tests.cpp
@@ -18,6 +18,8 @@
#include <gmock/gmock.h>
+#include <mesos/authentication/http/basic_authenticator_factory.hpp>
+
#include <process/future.hpp>
#include <process/gtest.hpp>
#include <process/http.hpp>
@@ -33,21 +35,64 @@
#include "files/files.hpp"
+#include "tests/mesos.hpp"
+
+namespace authentication = process::http::authentication;
+
using process::Future;
+using process::Owned;
using process::http::BadRequest;
using process::http::NotFound;
using process::http::OK;
using process::http::Response;
+using process::http::Unauthorized;
using std::string;
+using mesos::http::authentication::BasicAuthenticatorFactory;
+
namespace mesos {
namespace internal {
namespace tests {
+class FilesTest : public TemporaryDirectoryTest
+{
+protected:
+ void setBasicHttpAuthenticator(
+ const string& realm,
+ const Credentials& credentials)
+ {
+ Try<authentication::Authenticator*> authenticator =
+ BasicAuthenticatorFactory::create(realm, credentials);
+
+ ASSERT_SOME(authenticator);
+
+ // Add this realm to the set of realms which will be unset during teardown.
+ realms.insert(realm);
-class FilesTest : public TemporaryDirectoryTest {};
+ // Pass ownership of the authenticator to libprocess.
+ AWAIT_READY(authentication::setAuthenticator(
+ realm,
+ Owned<authentication::Authenticator>(authenticator.get())));
+ }
+
+ virtual void TearDown()
+ {
+ foreach (const string& realm, realms) {
+ // We need to wait in order to ensure that the operation completes before
+ // we leave `TearDown`. Otherwise, we may leak a mock object.
+ AWAIT_READY(authentication::unsetAuthenticator(realm));
+ }
+
+ realms.clear();
+
+ TemporaryDirectoryTest::TearDown();
+ }
+
+private:
+ hashset<string> realms;
+};
TEST_F(FilesTest, AttachTest)
@@ -297,6 +342,55 @@ TEST_F(FilesTest, DownloadTest)
AWAIT_EXPECT_RESPONSE_BODY_EQ(data, response);
}
+
+// Tests that requests to the '/files/*' endpoints receive an `Unauthorized`
+// response when HTTP authentication is enabled and an invalid credential is
+// provided.
+TEST_F(FilesTest, AuthenticationTest)
+{
+ const string AUTHENTICATION_REALM = "realm";
+
+ Credentials credentials;
+ credentials.add_credentials()->CopyFrom(DEFAULT_CREDENTIAL);
+
+ // Create a basic HTTP authenticator with the specified credentials and set it
+ // as the authenticator for `AUTHENTICATION_REALM`.
+ setBasicHttpAuthenticator(AUTHENTICATION_REALM, credentials);
+
+ // The realm is passed to `Files` to enable
+ // HTTP authentication on its endpoints.
+ Files files(AUTHENTICATION_REALM);
+
+ process::UPID upid("files", process::address());
+
+ Credential badCredential;
+ badCredential.set_principal("bad-principal");
+ badCredential.set_secret("bad-secret");
+
+ const string expectedAuthorizationHeader =
+ "Basic realm=\"" + AUTHENTICATION_REALM + "\"";
+
+ Future<Response> response = process::http::get(upid, "browse");
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+ EXPECT_EQ(response.get().headers.at("WWW-Authenticate"),
+ expectedAuthorizationHeader);
+
+ response = process::http::get(upid, "read");
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+ EXPECT_EQ(response.get().headers.at("WWW-Authenticate"),
+ expectedAuthorizationHeader);
+
+ response = process::http::get(upid, "download");
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+ EXPECT_EQ(response.get().headers.at("WWW-Authenticate"),
+ expectedAuthorizationHeader);
+
+ response = process::http::get(upid, "debug");
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response);
+ EXPECT_EQ(response.get().headers.at("WWW-Authenticate"),
+ expectedAuthorizationHeader);
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {