diff --git a/openstackclient/identity/common.py b/openstackclient/identity/common.py index 379f4114e..1e40f3968 100644 --- a/openstackclient/identity/common.py +++ b/openstackclient/identity/common.py @@ -30,21 +30,32 @@ def find_service(identity_client, name_type_or_id): """Find a service by id, name or type.""" try: - # search for the usual ID or name - return utils.find_resource(identity_client.services, name_type_or_id) - except exceptions.CommandError: - try: - # search for service type - return identity_client.services.find(type=name_type_or_id) - # FIXME(dtroyer): This exception should eventually come from - # common client exceptions - except identity_exc.NotFound: - msg = _("No service with a type, name or ID of '%s' exists.") - raise exceptions.CommandError(msg % name_type_or_id) - except identity_exc.NoUniqueMatch: - msg = _("Multiple service matches found for '%s', " - "use an ID to be more specific.") - raise exceptions.CommandError(msg % name_type_or_id) + # search for service id + return identity_client.services.get(name_type_or_id) + except identity_exc.NotFound: + # ignore NotFound exception, raise others + pass + + try: + # search for service name + return identity_client.services.find(name=name_type_or_id) + except identity_exc.NotFound: + pass + except identity_exc.NoUniqueMatch: + msg = _("Multiple service matches found for '%s', " + "use an ID to be more specific.") + raise exceptions.CommandError(msg % name_type_or_id) + + try: + # search for service type + return identity_client.services.find(type=name_type_or_id) + except identity_exc.NotFound: + msg = _("No service with a type, name or ID of '%s' exists.") + raise exceptions.CommandError(msg % name_type_or_id) + except identity_exc.NoUniqueMatch: + msg = _("Multiple service matches found for '%s', " + "use an ID to be more specific.") + raise exceptions.CommandError(msg % name_type_or_id) def _get_token_resource(client, resource, parsed_name): diff --git a/openstackclient/tests/identity/v2_0/test_endpoint.py b/openstackclient/tests/identity/v2_0/test_endpoint.py index b2b6d0f18..26ec654d8 100644 --- a/openstackclient/tests/identity/v2_0/test_endpoint.py +++ b/openstackclient/tests/identity/v2_0/test_endpoint.py @@ -103,9 +103,6 @@ class TestEndpointDelete(TestEndpoint): super(TestEndpointDelete, self).setUp() self.endpoints_mock.get.return_value = self.fake_endpoint - - self.services_mock.get.return_value = self.fake_service - self.endpoints_mock.delete.return_value = None # Get the command object to test diff --git a/openstackclient/tests/identity/v2_0/test_service.py b/openstackclient/tests/identity/v2_0/test_service.py index 318fa83d7..7efd2a604 100644 --- a/openstackclient/tests/identity/v2_0/test_service.py +++ b/openstackclient/tests/identity/v2_0/test_service.py @@ -13,6 +13,9 @@ # under the License. # +from keystoneclient import exceptions as identity_exc +from osc_lib import exceptions + from openstackclient.identity.v2_0 import service from openstackclient.tests.identity.v2_0 import fakes as identity_fakes @@ -170,7 +173,8 @@ class TestServiceDelete(TestService): def setUp(self): super(TestServiceDelete, self).setUp() - self.services_mock.get.return_value = self.fake_service + self.services_mock.get.side_effect = identity_exc.NotFound(None) + self.services_mock.find.return_value = self.fake_service self.services_mock.delete.return_value = None # Get the command object to test @@ -253,20 +257,23 @@ class TestServiceList(TestService): class TestServiceShow(TestService): + fake_service_s = identity_fakes.FakeService.create_one_service() + def setUp(self): super(TestServiceShow, self).setUp() - self.services_mock.get.return_value = self.fake_service + self.services_mock.get.side_effect = identity_exc.NotFound(None) + self.services_mock.find.return_value = self.fake_service_s # Get the command object to test self.cmd = service.ShowService(self.app, None) def test_service_show(self): arglist = [ - self.fake_service.name, + self.fake_service_s.name, ] verifylist = [ - ('service', self.fake_service.name), + ('service', self.fake_service_s.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -275,17 +282,35 @@ class TestServiceShow(TestService): # data to be shown. columns, data = self.cmd.take_action(parsed_args) - # ServiceManager.get(id) - self.services_mock.get.assert_called_with( - self.fake_service.name, + # ServiceManager.find(id) + self.services_mock.find.assert_called_with( + name=self.fake_service_s.name, ) collist = ('description', 'id', 'name', 'type') self.assertEqual(collist, columns) datalist = ( - self.fake_service.description, - self.fake_service.id, - self.fake_service.name, - self.fake_service.type, + self.fake_service_s.description, + self.fake_service_s.id, + self.fake_service_s.name, + self.fake_service_s.type, ) self.assertEqual(datalist, data) + + def test_service_show_nounique(self): + self.services_mock.find.side_effect = identity_exc.NoUniqueMatch(None) + arglist = [ + 'nounique_service', + ] + verifylist = [ + ('service', 'nounique_service'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + try: + self.cmd.take_action(parsed_args) + 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.", str(e)) diff --git a/openstackclient/tests/identity/v3/test_service.py b/openstackclient/tests/identity/v3/test_service.py index a1f85adc7..65d8ebd7d 100644 --- a/openstackclient/tests/identity/v3/test_service.py +++ b/openstackclient/tests/identity/v3/test_service.py @@ -15,6 +15,9 @@ import copy +from keystoneclient import exceptions as identity_exc +from osc_lib import exceptions + from openstackclient.identity.v3 import service from openstackclient.tests import fakes from openstackclient.tests.identity.v3 import fakes as identity_fakes @@ -185,7 +188,8 @@ class TestServiceDelete(TestService): def setUp(self): super(TestServiceDelete, self).setUp() - self.services_mock.get.return_value = fakes.FakeResource( + self.services_mock.get.side_effect = identity_exc.NotFound(None) + self.services_mock.find.return_value = fakes.FakeResource( None, copy.deepcopy(identity_fakes.SERVICE), loaded=True, @@ -282,7 +286,8 @@ class TestServiceSet(TestService): def setUp(self): super(TestServiceSet, self).setUp() - self.services_mock.get.return_value = fakes.FakeResource( + self.services_mock.get.side_effect = identity_exc.NotFound(None) + self.services_mock.find.return_value = fakes.FakeResource( None, copy.deepcopy(identity_fakes.SERVICE), loaded=True, @@ -460,7 +465,8 @@ class TestServiceShow(TestService): def setUp(self): super(TestServiceShow, self).setUp() - self.services_mock.get.return_value = fakes.FakeResource( + self.services_mock.get.side_effect = identity_exc.NotFound(None) + self.services_mock.find.return_value = fakes.FakeResource( None, copy.deepcopy(identity_fakes.SERVICE), loaded=True, @@ -484,8 +490,8 @@ class TestServiceShow(TestService): columns, data = self.cmd.take_action(parsed_args) # ServiceManager.get(id) - self.services_mock.get.assert_called_with( - identity_fakes.service_name, + self.services_mock.find.assert_called_with( + name=identity_fakes.service_name ) collist = ('description', 'enabled', 'id', 'name', 'type') @@ -498,3 +504,21 @@ class TestServiceShow(TestService): identity_fakes.service_type, ) self.assertEqual(datalist, data) + + def test_service_show_nounique(self): + self.services_mock.find.side_effect = identity_exc.NoUniqueMatch(None) + arglist = [ + 'nounique_service', + ] + verifylist = [ + ('service', 'nounique_service'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + try: + self.cmd.take_action(parsed_args) + 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.", str(e)) diff --git a/releasenotes/notes/bug-1597296-9735f33eacf5552e.yaml b/releasenotes/notes/bug-1597296-9735f33eacf5552e.yaml new file mode 100644 index 000000000..677f8d47c --- /dev/null +++ b/releasenotes/notes/bug-1597296-9735f33eacf5552e.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fixed service name lookup in Identity commands to properly handle + multiple matches. + [Bug `1597296 `_]