You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gr...@apache.org on 2017/08/02 22:45:08 UTC

[1/2] mesos git commit: Enabled filtering of the 'GET_AGENTS' v1 API call.

Repository: mesos
Updated Branches:
  refs/heads/master 00290fffa -> e87569b2a


Enabled filtering of the 'GET_AGENTS' v1 API call.

Enables filtering of the results of calls to the 'GET_AGENTS' v1
API. It filters the contents of different resources entries based
on the 'VIEW_ROLE' permissions of the principal doing the request
based on resource roles, allocation roles and reservations.

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


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

Branch: refs/heads/master
Commit: 2fe2562455d899545f2f6cbace989489867b8ee7
Parents: 00290ff
Author: Alexander Rojas <al...@mesosphere.io>
Authored: Wed Aug 2 13:14:01 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Wed Aug 2 15:35:22 2017 -0700

----------------------------------------------------------------------
 src/common/http.cpp           |  33 +++++
 src/common/http.hpp           |   8 ++
 src/common/protobuf_utils.cpp |  33 +++--
 src/common/protobuf_utils.hpp |   7 +-
 src/master/http.cpp           | 170 ++++++++++++++++---------
 src/master/master.hpp         |   6 +-
 src/tests/api_tests.cpp       | 255 ++++++++++++++++++++++++++++++++++++-
 7 files changed, 435 insertions(+), 77 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index dfd5f33..43d674e 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -976,6 +976,39 @@ bool approveViewRole(
   return approved.get();
 }
 
