openstack/glance_store
Code Issues Proposed changes

Use normal credentials for legacy image update

Browse Source 
When updating legacy images, currenly we use the user's
context and elevate priviledges. However, we do not
require admin priviledges for the cinder API calls.

This patch removes the special case where we elevate
priviledges as it wasn't doing anything rather avoiding
us to use right credentials and failing to fetch volume
in the right location because of wrong credentials.

The correct credentials are either the service ones
set in glance-api.conf file or the user context
credentials, using which the Image-Volume was created.

NOTE: When using cinder as glance backend and we want
to perform optimized volume upload to image, one thing
we should make sure is either using the context or the
cinder credentials set in glance-api.conf file, it should
match the following details on the cinder side (if we are
using internal context to create clone of image-volumes):

cinder_store_user_name = context.user_id/cinder_internal_tenant_user_id
cinder_store_project_name = context.project_id/cinder_internal_tenant_project_id

The cinder_internal_tenant_user_id and
cinder_internal_tenant_project_id are set in the
[DEFAULT] section of cinder.conf.

This issue was first discovered when testing the new
location APIs[1] where tempest creates a volume with
cinder's internal context and glance uses wrong (user context)
credentials to access it and failing with 404 not found.

[1] https://review.opendev.org/c/openstack/cinder/+/909847

Closes-Bug: #2056179
Change-Id: I4f27a9800f239da8dbf29f4c028678df1f867664
This commit is contained in:
Rajat Dhasmana 2024-03-05 03:26:55 +05:30
parent e9217809e7
commit c2b7b78d4f
3 changed files with 51 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
glance_store/_drivers/cinder/store.py
View File

@ -555,10 +555,29 @@ class Store(glance_store.driver.Store):
If above both conditions doesn't meet, it returns false.
"""
try:
cinder_client = self.get_cinderclient(context=context,
legacy_update=True)
# We will use either the service credentials defined in
# config file or the user context credentials
cinder_client = self.get_cinderclient(context=context)
cinder_volume_type = self.store_conf.cinder_volume_type
# Here we are assuming that the volume is stored in the
# service project or context user's project else this
# will return NotFound exception.
# Ideally we should be using service user's credentials
# defined in the config and the volume should be stored
# in the service (internal) project else we are opening the
# image-volume to modification by users which might lead
# to corruption of image.
try:
volume = cinder_client.volumes.get(volume_id)
except cinder_exception.NotFound:
reason = (_LW("Image-Volume %s not found. If you have "
"upgraded your environment from single store "
"to multi store, transfer all your "
"Image-Volumes from user projects to service "
"project."
% volume_id))
LOG.warning(reason)
return False
if cinder_volume_type and volume.volume_type == cinder_volume_type:
return True
elif not cinder_volume_type:
@ -584,17 +603,8 @@ class Store(glance_store.driver.Store):
for key in ['user_name', 'password',
'project_name', 'auth_address']])
def get_cinderclient(self, context=None, legacy_update=False,
version='3.0'):
# NOTE: For legacy image update from single store to multiple
# stores we need to use admin context rather than user provided
# credentials
if legacy_update:
user_overriden = False
context = context.elevated()
else:
def get_cinderclient(self, context=None, version='3.0'):
user_overriden = self.is_user_overriden()
session = get_cinder_session(self.store_conf)
if user_overriden:

23
glance_store/tests/unit/cinder/test_multistore_cinder.py
View File

@ -112,10 +112,12 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
self._test_get_cinderclient_with_ca_certificates(group='cinder1')
def test_get_cinderclient_legacy_update(self):
cc = self.store.get_cinderclient(self.fake_admin_context,
legacy_update=True)
self.assertEqual('admin_token', cc.client.auth.token)
self.assertEqual('http://foo/public_url', cc.client.auth.endpoint)
fake_endpoint = 'http://cinder.openstack.example.com/v2/fake_project'
self.config(cinder_endpoint_template=fake_endpoint, group='cinder1')
cc = self.store.get_cinderclient(self.context)
self.assertEqual(self.context.auth_token,
cc.client.auth.token)
self.assertEqual(fake_endpoint, cc.client.auth.endpoint)
def test_open_cinder_volume_multipath_enabled(self):
self.config(cinder_use_multipath=True, group='cinder1')
@ -221,6 +223,19 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
type_match = self.store.is_image_associated_with_store(
self.context, fake_vol_id)
self.assertFalse(type_match)
# When the Image-Volume is not found
mocked_cc.return_value.volumes.get = mock.MagicMock(
side_effect=cinder.cinder_exception.NotFound(code=404))
with mock.patch.object(cinder, 'LOG') as mock_log:
type_match = self.store.is_image_associated_with_store(
self.context, fake_vol_id)
mock_log.warning.assert_called_with(
"Image-Volume %s not found. If you have "
"upgraded your environment from single store "
"to multi store, transfer all your "
"Image-Volumes from user projects to service "
"project." % fake_vol_id)
self.assertFalse(type_match)
def test_cinder_get(self):
self._test_cinder_get(is_multi_store=True)

8
releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
`Bug #2056179 <https://bugs.launchpad.net/glance-store/+bug/2056179>`_:
Cinder Store: Fix issue when updating legacy image location.
Previously we only used the user context's credentials to make request
to cinder which we have now updated to use the service credentials
configured in the config file else use the user context's credentials.