Merge "Don't show host_name to non-admins"

cinder/api/v2/views/volumes.py

@@ -75,7 +75,6 @@ class ViewBuilder(common.ViewBuilder):
                 'availability_zone': volume.get('availability_zone'),
                 'created_at': volume.get('created_at'),
                 'updated_at': volume.get('updated_at'),
-                'attachments': self._get_attachments(volume),
                 'name': volume.get('display_name'),
                 'description': volume.get('display_description'),
                 'volume_type': self._get_volume_type(volume),
@@ -92,9 +91,10 @@ class ViewBuilder(common.ViewBuilder):
             }
         }
         ctxt = request.environ['cinder.context']
-        if not ctxt.is_admin:
-            if volume_ref.get('attachments'):
-                volume_ref['volume']['attachments']['host_name'] = None
+
+        attachments = self._get_attachments(volume, ctxt.is_admin)
+        volume_ref['volume']['attachments'] = attachments
+
         if ctxt.is_admin:
             volume_ref['volume']['migration_status'] = (
                 volume.get('migration_status'))
@@ -115,7 +115,7 @@ class ViewBuilder(common.ViewBuilder):
         """Determine if volume is encrypted."""
         return volume.get('encryption_key_id') is not None
 
-    def _get_attachments(self, volume):
+    def _get_attachments(self, volume, is_admin):
         """Retrieve the attachments of the volume object."""
         attachments = []
 
@@ -130,6 +130,8 @@ class ViewBuilder(common.ViewBuilder):
                      'device': attachment.get('mountpoint'),
                      'attached_at': attachment.get('attach_time'),
                      }
+                if not is_admin:
+                    a['host_name'] = None
                 attachments.append(a)
 
         return attachments

cinder/tests/unit/api/v2/test_volumes.py

@@ -917,6 +917,33 @@ class VolumeApiTest(test.TestCase):
         expected = {'volumes': [exp_vol['volume']]}
         self.assertEqual(expected, res_dict)
 
+    def test_volume_list_detail_host_name_admin_non_admin(self):
+        fake_host = 'fake_host'
+        volume = v2_fakes.create_fake_volume(fake.VOLUME_ID)
+        del volume['name']
+        del volume['volume_type']
+        db.volume_create(context.get_admin_context(), volume)
+        values = {'volume_id': fake.VOLUME_ID, }
+        attachment = db.volume_attach(context.get_admin_context(), values)
+        db.volume_attached(context.get_admin_context(),
+                           attachment['id'], fake.INSTANCE_ID, fake_host, '/')
+        db.volume_attachment_get(context.get_admin_context(),
+                                 attachment['id'])
+
+        req = fakes.HTTPRequest.blank('/v2/volumes/detail')
+        res_dict = self.controller.detail(req)
+        # host_name will always be None for non-admins
+        self.assertIsNone(
+            res_dict['volumes'][0]['attachments'][0]['host_name'])
+
+        admin_ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.detail(req)
+        # correct host_name is returned for admins
+        self.assertEqual(fake_host,
+                         res_dict['volumes'][0]['attachments'][0]['host_name']
+                         )
+
     def test_volume_index_with_marker(self):
         def fake_volume_get_all_by_project(context, project_id, marker, limit,
                                            sort_keys=None, sort_dirs=None,

cinder/tests/unit/api/v3/test_volumes.py

@@ -975,7 +975,7 @@ class VolumeApiTest(test.TestCase):
 
         # get_attachments should only return attachments with the
         # attached status = ATTACHED
-        attachments = ViewBuilder()._get_attachments(fake_volume)
+        attachments = ViewBuilder()._get_attachments(fake_volume, True)
 
         self.assertEqual(1, len(attachments))
         self.assertEqual(fake.UUID3, attachments[0]['attachment_id'])
@@ -985,6 +985,10 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual('na', attachments[0]['device'])
         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)
+        self.assertIsNone(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,