Bug #907521.

    Changes to support get roles by service.

Change-Id: I0646ab91f6961e5e3ed40b8697412c31047511d3

keystone/backends/api.py

@@ -208,6 +208,12 @@ def ref_get_page_markers(self, user_id, tenant_id, marker, limit):
     def ref_get_by_user(self, user_id, role_id, tenant_id):
         raise NotImplementedError
 
+    def get_by_service_get_page(self, service_id, marker, limit):
+        raise NotImplementedError
+
+    def get_by_service_get_page_markers(self, service_id, marker, limit):
+        raise NotImplementedError
+
 
 class BaseEndpointTemplateAPI(object):
     def __init__(self, *args, **kw):

keystone/backends/ldap/api/role.py

@@ -91,7 +91,7 @@ def add_user(self, role_id, user_id, tenant_id=None):
             role_id=role_id, user_id=user_id, tenant_id=tenant_id)
 
     def get_by_service(self, service_id):
-        roles = self.get_all('(serviceId=%s)' % \
+        roles = self.get_all('(service_id=%s)' % \
                     (ldap.filter.escape_filter_chars(service_id),))
         try:
             res = []
@@ -214,6 +214,14 @@ def ref_get_page_markers(self, user_id, tenant_id, marker, limit):
                 all_roles += self.ref_get_all_tenant_roles(user_id, tenant.id)
         return self._get_page_markers(marker, limit, all_roles)
 
+    def get_by_service_get_page(self, service_id, marker, limit):
+        all_roles = self.get_by_service(service_id)
+        return self._get_page(marker, limit, all_roles)
+
+    def get_by_service_get_page_markers(self, service_id, marker, limit):
+        all_roles = self.get_by_service(service_id)
+        return self._get_page_markers(marker, limit, all_roles)
+
     def ref_get_by_role(self, id):
         role_dn = self._id_to_dn(id)
         try:

keystone/backends/sqlalchemy/api/role.py

@@ -65,13 +65,68 @@ def get_page(self, marker, limit, session=None):
             session = get_session()
 
         if marker:
-            return session.query(models.Role).filter("id>:marker").params(\
-                    marker='%s' % marker).order_by(\
+            return session.query(models.Role).filter("id>:marker").params(
+                    marker='%s' % marker).order_by(
                     models.Role.id.desc()).limit(limit).all()
         else:
-            return session.query(models.Role).order_by(\
+            return session.query(models.Role).order_by(
                                 models.Role.id.desc()).limit(limit).all()
 
+    def get_by_service_get_page(self, service_id, marker, limit, session=None):
+        if not session:
+            session = get_session()
+
+        if marker:
+            return session.query(models.Role).filter("id>:marker").params(
+                    marker='%s' % marker).filter_by(
+                    service_id=service_id).order_by(
+                    models.Role.id.desc()).limit(limit).all()
+        else:
+            return session.query(models.Role).filter_by(
+                    service_id=service_id).order_by(
+                                models.Role.id.desc()).limit(limit).all()
+
+    # pylint: disable=R0912
+    def get_by_service_get_page_markers(self,
+            service_id, marker, limit, session=None):
+        if not session:
+            session = get_session()
+        first = session.query(models.Role).filter_by(
+                    service_id=service_id).order_by(
+                    models.Role.id).first()
+        last = session.query(models.Role).filter_by(
+                    service_id=service_id).order_by(
+                    models.Role.id.desc()).first()
+        if first is None:
+            return (None, None)
+        if marker is None:
+            marker = first.id
+        next_page = session.query(models.Role).filter("id > :marker").params(
+                        marker='%s' % marker).filter_by(
+                        service_id=service_id).order_by(
+                        models.Role.id).limit(limit).all()
+        prev_page = session.query(models.Role).filter("id < :marker").params(
+                        marker='%s' % marker).filter_by(
+                        service_id=service_id).order_by(
+                        models.Role.id.desc()).limit(int(limit)).all()
+        if not next_page:
+            next_page = last
+        else:
+            next_page = next_page[-1]
+        if not prev_page:
+            prev_page = first
+        else:
+            prev_page = prev_page[-1]
+        if prev_page.id == marker:
+            prev_page = None
+        else:
+            prev_page = prev_page.id
+        if next_page.id == last.id:
+            next_page = None
+        else:
+            next_page = next_page.id
+        return (prev_page, next_page)
+
     def ref_get_page(self, marker, limit, user_id, tenant_id, session=None):
         if not session:
             session = get_session()
@@ -88,11 +143,11 @@ def ref_get_page(self, marker, limit, user_id, tenant_id, session=None):
         else:
             query = query.filter("tenant_id is null")
         if marker:
-            results = query.filter("id>:marker").params(\
-                    marker='%s' % marker).order_by(\
+            results = query.filter("id>:marker").params(
+                    marker='%s' % marker).order_by(
                     models.UserRoleAssociation.id.desc()).limit(limit).all()
         else:
-            results = query.order_by(\
+            results = query.order_by(
                     models.UserRoleAssociation.id.desc()).limit(limit).all()
 
         for result in results:
@@ -168,30 +223,28 @@ def ref_delete(self, id, session=None):
     def get_page_markers(self, marker, limit, session=None):
         if not session:
             session = get_session()
-        first = session.query(models.Role).order_by(\
+        first = session.query(models.Role).order_by(
                             models.Role.id).first()
-        last = session.query(models.Role).order_by(\
+        last = session.query(models.Role).order_by(
                             models.Role.id.desc()).first()
         if first is None:
             return (None, None)
         if marker is None:
             marker = first.id
         next_page = session.query(models.Role).filter("id > :marker").params(\
-                        marker='%s' % marker).order_by(\
+                        marker='%s' % marker).order_by(
                         models.Role.id).limit(limit).all()
         prev_page = session.query(models.Role).filter("id < :marker").params(\
-                        marker='%s' % marker).order_by(\
+                        marker='%s' % marker).order_by(
                         models.Role.id.desc()).limit(int(limit)).all()
-        if len(next_page) == 0:
+        if not next_page:
             next_page = last
         else:
-            for t in next_page:
-                next_page = t
-        if len(prev_page) == 0:
+            next_page = next_page[-1]
+        if not prev_page:
             prev_page = first
         else:
-            for t in prev_page:
-                prev_page = t
+            prev_page = prev_page[-1]
         if prev_page.id == marker:
             prev_page = None
         else:
@@ -213,7 +266,7 @@ def ref_get_page_markers(self, user_id, tenant_id, marker,
         if hasattr(api.TENANT, 'uid_to_id'):
             tenant_id = api.TENANT.uid_to_id(tenant_id)
 
-        query = session.query(models.UserRoleAssociation).filter_by(\
+        query = session.query(models.UserRoleAssociation).filter_by(
                                             user_id=user_id)
         if tenant_id:
             query = query.filter_by(tenant_id=tenant_id)
@@ -240,16 +293,14 @@ def ref_get_page_markers(self, user_id, tenant_id, marker,
             limit(int(limit)).\
             all()
 
-        if len(next_page) == 0:
+        if not next_page:
             next_page = last
         else:
-            for t in next_page:
-                next_page = t
-        if len(prev_page) == 0:
+            next_page = next_page[-1]
+        if not prev_page:
             prev_page = first
         else:
-            for t in prev_page:
-                prev_page = t
+            prev_page = prev_page[-1]
         if prev_page.id == marker:
             prev_page = None
         else:

keystone/controllers/roles.py

@@ -29,11 +29,24 @@ def delete_role(self, req, role_id):
     def get_roles(self, req):
         role_name = req.GET["name"] if "name" in req.GET else None
         if role_name:
-            tenant = self.identity_service.get_role_by_name(
-                utils.get_auth_token(req), role_name)
-            return utils.send_result(200, req, tenant)
+            return self.__get_roles_by_name(req, role_name)
+        else:
+            return self.__get_all_roles(req)
+
+    def __get_roles_by_name(self, req, role_name):
+        tenant = self.identity_service.get_role_by_name(
+            utils.get_auth_token(req), role_name)
+        return utils.send_result(200, req, tenant)
+
+    def __get_all_roles(self, req):
+        service_id = req.GET["serviceId"] if "serviceId" in req.GET else None
+        marker, limit, url = get_marker_limit_and_url(req)
+        if service_id:
+            roles = self.identity_service.get_roles_by_service(
+                utils.get_auth_token(req), marker, limit, url,
+                service_id)
+            return utils.send_result(200, req, roles)
         else:
-            marker, limit, url = get_marker_limit_and_url(req)
             roles = self.identity_service.get_roles(
                 utils.get_auth_token(req), marker, limit, url)
             return utils.send_result(200, req, roles)

keystone/logic/service.py

@@ -844,19 +844,28 @@ def create_role(admin_token, role):
 
     def get_roles(self, admin_token, marker, limit, url):
         validate_service_admin_token(admin_token)
-
-        ts = []
         droles = api.ROLE.get_page(marker, limit)
-        for drole in droles:
-            ts.append(Role(drole.id, drole.name, drole.desc, drole.service_id))
         prev, next = api.ROLE.get_page_markers(marker, limit)
         links = self.get_links(url, prev, next, limit)
+        ts = self.transform_roles(droles)
         return Roles(ts, links)
 
+    def get_roles_by_service(self, admin_token, marker, limit, url, serviceId):
+        validate_service_admin_token(admin_token)
+        droles = api.ROLE.get_by_service_get_page(serviceId, marker, limit)
+        prev, next = api.ROLE.get_by_service_get_page_markers(
+            serviceId, marker, limit)
+        links = self.get_links(url, prev, next, limit)
+        ts = self.transform_roles(droles)
+        return Roles(ts, links)
+
+    def transform_roles(self, droles):
+        return [Role(drole.id, drole.name, drole.desc, drole.service_id)
+                for drole in droles]
+
     @staticmethod
     def get_role(admin_token, role_id):
         validate_service_admin_token(admin_token)
-
         drole = api.ROLE.get(role_id)
         if not drole:
             raise fault.ItemNotFoundFault("The role could not be found")
@@ -869,7 +878,8 @@ def get_role_by_name(admin_token, role_name):
         drole = api.ROLE.get_by_name(role_name)
         if not drole:
             raise fault.ItemNotFoundFault("The role could not be found")
-        return Role(drole.id, drole.name, drole.desc, drole.service_id)
+        return Role(drole.id, drole.name,
+            drole.desc, drole.service_id)
 
     @staticmethod
     def delete_role(admin_token, role_id):

keystone/test/functional/common.py

@@ -632,10 +632,17 @@ def post_role(self, **kwargs):
             path='/OS-KSADM/roles', **kwargs)
 
     def get_roles(self, **kwargs):
-        """GET /roles"""
+        """GET /OS-KSADM/roles"""
         return self.admin_request(method='GET',
             path='/OS-KSADM/roles', **kwargs)
 
+    def get_roles_by_service(self, service_id, **kwargs):
+        """GET /OS-KSADM/roles"""
+        return self.admin_request(method='GET', path=(
+            '/OS-KSADM/roles?serviceId=%s')
+            % (service_id),
+            **kwargs)
+
     def get_role(self, role_id, **kwargs):
         """GET /roles/{role_id}"""
         return self.admin_request(method='GET',
@@ -1221,8 +1228,11 @@ def create_role(self, role_name=None, role_description=None,
 
         return self.post_role(as_json=data, **kwargs)
 
-    def list_roles(self, **kwargs):
-        return self.get_roles(**kwargs)
+    def list_roles(self, service_id=None, **kwargs):
+        if service_id is None:
+            return self.get_roles(**kwargs)
+        else:
+            return self.get_roles_by_service(service_id, **kwargs)
 
     def fetch_role(self, role_id=None, **kwargs):
         role_id = optional_str(role_id)

keystone/test/functional/test_roles.py

@@ -155,6 +155,45 @@ def test_create_service_role_using_incorrect_role_name(self):
             assert_status=400)
 
 
+class GetRolesByServiceTest(common.FunctionalTestCase):
+    def setUp(self, *args, **kwargs):
+        super(GetRolesByServiceTest, self).setUp(*args, **kwargs)
+        service = self.create_service().json['OS-KSADM:service']
+        role_name = service['name'] + ':' + common.unique_str()
+        role = self.create_role(role_name=role_name,
+            service_id=service['id']).json['role']
+        self.service_id = service['id']
+
+    def tearDown(self, *args, **kwargs):
+        super(GetRolesByServiceTest, self).tearDown(*args, **kwargs)
+
+    def test_get_roles(self):
+        r = self.list_roles(assert_status=200, service_id=self.service_id)
+        self.assertTrue(len(r.json['roles']))
+
+    def test_get_roles_xml(self):
+        r = self.get_roles_by_service(assert_status=200, headers={
+            'Accept': 'application/xml'}, service_id=self.service_id,)
+        self.assertEquals(r.xml.tag, '{%s}roles' % self.xmlns)
+        roles = r.xml.findall('{%s}role' % self.xmlns)
+
+        for role in roles:
+            self.assertIsNotNone(role.get('id'))
+
+    def test_get_roles_exp_token(self):
+        self.fixture_create_expired_token()
+        self.admin_token = self.expired_admin_token
+        self.get_roles_by_service(
+            service_id=self.service_id, assert_status=403)
+
+    def test_get_roles_exp_token_xml(self):
+        self.fixture_create_expired_token()
+        self.admin_token = self.expired_admin_token
+        self.get_roles_by_service(
+            service_id=self.service_id, assert_status=403, headers={
+            'Accept': 'application/xml'})
+
+
 class GetRolesTest(RolesTest):
     def test_get_roles(self):
         r = self.list_roles(assert_status=200)