Filter images by status and add visibility shared

This patch allows the user to filter images by status.
Whenever a user does an image list, the following are displayed:
(1) public images
(2) images owned by the user
(3) images whom the user is a member of and has set status to accepted
The user can change the status in (3) and also choose to display
the images the user has been requested to be a member of,
rejected to be a member of, or all images irrespective of the status
set by user

If visibility filter can be shared, public or private. Where shared
filter displays images which the user is a member of. This can be
used in conjunction with member_status

Related to bp glance-api-v2-image-sharing

Change-Id: I052c0947f9ad17ef1b14af0ad0f51b886afd0d7a

glance/api/v2/images.py

@@ -65,7 +65,7 @@ class ImagesController(object):
         return image
 
     def index(self, req, marker=None, limit=None, sort_key='created_at',
-              sort_dir='desc', filters=None):
+              sort_dir='desc', filters=None, member_status='accepted'):
         result = {}
         if filters is None:
             filters = {}
@@ -79,7 +79,8 @@ class ImagesController(object):
         try:
             images = image_repo.list(marker=marker, limit=limit,
                                      sort_key=sort_key, sort_dir=sort_dir,
-                                     filters=filters)
+                                     filters=filters,
+                                     member_status=member_status)
             if len(images) != 0 and len(images) == limit:
                 result['next_marker'] = images[-1].image_id
         except (exception.NotFound, exception.InvalidSortKey,
@@ -331,12 +332,17 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
 
         return sort_dir
 
+    def _validate_member_status(self, member_status):
+        if member_status not in ['pending', 'accepted', 'rejected', 'all']:
+            msg = _('Invalid status: %s' % member_status)
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        return member_status
+
     def _get_filters(self, filters):
-        visibility = filters.pop('visibility', None)
+        visibility = filters.get('visibility', None)
         if visibility:
-            if visibility in ['public', 'private']:
+            if visibility not in ['public', 'private', 'shared']:
-                filters['is_public'] = visibility == 'public'
-            else:
                 msg = _('Invalid visibility value: %s') % visibility
                 raise webob.exc.HTTPBadRequest(explanation=msg)
 
@@ -347,10 +353,12 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
         limit = params.pop('limit', None)
         marker = params.pop('marker', None)
         sort_dir = params.pop('sort_dir', 'desc')
+        member_status = params.pop('member_status', 'accepted')
         query_params = {
             'sort_key': params.pop('sort_key', 'created_at'),
             'sort_dir': self._validate_sort_dir(sort_dir),
             'filters': self._get_filters(params),
+            'member_status': self._validate_member_status(member_status),
         }
 
         if marker is not None:

glance/db/__init__.py

@@ -79,11 +79,11 @@ class ImageRepo(object):
         return ImageProxy(image, self.context, self.db_api)
 
     def list(self, marker=None, limit=None, sort_key='created_at',
-             sort_dir='desc', filters=None):
+             sort_dir='desc', filters=None, member_status='accepted'):
-        db_filters = self._translate_filters(filters)
         db_api_images = self.db_api.image_get_all(
-                self.context, filters=db_filters, marker=marker, limit=limit,
+                self.context, filters=filters, marker=marker, limit=limit,
-                sort_key=sort_key, sort_dir=sort_dir)
+                sort_key=sort_key, sort_dir=sort_dir,
+                member_status=member_status)
         images = []
         for db_api_image in db_api_images:
             tags = self.db_api.image_tag_get_all(self.context,
@@ -92,17 +92,6 @@ class ImageRepo(object):
             images.append(image)
         return images
 
-    def _translate_filters(self, filters):
-        db_filters = {}
-        if filters is None:
-            return None
-        for key, value in filters.iteritems():
-            if key == 'visibility':
-                db_filters['is_public'] = value == 'public'
-            else:
-                db_filters[key] = value
-        return db_filters
-
     def _format_image_from_db(self, db_image, db_tags):
         visibility = 'public' if db_image['is_public'] else 'private'
         properties = {}

glance/db/simple/api.py

@@ -1,4 +1,4 @@
-# Copyright 2012 OpenStack Foundation
+# Copyright 2012 OpenStack, Foundation
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -124,7 +124,7 @@ def _image_format(image_id, **values):
     return image
 
 
-def _filter_images(images, filters, context):
+def _filter_images(images, filters, context, status='accepted'):
     filtered_images = []
     if 'properties' in filters:
         prop_filter = filters.pop('properties')
@@ -133,12 +133,33 @@ def _filter_images(images, filters, context):
     if 'is_public' in filters and filters['is_public'] is None:
         filters.pop('is_public')
 
+    if status == 'all':
+        status = None
+
+    visibility = filters.pop('visibility', None)
+
     for image in images:
+        member = image_member_find(context, image_id=image['id'],
+                                   member=context.owner, status=status)
+        is_member = len(member) > 0
         has_ownership = context.owner and image['owner'] == context.owner
-        can_see = image['is_public'] or has_ownership or context.is_admin
+        can_see = (image['is_public'] or has_ownership or context.is_admin or
+                   is_member)
         if not can_see:
             continue
 
+        if visibility:
+            if visibility == 'public':
+                filters['is_public'] = True
+                if not image['is_public']:
+                    continue
+            elif visibility == 'private':
+                filters['is_public'] = False
+                if not (has_ownership or context.is_admin):
+                    continue
+            elif visibility == 'shared':
+                if not is_member:
+                    continue
         add = True
         for k, value in filters.iteritems():
             key = k
@@ -172,14 +193,16 @@ def _filter_images(images, filters, context):
     return filtered_images
 
 
-def _do_pagination(context, images, marker, limit, show_deleted):
+def _do_pagination(context, images, marker, limit, show_deleted,
+                   status='accepted'):
     start = 0
     end = -1
     if marker is None:
         start = 0
     else:
         # Check that the image is accessible
-        _image_get(context, marker, force_show_deleted=show_deleted)
+        _image_get(context, marker, force_show_deleted=show_deleted,
+                   status=status)
 
         for i, image in enumerate(images):
             if image['id'] == marker:
@@ -203,7 +226,7 @@ def _sort_images(images, sort_key, sort_dir):
     return images
 
 
-def _image_get(context, image_id, force_show_deleted=False):
+def _image_get(context, image_id, force_show_deleted=False, status=None):
     try:
         image = DATA['images'][image_id]
     except KeyError:
@@ -229,10 +252,11 @@ def image_get(context, image_id, session=None, force_show_deleted=False):
 
 @log_call
 def image_get_all(context, filters=None, marker=None, limit=None,
-                  sort_key='created_at', sort_dir='desc'):
+                  sort_key='created_at', sort_dir='desc',
+                  member_status='accepted'):
     filters = filters or {}
     images = DATA['images'].values()
