You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ar...@apache.org on 2017/07/11 14:50:29 UTC

mesos git commit: Enabled filtering of reservations.

Repository: mesos
Updated Branches:
  refs/heads/master edba29265 -> 77005e581


Enabled filtering of reservations.

Adds support of the 'VIEW_ROLE' ACL to the results generated by the
'/slaves' as well as the 'GET_AGENTS' API v1 call, '/states' and
'/states-summary'.

This means that calls to these endpoints and API call which uses
reservations will only show the reserved resources the user making the
requests is allowed to see based on the reservations roles.

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


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

Branch: refs/heads/master
Commit: 77005e581906368c2e60eca379abe8e251f8f9d1
Parents: edba292
Author: Alexander Rojas <al...@mesosphere.io>
Authored: Fri Jul 7 17:03:08 2017 +0200
Committer: Alexander Rojas <al...@mesosphere.io>
Committed: Tue Jul 11 16:50:15 2017 +0200

----------------------------------------------------------------------
 src/master/http.cpp        | 527 +++++++++++++++++++++++-----------------
 src/master/master.hpp      |   1 +
 src/tests/master_tests.cpp |  89 +++++++
 3 files changed, 396 insertions(+), 221 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/77005e58/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 175a44c..dbf3d0f 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -375,38 +375,151 @@ struct FullFrameworkWriter {
 };
 
 
