MultiStore: Lazy update fails if image is not owned by owner

Lazy updating store information in location metadata fails if image
is not owned by the user. The reason is saving this information to
image object returns Forbidden error as the user is not allowed to change
the image information. Actually after updating the location information
there is no need to explicitly call save function because when location
information is updated using image.locations object it is updated to
database at the same time.

To fix this issue, remved the image_repo.save call, so that Lazy update
should be allowed to index as well as show call for all the users.

Change-Id: I92c85647ea4eea7069f8124334402f9127bf653c
Closes-Bug: #1840885
This commit is contained in:
Abhishek Kekane 2019-08-21 07:54:05 +00:00
parent cee9526c98
commit 3fda53c520
2 changed files with 33 additions and 6 deletions

View File

@ -33,11 +33,9 @@ def lazy_update_store_info(func):
"""Update store information in location metadata"""
@functools.wraps(func)
def wrapped(context, image, **kwargs):
image_repo = kwargs.get('image_repo')
if CONF.enabled_backends:
store_utils.update_store_in_locations(
image.locations, image.image_id)
image_repo.save(image)
return func(context, image, **kwargs)
@ -56,7 +54,7 @@ def is_image_mutable(context, image):
@lazy_update_store_info
def proxy_image(context, image, image_repo=None):
def proxy_image(context, image):
if is_image_mutable(context, image):
return ImageProxy(image, context)
else:
@ -129,12 +127,11 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
def get(self, image_id):
image = self.image_repo.get(image_id)
return proxy_image(self.context, image, image_repo=self.image_repo)
return proxy_image(self.context, image)
def list(self, *args, **kwargs):
images = self.image_repo.list(*args, **kwargs)
return [proxy_image(self.context, i,
image_repo=self.image_repo) for i in images]
return [proxy_image(self.context, i) for i in images]
def _validate_image_accepts_members(visibility):

View File

@ -50,6 +50,7 @@ UUID1 = 'c80a1a6c-bd1f-41c5-90ee-81afedb1d58d'
UUID2 = 'a85abd86-55b3-4d5b-b0b4-5d0a6e6042fc'
UUID3 = '971ec09a-8067-4bc8-a91f-ae3557f1c4c7'
UUID4 = '6bbe7cc2-eae7-4c0f-b50d-a7160b0c6a86'
UUID5 = '13c58ac4-210d-41ab-8cdb-1adfe4610019'
TENANT1 = '6838eb7b-6ded-434a-882c-b344c77fe8df'
TENANT2 = '2c014f32-55eb-467d-8fcb-4bd706012f81'
@ -4709,6 +4710,19 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest):
UUID2),
'metadata': {}, 'status': 'active'}],
created_at=DATETIME + datetime.timedelta(seconds=1)),
_db_fixture(UUID5, owner=TENANT3, checksum=CHKSUM1,
name='2', size=512, virtual_size=2048,
visibility='public',
disk_format='raw',
container_format='bare',
status='active',
tags=['redhat', '64bit', 'power'],
properties={'hypervisor_type': 'kvm', 'foo': 'bar',
'bar': 'foo'},
locations=[{'url': 'file://%s/%s' % (self.test_dir,
UUID2),
'metadata': {}, 'status': 'active'}],
created_at=DATETIME + datetime.timedelta(seconds=1)),
_db_fixture(UUID3, owner=TENANT3, checksum=CHKSUM1,
name='3', size=512, virtual_size=2048,
visibility='public', tags=['windows', '64bit', 'x86'],
@ -4758,6 +4772,22 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest):
for loc in image.locations:
self.assertIn('store', loc['metadata'])
def test_image_lazy_loading_store_different_owner(self):
# assert existing image does not have store in metadata
existing_image = self.images[2]
self.assertNotIn('store', existing_image['locations'][0]['metadata'])
# assert: store information will be added by lazy loading even if owner
# is different
request = unit_test_utils.get_fake_request()
request.headers.update({'X-Tenant_id': TENANT1})
with mock.patch.object(store_utils,
"_get_store_id_from_uri") as mock_uri:
mock_uri.return_value = "fast"
image = self.controller.show(request, UUID5)
for loc in image.locations:
self.assertIn('store', loc['metadata'])
def test_image_import_invalid_backend_in_request_header(self):
request = unit_test_utils.get_fake_request()
request.headers['x-image-meta-store'] = 'dummy'