-    images = _filter_images(images, filters, context)
+    images = _filter_images(images, filters, context, member_status)
     images = _sort_images(images, sort_key, sort_dir)
     images = _do_pagination(context, images, marker, limit,
                             filters.get('deleted'))
@@ -462,7 +486,7 @@ def is_image_sharable(context, image, **kwargs):
     return member['can_share']
 
 
-def is_image_visible(context, image):
+def is_image_visible(context, image, status=None):
     """Return True if the image is visible in this context."""
     # Is admin == image visible
     if context.is_admin:
@@ -482,10 +506,13 @@ def is_image_visible(context, image):
             return True
 
         # Figure out if this image is shared with that tenant
+        if status == 'all':
+            status = None
         members = image_member_find(context,
                                     image_id=image['id'],
-                                    member=context.owner)
+                                    member=context.owner,
-        if members:
+                                    status=status)
+        if len(members) > 0:
             return True
 
     # Private image

glance/db/sqlalchemy/api.py

@@ -33,9 +33,11 @@ import sqlalchemy.sql as sa_sql
 from glance.common import exception
 from glance.db.sqlalchemy import migration
 from glance.db.sqlalchemy import models
+from glance.openstack.common import cfg
 import glance.openstack.common.log as os_logging
 from glance.openstack.common import timeutils
 
+
 _ENGINE = None
 _MAKER = None
 _MAX_RETRIES = None
@@ -338,7 +340,7 @@ def is_image_sharable(context, image, **kwargs):
     return member['can_share']
 
 
-def is_image_visible(context, image):
+def is_image_visible(context, image, status=None):
     """Return True if the image is visible in this context."""
     # Is admin == image visible
     if context.is_admin:
@@ -360,7 +362,8 @@ def is_image_visible(context, image):
         # Figure out if this image is shared with that tenant
         members = image_member_find(context,
                                     image_id=image['id'],
-                                    member=context.owner)
+                                    member=context.owner,
+                                    status=status)
         if members:
             return True
 