-static void json(JSON::ObjectWriter* writer, const Summary<Slave>& summary)
+struct SlaveWriter
 {
-  const Slave& slave = summary;
+  SlaveWriter(const Slave& slave, const Owned<ObjectApprover>& roleApprover)
+    : slave_(slave), roleApprover_(roleApprover) {}
 
-  json(writer, slave.info);
+  void operator()(JSON::ObjectWriter* writer) const
+  {
+    json(writer, slave_.info);
 
-  writer->field("pid", string(slave.pid));
-  writer->field("registered_time", slave.registeredTime.secs());
+    writer->field("pid", string(slave_.pid));
+    writer->field("registered_time", slave_.registeredTime.secs());
 
-  if (slave.reregisteredTime.isSome()) {
-    writer->field("reregistered_time", slave.reregisteredTime.get().secs());
-  }
+    if (slave_.reregisteredTime.isSome()) {
+      writer->field("reregistered_time", slave_.reregisteredTime.get().secs());
+    }
 
-  const Resources& totalResources = slave.totalResources;
-  writer->field("resources", totalResources);
-  writer->field("used_resources", Resources::sum(slave.usedResources));
-  writer->field("offered_resources", slave.offeredResources);
-  writer->field("reserved_resources", totalResources.reservations());
-  writer->field("unreserved_resources", totalResources.unreserved());
+    const Resources& totalResources = slave_.totalResources;
+    writer->field("resources", totalResources);
+    writer->field("used_resources", Resources::sum(slave_.usedResources));
+    writer->field("offered_resources", slave_.offeredResources);
+    writer->field(
+        "reserved_resources",
+        [&totalResources, this](JSON::ObjectWriter* writer) {
+          foreachpair (const string& role, const Resources& reservation,
+                       totalResources.reservations()) {
+            // TODO(arojas): Consider showing unapproved resources in an
+            // aggregated special field, so that all resource values add up
+            // MESOS-7779.
+            if (approveViewRole(roleApprover_, role)) {
+              writer->field(role, reservation);
+            }
+          }
+        });
+    writer->field("unreserved_resources", totalResources.unreserved());
 
-  writer->field("active", slave.active);
-  writer->field("version", slave.version);
-  writer->field("capabilities", slave.capabilities.toRepeatedPtrField());
-}
+    writer->field("active", slave_.active);
+    writer->field("version", slave_.version);
+    writer->field("capabilities", slave_.capabilities.toRepeatedPtrField());
+  }
+
+  const Slave& slave_;
+  const Owned<ObjectApprover>& roleApprover_;
+};
 
 
-static void json(JSON::ObjectWriter* writer, const Full<Slave>& full)
+struct SlavesWriter
 {
-  const Slave& slave = full;
+  SlavesWriter(
+      const Master::Slaves& slaves,
+      const Owned<ObjectApprover>& roleApprover)
+    : slaves_(slaves),
+      roleApprover_(roleApprover) {}
 
-  json(writer, Summary<Slave>(slave));
-}
+  void operator()(JSON::ObjectWriter* writer) const
+  {
+    writer->field("slaves", [this](JSON::ArrayWriter* writer) {
+      foreachvalue (const Slave* slave, slaves_.registered) {
+        writer->element([this, &slave](JSON::ObjectWriter* writer) {
+          writeSlave(slave, writer);
+        });
+      }
+    });
+
+    writer->field("recovered_slaves", [this](JSON::ArrayWriter* writer) {
+      foreachvalue (const SlaveInfo& slaveInfo, slaves_.recovered) {
+        writer->element([&slaveInfo](JSON::ObjectWriter* writer) {
+          json(writer, slaveInfo);
+        });
+      }
+    });
+  }
+
+  void writeSlave(const Slave* slave, JSON::ObjectWriter* writer) const
+  {
+    SlaveWriter(*slave, roleApprover_)(writer);
+
+    // Add the complete protobuf->JSON for all used, reserved,
+    // and offered resources. The other endpoints summarize
+    // resource information, which omits the details of
+    // reservations and persistent volumes. Full resource
+    // information is necessary so that operators can use the
+    // `/unreserve` and `/destroy-volumes` endpoints.
+
+    hashmap<string, Resources> reserved = slave->totalResources.reservations();
+
+    writer->field(
+        "reserved_resources_full",
+        [&reserved, this](JSON::ObjectWriter* writer) {
+          foreachpair (const string& role,
+                       const Resources& resources,
+                       reserved) {
+            if (approveViewRole(roleApprover_, role)) {
+              writer->field(role, [&resources](JSON::ArrayWriter* writer) {
+                foreach (Resource resource, resources) {
+                  convertResourceFormat(&resource, ENDPOINT);
+                  writer->element(JSON::Protobuf(resource));
+                }
+              });
+            }
+          }
+        });
+
+    Resources unreservedResources = slave->totalResources.unreserved();
+
+    writer->field(
+        "unreserved_resources_full",
+        [&unreservedResources](JSON::ArrayWriter* writer) {
+          foreach (Resource resource, unreservedResources) {
+            convertResourceFormat(&resource, ENDPOINT);
+            writer->element(JSON::Protobuf(resource));
+          }
+        });
+
+    Resources usedResources = Resources::sum(slave->usedResources);
+
+    writer->field(
+        "used_resources_full",
+        [&usedResources, this](JSON::ArrayWriter* writer) {
+          foreach (Resource resource, usedResources) {
+            if (approveViewRole(roleApprover_, resource.role()) &&
+                approveViewRole(roleApprover_,
+                                resource.allocation_info().role())) {
+              convertResourceFormat(&resource, ENDPOINT);
+              writer->element(JSON::Protobuf(resource));
+            }
+          }
+        });
+
+    const Resources& offeredResources = slave->offeredResources;
+
+    writer->field(
+        "offered_resources_full",
+        [&offeredResources, this](JSON::ArrayWriter* writer) {
+          foreach (Resource resource, offeredResources) {
+            if (approveViewRole(roleApprover_, resource.role())) {
+              convertResourceFormat(&resource, ENDPOINT);
+              writer->element(JSON::Protobuf(resource));
+            }
+          }
+        });
+  }
+
+  const Master::Slaves &slaves_;
+  const Owned<ObjectApprover> &roleApprover_;
+};
 
 
 static void json(JSON::ObjectWriter* writer, const Summary<Framework>& summary)
