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. {