Limit robots displayed in entity search

Before, we'd load *all* the robots, which can be a huge issue in namespaces with a large number of robots. Now, we only load the top-20 robots (as per recency in login), and we also limit the information returned to the entity search to save some bandwidth.

Fixes https://jira.coreos.com/browse/QUAY-927

data/model/user.py

@@ -410,7 +410,7 @@ def _list_entity_robots(entity_name, include_metadata=True):
   return query
 
 
-def list_entity_robot_permission_teams(entity_name, include_permissions=False):
+def list_entity_robot_permission_teams(entity_name, limit=None, include_permissions=False):
   query = (_list_entity_robots(entity_name))
 
   fields = [User.username, User.creation_date, User.last_accessed, FederatedLogin.service_ident,
@@ -427,6 +427,7 @@ def list_entity_robot_permission_teams(entity_name, include_permissions=False):
     fields.append(Repository.name)
     fields.append(Team.name)
 
+  query = query.limit(limit).order_by(User.last_accessed.desc())
   return TupleSelector(query, fields)
 
 

endpoints/api/robot.py

@@ -31,9 +31,11 @@ CREATE_ROBOT_SCHEMA = {
 ROBOT_MAX_SIZE = 1024 * 1024 # 1 KB.
 
 
-def robots_list(prefix, include_permissions=False):
+def robots_list(prefix, include_permissions=False, include_token=False, limit=None):
-  robots = model.list_entity_robot_permission_teams(prefix, include_permissions=include_permissions)
+  robots = model.list_entity_robot_permission_teams(prefix, limit=limit,
-  return {'robots': [robot.to_dict() for robot in robots]}
+                                                    include_token=include_token,
+                                                    include_permissions=include_permissions)
+  return {'robots': [robot.to_dict(include_token=include_token) for robot in robots]}
 
 
 @resource('/v1/user/robots')
@@ -46,10 +48,18 @@ class UserRobotList(ApiResource):
   @query_param('permissions',
                'Whether to include repositories and teams in which the robots have permission.',
                type=truthy_bool, default=False)
+  @query_param('token',
+               'If false, the robot\'s token is not returned.',
+               type=truthy_bool, default=True)
+  @query_param('limit',
+               'If specified, the number of robots to return.',
+               type=int, default=None)
   def get(self, parsed_args):
     """ List the available robots for the user. """
     user = get_authenticated_user()
-    return robots_list(user.username, include_permissions=parsed_args.get('permissions', False))
+    return robots_list(user.username, include_token=parsed_args.get('token', True),
+                       include_permissions=parsed_args.get('permissions', False),
+                       limit=parsed_args.get('limit'))
 
 
 @resource('/v1/user/robots/<robot_shortname>')
@@ -67,7 +77,7 @@ class UserRobot(ApiResource):
     """ Returns the user's robot with the specified name. """
     parent = get_authenticated_user()
     robot = model.get_user_robot(robot_shortname, parent)
-    return robot.to_dict(include_metadata=True)
+    return robot.to_dict(include_metadata=True, include_token=True)
 
   @require_user_admin
   @nickname('createUserRobot')
@@ -84,7 +94,7 @@ class UserRobot(ApiResource):
       'description': create_data.get('description'),
       'unstructured_metadata': create_data.get('unstructured_metadata'),
     })
-    return robot.to_dict(include_metadata=True), 201
+    return robot.to_dict(include_metadata=True, include_token=True), 201
 
   @require_user_admin
   @nickname('deleteUserRobot')
@@ -108,11 +118,23 @@ class OrgRobotList(ApiResource):
   @query_param('permissions',
                'Whether to include repostories and teams in which the robots have permission.',
                type=truthy_bool, default=False)
+  @query_param('token',
+               'If false, the robot\'s token is not returned.',
+               type=truthy_bool, default=True)
+  @query_param('limit',
+               'If specified, the number of robots to return.',
+               type=int, default=None)
   def get(self, orgname, parsed_args):
     """ List the organization's robots. """
     permission = OrganizationMemberPermission(orgname)
     if permission.can():
-      return robots_list(orgname, include_permissions=parsed_args.get('permissions', False))
+      include_token = (AdministerOrganizationPermission(orgname).can() and
+                       parsed_args.get('token', True))
+      include_permissions = (AdministerOrganizationPermission(orgname).can() and
+                             parsed_args.get('permissions', False))
+      return robots_list(orgname, include_permissions=include_permissions,
+                         include_token=include_token,
+                         limit=parsed_args.get('limit'))
 
     raise Unauthorized()
 
