diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 02be4bb1..eed7e734 100644 --- a/manila/api/openstack/api_version_request.py +++ b/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 diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 70992f12..e10a14dc 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/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. diff --git a/manila/api/views/share_snapshots.py b/manila/api/views/share_snapshots.py index a71c2c96..6314a0ba 100644 --- a/manila/api/views/share_snapshots.py +++ b/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'] diff --git a/manila/tests/api/v2/test_share_snapshots.py b/manila/tests/api/v2/test_share_snapshots.py index fe2a6453..1707c7c3 100644 --- a/manila/tests/api/v2/test_share_snapshots.py +++ b/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): - req = fakes.HTTPRequest.blank('/snapshots/200') + @ddt.data('2.0', '2.16', '2.17') + 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) - res_dict = self.controller.detail(req) - expected_s = fake_share.expected_snapshot(id=2) + req = fakes.HTTPRequest.blank('/snapshots/detail', environ=env, + version=version) + 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): - snp = self.snp_example - body = {"snapshot": snp} + if (api_version.APIVersionRequest(version) <= + api_version.APIVersionRequest('2.16')): + 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') - res_dict = self.controller.update(req, 1, body) - - self.assertEqual(snp["display_description"], - res_dict['snapshot']["description"]) - - def test_share_not_updates_size(self): + def test_share_update_invalid_key(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'), - get_fake_manage_body(display_name='foo', description='bar'), - get_fake_manage_body(name='foo', display_description='bar'), - get_fake_manage_body(display_name='foo', display_description='bar'), - get_fake_manage_body(display_name='foo', display_description='bar'), + {'version': '2.12', + 'data': get_fake_manage_body(name='foo', display_description='bar')}, + {'version': '2.12', + 'data': get_fake_manage_body(display_name='foo', 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'], - self.resource_name, 'manage_snapshot') + req.environ['manila.context'], self.resource_name, + 'manage_snapshot') @ddt.data(exception.ShareNotFound(share_id='fake'), exception.ShareSnapshotNotFound(snapshot_id='fake'), diff --git a/manila/tests/fake_share.py b/manila/tests/fake_share.py index 7dc27e17..e259d37b 100644 --- a/manila/tests/fake_share.py +++ b/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} diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py index 4e51ad58..8cf618aa 100644 --- a/manila_tempest_tests/config.py +++ b/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", diff --git a/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py b/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py index 3eed8863..ecdd3bd9 100644 --- a/manila_tempest_tests/tests/api/admin/test_snapshot_manage.py +++ b/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': { - 'share_type_id': cls.st['share_type']['id'], - 'share_protocol': cls.protocol, - }} + # Create the base share + cls.share = cls.create_share(share_type_id=cls.st['share_type']['id'], + share_protocol=cls.protocol) - # Data for creating shares - data = [creation_data] - 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']) + # Get updated data + cls.share = cls.shares_v2_client.get_share(cls.share['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): diff --git a/manila_tempest_tests/tests/api/test_shares.py b/manila_tempest_tests/tests/api/test_shares.py index 1da57e47..2acd81a8 100644 --- a/manila_tempest_tests/tests/api/test_shares.py +++ b/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()), - 'At least one expected element missing from snapshot ' - 'response. Expected %(expected)s, got %(actual)s.' % { - "expected": detailed_elements, - "actual": snap.keys()}) + msg = ( + "At least one expected element missing from share " + "response. Expected %(expected)s, got %(actual)s." % { + "expected": detailed_elements, + "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"]) diff --git a/manila_tempest_tests/tests/api/test_shares_actions.py b/manila_tempest_tests/tests/api/test_shares_actions.py index 16b8b587..3d4c54bb 100644 --- a/manila_tempest_tests/tests/api/test_shares_actions.py +++ b/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"] - actual_keys = get.keys() - [self.assertIn(key, actual_keys) for key in expected_keys] + "description", "id", "share_size", "size"] + if version and 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 msg = "Expected name: '%s', actual name: '%s'" % (self.snap_name, - get["name"]) - self.assertEqual(self.snap_name, get["name"], msg) + snapshot["name"]) + self.assertEqual(self.snap_name, snapshot["name"], msg) - msg = "Expected description: '%s', "\ - "actual description: '%s'" % (self.snap_desc, get["description"]) - self.assertEqual(self.snap_desc, get["description"], msg) + msg = ("Expected description: '%s' actual description: '%s'" % + (self.snap_desc, snapshot["description"])) + self.assertEqual(self.snap_desc, snapshot["description"], msg) - msg = "Expected share_id: '%s', "\ - "actual share_id: '%s'" % (self.shares[0]["id"], get["share_id"]) - self.assertEqual(self.shares[0]["id"], get["share_id"], msg) + msg = ("Expected share_id: '%s', actual share_id: '%s'" % + (self.shares[0]["id"], snapshot["share_id"])) + 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 - snaps = self.shares_client.list_snapshots_with_detail() + 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", - "share_proto", "created_at", - "description", "id", "share_size"] - [self.assertIn(key, sn.keys()) for sn in snaps for key in keys] + expected_keys = ["status", "links", "share_id", "name", + "share_proto", "created_at", "description", "id", + "share_size", "size"] + 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"]] diff --git a/releasenotes/notes/add_user_id_and_project_id_to_snapshot_APIs-157614b4b8d01e15.yaml b/releasenotes/notes/add_user_id_and_project_id_to_snapshot_APIs-157614b4b8d01e15.yaml new file mode 100644 index 00000000..32f4b731 --- /dev/null +++ b/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. \ No newline at end of file