@@ -2319,91 +2432,33 @@ string Master::Http::SLAVES_HELP()
 
 Future<Response> Master::Http::slaves(
     const Request& request,
-    const Option<Principal>&) const
+    const Option<Principal>& principal) const
 {
   // When current master is not the leader, redirect to the leading master.
   if (!master->elected()) {
     return redirect(request);
   }
 
-  auto slaves = [this](JSON::ObjectWriter* writer) {
-    writer->field("slaves", [this](JSON::ArrayWriter* writer) {
-      foreachvalue (const Slave* slave, master->slaves.registered) {
-        writer->element([&slave](JSON::ObjectWriter* writer) {
-          json(writer, Full<Slave>(*slave));
-
-          // Add the complete protobuf->JSON for all used, reserved,
-          // and offered resources. The other endpoints summarize
-          // resource information, which omits the details of
-          // reservations and persistent volumes. Full resource
-          // information is necessary so that operators can use the
-          // `/unreserve` and `/destroy-volumes` endpoints.
-
-          hashmap<string, Resources> reserved =
-            slave->totalResources.reservations();
-
-          writer->field(
-              "reserved_resources_full",
-              [&reserved](JSON::ObjectWriter* writer) {
-                foreachpair (const string& role,
-                             const Resources& resources,
-                             reserved) {
-                  writer->field(role, [&resources](JSON::ArrayWriter* writer) {
-                    foreach (Resource resource, resources) {
-                      convertResourceFormat(&resource, ENDPOINT);
-                      writer->element(JSON::Protobuf(resource));
-                    }
-                  });
-                }
-              });
-
-          Resources unreservedResources = slave->totalResources.unreserved();
-
-          writer->field(
-              "unreserved_resources_full",
-              [&unreservedResources](JSON::ArrayWriter* writer) {
-                foreach (Resource resource, unreservedResources) {
-                  convertResourceFormat(&resource, ENDPOINT);
-                  writer->element(JSON::Protobuf(resource));
-                }
-              });
-
-          Resources usedResources = Resources::sum(slave->usedResources);
-
-          writer->field(
-              "used_resources_full",
-              [&usedResources](JSON::ArrayWriter* writer) {
-                foreach (Resource resource, usedResources) {
-                  convertResourceFormat(&resource, ENDPOINT);
-                  writer->element(JSON::Protobuf(resource));
-                }
-              });
+  // Retrieve `ObjectApprover`s for authorizing roles.
+  Future<Owned<ObjectApprover>> rolesApprover;
 
-          const Resources& offeredResources = slave->offeredResources;
+  if (master->authorizer.isSome()) {
+    Option<authorization::Subject> subject = createSubject(principal);
 
-          writer->field(
-              "offered_resources_full",
-              [&offeredResources](JSON::ArrayWriter* writer) {
-                foreach (Resource resource, offeredResources) {
-                  convertResourceFormat(&resource, ENDPOINT);
-                  writer->element(JSON::Protobuf(resource));
-                }
-              });
-        });
-      };
-    });
+    rolesApprover = master->authorizer.get()->getObjectApprover(
+        subject, authorization::VIEW_ROLE);
+  } else {
+    rolesApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
+  }
 
-    // Model all of the recovered slaves.
-    writer->field("recovered_slaves", [this](JSON::ArrayWriter* writer) {
-      foreachvalue (const SlaveInfo& slaveInfo, master->slaves.recovered) {
-        writer->element([&slaveInfo](JSON::ObjectWriter* writer) {
-          json(writer, slaveInfo);
-        });
-      }
-    });
-  };
+  Master* master = this->master;
+  Option<string> jsonp = request.url.query.get("jsonp");
 
-  return OK(jsonify(slaves), request.url.query.get("jsonp"));
+  return rolesApprover.then(defer(master->self(),
+      [master, jsonp](const Owned<ObjectApprover>& rolesApprover)
+          -> Future<Response> {
+        return OK(jsonify(SlavesWriter(master->slaves, rolesApprover)), jsonp);
+      }));
 }
 
 
@@ -2683,6 +2738,7 @@ Future<Response> Master::Http::state(
   }
 
   // Retrieve `ObjectApprover`s for authorizing frameworks and tasks.
+  Future<Owned<ObjectApprover>> rolesApprover;
   Future<Owned<ObjectApprover>> frameworksApprover;
   Future<Owned<ObjectApprover>> tasksApprover;
   Future<Owned<ObjectApprover>> executorsApprover;
