diff --git a/openstackclient/identity/common.py b/openstackclient/identity/common.py index a9d8afa6f5..4e5f083de2 100644 --- a/openstackclient/identity/common.py +++ b/openstackclient/identity/common.py @@ -20,6 +20,7 @@ from keystoneclient.v3 import domains from keystoneclient.v3 import groups from keystoneclient.v3 import projects from keystoneclient.v3 import users +from openstack import exceptions as sdk_exceptions from osc_lib import exceptions from osc_lib import utils @@ -73,6 +74,39 @@ def find_service(identity_client, name_type_or_id): raise exceptions.CommandError(msg % name_type_or_id) +def find_service_sdk(identity_client, name_type_or_id): + """Find a service by id, name or type.""" + + try: + # search for service name or ID + return identity_client.find_service( + name_type_or_id, ignore_missing=False + ) + except sdk_exceptions.ResourceNotFound: + pass + except sdk_exceptions.DuplicateResource as e: + raise exceptions.CommandError(e.message) + + # search for service type + services = identity_client.services() + result = None + for service in services: + if name_type_or_id == service.type: + if result: + msg = _( + "Multiple service matches found for '%s', " + "use an ID or name to be more specific." + ) + raise exceptions.CommandError(msg % name_type_or_id) + result = service + + if result is None: + msg = _("No service with a type, name or ID of '%s' exists.") + raise exceptions.CommandError(msg % name_type_or_id) + + return result + + def get_resource(manager, name_type_or_id): # NOTE (vishakha): Due to bug #1799153 and for any another related case # where GET resource API does not support the filter by name, diff --git a/openstackclient/identity/v3/service.py b/openstackclient/identity/v3/service.py index 42ee1e474d..b7d4befc96 100644 --- a/openstackclient/identity/v3/service.py +++ b/openstackclient/identity/v3/service.py @@ -28,6 +28,31 @@ from openstackclient.identity import common LOG = logging.getLogger(__name__) +def _format_service(service): + columns = ( + 'id', + 'name', + 'type', + 'is_enabled', + 'description', + ) + column_headers = ( + 'ID', + 'Name', + 'Type', + 'Enabled', + 'Description', + ) + + return ( + column_headers, + utils.get_item_properties( + service, + columns, + ), + ) + + class CreateService(command.ShowOne): _description = _("Create new service") @@ -66,17 +91,16 @@ class CreateService(command.ShowOne): return parser def take_action(self, parsed_args): - identity_client = self.app.client_manager.identity + identity_client = self.app.client_manager.sdk_connection.identity - service = identity_client.services.create( + service = identity_client.create_service( name=parsed_args.name, type=parsed_args.type, description=parsed_args.description, - enabled=parsed_args.is_enabled, + is_enabled=parsed_args.is_enabled, ) - service._info.pop('links') - return zip(*sorted(service._info.items())) + return _format_service(service) class DeleteService(command.Command): @@ -93,12 +117,12 @@ class DeleteService(command.Command): return parser def take_action(self, parsed_args): - identity_client = self.app.client_manager.identity + identity_client = self.app.client_manager.sdk_connection.identity result = 0 for i in parsed_args.service: try: - service = common.find_service(identity_client, i) - identity_client.services.delete(service.id) + service = common.find_service_sdk(identity_client, i) + identity_client.delete_service(service.id) except Exception as e: result += 1 LOG.error( @@ -131,13 +155,19 @@ class ListService(command.Lister): return parser def take_action(self, parsed_args): + identity_client = self.app.client_manager.sdk_connection.identity + if parsed_args.long: - columns = ('ID', 'Name', 'Type', 'Description', 'Enabled') + columns = ('id', 'name', 'type', 'description', 'is_enabled') + column_headers = ('ID', 'Name', 'Type', 'Description', 'Enabled') else: - columns = ('ID', 'Name', 'Type') - data = self.app.client_manager.identity.services.list() + columns = ('id', 'name', 'type') + column_headers = ('ID', 'Name', 'Type') + + data = identity_client.services() + return ( - columns, + column_headers, (utils.get_item_properties(s, columns) for s in data), ) @@ -185,9 +215,9 @@ class SetService(command.Command): return parser def take_action(self, parsed_args): - identity_client = self.app.client_manager.identity + identity_client = self.app.client_manager.sdk_connection.identity - service = common.find_service(identity_client, parsed_args.service) + service = common.find_service_sdk(identity_client, parsed_args.service) kwargs = {} if parsed_args.type: kwargs['type'] = parsed_args.type @@ -195,10 +225,9 @@ class SetService(command.Command): kwargs['name'] = parsed_args.name if parsed_args.description: kwargs['description'] = parsed_args.description - if parsed_args.is_enabled is not None: - kwargs['enabled'] = parsed_args.is_enabled + kwargs['is_enabled'] = parsed_args.is_enabled - identity_client.services.update(service.id, **kwargs) + identity_client.update_service(service.id, **kwargs) class ShowService(command.ShowOne): @@ -214,9 +243,8 @@ class ShowService(command.ShowOne): return parser def take_action(self, parsed_args): - identity_client = self.app.client_manager.identity + identity_client = self.app.client_manager.sdk_connection.identity - service = common.find_service(identity_client, parsed_args.service) + service = common.find_service_sdk(identity_client, parsed_args.service) - service._info.pop('links') - return zip(*sorted(service._info.items())) + return _format_service(service) diff --git a/openstackclient/tests/functional/identity/v3/common.py b/openstackclient/tests/functional/identity/v3/common.py index 8607c05731..7ae56b8de1 100644 --- a/openstackclient/tests/functional/identity/v3/common.py +++ b/openstackclient/tests/functional/identity/v3/common.py @@ -48,7 +48,7 @@ class IdentityTests(base.TestCase): 'parent_id', ] ROLE_FIELDS = ['id', 'name', 'domain_id', 'description'] - SERVICE_FIELDS = ['id', 'enabled', 'name', 'type', 'description'] + SERVICE_FIELDS = ['ID', 'Enabled', 'Name', 'Type', 'Description'] REGION_FIELDS = ['description', 'enabled', 'parent_region', 'region'] ENDPOINT_FIELDS = [ 'id', @@ -367,7 +367,7 @@ class IdentityTests(base.TestCase): if add_clean_up: service = self.parse_show_as_object(raw_output) self.addCleanup( - self.openstack, 'service delete %s' % service['id'] + self.openstack, 'service delete %s' % service['ID'] ) items = self.parse_show(raw_output) self.assert_show_fields(items, self.SERVICE_FIELDS) diff --git a/openstackclient/tests/functional/identity/v3/test_limit.py b/openstackclient/tests/functional/identity/v3/test_limit.py index 32c65107d0..d096762d2e 100644 --- a/openstackclient/tests/functional/identity/v3/test_limit.py +++ b/openstackclient/tests/functional/identity/v3/test_limit.py @@ -32,7 +32,7 @@ class LimitTestCase(common.IdentityTests): raw_output = self.openstack('service show %s' % service_id) items = self.parse_show(raw_output) - service_name = self._extract_value_from_items('name', items) + service_name = self._extract_value_from_items('Name', items) project_name = self._create_dummy_project() raw_output = self.openstack('project show %s' % project_name) @@ -73,7 +73,7 @@ class LimitTestCase(common.IdentityTests): raw_output = self.openstack('service show %s' % service_id) items = self.parse_show(raw_output) - service_name = self._extract_value_from_items('name', items) + service_name = self._extract_value_from_items('Name', items) project_name = self._create_dummy_project() diff --git a/openstackclient/tests/functional/identity/v3/test_registered_limit.py b/openstackclient/tests/functional/identity/v3/test_registered_limit.py index 6e3c30aeb0..ba7f8cbd4d 100644 --- a/openstackclient/tests/functional/identity/v3/test_registered_limit.py +++ b/openstackclient/tests/functional/identity/v3/test_registered_limit.py @@ -29,7 +29,7 @@ class RegisteredLimitTestCase(common.IdentityTests): 'service show' ' %(service_name)s' % {'service_name': service_name} ) service_items = self.parse_show(raw_output) - service_id = self._extract_value_from_items('id', service_items) + service_id = self._extract_value_from_items('ID', service_items) raw_output = self.openstack( 'registered limit create' diff --git a/openstackclient/tests/functional/identity/v3/test_service.py b/openstackclient/tests/functional/identity/v3/test_service.py index 98fa349d1e..76009bf48b 100644 --- a/openstackclient/tests/functional/identity/v3/test_service.py +++ b/openstackclient/tests/functional/identity/v3/test_service.py @@ -61,9 +61,9 @@ class ServiceTests(common.IdentityTests): raw_output = self.openstack('service show %s' % new_service_name) # assert service details service = self.parse_show_as_object(raw_output) - self.assertEqual(new_service_type, service['type']) - self.assertEqual(new_service_name, service['name']) - self.assertEqual(new_service_description, service['description']) + self.assertEqual(new_service_type, service['Type']) + self.assertEqual(new_service_name, service['Name']) + self.assertEqual(new_service_description, service['Description']) def test_service_show(self): service_name = self._create_dummy_service() diff --git a/openstackclient/tests/unit/identity/v3/test_service.py b/openstackclient/tests/unit/identity/v3/test_service.py index 57de1a1f7a..d447121244 100644 --- a/openstackclient/tests/unit/identity/v3/test_service.py +++ b/openstackclient/tests/unit/identity/v3/test_service.py @@ -13,43 +13,37 @@ # under the License. # -from keystoneclient import exceptions as identity_exc +from openstack import exceptions as sdk_exceptions +from openstack.identity.v3 import service as _service +from openstack.test import fakes as sdk_fakes from osc_lib import exceptions from openstackclient.identity.v3 import service from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes -class TestService(identity_fakes.TestIdentityv3): - def setUp(self): - super().setUp() - - # Get a shortcut to the ServiceManager Mock - self.services_mock = self.identity_client.services - self.services_mock.reset_mock() - - -class TestServiceCreate(TestService): +class TestServiceCreate(identity_fakes.TestIdentityv3): columns = ( - 'description', - 'enabled', - 'id', - 'name', - 'type', + 'ID', + 'Name', + 'Type', + 'Enabled', + 'Description', ) def setUp(self): super().setUp() - self.service = identity_fakes.FakeService.create_one_service() + self.service = sdk_fakes.generate_fake_resource(_service.Service) + self.datalist = ( - self.service.description, - True, self.service.id, self.service.name, self.service.type, + True, + self.service.description, ) - self.services_mock.create.return_value = self.service + self.identity_sdk_client.create_service.return_value = self.service # Get the command object to test self.cmd = service.CreateService(self.app, None) @@ -73,12 +67,11 @@ class TestServiceCreate(TestService): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServiceManager.create(name=, type=, enabled=, **kwargs) - self.services_mock.create.assert_called_with( + self.identity_sdk_client.create_service.assert_called_with( name=self.service.name, type=self.service.type, description=None, - enabled=True, + is_enabled=True, ) self.assertEqual(self.columns, columns) @@ -103,12 +96,11 @@ class TestServiceCreate(TestService): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServiceManager.create(name=, type=, enabled=, **kwargs) - self.services_mock.create.assert_called_with( + self.identity_sdk_client.create_service.assert_called_with( name=None, type=self.service.type, description=self.service.description, - enabled=True, + is_enabled=True, ) self.assertEqual(self.columns, columns) @@ -132,12 +124,11 @@ class TestServiceCreate(TestService): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServiceManager.create(name=, type=, enabled=, **kwargs) - self.services_mock.create.assert_called_with( + self.identity_sdk_client.create_service.assert_called_with( name=None, type=self.service.type, description=None, - enabled=True, + is_enabled=True, ) self.assertEqual(self.columns, columns) @@ -161,27 +152,28 @@ class TestServiceCreate(TestService): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServiceManager.create(name=, type=, enabled=, **kwargs) - self.services_mock.create.assert_called_with( + self.identity_sdk_client.create_service.assert_called_with( name=None, type=self.service.type, description=None, - enabled=False, + is_enabled=False, ) self.assertEqual(self.columns, columns) self.assertEqual(self.datalist, data) -class TestServiceDelete(TestService): - service = identity_fakes.FakeService.create_one_service() +class TestServiceDelete(identity_fakes.TestIdentityv3): + service = sdk_fakes.generate_fake_resource(_service.Service) def setUp(self): super().setUp() - self.services_mock.get.side_effect = identity_exc.NotFound(None) - self.services_mock.find.return_value = self.service - self.services_mock.delete.return_value = None + self.identity_sdk_client.get_service.side_effect = ( + sdk_exceptions.ResourceNotFound + ) + self.identity_sdk_client.find_service.return_value = self.service + self.identity_sdk_client.delete_service.return_value = None # Get the command object to test self.cmd = service.DeleteService(self.app, None) @@ -197,19 +189,19 @@ class TestServiceDelete(TestService): result = self.cmd.take_action(parsed_args) - self.services_mock.delete.assert_called_with( + self.identity_sdk_client.delete_service.assert_called_with( self.service.id, ) self.assertIsNone(result) -class TestServiceList(TestService): - service = identity_fakes.FakeService.create_one_service() +class TestServiceList(identity_fakes.TestIdentityv3): + service = sdk_fakes.generate_fake_resource(_service.Service) def setUp(self): super().setUp() - self.services_mock.list.return_value = [self.service] + self.identity_sdk_client.services.return_value = [self.service] # Get the command object to test self.cmd = service.ListService(self.app, None) @@ -224,7 +216,7 @@ class TestServiceList(TestService): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.services_mock.list.assert_called_with() + self.identity_sdk_client.services.assert_called_with() collist = ('ID', 'Name', 'Type') self.assertEqual(collist, columns) @@ -251,7 +243,7 @@ class TestServiceList(TestService): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.services_mock.list.assert_called_with() + self.identity_sdk_client.services.assert_called_with() collist = ('ID', 'Name', 'Type', 'Description', 'Enabled') self.assertEqual(collist, columns) @@ -267,15 +259,17 @@ class TestServiceList(TestService): self.assertEqual(datalist, tuple(data)) -class TestServiceSet(TestService): - service = identity_fakes.FakeService.create_one_service() +class TestServiceSet(identity_fakes.TestIdentityv3): + service = sdk_fakes.generate_fake_resource(_service.Service) def setUp(self): super().setUp() - self.services_mock.get.side_effect = identity_exc.NotFound(None) - self.services_mock.find.return_value = self.service - self.services_mock.update.return_value = self.service + self.identity_sdk_client.get_service.side_effect = ( + sdk_exceptions.ResourceNotFound + ) + self.identity_sdk_client.find_service.return_value = self.service + self.identity_sdk_client.update_service.return_value = self.service # Get the command object to test self.cmd = service.SetService(self.app, None) @@ -317,9 +311,11 @@ class TestServiceSet(TestService): # Set expected values kwargs = { 'type': self.service.type, + 'is_enabled': None, } - # ServiceManager.update(service, name=, type=, enabled=, **kwargs) - self.services_mock.update.assert_called_with(self.service.id, **kwargs) + self.identity_sdk_client.update_service.assert_called_with( + self.service.id, **kwargs + ) self.assertIsNone(result) def test_service_set_name(self): @@ -342,9 +338,11 @@ class TestServiceSet(TestService): # Set expected values kwargs = { 'name': self.service.name, + 'is_enabled': None, } - # ServiceManager.update(service, name=, type=, enabled=, **kwargs) - self.services_mock.update.assert_called_with(self.service.id, **kwargs) + self.identity_sdk_client.update_service.assert_called_with( + self.service.id, **kwargs + ) self.assertIsNone(result) def test_service_set_description(self): @@ -367,9 +365,11 @@ class TestServiceSet(TestService): # Set expected values kwargs = { 'description': self.service.description, + 'is_enabled': None, } - # ServiceManager.update(service, name=, type=, enabled=, **kwargs) - self.services_mock.update.assert_called_with(self.service.id, **kwargs) + self.identity_sdk_client.update_service.assert_called_with( + self.service.id, **kwargs + ) self.assertIsNone(result) def test_service_set_enable(self): @@ -390,10 +390,11 @@ class TestServiceSet(TestService): # Set expected values kwargs = { - 'enabled': True, + 'is_enabled': True, } - # ServiceManager.update(service, name=, type=, enabled=, **kwargs) - self.services_mock.update.assert_called_with(self.service.id, **kwargs) + self.identity_sdk_client.update_service.assert_called_with( + self.service.id, **kwargs + ) self.assertIsNone(result) def test_service_set_disable(self): @@ -414,21 +415,24 @@ class TestServiceSet(TestService): # Set expected values kwargs = { - 'enabled': False, + 'is_enabled': False, } - # ServiceManager.update(service, name=, type=, enabled=, **kwargs) - self.services_mock.update.assert_called_with(self.service.id, **kwargs) + self.identity_sdk_client.update_service.assert_called_with( + self.service.id, **kwargs + ) self.assertIsNone(result) -class TestServiceShow(TestService): - service = identity_fakes.FakeService.create_one_service() +class TestServiceShow(identity_fakes.TestIdentityv3): + service = sdk_fakes.generate_fake_resource(_service.Service) def setUp(self): super().setUp() - self.services_mock.get.side_effect = identity_exc.NotFound(None) - self.services_mock.find.return_value = self.service + self.identity_sdk_client.get_service.side_effect = ( + sdk_exceptions.ResourceNotFound + ) + self.identity_sdk_client.find_service.return_value = self.service # Get the command object to test self.cmd = service.ShowService(self.app, None) @@ -447,22 +451,25 @@ class TestServiceShow(TestService): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServiceManager.get(id) - self.services_mock.find.assert_called_with(name=self.service.name) + self.identity_sdk_client.find_service.assert_called_with( + self.service.name, ignore_missing=False + ) - collist = ('description', 'enabled', 'id', 'name', 'type') + collist = ('ID', 'Name', 'Type', 'Enabled', 'Description') self.assertEqual(collist, columns) datalist = ( - self.service.description, - True, self.service.id, self.service.name, self.service.type, + True, + self.service.description, ) self.assertEqual(datalist, data) def test_service_show_nounique(self): - self.services_mock.find.side_effect = identity_exc.NoUniqueMatch(None) + self.identity_sdk_client.find_service.side_effect = ( + sdk_exceptions.DuplicateResource(None) + ) arglist = [ 'nounique_service', ] @@ -476,7 +483,6 @@ class TestServiceShow(TestService): self.fail('CommandError should be raised.') except exceptions.CommandError as e: self.assertEqual( - "Multiple service matches found for 'nounique_service'," - " use an ID to be more specific.", + "DuplicateResource", str(e), ) diff --git a/releasenotes/notes/migrate-service-to-sdk-6ff62ebf7e41db7c.yaml b/releasenotes/notes/migrate-service-to-sdk-6ff62ebf7e41db7c.yaml new file mode 100644 index 0000000000..90a55fcaab --- /dev/null +++ b/releasenotes/notes/migrate-service-to-sdk-6ff62ebf7e41db7c.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + The following commands have been migrated to SDK: + + - ``service create`` + - ``service delete`` + - ``service set`` + - ``service list`` + - ``service show``