Merge "Filter images by status and add visibility shared"

This commit is contained in:
Jenkins 2013-02-21 03:54:07 +00:00 committed by Gerrit Code Review
commit 8e43e39025
8 changed files with 268 additions and 48 deletions

View File

@ -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:

View File

@ -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 = {}

View File

@ -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

View File

@ -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

View File

@ -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()

View File

@ -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))

View File

@ -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]

View File

@ -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)