Fix error for find_service() in identity
if there are more than one services be found with one name, a NoUniqueMatch exception should be raised but we can see a NotFound Exception raised instead. It is because in "find_service()", we use "find_resource()" first, if "find_resource()" return a exception, we just think it is a NotFound Exception and continue to find by type but ignore a NoUniqueMatch exception of "find_resource()". This patch refactor the "find_service()" method to solve this problem. Change-Id: Id4619092c57f276ae0698c89df0d5503b7423a4e Co-Authored-By: Huanxuan Ao <huanxuan.ao@easystack.cn> Closes-Bug:#1597296
This commit is contained in:
parent
60639d76a7
commit
c45b1d7b23
@ -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):
|
||||
|
@ -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
|
||||
|
@ -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))
|
||||
|
@ -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))
|
||||
|
5
releasenotes/notes/bug-1597296-9735f33eacf5552e.yaml
Normal file
5
releasenotes/notes/bug-1597296-9735f33eacf5552e.yaml
Normal file
@ -0,0 +1,5 @@
|
||||
---
|
||||
fixes:
|
||||
- Fixed service name lookup in Identity commands to properly handle
|
||||
multiple matches.
|
||||
[Bug `1597296 <https://bugs.launchpad.net/python-openstackclient/+bug/1597296>`_]
|
Loading…
Reference in New Issue
Block a user