@@ -135,7 +157,7 @@ class OrgRobot(ApiResource):
     permission = AdministerOrganizationPermission(orgname)
     if permission.can():
       robot = model.get_org_robot(robot_shortname, orgname)
-      return robot.to_dict(include_metadata=True)
+      return robot.to_dict(include_metadata=True, include_token=True)
 
     raise Unauthorized()
 
@@ -155,7 +177,7 @@ class OrgRobot(ApiResource):
         'description': create_data.get('description'),
         'unstructured_metadata': create_data.get('unstructured_metadata'),
       })
-      return robot.to_dict(include_metadata=True), 201
+      return robot.to_dict(include_metadata=True, include_token=True), 201
 
     raise Unauthorized()
 
@@ -228,7 +250,7 @@ class RegenerateUserRobot(ApiResource):
     parent = get_authenticated_user()
     robot = model.regenerate_user_robot_token(robot_shortname, parent)
     log_action('regenerate_robot_token', parent.username, {'robot': robot_shortname})
-    return robot.to_dict()
+    return robot.to_dict(include_token=True)
 
 
 @resource('/v1/organization/<orgname>/robots/<robot_shortname>/regenerate')
@@ -247,6 +269,6 @@ class RegenerateOrgRobot(ApiResource):
     if permission.can():
       robot = model.regenerate_org_robot_token(robot_shortname, orgname)
       log_action('regenerate_robot_token', orgname, {'robot': robot_shortname})
-      return robot.to_dict()
+      return robot.to_dict(include_token=True)
 
     raise Unauthorized()

endpoints/api/robot_models_interface.py

