diff --git a/openstackclient/identity/common.py b/openstackclient/identity/common.py index 745ffb2b03..a9d8afa6f5 100644 --- a/openstackclient/identity/common.py +++ b/openstackclient/identity/common.py @@ -92,6 +92,18 @@ def get_resource(manager, name_type_or_id): raise exceptions.CommandError(msg % name_type_or_id) +def get_resource_by_id(manager, resource_id): + """Get resource by ID + + Raises CommandError if the resource is not found + """ + try: + return manager.get(resource_id) + except identity_exc.NotFound: + msg = _("Resource with id {} not found") + raise exceptions.CommandError(msg.format(resource_id)) + + def _get_token_resource(client, resource, parsed_name, parsed_domain=None): """Peek into the user's auth token to get resource IDs diff --git a/openstackclient/identity/v3/access_rule.py b/openstackclient/identity/v3/access_rule.py index 5eff39c775..b9a66e9a5d 100644 --- a/openstackclient/identity/v3/access_rule.py +++ b/openstackclient/identity/v3/access_rule.py @@ -37,7 +37,7 @@ class DeleteAccessRule(command.Command): 'access_rule', metavar='', nargs="+", - help=_('Access rule(s) to delete (name or ID)'), + help=_('Access rule ID(s) to delete'), ) return parser @@ -47,7 +47,7 @@ class DeleteAccessRule(command.Command): errors = 0 for ac in parsed_args.access_rule: try: - access_rule = utils.find_resource( + access_rule = common.get_resource_by_id( identity_client.access_rules, ac ) identity_client.access_rules.delete(access_rule.id) @@ -114,13 +114,13 @@ class ShowAccessRule(command.ShowOne): parser.add_argument( 'access_rule', metavar='', - help=_('Access rule to display (name or ID)'), + help=_('Access rule ID to display'), ) return parser def take_action(self, parsed_args): identity_client = self.app.client_manager.identity - access_rule = utils.find_resource( + access_rule = common.get_resource_by_id( identity_client.access_rules, parsed_args.access_rule ) diff --git a/openstackclient/tests/unit/identity/v3/test_access_rule.py b/openstackclient/tests/unit/identity/v3/test_access_rule.py index 3727dcfecf..b62f283f30 100644 --- a/openstackclient/tests/unit/identity/v3/test_access_rule.py +++ b/openstackclient/tests/unit/identity/v3/test_access_rule.py @@ -14,10 +14,9 @@ # import copy -from unittest import mock +from keystoneclient import exceptions as identity_exc from osc_lib import exceptions -from osc_lib import utils from openstackclient.identity.v3 import access_rule from openstackclient.tests.unit import fakes @@ -64,11 +63,12 @@ class TestAccessRuleDelete(TestAccessRule): ) self.assertIsNone(result) - @mock.patch.object(utils, 'find_resource') - def test_delete_multi_access_rules_with_exception(self, find_mock): - find_mock.side_effect = [ - self.access_rules_mock.get.return_value, - exceptions.CommandError, + def test_delete_multi_access_rules_with_exception(self): + # mock returns for common.get_resource_by_id + mock_get = self.access_rules_mock.get + mock_get.side_effect = [ + mock_get.return_value, + identity_exc.NotFound, ] arglist = [ identity_fakes.access_rule_id, @@ -87,14 +87,10 @@ class TestAccessRuleDelete(TestAccessRule): '1 of 2 access rules failed to' ' delete.', str(e) ) - find_mock.assert_any_call( - self.access_rules_mock, identity_fakes.access_rule_id - ) - find_mock.assert_any_call( - self.access_rules_mock, 'nonexistent_access_rule' - ) + mock_get.assert_any_call(identity_fakes.access_rule_id) + mock_get.assert_any_call('nonexistent_access_rule') - self.assertEqual(2, find_mock.call_count) + self.assertEqual(2, mock_get.call_count) self.access_rules_mock.delete.assert_called_once_with( identity_fakes.access_rule_id ) diff --git a/releasenotes/notes/fix-story-2010775-953dbdf03b2b6746.yaml b/releasenotes/notes/fix-story-2010775-953dbdf03b2b6746.yaml new file mode 100644 index 0000000000..e4c98b7443 --- /dev/null +++ b/releasenotes/notes/fix-story-2010775-953dbdf03b2b6746.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixed a bug in "access rule" subcommands where the client logic incorrectly + assumed that access rules have a "name" property which resulted in + unpredictable behaviors. e.g. "access rule delete {non-existent-id}" now + results in a not-found error instead of sometimes deleting an unrelated + rule.