Add user_id and project_id to snapshot APIs

In a cloud with multi-tenancy, administrators
would like to see the project_id of the tenant
while listing snapshots of all tenants.

Likewise, we also should expose the user_id field
as it would help in cases where tenants have multiple
users.

DocImpact
User and administrator references need to capture
this information.

APIImpact
API changes are micro-versioned.

Change-Id: I3ad655dc9ab7440f205b0e153fccaa99abb79bbd
Closes-Bug: #1587161

manila/api/openstack/api_version_request.py

@@ -66,13 +66,15 @@ REST_API_VERSION_HISTORY = """
             'migrate_share' to 'migration_start' and added notify parameter
              to 'migration_start'.
     * 2.16 - Add user_id in share show/create/manage API.
+    * 2.17 - Added project_id and user_id fields to the JSON response of
+             snapshot show/create/manage API.
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # the minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.16"
+_MAX_API_VERSION = "2.17"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -110,3 +110,7 @@ user documentation.
 2.16
 ----
   Add user_id in share show/create/manage API.
+
+2.17
+----
+  Added user_id and project_id in snapshot show/create/manage APIs.

manila/api/views/share_snapshots.py

@@ -22,6 +22,7 @@ class ViewBuilder(common.ViewBuilder):
     _collection_name = 'snapshots'
     _detail_version_modifiers = [
         "add_provider_location_field",
+        "add_project_and_user_ids",
     ]
 
     def summary_list(self, request, snapshots):
@@ -68,6 +69,11 @@ class ViewBuilder(common.ViewBuilder):
             snapshot_dict['provider_location'] = snapshot.get(
                 'provider_location')
 
+    @common.ViewBuilder.versioned_method("2.17")
+    def add_project_and_user_ids(self, context, snapshot_dict, snapshot):
+        snapshot_dict['user_id'] = snapshot.get('user_id')
+        snapshot_dict['project_id'] = snapshot.get('project_id')
+
     def _list_view(self, func, request, snapshots):
         """Provide a view for a list of share snapshots."""
         snapshots_list = [func(request, snapshot)['snapshot']

manila/tests/api/v2/test_share_snapshots.py

@@ -19,6 +19,7 @@ from oslo_serialization import jsonutils
 import six
 import webob
 
+from manila.api.openstack import api_version_request as api_version
 from manila.api.v2 import share_snapshots
 from manila.common import constants
 from manila import context
@@ -41,6 +42,8 @@ def get_fake_manage_body(share_id=None, provider_location=None,
         'share_id': share_id,
         'provider_location': provider_location,
         'driver_options': driver_options,
+        'user_id': 'fake_user_id',
+        'project_id': 'fake_project_id',
     }
     fake_snapshot.update(kwargs)
     return {'snapshot': fake_snapshot}
@@ -69,9 +72,11 @@ class ShareSnapshotAPITest(test.TestCase):
             'display_description': 'updated_snapshot_description',
         }
 
-    def test_snapshot_create(self):
+    @ddt.data('1.0', '2.16', '2.17')
+    def test_snapshot_create(self, version):
         self.mock_object(share_api.API, 'create_snapshot',
                          stubs.stub_snapshot_create)
+
         body = {
             'snapshot': {
                 'share_id': 'fakeshareid',
@@ -80,11 +85,11 @@ class ShareSnapshotAPITest(test.TestCase):
                 'description': 'displaysnapdesc',
             }
         }
-        req = fakes.HTTPRequest.blank('/snapshots')
+        req = fakes.HTTPRequest.blank('/snapshots', version=version)
 
         res_dict = self.controller.create(req, body)
 
-        expected = fake_share.expected_snapshot(id=200)
+        expected = fake_share.expected_snapshot(version=version, id=200)
 
         self.assertEqual(expected, res_dict)
 
@@ -135,10 +140,13 @@ class ShareSnapshotAPITest(test.TestCase):
                           req,
                           200)
 
-    def test_snapshot_show(self):
+    @ddt.data('2.0', '2.16', '2.17')
-        req = fakes.HTTPRequest.blank('/snapshots/200')
+    def test_snapshot_show(self, version):
+        req = fakes.HTTPRequest.blank('/snapshots/200', version=version)
+        expected = fake_share.expected_snapshot(version=version, id=200)
+
         res_dict = self.controller.show(req, 200)
-        expected = fake_share.expected_snapshot(id=200)
+
         self.assertEqual(expected, res_dict)
 
     def test_snapshot_show_nofound(self):
@@ -280,33 +288,37 @@ class ShareSnapshotAPITest(test.TestCase):
     def test_snapshot_list_detail_with_search_opts_by_admin(self):
         self._snapshot_list_detail_with_search_opts(use_admin_context=True)
 
-    def test_snapshot_list_detail(self):
+    @ddt.data('2.0', '2.16', '2.17')
+    def test_snapshot_list_detail(self, version):
         env = {'QUERY_STRING': 'name=Share+Test+Name'}
-        req = fakes.HTTPRequest.blank('/shares/detail', environ=env)
+        req = fakes.HTTPRequest.blank('/snapshots/detail', environ=env,
-        res_dict = self.controller.detail(req)
+                                      version=version)
-        expected_s = fake_share.expected_snapshot(id=2)
+        expected_s = fake_share.expected_snapshot(version=version, id=2)
         expected = {'snapshots': [expected_s['snapshot']]}
+
+        res_dict = self.controller.detail(req)
+
         self.assertEqual(expected, res_dict)
 
-    def test_snapshot_updates_description(self):
+    @ddt.data('2.0', '2.16', '2.17')
+    def test_snapshot_updates_display_name_and_description(self, version):
         snp = self.snp_example
         body = {"snapshot": snp}
+        req = fakes.HTTPRequest.blank('/snapshot/1', version=version)
 
-        req = fakes.HTTPRequest.blank('/snapshot/1')
         res_dict = self.controller.update(req, 1, body)
+
         self.assertEqual(snp["display_name"], res_dict['snapshot']["name"])
 
-    def test_snapshot_updates_display_descr(self):
+        if (api_version.APIVersionRequest(version) <=
-        snp = self.snp_example
+                api_version.APIVersionRequest('2.16')):
-        body = {"snapshot": snp}
+            self.assertNotIn('user_id', res_dict['snapshot'])
+            self.assertNotIn('project_id', res_dict['snapshot'])
+        else:
+            self.assertIn('user_id', res_dict['snapshot'])
+            self.assertIn('project_id', res_dict['snapshot'])
 
-        req = fakes.HTTPRequest.blank('/snapshot/1')
+    def test_share_update_invalid_key(self):
-        res_dict = self.controller.update(req, 1, body)
-
-        self.assertEqual(snp["display_description"],
-                         res_dict['snapshot']["description"])
-
-    def test_share_not_updates_size(self):
         snp = self.snp_example
         body = {"snapshot": snp}
 
@@ -451,19 +463,25 @@ class ShareSnapshotAdminActionsAPITest(test.TestCase):
             self.resource_name, 'manage_snapshot')
 
     @ddt.data(
-        get_fake_manage_body(name='foo', description='bar'),
+        {'version': '2.12',
-        get_fake_manage_body(display_name='foo', description='bar'),
+         'data': get_fake_manage_body(name='foo', display_description='bar')},
-        get_fake_manage_body(name='foo', display_description='bar'),
+        {'version': '2.12',
-        get_fake_manage_body(display_name='foo', display_description='bar'),
+         'data': get_fake_manage_body(display_name='foo', description='bar')},
-        get_fake_manage_body(display_name='foo', display_description='bar'),
+        {'version': '2.17',
+         'data': get_fake_manage_body(display_name='foo', description='bar')},
+        {'version': '2.17',
+         'data': get_fake_manage_body(name='foo', display_description='bar')},
     )
-    def test_snapshot_manage(self, data):
+    @ddt.unpack
+    def test_snapshot_manage(self, version, data):
         self.mock_policy_check = self.mock_object(
             policy, 'check_policy', mock.Mock(return_value=True))
         data['snapshot']['share_id'] = 'fake'
         data['snapshot']['provider_location'] = 'fake_volume_snapshot_id'
         data['snapshot']['driver_options'] = {}
-        return_snapshot = {'id': 'fake_snap'}
+        return_snapshot = fake_share.fake_snapshot(
+            create_instance=True, id='fake_snap',
+            provider_location='fake_volume_snapshot_id')
         self.mock_object(
             share_api.API, 'manage_snapshot', mock.Mock(
                 return_value=return_snapshot))
@@ -474,15 +492,31 @@ class ShareSnapshotAdminActionsAPITest(test.TestCase):
             'display_description': 'bar',
         }
 
-        actual_result = self.controller.manage(self.manage_request, data)
+        req = fakes.HTTPRequest.blank(
+            '/snapshots/manage', use_admin_context=True, version=version)
 
+        actual_result = self.controller.manage(req, data)
+
+        actual_snapshot = actual_result['snapshot']
         share_api.API.manage_snapshot.assert_called_once_with(
             mock.ANY, share_snapshot, data['snapshot']['driver_options'])
         self.assertEqual(return_snapshot['id'],
                          actual_result['snapshot']['id'])
+        self.assertEqual('fake_volume_snapshot_id',
+                         actual_result['snapshot']['provider_location'])
+
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.17')):
+            self.assertEqual(return_snapshot['user_id'],
+                             actual_snapshot['user_id'])
+            self.assertEqual(return_snapshot['project_id'],
+                             actual_snapshot['project_id'])
+        else:
+            self.assertNotIn('user_id', actual_snapshot)
+            self.assertNotIn('project_id', actual_snapshot)
         self.mock_policy_check.assert_called_once_with(
-            self.manage_request.environ['manila.context'],
+            req.environ['manila.context'], self.resource_name,
-            self.resource_name, 'manage_snapshot')
+            'manage_snapshot')
 
     @ddt.data(exception.ShareNotFound(share_id='fake'),
               exception.ShareSnapshotNotFound(snapshot_id='fake'),

manila/tests/fake_share.py

@@ -16,6 +16,7 @@
 import datetime
 import uuid
 
+from manila.api.openstack import api_version_request as api_version
 from manila.common import constants
 from manila.db.sqlalchemy import models
 from manila.tests.db import fakes as db_fakes
@@ -161,7 +162,7 @@ def fake_snapshot_instance(base_snapshot=None, **kwargs):
     return db_fakes.FakeModel(snapshot_instance)
 
 
-def expected_snapshot(id='fake_snapshot_id', **kwargs):
+def expected_snapshot(version=None, id='fake_snapshot_id', **kwargs):
     self_link = 'http://localhost/v1/fake/snapshots/%s' % id
     bookmark_link = 'http://localhost/fake/snapshots/%s' % id
     snapshot = {
@@ -185,6 +186,14 @@ def expected_snapshot(id='fake_snapshot_id', **kwargs):
             },
         ],
     }
+
+    if version and (api_version.APIVersionRequest(version)
+                    >= api_version.APIVersionRequest('2.17')):
+        snapshot.update({
+            'user_id': 'fakesnapuser',
+            'project_id': 'fakesnapproject',
+        })
+
     snapshot.update(kwargs)
     return {'snapshot': snapshot}
 

manila_tempest_tests/config.py

@@ -36,7 +36,7 @@ ShareGroup = [
                help="The minimum api microversion is configured to be the "
                     "value of the minimum microversion supported by Manila."),
     cfg.StrOpt("max_api_microversion",
-               default="2.16",
+               default="2.17",
                help="The maximum api microversion is configured to be the "
                     "value of the latest microversion supported by Manila."),
     cfg.StrOpt("region",

manila_tempest_tests/tests/api/admin/test_snapshot_manage.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
 import six
 from tempest import config
 from tempest.lib.common.utils import data_utils
@@ -21,10 +22,12 @@ from tempest import test
 import testtools
 
 from manila_tempest_tests.tests.api import base
+from manila_tempest_tests import utils
 
 CONF = config.CONF
 
 
+@ddt.ddt
 class ManageNFSSnapshotTest(base.BaseSharesAdminTest):
     protocol = 'nfs'
 
@@ -59,31 +62,12 @@ class ManageNFSSnapshotTest(base.BaseSharesAdminTest):
             cleanup_in_class=True,
             extra_specs=cls.extra_specs)
 
-        creation_data = {'kwargs': {
+        # Create the base share
-            'share_type_id': cls.st['share_type']['id'],
+        cls.share = cls.create_share(share_type_id=cls.st['share_type']['id'],
-            'share_protocol': cls.protocol,
+                                     share_protocol=cls.protocol)
-        }}
 
-        # Data for creating shares
+        # Get updated data
-        data = [creation_data]
+        cls.share = cls.shares_v2_client.get_share(cls.share['id'])
-        shares_created = cls.create_shares(data)
-
-        cls.snapshot = None
-        cls.shares = []
-        # Load all share data (host, etc.)
-        for share in shares_created:
-            cls.shares.append(cls.shares_v2_client.get_share(share['id']))
-            # Create snapshot
-            snap_name = data_utils.rand_name("tempest-snapshot-name")
-            snap_desc = data_utils.rand_name(
-                "tempest-snapshot-description")
-            snap = cls.create_snapshot_wait_for_active(
-                share['id'], snap_name, snap_desc)
-            cls.snapshot = cls.shares_v2_client.get_snapshot(snap['id'])
-            # Unmanage snapshot
-            cls.shares_v2_client.unmanage_snapshot(snap['id'])
-            cls.shares_client.wait_for_resource_deletion(
-                snapshot_id=snap['id'])
 
     def _test_manage(self, snapshot, version=CONF.share.max_api_microversion):
         name = ("Name for 'managed' snapshot that had ID %s" %
@@ -97,7 +81,8 @@ class ManageNFSSnapshotTest(base.BaseSharesAdminTest):
             snapshot['provider_location'],
             name=name,
             description=description,
-            driver_options={}
+            driver_options={},
+            version=version,
         )
 
         # Add managed snapshot to cleanup queue
@@ -109,6 +94,19 @@ class ManageNFSSnapshotTest(base.BaseSharesAdminTest):
         self.shares_v2_client.wait_for_snapshot_status(snapshot['id'],
                                                        'available')
 
+        # Verify manage snapshot API response
+        expected_keys = ["status", "links", "share_id", "name",
+                         "share_proto", "created_at",
+                         "description", "id", "share_size", "size",
+                         "provider_location"]
+        if utils.is_microversion_ge(version, '2.17'):
+            expected_keys.extend(["user_id", "project_id"])
+
+        actual_keys = snapshot.keys()
+
+        # Strict key check
+        self.assertEqual(set(expected_keys), set(actual_keys))
+
         # Verify data of managed snapshot
         get_snapshot = self.shares_v2_client.get_snapshot(snapshot['id'])
         self.assertEqual(name, get_snapshot['name'])
@@ -126,9 +124,31 @@ class ManageNFSSnapshotTest(base.BaseSharesAdminTest):
                           get_snapshot['id'])
 
     @test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
-    def test_manage(self):
+    @ddt.data('2.12', '2.16', CONF.share.max_api_microversion)
+    def test_manage_different_versions(self, version):
+        """Run snapshot manage test for multiple versions.
+
+        This test is configured with ddt to run for the configured maximum
+        version as well as versions 2.12 (when the API was introduced) and
+        2.16.
+        """
+        # Skip in case specified version is not supported
+        utils.skip_if_microversion_not_supported(version)
+
+        snap_name = data_utils.rand_name("tempest-snapshot-name")
+        snap_desc = data_utils.rand_name("tempest-snapshot-description")
+        # Create snapshot
+        snapshot = self.create_snapshot_wait_for_active(
+            self.share['id'], snap_name, snap_desc)
+        snapshot = self.shares_v2_client.get_snapshot(snapshot['id'])
+        # Unmanage snapshot
+        self.shares_v2_client.unmanage_snapshot(snapshot['id'],
+                                                version=version)
+        self.shares_client.wait_for_resource_deletion(
+            snapshot_id=snapshot['id'])
+
         # Manage snapshot
-        self._test_manage(snapshot=self.snapshot)
+        self._test_manage(snapshot=snapshot, version=version)
 
 
 class ManageCIFSSnapshotTest(ManageNFSSnapshotTest):

manila_tempest_tests/tests/api/test_shares.py

@@ -104,14 +104,26 @@ class SharesNFSTest(base.BaseSharesTest):
 
         # create snapshot
         snap = self.create_snapshot_wait_for_active(self.share["id"])
+
         detailed_elements = {'name', 'id', 'description',
                              'created_at', 'share_proto', 'size', 'share_size',
                              'share_id', 'status', 'links'}
-        self.assertTrue(detailed_elements.issubset(snap.keys()),
+        msg = (
-                        'At least one expected element missing from snapshot '
+            "At least one expected element missing from share "
-                        'response. Expected %(expected)s, got %(actual)s.' % {
+            "response. Expected %(expected)s, got %(actual)s." % {
                 "expected": detailed_elements,
-                            "actual": snap.keys()})
+                "actual": snap.keys(),
+            }
+        )
+        self.assertTrue(detailed_elements.issubset(snap.keys()), msg)
+
+        # In v2.17 and beyond, we expect user_id and project_id keys
+        if utils.is_microversion_supported('2.17'):
+            detailed_elements.update({'user_id', 'project_id'})
+            self.assertTrue(detailed_elements.issubset(snap.keys()), msg)
+        else:
+            self.assertNotIn('user_id', detailed_elements)
+            self.assertNotIn('project_id', detailed_elements)
 
         # delete snapshot
         self.shares_client.delete_snapshot(snap["id"])

manila_tempest_tests/tests/api/test_shares_actions.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
 import six
 from tempest import config
 from tempest.lib.common.utils import data_utils
@@ -23,8 +24,10 @@ from manila_tempest_tests.tests.api import base
 from manila_tempest_tests import utils
 
 CONF = config.CONF
+LATEST_MICROVERSION = CONF.share.max_api_microversion
 
 
+@ddt.ddt
 class SharesActionsTest(base.BaseSharesTest):
     """Covers share functionality, that doesn't related to share type."""
 
