Repository: mesos
Updated Branches:
  refs/heads/master f7d91ba2f -> a28917f18


Used UPDATE_QUOTA_WITH_ROLE for both quota set and remove.

To consolidate authorization actions for quota, we introduce a new
authorization action `UPDATE_QUOTA_WITH_ROLE` and corresponding
ACL. They new action and ACL should be used instead of now deprecated
`SET_QUOTA_WITH_ROLE` and `DESTROY_QUOTA_WITH_PRINCIPAL`. Until the
end of the deprecation cycle, we will be using both combinations by
querying the authorizer twice.

Review: https://reviews.apache.org/r/47399/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a28917f1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a28917f1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a28917f1

Branch: refs/heads/master
Commit: a28917f188183a4be1c974fc61ef20797cf255af
Parents: f7d91ba
Author: Zhitao Li <zhitaoli...@gmail.com>
Authored: Wed May 18 16:17:11 2016 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Wed May 18 18:21:40 2016 +0200

----------------------------------------------------------------------
 include/mesos/authorizer/acls.proto       |  24 +-
 include/mesos/authorizer/authorizer.proto |  10 +-
 src/authorizer/local/authorizer.cpp       |  63 ++++++
 src/authorizer/local/authorizer.hpp       |   3 +
 src/master/master.hpp                     |   4 +
 src/master/quota_handler.cpp              |  69 +++++-
 src/tests/authorization_tests.cpp         | 156 +++++++++++++
 src/tests/master_quota_tests.cpp          | 300 ++++++++++++++++++++++++-
 8 files changed, 604 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/include/mesos/authorizer/acls.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/acls.proto 
b/include/mesos/authorizer/acls.proto
index e0c9524..b178f53 100644
--- a/include/mesos/authorizer/acls.proto
+++ b/include/mesos/authorizer/acls.proto
@@ -129,7 +129,19 @@ message ACL {
     optional Entity roles = 2;
   }
 
+  // Which principals are authorized to update quotas for the given roles.
+  message UpdateQuota {
+    // Subjects: Operator username.
+    required Entity principals = 1;
+
+    // Objects: The list of roles whose quotas can be updated.
+    optional Entity roles = 2;
+  }
+
   // Which principals are authorized to set quotas for given roles.
