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 <alexan...@mesosphere.io>
Authored: Wed Aug 2 13:14:01 2017 -0700
Committer: Greg Mann <gregorywm...@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.
   {

Reply via email to