Added new authorization for `ResizeVolume`. The new authorization action is modelled after `CreateVolume`, and will be shared by both `GROW_VOLUME` and `SHRINK_VOLUME` operations and corresponding operator APIs.
Review: https://reviews.apache.org/r/66531/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9656329c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9656329c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9656329c Branch: refs/heads/master Commit: 9656329cb313316e7e82ba5a73fae6ca0997e3af Parents: 274f2e6 Author: Zhitao Li <zhitaoli...@gmail.com> Authored: Thu May 3 17:04:50 2018 -0700 Committer: Chun-Hung Hsiao <chhs...@mesosphere.io> Committed: Thu May 3 17:04:50 2018 -0700 ---------------------------------------------------------------------- include/mesos/authorizer/acls.proto | 10 +++ include/mesos/authorizer/authorizer.proto | 3 + src/authorizer/local/authorizer.cpp | 9 +++ src/master/master.cpp | 103 ++++++++++++++++++++++++- src/master/master.hpp | 23 ++++++ 5 files changed, 144 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/9656329c/include/mesos/authorizer/acls.proto ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/acls.proto b/include/mesos/authorizer/acls.proto index 8ef3321..e488993 100644 --- a/include/mesos/authorizer/acls.proto +++ b/include/mesos/authorizer/acls.proto @@ -114,6 +114,15 @@ message ACL { required Entity creator_principals = 2; } + // Specifies which roles a principal can resize volumes for. + message ResizeVolume { + // Subjects: Framework principal or Operator username. + required Entity principals = 1; + + // Objects: The principal(s) can resize volumes for these roles. + required Entity roles = 2; + } + // Which principals are authorized to see quotas for the given roles. message GetQuota { // Subjects: Operator username. @@ -589,4 +598,5 @@ message ACLs { repeated ACL.ViewStandaloneContainer view_standalone_containers = 46; repeated ACL.ModifyResourceProviderConfig modify_resource_provider_configs = 45; repeated ACL.PruneImages prune_images = 47; + repeated ACL.ResizeVolume resize_volumes = 48; } http://git-wip-us.apache.org/repos/asf/mesos/blob/9656329c/include/mesos/authorizer/authorizer.proto ---------------------------------------------------------------------- diff --git a/include/mesos/authorizer/authorizer.proto b/include/mesos/authorizer/authorizer.proto index 1508c01..bb1010d 100644 --- a/include/mesos/authorizer/authorizer.proto +++ b/include/mesos/authorizer/authorizer.proto @@ -254,6 +254,9 @@ enum Action { // This action will not fill in any object fields. A principal is either // allowed to prune unused container images or is unauthorized. PRUNE_IMAGES = 41; + + // `RESIZE_VOLUME` will have an object with `Resource` set. + RESIZE_VOLUME = 42; } http://git-wip-us.apache.org/repos/asf/mesos/blob/9656329c/src/authorizer/local/authorizer.cpp ---------------------------------------------------------------------- diff --git a/src/authorizer/local/authorizer.cpp b/src/authorizer/local/authorizer.cpp index c098ba9..61e9ab5 100644 --- a/src/authorizer/local/authorizer.cpp +++ b/src/authorizer/local/authorizer.cpp @@ -417,6 +417,7 @@ public: break; case authorization::CREATE_VOLUME: + case authorization::RESIZE_VOLUME: case authorization::GET_QUOTA: case authorization::RESERVE_RESOURCES: case authorization::UPDATE_QUOTA: @@ -594,6 +595,7 @@ public: } else { switch (action_) { case authorization::CREATE_VOLUME: + case authorization::RESIZE_VOLUME: case authorization::RESERVE_RESOURCES: { entityObject.set_type(ACL::Entity::SOME); if (object->resource) { @@ -875,6 +877,11 @@ public: createHierarchicalRoleACLs(acls.create_volumes()); break; } + case authorization::RESIZE_VOLUME: { + hierarchicalRoleACLs = + createHierarchicalRoleACLs(acls.resize_volumes()); + break; + } case authorization::RESERVE_RESOURCES: { hierarchicalRoleACLs = createHierarchicalRoleACLs(acls.reserve_resources()); @@ -1113,6 +1120,7 @@ public: return getNestedContainerObjectApprover(subject, action); } case authorization::CREATE_VOLUME: + case authorization::RESIZE_VOLUME: case authorization::RESERVE_RESOURCES: case authorization::UPDATE_WEIGHT: case authorization::VIEW_ROLE: @@ -1520,6 +1528,7 @@ private: return acls_; case authorization::REGISTER_FRAMEWORK: case authorization::CREATE_VOLUME: + case authorization::RESIZE_VOLUME: case authorization::RESERVE_RESOURCES: case authorization::UPDATE_WEIGHT: case authorization::VIEW_ROLE: http://git-wip-us.apache.org/repos/asf/mesos/blob/9656329c/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index b9946b5..c117b8c 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -3801,6 +3801,43 @@ Future<bool> Master::authorizeDestroyVolume( } +Future<bool> Master::authorizeResizeVolume( + const Resource& volume, + const Option<Principal>& principal) +{ + if (authorizer.isNone()) { + return true; // Authorization is disabled. + } + + authorization::Request request; + request.set_action(authorization::RESIZE_VOLUME); + + Option<authorization::Subject> subject = createSubject(principal); + if (subject.isSome()) { + request.mutable_subject()->CopyFrom(subject.get()); + } + + request.mutable_object()->mutable_resource()->CopyFrom(volume); + + string role; + if (volume.reservations_size() > 0) { + // Check for role in the "post-reservation-refinement" format. + role = volume.reservations().rbegin()->role(); + } else { + // Check for role in the "pre-reservation-refinement" format. + role = volume.role(); + } + + request.mutable_object()->set_value(role); + + LOG(INFO) << "Authorizing principal '" + << (principal.isSome() ? stringify(principal.get()) : "ANY") + << "' to resize volume '" << volume << "'"; + + return authorizer.get()->authorized(request); +} + + Future<bool> Master::authorizeSlave( const SlaveInfo& slaveInfo, const Option<Principal>& principal) @@ -4410,12 +4447,26 @@ void Master::accept( } case Offer::Operation::GROW_VOLUME: { - // TODO(zhitao): Add support for authorization of grow volume. + Option<Principal> principal = framework->info.has_principal() + ? Principal(framework->info.principal()) + : Option<Principal>::none(); + + futures.push_back( + authorizeResizeVolume( + operation.grow_volume().volume(), principal)); + break; } case Offer::Operation::SHRINK_VOLUME: { - // TODO(zhitao): Add support for authorization of shrink volume. + Option<Principal> principal = framework->info.has_principal() + ? Principal(framework->info.principal()) + : Option<Principal>::none(); + + futures.push_back( + authorizeResizeVolume( + operation.shrink_volume().volume(), principal)); + break; } @@ -4907,7 +4958,29 @@ void Master::_accept( } case Offer::Operation::GROW_VOLUME: { - // TODO(zhitao): Authorize GROW_VOLUME from `authorizations`. + Future<bool> authorization = authorizations.front(); + authorizations.pop_front(); + + CHECK(!authorization.isDiscarded()); + + if (authorization.isFailed()) { + // TODO(greggomann): We may want to retry this failed authorization + // request rather than dropping it immediately. + drop(framework, + operation, + "Authorization of principal '" + framework->info.principal() + + "' to grow a volume failed: " + + authorization.failure()); + + continue; + } else if (!authorization.get()) { + drop(framework, + operation, + "Not authorized to grow a volume as '" + + framework->info.principal() + "'"); + + continue; + } // Make sure this grow volume operation is valid. Option<Error> error = validation::operation::validate( @@ -4967,7 +5040,29 @@ void Master::_accept( } case Offer::Operation::SHRINK_VOLUME: { - // TODO(zhitao): Authorize SHRINK_VOLUME from `authorizations`. + Future<bool> authorization = authorizations.front(); + authorizations.pop_front(); + + CHECK(!authorization.isDiscarded()); + + if (authorization.isFailed()) { + // TODO(greggomann): We may want to retry this failed authorization + // request rather than dropping it immediately. + drop(framework, + operation, + "Authorization of principal '" + framework->info.principal() + + "' to shrink a volume failed: " + + authorization.failure()); + + continue; + } else if (!authorization.get()) { + drop(framework, + operation, + "Not authorized to shrink a volume as '" + + framework->info.principal() + "'"); + + continue; + } // Make sure this shrink volume operation is valid. Option<Error> error = validation::operation::validate( http://git-wip-us.apache.org/repos/asf/mesos/blob/9656329c/src/master/master.hpp ---------------------------------------------------------------------- diff --git a/src/master/master.hpp b/src/master/master.hpp index c30cf08..270f60a 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -873,6 +873,29 @@ protected: const Offer::Operation::Destroy& destroy, const Option<process::http::authentication::Principal>& principal); + /** + * Authorizes resize of a volume triggered by either `GROW_VOLUME` or + * `SHRINK_VOLUME` operations. + * + * Returns whether the triggering operation is authorized with the provided + * principal. This function is used for authorization of operations + * originating both from frameworks and operators. Note that operations may be + * validated AFTER authorization, so it's possible that the operation could be + * malformed. + * + * @param volume The volume being resized. + * @param principal An `Option` containing the principal attempting this + * operation. + * + * @return A `Future` containing a boolean value representing the success or + * failure of this authorization. A failed `Future` implies that + * validation of the operation did not succeed. + */ + process::Future<bool> authorizeResizeVolume( + const Resource& volume, + const Option<process::http::authentication::Principal>& principal); + + // Determine if a new executor needs to be launched. bool isLaunchExecutor ( const ExecutorID& executorId,