You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2017/04/28 21:55:47 UTC
[1/6] mesos git commit: Added and implemented RegisterAgent ACL.
Repository: mesos
Updated Branches:
refs/heads/master 72752fc6d -> 9fb6a11a5
Added and implemented RegisterAgent ACL.
Review: https://reviews.apache.org/r/57534
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/38afa9c1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/38afa9c1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/38afa9c1
Branch: refs/heads/master
Commit: 38afa9c1afa65d931bfcc215b5faa7da76d62c19
Parents: 72752fc
Author: Jiang Yan Xu <xu...@apple.com>
Authored: Fri Mar 10 15:25:07 2017 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 11:13:36 2017 -0700
----------------------------------------------------------------------
include/mesos/authorizer/acls.proto | 11 ++++++
include/mesos/authorizer/authorizer.proto | 4 ++
src/authorizer/local/authorizer.cpp | 24 ++++++++++++
src/tests/authorization_tests.cpp | 54 ++++++++++++++++++++++++++
4 files changed, 93 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/38afa9c1/include/mesos/authorizer/acls.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto
index 796ebb7..ae0b1ea 100644
--- a/include/mesos/authorizer/acls.proto
+++ b/include/mesos/authorizer/acls.proto
@@ -343,6 +343,16 @@ message ACL {
// access.
required Entity level = 2;
}
+
+ // Which principals are authorized to register (and re-register) as agents.
+ message RegisterAgent {
+ // Subjects: Agent principals.
+ required Entity principals = 1;
+
+ // Objects: Given implicitly. Use Entity type ANY or NONE to allow or deny
+ // access.
+ required Entity agent = 2;
+ }
}
@@ -407,4 +417,5 @@ message ACLs {
repeated ACL.ViewContainer view_containers = 31;
repeated ACL.SetLogLevel set_log_level = 32;
repeated ACL.RemoveNestedContainer remove_nested_containers = 33;
+ repeated ACL.RegisterAgent register_agents = 34;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/38afa9c1/include/mesos/authorizer/authorizer.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto
index 3ae5df5..c9184d1 100644
--- a/include/mesos/authorizer/authorizer.proto
+++ b/include/mesos/authorizer/authorizer.proto
@@ -193,6 +193,10 @@ enum Action {
// This action will set objects of type `ExecutorInfo`, `FrameworkInfo`, and
// `ContainerID`.
REMOVE_NESTED_CONTAINER = 27;
+
+ // This action will not fill in any object fields, since a principal is
+ // either allowed to register as an agent or is unauthorized.
+ REGISTER_AGENT = 28;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/38afa9c1/src/authorizer/local/authorizer.cpp
----------------------------------------------------------------------
diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp
index 0e323f3..89aaf4b 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -392,6 +392,10 @@ public:
aclObject.set_type(mesos::ACL::Entity::ANY);
break;
+ case authorization::REGISTER_AGENT:
+ aclObject.set_type(mesos::ACL::Entity::ANY);
+
+ break;
case authorization::CREATE_VOLUME:
case authorization::GET_QUOTA:
case authorization::RESERVE_RESOURCES:
@@ -654,6 +658,7 @@ public:
case authorization::VIEW_FRAMEWORK:
case authorization::VIEW_TASK:
case authorization::WAIT_NESTED_CONTAINER:
+ case authorization::REGISTER_AGENT:
case authorization::UNKNOWN:
UNREACHABLE();
}
@@ -859,6 +864,7 @@ public:
case authorization::VIEW_TASK:
case authorization::WAIT_NESTED_CONTAINER:
case authorization::REMOVE_NESTED_CONTAINER:
+ case authorization::REGISTER_AGENT:
UNREACHABLE();
}
@@ -1016,6 +1022,7 @@ public:
case authorization::VIEW_TASK:
case authorization::WAIT_NESTED_CONTAINER:
case authorization::REMOVE_NESTED_CONTAINER:
+ case authorization::REGISTER_AGENT:
case authorization::UNKNOWN: {
Result<vector<GenericACL>> genericACLs =
createGenericACLs(action, acls);
@@ -1231,6 +1238,16 @@ private:
}
return acls_;
+ case authorization::REGISTER_AGENT:
+ foreach (const ACL::RegisterAgent& acl, acls.register_agents()) {
+ GenericACL acl_;
+ acl_.subjects = acl.principals();
+ acl_.objects = acl.agent();
+
+ acls_.push_back(acl_);
+ }
+
+ return acls_;
case authorization::REGISTER_FRAMEWORK:
case authorization::CREATE_VOLUME:
case authorization::RESERVE_RESOURCES:
@@ -1319,6 +1336,13 @@ Option<Error> LocalAuthorizer::validate(const ACLs& acls)
}
}
+ foreach (const ACL::RegisterAgent& acl, acls.register_agents()) {
+ if (acl.agent().type() == ACL::Entity::SOME) {
+ return Error(
+ "acls.register_agents.agent type must be either NONE or ANY");
+ }
+ }
+
// TODO(alexr): Consider validating not only protobuf, but also the original
// JSON in order to spot misspelled names. A misspelled action may affect
// authorization result and hence lead to a security issue (e.g. when there
http://git-wip-us.apache.org/repos/asf/mesos/blob/38afa9c1/src/tests/authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/authorization_tests.cpp b/src/tests/authorization_tests.cpp
index b59623f..32aa6ac 100644
--- a/src/tests/authorization_tests.cpp
+++ b/src/tests/authorization_tests.cpp
@@ -4889,6 +4889,60 @@ TYPED_TEST(AuthorizationTest, GetQuota)
}
}
+
+TYPED_TEST(AuthorizationTest, RegisterAgent)
+{
+ ACLs acls;
+
+ {
+ // "foo" principal can register as an agent.
+ mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
+ acl->mutable_principals()->add_values("foo");
+ acl->mutable_agent()->set_type(mesos::ACL::Entity::ANY);
+ }
+
+ {
+ // Nobody else can register as an agent.
+ mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
+ acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+ acl->mutable_agent()->set_type(mesos::ACL::Entity::NONE);
+ }
+
+ Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+ ASSERT_SOME(create);
+ Owned<Authorizer> authorizer(create.get());
+
+ {
+ // "foo" is in the "whitelist".
+ authorization::Request request;
+ request.set_action(authorization::REGISTER_AGENT);
+ request.mutable_subject()->set_value("foo");
+
+ AWAIT_EXPECT_TRUE(authorizer.get()->authorized(request));
+ }
+
+ {
+ // "bar" is not in the "whitelist".
+ authorization::Request request;
+ request.set_action(authorization::REGISTER_AGENT);
+ request.mutable_subject()->set_value("bar");
+
+ AWAIT_EXPECT_FALSE(authorizer.get()->authorized(request));
+ }
+
+ {
+ // Test that no authorizer is created with invalid ACLs.
+ ACLs invalid;
+
+ mesos::ACL::RegisterAgent* acl = invalid.add_register_agents();
+ acl->mutable_principals()->add_values("foo");
+ acl->mutable_agent()->add_values("yoda");
+
+ Try<Authorizer*> create = TypeParam::create(parameterize(invalid));
+ EXPECT_ERROR(create);
+ }
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {
[2/6] mesos git commit: Applied RegisterAgent ACL to the master.
Posted by ya...@apache.org.
Applied RegisterAgent ACL to the master.
Review: https://reviews.apache.org/r/57535
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/29fc2dfc
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/29fc2dfc
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/29fc2dfc
Branch: refs/heads/master
Commit: 29fc2dfcb110a51923d4d7c144bdd797b348f96b
Parents: 38afa9c
Author: Jiang Yan Xu <xu...@apple.com>
Authored: Fri Mar 10 15:25:43 2017 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 14:55:08 2017 -0700
----------------------------------------------------------------------
src/master/master.cpp | 247 +++++++++++++++++++++-----
src/master/master.hpp | 39 +++-
src/tests/master_authorization_tests.cpp | 162 +++++++++++++++++
3 files changed, 394 insertions(+), 54 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/29fc2dfc/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index e8c2a96..2be4056 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3651,6 +3651,31 @@ Future<bool> Master::authorizeDestroyVolume(
}
+Future<bool> Master::authorizeSlave(const Option<string>& principal)
+{
+ if (authorizer.isNone()) {
+ return true;
+ }
+
+ LOG(INFO) << "Authorizing agent "
+ << (principal.isSome()
+ ? "with principal '" + principal.get() + "'"
+ : "without a principal");
+
+ authorization::Request request;
+ request.set_action(authorization::REGISTER_AGENT);
+
+ if (principal.isSome()) {
+ request.mutable_subject()->set_value(principal.get());
+ }
+
+ // No need to set the request's object as it is implicitly set to
+ // ANY by the authorizer.
+
+ return authorizer.get()->authorized(request);
+}
+
+
Resources Master::addTask(
const TaskInfo& task,
Framework* framework,
@@ -5388,26 +5413,91 @@ void Master::registerSlave(
return;
}
+ if (slaves.registering.contains(from)) {
+ LOG(INFO) << "Ignoring register agent message from " << from
+ << " (" << slaveInfo.hostname() << ") as registration"
+ << " is already in progress";
+ return;
+ }
+
+ slaves.registering.insert(from);
+
+ // Note that the principal may be empty if authentication is not
+ // required. Also it is passed along because it may be removed from
+ // `authenticated` while the authorization is pending.
+ Option<string> principal = authenticated.get(from);
+
+ authorizeSlave(principal)
+ .onAny(defer(self(),
+ &Self::_registerSlave,
+ slaveInfo,
+ from,
+ principal,
+ checkpointedResources,
+ version,
+ agentCapabilities,
+ lambda::_1));
+}
+
+
+void Master::_registerSlave(
+ const SlaveInfo& slaveInfo,
+ const UPID& pid,
+ const Option<string>& principal,
+ const vector<Resource>& checkpointedResources,
+ const string& version,
+ const vector<SlaveInfo::Capability>& agentCapabilities,
+ const Future<bool>& authorized)
+{
+ CHECK(!authorized.isDiscarded());
+ CHECK(slaves.registering.contains(pid));
+
+ Option<string> authorizationError = None();
+
+ if (authorized.isFailed()) {
+ authorizationError = "Authorization failure: " + authorized.failure();
+ } else if (!authorized.get()) {
+ authorizationError =
+ "Not authorized to register as agent " +
+ (principal.isSome()
+ ? "with principal '" + principal.get() + "'"
+ : "without a principal");
+ }
+
+ if (authorizationError.isSome()) {
+ LOG(WARNING) << "Refusing registration of agent at " << pid
+ << ": " << authorizationError.get();
+
+ ShutdownMessage message;
+ message.set_message(authorizationError.get());
+ send(pid, message);
+
+ slaves.registering.erase(pid);
+ return;
+ }
+
MachineID machineId;
machineId.set_hostname(slaveInfo.hostname());
- machineId.set_ip(stringify(from.address.ip));
+ machineId.set_ip(stringify(pid.address.ip));
// Slaves are not allowed to register while the machine they are on is in
// `DOWN` mode.
if (machines.contains(machineId) &&
machines[machineId].info.mode() == MachineInfo::DOWN) {
- LOG(WARNING) << "Refusing registration of agent at " << from
+ LOG(WARNING) << "Refusing registration of agent at " << pid
<< " because the machine '" << machineId << "' that it is "
<< "running on is `DOWN`";
ShutdownMessage message;
message.set_message("Machine is `DOWN`");
- send(from, message);
+ send(pid, message);
+
+ slaves.registering.erase(pid);
return;
}
// Check if this slave is already registered (because it retries).
- if (Slave* slave = slaves.registered.get(from)) {
+ if (Slave* slave = slaves.registered.get(pid)) {
if (!slave->connected) {
// The slave was previously disconnected but it is now trying
// to register as a new slave. This could happen if the slave
@@ -5433,33 +5523,25 @@ void Master::registerSlave(
SlaveRegisteredMessage message;
message.mutable_slave_id()->CopyFrom(slave->id);
message.mutable_connection()->CopyFrom(connection);
- send(from, message);
+ send(pid, message);
+
+ slaves.registering.erase(pid);
return;
}
}
- // We need to generate a SlaveID and admit this slave only *once*.
- if (slaves.registering.contains(from)) {
- LOG(INFO) << "Ignoring register agent message from " << from
- << " (" << slaveInfo.hostname() << ") as admission is"
- << " already in progress";
- return;
- }
-
- slaves.registering.insert(from);
-
// Create and add the slave id.
SlaveInfo slaveInfo_ = slaveInfo;
slaveInfo_.mutable_id()->CopyFrom(newSlaveId());
- LOG(INFO) << "Registering agent at " << from << " ("
+ LOG(INFO) << "Registering agent at " << pid << " ("
<< slaveInfo.hostname() << ") with id " << slaveInfo_.id();
registrar->apply(Owned<Operation>(new AdmitSlave(slaveInfo_)))
.onAny(defer(self(),
- &Self::_registerSlave,
+ &Self::__registerSlave,
slaveInfo_,
- from,
+ pid,
checkpointedResources,
version,
agentCapabilities,
@@ -5467,7 +5549,7 @@ void Master::registerSlave(
}
-void Master::_registerSlave(
+void Master::__registerSlave(
const SlaveInfo& slaveInfo,
const UPID& pid,
const vector<Resource>& checkpointedResources,
@@ -5476,7 +5558,6 @@ void Master::_registerSlave(
const Future<bool>& admit)
{
CHECK(slaves.registering.contains(pid));
- slaves.registering.erase(pid);
CHECK(!admit.isDiscarded());
@@ -5495,6 +5576,8 @@ void Master::_registerSlave(
<< " (" << slaveInfo.hostname() << ") was assigned"
<< " an agent ID that already appears in the registry;"
<< " ignoring registration attempt";
+
+ slaves.registering.erase(pid);
return;
}
@@ -5528,6 +5611,8 @@ void Master::_registerSlave(
LOG(INFO) << "Registered agent " << *slave
<< " with " << slave->info.resources();
+
+ slaves.registering.erase(pid);
}
@@ -5585,21 +5670,95 @@ void Master::reregisterSlave(
return;
}
+ if (slaves.reregistering.contains(slaveInfo.id())) {
+ LOG(INFO)
+ << "Ignoring re-register agent message from agent "
+ << slaveInfo.id() << " at " << from << " ("
+ << slaveInfo.hostname() << ") as re-registration is already in progress";
+ return;
+ }
+
+ slaves.reregistering.insert(slaveInfo.id());
+
+ // Note that the principal may be empty if authentication is not
+ // required. Also it is passed along because it may be removed from
+ // `authenticated` while the authorization is pending.
+ Option<string> principal = authenticated.get(from);
+
+ authorizeSlave(principal)
+ .onAny(defer(self(),
+ &Self::_reregisterSlave,
+ slaveInfo,
+ from,
+ principal,
+ checkpointedResources,
+ executorInfos,
+ tasks,
+ frameworks,
+ completedFrameworks,
+ version,
+ agentCapabilities,
+ lambda::_1));
+}
+
+
+void Master::_reregisterSlave(
+ const SlaveInfo& slaveInfo,
+ const UPID& pid,
+ const Option<string>& principal,
+ const vector<Resource>& checkpointedResources,
+ const vector<ExecutorInfo>& executorInfos,
+ const vector<Task>& tasks,
+ const vector<FrameworkInfo>& frameworks,
+ const vector<Archive::Framework>& completedFrameworks,
+ const string& version,
+ const vector<SlaveInfo::Capability>& agentCapabilities,
+ const Future<bool>& authorized)
+{
+ CHECK(!authorized.isDiscarded());
+ CHECK(slaves.reregistering.contains(slaveInfo.id()));
+
+ Option<string> authorizationError = None();
+
+ if (authorized.isFailed()) {
+ authorizationError = "Authorization failure: " + authorized.failure();
+ } else if (!authorized.get()) {
+ authorizationError =
+ "Not authorized to re-register as agent with principal " +
+ (principal.isSome()
+ ? "with principal '" + principal.get() + "'"
+ : "without a principal");
+ }
+
+ if (authorizationError.isSome()) {
+ LOG(WARNING) << "Refusing re-registration of agent at " << pid
+ << ": " << authorizationError.get();
+
+ ShutdownMessage message;
+ message.set_message(authorizationError.get());
+ send(pid, message);
+
+ slaves.reregistering.erase(slaveInfo.id());
+ return;
+ }
+
MachineID machineId;
machineId.set_hostname(slaveInfo.hostname());
- machineId.set_ip(stringify(from.address.ip));
+ machineId.set_ip(stringify(pid.address.ip));
- // Slaves are not allowed to register while the machine they are on is in
+ // Slaves are not allowed to re-register while the machine they are on is in
// 'DOWN` mode.
if (machines.contains(machineId) &&
machines[machineId].info.mode() == MachineInfo::DOWN) {
- LOG(WARNING) << "Refusing re-registration of agent at " << from
+ LOG(WARNING) << "Refusing re-registration of agent at " << pid
<< " because the machine '" << machineId << "' that it is "
<< "running on is `DOWN`";
ShutdownMessage message;
message.set_message("Machine is `DOWN`");
- send(from, message);
+ send(pid, message);
+
+ slaves.reregistering.erase(slaveInfo.id());
return;
}
@@ -5619,9 +5778,9 @@ void Master::reregisterSlave(
// hostname. This is because maintenance is scheduled at the
// machine level; so we would need to re-validate the slave's
// unavailability if the machine it is running on changed.
- if (slave->pid.address.ip != from.address.ip ||
+ if (slave->pid.address.ip != pid.address.ip ||
slave->info.hostname() != slaveInfo.hostname()) {
- LOG(WARNING) << "Agent " << slaveInfo.id() << " at " << from
+ LOG(WARNING) << "Agent " << slaveInfo.id() << " at " << pid
<< " (" << slaveInfo.hostname() << ") attempted to "
<< "re-register with different IP / hostname; expected "
<< slave->pid.address.ip << " (" << slave->info.hostname()
@@ -5631,7 +5790,9 @@ void Master::reregisterSlave(
message.set_message(
"Agent attempted to re-register with different IP / hostname");
- send(from, message);
+ send(pid, message);
+
+ slaves.reregistering.erase(slaveInfo.id());
return;
}
@@ -5641,7 +5802,7 @@ void Master::reregisterSlave(
// in succession for a disconnected slave. As a result, we
// ignore duplicate exited events for disconnected slaves.
// See: https://issues.apache.org/jira/browse/MESOS-675
- slave->pid = from;
+ slave->pid = pid;
link(slave->pid);
// Update slave's version, re-registration timestamp and
@@ -5676,26 +5837,15 @@ void Master::reregisterSlave(
// Inform the agent of the master's version of its checkpointed
// resources and the new framework pids for its tasks.
- __reregisterSlave(slave, tasks, frameworks);
-
- return;
- }
+ ___reregisterSlave(slave, tasks, frameworks);
- // If we're already re-registering this slave, then no need to ask
- // the registrar again.
- if (slaves.reregistering.contains(slaveInfo.id())) {
- LOG(INFO)
- << "Ignoring re-register agent message from agent "
- << slaveInfo.id() << " at " << from << " ("
- << slaveInfo.hostname() << ") as readmission is already in progress";
+ slaves.reregistering.erase(slaveInfo.id());
return;
}
- LOG(INFO) << "Re-registering agent " << slaveInfo.id() << " at " << from
+ LOG(INFO) << "Re-registering agent " << slaveInfo.id() << " at " << pid
<< " (" << slaveInfo.hostname() << ")";
- slaves.reregistering.insert(slaveInfo.id());
-
// Consult the registry to determine whether to readmit the
// slave. In the common case, the slave has been marked unreachable
// by the master, so we move the slave to the reachable list and
@@ -5704,9 +5854,9 @@ void Master::reregisterSlave(
// GC'd), we admit the slave anyway.
registrar->apply(Owned<Operation>(new MarkSlaveReachable(slaveInfo)))
.onAny(defer(self(),
- &Self::_reregisterSlave,
+ &Self::__reregisterSlave,
slaveInfo,
- from,
+ pid,
checkpointedResources,
executorInfos,
tasks,
@@ -5718,7 +5868,7 @@ void Master::reregisterSlave(
}
-void Master::_reregisterSlave(
+void Master::__reregisterSlave(
const SlaveInfo& slaveInfo,
const UPID& pid,
const vector<Resource>& checkpointedResources,
@@ -5731,7 +5881,6 @@ void Master::_reregisterSlave(
const Future<bool>& readmit)
{
CHECK(slaves.reregistering.contains(slaveInfo.id()));
- slaves.reregistering.erase(slaveInfo.id());
if (readmit.isFailed()) {
LOG(FATAL) << "Failed to readmit agent " << slaveInfo.id() << " at " << pid
@@ -5966,11 +6115,13 @@ void Master::_reregisterSlave(
}
}
- __reregisterSlave(slave, tasks, frameworks);
+ ___reregisterSlave(slave, tasks, frameworks);
+
+ slaves.reregistering.erase(slaveInfo.id());
}
-void Master::__reregisterSlave(
+void Master::___reregisterSlave(
Slave* slave,
const vector<Task>& tasks,
const vector<FrameworkInfo>& frameworks)
http://git-wip-us.apache.org/repos/asf/mesos/blob/29fc2dfc/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index d537933..eca353b 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -514,6 +514,15 @@ protected:
void _registerSlave(
const SlaveInfo& slaveInfo,
const process::UPID& pid,
+ const Option<std::string>& principal,
+ const std::vector<Resource>& checkpointedResources,
+ const std::string& version,
+ const std::vector<SlaveInfo::Capability>& agentCapabilities,
+ const process::Future<bool>& authorized);
+
+ void __registerSlave(
+ const SlaveInfo& slaveInfo,
+ const process::UPID& pid,
const std::vector<Resource>& checkpointedResources,
const std::string& version,
const std::vector<SlaveInfo::Capability>& agentCapabilities,
@@ -522,6 +531,7 @@ protected:
void _reregisterSlave(
const SlaveInfo& slaveInfo,
const process::UPID& pid,
+ const Option<std::string>& principal,
const std::vector<Resource>& checkpointedResources,
const std::vector<ExecutorInfo>& executorInfos,
const std::vector<Task>& tasks,
@@ -529,9 +539,21 @@ protected:
const std::vector<Archive::Framework>& completedFrameworks,
const std::string& version,
const std::vector<SlaveInfo::Capability>& agentCapabilities,
- const process::Future<bool>& readmit);
+ const process::Future<bool>& authorized);
void __reregisterSlave(
+ const SlaveInfo& slaveInfo,
+ const process::UPID& pid,
+ const std::vector<Resource>& checkpointedResources,
+ const std::vector<ExecutorInfo>& executorInfos,
+ const std::vector<Task>& tasks,
+ const std::vector<FrameworkInfo>& frameworks,
+ const std::vector<Archive::Framework>& completedFrameworks,
+ const std::string& version,
+ const std::vector<SlaveInfo::Capability>& agentCapabilities,
+ const process::Future<bool>& readmit);
+
+ void ___reregisterSlave(
Slave* slave,
const std::vector<Task>& tasks,
const std::vector<FrameworkInfo>& frameworks);
@@ -659,6 +681,9 @@ protected:
process::Future<bool> authorizeFramework(
const FrameworkInfo& frameworkInfo);
+ // Returns whether the principal is authorized to (re-)register an agent.
+ process::Future<bool> authorizeSlave(const Option<std::string>& principal);
+
// Returns whether the task is authorized.
// Returns failure for transient authorization failures.
process::Future<bool> authorizeTask(
@@ -1623,14 +1648,16 @@ private:
// failover. Slaves are removed from this collection when they
// either re-register with the master or are marked unreachable
// because they do not re-register before `recoveredTimer` fires.
+ // We must not answer questions related to these slaves (e.g.,
+ // during task reconciliation) until we determine their fate
+ // because their are in this transitioning state.
hashmap<SlaveID, SlaveInfo> recovered;
- // Slaves that are in the process of registering.
+ // Agents that are in the process of (re-)registering. They are
+ // maintained here while the (re-)registration is in progress and
+ // possibly pending in the authorizer or the registrar in order
+ // to help deduplicate (re-)registration requests.
hashset<process::UPID> registering;
-
- // Only those slaves that are re-registering for the first time
- // with this master. We must not answer questions related to
- // these slaves until the registrar determines their fate.
hashset<SlaveID> reregistering;
// Registered slaves are indexed by SlaveID and UPID. Note that
http://git-wip-us.apache.org/repos/asf/mesos/blob/29fc2dfc/src/tests/master_authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_authorization_tests.cpp b/src/tests/master_authorization_tests.cpp
index 1a0285a..a646768 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -47,6 +47,7 @@
#include "messages/messages.hpp"
+#include "slave/constants.hpp"
#include "slave/slave.hpp"
#include "tests/containerizer.hpp"
@@ -67,6 +68,7 @@ using mesos::master::detector::StandaloneMasterDetector;
using process::Clock;
using process::Future;
+using process::Message;
using process::Owned;
using process::PID;
using process::Promise;
@@ -82,6 +84,7 @@ using testing::_;
using testing::An;
using testing::AtMost;
using testing::DoAll;
+using testing::Eq;
using testing::Return;
namespace mesos {
@@ -2323,6 +2326,165 @@ TYPED_TEST(MasterAuthorizerTest, FilterOrphanedTasks)
driver.join();
}
+
+TEST_F(MasterAuthorizationTest, AuthorizedToRegisterAndReregisterAgent)
+{
+ // Set up ACLs so that the agent can (re)register.
+ ACLs acls;
+ mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
+ acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+ acl->mutable_agent()->set_type(ACL::Entity::ANY);
+
+ master::Flags masterFlags = CreateMasterFlags();
+ masterFlags.acls = acls;
+
+ Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
+ ASSERT_SOME(master);
+
+ slave::Flags slaveFlags = CreateSlaveFlags();
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ Future<Message> slaveRegisteredMessage =
+ FUTURE_MESSAGE(Eq(SlaveRegisteredMessage().GetTypeName()), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ // Simulate a recovered agent and verify that it is allowed to reregister.
+ slave->reset();
+
+ Future<Message> slaveReregisteredMessage =
+ FUTURE_MESSAGE(Eq(SlaveReregisteredMessage().GetTypeName()), _, _);
+
+ slave = StartSlave(detector.get(), slaveFlags);
+
+ AWAIT_READY(slaveReregisteredMessage);
+}
+
+
+// This test verifies that the agent is shut down by the master if
+// it is not authorized to register.
+TEST_F(MasterAuthorizationTest, UnauthorizedToRegisterAgent)
+{
+ // Set up ACLs that disallows the agent's principal to register.
+ ACLs acls;
+ mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
+ acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+ acl->mutable_agent()->set_type(ACL::Entity::NONE);
+
+ master::Flags flags = CreateMasterFlags();
+ flags.acls = acls;
+
+ Try<Owned<cluster::Master>> master = StartMaster(flags);
+ ASSERT_SOME(master);
+
+ Future<Message> shutdownMessage =
+ FUTURE_MESSAGE(Eq(ShutdownMessage ().GetTypeName()), _, _);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(shutdownMessage);
+}
+
+
+// This test verifies that an agent authorized to register can be
+// unauthorized to re-register due to master ACL change (after failover).
+TEST_F(MasterAuthorizationTest, UnauthorizedToReregisterAgent)
+{
+ // Set up ACLs so that the agent can register.
+ ACLs acls;
+ mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
+ acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+ acl->mutable_agent()->set_type(ACL::Entity::ANY);
+
+ master::Flags flags = CreateMasterFlags();
+ flags.acls = acls;
+
+ Try<Owned<cluster::Master>> master = StartMaster(flags);
+ ASSERT_SOME(master);
+
+ slave::Flags slaveFlags = CreateSlaveFlags();
+ StandaloneMasterDetector detector(master.get()->pid);
+
+ Future<Message> slaveRegisteredMessage =
+ FUTURE_MESSAGE(Eq(SlaveRegisteredMessage().GetTypeName()), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(&detector);
+ ASSERT_SOME(slave);
+
+ AWAIT_READY(slaveRegisteredMessage);
+
+ // Master fails over.
+ master->reset();
+
+ // The new master doesn't allow this agent principal to re-register.
+ acl->mutable_agent()->set_type(ACL::Entity::NONE);
+ flags.acls = acls;
+
+ Future<Message> shutdownMessage =
+ FUTURE_MESSAGE(Eq(ShutdownMessage().GetTypeName()), _, _);
+
+ master = StartMaster(flags);
+ ASSERT_SOME(master);
+
+ detector.appoint(master.get()->pid);
+
+ AWAIT_READY(shutdownMessage);
+}
+
+
+// This test verifies that duplicate agent registration attempts are
+// ignored when the ongoing registration is pending in the authorizer.
+TEST_F(MasterAuthorizationTest, RetryRegisterAgent)
+{
+ // Use a paused clock to control agent registration retries.
+ Clock::pause();
+
+ MockAuthorizer authorizer;
+ Try<Owned<cluster::Master>> master = StartMaster(&authorizer);
+ ASSERT_SOME(master);
+
+ // Return a pending future from authorizer.
+ Future<Nothing> authorize;
+ Promise<bool> promise;
+
+ // Expect the authorizer to be called only once, i.e.,
+ // the retry is ignored.
+ EXPECT_CALL(authorizer, authorized(_))
+ .WillOnce(DoAll(FutureSatisfy(&authorize),
+ Return(promise.future())));
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get());
+ ASSERT_SOME(slave);
+
+ // Trigger the first registration attempt (with authentication).
+ Clock::advance(slave::DEFAULT_REGISTRATION_BACKOFF_FACTOR);
+
+ // Wait until the authorization is in progress.
+ AWAIT_READY(authorize);
+
+ // Advance to trigger the second registration attempt.
+ Clock::advance(slave::REGISTER_RETRY_INTERVAL_MAX);
+
+ // Settle to make sure the second registration attempt is received
+ // by the master. We can verify that it's ignored if the EXPECT_CALL
+ // above doesn't oversaturate.
+ Clock::settle();
+
+ Future<Message> slaveRegisteredMessage =
+ FUTURE_MESSAGE(Eq(SlaveRegisteredMessage().GetTypeName()), _, _);
+
+ // Now authorize the agent and verify it's registered.
+ promise.set(true);
+
+ AWAIT_READY(slaveRegisteredMessage);
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {
[4/6] mesos git commit: Fixed a test in MasterAuthorizationTest.
Posted by ya...@apache.org.
Fixed a test in MasterAuthorizationTest.
- `MasterAuthorizationTest.PendingExecutorInfoDiffersOnDifferentSlaves`
needs used to assume the mock authorizer is only called for tasks
authorization but with the new `regsiter_agents` ACL this is no
longer true.
Review: https://reviews.apache.org/r/57731
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/70ce5160
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/70ce5160
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/70ce5160
Branch: refs/heads/master
Commit: 70ce51606731de0af03678f2bd3ddaf3d9d3627f
Parents: 13d0591
Author: Jiang Yan Xu <xu...@apple.com>
Authored: Fri Mar 17 01:09:40 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 14:55:13 2017 -0700
----------------------------------------------------------------------
src/tests/master_authorization_tests.cpp | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/70ce5160/src/tests/master_authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_authorization_tests.cpp b/src/tests/master_authorization_tests.cpp
index a646768..e4233c1 100644
--- a/src/tests/master_authorization_tests.cpp
+++ b/src/tests/master_authorization_tests.cpp
@@ -884,11 +884,17 @@ TEST_F(MasterAuthorizationTest, PendingExecutorInfoDiffersOnDifferentSlaves)
offers1.get()[0], executor1.command().value(), executor1.executor_id());
// Return a pending future from authorizer.
+ // Note that we retire this expectation after its use because
+ // the authorizer will next be called when `slave2` registers and
+ // this expectation would be hit again (and be oversaturated) if
+ // we don't retire. New expectations on `authorizer` will be set
+ // after `slave2` is registered.
Future<Nothing> authorize;
Promise<bool> promise;
EXPECT_CALL(authorizer, authorized(_))
.WillOnce(DoAll(FutureSatisfy(&authorize),
- Return(promise.future())));
+ Return(promise.future())))
+ .RetiresOnSaturation();
driver.launchTasks(offers1.get()[0].id(), {task1});
[5/6] mesos git commit: Logged when an agent (re-)registration
request is received.
Posted by ya...@apache.org.
Logged when an agent (re-)registration request is received.
- This log happens after the master has done initial validation but
before authorization, which is consistent with the (re-)register
framework code paths.
Review: https://reviews.apache.org/r/58676
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9fb6a11a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9fb6a11a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9fb6a11a
Branch: refs/heads/master
Commit: 9fb6a11a5dc7645ebb5eb26a58f305dc81fe7b4d
Parents: 70ce516
Author: Jiang Yan Xu <xu...@apple.com>
Authored: Fri Apr 21 22:21:18 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 14:55:13 2017 -0700
----------------------------------------------------------------------
src/master/master.cpp | 7 +++++++
1 file changed, 7 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/9fb6a11a/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 2be4056..97df727 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5420,6 +5420,9 @@ void Master::registerSlave(
return;
}
+ LOG(INFO) << "Received register agent message from "
+ << from << " (" << slaveInfo.hostname() << ")";
+
slaves.registering.insert(from);
// Note that the principal may be empty if authentication is not
@@ -5678,6 +5681,10 @@ void Master::reregisterSlave(
return;
}
+ LOG(INFO) << "Received re-register agent message from agent "
+ << slaveInfo.id() << " at " << from << " ("
+ << slaveInfo.hostname() << ")";
+
slaves.reregistering.insert(slaveInfo.id());
// Note that the principal may be empty if authentication is not
[3/6] mesos git commit: Fixed example tests which broke due to the
new `register_agents` ACL.
Posted by ya...@apache.org.
Fixed example tests which broke due to the new `register_agents` ACL.
- With the new `register_agents` ACL, `permissive == false` would lead
to the agent unable to register unless explicitly allowed.
Review: https://reviews.apache.org/r/57730
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/13d0591c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/13d0591c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/13d0591c
Branch: refs/heads/master
Commit: 13d0591cddca4a5c9e892e1d84dcde20ddc8b6c3
Parents: f77be68
Author: Jiang Yan Xu <xu...@apple.com>
Authored: Thu Mar 16 17:55:20 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 14:55:13 2017 -0700
----------------------------------------------------------------------
src/tests/script.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/13d0591c/src/tests/script.cpp
----------------------------------------------------------------------
diff --git a/src/tests/script.cpp b/src/tests/script.cpp
index 3b68b84..791d331 100644
--- a/src/tests/script.cpp
+++ b/src/tests/script.cpp
@@ -158,6 +158,14 @@ void execute(const string& script)
register_->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
register_->mutable_roles()->add_values("*");
+ // Allow agents with any principal or no principal to register.
+ // Currently the agents in the example tests don't have authentication
+ // enabled so the agent's principal would be none.
+ // TODO(xujyan): Enable agent authN and authZ by default in example tests.
+ mesos::ACL::RegisterAgent* registerAgent = acls.add_register_agents();
+ registerAgent->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+ registerAgent->mutable_agent()->set_type(mesos::ACL::Entity::ANY);
+
const string& aclsPath = path::join(directory.get(), "acls");
CHECK_SOME(os::write(aclsPath, stringify(JSON::protobuf(acls))))
[6/6] mesos git commit: Added `register_agents` to authorization.md.
Posted by ya...@apache.org.
Added `register_agents` to authorization.md.
Review: https://reviews.apache.org/r/57710
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f77be687
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f77be687
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f77be687
Branch: refs/heads/master
Commit: f77be6875b3d0006caf7029642a4f145dc42857f
Parents: 29fc2df
Author: Jiang Yan Xu <xu...@apple.com>
Authored: Thu Mar 16 15:49:05 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Apr 28 14:55:13 2017 -0700
----------------------------------------------------------------------
docs/authorization.md | 8 ++++++++
1 file changed, 8 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f77be687/docs/authorization.md
----------------------------------------------------------------------
diff --git a/docs/authorization.md b/docs/authorization.md
index 42337e7..d94f0f9 100644
--- a/docs/authorization.md
+++ b/docs/authorization.md
@@ -273,6 +273,14 @@ entries, each representing an authorizable action:
</td>
<td>Access Mesos logs.</td>
</tr>
+<tr>
+ <td><code>register_agents</code></td>
+ <td>Agent principal.</td>
+ <td>Implicitly given. A user should only use types ANY and NONE to allow/deny
+ agent (re-)registration.
+ </td>
+ <td>(Re-)registration of agents.</td>
+</tr>
</tbody>
</table>