Add a member field to Image when appropriate
As noted by lbragstad, we need to make ImageTarget contain a member field so that we can generically apply policies and be able to properly include images for which we are a member. This was hacked into place for ImageRepoProxy.get() but in order to apply it generally to listing and other ops, we need to formalize that. Partially-Implements: blueprint policy-refactor Change-Id: I92d3792602a69922078d109095ad8ac9afc89d14
This commit is contained in:
parent
ba37ea3227
commit
8ddbdb9526
@ -333,6 +333,7 @@ class ImmutableImageProxy(object):
|
|||||||
virtual_size = _immutable_attr('base', 'virtual_size')
|
virtual_size = _immutable_attr('base', 'virtual_size')
|
||||||
extra_properties = _immutable_attr('base', 'extra_properties',
|
extra_properties = _immutable_attr('base', 'extra_properties',
|
||||||
proxy=ImmutableProperties)
|
proxy=ImmutableProperties)
|
||||||
|
member = _immutable_attr('base', 'member')
|
||||||
tags = _immutable_attr('base', 'tags', proxy=ImmutableTags)
|
tags = _immutable_attr('base', 'tags', proxy=ImmutableTags)
|
||||||
|
|
||||||
def delete(self):
|
def delete(self):
|
||||||
|
@ -548,12 +548,13 @@ class ImageTarget(abc.Mapping):
|
|||||||
yield alias
|
yield alias
|
||||||
|
|
||||||
def key_transforms(self, key):
|
def key_transforms(self, key):
|
||||||
if key == 'id':
|
transforms = {
|
||||||
key = 'image_id'
|
'id': 'image_id',
|
||||||
elif key == 'project_id':
|
'project_id': 'owner',
|
||||||
key = 'owner'
|
'member_id': 'member',
|
||||||
|
}
|
||||||
|
|
||||||
return key
|
return transforms.get(key, key)
|
||||||
|
|
||||||
|
|
||||||
# Metadef Namespace classes
|
# Metadef Namespace classes
|
||||||
|
@ -105,6 +105,18 @@ class ImageRepo(object):
|
|||||||
key = CONF.metadata_encryption_key
|
key = CONF.metadata_encryption_key
|
||||||
for l in locations:
|
for l in locations:
|
||||||
l['url'] = crypt.urlsafe_decrypt(key, l['url'])
|
l['url'] = crypt.urlsafe_decrypt(key, l['url'])
|
||||||
|
|
||||||
|
# NOTE(danms): If the image is shared and we are not the
|
||||||
|
# owner, we must have found it because we are a member. Set
|
||||||
|
# our tenant on the image as 'member' for policy checks in the
|
||||||
|
# upper layers. For any other image stage, we found the image
|
||||||
|
# some other way, so leave member=None.
|
||||||
|
if (db_image['visibility'] == 'shared' and
|
||||||
|
self.context.owner != db_image['owner']):
|
||||||
|
member = self.context.owner
|
||||||
|
else:
|
||||||
|
member = None
|
||||||
|
|
||||||
return glance.domain.Image(
|
return glance.domain.Image(
|
||||||
image_id=db_image['id'],
|
image_id=db_image['id'],
|
||||||
name=db_image['name'],
|
name=db_image['name'],
|
||||||
@ -127,6 +139,7 @@ class ImageRepo(object):
|
|||||||
extra_properties=properties,
|
extra_properties=properties,
|
||||||
tags=db_tags,
|
tags=db_tags,
|
||||||
os_hidden=db_image['os_hidden'],
|
os_hidden=db_image['os_hidden'],
|
||||||
|
member=member,
|
||||||
)
|
)
|
||||||
|
|
||||||
def _format_image_to_db(self, image):
|
def _format_image_to_db(self, image):
|
||||||
|
@ -138,6 +138,7 @@ class Image(object):
|
|||||||
extra_properties = kwargs.pop('extra_properties', {})
|
extra_properties = kwargs.pop('extra_properties', {})
|
||||||
self.extra_properties = ExtraProperties(extra_properties)
|
self.extra_properties = ExtraProperties(extra_properties)
|
||||||
self.tags = kwargs.pop('tags', [])
|
self.tags = kwargs.pop('tags', [])
|
||||||
|
self.member = kwargs.pop('member', None)
|
||||||
if kwargs:
|
if kwargs:
|
||||||
message = _("__init__() got unexpected keyword argument '%s'")
|
message = _("__init__() got unexpected keyword argument '%s'")
|
||||||
raise TypeError(message % list(kwargs.keys())[0])
|
raise TypeError(message % list(kwargs.keys())[0])
|
||||||
|
@ -194,6 +194,7 @@ class Image(object):
|
|||||||
virtual_size = _proxy('base', 'virtual_size')
|
virtual_size = _proxy('base', 'virtual_size')
|
||||||
extra_properties = _proxy('base', 'extra_properties')
|
extra_properties = _proxy('base', 'extra_properties')
|
||||||
tags = _proxy('base', 'tags')
|
tags = _proxy('base', 'tags')
|
||||||
|
member = _proxy('base', 'member')
|
||||||
|
|
||||||
def delete(self):
|
def delete(self):
|
||||||
self.base.delete()
|
self.base.delete()
|
||||||
|
@ -56,6 +56,7 @@ class ImageStub(object):
|
|||||||
self.owner = owner
|
self.owner = owner
|
||||||
self.virtual_size = 0
|
self.virtual_size = 0
|
||||||
self.tags = []
|
self.tags = []
|
||||||
|
self.member = self.owner
|
||||||
|
|
||||||
|
|
||||||
class TestCacheMiddlewareURLMatching(testtools.TestCase):
|
class TestCacheMiddlewareURLMatching(testtools.TestCase):
|
||||||
|
@ -21,6 +21,7 @@ import uuid
|
|||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_db import exception as db_exc
|
from oslo_db import exception as db_exc
|
||||||
from oslo_utils import encodeutils
|
from oslo_utils import encodeutils
|
||||||
|
from oslo_utils.fixture import uuidsentinel as uuids
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
from sqlalchemy import orm as sa_orm
|
from sqlalchemy import orm as sa_orm
|
||||||
|
|
||||||
@ -309,6 +310,27 @@ class TestImageRepo(test_utils.BaseTestCase):
|
|||||||
image_ids = set([i.image_id for i in images])
|
image_ids = set([i.image_id for i in images])
|
||||||
self.assertEqual(set([UUID2]), image_ids)
|
self.assertEqual(set([UUID2]), image_ids)
|
||||||
|
|
||||||
|
def test_list_shared_images_other_tenant(self):
|
||||||
|
# Create a private image owned by TENANT3
|
||||||
|
image5 = _db_fixture(uuids.image5, owner=TENANT3,
|
||||||
|
name='5', size=512, is_public=False)
|
||||||
|
self.db.image_create(None, image5)
|
||||||
|
|
||||||
|
# Get a repo as TENANT3, since it has access to public,
|
||||||
|
# shared, and private images
|
||||||
|
context = glance.context.RequestContext(user=USER1, tenant=TENANT3)
|
||||||
|
image_repo = glance.db.ImageRepo(context, self.db)
|
||||||
|
images = {i.image_id: i for i in image_repo.list()}
|
||||||
|
|
||||||
|
# No member set for public image UUID1
|
||||||
|
self.assertIsNone(images[UUID1].member)
|
||||||
|
|
||||||
|
# Member should be set to our tenant id for shared image UUID2
|
||||||
|
self.assertEqual(TENANT3, images[UUID2].member)
|
||||||
|
|
||||||
|
# No member set for private image5
|
||||||
|
self.assertIsNone(images[uuids.image5].member)
|
||||||
|
|
||||||
def test_list_all_images(self):
|
def test_list_all_images(self):
|
||||||
filters = {'visibility': 'all'}
|
filters = {'visibility': 'all'}
|
||||||
images = self.image_repo.list(filters=filters)
|
images = self.image_repo.list(filters=filters)
|
||||||
|
@ -17,8 +17,10 @@ from unittest import mock
|
|||||||
|
|
||||||
from glance.api import authorization
|
from glance.api import authorization
|
||||||
from glance.api import property_protections
|
from glance.api import property_protections
|
||||||
|
from glance import context
|
||||||
from glance import gateway
|
from glance import gateway
|
||||||
from glance import notifier
|
from glance import notifier
|
||||||
|
from glance.tests.unit import utils as unit_test_utils
|
||||||
import glance.tests.utils as test_utils
|
import glance.tests.utils as test_utils
|
||||||
|
|
||||||
|
|
||||||
@ -90,3 +92,24 @@ class TestGateway(test_utils.BaseTestCase):
|
|||||||
repo = self.gateway.get_repo(self.context, authorization_layer=False)
|
repo = self.gateway.get_repo(self.context, authorization_layer=False)
|
||||||
self.assertIsInstance(repo,
|
self.assertIsInstance(repo,
|
||||||
property_protections.ProtectedImageRepoProxy)
|
property_protections.ProtectedImageRepoProxy)
|
||||||
|
|
||||||
|
def test_get_repo_member_property(self):
|
||||||
|
"""Test that the image.member property is propagated all the way from
|
||||||
|
the DB to the top of the gateway repo stack.
|
||||||
|
"""
|
||||||
|
db_api = unit_test_utils.FakeDB()
|
||||||
|
gw = gateway.Gateway(db_api=db_api)
|
||||||
|
|
||||||
|
# Get the UUID1 image as TENANT1
|
||||||
|
ctxt = context.RequestContext(tenant=unit_test_utils.TENANT1)
|
||||||
|
repo = gw.get_repo(ctxt)
|
||||||
|
image = repo.get(unit_test_utils.UUID1)
|
||||||
|
# We own the image, so member is None
|
||||||
|
self.assertIsNone(image.member)
|
||||||
|
|
||||||
|
# Get the UUID1 image as TENANT2
|
||||||
|
ctxt = context.RequestContext(tenant=unit_test_utils.TENANT2)
|
||||||
|
repo = gw.get_repo(ctxt)
|
||||||
|
image = repo.get(unit_test_utils.UUID1)
|
||||||
|
# We are a member, so member is our tenant id
|
||||||
|
self.assertEqual(unit_test_utils.TENANT2, image.member)
|
||||||
|
@ -96,6 +96,7 @@ class ImageStub(object):
|
|||||||
self.virtual_size = 0
|
self.virtual_size = 0
|
||||||
self.tags = []
|
self.tags = []
|
||||||
self.os_hidden = os_hidden
|
self.os_hidden = os_hidden
|
||||||
|
self.member = self.owner
|
||||||
|
|
||||||
def delete(self):
|
def delete(self):
|
||||||
self.status = 'deleted'
|
self.status = 'deleted'
|
||||||
@ -1115,3 +1116,21 @@ class TestImageTarget(base.IsolatedUnitTest):
|
|||||||
self.assertIn('project_id', target)
|
self.assertIn('project_id', target)
|
||||||
self.assertEqual(image.owner, target['project_id'])
|
self.assertEqual(image.owner, target['project_id'])
|
||||||
self.assertEqual(image.owner, target['owner'])
|
self.assertEqual(image.owner, target['owner'])
|
||||||
|
|
||||||
|
def test_image_target_transforms(self):
|
||||||
|
fake_image = mock.MagicMock()
|
||||||
|
fake_image.image_id = mock.sentinel.image_id
|
||||||
|
fake_image.owner = mock.sentinel.owner
|
||||||
|
fake_image.member = mock.sentinel.member
|
||||||
|
|
||||||
|
target = glance.api.policy.ImageTarget(fake_image)
|
||||||
|
|
||||||
|
# Make sure the key transforms work
|
||||||
|
self.assertEqual(mock.sentinel.image_id, target['id'])
|
||||||
|
self.assertEqual(mock.sentinel.owner, target['project_id'])
|
||||||
|
self.assertEqual(mock.sentinel.member, target['member_id'])
|
||||||
|
|
||||||
|
# Also make sure the base properties still work
|
||||||
|
self.assertEqual(mock.sentinel.image_id, target['image_id'])
|
||||||
|
self.assertEqual(mock.sentinel.owner, target['owner'])
|
||||||
|
self.assertEqual(mock.sentinel.member, target['member'])
|
||||||
|
@ -187,6 +187,7 @@ class FakeImage(object):
|
|||||||
self.name = 'foo'
|
self.name = 'foo'
|
||||||
self.tags = []
|
self.tags = []
|
||||||
self.extra_properties = {}
|
self.extra_properties = {}
|
||||||
|
self.member = self.owner
|
||||||
|
|
||||||
# NOTE(danms): This fixture looks more like the db object than
|
# NOTE(danms): This fixture looks more like the db object than
|
||||||
# the proxy model. This needs fixing all through the tests
|
# the proxy model. This needs fixing all through the tests
|
||||||
|
Loading…
Reference in New Issue
Block a user