@@ -467,7 +470,8 @@ def paginate_query(query, model, limit, sort_keys, marker=None,
 
 
 def image_get_all(context, filters=None, marker=None, limit=None,
-                  sort_key='created_at', sort_dir='desc'):
+                  sort_key='created_at', sort_dir='desc',
+                  member_status='accepted'):
     """
     Get all images that match zero or more filters.
 
@@ -493,13 +497,37 @@ def image_get_all(context, filters=None, marker=None, limit=None,
         visibility_filters = [models.Image.is_public == True]
 
         if context.owner is not None:
-            visibility_filters.extend([
+            if member_status == 'all':
-                models.Image.owner == context.owner,
+                visibility_filters.extend([
-                models.Image.members.any(member=context.owner, deleted=False),
+                    models.Image.owner == context.owner,
-            ])
+                    models.Image.members.any(member=context.owner,
+                                             deleted=False),
+                ])
+            else:
+                visibility_filters.extend([
+                    models.Image.owner == context.owner,
+                    models.Image.members.any(member=context.owner,
+                                             deleted=False,
+                                             status=member_status),
+                ])
 
         query = query.filter(sa_sql.or_(*visibility_filters))
 
+    if 'visibility' in filters:
+        visibility = filters.pop('visibility')
+        if visibility == 'public':
+            query = query.filter(models.Image.is_public == True)
+            filters['is_public'] = True
+        elif visibility == 'private':
+            filters['is_public'] = False
+            if (not context.is_admin) and context.owner is not None:
+                query = query.filter(
+                            models.Image.owner == context.owner)
+        else:
+            query = query.filter(
+                        models.Image.members.any(member=context.owner,
+                                                 deleted=False))
+
     showing_deleted = False
     if 'changes-since' in filters:
         # normalize timestamp to UTC, as sqlalchemy doesn't appear to

glance/tests/functional/db/base.py

@@ -430,6 +430,37 @@ class DriverTests(object):
         image = self.db_api.image_get(ctxt2, UUIDX)
         self.assertEquals(UUIDX, image['id'])
 
+        # by default get_all displays only images with status 'accepted'
+        images = self.db_api.image_get_all(ctxt2)
+        self.assertEquals(3, len(images))
+
+        # filter by rejected
+        images = self.db_api.image_get_all(ctxt2, member_status='rejected')
+        self.assertEquals(3, len(images))
+
+        # filter by visibility
+        images = self.db_api.image_get_all(ctxt2,
+                                           filters={'visibility': 'shared'})
+        self.assertEquals(0, len(images))
+
+        # filter by visibility
+        images = self.db_api.image_get_all(ctxt2, member_status='pending',
+                                           filters={'visibility': 'shared'})
+        self.assertEquals(1, len(images))
+
+        # filter by visibility
+        images = self.db_api.image_get_all(ctxt2, member_status='all',
+                                           filters={'visibility': 'shared'})
+        self.assertEquals(1, len(images))
+
+        # filter by status pending
+        images = self.db_api.image_get_all(ctxt2, member_status='pending')
+        self.assertEquals(4, len(images))
+
+        # filter by status all
+        images = self.db_api.image_get_all(ctxt2, member_status='all')
+        self.assertEquals(4, len(images))
+
     def test_is_image_visible(self):
         TENANT1 = uuidutils.generate_uuid()
         TENANT2 = uuidutils.generate_uuid()

glance/tests/functional/v2/test_images.py

@@ -801,6 +801,13 @@ class TestImageMembers(functional.FunctionalTest):
         images = json.loads(response.text)['images']
         self.assertEqual(4, len(images))
 
+        # Image list should contain 3 images for TENANT3
+        path = self._url('/v2/images')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(3, len(images))
+
         # Add Image member for tenant1-private image
         path = self._url('/v2/images/%s/members' % image_fixture[1]['id'])
         body = json.dumps({'member': TENANT3})
@@ -814,6 +821,68 @@ class TestImageMembers(functional.FunctionalTest):
         self.assertTrue('updated_at' in image_member)
         self.assertEqual('pending', image_member['status'])
 
+        # Image list should contain 3 images for TENANT3
+        path = self._url('/v2/images')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(3, len(images))
+
+        # Image list should contain 0 shared images for TENANT3
+        # becuase default is accepted
+        path = self._url('/v2/images?visibility=shared')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(0, len(images))
+
+        # Image list should contain 4 images for TENANT3 with status pending
+        path = self._url('/v2/images?member_status=pending')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(4, len(images))
+
+        # Image list should contain 4 images for TENANT3 with status all
+        path = self._url('/v2/images?member_status=all')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(4, len(images))
+
+        # Image list should contain 1 image for TENANT3 with status pending
+        # and visibility shared
+        path = self._url('/v2/images?member_status=pending&visibility=shared')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(1, len(images))
+        self.assertEqual(images[0]['name'], 'tenant1-private')
+
+        # Image list should contain 0 image for TENANT3 with status rejected
+        # and visibility shared
+        path = self._url('/v2/images?member_status=rejected&visibility=shared')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(0, len(images))
+
+        # Image list should contain 0 image for TENANT3 with status accepted
+        # and visibility shared
+        path = self._url('/v2/images?member_status=accepted&visibility=shared')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(0, len(images))
+
+        # Image list should contain 0 image for TENANT3 with status accepted
+        # and visibility private
+        path = self._url('/v2/images?visibility=private')
+        response = requests.get(path, headers=get_header(TENANT3))
+        self.assertEqual(200, response.status_code)
+        images = json.loads(response.text)['images']
+        self.assertEqual(0, len(images))
+
         # Image tenant2-private's image members list should contain no members
         path = self._url('/v2/images/%s/members' % image_fixture[3]['id'])
         response = requests.get(path, headers=get_header('tenant2'))
@@ -828,13 +897,6 @@ class TestImageMembers(functional.FunctionalTest):
         response = requests.put(path, headers=get_header('tenant1'), data=body)
         self.assertEqual(403, response.status_code)
 
-        # Image list should contain 4 images for TENANT3
-        path = self._url('/v2/images')
-        response = requests.get(path, headers=get_header(TENANT3))
-        self.assertEqual(200, response.status_code)
-        images = json.loads(response.text)['images']
-        self.assertEqual(4, len(images))
-
         # Tenant 3 can change status of image
         path = self._url('/v2/images/%s/members/%s' % (image_fixture[1]['id'],
                                                        TENANT3))

glance/tests/unit/test_db.py

@@ -76,6 +76,7 @@ class TestImageRepo(test_utils.BaseTestCase):
         self.image_repo = glance.db.ImageRepo(self.context, self.db)
         self.image_factory = glance.domain.ImageFactory()
         self._create_images()
+        self._create_image_members()
         super(TestImageRepo, self).setUp()
 
     def _create_images(self):
@@ -93,6 +94,14 @@ class TestImageRepo(test_utils.BaseTestCase):
 
         self.db.image_tag_set_all(None, UUID1, ['ping', 'pong'])
 
+    def _create_image_members(self):
+        self.image_members = [
+            _db_image_member_fixture(UUID2, TENANT2),
+            _db_image_member_fixture(UUID2, TENANT3, status='accepted'),
+        ]
+        [self.db.image_member_create(None, image_member)
+            for image_member in self.image_members]
+
     def test_get(self):
         image = self.image_repo.get(UUID1)
         self.assertEquals(image.image_id, UUID1)
@@ -115,6 +124,25 @@ class TestImageRepo(test_utils.BaseTestCase):
         image_ids = set([i.image_id for i in images])
         self.assertEqual(set([UUID1, UUID2, UUID3]), image_ids)
 
+    def _do_test_list_status(self, status, expected):
+        self.context = glance.context.RequestContext(
+                        user=USER1, tenant=TENANT3)
+        self.image_repo = glance.db.ImageRepo(self.context, self.db)
+        images = self.image_repo.list(member_status=status)
+        self.assertEqual(expected, len(images))
+
+    def test_list_status(self):
+        self._do_test_list_status(None, 3)
+
+    def test_list_status_pending(self):
+        self._do_test_list_status('pending', 2)
+
+    def test_list_status_rejected(self):
+        self._do_test_list_status('rejected', 2)
+
+    def test_list_status_all(self):
+        self._do_test_list_status('all', 3)
+
     def test_list_with_marker(self):
         full_images = self.image_repo.list()
         full_ids = [i.image_id for i in full_images]

glance/tests/unit/v2/test_images_resource.py

@@ -92,6 +92,15 @@ def _domain_fixture(id, **kwargs):
     return glance.domain.Image(**properties)
 
 
+def _db_image_member_fixture(image_id, member_id, **kwargs):
+    obj = {
+        'image_id': image_id,
+        'member': member_id,
+    }
+    obj.update(kwargs)
+    return obj
+
+
 class TestImagesController(test_utils.BaseTestCase):
 
     def setUp(self):
@@ -101,6 +110,7 @@ class TestImagesController(test_utils.BaseTestCase):
         self.notifier = unit_test_utils.FakeNotifier()
         self.store = unit_test_utils.FakeStoreAPI()
         self._create_images()
+        self._create_image_members()
         self.controller = glance.api.v2.images.ImagesController(self.db,
                                                                 self.policy,
                                                                 self.notifier,
@@ -122,6 +132,15 @@ class TestImagesController(test_utils.BaseTestCase):
 
         self.db.image_tag_set_all(None, UUID1, ['ping', 'pong'])
 
+    def _create_image_members(self):
+        self.image_members = [
+            _db_image_member_fixture(UUID4, TENANT2),
+            _db_image_member_fixture(UUID4, TENANT3,
+                                     status='accepted'),
+        ]
+        [self.db.image_member_create(None, image_member)
+            for image_member in self.image_members]
+
     def test_index(self):
         self.config(limit_param_default=1, api_limit_max=3)
         request = unit_test_utils.get_fake_request()
@@ -131,6 +150,23 @@ class TestImagesController(test_utils.BaseTestCase):
         expected = set([UUID3])
         self.assertEqual(actual, expected)
 
+    def test_index_member_status_accepted(self):
+        self.config(limit_param_default=5, api_limit_max=5)
+        request = unit_test_utils.get_fake_request(tenant=TENANT2)
+        output = self.controller.index(request)
+        self.assertEqual(3, len(output['images']))
+        actual = set([image.image_id for image in output['images']])
+        expected = set([UUID1, UUID2, UUID3])
+        # can see only the public image
+        self.assertEqual(actual, expected)
+
+        request = unit_test_utils.get_fake_request(tenant=TENANT3)
+        output = self.controller.index(request)
+        self.assertEqual(4, len(output['images']))
+        actual = set([image.image_id for image in output['images']])
+        expected = set([UUID1, UUID2, UUID3, UUID4])
+        self.assertEqual(actual, expected)
+
     def test_index_admin(self):
         request = unit_test_utils.get_fake_request(is_admin=True)
         output = self.controller.index(request)
@@ -240,7 +276,8 @@ class TestImagesController(test_utils.BaseTestCase):
         self.db.image_create(None, image)
         path = '/images?visibility=private'
         request = unit_test_utils.get_fake_request(path, is_admin=True)
-        output = self.controller.index(request, filters={'is_public': False})
+        output = self.controller.index(request,
+                                       filters={'visibility': 'private'})
         self.assertEqual(2, len(output['images']))
 
     def test_index_with_many_filters(self):
@@ -1061,12 +1098,13 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
 
     def test_index(self):
         marker = uuidutils.generate_uuid()
-        path = '/images?limit=1&marker=%s' % marker
+        path = '/images?limit=1&marker=%s&member_status=pending' % marker
         request = unit_test_utils.get_fake_request(path)
         expected = {'limit': 1,
                     'marker': marker,
                     'sort_key': 'created_at',
                     'sort_dir': 'desc',
+                    'member_status': 'pending',
                     'filters': {}}
         output = self.deserializer.index(request)
         self.assertEqual(output, expected)
@@ -1112,6 +1150,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         request = unit_test_utils.get_fake_request('/images?limit=0')
         expected = {'limit': 0,
                     'sort_key': 'created_at',
+                    'member_status': 'accepted',
                     'sort_dir': 'desc',
                     'filters': {}}
         output = self.deserializer.index(request)
@@ -1127,6 +1166,12 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.deserializer.index, request)
 
+    def test_index_invalid_status(self):
+        path = '/images?member_status=blah'
+        request = unit_test_utils.get_fake_request(path)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.deserializer.index, request)
+
     def test_index_marker(self):
         marker = uuidutils.generate_uuid()
         path = '/images?marker=%s' % marker
@@ -1150,6 +1195,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         expected = {
             'sort_key': 'id',
             'sort_dir': 'desc',
+            'member_status': 'accepted',
             'filters': {}
         }
         self.assertEqual(output, expected)
@@ -1160,6 +1206,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         expected = {
             'sort_key': 'created_at',
             'sort_dir': 'asc',
+            'member_status': 'accepted',
             'filters': {}}
         self.assertEqual(output, expected)