Repository: mesos Updated Branches: refs/heads/master 71e5099c5 -> 6224c8c7b
Added authorization to agent's '/containers' endpoint. Used `GET_ENDPOINT_WITH_PATH` coarse-grained authz on agent's '/containers' endpoint to enable authorization on this endpoint. Updated docs and tests as well. Review: https://reviews.apache.org/r/47530/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6224c8c7 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6224c8c7 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6224c8c7 Branch: refs/heads/master Commit: 6224c8c7b8c0877701ed1d6c9d2de952d560ffdb Parents: 71e5099 Author: Abhishek Dasgupta <a10gu...@linux.vnet.ibm.com> Authored: Thu May 19 09:53:14 2016 +0200 Committer: Alexander Rukletsov <al...@apache.org> Committed: Thu May 19 11:00:19 2016 +0200 ---------------------------------------------------------------------- docs/endpoints/slave/containers.md | 6 +++- src/slave/http.cpp | 41 ++++++++++++++++++++++++++-- src/slave/slave.hpp | 7 ++++- src/tests/slave_authorization_tests.cpp | 5 +++- 4 files changed, 54 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/docs/endpoints/slave/containers.md ---------------------------------------------------------------------- diff --git a/docs/endpoints/slave/containers.md b/docs/endpoints/slave/containers.md index 959f40b..f8bcb38 100644 --- a/docs/endpoints/slave/containers.md +++ b/docs/endpoints/slave/containers.md @@ -50,4 +50,8 @@ Example (**Note**: this is not exhaustive): ### AUTHENTICATION ### This endpoint requires authentication iff HTTP authentication is -enabled. \ No newline at end of file +enabled. + +### AUTHORIZATION ### +The request principal should be authorized to query this endpoint. +See the authorization documentation for details. \ No newline at end of file http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index fb48ec6..dd1f509 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -744,13 +744,50 @@ string Slave::Http::CONTAINERS_HELP() " }", "}]", "```"), - AUTHENTICATION(true)); + AUTHENTICATION(true), + AUTHORIZATION( + "The request principal should be authorized to query this endpoint.", + "See the authorization documentation for details.")); } Future<Response> Slave::Http::containers( const Request& request, - const Option<string>& /* principal */) const + const Option<string>& principal) const +{ + // TODO(a10gupta): Remove check for enabled + // authorization as part of MESOS-5346. + if (request.method != "GET" && slave->authorizer.isSome()) { + return MethodNotAllowed({"GET"}, request.method); + } + + Try<string> endpoint = extractEndpoint(request.url); + if (endpoint.isError()) { + return Failure("Failed to extract endpoint: " + endpoint.error()); + } + + const PID<Slave> pid = slave->self(); + + // NOTE: We should not leak the slave instance because the its + // lifetime is not guaranteed. See MESOS-5293 for details. + const Slave* localSlave = slave; + + return authorizeEndpoint(principal, endpoint.get(), request.method) + .then(defer( + pid, + [pid, localSlave, request](bool authorized) -> Future<Response> { + if (!authorized) { + return Forbidden(); + } + + return _containers(request, localSlave); + })); +} + + +Future<Response> Slave::Http::_containers( + const Request& request, + const Slave* slave) { Owned<list<JSON::Object>> metadata(new list<JSON::Object>()); list<Future<ContainerStatus>> statusFutures; http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/src/slave/slave.hpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp index 209f071..0de6a57 100644 --- a/src/slave/slave.hpp +++ b/src/slave/slave.hpp @@ -458,7 +458,7 @@ private: // /slave/containers process::Future<process::http::Response> containers( const process::http::Request& request, - const Option<std::string>& /* principal */) const; + const Option<std::string>& principal) const; static std::string EXECUTOR_HELP(); static std::string FLAGS_HELP(); @@ -479,6 +479,11 @@ private: const ResourceUsage& usage, const process::http::Request& request); + // Continuation for `/containers` endpoint + static process::Future<process::http::Response> _containers( + const process::http::Request& request, + const Slave* slave); + // Helper routines for endpoint authorization. Try<std::string> extractEndpoint(const process::http::URL& url) const; http://git-wip-us.apache.org/repos/asf/mesos/blob/6224c8c7/src/tests/slave_authorization_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/slave_authorization_tests.cpp b/src/tests/slave_authorization_tests.cpp index 843cf1c..98fde49 100644 --- a/src/tests/slave_authorization_tests.cpp +++ b/src/tests/slave_authorization_tests.cpp @@ -178,7 +178,10 @@ INSTANTIATE_TEST_CASE_P( Endpoint, SlaveEndpointTest, ::testing::Values( - "monitor/statistics", "monitor/statistics.json", "flags")); + "monitor/statistics", + "monitor/statistics.json", + "flags", + "containers")); // Tests that an agent endpoint handler forms