Skip to content

Commit

Permalink
Fixes is_image_visible to not use deleted key
Browse files Browse the repository at this point in the history
The 'is_image_visible' method is looking for deleted members while
they are already being filtered in the query. Registry while validating
all the members gets a KeyError as the key deleted does not exist.

Fixes Bug: 1077106

Change-Id: Ia53b6be3b2f740357df86e6c7a6a257a990c3aab
  • Loading branch information
Nikhil Komawar committed Nov 13, 2012
1 parent 2bd8ee5 commit d635c41
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 4 deletions.
9 changes: 6 additions & 3 deletions glance/db/simple/api.py
Expand Up @@ -207,8 +207,11 @@ def image_get(context, image_id, session=None, force_show_deleted=False):
and (not image.get('is_public')) \
and (image['owner'] is not None) \
and (image['owner'] != context.owner):
LOG.info('Unable to get unowned image')
raise exception.Forbidden()
members = image_member_find(context, image_id=image_id,
member=context.owner)
if not members:
LOG.info('Unable to get unowned image')
raise exception.Forbidden()

return image

Expand Down Expand Up @@ -467,7 +470,7 @@ def is_image_visible(context, image):
image_id=image['id'],
member=context.owner)
if members:
return not members[0]['deleted']
return True

# Private image
return False
2 changes: 1 addition & 1 deletion glance/db/sqlalchemy/api.py
Expand Up @@ -323,7 +323,7 @@ def is_image_visible(context, image):
image_id=image['id'],
member=context.owner)
if members:
return not members[0]['deleted']
return True

# Private image
return False
Expand Down
47 changes: 47 additions & 0 deletions glance/tests/functional/db/base.py
Expand Up @@ -24,6 +24,7 @@
from glance.common import utils
from glance import context
from glance.openstack.common import timeutils
from glance.openstack.common import uuidutils
import glance.tests.functional.db as db_tests
from glance.tests.unit import base

Expand Down Expand Up @@ -410,6 +411,52 @@ def test_image_get_all_limit_marker(self):
images = self.db_api.image_get_all(self.context, limit=2)
self.assertEquals(2, len(images))

def test_image_get_multiple_members(self):
TENANT1 = uuidutils.generate_uuid()
TENANT2 = uuidutils.generate_uuid()
ctxt1 = context.RequestContext(is_admin=False, tenant=TENANT1,
owner_is_tenant=True)
ctxt2 = context.RequestContext(is_admin=False, user=TENANT2,
owner_is_tenant=False)
UUIDX = uuidutils.generate_uuid()
#we need private image and context.owner should not match image owner
self.db_api.image_create(ctxt1, {'id': UUIDX,
'status': 'queued',
'is_public': False,
'owner': TENANT1})
values = {'image_id': UUIDX, 'member': TENANT2, 'can_share': False}
self.db_api.image_member_create(ctxt1, values)

image = self.db_api.image_get(ctxt2, UUIDX)
self.assertEquals(UUIDX, image['id'])

def test_is_image_visible(self):
TENANT1 = uuidutils.generate_uuid()
TENANT2 = uuidutils.generate_uuid()
ctxt1 = context.RequestContext(is_admin=False, tenant=TENANT1,
owner_is_tenant=True)
ctxt2 = context.RequestContext(is_admin=False, user=TENANT2,
owner_is_tenant=False)
UUIDX = uuidutils.generate_uuid()
#we need private image and context.owner should not match image owner
image = self.db_api.image_create(ctxt1, {'id': UUIDX,
'status': 'queued',
'is_public': False,
'owner': TENANT1})

values = {'image_id': UUIDX, 'member': TENANT2, 'can_share': False}
self.db_api.image_member_create(ctxt1, values)

result = self.db_api.is_image_visible(ctxt2, image)
self.assertTrue(result)

# image should not be visible for a deleted memeber
members = self.db_api.image_member_find(ctxt1, image_id=UUIDX)
self.db_api.image_member_delete(ctxt1, members[0]['id'])

result = self.db_api.is_image_visible(ctxt2, image)
self.assertFalse(result)

def test_image_tag_create(self):
tag = self.db_api.image_tag_create(self.context, UUID1, 'snap')
self.assertEqual('snap', tag)
Expand Down

0 comments on commit d635c41

Please sign in to comment.