diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 1c5ba3c22c..bf7c11a6a0 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -53,14 +53,14 @@ REST_API_VERSION_HISTORY = """ * 2.5 - Share Migration admin API * 2.6 - Return share_type UUID instead of name in Share API * 2.7 - Rename old extension-like API URLs to core-API-like - + * 2.8 - Attr "is_public" can be set for share using API "manage" """ # 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.7" +_MAX_API_VERSION = "2.8" 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 08f5bdb882..1fa58f701c 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -65,3 +65,7 @@ user documentation. 2.7 --- Rename old extension-like API URLs to core-API-like. + +2.8 +--- + Allow to set share visibility explicitly using "manage" API. diff --git a/manila/api/v1/share_manage.py b/manila/api/v1/share_manage.py index da032ecc7d..a16cfcddd6 100644 --- a/manila/api/v1/share_manage.py +++ b/manila/api/v1/share_manage.py @@ -49,6 +49,9 @@ class ShareManageMixin(object): 'display_description': description, } + if share_data.get('is_public') is not None: + share['is_public'] = share_data['is_public'] + driver_options = share_data.get('driver_options', {}) try: @@ -125,6 +128,7 @@ class ShareManageController(ShareManageMixin, wsgi.Controller): Should be removed when minimum API version becomes equal to or greater than v2.7 """ + body.get('share', {}).pop('is_public', None) return self._manage(req, body) diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index 76c716cd21..471241a04f 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -136,9 +136,16 @@ class ShareController(shares.ShareMixin, """Shrink size of a share.""" return self._shrink(req, id, body) - @wsgi.Controller.api_version('2.7') + @wsgi.Controller.api_version('2.7', '2.7') def manage(self, req, body): - return self._manage(req, body) + body.get('share', {}).pop('is_public', None) + detail = self._manage(req, body) + return detail + + @wsgi.Controller.api_version("2.8") # noqa + def manage(self, req, body): # pylint: disable=E0102 + detail = self._manage(req, body) + return detail @wsgi.Controller.api_version('2.7') @wsgi.action('unmanage') diff --git a/manila/tests/api/v1/test_share_manage.py b/manila/tests/api/v1/test_share_manage.py index 2e4d7ab836..2c87bceaa0 100644 --- a/manila/tests/api/v1/test_share_manage.py +++ b/manila/tests/api/v1/test_share_manage.py @@ -54,7 +54,6 @@ class ShareManageTest(test.TestCase): policy, 'check_policy', mock.Mock(return_value=True)) @ddt.data({}, - {'share': None}, {'shares': {}}, {'share': get_fake_manage_body('', None, None)}) def test_share_manage_invalid_body(self, body): @@ -173,6 +172,7 @@ class ShareManageTest(test.TestCase): 'display_name': 'foo', 'display_description': 'bar', } + data['share']['is_public'] = 'foo' driver_options = data['share'].get('driver_options', {}) actual_result = self.controller.create(self.request, data) diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index c940107c8b..5265bc2c8c 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -30,6 +30,7 @@ from manila.common import constants from manila import context from manila import db from manila import exception +from manila import policy from manila.share import api as share_api from manila.share import share_types from manila import test @@ -1299,8 +1300,11 @@ class ShareManageTest(test.TestCase): def setUp(self): super(self.__class__, self).setUp() self.controller = shares.ShareController() + self.resource_name = self.controller.resource_name self.request = fakes.HTTPRequest.blank( - '/v2/share/manage', use_admin_context=True, version='2.7') + '/v2/shares/manage', use_admin_context=True, version='2.7') + self.mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(return_value=True)) def _setup_manage_mocks(self, service_is_up=True): self.mock_object(db, 'service_get_by_host_and_topic', mock.Mock( @@ -1318,7 +1322,6 @@ class ShareManageTest(test.TestCase): mock.Mock(side_effect=exception.ServiceIsDown(service='fake'))) @ddt.data({}, - {'share': None}, {'shares': {}}, {'share': get_fake_manage_body('', None, None)}) def test_share_manage_invalid_body(self, body): @@ -1404,6 +1407,16 @@ class ShareManageTest(test.TestCase): driver_options=dict(volume_id='quuz')), ) def test_share_manage(self, data): + self._test_share_manage(data, "2.7") + + @ddt.data( + get_fake_manage_body(name='foo', description='bar', is_public=True), + get_fake_manage_body(name='foo', description='bar', is_public=False) + ) + def test_share_manage_with_is_public(self, data): + self._test_share_manage(data, "2.8") + + def _test_share_manage(self, data, version): self._setup_manage_mocks() return_share = {'share_type_id': '', 'id': 'fake'} self.mock_object( @@ -1418,11 +1431,20 @@ class ShareManageTest(test.TestCase): } driver_options = data['share'].get('driver_options', {}) - actual_result = self.controller.manage(self.request, data) + if (api_version.APIVersionRequest(version) >= + api_version.APIVersionRequest('2.8')): + share['is_public'] = data['share']['is_public'] + + req = fakes.HTTPRequest.blank('/v2/shares/manage', version=version, + use_admin_context=True) + + actual_result = self.controller.manage(req, data) share_api.API.manage.assert_called_once_with( mock.ANY, share, driver_options) self.assertIsNotNone(actual_result) + self.mock_policy_check.assert_called_once_with( + req.environ['manila.context'], self.resource_name, 'manage') def test_wrong_permissions(self): body = get_fake_manage_body() diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py index 63b3211fba..ddf11cd098 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.7", + default="2.8", 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/services/share/v2/json/shares_client.py b/manila_tempest_tests/services/share/v2/json/shares_client.py index 877a82f526..22825106af 100644 --- a/manila_tempest_tests/services/share/v2/json/shares_client.py +++ b/manila_tempest_tests/services/share/v2/json/shares_client.py @@ -323,7 +323,8 @@ class SharesV2Client(shares_client.SharesClient): def manage_share(self, service_host, protocol, export_path, share_type_id, name=None, description=None, - version=LATEST_MICROVERSION, url=None): + is_public=False, version=LATEST_MICROVERSION, + url=None): post_body = { "share": { "export_path": export_path, @@ -332,6 +333,7 @@ class SharesV2Client(shares_client.SharesClient): "share_type": share_type_id, "name": name, "description": description, + "is_public": is_public, } } if url is None: diff --git a/manila_tempest_tests/tests/api/admin/test_share_manage.py b/manila_tempest_tests/tests/api/admin/test_share_manage.py index 78255ca780..78d28ee6dd 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_manage.py +++ b/manila_tempest_tests/tests/api/admin/test_share_manage.py @@ -75,31 +75,37 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): 'share_type_id': cls.st['share_type']['id'], 'share_protocol': cls.protocol, }} - # Create two shares in parallel - cls.shares = cls.create_shares([creation_data, creation_data]) + # Data for creating shares in parallel + data = [creation_data, creation_data] + if float(CONF.share.max_api_microversion) >= 2.8: + data.append(creation_data) + shares_created = cls.create_shares(data) + + cls.shares = [] # Load all share data (host, etc.) - cls.share1 = cls.shares_v2_client.get_share(cls.shares[0]['id']) - cls.share2 = cls.shares_v2_client.get_share(cls.shares[1]['id']) + for share in shares_created: + # Unmanage shares from manila + cls.shares.append(cls.shares_client.get_share(share['id'])) + cls.shares_client.unmanage_share(share['id']) + cls.shares_client.wait_for_resource_deletion( + share_id=share['id']) - # Unmanage shares from manila - for share_id in (cls.share1['id'], cls.share2['id']): - cls.shares_v2_client.unmanage_share(share_id) - cls.shares_v2_client.wait_for_resource_deletion(share_id=share_id) - - @test.attr(type=["gate", "smoke"]) - def test_manage(self): - name = "Name for 'managed' share that had ID %s" % self.share1["id"] + def _test_manage(self, share, is_public=False, + version=CONF.share.max_api_microversion): + name = "Name for 'managed' share that had ID %s" % \ + share['id'] description = "Description for 'managed' share" # Manage share share = self.shares_v2_client.manage_share( - service_host=self.share1['host'], - export_path=self.share1['export_locations'][0], - protocol=self.share1['share_proto'], + service_host=share['host'], + export_path=share['export_locations'][0], + protocol=share['share_proto'], share_type_id=self.st['share_type']['id'], name=name, description=description, + is_public=is_public, ) # Add managed share to cleanup queue @@ -114,12 +120,17 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): get = self.shares_v2_client.get_share(share['id'], version="2.5") self.assertEqual(name, get['name']) self.assertEqual(description, get['description']) - self.assertEqual(self.share1['host'], get['host']) - self.assertEqual(self.share1['share_proto'], get['share_proto']) + self.assertEqual(share['host'], get['host']) + self.assertEqual(share['share_proto'], get['share_proto']) self.assertEqual(self.st['share_type']['name'], get['share_type']) - get = self.shares_v2_client.get_share(share['id'], version="2.6") - self.assertEqual(self.st['share_type']['id'], get['share_type']) + share = self.shares_v2_client.get_share(share['id'], version="2.6") + self.assertEqual(self.st['share_type']['id'], share['share_type']) + + if float(version) >= 2.8: + self.assertEqual(is_public, share['is_public']) + else: + self.assertFalse(share['is_public']) # Delete share self.shares_v2_client.delete_share(share['id']) @@ -128,6 +139,17 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): self.shares_v2_client.get_share, share['id']) + @testtools.skipIf( + float(CONF.share.max_api_microversion) < 2.8, + "Only for API Microversion >= 2.8") + @test.attr(type=["gate", "smoke"]) + def test_manage_with_is_public_True(self): + self._test_manage(share=self.shares[2], is_public=True) + + @test.attr(type=["gate", "smoke"]) + def test_manage(self): + self._test_manage(share=self.shares[0]) + @test.attr(type=["gate", "smoke"]) def test_manage_retry(self): # Manage share with invalid parameters @@ -137,9 +159,9 @@ class ManageNFSShareTest(base.BaseSharesAdminTest): for share_type_id, status in parameters: share = self.shares_v2_client.manage_share( - service_host=self.share2['host'], - export_path=self.share2['export_locations'][0], - protocol=self.share2['share_proto'], + service_host=self.shares[1]['host'], + export_path=self.shares[1]['export_locations'][0], + protocol=self.shares[1]['share_proto'], share_type_id=share_type_id) # Add managed share to cleanup queue