diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 3312e322a8..3be68e98ec 100644 --- a/glance/api/v2/images.py +++ b/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']: - filters['is_public'] = visibility == 'public' - else: + if visibility not in ['public', 'private', 'shared']: 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: diff --git a/glance/db/__init__.py b/glance/db/__init__.py index e22326500e..59f2677e99 100644 --- a/glance/db/__init__.py +++ b/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): - db_filters = self._translate_filters(filters) + sort_dir='desc', filters=None, member_status='accepted'): db_api_images = self.db_api.image_get_all( - self.context, filters=db_filters, marker=marker, limit=limit, - sort_key=sort_key, sort_dir=sort_dir) + self.context, filters=filters, marker=marker, limit=limit, + 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 = {} diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index bb28e5fe17..a5d8948dc1 100644 --- a/glance/db/simple/api.py +++ b/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) - if members: + member=context.owner, + status=status) + if len(members) > 0: return True # Private image diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 116a0e8f00..00d5c4d0c6 100644 --- a/glance/db/sqlalchemy/api.py +++ b/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([ - models.Image.owner == context.owner, - models.Image.members.any(member=context.owner, deleted=False), - ]) + if member_status == 'all': + visibility_filters.extend([ + 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 diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py index 8da21d4c17..db4af50b5f 100644 --- a/glance/tests/functional/db/base.py +++ b/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() diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index 5b2100c2a8..6f94922794 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/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)) diff --git a/glance/tests/unit/test_db.py b/glance/tests/unit/test_db.py index 9c8d58b90a..ac0576f0d6 100644 --- a/glance/tests/unit/test_db.py +++ b/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] diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index e114f8c749..f363279cbf 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/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)