+
+bool authorizeResource(
+    const Resource& resource,
+    const Option<Owned<AuthorizationAcceptor>>& acceptor)
+{
+  if (acceptor.isNone()) {
+    return true;
+  }
+
+  // Necessary because recovered agents are presented in old format.
+  if (resource.has_role() && resource.role() != "*" &&
+      !acceptor.get()->accept(resource.role())) {
+    return false;
+  }
+
+  if (resource.has_allocation_info() &&
+      !acceptor.get()->accept(resource.allocation_info().role())) {
+    return false;
+  }
+
+  // Reservations follow a path model where each entry is a child of the
+  // previous one. Therefore, to accept the resource the acceptor has to
+  // accept all entries.
+  foreach (Resource::ReservationInfo reservation, resource.reservations()) {
+    if (!acceptor.get()->accept(reservation.role())) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+
 namespace {
 
 Result<process::http::authentication::Authenticator*> createBasicAuthenticator(

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index ba8dda1..0e6b1c5 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -270,6 +270,14 @@ bool approveViewRole(
     const process::Owned<ObjectApprover>& rolesApprover,
     const std::string& role);
 
+// Authorizes resources in either the pre- or the post-reservation-refinement
+// formats.
+// TODO(arojas): Update this helper to only accept the
+// post-reservation-refinement format once MESOS-7851 is resolved.
+bool authorizeResource(
+    const Resource& resource,
+    const Option<process::Owned<AuthorizationAcceptor>>& acceptor);
+
 
 /**
  * Helper function to create HTTP authenticators

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/common/protobuf_utils.cpp
----------------------------------------------------------------------
diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp
index 49d3a22..3ae68e9 100644
--- a/src/common/protobuf_utils.cpp
+++ b/src/common/protobuf_utils.cpp
@@ -44,6 +44,7 @@
 #include <stout/windows.hpp>
 #endif // __WINDOWS__
 
+#include "common/http.hpp"
 #include "common/protobuf_utils.hpp"
 #include "common/resources_utils.hpp"
 
@@ -60,6 +61,7 @@ using google::protobuf::RepeatedPtrField;
 using mesos::slave::ContainerLimitation;
 using mesos::slave::ContainerState;
 
+using process::Owned;
 using process::UPID;
 
 namespace mesos {
@@ -879,7 +881,8 @@ mesos::master::Event createFrameworkRemoved(const FrameworkInfo& frameworkInfo)
 
 
 mesos::master::Response::GetAgents::Agent createAgentResponse(
-    const mesos::internal::master::Slave& slave)
+    const mesos::internal::master::Slave& slave,
+    const Option<Owned<AuthorizationAcceptor>>& rolesAcceptor)
 {
   mesos::master::Response::GetAgents::Agent agent;
 
@@ -897,22 +900,32 @@ mesos::master::Response::GetAgents::Agent createAgentResponse(
         slave.reregisteredTime.get().duration().ns());
   }
 
-  foreach (Resource resource, slave.totalResources) {
-    convertResourceFormat(&resource, ENDPOINT);
+  agent.mutable_agent_info()->clear_resources();
+  foreach (const Resource& resource, slave.info.resources()) {
+    if (authorizeResource(resource, rolesAcceptor)) {
+      agent.mutable_agent_info()->add_resources()->CopyFrom(resource);
+    }
+  }
 
-    agent.add_total_resources()->CopyFrom(resource);
+  foreach (Resource resource, slave.totalResources) {
+    if (authorizeResource(resource, rolesAcceptor)) {
+      convertResourceFormat(&resource, ENDPOINT);
+      agent.add_total_resources()->CopyFrom(resource);
+    }
   }
 
   foreach (Resource resource, Resources::sum(slave.usedResources)) {
-    convertResourceFormat(&resource, ENDPOINT);
-
-    agent.add_allocated_resources()->CopyFrom(resource);
+    if (authorizeResource(resource, rolesAcceptor)) {
+      convertResourceFormat(&resource, ENDPOINT);
+      agent.add_allocated_resources()->CopyFrom(resource);
+    }
   }
 
   foreach (Resource resource, slave.offeredResources) {
-    convertResourceFormat(&resource, ENDPOINT);
-
-    agent.add_offered_resources()->CopyFrom(resource);
+    if (authorizeResource(resource, rolesAcceptor)) {
+      convertResourceFormat(&resource, ENDPOINT);
+      agent.add_offered_resources()->CopyFrom(resource);
+    }
   }
 
   agent.mutable_capabilities()->CopyFrom(

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/common/protobuf_utils.hpp
----------------------------------------------------------------------
diff --git a/src/common/protobuf_utils.hpp b/src/common/protobuf_utils.hpp
index 80d2edd..ff0fd01 100644
--- a/src/common/protobuf_utils.hpp
+++ b/src/common/protobuf_utils.hpp
@@ -48,6 +48,9 @@ struct UPID;
 }
 
 namespace mesos {
+
+class AuthorizationAcceptor;
+
 namespace internal {
 
 namespace master {
@@ -320,7 +323,9 @@ mesos::master::Event createFrameworkRemoved(const FrameworkInfo& frameworkInfo);
 
 // Helper for creating an `Agent` response.
 mesos::master::Response::GetAgents::Agent createAgentResponse(
-    const mesos::internal::master::Slave& slave);
+    const mesos::internal::master::Slave& slave,
+    const Option<process::Owned<AuthorizationAcceptor>>& rolesAcceptor =
+      None());
 
 
 // Helper for creating an `AGENT_ADDED` event from a `Slave`.

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 9df086c..e589829 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -806,38 +806,52 @@ Future<Response> Master::Http::subscribe(
     executorsApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
   }
 
-  return collect(frameworksApprover, tasksApprover, executorsApprover)
-    .then(defer(master->self(),
-      [=](const tuple<Owned<ObjectApprover>,
-                      Owned<ObjectApprover>,
-                      Owned<ObjectApprover>>& approvers)
-        -> Future<Response> {
-      // Get approver from tuple.
-      Owned<ObjectApprover> frameworksApprover;
-      Owned<ObjectApprover> tasksApprover;
-      Owned<ObjectApprover> executorsApprover;
-      tie(frameworksApprover, tasksApprover, executorsApprover) = approvers;
-
-      Pipe pipe;
-      OK ok;
-
-      ok.headers["Content-Type"] = stringify(contentType);
-      ok.type = Response::PIPE;
-      ok.reader = pipe.reader();
-
-      HttpConnection http {pipe.writer(), contentType, UUID::random()};
-      master->subscribe(http);
-
-      mesos::master::Event event;
-      event.set_type(mesos::master::Event::SUBSCRIBED);
-      event.mutable_subscribed()->mutable_get_state()->CopyFrom(
-          _getState(frameworksApprover,
-                    tasksApprover,
-                    executorsApprover));
-
-      http.send<mesos::master::Event, v1::master::Event>(event);
+  Future<Owned<AuthorizationAcceptor>> rolesAcceptor =
+    AuthorizationAcceptor::create(
+        principal,
+        master->authorizer,
+        authorization::VIEW_ROLE);
 
-      return ok;
+  return collect(
+      frameworksApprover, tasksApprover, executorsApprover, rolesAcceptor)
+    .then(defer(master->self(),
+        [=](const tuple<Owned<ObjectApprover>,
+                        Owned<ObjectApprover>,
+                        Owned<ObjectApprover>,
+                        Owned<AuthorizationAcceptor>>& approvers)
+            -> Future<Response> {
+          // Get approver from tuple.
+          Owned<ObjectApprover> frameworksApprover;
+          Owned<ObjectApprover> tasksApprover;
+          Owned<ObjectApprover> executorsApprover;
+          Owned<AuthorizationAcceptor> rolesAcceptor;
+          tie(frameworksApprover,
+              tasksApprover,
+              executorsApprover,
+              rolesAcceptor) = approvers;
+
+          Pipe pipe;
+          OK ok;
+
+          ok.headers["Content-Type"] = stringify(contentType);
+          ok.type = Response::PIPE;
+          ok.reader = pipe.reader();
+
+          HttpConnection http{pipe.writer(), contentType, UUID::random()};
+          master->subscribe(http);
+
+          mesos::master::Event event;
+          event.set_type(mesos::master::Event::SUBSCRIBED);
+          event.mutable_subscribed()->mutable_get_state()->CopyFrom(
+              _getState(
+                  frameworksApprover,
+                  tasksApprover,
+                  executorsApprover,
+                  rolesAcceptor));
+
+          http.send<mesos::master::Event, v1::master::Event>(event);
+
+          return ok;
     }));
 }
 
@@ -1837,27 +1851,41 @@ Future<Response> Master::Http::getState(
     executorsApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
   }
 
-  return collect(frameworksApprover, tasksApprover, executorsApprover)
-    .then(defer(master->self(),
-      [=](const tuple<Owned<ObjectApprover>,
-                      Owned<ObjectApprover>,
-                      Owned<ObjectApprover>>& approvers)
-        -> Future<Response> {
-      // Get approver from tuple.
-      Owned<ObjectApprover> frameworksApprover;
-      Owned<ObjectApprover> tasksApprover;
-      Owned<ObjectApprover> executorsApprover;
-      tie(frameworksApprover, tasksApprover, executorsApprover) = approvers;
-
-      mesos::master::Response response;
-      response.set_type(mesos::master::Response::GET_STATE);
-      response.mutable_get_state()->CopyFrom(
-          _getState(frameworksApprover,
-                    tasksApprover,
-                    executorsApprover));
+  Future<Owned<AuthorizationAcceptor>> rolesAcceptor =
+    AuthorizationAcceptor::create(
+        principal,
+        master->authorizer,
+        authorization::VIEW_ROLE);
 
-      return OK(serialize(contentType, evolve(response)),
-                stringify(contentType));
+  return collect(
+      frameworksApprover, tasksApprover, executorsApprover, rolesAcceptor)
+    .then(defer(master->self(),
+        [=](const tuple<Owned<ObjectApprover>,
+                        Owned<ObjectApprover>,
+                        Owned<ObjectApprover>,
+                        Owned<AuthorizationAcceptor>>& approvers)
+            -> Future<Response> {
+          // Get approver from tuple.
+          Owned<ObjectApprover> frameworksApprover;
+          Owned<ObjectApprover> tasksApprover;
+          Owned<ObjectApprover> executorsApprover;
+          Owned<AuthorizationAcceptor> rolesAcceptor;
+          tie(frameworksApprover,
+              tasksApprover,
+              executorsApprover,
+              rolesAcceptor) = approvers;
+
+          mesos::master::Response response;
+          response.set_type(mesos::master::Response::GET_STATE);
+          response.mutable_get_state()->CopyFrom(
+              _getState(
+                  frameworksApprover,
+                  tasksApprover,
+                  executorsApprover,
+                  rolesAcceptor));
+
+          return OK(
+              serialize(contentType, evolve(response)), stringify(contentType));
     }));
 }
 
@@ -1865,7 +1893,8 @@ Future<Response> Master::Http::getState(
 mesos::master::Response::GetState Master::Http::_getState(
     const Owned<ObjectApprover>& frameworksApprover,
     const Owned<ObjectApprover>& tasksApprover,
-    const Owned<ObjectApprover>& executorsApprover) const
+    const Owned<ObjectApprover>& executorsApprover,
+    const Owned<AuthorizationAcceptor>& rolesAcceptor) const
 {
   // NOTE: This function must be blocking instead of returning a
   // `Future`. This is because `subscribe()` needs to atomically
@@ -1883,7 +1912,7 @@ mesos::master::Response::GetState Master::Http::_getState(
   getState.mutable_get_frameworks()->CopyFrom(
       _getFrameworks(frameworksApprover));
 
-  getState.mutable_get_agents()->CopyFrom(_getAgents());
+  getState.mutable_get_agents()->CopyFrom(_getAgents(rolesAcceptor));
 
   return getState;
 }
@@ -2498,25 +2527,42 @@ Future<process::http::Response> Master::Http::getAgents(
 {
   CHECK_EQ(mesos::master::Call::GET_AGENTS, call.type());
 
-  mesos::master::Response response;
-  response.set_type(mesos::master::Response::GET_AGENTS);
-  response.mutable_get_agents()->CopyFrom(_getAgents());
-
-  return OK(serialize(contentType, evolve(response)),
-            stringify(contentType));
+  return AuthorizationAcceptor::create(
+      principal,
+      master->authorizer,
+      authorization::VIEW_ROLE)
+    .then(defer(master->self(),
+        [=](const Owned<AuthorizationAcceptor>& rolesAcceptor)
+            -> Future<process::http::Response> {
+          mesos::master::Response response;
+          response.set_type(mesos::master::Response::GET_AGENTS);
+          response.mutable_get_agents()->CopyFrom(_getAgents(rolesAcceptor));
+
+          return OK(serialize(contentType, evolve(response)),
+                    stringify(contentType));
+    }));
 }
 
 
-mesos::master::Response::GetAgents Master::Http::_getAgents() const
+mesos::master::Response::GetAgents Master::Http::_getAgents(
+    const Owned<AuthorizationAcceptor>& rolesAcceptor) const
 {
   mesos::master::Response::GetAgents getAgents;
   foreachvalue (const Slave* slave, master->slaves.registered) {
     mesos::master::Response::GetAgents::Agent* agent = getAgents.add_agents();
-    agent->CopyFrom(protobuf::master::event::createAgentResponse(*slave));
+    agent->CopyFrom(
+        protobuf::master::event::createAgentResponse(*slave, rolesAcceptor));
   }
 
   foreachvalue (const SlaveInfo& slaveInfo, master->slaves.recovered) {
-    getAgents.add_recovered_agents()->CopyFrom(slaveInfo);
+    SlaveInfo* agent = getAgents.add_recovered_agents();
+    agent->CopyFrom(slaveInfo);
+    agent->clear_resources();
+    foreach (const Resource& resource, slaveInfo.resources()) {
+      if (authorizeResource(resource, rolesAcceptor)) {
+        agent->add_resources()->CopyFrom(resource);
+      }
+    }
   }
 
   return getAgents;

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 84465af..b802fd1 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1452,7 +1452,8 @@ private:
         const Option<process::http::authentication::Principal>& principal,
         ContentType contentType) const;
 
-    mesos::master::Response::GetAgents _getAgents() const;
+    mesos::master::Response::GetAgents _getAgents(
+        const process::Owned<AuthorizationAcceptor>& rolesAcceptor) const;
 
     process::Future<process::http::Response> getFlags(
         const mesos::master::Call& call,
@@ -1578,7 +1579,8 @@ private:
     mesos::master::Response::GetState _getState(
         const process::Owned<ObjectApprover>& frameworksApprover,
         const process::Owned<ObjectApprover>& taskApprover,
-        const process::Owned<ObjectApprover>& executorsApprover) const;
+        const process::Owned<ObjectApprover>& executorsApprover,
+        const process::Owned<AuthorizationAcceptor>& rolesAcceptor) const;
 
     process::Future<process::http::Response> subscribe(
         const mesos::master::Call& call,

http://git-wip-us.apache.org/repos/asf/mesos/blob/2fe25624/src/tests/api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index 1d5b080..75f7a58 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -63,6 +63,8 @@
 
 namespace http = process::http;
 
+using google::protobuf::RepeatedPtrField;
+
 using mesos::master::detector::MasterDetector;
 using mesos::master::detector::StandaloneMasterDetector;
 
@@ -1498,7 +1500,7 @@ TEST_P(MasterAPITest, StartAndStopMaintenance)
         "api/v1",
         headers,
         serialize(contentType, v1StopMaintenanceCall),
-        stringify(contentType));
+       stringify(contentType));
 
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::Forbidden().status, response);
   }
@@ -1633,13 +1635,236 @@ TEST_P(MasterAPITest, SubscribeAgentEvents)
 }
 
 
+// This test verifies that no information about reservations and/or allocations
+// is returned to unauthorized users in response to the GET_AGENTS call.
+TEST_P(MasterAPITest, GetAgentsFiltering)
+{
+  master::Flags flags = CreateMasterFlags();
+
+  const string roleSuperhero = "superhero";
+  const string roleMuggle = "muggle";
+
+  {
+    mesos::ACL::ViewRole* acl = flags.acls.get().add_view_roles();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+    acl->mutable_roles()->add_values(roleSuperhero);
+
+    acl = flags.acls.get().add_view_roles();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+    acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE);
+  }
+
+  {
+    mesos::ACL::ViewRole* acl = flags.acls.get().add_view_roles();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
+    acl->mutable_roles()->add_values(roleMuggle);
+
+    acl = flags.acls.get().add_view_roles();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
+    acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE);
+  }
+
+  Try<Owned<cluster::Master>> master = StartMaster(flags);
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  Future<SlaveRegisteredMessage> agentRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), master.get()->pid, _);
+
+  slave::Flags slaveFlags = this->CreateSlaveFlags();
+
+  // Statically reserve some resources on the agent.
+  slaveFlags.resources =
+    "cpus(muggle):1;cpus(*):2;gpus(*):0;mem(muggle):1024;mem(*):1024;"
+    "disk(muggle):1024;disk(*):1024;ports(muggle):[30000-30999];"
+    "ports(*):[31000-32000]";
+
+  Try<Owned<cluster::Slave>> agent = StartSlave(detector.get(), slaveFlags);
+  ASSERT_SOME(agent);
+
+  AWAIT_READY(agentRegisteredMessage);
+  const SlaveID& agentId = agentRegisteredMessage->slave_id();
+
+  // Create dynamic reservation.
+  {
+    RepeatedPtrField<Resource> reservation =
+      Resources::parse("cpus:1;mem:12")->pushReservation(
+          createDynamicReservationInfo(
+              roleSuperhero,
+              DEFAULT_CREDENTIAL.principal()));
+
+    Future<http::Response> response = process::http::post(
+        master.get()->pid,
+        "reserve",
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+        strings::format(
+            "slaveId=%s&resources=%s",
+            agentId,
+            JSON::protobuf(reservation)).get());
+
+    AWAIT_READY(response);
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::Accepted().status, response);
+  }
+
+  v1::master::Call v1Call;
+  v1Call.set_type(v1::master::Call::GET_AGENTS);
+  ContentType contentType = GetParam();
+
+  // Default credential principal should only be allowed to see resources
+  // which are reserved for the role 'superhero'.
+  {
+    http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+    headers["Accept"] = stringify(contentType);
+
+    Future<http::Response> response = http::post(
+        master.get()->pid,
+        "api/v1",
+        headers,
+        serialize(contentType, v1Call),
+        stringify(contentType));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::OK().status, response);
+
+    Try<v1::master::Response> v1Response =
+      deserialize<v1::master::Response>(contentType, response->body);
+
+    ASSERT_TRUE(v1Response->IsInitialized());
+    ASSERT_EQ(v1::master::Response::GET_AGENTS, v1Response->type());
+    ASSERT_EQ(1, v1Response->get_agents().agents_size());
+
+    // AgentInfo.resources is not passed through `convertResourceFormat()` so
+    // its format is different.
+    foreach (const v1::Resource& resource,
+             v1Response->get_agents().agents(0).agent_info().resources()) {
+      EXPECT_FALSE(resource.has_role());
+      EXPECT_FALSE(resource.has_allocation_info());
+      EXPECT_FALSE(resource.has_reservation());
+      EXPECT_EQ(0, resource.reservations_size());
+    }
+
+    vector<RepeatedPtrField<v1::Resource>> resourceFields = {
+      v1Response->get_agents().agents(0).total_resources(),
+      v1Response->get_agents().agents(0).allocated_resources(),
+      v1Response->get_agents().agents(0).offered_resources()
+    };
+
+    bool hasReservedResources = false;
+    foreach (const RepeatedPtrField<v1::Resource>& resources, resourceFields) {
+      foreach (const v1::Resource& resource, resources) {
+        EXPECT_TRUE(resource.has_role());
+        EXPECT_TRUE(roleSuperhero == resource.role() || "*" == resource.role());
+
+        EXPECT_FALSE(resource.has_allocation_info());
+
+        if (resource.role() != "*") {
+          hasReservedResources = true;
+
+          EXPECT_TRUE(resource.has_reservation());
+          EXPECT_FALSE(resource.reservation().has_role());
+
+          EXPECT_NE(0, resource.reservations_size());
+          foreach (const v1::Resource::ReservationInfo& reservation,
+                   resource.reservations()) {
+            EXPECT_EQ(roleSuperhero, reservation.role());
+          }
+        } else {
+          EXPECT_FALSE(resource.has_reservation());
+          EXPECT_EQ(0, resource.reservations_size());
+        }
+      }
+    }
+    EXPECT_TRUE(hasReservedResources);
+  }
+
+  // Default credential principal 2 should only be allowed to see resources
+  // which are reserved for the role 'muggle'.
+  {
+    http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL_2);
+    headers["Accept"] = stringify(contentType);
+
+    Future<http::Response> response = http::post(
+        master.get()->pid,
+        "api/v1",
+        headers,
+        serialize(contentType, v1Call),
+        stringify(contentType));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::OK().status, response);
+
+    Try<v1::master::Response> v1Response =
+      deserialize<v1::master::Response>(contentType, response->body);
+
+    ASSERT_TRUE(v1Response->IsInitialized());
+    ASSERT_EQ(v1::master::Response::GET_AGENTS, v1Response->type());
+    ASSERT_EQ(1, v1Response->get_agents().agents_size());
+
+    // AgentInfo.resources is not passed through `convertResourceFormat()` so
+    // its format is different.
+    foreach (const v1::Resource& resource,
+             v1Response->get_agents().agents(0).agent_info().resources()) {
+      EXPECT_FALSE(resource.has_role());
+      EXPECT_FALSE(resource.has_allocation_info());
+      EXPECT_FALSE(resource.has_reservation());
+      if (resource.reservations_size() > 0) {
+        foreach (const v1::Resource::ReservationInfo& reservation,
+                 resource.reservations()) {
+          EXPECT_EQ(roleMuggle, reservation.role());
+        }
+      }
+    }
+
+    vector<RepeatedPtrField<v1::Resource>> resourceFields = {
+      v1Response->get_agents().agents(0).total_resources(),
+      v1Response->get_agents().agents(0).allocated_resources(),
+      v1Response->get_agents().agents(0).offered_resources()
+    };
+
+    bool hasReservedResources = false;
+    foreach (const RepeatedPtrField<v1::Resource>& resources, resourceFields) {
+      foreach (const v1::Resource& resource, resources) {
+        EXPECT_TRUE(resource.has_role());
+        EXPECT_TRUE(roleMuggle == resource.role() || "*" == resource.role());
+
+        EXPECT_FALSE(resource.has_allocation_info());
+
+        if (resource.role() != "*") {
+          hasReservedResources = true;
+          EXPECT_FALSE(resource.has_reservation());
+
+          EXPECT_NE(0, resource.reservations_size());
+          foreach (const v1::Resource::ReservationInfo& reservation,
+                   resource.reservations()) {
+            EXPECT_EQ(roleMuggle, reservation.role());
+          }
+        } else {
+          EXPECT_FALSE(resource.has_reservation());
+          EXPECT_EQ(0, resource.reservations_size());
+        }
+      }
+    }
+    EXPECT_TRUE(hasReservedResources);
+  }
+}
+
+
 // This test verifies that recovered but yet to reregister agents are returned
-// in `recovered_agents` field of `GetAgents` response.
+// in `recovered_agents` field of `GetAgents` response. Authorization is enabled
+// to ensure that authorization-based filtering is able to handle recovered
+// agents, whose resources are currently stored in the
+// pre-reservation-refinement format (see MESOS-7851).
 TEST_P_TEMP_DISABLED_ON_WINDOWS(MasterAPITest, GetRecoveredAgents)
 {
   master::Flags masterFlags = CreateMasterFlags();
   masterFlags.registry = "replicated_log";
 
+  // This forces the authorizer to be initialized.
+  {
+    mesos::ACL::ViewRole* acl = masterFlags.acls.get().add_view_roles();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+    acl->mutable_roles()->set_type(mesos::ACL::Entity::ANY);
+  }
+
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
@@ -1649,6 +1874,11 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(MasterAPITest, GetRecoveredAgents)
   // Reuse slaveFlags so both StartSlave() use the same work_dir.
   slave::Flags slaveFlags = this->CreateSlaveFlags();
 
+  // Statically reserve some resources on the agent.
+  slaveFlags.resources =
+    "cpus(foo):1;cpus(*):2;gpus(*):0;mem(foo):1024;mem(*):1024;"
+    "disk(foo):1024;disk(*):1024;ports(*):[31000-32000]";
+
   Owned<MasterDetector> detector = master.get()->createDetector();
   Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
   ASSERT_SOME(slave);
@@ -1657,6 +1887,27 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(MasterAPITest, GetRecoveredAgents)
 
   v1::AgentID agentId = evolve(slaveRegisteredMessage->slave_id());
 
+  // Create dynamic reservation.
+  {
+    RepeatedPtrField<Resource> reservation =
+      Resources::parse("cpus:1;mem:12")->pushReservation(
+          createDynamicReservationInfo(
+              "bar",
+              DEFAULT_CREDENTIAL.principal()));
+
+    Future<http::Response> response = process::http::post(
+        master.get()->pid,
+        "reserve",
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+        strings::format(
+            "slaveId=%s&resources=%s",
+            agentId,
+            JSON::protobuf(reservation)).get());
+
+    AWAIT_READY(response);
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::Accepted().status, response);
+  }
+
   // Ensure that the agent is present in `GetAgent.agents` while
   // `GetAgents.recovered_agents` is empty.
   {


[2/2] mesos git commit: Added full authz for non summarized fields of `/slaves` endpoint.

Posted by gr...@apache.org.
Added full authz for non summarized fields of `/slaves` endpoint.

Fields were authorized based on partial elements of each
resource. Moreover, some fields which required authorization were not
being authorized at all. This patch enables full authorization of all
fields.

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


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

Branch: refs/heads/master
Commit: e87569b2ae3c7f8303ce146f882c340b4fdd5ca4
Parents: 2fe2562
Author: Alexander Rojas <al...@mesosphere.io>
Authored: Wed Aug 2 13:14:07 2017 -0700
Committer: Greg Mann <gr...@gmail.com>
Committed: Wed Aug 2 15:35:29 2017 -0700

----------------------------------------------------------------------
 src/master/http.cpp | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e87569b2/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index e589829..959091c 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -484,10 +484,13 @@ struct SlavesWriter
                        const Resources& resources,
                        reserved) {
             if (authorizeRole_->accept(role)) {
-              writer->field(role, [&resources](JSON::ArrayWriter* writer) {
+              writer->field(role, [&resources, this](
+                  JSON::ArrayWriter* writer) {
                 foreach (Resource resource, resources) {
-                  convertResourceFormat(&resource, ENDPOINT);
-                  writer->element(JSON::Protobuf(resource));
+                  if (authorizeResource(resource, authorizeRole_)) {
+                    convertResourceFormat(&resource, ENDPOINT);
+                    writer->element(JSON::Protobuf(resource));
+                  }
                 }
               });
             }
@@ -498,10 +501,12 @@ struct SlavesWriter
 
     writer->field(
         "unreserved_resources_full",
-        [&unreservedResources](JSON::ArrayWriter* writer) {
+        [&unreservedResources, this](JSON::ArrayWriter* writer) {
           foreach (Resource resource, unreservedResources) {
-            convertResourceFormat(&resource, ENDPOINT);
-            writer->element(JSON::Protobuf(resource));
+            if (authorizeResource(resource, authorizeRole_)) {
+              convertResourceFormat(&resource, ENDPOINT);
+              writer->element(JSON::Protobuf(resource));
+            }
           }
         });
 
@@ -511,8 +516,7 @@ struct SlavesWriter
         "used_resources_full",
         [&usedResources, this](JSON::ArrayWriter* writer) {
           foreach (Resource resource, usedResources) {
-            if (authorizeRole_->accept(resource.role()) &&
-                authorizeRole_->accept(resource.allocation_info().role())) {
+            if (authorizeResource(resource, authorizeRole_)) {
               convertResourceFormat(&resource, ENDPOINT);
               writer->element(JSON::Protobuf(resource));
             }
@@ -525,7 +529,7 @@ struct SlavesWriter
         "offered_resources_full",
         [&offeredResources, this](JSON::ArrayWriter* writer) {
           foreach (Resource resource, offeredResources) {
-            if (authorizeRole_->accept(resource.role())) {
+            if (authorizeResource(resource, authorizeRole_)) {
               convertResourceFormat(&resource, ENDPOINT);
               writer->element(JSON::Protobuf(resource));
             }