+  //
+  // TODO(zhitao): Remove this message at the end of the deprecation cycle
+  // which started with 0.29. It will be fully replaced by `UpdateQuota`.
   message SetQuota {
     // Subjects: Operator username.
     required Entity principals = 1;
@@ -139,6 +151,9 @@ message ACL {
   }
 
   // Which principals can remove quotas set by which other principals.
+  //
+  // TODO(zhitao): Remove this message at the end of the deprecation cycle
+  // which started with 0.29. It will be fully replaced by `UpdateQuota`.
   message RemoveQuota {
     // Subjects: Operator username.
     required Entity principals = 1;
@@ -198,8 +213,13 @@ message ACLs {
   repeated ACL.CreateVolume create_volumes = 7;
   repeated ACL.DestroyVolume destroy_volumes = 8;
   repeated ACL.GetQuota get_quotas = 14;
-  repeated ACL.SetQuota set_quotas = 9;
-  repeated ACL.RemoveQuota remove_quotas = 10;
+  repeated ACL.UpdateQuota update_quotas = 15;
+
+  // TODO(zhitao): Remove the following two fields at the end
+  // of the deprecation cycle which started with 0.29.
+  repeated ACL.SetQuota set_quotas = 9 [deprecated = true];
+  repeated ACL.RemoveQuota remove_quotas = 10 [deprecated = true];
+
   repeated ACL.TeardownFramework teardown_frameworks = 11;
   repeated ACL.UpdateWeights update_weights = 12;
   repeated ACL.GetEndpoint get_endpoints = 13;

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/include/mesos/authorizer/authorizer.proto
----------------------------------------------------------------------
diff --git a/include/mesos/authorizer/authorizer.proto 
b/include/mesos/authorizer/authorizer.proto
index 7d4aa32..b0d9f79 100644
--- a/include/mesos/authorizer/authorizer.proto
+++ b/include/mesos/authorizer/authorizer.proto
@@ -56,8 +56,14 @@ enum Action {
   CREATE_VOLUME_WITH_ROLE = 6;
   DESTROY_VOLUME_WITH_PRINCIPAL = 7;
   GET_QUOTA_WITH_ROLE = 12;
-  SET_QUOTA_WITH_ROLE = 8;
-  DESTROY_QUOTA_WITH_PRINCIPAL = 9;
+  UPDATE_QUOTA_WITH_ROLE = 13;
+
+  // TODO(zhitao): Remove the following two actions at the end of
+  // the deprecation cycle which started with 0.29. They will be
+  // fully replaced by `UPDATE_QUOTA_WITH_ROLE`.
+  SET_QUOTA_WITH_ROLE = 8 [deprecated = true];
+  DESTROY_QUOTA_WITH_PRINCIPAL = 9 [deprecated = true];
+
   UPDATE_WEIGHTS_WITH_ROLE = 10;
   GET_ENDPOINT_WITH_PATH = 11;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/src/authorizer/local/authorizer.cpp
----------------------------------------------------------------------
diff --git a/src/authorizer/local/authorizer.cpp 
b/src/authorizer/local/authorizer.cpp
index bd47f4f..dc53bc4 100644
--- a/src/authorizer/local/authorizer.cpp
+++ b/src/authorizer/local/authorizer.cpp
@@ -30,6 +30,7 @@
 #include <process/protobuf.hpp>
 
 #include <stout/foreach.hpp>
+#include <stout/none.hpp>
 #include <stout/option.hpp>
 #include <stout/path.hpp>
 #include <stout/protobuf.hpp>
@@ -62,6 +63,14 @@ public:
 
   virtual void initialize()
   {
+    // TODO(zhitao): Remove the following log warning at the end of the
+    // deprecation cycle which started with 0.29.
+    if (acls.set_quotas_size() > 0 ||
+        acls.remove_quotas_size() > 0) {
+      LOG(WARNING) << "SetQuota and RemoveQuota ACLs are deprecacted; "
+                   << "please use UpdateQuota";
+    }
+
     // TODO(arojas): Remove the following two if blocks once
     // ShutdownFramework reaches the end of deprecation cycle
     // which started with 0.27.0.
@@ -183,7 +192,38 @@ public:
 
         return authorized(request, acls_);
         break;
+      case authorization::UPDATE_QUOTA_WITH_ROLE:
+        // Deprecation case: If `update_quotas` is empty but
+        // `set_quotas` or `remove_quotas` are not, "skip"
+        // authorization here under assumption that the caller,
+        // i.e., `QuotaHandler`, checks `set_quotas`/`remove_quotas`.
+        //
+        // TODO(zhitao): Remove this special case at the end
+        // of the deprecation cycle which started with 0.29.
+        if (acls.set_quotas_size() > 0 || acls.remove_quotas_size() > 0) {
+          CHECK(acls.update_quotas_size() == 0);
+          return true;
+        }
+
+        foreach (const ACL::UpdateQuota& acl, acls.update_quotas()) {
+          GenericACL acl_;
+          acl_.subjects = acl.principals();
+          acl_.objects = acl.roles();
+
+          acls_.push_back(acl_);
+        }
+
+        return authorized(request, acls_);
+        break;
+
+      // TODO(zhitao): Remove the following two cases at the end
+      // of the deprecation cycle which started with 0.29.
       case authorization::SET_QUOTA_WITH_ROLE:
+        if (acls.update_quotas_size() > 0) {
+          CHECK(acls.set_quotas_size() == 0);
+          return true;
+        }
+
         foreach (const ACL::SetQuota& acl, acls.set_quotas()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -195,6 +235,11 @@ public:
         return authorized(request, acls_);
         break;
       case authorization::DESTROY_QUOTA_WITH_PRINCIPAL:
+        if (acls.update_quotas_size() > 0) {
+          CHECK(acls.remove_quotas_size() == 0);
+          return true;
+        }
+
         foreach (const ACL::RemoveQuota& acl, acls.remove_quotas()) {
           GenericACL acl_;
           acl_.subjects = acl.principals();
@@ -205,6 +250,7 @@ public:
 
         return authorized(request, acls_);
         break;
+
       case authorization::UPDATE_WEIGHTS_WITH_ROLE:
         foreach (const ACL::UpdateWeights& acl, acls.update_weights()) {
           GenericACL acl_;
@@ -381,6 +427,11 @@ private:
 
 Try<Authorizer*> LocalAuthorizer::create(const ACLs& acls)
 {
+  Option<Error> validationError = validate(acls);
+  if (validationError.isSome()) {
+    return validationError.get();
+  }
+
   Authorizer* local = new LocalAuthorizer(acls);
 
   return local;
@@ -410,6 +461,18 @@ Try<Authorizer*> LocalAuthorizer::create(const Parameters& 
parameters)
 }
 
 
+Option<Error> LocalAuthorizer::validate(const ACLs& acls)
+{
+  if (acls.update_quotas_size() > 0 &&
+      (acls.set_quotas_size() > 0 || acls.remove_quotas_size() > 0)) {
+    return Error("acls.update_quotas cannot be used "
+                 "together with deprecated set_quotas/remove_quotas!");
+  }
+
+  return None();
+}
+
+
 LocalAuthorizer::LocalAuthorizer(const ACLs& acls)
     : process(new LocalAuthorizerProcess(acls))
 {

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/src/authorizer/local/authorizer.hpp
----------------------------------------------------------------------
diff --git a/src/authorizer/local/authorizer.hpp 
b/src/authorizer/local/authorizer.hpp
index d15d3a6..6138845 100644
--- a/src/authorizer/local/authorizer.hpp
+++ b/src/authorizer/local/authorizer.hpp
@@ -22,6 +22,7 @@
 #include <process/future.hpp>
 #include <process/once.hpp>
 
+#include <stout/error.hpp>
 #include <stout/nothing.hpp>
 #include <stout/option.hpp>
 #include <stout/try.hpp>
@@ -67,6 +68,8 @@ public:
 private:
   LocalAuthorizer(const ACLs& acls);
 
+  static Option<Error> validate(const ACLs& acls);
+
   LocalAuthorizerProcess* process;
 };
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 22a1538..647b43c 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1003,6 +1003,10 @@ private:
         const Option<std::string>& principal,
         const std::string& role) const;
 
+    process::Future<bool> authorizeUpdateQuota(
+        const Option<std::string>& principal,
+        const std::string& role) const;
+
     process::Future<bool> authorizeSetQuota(
         const Option<std::string>& principal,
         const std::string& role) const;

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/src/master/quota_handler.cpp
----------------------------------------------------------------------
diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index 86a2e17..04639ef 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -335,11 +335,21 @@ Future<http::Response> Master::QuotaHandler::set(
     quotaInfo.set_principal(principal.get());
   }
 
-  return authorizeSetQuota(principal, quotaInfo.role())
-    .then(defer(master->self(), [=](bool authorized) -> Future<http::Response> 
{
-      if (!authorized) {
-        return Forbidden();
-      }
+  // TODO(zhitao): Remove the deprecated action at the end
+  // of the deprecation cycle which started with 0.29.
+  list<Future<bool>> authorizeRequests = {
+    authorizeSetQuota(principal, quotaInfo.role()),
+    authorizeUpdateQuota(principal, quotaInfo.role())};
+
+  return process::collect(authorizeRequests)
+    .then(defer(
+        master->self(),
+        [=](const list<bool>& authorizeResults) -> Future<http::Response> {
+          foreach (const bool& authorized, authorizeResults) {
+            if (!authorized) {
+              return Forbidden();
+            }
+          }
 
       return _set(quotaInfo, forced);
     }));
@@ -444,11 +454,21 @@ Future<http::Response> Master::QuotaHandler::remove(
     ? master->quotas[role].info.principal()
     : Option<string>::none();
 
-  return authorizeRemoveQuota(principal, quota_principal)
-    .then(defer(master->self(), [=](bool authorized) -> Future<http::Response> 
{
-      if (!authorized) {
-        return Forbidden();
-      }
+  // TODO(zhitao): Remove the deprecated action at the end
+  // of the deprecation cycle which started with 0.29.
+  list<Future<bool>> authorizeRequests = {
+    authorizeRemoveQuota(principal, quota_principal),
+    authorizeUpdateQuota(principal, role)};
+
+  return process::collect(authorizeRequests)
+    .then(defer(
+        master->self(),
+        [=](const list<bool>& authorizeResults) -> Future<http::Response> {
+          foreach (const bool& authorized, authorizeResults) {
+            if (!authorized) {
+              return Forbidden();
+            }
+          }
 
       return _remove(role);
     }));
@@ -504,6 +524,33 @@ Future<bool> Master::QuotaHandler::authorizeGetQuota(
 }
 
 
+Future<bool> Master::QuotaHandler::authorizeUpdateQuota(
+    const Option<string>& principal,
+    const string& role) const
+{
+  if (master->authorizer.isNone()) {
+    return true;
+  }
+
+  LOG(INFO) << "Authorizing principal '"
+            << (principal.isSome() ? principal.get() : "ANY")
+            << "' to update quota for role '" << role << "'";
+
+  authorization::Request request;
+  request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+
+  if (principal.isSome()) {
+    request.mutable_subject()->set_value(principal.get());
+  }
+
+  request.mutable_object()->set_value(role);
+
+  return master->authorizer.get()->authorized(request);
+}
+
+
+// TODO(zhitao): Remove this function at the end of the
+// deprecation cycle which started with 0.29.
 Future<bool> Master::QuotaHandler::authorizeSetQuota(
     const Option<string>& principal,
     const string& role) const
@@ -529,6 +576,8 @@ Future<bool> Master::QuotaHandler::authorizeSetQuota(
 }
 
 
+// TODO(zhitao): Remove this function at the end of the
+// deprecation cycle which started with 0.29.
 Future<bool> Master::QuotaHandler::authorizeRemoveQuota(
     const Option<string>& requestPrincipal,
     const Option<string>& quotaPrincipal) const

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/src/tests/authorization_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/authorization_tests.cpp 
b/src/tests/authorization_tests.cpp
index f50ac69..54bfb46 100644
--- a/src/tests/authorization_tests.cpp
+++ b/src/tests/authorization_tests.cpp
@@ -957,7 +957,161 @@ TYPED_TEST(AuthorizationTest, DestroyVolume)
 }
 
 
+// This tests the authorization of requests to update quotas.
+TYPED_TEST(AuthorizationTest, UpdateQuota)
+{
+  ACLs acls;
+
+  {
+    // "foo" principal can update quotas for all roles.
+    mesos::ACL::UpdateQuota* acl = acls.add_update_quotas();
+    acl->mutable_principals()->add_values("foo");
+    acl->mutable_roles()->set_type(mesos::ACL::Entity::ANY);
+  }
+
+  {
+    // "bar" principal can update quotas for "dev" role.
+    mesos::ACL::UpdateQuota* acl = acls.add_update_quotas();
+    acl->mutable_principals()->add_values("bar");
+    acl->mutable_roles()->add_values("dev");
+  }
+
+  {
+    // Anyone can update quotas for "test" role.
+    mesos::ACL::UpdateQuota* acl = acls.add_update_quotas();
+    acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+    acl->mutable_roles()->add_values("test");
+  }
+
+  {
+    // No other principal can update quotas.
+    mesos::ACL::UpdateQuota* acl = acls.add_update_quotas();
+    acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+    acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE);
+  }
+
+  // Create an `Authorizer` with the ACLs.
+  Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+  ASSERT_SOME(create);
+  Owned<Authorizer> authorizer(create.get());
+
+  // Principal "foo" can update quota for all roles, so requests 1 and 2 will
+  // pass.
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_subject()->set_value("foo");
+    AWAIT_EXPECT_TRUE(authorizer.get()->authorized(request));
+  }
+
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_subject()->set_value("foo");
+    request.mutable_object()->set_value("prod");
+    AWAIT_EXPECT_TRUE(authorizer.get()->authorized(request));
+  }
+
+  // Principal "bar" can update quotas for role "dev", so this will pass.
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_subject()->set_value("bar");
+    request.mutable_object()->set_value("dev");
+    AWAIT_EXPECT_TRUE(authorizer.get()->authorized(request));
+  }
+
+  // Principal "bar" can only update quotas for role "dev",
+  // so requests 4 and 5 will fail.
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_subject()->set_value("bar");
+    request.mutable_object()->set_value("prod");
+    AWAIT_EXPECT_FALSE(authorizer.get()->authorized(request));
+  }
+
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_subject()->set_value("bar");
+    AWAIT_EXPECT_FALSE(authorizer.get()->authorized(request));
+  }
+
+  // Anyone can update quotas for role "test", so request 6 will pass.
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_object()->set_value("test");
+    AWAIT_EXPECT_TRUE(authorizer.get()->authorized(request));
+  }
+
+  // Principal "jeff" is not mentioned in the ACLs of the `Authorizer`, so it
+  // will be caught by the final ACL, which provides a default case that denies
+  // access for all other principals. This case will fail.
+  {
+    authorization::Request request;
+    request.set_action(authorization::UPDATE_QUOTA_WITH_ROLE);
+    request.mutable_subject()->set_value("jeff");
+    AWAIT_EXPECT_FALSE(authorizer.get()->authorized(request));
+  }
+}
+
+
+// This tests that update_quotas and set_quotas/remove_quotas
+// cannot be used together.
+// TODO(zhitao): Remove this test case at the end of the deprecation
+// cycle started with 0.29.
+TYPED_TEST(AuthorizationTest, ConflictQuotaACLs) {
+  {
+    ACLs acls;
+
+    {
+      // Add an UpdateQuota ACL.
+      mesos::ACL::UpdateQuota* acl = acls.add_update_quotas();
+      acl->mutable_principals()->add_values("foo");
+      acl->mutable_roles()->set_type(mesos::ACL::Entity::ANY);
+    }
+
+    {
+      // Add a SetQuota ACL.
+      mesos::ACL::SetQuota* acl = acls.add_set_quotas();
+      acl->mutable_principals()->add_values("foo");
+      acl->mutable_roles()->set_type(mesos::ACL::Entity::ANY);
+    }
+
+    // Create an `Authorizer` with the ACLs should error out.
+    Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+    ASSERT_ERROR(create);
+  }
+
+  {
+    ACLs acls;
+
+    {
+      // Add an UpdateQuota ACL.
+      mesos::ACL::UpdateQuota* acl = acls.add_update_quotas();
+      acl->mutable_principals()->add_values("foo");
+      acl->mutable_roles()->set_type(mesos::ACL::Entity::ANY);
+    }
+
+    {
+      // Add a RemoveQuota ACL.
+      mesos::ACL::RemoveQuota* acl = acls.add_remove_quotas();
+      acl->mutable_principals()->add_values("foo");
+      acl->mutable_quota_principals()->set_type(mesos::ACL::Entity::ANY);
+    }
+
+    // Create an `Authorizer` with the ACLs should error out.
+    Try<Authorizer*> create = TypeParam::create(parameterize(acls));
+    ASSERT_ERROR(create);
+  }
+}
+
+
 // This tests the authorization of requests to set quotas.
+// TODO(zhitao): Remove this test case at the end of the deprecation
+// cycle started with 0.29.
 TYPED_TEST(AuthorizationTest, SetQuota)
 {
   ACLs acls;
@@ -1057,6 +1211,8 @@ TYPED_TEST(AuthorizationTest, SetQuota)
 
 
 // This tests the authorization of requests to remove quotas.
+// TODO(zhitao): Remove this test case at the end of the deprecation
+// cycle started with 0.29.
 TYPED_TEST(AuthorizationTest, RemoveQuota)
 {
   ACLs acls;

http://git-wip-us.apache.org/repos/asf/mesos/blob/a28917f1/src/tests/master_quota_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 6e70910..62aaafe 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -1129,8 +1129,8 @@ TEST_F(MasterQuotaTest, UnauthenticatedQuotaRequest)
         createBasicAuthHeaders(credential),
         createRequestBody(ROLE1, quotaResources));
 
-    AWAIT_EXPECT_RESPONSE_STATUS_EQ(
-      Unauthorized({}).status, response) << response.get().body;
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response)
+      << response.get().body;
   }
 
   // The absense of credentials leads to authentication failure as well.
@@ -1141,15 +1141,199 @@ TEST_F(MasterQuotaTest, UnauthenticatedQuotaRequest)
         None(),
         createRequestBody(ROLE1, quotaResources));
 
-    AWAIT_EXPECT_RESPONSE_STATUS_EQ(
-      Unauthorized({}).status, response) << response.get().body;
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Unauthorized({}).status, response)
+      << response.get().body;
   }
 }
 
 
 // Checks that an authorized principal can set and remove quota while
-// unauthorized principals cannot.
-TEST_F(MasterQuotaTest, AuthorizeQuotaRequests)
+// using get_quotas and update_quotas ACLs, while unauthorized user
+// cannot.
+TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequests)
+{
+  TestAllocator<> allocator;
+  EXPECT_CALL(allocator, initialize(_, _, _, _));
+
+  // Setup ACLs so that only the default principal can modify quotas
+  // for `ROLE1` and read status.
+  ACLs acls;
+
+  mesos::ACL::UpdateQuota* acl1 = acls.add_update_quotas();
+  acl1->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+  acl1->mutable_roles()->add_values(ROLE1);
+
+  mesos::ACL::UpdateQuota* acl2 = acls.add_update_quotas();
+  acl2->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+  acl2->mutable_roles()->set_type(mesos::ACL::Entity::NONE);
+
+  mesos::ACL::GetQuota* acl3 = acls.add_get_quotas();
+  acl3->mutable_principals()->add_values(DEFAULT_CREDENTIAL.principal());
+  acl3->mutable_roles()->add_values(ROLE1);
+
+  mesos::ACL::GetQuota* acl4 = acls.add_get_quotas();
+  acl4->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+  acl4->mutable_roles()->set_type(mesos::ACL::Entity::NONE);
+
+  master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.acls = acls;
+
+  Try<Owned<cluster::Master>> master = StartMaster(&allocator, masterFlags);
+  ASSERT_SOME(master);
+
+  // Use the force flag for setting quota that cannot be satisfied in
+  // this empty cluster without any agents.
+  const bool FORCE = true;
+
+  // Try to update quota using a principal that is not the default principal.
+  // This request will fail because only the default principal is authorized
+  // to do that.
+  {
+    // As we don't care about the enforcement of quota but only the
+    // authorization of the quota request we set the force flag in the post
+    // request below to override the capacity heuristic check.
+    Resources quotaResources = Resources::parse("cpus:1;mem:512").get();
+
+    Future<Response> response = process::http::post(
+        master.get()->pid,
+        "quota",
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL_2),
+        createRequestBody(ROLE1, quotaResources, FORCE));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response)
+      << response.get().body;
+  }
+
+  // Set quota using the default principal.
+  {
+    // As we don't care about the enforcement of quota but only the
+    // authorization of the quota request we set the force flag in the post
+    // request below to override the capacity heuristic check.
+    Resources quotaResources = Resources::parse("cpus:1;mem:512").get();
+
+    Future<Quota> quota;
+    EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _))
+      .WillOnce(DoAll(InvokeSetQuota(&allocator),
+                      FutureArg<1>(&quota)));
+
+    Future<Response> response = process::http::post(
+        master.get()->pid,
+        "quota",
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+        createRequestBody(ROLE1, quotaResources, FORCE));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    AWAIT_READY(quota);
+
+    // Extract the principal from `DEFAULT_CREDENTIAL` because `EXPECT_EQ`
+    // does not compile if `DEFAULT_CREDENTIAL.principal()` is used as an
+    // argument.
+    const string principal = DEFAULT_CREDENTIAL.principal();
+
+    EXPECT_EQ(ROLE1, quota.get().info.role());
+    EXPECT_EQ(principal, quota.get().info.principal());
+    EXPECT_EQ(quotaResources, quota.get().info.guarantee());
+  }
+
+  // Try to get the previously requested quota using a princilal that is
+  // not authorized to see it. This will result in empty information
+  // returned.
+  {
+    Future<Response> response = process::http::get(
+        master.get()->pid,
+        "quota",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    EXPECT_SOME_EQ(
+        "application/json",
+        response.get().headers.get("Content-Type"));
+
+    const Try<JSON::Object> parse =
+      JSON::parse<JSON::Object>(response.get().body);
+
+    ASSERT_SOME(parse);
+
+    // Convert JSON response to `QuotaStatus` protobuf.
+    const Try<QuotaStatus> status = 
::protobuf::parse<QuotaStatus>(parse.get());
+    ASSERT_FALSE(status.isError());
+
+    EXPECT_EQ(0, status.get().infos().size());
+  }
+
+  // Get the previous requested quota using default principal, which is
+  // authorized to see it.
+  {
+    Future<Response> response = process::http::get(
+        master.get()->pid,
+        "quota",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    EXPECT_SOME_EQ(
+        "application/json",
+        response.get().headers.get("Content-Type"));
+
+    const Try<JSON::Object> parse =
+      JSON::parse<JSON::Object>(response.get().body);
+
+    ASSERT_SOME(parse);
+
+    // Convert JSON response to `QuotaStatus` protobuf.
+    const Try<QuotaStatus> status = 
::protobuf::parse<QuotaStatus>(parse.get());
+    ASSERT_FALSE(status.isError());
+
+    EXPECT_EQ(1, status.get().infos().size());
+    EXPECT_EQ(ROLE1, status.get().infos(0).role());
+  }
+
+  // Try to remove the previously requested quota using a principal that is
+  // not the default principal. This will fail because only the default
+  // principal is authorized to do that.
+  {
+    Future<Response> response = process::http::requestDelete(
+        master.get()->pid,
+        "quota/" + ROLE1,
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response)
+      << response.get().body;
+  }
+
+  // Remove the previously requested quota using the default principal.
+  {
+    Future<Nothing> receivedRemoveRequest;
+    EXPECT_CALL(allocator, removeQuota(Eq(ROLE1)))
+      .WillOnce(DoAll(InvokeRemoveQuota(&allocator),
+                      FutureSatisfy(&receivedRemoveRequest)));
+
+    Future<Response> response = process::http::requestDelete(
+        master.get()->pid,
+        "quota/" + ROLE1,
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    AWAIT_READY(receivedRemoveRequest);
+  }
+}
+
+
+// Checks that an authorized principal can update quota using deprecated
+// set_quotas and remove_quotas, while unauthorized principals cannot.
+//
+// TODO(zhitao): Remove this test case at the end of deprecation cycle
+// started with 0.29.
+TEST_F(MasterQuotaTest, AuthorizeSetAndRemoveQuotaRequests)
 {
   TestAllocator<> allocator;
   EXPECT_CALL(allocator, initialize(_, _, _, _));
@@ -1207,8 +1391,8 @@ TEST_F(MasterQuotaTest, AuthorizeQuotaRequests)
         createBasicAuthHeaders(DEFAULT_CREDENTIAL_2),
         createRequestBody(ROLE1, quotaResources, FORCE));
 
-    AWAIT_EXPECT_RESPONSE_STATUS_EQ(
-        Forbidden().status, response) << response.get().body;
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response)
+      << response.get().body;
   }
 
   // Request quota using the default principal.
