diff --git a/manila/api/v2/share_group_types.py b/manila/api/v2/share_group_types.py index fe9114ca9b..6171265d39 100644 --- a/manila/api/v2/share_group_types.py +++ b/manila/api/v2/share_group_types.py @@ -131,6 +131,14 @@ class ShareGroupTypesController(wsgi.Controller): if name is None or name == "" or len(name) > 255: msg = _("Share group type name is not valid.") raise webob.exc.HTTPBadRequest(explanation=msg) + if not (specs is None or isinstance(specs, dict)): + msg = _("Group specs can be either of 'None' or 'dict' types.") + raise webob.exc.HTTPBadRequest(explanation=msg) + if specs: + for element in list(specs.keys()) + list(specs.values()): + if not isinstance(element, six.string_types): + msg = _("Group specs keys and values should be strings.") + raise webob.exc.HTTPBadRequest(explanation=msg) try: share_group_types.create( context, name, share_types, specs, is_public) diff --git a/manila/tests/api/v2/test_share_group_types.py b/manila/tests/api/v2/test_share_group_types.py index abecc498d5..4810727b2a 100644 --- a/manila/tests/api/v2/test_share_group_types.py +++ b/manila/tests/api/v2/test_share_group_types.py @@ -298,11 +298,13 @@ class ShareGroupTypesAPITest(test.TestCase): mock_get.assert_called_once_with(mock.ANY, GROUP_TYPE_1['name']) self.assertEqual(expected_type, res_dict['share_group_type']) - def test_create_with_group_specs(self): - fake_group_specs = {'my_fake_group_spec': 'false'} + @ddt.data( + None, {'my_fake_group_spec': 'false'}, + ) + def test_create_with_group_specs(self, specs): fake_type = copy.deepcopy(GROUP_TYPE_1) fake_type['share_types'] = [{'share_type_id': SHARE_TYPE_ID}] - fake_type['group_specs'] = fake_group_specs + fake_type['group_specs'] = specs mock_create = self.mock_object(share_group_types, 'create') mock_get = self.mock_object( share_group_types, 'get_by_name', @@ -311,24 +313,49 @@ class ShareGroupTypesAPITest(test.TestCase): fake_body = {'share_group_type': { 'name': GROUP_TYPE_1['name'], 'share_types': [SHARE_TYPE_ID], - 'group_specs': fake_group_specs, + 'group_specs': specs, }} expected_type = { 'id': GROUP_TYPE_1['id'], 'name': GROUP_TYPE_1['name'], 'is_public': True, - 'group_specs': fake_group_specs, + 'group_specs': specs, 'share_types': [SHARE_TYPE_ID], } res_dict = self.controller._create(req, fake_body) mock_create.assert_called_once_with( - mock.ANY, GROUP_TYPE_1['name'], [SHARE_TYPE_ID], fake_group_specs, + mock.ANY, GROUP_TYPE_1['name'], [SHARE_TYPE_ID], specs, True) mock_get.assert_called_once_with(mock.ANY, GROUP_TYPE_1['name']) self.assertEqual(expected_type, res_dict['share_group_type']) + @ddt.data( + 'str', ['l', 'i', 's', 't'], set([1]), ('t', 'u', 'p', 'l', 'e'), 1, + {"foo": 1}, {1: "foo"}, {"foo": "bar", "quuz": []} + ) + def test_create_with_wrong_group_specs(self, specs): + fake_type = copy.deepcopy(GROUP_TYPE_1) + fake_type['share_types'] = [{'share_type_id': SHARE_TYPE_ID}] + fake_type['group_specs'] = specs + mock_create = self.mock_object(share_group_types, 'create') + mock_get = self.mock_object( + share_group_types, 'get_by_name', + mock.Mock(return_value=fake_type)) + req = fake_request('/v2/fake/share-group-types') + fake_body = {'share_group_type': { + 'name': GROUP_TYPE_1['name'], + 'share_types': [SHARE_TYPE_ID], + 'group_specs': specs, + }} + + self.assertRaises( + webob.exc.HTTPBadRequest, self.controller._create, req, fake_body) + + self.assertEqual(0, mock_create.call_count) + self.assertEqual(0, mock_get.call_count) + def test_create_private_share_group_type(self): fake_type = copy.deepcopy(GROUP_TYPE_1) fake_type['share_types'] = [{'share_type_id': SHARE_TYPE_ID}] diff --git a/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py b/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py index 18e8cdde2f..6dfe20456b 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py @@ -69,6 +69,15 @@ class ShareGroupTypesAdminNegativeTest(base.BaseSharesMixedTest): self.create_share_group_type, "x" * 256, client=self.admin_shares_v2_client) + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) + def test_create_share_group_type_with_wrong_value_for_group_specs(self): + self.assertRaises( + lib_exc.BadRequest, + self.admin_shares_v2_client.create_share_group_type, + name=data_utils.rand_name("tempest_manila"), + share_types=[self.share_type['share_type']['id']], + group_specs="expecting_error_code_400") + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_get_share_group_type_using_nonexistent_id(self): self.assertRaises( diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py index 236351dd61..3df71537ab 100644 --- a/manila_tempest_tests/tests/api/base.py +++ b/manila_tempest_tests/tests/api/base.py @@ -595,7 +595,8 @@ class BaseSharesTest(test.BaseTestCase): group_specs=None, client=None, cleanup_in_class=True, **kwargs): client = client or cls.shares_v2_client - if group_specs is None: + if (group_specs is None and + CONF.share.capability_sg_consistent_snapshot_support): group_specs = { 'consistent_snapshot_support': ( CONF.share.capability_sg_consistent_snapshot_support),