This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new e5342fd Moved creating authorization Object out of
Master::authorizeSlave.
e5342fd is described below
commit e5342fd00405551022984bf8d4e0b541e369c2e1
Author: Andrei Sekretenko <[email protected]>
AuthorDate: Tue Jan 7 14:00:43 2020 -0500
Moved creating authorization Object out of Master::authorizeSlave.
Review: https://reviews.apache.org/r/71865/
---
src/master/authorization.cpp | 24 +++++++++++++++++++
src/master/authorization.hpp | 4 ++++
src/master/master.cpp | 57 ++++----------------------------------------
src/master/master.hpp | 8 -------
4 files changed, 33 insertions(+), 60 deletions(-)
diff --git a/src/master/authorization.cpp b/src/master/authorization.cpp
index d97288a..ea10175 100644
--- a/src/master/authorization.cpp
+++ b/src/master/authorization.cpp
@@ -77,6 +77,30 @@ ActionObject ActionObject::frameworkRegistration(
}
+vector<ActionObject> ActionObject::agentRegistration(
+ const SlaveInfo& slaveInfo)
+{
+ vector<ActionObject> result;
+
+ if (!Resources(slaveInfo.resources()).reserved().empty()) {
+ // If static reservations exist, we also need to authorize them.
+ //
+ // NOTE: We don't look at dynamic reservations in checkpointed
+ // resources because they should have gone through authorization
+ // against the framework / operator's principal when they were
+ // created. In constrast, static reservations are initiated by the
+ // agent's principal and authorizing them helps prevent agents from
+ // advertising reserved resources of arbitrary roles.
+ Offer::Operation::Reserve reserve;
+ *reserve.mutable_resources() = slaveInfo.resources();
+ result = ActionObject::reserve(reserve);
+ }
+
+ result.push_back(ActionObject(authorization::REGISTER_AGENT, None()));
+ return result;
+}
+
+
// Returns effective reservation role of resource for the purpose
// of authorizing an operation; that is, the the role of the most
// refined reservation if the resource is reserved.
diff --git a/src/master/authorization.hpp b/src/master/authorization.hpp
index 58bf214..f28b29f 100644
--- a/src/master/authorization.hpp
+++ b/src/master/authorization.hpp
@@ -48,6 +48,10 @@ public:
static ActionObject frameworkRegistration(
const FrameworkInfo& frameworkInfo);
+ // Returns action-object pair(s) for authorizing agent re(registration).
+ static std::vector<ActionObject> agentRegistration(
+ const SlaveInfo& slaveInfo);
+
// Methods that return action-object pair(s) for authorizing
// offer operations other than LAUNCH/LAUNCH_GROUP.
//
diff --git a/src/master/master.cpp b/src/master/master.cpp
index c9ae0cd..23f747c 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3754,54 +3754,6 @@ Future<bool> Master::authorize(
}
-Future<bool> Master::authorizeSlave(
- const SlaveInfo& slaveInfo,
- const Option<Principal>& principal)
-{
- if (authorizer.isNone()) {
- return true;
- }
-
- vector<Future<bool>> authorizations;
-
- // First authorize whether the agent can register.
- LOG(INFO) << "Authorizing agent providing resources "
- << "'" << stringify(Resources(slaveInfo.resources())) << "' "
- << (principal.isSome()
- ? "with principal '" + stringify(principal.get()) + "'"
- : "without a principal");
-
- authorization::Request request;
- request.set_action(authorization::REGISTER_AGENT);
-
- Option<authorization::Subject> subject = createSubject(principal);
- if (subject.isSome()) {
- request.mutable_subject()->CopyFrom(subject.get());
- }
-
- // No need to set the request's object as it is implicitly set to
- // ANY by the authorizer.
- authorizations.push_back(authorizer.get()->authorized(request));
-
- // Next, if static reservations exist, also authorize them.
- //
- // NOTE: We don't look at dynamic reservations in checkpointed
- // resources because they should have gone through authorization
- // against the framework / operator's principal when they were
- // created. In constrast, static reservations are initiated by the
- // agent's principal and authorizing them helps prevent agents from
- // advertising reserved resources of arbitrary roles.
- if (!Resources(slaveInfo.resources()).reserved().empty()) {
- Offer::Operation::Reserve reserve;
- reserve.mutable_resources()->CopyFrom(slaveInfo.resources());
- authorizations.push_back(
- authorize(principal, ActionObject::reserve(reserve)));
- }
-
- return authorization::collectAuthorizations(authorizations);
-}
-
-
bool Master::isLaunchExecutor(
const ExecutorID& executorId,
Framework* framework,
@@ -6696,8 +6648,8 @@ void Master::registerSlave(
// Calling the `onAny` continuation below separately so we can move
// `registerSlaveMessage` without it being evaluated before it's used
// by `authorizeSlave`.
- Future<bool> authorization =
- authorizeSlave(registerSlaveMessage.slave(), principal);
+ Future<bool> authorization = authorize(
+ principal,
ActionObject::agentRegistration(registerSlaveMessage.slave()));
authorization
.onAny(defer(self(),
@@ -7050,8 +7002,9 @@ void Master::reregisterSlave(
// Calling the `onAny` continuation below separately so we can move
// `reregisterSlaveMessage` without it being evaluated before it's used
// by `authorizeSlave`.
- Future<bool> authorization =
- authorizeSlave(reregisterSlaveMessage.slave(), principal);
+ Future<bool> authorization = authorize(
+ principal,
+ ActionObject::agentRegistration(reregisterSlaveMessage.slave()));
authorization
.onAny(defer(self(),
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 0ac3332..c9d417c 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -756,19 +756,11 @@ protected:
const Option<process::http::authentication::Principal>& principal,
std::vector<authorization::ActionObject>&& actionObjects);
- // TODO(asekretenko): get rid of action-specific authorizeSomething()
methods.
-
// Returns whether the framework is authorized.
// Returns failure for transient authorization failures.
process::Future<bool> authorizeFramework(
const FrameworkInfo& frameworkInfo);
- // Returns whether the principal is authorized to (re-)register an agent
- // and whether the `SlaveInfo` is authorized.
- process::Future<bool> authorizeSlave(
- const SlaveInfo& slaveInfo,
- const Option<process::http::authentication::Principal>& principal);
-
// Determine if a new executor needs to be launched.
bool isLaunchExecutor (
const ExecutorID& executorId,