@@ -55,10 +55,9 @@ class RobotWithPermissions(
   :type description: string
   """
 
-  def to_dict(self):
+  def to_dict(self, include_token=False):
-    return {
+    data = {
       'name': self.name,
-      'token': self.password,
       'created': format_date(self.created) if self.created is not None else None,
       'last_accessed': format_date(self.last_accessed) if self.last_accessed is not None else None,
       'teams': [team.to_dict() for team in self.teams],
@@ -66,6 +65,11 @@ class RobotWithPermissions(
       'description': self.description,
     }
 
+    if include_token:
+      data['token'] = self.password
+
+    return data
+
 
 class Robot(
   namedtuple('Robot', [
@@ -86,15 +90,17 @@ class Robot(
   :type unstructured_metadata: dict
   """
 
-  def to_dict(self, include_metadata=False):
+  def to_dict(self, include_metadata=False, include_token=False):
     data = {
       'name': self.name,
-      'token': self.password,
       'created': format_date(self.created) if self.created is not None else None,
       'last_accessed': format_date(self.last_accessed) if self.last_accessed is not None else None,
       'description': self.description,
     }
 
+    if include_token:
+      data['token'] = self.password
+
     if include_metadata:
       data['unstructured_metadata'] = self.unstructured_metadata
     
@@ -171,7 +177,8 @@ class RobotInterface(object):
     """
 
   @abstractmethod
-  def list_entity_robot_permission_teams(self, prefix, include_permissions=False):
+  def list_entity_robot_permission_teams(self, prefix, include_permissions=False,
+                                         include_token=False, limit=None):
     """
 
     Returns:

endpoints/api/robot_models_pre_oci.py

@@ -11,8 +11,9 @@ class RobotPreOCIModel(RobotInterface):
     return [Permission(permission.repository.name, permission.repository.visibility.name, permission.role.name) for
             permission in permissions]
 
-  def list_entity_robot_permission_teams(self, prefix, include_permissions=False):
+  def list_entity_robot_permission_teams(self, prefix, include_token=False,
-    tuples = model.user.list_entity_robot_permission_teams(prefix,
+                                         include_permissions=False, limit=None):
+    tuples = model.user.list_entity_robot_permission_teams(prefix, limit=limit,
                                                            include_permissions=include_permissions)
     robots = {}
     robot_teams = set()
@@ -22,7 +23,7 @@ class RobotPreOCIModel(RobotInterface):
       if robot_name not in robots:
         robot_dict = {
           'name': robot_name,
-          'token': robot_tuple.get(FederatedLogin.service_ident),
+          'token': robot_tuple.get(FederatedLogin.service_ident) if include_token else None,
           'created': robot_tuple.get(User.creation_date),
           'last_accessed': robot_tuple.get(User.last_accessed),
           'description': robot_tuple.get(RobotAccountMetadata.description),
@@ -56,7 +57,7 @@ class RobotPreOCIModel(RobotInterface):
           if repository_name not in robot_dict['repositories']:
             robot_dict['repositories'].append(repository_name)
         robots[robot_name] = RobotWithPermissions(robot_dict['name'], robot_dict['token'],
-                                                  robot_dict['created'], 
+                                                  robot_dict['created'],
                                                   robot_dict['last_accessed'],
                                                   robot_dict['teams'],
                                                   robot_dict['repositories'],

endpoints/api/test/test_robot.py

@@ -4,7 +4,7 @@ import json
 from data import model
 from endpoints.api import api
 from endpoints.api.test.shared import conduct_api_call
-from endpoints.api.robot import UserRobot, OrgRobot
+from endpoints.api.robot import UserRobot, OrgRobot, UserRobotList, OrgRobotList
 from endpoints.test.shared import client_with_identity
 
 from test.test_ldap import mock_ldap
@@ -36,3 +36,63 @@ def test_create_robot_with_metadata(endpoint, body, client):
     body = body or {}
     assert resp.json['description'] == (body.get('description') or '')
     assert resp.json['unstructured_metadata'] == (body.get('unstructured_metadata') or {})
+
+
+@pytest.mark.parametrize('endpoint, params', [
+  (UserRobot, {'robot_shortname': 'dtrobot'}),
+  (OrgRobot, {'orgname': 'buynlarge', 'robot_shortname': 'coolrobot'}),
+])
+def test_retrieve_robot(endpoint, params, app, client):
+  with client_with_identity('devtable', client) as cl:
+    result = conduct_api_call(cl, endpoint, 'GET', params, None)
+    assert result.json['token'] is not None
+
+
+@pytest.mark.parametrize('endpoint, params', [
+  (UserRobotList, {}),
+  (OrgRobotList, {'orgname': 'buynlarge'}),
+])
+@pytest.mark.parametrize('include_token', [
+  True,
+  False,
+])
+@pytest.mark.parametrize('limit', [
+  None,
+  1,
+  5,
+])
+def test_retrieve_robots(endpoint, params, include_token, limit, app, client):
+  params['token'] = 'true' if include_token else 'false'
+
+  if limit is not None:
+    params['limit'] = limit
+
+  with client_with_identity('devtable', client) as cl:
+    result = conduct_api_call(cl, endpoint, 'GET', params, None)
+
+    if limit is not None:
+      assert len(result.json['robots']) <= limit
+
+    for robot in result.json['robots']:
+      assert (robot.get('token') is not None) == include_token
+
+
+@pytest.mark.parametrize('username, is_admin', [
+  ('devtable', True),
+  ('reader', False),
+])
+@pytest.mark.parametrize('with_permissions', [
+  True,
+  False,
+])
+def test_retrieve_robots_token_permission(username, is_admin, with_permissions, app, client):
+  with client_with_identity(username, client) as cl:
+    params = {'orgname': 'buynlarge', 'token': 'true'}
+    if with_permissions:
+      params['permissions'] = 'true'
+
+    result = conduct_api_call(cl, OrgRobotList, 'GET', params, None)
+    assert result.json['robots']
+    for robot in result.json['robots']:
+      assert (robot.get('token') is not None) == is_admin
+      assert (robot.get('repositories') is not None) == (is_admin and with_permissions)

static/js/directives/ui/entity-search.js

@@ -121,8 +121,12 @@ angular.module('quay').directive('entitySearch', function () {
         // Load the user/organization's robots (if applicable).
         if ($scope.isAdmin && isSupported('robot')) {
           requiredOperations++;
-          
+          var params = {
-          ApiService.getRobots($scope.isOrganization ? $scope.namespace : null).then(function(resp) {
+            'token': false,
+            'limit': 20
+          };
+
+          ApiService.getRobots($scope.isOrganization ? $scope.namespace : null, null, params).then(function(resp) {
             $scope.page.robots = resp.robots;
             operationComplete();
           }, operationComplete);

static/js/services/api-service.js

@@ -71,7 +71,7 @@ angular.module('quay').factory('ApiService', ['Restangular', '$q', 'UtilService'
       if (used[paramName]) { continue; }
 
       var value = parameters[paramName];
-      if (value) {
+      if (value != null) {
         url += isFirst ? '?' : '&';
         url += paramName + '=' + encodeURIComponent(value)
         isFirst = false;