From 4de86d30a4a208e2fe25507b70e44867b2877564 Mon Sep 17 00:00:00 2001 From: Rajat Dhasmana Date: Tue, 5 Mar 2024 03:26:55 +0530 Subject: [PATCH] Use normal credentials for legacy image update When updating legacy images, we use the user's context and elevate priviledges. However, neither do we require admin priviledges for the cinder API calls (in this case) nor the user's context credentials. 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. 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 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 --- glance_store/_drivers/cinder/store.py | 26 +++++++++---------- .../unit/cinder/test_multistore_cinder.py | 10 ++++--- ...-legacy-image-update-49a149ec267dccb6.yaml | 8 ++++++ 3 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml diff --git a/glance_store/_drivers/cinder/store.py b/glance_store/_drivers/cinder/store.py index ad0f1b16..11e53233 100644 --- a/glance_store/_drivers/cinder/store.py +++ b/glance_store/_drivers/cinder/store.py @@ -555,9 +555,18 @@ 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. volume = cinder_client.volumes.get(volume_id) if cinder_volume_type and volume.volume_type == cinder_volume_type: return True @@ -584,17 +593,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: - user_overriden = self.is_user_overriden() - + 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: diff --git a/glance_store/tests/unit/cinder/test_multistore_cinder.py b/glance_store/tests/unit/cinder/test_multistore_cinder.py index f40965bf..9e8bbc24 100644 --- a/glance_store/tests/unit/cinder/test_multistore_cinder.py +++ b/glance_store/tests/unit/cinder/test_multistore_cinder.py @@ -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') diff --git a/releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml b/releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml new file mode 100644 index 00000000..e9372006 --- /dev/null +++ b/releasenotes/notes/fix-legacy-image-update-49a149ec267dccb6.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `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.