diff --git a/cinder/api/v2/views/volumes.py b/cinder/api/v2/views/volumes.py index 32a9a7f05c3..ee19a96d5d1 100644 --- a/cinder/api/v2/views/volumes.py +++ b/cinder/api/v2/views/volumes.py @@ -90,7 +90,7 @@ class ViewBuilder(common.ViewBuilder): } ctxt = request.environ['cinder.context'] - attachments = self._get_attachments(volume, ctxt.is_admin) + attachments = self._get_attachments(volume, ctxt) volume_ref['volume']['attachments'] = attachments if ctxt.is_admin: @@ -113,7 +113,7 @@ class ViewBuilder(common.ViewBuilder): """Determine if volume is encrypted.""" return volume.get('encryption_key_id') is not None - def _get_attachments(self, volume, is_admin): + def _get_attachments(self, volume, ctxt): """Retrieve the attachments of the volume object.""" attachments = [] @@ -124,12 +124,17 @@ class ViewBuilder(common.ViewBuilder): 'attachment_id': attachment.get('id'), 'volume_id': attachment.get('volume_id'), 'server_id': attachment.get('instance_uuid'), - 'host_name': attachment.get('attached_host'), + 'host_name': None, 'device': attachment.get('mountpoint'), 'attached_at': attachment.get('attach_time'), } - if not is_admin: - a['host_name'] = None + # When glance is cinder backed, we require the + # host_name to determine when to detach a multiattach + # volume. Glance always uses service credentials to + # request Cinder so we are not exposing the host value + # to end users (non-admin). + if ctxt.is_admin or 'service' in ctxt.roles: + a['host_name'] = attachment.get('attached_host') attachments.append(a) return attachments diff --git a/cinder/tests/unit/api/v3/test_volumes.py b/cinder/tests/unit/api/v3/test_volumes.py index 72045da7c11..ba379372147 100644 --- a/cinder/tests/unit/api/v3/test_volumes.py +++ b/cinder/tests/unit/api/v3/test_volumes.py @@ -1086,6 +1086,8 @@ class VolumeApiTest(test.TestCase): {'revert': {'snapshot_id': fake_snapshot['id']}}) def test_view_get_attachments(self): + req = fakes.HTTPRequest.blank('/v3/volumes') + context = req.environ['cinder.context'] fake_volume = self._fake_create_volume() fake_volume['attach_status'] = fields.VolumeAttachStatus.ATTACHING att_time = datetime.datetime(2017, 8, 31, 21, 55, 7, @@ -1117,7 +1119,8 @@ class VolumeApiTest(test.TestCase): # get_attachments should only return attachments with the # attached status = ATTACHED - attachments = ViewBuilder()._get_attachments(fake_volume, True) + context.is_admin = True + attachments = ViewBuilder()._get_attachments(fake_volume, context) self.assertEqual(1, len(attachments)) self.assertEqual(fake.UUID3, attachments[0]['attachment_id']) @@ -1128,9 +1131,16 @@ class VolumeApiTest(test.TestCase): self.assertEqual(att_time, attachments[0]['attached_at']) # When admin context is false (non-admin), host_name will be None - attachments = ViewBuilder()._get_attachments(fake_volume, False) + context.is_admin = False + attachments = ViewBuilder()._get_attachments(fake_volume, context) self.assertIsNone(attachments[0]['host_name']) + # When the request is coming from another service (glance), + # We should be able to see 'host_name' + context.roles.append('service') + attachments = ViewBuilder()._get_attachments(fake_volume, context) + self.assertEqual('host1', attachments[0]['host_name']) + @ddt.data(('created_at=gt:', 0), ('created_at=lt:', 2)) @ddt.unpack def test_volume_index_filter_by_created_at_with_gt_and_lt(self, change,