@@ -2691,6 +2747,9 @@ Future<Response> Master::Http::state(
   if (master->authorizer.isSome()) {
     Option<authorization::Subject> subject = createSubject(principal);
 
+    rolesApprover = master->authorizer.get()->getObjectApprover(
+        subject, authorization::VIEW_ROLE);
+
     frameworksApprover = master->authorizer.get()->getObjectApprover(
         subject, authorization::VIEW_FRAMEWORK);
 
@@ -2703,6 +2762,7 @@ Future<Response> Master::Http::state(
     flagsApprover = master->authorizer.get()->getObjectApprover(
         subject, authorization::VIEW_FLAGS);
   } else {
+    rolesApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
     frameworksApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
     tasksApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
     executorsApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
@@ -2710,6 +2770,7 @@ Future<Response> Master::Http::state(
   }
 
   return collect(
+      rolesApprover,
       frameworksApprover,
       tasksApprover,
       executorsApprover,
@@ -2719,17 +2780,20 @@ Future<Response> Master::Http::state(
         [this, request](const tuple<Owned<ObjectApprover>,
                                     Owned<ObjectApprover>,
                                     Owned<ObjectApprover>,
+                                    Owned<ObjectApprover>,
                                     Owned<ObjectApprover>>& approvers)
           -> Response {
       // This lambda is consumed before the outer lambda
       // returns, hence capture by reference is fine here.
       auto state = [this, &approvers](JSON::ObjectWriter* writer) {
         // Get approver from tuple.
+        Owned<ObjectApprover> rolesApprover;
         Owned<ObjectApprover> frameworksApprover;
         Owned<ObjectApprover> tasksApprover;
         Owned<ObjectApprover> executorsApprover;
         Owned<ObjectApprover> flagsApprover;
-        tie(frameworksApprover,
+        tie(rolesApprover,
+            frameworksApprover,
             tasksApprover,
             executorsApprover,
             flagsApprover) = approvers;
@@ -2800,11 +2864,12 @@ Future<Response> Master::Http::state(
         }
 
         // Model all of the registered slaves.
-        writer->field("slaves", [this](JSON::ArrayWriter* writer) {
-          foreachvalue (Slave* slave, master->slaves.registered) {
-            writer->element(Full<Slave>(*slave));
-          }
-        });
+        writer->field("slaves",
+          [this, rolesApprover](JSON::ArrayWriter* writer) {
+            foreachvalue (Slave* slave, master->slaves.registered) {
+              writer->element(SlaveWriter(*slave, rolesApprover));
+            }
+          });
 
         // Model all of the recovered slaves.
         writer->field("recovered_slaves", [this](JSON::ArrayWriter* writer) {
@@ -3141,147 +3206,167 @@ Future<Response> Master::Http::stateSummary(
     return redirect(request);
   }
 
+  Future<Owned<ObjectApprover>> rolesApprover;
   Future<Owned<ObjectApprover>> frameworksApprover;
 
   if (master->authorizer.isSome()) {
     Option<authorization::Subject> subject = createSubject(principal);
 
+    rolesApprover = master->authorizer.get()->getObjectApprover(
+        subject, authorization::VIEW_ROLE);
     frameworksApprover = master->authorizer.get()->getObjectApprover(
         subject, authorization::VIEW_FRAMEWORK);
   } else {
+    rolesApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
     frameworksApprover = Owned<ObjectApprover>(new AcceptingObjectApprover());
   }
 
-  return frameworksApprover
-    .then(defer(
-        master->self(),
-        [this, request](const Owned<ObjectApprover>& frameworksApprover)
+  return collect(rolesApprover, frameworksApprover).then(defer(
+      master->self(),
+      [this, request](const tuple<Owned<ObjectApprover>,
+                                    Owned<ObjectApprover>>& approvers)
           -> Response {
-      auto stateSummary =
-          [this, &frameworksApprover](JSON::ObjectWriter* writer) {
-        writer->field("hostname", master->info().hostname());
+        auto stateSummary = [this, &approvers](JSON::ObjectWriter* writer) {
+          Owned<ObjectApprover> rolesApprover;
+          Owned<ObjectApprover> frameworksApprover;
+          tie(rolesApprover,
+              frameworksApprover) = approvers;
 
-        if (master->flags.cluster.isSome()) {
-          writer->field("cluster", master->flags.cluster.get());
-        }
+          writer->field("hostname", master->info().hostname());
 
-        // We use the tasks in the 'Frameworks' struct to compute summaries
-        // for this endpoint. This is done 1) for consistency between the
-        // 'slaves' and 'frameworks' subsections below 2) because we want to
-        // provide summary information for frameworks that are currently
-        // registered 3) the frameworks keep a circular buffer of completed
-        // tasks that we can use to keep a limited view on the history of
-        // recent completed / failed tasks.
+          if (master->flags.cluster.isSome()) {
+            writer->field("cluster", master->flags.cluster.get());
+          }
 
-        // Generate mappings from 'slave' to 'framework' and reverse.
-        SlaveFrameworkMapping slaveFrameworkMapping(
-            master->frameworks.registered);
+          // We use the tasks in the 'Frameworks' struct to compute summaries
+          // for this endpoint. This is done 1) for consistency between the
+          // 'slaves' and 'frameworks' subsections below 2) because we want to
+          // provide summary information for frameworks that are currently
+          // registered 3) the frameworks keep a circular buffer of completed
+          // tasks that we can use to keep a limited view on the history of
+          // recent completed / failed tasks.
 
-        // Generate 'TaskState' summaries for all framework and slave ids.
-        TaskStateSummaries taskStateSummaries(master->frameworks.registered);
+          // Generate mappings from 'slave' to 'framework' and reverse.
+          SlaveFrameworkMapping slaveFrameworkMapping(
+              master->frameworks.registered);
 
-        // Model all of the slaves.
-        writer->field(
-            "slaves",
-            [this, &slaveFrameworkMapping, &taskStateSummaries](
-                JSON::ArrayWriter* writer) {
-          foreachvalue (Slave* slave, master->slaves.registered) {
-            writer->element(
-                [&slave, &slaveFrameworkMapping, &taskStateSummaries](
-                    JSON::ObjectWriter* writer) {
-              json(writer, Summary<Slave>(*slave));
-
-              // Add the 'TaskState' summary for this slave.
-              const TaskStateSummary& summary =
-                taskStateSummaries.slave(slave->id);
-
-              // Certain per-agent status totals will always be zero
-              // (e.g., TASK_ERROR, TASK_UNREACHABLE). We report them
-              // here anyway, for completeness.
-              //
-              // TODO(neilc): Update for TASK_GONE and TASK_GONE_BY_OPERATOR.
-              writer->field("TASK_STAGING", summary.staging);
-              writer->field("TASK_STARTING", summary.starting);
-              writer->field("TASK_RUNNING", summary.running);
-              writer->field("TASK_KILLING", summary.killing);
-              writer->field("TASK_FINISHED", summary.finished);
-              writer->field("TASK_KILLED", summary.killed);
-              writer->field("TASK_FAILED", summary.failed);
-              writer->field("TASK_LOST", summary.lost);
-              writer->field("TASK_ERROR", summary.error);
-              writer->field("TASK_UNREACHABLE", summary.unreachable);
-
-              // Add the ids of all the frameworks running on this slave.
-              const hashset<FrameworkID>& frameworks =
-                slaveFrameworkMapping.frameworks(slave->id);
-
-              writer->field(
-                  "framework_ids",
-                  [&frameworks](JSON::ArrayWriter* writer) {
-                foreach (const FrameworkID& frameworkId, frameworks) {
-                  writer->element(frameworkId.value());
+          // Generate 'TaskState' summaries for all framework and slave ids.
+          TaskStateSummaries taskStateSummaries(master->frameworks.registered);
+
+          // Model all of the slaves.
+          writer->field(
+              "slaves",
+              [this,
+               &slaveFrameworkMapping,
+               &taskStateSummaries,
+               &rolesApprover](JSON::ArrayWriter* writer) {
+                foreachvalue (Slave* slave, master->slaves.registered) {
+                  writer->element(
+                      [&slave,
+                       &slaveFrameworkMapping,
+                       &taskStateSummaries,
+                       &rolesApprover](JSON::ObjectWriter* writer) {
+                        SlaveWriter slaveWriter(*slave, rolesApprover);
+                        slaveWriter(writer);
+
+                        // Add the 'TaskState' summary for this slave.
+                        const TaskStateSummary& summary =
+                            taskStateSummaries.slave(slave->id);
+
+                        // Certain per-agent status totals will always be zero
+                        // (e.g., TASK_ERROR, TASK_UNREACHABLE). We report them
+                        // here anyway, for completeness.
+                        //
+                        // TODO(neilc): Update for TASK_GONE and
+                        // TASK_GONE_BY_OPERATOR.
+                        writer->field("TASK_STAGING", summary.staging);
+                        writer->field("TASK_STARTING", summary.starting);
+                        writer->field("TASK_RUNNING", summary.running);
+                        writer->field("TASK_KILLING", summary.killing);
+                        writer->field("TASK_FINISHED", summary.finished);
+                        writer->field("TASK_KILLED", summary.killed);
+                        writer->field("TASK_FAILED", summary.failed);
+                        writer->field("TASK_LOST", summary.lost);
+                        writer->field("TASK_ERROR", summary.error);
+                        writer->field("TASK_UNREACHABLE", summary.unreachable);
+
+                        // Add the ids of all the frameworks running on this
+                        // slave.
+                        const hashset<FrameworkID>& frameworks =
+                            slaveFrameworkMapping.frameworks(slave->id);
+
+                        writer->field(
+                            "framework_ids",
+                            [&frameworks](JSON::ArrayWriter* writer) {
+                              foreach (
+                                  const FrameworkID& frameworkId,
+                                  frameworks) {
+                                writer->element(frameworkId.value());
+                              }
+                            });
+                      });
                 }
               });
-            });
-          }
-        });
 
-        // Model all of the frameworks.
-        writer->field(
-            "frameworks",
-            [this,
-             &slaveFrameworkMapping,
-             &taskStateSummaries,
-             &frameworksApprover](JSON::ArrayWriter* writer) {
-          foreachpair (const FrameworkID& frameworkId,
-                       Framework* framework,
-                       master->frameworks.registered) {
-            // Skip unauthorized frameworks.
-            if (!approveViewFrameworkInfo(
-                    frameworksApprover,
-                    framework->info)) {
-              continue;
-            }
-
-            writer->element(
-                [&frameworkId,
-                 &framework,
-                 &slaveFrameworkMapping,
-                 &taskStateSummaries](JSON::ObjectWriter* writer) {
-              json(writer, Summary<Framework>(*framework));
-
-              // Add the 'TaskState' summary for this framework.
-              const TaskStateSummary& summary =
-                taskStateSummaries.framework(frameworkId);
-
-              // TODO(neilc): Update for TASK_GONE and TASK_GONE_BY_OPERATOR.
-              writer->field("TASK_STAGING", summary.staging);
-              writer->field("TASK_STARTING", summary.starting);
-              writer->field("TASK_RUNNING", summary.running);
-              writer->field("TASK_KILLING", summary.killing);
-              writer->field("TASK_FINISHED", summary.finished);
-              writer->field("TASK_KILLED", summary.killed);
-              writer->field("TASK_FAILED", summary.failed);
-              writer->field("TASK_LOST", summary.lost);
-              writer->field("TASK_ERROR", summary.error);
-              writer->field("TASK_UNREACHABLE", summary.unreachable);
-
-              // Add the ids of all the slaves running this framework.
-              const hashset<SlaveID>& slaves =
-                slaveFrameworkMapping.slaves(frameworkId);
-
-              writer->field("slave_ids", [&slaves](JSON::ArrayWriter* writer) {
-                foreach (const SlaveID& slaveId, slaves) {
-                  writer->element(slaveId.value());
+          // Model all of the frameworks.
+          writer->field(
+              "frameworks",
+              [this,
+               &slaveFrameworkMapping,
+               &taskStateSummaries,
+               &frameworksApprover](JSON::ArrayWriter* writer) {
+                foreachpair (const FrameworkID& frameworkId,
+                             Framework* framework,
+                             master->frameworks.registered) {
+                  // Skip unauthorized frameworks.
+                  if (!approveViewFrameworkInfo(
+                          frameworksApprover,
+                          framework->info)) {
+                    continue;
+                  }
+
+                  writer->element(
+                      [&frameworkId,
+                       &framework,
+                       &slaveFrameworkMapping,
+                       &taskStateSummaries](JSON::ObjectWriter* writer) {
+                        json(writer, Summary<Framework>(*framework));
+
+                        // Add the 'TaskState' summary for this framework.
+                        const TaskStateSummary& summary =
+                            taskStateSummaries.framework(frameworkId);
+
+                        // TODO(neilc): Update for TASK_GONE and
+                        // TASK_GONE_BY_OPERATOR.
+                        writer->field("TASK_STAGING", summary.staging);
+                        writer->field("TASK_STARTING", summary.starting);
+                        writer->field("TASK_RUNNING", summary.running);
+                        writer->field("TASK_KILLING", summary.killing);
+                        writer->field("TASK_FINISHED", summary.finished);
+                        writer->field("TASK_KILLED", summary.killed);
+                        writer->field("TASK_FAILED", summary.failed);
+                        writer->field("TASK_LOST", summary.lost);
+                        writer->field("TASK_ERROR", summary.error);
+                        writer->field("TASK_UNREACHABLE", summary.unreachable);
+
+                        // Add the ids of all the slaves running this framework.
+                        const hashset<SlaveID>& slaves =
+                            slaveFrameworkMapping.slaves(frameworkId);
+
+                        writer->field(
+                            "slave_ids",
+                            [&slaves](JSON::ArrayWriter* writer) {
+                              foreach (const SlaveID& slaveId, slaves) {
+                                writer->element(slaveId.value());
+                              }
+                            });
+                      });
                 }
               });
-            });
-          }
-        });
-      };
+        };
 
-      return OK(jsonify(stateSummary), request.url.query.get("jsonp"));
-    }));
+        return OK(jsonify(stateSummary), request.url.query.get("jsonp"));
+      }));
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/77005e58/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index b9b3ea4..28b384c 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1607,6 +1607,7 @@ private:
   friend struct Framework;
   friend struct Metrics;
   friend struct Slave;
+  friend struct SlavesWriter;
 
   // NOTE: Since 'getOffer', 'getInverseOffer' and 'slaves' are
   // protected, we need to make the following functions friends.

http://git-wip-us.apache.org/repos/asf/mesos/blob/77005e58/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index c778c6c..9cfa510 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -2222,6 +2222,95 @@ TEST_F(MasterTest, SlavesEndpointWithoutSlaves)
 }
 
 
+// Tests that reservations can only be seen by authorized users.
+TEST_F(MasterTest, SlavesEndpointFiltering)
+{
+  // Start up the master.
+  master::Flags flags = CreateMasterFlags();
+
+  {
+    mesos::ACL::ViewRole* acl = flags.acls.get().add_view_roles();
+    acl->mutable_principals()->add_values(DEFAULT_CREDENTIAL_2.principal());
+    acl->mutable_roles()->set_type(mesos::ACL::Entity::NONE);
+  }
+
+  Try<Owned<cluster::Master>> master = StartMaster(flags);
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  Future<SlaveRegisteredMessage> agentRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), master.get()->pid, _);
+
+  Try<Owned<cluster::Slave>> agent = StartSlave(detector.get());
+  ASSERT_SOME(agent);
+
+  AWAIT_READY(agentRegisteredMessage);
+  const SlaveID& agentId = agentRegisteredMessage->slave_id();
+
+  // Create reservation.
+  {
+    RepeatedPtrField<Resource> reservation =
+      Resources::parse("cpus:1;mem:12")->pushReservation(
+          createDynamicReservationInfo(
+              "superhero",
+              DEFAULT_CREDENTIAL.principal()));
+
+    Future<Response> response = process::http::post(
+        master.get()->pid,
+        "reserve",
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL),
+        strings::format(
+            "slaveId=%s&resources=%s",
+            agentId,
+            JSON::protobuf(reservation)).get());
+
+    AWAIT_READY(response);
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(Accepted().status, response);
+  }
+
+  // Query master with invalid user.
+  {
+    Future<Response> response = process::http::get(
+        master.get()->pid,
+        "slaves",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL_2));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+    AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
+
+    const Try<JSON::Object> json = JSON::parse<JSON::Object>(response->body);
+    ASSERT_SOME(json);
+
+    Result<JSON::Object> reservations =
+      json->find<JSON::Object>("slaves[0].reserved_resources");
+    ASSERT_SOME(reservations);
+    EXPECT_TRUE(reservations->values.empty());
+  }
+
+  // Query master with valid user.
+  {
+    Future<Response> response = process::http::get(
+        master.get()->pid,
+        "slaves",
+        None(),
+        createBasicAuthHeaders(DEFAULT_CREDENTIAL));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+    AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", response);
+
+    const Try<JSON::Object> json = JSON::parse<JSON::Object>(response->body);
+    ASSERT_SOME(json);
+
+    Result<JSON::Object> reservations =
+      json->find<JSON::Object>("slaves[0].reserved_resources");
+    ASSERT_SOME(reservations);
+    EXPECT_FALSE(reservations->values.empty());
+  }
+}
+
+
 // Ensures that the number of registered slaves reported by
 // /master/slaves coincides with the actual number of registered
 // slaves.