@@ -399,30 +402,58 @@ class SharesActionsTest(base.BaseSharesTest):
     @test.attr(type=[base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND])
     @testtools.skipUnless(CONF.share.run_snapshot_tests,
                           "Snapshot tests are disabled.")
-    def test_get_snapshot(self):
+    @ddt.data(None, '2.16', LATEST_MICROVERSION)
+    def test_get_snapshot(self, version):
 
         # get snapshot
-        get = self.shares_client.get_snapshot(self.snap["id"])
+        if version is None:
+            snapshot = self.shares_client.get_snapshot(self.snap["id"])
+        else:
+            utils.skip_if_microversion_not_supported(version)
+            snapshot = self.shares_v2_client.get_snapshot(
+                self.snap["id"], version=version)
 
         # verify keys
         expected_keys = ["status", "links", "share_id", "name",
                          "share_proto", "created_at",
-                         "description", "id", "share_size"]
+                         "description", "id", "share_size", "size"]
-        actual_keys = get.keys()
+        if version and utils.is_microversion_ge(version, '2.17'):
-        [self.assertIn(key, actual_keys) for key in expected_keys]
+            expected_keys.extend(["user_id", "project_id"])
+        actual_keys = snapshot.keys()
+
+        # strict key check
+        self.assertEqual(set(expected_keys), set(actual_keys))
 
         # verify data
         msg = "Expected name: '%s', actual name: '%s'" % (self.snap_name,
-                                                          get["name"])
+                                                          snapshot["name"])
-        self.assertEqual(self.snap_name, get["name"], msg)
+        self.assertEqual(self.snap_name, snapshot["name"], msg)
 
