You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2015/06/24 17:47:10 UTC
[1/2] mesos git commit: Fixed some nits in libprocess firewall.
Repository: mesos
Updated Branches:
refs/heads/master d86341b21 -> b6187a3b2
Fixed some nits in libprocess firewall.
Review: https://reviews.apache.org/r/35353
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/70dfe873
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/70dfe873
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/70dfe873
Branch: refs/heads/master
Commit: 70dfe873777520df17ae196e5dd2874c47988308
Parents: d86341b
Author: Alexander Rojas <al...@mesosphere.io>
Authored: Wed Jun 24 17:35:49 2015 +0200
Committer: Till Toenshoff <to...@me.com>
Committed: Wed Jun 24 17:35:50 2015 +0200
----------------------------------------------------------------------
.../libprocess/include/process/firewall.hpp | 37 +++++++++++---------
3rdparty/libprocess/include/process/process.hpp | 20 ++++++-----
3rdparty/libprocess/src/process.cpp | 27 ++++++++------
3rdparty/libprocess/src/tests/process_tests.cpp | 36 ++++++++++---------
4 files changed, 69 insertions(+), 51 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/70dfe873/3rdparty/libprocess/include/process/firewall.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/firewall.hpp b/3rdparty/libprocess/include/process/firewall.hpp
index 16ed852..f71d654 100644
--- a/3rdparty/libprocess/include/process/firewall.hpp
+++ b/3rdparty/libprocess/include/process/firewall.hpp
@@ -20,20 +20,23 @@
#include <process/http.hpp>
#include <process/socket.hpp>
+#include <stout/error.hpp>
#include <stout/hashset.hpp>
+#include <stout/option.hpp>
namespace process {
namespace firewall {
/**
- * A 'FirewallRule' is an interface which allows control over which
- * incoming HTTP connections are allowed. Concrete classes based on
- * this interface must implement the 'apply' method; this method
- * receives as parameters the socket where the connection is being
- * initiated, as well as the request itself.
+ * A 'FirewallRule' describes an interface which provides control
+ * over incoming HTTP requests while also taking the underlying
+ * connection into account.
+ *
+ * Concrete classes based on this interface must implement the
+ * 'apply' method.
*
* Rules can be installed using the free function
- * 'process::firewall::install()' defined in 'process.hpp'
+ * 'process::firewall::install()' defined in 'process.hpp'.
*/
class FirewallRule
{
@@ -42,24 +45,24 @@ public:
virtual ~FirewallRule() {}
/**
- * Method used to do inspection of incoming HTTP requests. It allows
- * implementations to verify conditions on the requests and notify
- * the caller if the rule has been broken.
+ * Verify rule by applying it to an HTTP request and its underlying
+ * socket connection.
*
- * @param socket Socket used to attempt the HTTP connection.
+ * @param socket Socket used to deliver the HTTP request.
* @param request HTTP request made by the client to libprocess.
- * @return If the condition verification fails, i.e. the condition
- * has been broken, the returned Try contains an error.
+ * @return If the rule verification fails, i.e. the rule didn't
+ * match, the returned error is set with an explanation for the
+ * failure. Otherwise None is returned.
*/
- virtual Try<Nothing> apply(
+ virtual Option<Error> apply(
const network::Socket& socket,
const http::Request& request) = 0;
};
/**
- * Simple firewall rule to reject any connection requesting a path
- * in the provided list of disabled endpoints.
+ * Simple firewall rule to forbid any HTTP request to a path
+ * in the provided list of endpoints.
*
* Matches are required to be exact, no substrings nor wildcards are
* considered for a match.
@@ -72,7 +75,7 @@ public:
virtual ~DisabledEndpointsFirewallRule() {}
- virtual Try<Nothing> apply(
+ virtual Option<Error> apply(
const network::Socket&,
const http::Request& request)
{
@@ -80,7 +83,7 @@ public:
return Error("'" + request.path + "' is disabled");
}
- return Nothing();
+ return None();
}
private:
http://git-wip-us.apache.org/repos/asf/mesos/blob/70dfe873/3rdparty/libprocess/include/process/process.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/process.hpp b/3rdparty/libprocess/include/process/process.hpp
index 6a0b21d..59b50af 100644
--- a/3rdparty/libprocess/include/process/process.hpp
+++ b/3rdparty/libprocess/include/process/process.hpp
@@ -5,6 +5,7 @@
#include <map>
#include <queue>
+#include <vector>
#include <process/address.hpp>
#include <process/clock.hpp>
@@ -14,6 +15,7 @@
#include <process/http.hpp>
#include <process/message.hpp>
#include <process/mime.hpp>
+#include <process/owned.hpp>
#include <process/pid.hpp>
#include <stout/duration.hpp>
@@ -25,16 +27,18 @@
namespace process {
namespace firewall {
+
/**
- * Installs the list of firewall rules to be used to allow or reject
- * incoming connections. Each incoming connection will be tested
- * against each rule in the order in which they appear in the vector
- * 'rules'. As soon as any of the tests (calling the apply method)
- * fails, the connection is rejected. If no test fails, the connection
- * is allowed.
+ * Install a list of firewall rules which are used to forbid incoming
+ * HTTP requests. The rules will be applied in the provided order to
+ * each incoming request. If any rule forbids the request, no more
+ * rules are applied and a "403 Forbidden" response will be returned
+ * containing the reason from the rule. Note that if a request is
+ * forbidden, the request's handler is not notified.
+ * @see process#firewall#FirewallRule
*
- * @param rules List of rules which will be applied to incoming
- * connections.
+ * @param rules List of rules which will be applied to all incoming
+ * HTTP requests.
*/
void install(std::vector<Owned<FirewallRule>>&& rules);
http://git-wip-us.apache.org/repos/asf/mesos/blob/70dfe873/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index a67a3af..4f0b281 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -58,6 +58,7 @@
#include <process/io.hpp>
#include <process/logging.hpp>
#include <process/mime.hpp>
+#include <process/owned.hpp>
#include <process/process.hpp>
#include <process/profiler.hpp>
#include <process/socket.hpp>
@@ -360,7 +361,7 @@ public:
void terminate(const UPID& pid, bool inject, ProcessBase* sender = NULL);
bool wait(const UPID& pid);
- void installFirewall(std::vector<Owned<FirewallRule>>&& rules);
+ void installFirewall(vector<Owned<FirewallRule>>&& rules);
void enqueue(ProcessBase* process);
ProcessBase* dequeue();
@@ -388,8 +389,8 @@ private:
// Number of running processes, to support Clock::settle operation.
int running;
- // List of rules applied to all incoming HTTP connections.
- std::vector<Owned<FirewallRule>> firewallRules;
+ // List of rules applied to all incoming HTTP requests.
+ vector<Owned<FirewallRule>> firewallRules;
std::recursive_mutex firewall_mutex;
};
@@ -696,7 +697,7 @@ void on_accept(const Future<Socket>& socket)
namespace firewall {
-void install(std::vector<Owned<FirewallRule>>&& rules)
+void install(vector<Owned<FirewallRule>>&& rules)
{
process::initialize();
@@ -2008,12 +2009,15 @@ bool ProcessManager::handle(
}
synchronized (firewall_mutex) {
+ // Don't use a const reference, since it cannot be guaranteed
+ // that the rules don't keep an internal state.
foreach (Owned<FirewallRule>& rule, firewallRules) {
- Try<Nothing> applied = rule->apply(socket, *request);
- if (applied.isError()) {
+ Option<Error> rejection = rule->apply(socket, *request);
+ if (rejection.isSome()) {
VLOG(1) << "Returning '403 Forbidden' for '" << request->path
- << "' (firewall rule forbids connection): "
- << applied.error();
+ << "' (firewall rule forbids request): "
+ << rejection.get().message;
+
// TODO(arojas): Get rid of the duplicated code to return an
// error.
@@ -2023,7 +2027,10 @@ bool ProcessManager::handle(
// Enqueue the response with the HttpProxy so that it respects
// the order of requests to account for HTTP/1.1 pipelining.
dispatch(
- proxy, &HttpProxy::enqueue, Forbidden(applied.error()), *request);
+ proxy,
+ &HttpProxy::enqueue,
+ Forbidden(rejection.get().message),
+ *request);
// Cleanup request.
delete request;
@@ -2478,7 +2485,7 @@ bool ProcessManager::wait(const UPID& pid)
}
-void ProcessManager::installFirewall(std::vector<Owned<FirewallRule>>&& rules)
+void ProcessManager::installFirewall(vector<Owned<FirewallRule>>&& rules)
{
synchronized (firewall_mutex) {
firewallRules = std::move(rules);
http://git-wip-us.apache.org/repos/asf/mesos/blob/70dfe873/3rdparty/libprocess/src/tests/process_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/process_tests.cpp b/3rdparty/libprocess/src/tests/process_tests.cpp
index f4633aa..99065b3 100644
--- a/3rdparty/libprocess/src/tests/process_tests.cpp
+++ b/3rdparty/libprocess/src/tests/process_tests.cpp
@@ -8,6 +8,7 @@
#include <string>
#include <sstream>
#include <tuple>
+#include <vector>
#include <process/async.hpp>
#include <process/collect.hpp>
@@ -22,6 +23,7 @@
#include <process/gmock.hpp>
#include <process/gtest.hpp>
#include <process/network.hpp>
+#include <process/owned.hpp>
#include <process/process.hpp>
#include <process/run.hpp>
#include <process/socket.hpp>
@@ -48,7 +50,9 @@ using process::firewall::FirewallRule;
using process::network::Address;
using process::network::Socket;
+using std::move;
using std::string;
+using std::vector;
using testing::_;
using testing::Assign;
@@ -1917,19 +1921,19 @@ public:
// attempts to connect to those endpoints.
TEST(Process, FirewallDisablePaths)
{
- const string processId = "testprocess";
+ const string id = "testprocess";
+ // TODO(arojas): Add initilization list construction when available.
hashset<string> endpoints;
- endpoints.insert(path::join("", processId, "handler1"));
- endpoints.insert(path::join("", processId, "handler2/nested"));
+ endpoints.insert(path::join("", id, "handler1"));
+ endpoints.insert(path::join("", id, "handler2/nested"));
// Patterns are not supported, so this should do nothing.
- endpoints.insert(path::join("", processId, "handler3/*"));
+ endpoints.insert(path::join("", id, "handler3/*"));
- std::vector<Owned<FirewallRule>> rules;
- rules.emplace_back(new DisabledEndpointsFirewallRule(endpoints));
- process::firewall::install(std::move(rules));
+ process::firewall::install(
+ {Owned<FirewallRule>(new DisabledEndpointsFirewallRule(endpoints))});
- HTTPEndpointProcess process(processId);
+ HTTPEndpointProcess process(id);
PID<HTTPEndpointProcess> pid = spawn(process);
@@ -1990,17 +1994,17 @@ TEST(Process, FirewallDisablePaths)
// An empty vector should allow all paths.
TEST(Process, FirewallUninstall)
{
- const string processId = "testprocess";
+ const string id = "testprocess";
+ // TODO(arojas): Add initilization list construction when available.
hashset<string> endpoints;
- endpoints.insert(path::join("", processId, "handler1"));
- endpoints.insert(path::join("", processId, "handler2"));
+ endpoints.insert(path::join("", id, "handler1"));
+ endpoints.insert(path::join("", id, "handler2"));
- std::vector<Owned<FirewallRule>> rules;
- rules.emplace_back(new DisabledEndpointsFirewallRule(endpoints));
- process::firewall::install(std::move(rules));
+ process::firewall::install(
+ {Owned<FirewallRule>(new DisabledEndpointsFirewallRule(endpoints))});
- HTTPEndpointProcess process(processId);
+ HTTPEndpointProcess process(id);
PID<HTTPEndpointProcess> pid = spawn(process);
@@ -2014,7 +2018,7 @@ TEST(Process, FirewallUninstall)
AWAIT_READY(response);
EXPECT_EQ(http::statuses[403], response.get().status);
- process::firewall::install(std::vector<Owned<FirewallRule>>());
+ process::firewall::install({});
EXPECT_CALL(process, handler1(_))
.WillOnce(Return(http::OK()));
[2/2] mesos git commit: Fixed some nits in libprocess firewall
initialization.
Posted by ti...@apache.org.
Fixed some nits in libprocess firewall initialization.
Review: https://reviews.apache.org/r/35354
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b6187a3b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b6187a3b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b6187a3b
Branch: refs/heads/master
Commit: b6187a3b2b206088c4b90cd0187a54240dbde588
Parents: 70dfe87
Author: Alexander Rojas <al...@mesosphere.io>
Authored: Wed Jun 24 17:36:24 2015 +0200
Committer: Till Toenshoff <to...@me.com>
Committed: Wed Jun 24 17:36:25 2015 +0200
----------------------------------------------------------------------
src/master/main.cpp | 16 +++++++++-------
src/messages/flags.hpp | 2 +-
src/slave/main.cpp | 23 ++++++++++++++++-------
3 files changed, 26 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b6187a3b/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index 1c33e3b..2624b7e 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -21,6 +21,7 @@
#include <memory>
#include <set>
#include <string>
+#include <utility>
#include <vector>
#include <mesos/mesos.hpp>
@@ -95,6 +96,7 @@ using process::firewall::FirewallRule;
using std::cerr;
using std::cout;
using std::endl;
+using std::move;
using std::set;
using std::shared_ptr;
using std::string;
@@ -344,21 +346,21 @@ int main(int argc, char** argv)
}
if (flags.firewall_rules.isSome()) {
- const Firewall rules = flags.firewall_rules.get();
+ vector<Owned<FirewallRule>> rules;
- std::vector<Owned<FirewallRule>> _rules;
+ const Firewall firewall = flags.firewall_rules.get();
- if (rules.has_disabled_endpoints()) {
+ if (firewall.has_disabled_endpoints()) {
hashset<string> paths;
- for (int i = 0; i < rules.disabled_endpoints().paths_size(); ++i) {
- paths.insert(rules.disabled_endpoints().paths(i));
+ foreach (const string& path, firewall.disabled_endpoints().paths()) {
+ paths.insert(path);
}
- _rules.emplace_back(new DisabledEndpointsFirewallRule(paths));
+ rules.emplace_back(new DisabledEndpointsFirewallRule(paths));
}
- process::firewall::install(std::move(_rules));
+ process::firewall::install(move(rules));
}
// Create anonymous modules.
http://git-wip-us.apache.org/repos/asf/mesos/blob/b6187a3b/src/messages/flags.hpp
----------------------------------------------------------------------
diff --git a/src/messages/flags.hpp b/src/messages/flags.hpp
index 41be419..17f8bf3 100644
--- a/src/messages/flags.hpp
+++ b/src/messages/flags.hpp
@@ -20,6 +20,7 @@
#define __MESSAGES_FLAGS_HPP__
#include <string>
+#include <ostream>
#include <stout/error.hpp>
#include <stout/json.hpp>
@@ -43,7 +44,6 @@ inline Try<mesos::internal::Firewall> parse(const std::string& value)
return Error(json.error());
}
- // Convert from JSON to Protobuf.
return protobuf::parse<mesos::internal::Firewall>(json.get());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/b6187a3b/src/slave/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index c379243..8008430 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -18,12 +18,17 @@
#include <stdint.h>
+#include <vector>
+#include <utility>
+
#include <mesos/mesos.hpp>
#include <mesos/module/anonymous.hpp>
#include <mesos/slave/resource_estimator.hpp>
+#include <process/owned.hpp>
+
#include <stout/check.hpp>
#include <stout/flags.hpp>
#include <stout/hashset.hpp>
@@ -60,13 +65,17 @@ using mesos::slave::ResourceEstimator;
using mesos::SlaveInfo;
+using process::Owned;
+
using process::firewall::DisabledEndpointsFirewallRule;
using process::firewall::FirewallRule;
using std::cerr;
using std::cout;
using std::endl;
+using std::move;
using std::string;
+using std::vector;
void version()
@@ -180,21 +189,21 @@ int main(int argc, char** argv)
}
if (flags.firewall_rules.isSome()) {
- const Firewall rules = flags.firewall_rules.get();
+ vector<Owned<FirewallRule>> rules;
- std::vector<Owned<FirewallRule>> _rules;
+ const Firewall firewall = flags.firewall_rules.get();
- if (rules.has_disabled_endpoints()) {
+ if (firewall.has_disabled_endpoints()) {
hashset<string> paths;
- for (int i = 0; i < rules.disabled_endpoints().paths_size(); ++i) {
- paths.insert(rules.disabled_endpoints().paths(i));
+ foreach (const string& path, firewall.disabled_endpoints().paths()) {
+ paths.insert(path);
}
- _rules.emplace_back(new DisabledEndpointsFirewallRule(paths));
+ rules.emplace_back(new DisabledEndpointsFirewallRule(paths));
}
- process::firewall::install(std::move(_rules));
+ process::firewall::install(move(rules));
}
// Create anonymous modules.