You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/05/04 01:18:31 UTC

[08/13] mesos git commit: Added new authorization for `ResizeVolume`.

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 <zh...@gmail.com>
Authored: Thu May 3 17:04:50 2018 -0700
Committer: Chun-Hung Hsiao <ch...@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,