-        msg = "Expected description: '%s', "\
+        msg = ("Expected description: '%s' actual description: '%s'" %
-              "actual description: '%s'" % (self.snap_desc, get["description"])
+               (self.snap_desc, snapshot["description"]))
-        self.assertEqual(self.snap_desc, get["description"], msg)
+        self.assertEqual(self.snap_desc, snapshot["description"], msg)
 
-        msg = "Expected share_id: '%s', "\
+        msg = ("Expected share_id: '%s', actual share_id: '%s'" %
-              "actual share_id: '%s'" % (self.shares[0]["id"], get["share_id"])
+               (self.shares[0]["id"], snapshot["share_id"]))
-        self.assertEqual(self.shares[0]["id"], get["share_id"], msg)
+        self.assertEqual(self.shares[0]["id"], snapshot["share_id"], msg)
+
+        # Verify that the user_id and project_id are same as the one for
+        # the base share
+        if version and utils.is_microversion_ge(version, '2.17'):
+            msg = ("Expected %(key)s in snapshot: '%(expected)s', "
+                   "actual %(key)s in snapshot: '%(actual)s'")
+            self.assertEqual(self.shares[0]['user_id'],
+                             snapshot['user_id'],
+                             msg % {
+                                 'expected': self.shares[0]['user_id'],
+                                 'actual': snapshot['user_id'],
+                                 'key': 'user_id'})
+            self.assertEqual(self.shares[0]['project_id'],
+                             snapshot['project_id'],
+                             msg % {
+                                 'expected': self.shares[0]['project_id'],
+                                 'actual': snapshot['project_id'],
+                                 'key': 'project_id'})
 
     @test.attr(type=[base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND])
     @testtools.skipUnless(CONF.share.run_snapshot_tests,
@@ -444,16 +475,26 @@ class SharesActionsTest(base.BaseSharesTest):
     @test.attr(type=[base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND])
     @testtools.skipUnless(CONF.share.run_snapshot_tests,
                           "Snapshot tests are disabled.")
-    def test_list_snapshots_with_detail(self):
+    @ddt.data(None, '2.16', LATEST_MICROVERSION)
+    def test_list_snapshots_with_detail(self, version):
 
         # list share snapshots
+        if version is None:
             snaps = self.shares_client.list_snapshots_with_detail()
+        else:
+            utils.skip_if_microversion_not_supported(version)
+            snaps = self.shares_v2_client.list_snapshots_with_detail(
+                version=version)
 
         # verify keys
-        keys = ["status", "links", "share_id", "name",
+        expected_keys = ["status", "links", "share_id", "name",
-                "share_proto", "created_at",
+                         "share_proto", "created_at", "description", "id",
-                "description", "id", "share_size"]
+                         "share_size", "size"]
-        [self.assertIn(key, sn.keys()) for sn in snaps for key in keys]
+        if version and utils.is_microversion_ge(version, '2.17'):
+            expected_keys.extend(["user_id", "project_id"])
+
+        # strict key check
+        [self.assertEqual(set(expected_keys), set(s.keys())) for s in snaps]
 
         # our share id in list and have no duplicates
         gen = [sid["id"] for sid in snaps if sid["id"] in self.snap["id"]]

releasenotes/notes/add_user_id_and_project_id_to_snapshot_APIs-157614b4b8d01e15.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - user_id and project_id fields are added to the JSON response of /snapshots
+    APIs.