diff --git a/manilaclient/v1/security_services.py b/manilaclient/v1/security_services.py index 391238b..0674cce 100644 --- a/manilaclient/v1/security_services.py +++ b/manilaclient/v1/security_services.py @@ -33,8 +33,16 @@ class SecurityService(common_base.Resource): def __repr__(self): return "" % self.id + def update(self, **kwargs): + """Update this security service.""" + return self.manager.update(self, **kwargs) -class SecurityServiceManager(base.Manager): + def delete(self): + """"Delete this security service.""" + self.manager.delete(self) + + +class SecurityServiceManager(base.ManagerWithFind): """Manage :class:`SecurityService` resources.""" resource_class = SecurityService @@ -80,7 +88,10 @@ class SecurityServiceManager(base.Manager): :param security_service: security service to get. :rtype: :class:`SecurityService` """ - return self._get(RESOURCE_PATH % security_service, RESOURCE_NAME) + return self._get( + RESOURCE_PATH % common_base.getid(security_service), + RESOURCE_NAME, + ) def update(self, security_service, dns_ip=None, server=None, domain=None, password=None, user=None, name=None, description=None): @@ -119,18 +130,20 @@ class SecurityServiceManager(base.Manager): body = {RESOURCE_NAME: values} - return self._update(RESOURCE_PATH % security_service, - body, - RESOURCE_NAME) + return self._update( + RESOURCE_PATH % common_base.getid(security_service), + body, + RESOURCE_NAME, + ) def delete(self, security_service): """Delete a security service. :param security_service: security service to be deleted. """ - self._delete(RESOURCE_PATH % security_service) + self._delete(RESOURCE_PATH % common_base.getid(security_service)) - def list(self, detailed=False, search_opts=None): + def list(self, detailed=True, search_opts=None): """Get a list of all security services. :rtype: list of :class:`SecurityService` diff --git a/manilaclient/v1/share_networks.py b/manilaclient/v1/share_networks.py index 05b014d..879eb60 100644 --- a/manilaclient/v1/share_networks.py +++ b/manilaclient/v1/share_networks.py @@ -70,34 +70,40 @@ class ShareNetworkManager(base.ManagerWithFind): return self._create(RESOURCES_PATH, body, RESOURCE_NAME) def add_security_service(self, share_network, security_service): - """Associate given security service with a share network + """Associate given security service with a share network. :param share_network: share network name, id or ShareNetwork instance - :param security_service: security service name or id + :param security_service: name, id or SecurityService instance :rtype: :class:`ShareNetwork` """ - body = {'add_security_service': {'security_service_id': - security_service}} - - return self._create(RESOURCE_PATH % common_base.getid(share_network) + - '/action', - body, - RESOURCE_NAME) + body = { + 'add_security_service': { + 'security_service_id': common_base.getid(security_service), + }, + } + return self._create( + RESOURCE_PATH % common_base.getid(share_network) + '/action', + body, + RESOURCE_NAME, + ) def remove_security_service(self, share_network, security_service): - """Dissociate security service from a share network + """Dissociate security service from a share network. :param share_network: share network name, id or ShareNetwork instance - :param security_service: security service name or id + :param security_service: name, id or SecurityService instance :rtype: :class:`ShareNetwork` """ - body = {'remove_security_service': {'security_service_id': - security_service}} - - return self._create(RESOURCE_PATH % common_base.getid(share_network) + - '/action', - body, - RESOURCE_NAME) + body = { + 'remove_security_service': { + 'security_service_id': common_base.getid(security_service), + }, + } + return self._create( + RESOURCE_PATH % common_base.getid(share_network) + '/action', + body, + RESOURCE_NAME, + ) def get(self, share_network): """Get a share network. diff --git a/manilaclient/v1/shell.py b/manilaclient/v1/shell.py index a5bbf3e..007e5be 100644 --- a/manilaclient/v1/shell.py +++ b/manilaclient/v1/shell.py @@ -81,6 +81,11 @@ def _find_share_network(cs, share_network): return cliutils.find_resource(cs.share_networks, share_network) +def _find_security_service(cs, security_service): + "Get a security service by ID or name." + return cliutils.find_resource(cs.security_services, security_service) + + def _translate_keys(collection, convert): for item in collection: keys = item.__dict__ @@ -880,12 +885,12 @@ def do_share_network_list(cs, args): @cliutils.arg( 'security_service', metavar='', - help='Security service to associate with.') + help='Security service name or ID to associate with.') def do_share_network_security_service_add(cs, args): """Associate security service with share network.""" share_network = _find_share_network(cs, args.share_network) - cs.share_networks.add_security_service(share_network, - args.security_service) + security_service = _find_security_service(cs, args.security_service) + cs.share_networks.add_security_service(share_network, security_service) @cliutils.arg( @@ -895,12 +900,12 @@ def do_share_network_security_service_add(cs, args): @cliutils.arg( 'security_service', metavar='', - help='Security service to dissociate.') + help='Security service name or ID to dissociate.') def do_share_network_security_service_remove(cs, args): """Dissociate security service from share network.""" share_network = _find_share_network(cs, args.share_network) - cs.share_networks.remove_security_service(share_network, - args.security_service) + security_service = _find_security_service(cs, args.security_service) + cs.share_networks.remove_security_service(share_network, security_service) @cliutils.arg( @@ -985,7 +990,7 @@ def do_security_service_create(cs, args): @cliutils.arg( 'security_service', metavar='', - help='Security service to update.') + help='Security service name or ID to update.') @cliutils.arg( '--dns-ip', metavar='', @@ -1032,19 +1037,18 @@ def do_security_service_update(cs, args): 'name': args.name, 'description': args.description, } - security_service = cs.security_services.update(args.security_service, - **values) - info = security_service._info.copy() - utils.print_dict(info) + security_service = _find_security_service( + cs, args.security_service).update(**values) + utils.print_dict(security_service._info) @cliutils.arg( 'security_service', metavar='', - help='Security service to show.') + help='Security service name or ID to show.') def do_security_service_show(cs, args): """Show security service.""" - security_service = cs.security_services.get(args.security_service) + security_service = _find_security_service(cs, args.security_service) info = security_service._info.copy() utils.print_dict(info) @@ -1078,10 +1082,11 @@ def do_security_service_list(cs, args): @cliutils.arg( 'security_service', metavar='', - help='Security service to delete.') + help='Security service name or ID to delete.') def do_security_service_delete(cs, args): """Delete security service.""" - cs.security_services.delete(args.security_service) + security_service = _find_security_service(cs, args.security_service) + security_service.delete() @cliutils.arg( diff --git a/tests/v1/fakes.py b/tests/v1/fakes.py index 8692c2b..12bfbea 100644 --- a/tests/v1/fakes.py +++ b/tests/v1/fakes.py @@ -122,6 +122,10 @@ class FakeHTTPClient(fakes.FakeHTTPClient): share_nw = {'share_network': {'id': 1111, 'name': 'fake_share_nw'}} return (200, {}, share_nw) + def post_share_networks_1234_action(self, **kw): + share_nw = {'share_network': {'id': 1111, 'name': 'fake_share_nw'}} + return (200, {}, share_nw) + def get_share_networks_detail(self, **kw): share_nw = { 'share_networks': [ @@ -134,9 +138,37 @@ class FakeHTTPClient(fakes.FakeHTTPClient): def get_security_services(self, **kw): security_services = { - 'security_services': [{'id': 1111, - 'name': 'fake_security_service', - 'share_network_id': 1234}]} + 'security_services': [ + { + 'id': 1111, + 'name': 'fake_security_service', + 'type': 'fake_type', + 'status': 'fake_status', + }, + ], + } + return (200, {}, security_services) + + def get_security_services_detail(self, **kw): + security_services = { + 'security_services': [ + { + 'id': 1111, + 'name': 'fake_security_service', + 'description': 'fake_description', + 'share_network_id': 'fake_share-network_id', + 'user': 'fake_user', + 'password': 'fake_password', + 'domain': 'fake_domain', + 'server': 'fake_server', + 'dns_ip': 'fake_dns_ip', + 'type': 'fake_type', + 'status': 'fake_status', + 'project_id': 'fake_project_id', + 'updated_at': 'fake_updated_at', + }, + ], + } return (200, {}, security_services) # diff --git a/tests/v1/test_security_services.py b/tests/v1/test_security_services.py index a8e64c6..36c213d 100644 --- a/tests/v1/test_security_services.py +++ b/tests/v1/test_security_services.py @@ -1,4 +1,4 @@ -# Copyright 2013 OpenStack LLC. +# Copyright 2013 OpenStack Foundation. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -12,11 +12,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -try: - from collections import OrderedDict # noqa -except ImportError: - from ordereddict import OrderedDict # noqa - import mock from manilaclient import exceptions @@ -27,6 +22,9 @@ from tests.v1 import fakes class SecurityServiceTest(utils.TestCase): + class _FakeSecurityService(object): + id = 'fake_security_service_id' + def setUp(self): super(SecurityServiceTest, self).setUp() self.manager = security_services.SecurityServiceManager(api=None) @@ -79,6 +77,13 @@ class SecurityServiceTest(utils.TestCase): self.manager._delete.assert_called_once_with( security_services.RESOURCE_PATH % security_service) + def test_delete_by_object(self): + security_service = self._FakeSecurityService() + with mock.patch.object(self.manager, '_delete', mock.Mock()): + self.manager.delete(security_service) + self.manager._delete.assert_called_once_with( + security_services.RESOURCE_PATH % security_service.id) + def test_get(self): security_service = 'fake service' with mock.patch.object(self.manager, '_get', mock.Mock()): @@ -87,15 +92,23 @@ class SecurityServiceTest(utils.TestCase): security_services.RESOURCE_PATH % security_service, security_services.RESOURCE_NAME) - def test_list_no_filters(self): + def test_get_by_object(self): + security_service = self._FakeSecurityService() + with mock.patch.object(self.manager, '_get', mock.Mock()): + self.manager.get(security_service) + self.manager._get.assert_called_once_with( + security_services.RESOURCE_PATH % security_service.id, + security_services.RESOURCE_NAME) + + def test_list_summary(self): with mock.patch.object(self.manager, '_list', mock.Mock(return_value=None)): - self.manager.list() + self.manager.list(detailed=False) self.manager._list.assert_called_once_with( security_services.RESOURCES_PATH, security_services.RESOURCES_NAME) - def test_list_detailed(self): + def test_list_detail(self): with mock.patch.object(self.manager, '_list', mock.Mock(return_value=None)): self.manager.list(detailed=True) @@ -103,12 +116,17 @@ class SecurityServiceTest(utils.TestCase): security_services.RESOURCES_PATH + '/detail', security_services.RESOURCES_NAME) - def test_list_with_filters(self): - filters = OrderedDict([('all_tenants', 1), - ('status', 'ERROR'), - ('network', 'fake_network')]) - expected_postfix = '?all_tenants=1&network=fake_network&status=ERROR' + def test_list_no_filters(self): + with mock.patch.object(self.manager, '_list', + mock.Mock(return_value=None)): + self.manager.list() + self.manager._list.assert_called_once_with( + security_services.RESOURCES_PATH + '/detail', + security_services.RESOURCES_NAME) + def test_list_with_filters(self): + filters = {'all_tenants': 1, 'network': 'fake', 'status': 'ERROR'} + expected_postfix = ('/detail?all_tenants=1&network=fake&status=ERROR') with mock.patch.object(self.manager, '_list', mock.Mock(return_value=None)): self.manager.list(search_opts=filters) @@ -125,16 +143,32 @@ class SecurityServiceTest(utils.TestCase): 'user': 'new user', 'password': 'fake password', } - with mock.patch.object(self.manager, '_update', fakes.fake_update): result = self.manager.update(security_service, **values) self.assertEqual( result['url'], security_services.RESOURCE_PATH % security_service) - self.assertEqual(result['resp_key'], - security_services.RESOURCE_NAME) - self.assertEqual(result['body'][security_services.RESOURCE_NAME], - values) + self.assertEqual( + result['resp_key'], + security_services.RESOURCE_NAME) + self.assertEqual( + result['body'][security_services.RESOURCE_NAME], + values) + + def test_update_by_object(self): + security_service = self._FakeSecurityService() + values = {'user': 'fake_user'} + with mock.patch.object(self.manager, '_update', fakes.fake_update): + result = self.manager.update(security_service, **values) + self.assertEqual( + result['url'], + security_services.RESOURCE_PATH % security_service.id) + self.assertEqual( + result['resp_key'], + security_services.RESOURCE_NAME) + self.assertEqual( + result['body'][security_services.RESOURCE_NAME], + values) def test_update_no_fields_specified(self): security_service = 'fake service' diff --git a/tests/v1/test_share_networks.py b/tests/v1/test_share_networks.py index c046389..0a7c8dc 100644 --- a/tests/v1/test_share_networks.py +++ b/tests/v1/test_share_networks.py @@ -1,4 +1,4 @@ -# Copyright 2013 OpenStack LLC. +# Copyright 2013 OpenStack Foundation. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -12,11 +12,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -try: - from collections import OrderedDict # noqa -except ImportError: - from ordereddict import OrderedDict # noqa - import mock from manilaclient import exceptions @@ -27,8 +22,11 @@ from tests.v1 import fakes class ShareNetworkTest(utils.TestCase): - class _FakeShareNetwork: - id = 'fake id' + class _FakeShareNetwork(object): + id = 'fake_share_network_id' + + class _FakeSecurityService(object): + id = 'fake_security_service_id' def setUp(self): super(ShareNetworkTest, self).setUp() @@ -89,7 +87,7 @@ class ShareNetworkTest(utils.TestCase): share_networks.RESOURCES_NAME) def test_list_with_filters(self): - filters = OrderedDict([('all_tenants', 1), ('status', 'ERROR')]) + filters = {'all_tenants': 1, 'status': 'ERROR'} expected_path = ("%s/detail?all_tenants=1&status=" "ERROR" % share_networks.RESOURCES_PATH) @@ -132,28 +130,30 @@ class ShareNetworkTest(utils.TestCase): security_service = 'fake security service' share_nw = 'fake share nw' expected_path = (share_networks.RESOURCE_PATH + '/action') % share_nw - expected_body = {'add_security_service': {'security_service_id': - security_service}} - + expected_body = { + 'add_security_service': { + 'security_service_id': security_service, + }, + } with mock.patch.object(self.manager, '_create', mock.Mock()): self.manager.add_security_service(share_nw, security_service) - self.manager._create.assert_called_once_with( expected_path, expected_body, share_networks.RESOURCE_NAME) def test_add_security_service_to_share_nw_object(self): - security_service = 'fake security service' + security_service = self._FakeSecurityService() share_nw = self._FakeShareNetwork() expected_path = ((share_networks.RESOURCE_PATH + '/action') % share_nw.id) - expected_body = {'add_security_service': {'security_service_id': - security_service}} - + expected_body = { + 'add_security_service': { + 'security_service_id': security_service.id, + }, + } with mock.patch.object(self.manager, '_create', mock.Mock()): self.manager.add_security_service(share_nw, security_service) - self.manager._create.assert_called_once_with( expected_path, expected_body, @@ -163,28 +163,30 @@ class ShareNetworkTest(utils.TestCase): security_service = 'fake security service' share_nw = 'fake share nw' expected_path = (share_networks.RESOURCE_PATH + '/action') % share_nw - expected_body = {'remove_security_service': {'security_service_id': - security_service}} - + expected_body = { + 'remove_security_service': { + 'security_service_id': security_service, + }, + } with mock.patch.object(self.manager, '_create', mock.Mock()): self.manager.remove_security_service(share_nw, security_service) - self.manager._create.assert_called_once_with( expected_path, expected_body, share_networks.RESOURCE_NAME) def test_remove_security_service_from_share_nw_object(self): - security_service = 'fake security service' + security_service = self._FakeSecurityService() share_nw = self._FakeShareNetwork() expected_path = ((share_networks.RESOURCE_PATH + '/action') % share_nw.id) - expected_body = {'remove_security_service': {'security_service_id': - security_service}} - + expected_body = { + 'remove_security_service': { + 'security_service_id': security_service.id, + }, + } with mock.patch.object(self.manager, '_create', mock.Mock()): self.manager.remove_security_service(share_nw, security_service) - self.manager._create.assert_called_once_with( expected_path, expected_body, diff --git a/tests/v1/test_shell.py b/tests/v1/test_shell.py index f7af31f..799715e 100644 --- a/tests/v1/test_shell.py +++ b/tests/v1/test_shell.py @@ -220,9 +220,28 @@ class ShellTest(utils.TestCase): expected = {'os-reset_status': {'status': 'error'}} self.assert_called('POST', '/snapshots/1234/action', body=expected) + def test_share_network_security_service_add(self): + self.run_command('share-network-security-service-add fake_share_nw ' + 'fake_security_service') + self.assert_called( + 'POST', + '/share-networks/1234/action', + ) + + def test_share_network_security_service_remove(self): + self.run_command('share-network-security-service-remove fake_share_nw ' + 'fake_security_service') + self.assert_called( + 'POST', + '/share-networks/1234/action', + ) + def test_share_network_security_service_list_by_name(self): self.run_command('share-network-security-service-list fake_share_nw') - self.assert_called('GET', '/security-services?share_network_id=1234') + self.assert_called( + 'GET', + '/security-services/detail?share_network_id=1234', + ) def test_share_network_security_service_list_by_name_not_found(self): self.assertRaises( @@ -240,7 +259,10 @@ class ShellTest(utils.TestCase): def test_share_network_security_service_list_by_id(self): self.run_command('share-network-security-service-list 1111') - self.assert_called('GET', '/security-services?share_network_id=1111') + self.assert_called( + 'GET', + '/security-services/detail?share_network_id=1111', + ) def test_share_server_delete(self): self.run_command('share-server-delete 1234')