@@ -1311,8 +1495,8 @@ TEST_F(MasterQuotaTest, AuthorizeQuotaRequests)
         "quota/" + ROLE1,
         createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
 
-    AWAIT_EXPECT_RESPONSE_STATUS_EQ(
-        Forbidden().status, response) << response.get().body;
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response)
+      << response.get().body;
   }
 
   // Remove the previously requested quota using the default principal.
@@ -1335,10 +1519,104 @@ TEST_F(MasterQuotaTest, AuthorizeQuotaRequests)
 }
 
 
+// Checks that get and update quota requests can be authorized without
+// authentication if an authorization rule exists that applies to anyone.
+// The authorizer will map the absence of a principal to "ANY".
+TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequestsWithoutPrincipal)
+{
+  // Setup ACLs so that any principal can set quotas for `ROLE1` and remove
+  // anyone's quotas.
+  ACLs acls;
+
+  mesos::ACL::GetQuota* acl1 = acls.add_get_quotas();
+  acl1->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+  acl1->mutable_roles()->add_values(ROLE1);
+
+  mesos::ACL::UpdateQuota* acl2 = acls.add_update_quotas();
+  acl2->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
+  acl2->mutable_roles()->set_type(mesos::ACL::Entity::ANY);
+
+  master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.acls = acls;
+  masterFlags.authenticate_http = false;
+  masterFlags.authenticate_http_frameworks = false;
+  masterFlags.credentials = None();
+
+  Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
+  ASSERT_SOME(master);
+
+  // Use the force flag for setting quota that cannot be satisfied in
+  // this empty cluster without any agents.
+  const bool FORCE = true;
+
+  // Request quota without providing authorization headers.
+  {
+    // As we don't care about the enforcement of quota but only the
+    // authorization of the quota request we set the force flag in the post
+    // request below to override the capacity heuristic check.
+    Resources quotaResources = Resources::parse("cpus:1;mem:512").get();
+
+    // Create an HTTP request without authorization headers.
+    Future<Response> response = process::http::post(
+        master.get()->pid,
+        "quota",
+        None(),
+        createRequestBody(ROLE1, quotaResources, FORCE));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+  }
+
+  // Get the previously requested quota without providing authoriation
+  // headers.
+  {
+    Future<Response> response = process::http::get(
+        master.get()->pid,
+        "quota",
+        None(),
+        None());
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+
+    EXPECT_SOME_EQ(
+        "application/json",
+        response.get().headers.get("Content-Type"));
+
+    const Try<JSON::Object> parse =
+      JSON::parse<JSON::Object>(response.get().body);
+
+    ASSERT_SOME(parse);
+
+    // Convert JSON response to `QuotaStatus` protobuf.
+    const Try<QuotaStatus> status = 
::protobuf::parse<QuotaStatus>(parse.get());
+    ASSERT_FALSE(status.isError());
+
+    EXPECT_EQ(1, status.get().infos().size());
+    EXPECT_EQ(ROLE1, status.get().infos(0).role());
+  }
+
+  // Remove the previously requested quota without providing authorization
+  // headers.
+  {
+    Future<Response> response = process::http::requestDelete(
+        master.get()->pid,
+        "quota/" + ROLE1,
+        None());
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
+      << response.get().body;
+  }
+}
+
+
 // Checks that set and remove quota requests can be authorized without
 // authentication if an authorization rule exists that applies to anyone.
 // The authorizer will map the absence of a principal to "ANY".
-TEST_F(MasterQuotaTest, AuthorizeQuotaRequestsWithoutPrincipal)
+//
+// TODO(zhitao): Remove this test case at the end of deprecation cycle
+// started with 0.29.
+TEST_F(MasterQuotaTest, AuthorizeSetRemoveQuotaRequestsWithoutPrincipal)
 {
   // Setup ACLs so that any principal can set quotas for `ROLE1` and remove
   // anyone's quotas.

Reply via email to