From 72b4d47747cbab8a6460b2f92129470355279d43 Mon Sep 17 00:00:00 2001 From: Yusuf Hussien Date: Wed, 7 Oct 2026 21:41:57 +0300 Subject: [PATCH] feat(st2client): allow delete commands to accept multiple ids All st2 delete commands now accept multiple positional ids, e.g. st2 action-alias delete id1 id2 id3. Existing single id behavior is unchanged; when deleting multiple ids, resources which are not found are reported and the command exits with a non-zero return code after attempting to delete the remaining resources. Fixes #4729 Signed-off-by: Yusuf Hussien --- CHANGELOG.rst | 1 + st2client/st2client/commands/action.py | 13 +-- st2client/st2client/commands/keyvalue.py | 4 +- st2client/st2client/commands/resource.py | 62 +++++++++++-- st2client/tests/unit/test_commands.py | 110 +++++++++++++++++++++++ 5 files changed, 166 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 107cf6a43f..22f468a244 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -37,6 +37,7 @@ Changed Added ~~~~~ +* ``st2 delete`` now accepts multiple ids, e.g. ``st2 action-alias delete id1 id2 id3``. Resources which are not found are reported and the command exits with a non-zero return code. #4729 * added raw_string type to allow template strings to pass through variable processing (by @guzzijones12@gmail.com) #6351 3.9.0 - October 10, 2025 diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index bfe9dccd74..2f5fc195e2 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -291,21 +291,10 @@ def __init__(self, resource, *args, **kwargs): help="Remove action files from disk.", ) - @add_auth_token_to_kwargs_from_cli - def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) + def delete_resource(self, args, resource_id, **kwargs): instance = self.get_resource(resource_id, **kwargs) remove_files = args.remove_files self.manager.delete_action(instance, remove_files, **kwargs) - print('Resource with id "%s" has been successfully deleted.' % (resource_id)) - - def run_and_print(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name) - - try: - self.run(args, **kwargs) - except ResourceNotFoundError: - self.print_not_found(resource_id) class ActionCloneCommand(resource.ContentPackResourceCloneCommand): diff --git a/st2client/st2client/commands/keyvalue.py b/st2client/st2client/commands/keyvalue.py index b825185dbe..32c9e8c67f 100644 --- a/st2client/st2client/commands/keyvalue.py +++ b/st2client/st2client/commands/keyvalue.py @@ -336,9 +336,7 @@ def __init__(self, resource, *args, **kwargs): help="User for user scoped items (admin only).", ) - @resource.add_auth_token_to_kwargs_from_cli - def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) + def delete_resource(self, args, resource_id, **kwargs): scope = getattr(args, "scope", DEFAULT_CUD_SCOPE) kwargs["params"] = {} kwargs["params"]["scope"] = scope diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index 982c1b7875..1d8d882cb5 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -731,26 +731,70 @@ def __init__(self, resource, *args, **kwargs): resource=resource, argument=self.pk_argument_name ) - self.parser.add_argument(argument, metavar=metavar, help=help) + self.parser.add_argument(argument, metavar=metavar, nargs="+", help=help) - @add_auth_token_to_kwargs_from_cli - def run(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) + def delete_resource(self, args, resource_id, **kwargs): + """ + Delete a single resource. + + Subclasses can override this method to customize how an individual resource is + deleted. + """ instance = self.get_resource(resource_id, **kwargs) + + if not instance: + raise ResourceNotFoundError( + 'Resource with id "%s" doesn\'t exist.' % (resource_id) + ) + self.manager.delete(instance, **kwargs) + @add_auth_token_to_kwargs_from_cli + def run(self, args, **kwargs): + resource_ids = getattr(args, self.pk_argument_name, None) or [] + more_than_one_resource = len(resource_ids) > 1 + + deleted_ids = [] + not_found_ids = [] + + for resource_id in resource_ids: + try: + self.delete_resource(args, resource_id, **kwargs) + except ResourceNotFoundError: + if not more_than_one_resource: + # For backward compatibility reasons and to comply with common "delete one" + # behavior, we only fail if a single resource is requested + raise + + self.print_not_found(resource_id) + not_found_ids.append(resource_id) + continue + + deleted_ids.append(resource_id) + + return deleted_ids, not_found_ids + def run_and_print(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) + resource_ids = getattr(args, self.pk_argument_name, None) or [] try: - self.run(args, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted.' % (resource_id) - ) + deleted_ids, not_found_ids = self.run(args, **kwargs) except ResourceNotFoundError: + resource_id = resource_ids[0] self.print_not_found(resource_id) raise OperationFailureException("Resource %s not found." % resource_id) + for resource_id in deleted_ids: + print( + 'Resource with id "%s" has been successfully deleted.' % (resource_id) + ) + + if not_found_ids: + raise OperationFailureException( + "Failed to delete %s resource(s): %s." + % (len(not_found_ids), ", ".join(not_found_ids)) + ) + class ContentPackResourceDeleteCommand(ResourceDeleteCommand): """ diff --git a/st2client/tests/unit/test_commands.py b/st2client/tests/unit/test_commands.py index e8fa417cb9..cc1b7669d1 100644 --- a/st2client/tests/unit/test_commands.py +++ b/st2client/tests/unit/test_commands.py @@ -33,6 +33,7 @@ from st2client.utils import httpclient from st2client.commands import resource from st2client.commands.resource import ResourceViewCommand +from st2client.exceptions.operations import OperationFailureException __all__ = ["TestResourceCommand", "ResourceViewCommandTestCase"] @@ -103,6 +104,54 @@ def test_all_resources_get_multi(self): self.assertTrue('%s "id3" is not found.' % (display_name) in stdout) self._reset_output_streams() + @mock.patch.object( + httpclient.HTTPClient, + "get", + mock.MagicMock( + return_value=base.FakeResponse(json.dumps({}), 404, "NOT FOUND") + ), + ) + def test_all_resources_delete_multi(self): + # Resources which support the delete command. + resources = [ + ("action", models.Action), + ("action-alias", models.ActionAlias), + ("rule", models.Rule), + ("key", models.KeyValuePair), + ("trigger", models.TriggerType), + ("apikey", models.ApiKey), + ("policy", models.Policy), + ] + + # 1. st2 delete ... notation should attempt to delete every + # provided id, report every id which was not found and return non-zero. + for command_name, resource_ in resources: + display_name = resource_.get_display_name() + + self._reset_output_streams() + return_code = self.shell.run([command_name, "delete", "id1", "id2", "id3"]) + self.assertEqual(return_code, 2) + + stdout = self.stdout.getvalue() + + for resource_id in ["id1", "id2", "id3"]: + self.assertTrue( + '%s "%s" is not found.' % (display_name, resource_id) in stdout + ) + self._reset_output_streams() + + # 2. Single id delete which is not found should still return non-zero. + for command_name, resource_ in resources: + display_name = resource_.get_display_name() + + self._reset_output_streams() + return_code = self.shell.run([command_name, "delete", "id3"]) + self.assertEqual(return_code, 2) + + stdout = self.stdout.getvalue() + self.assertTrue('%s "id3" is not found.' % (display_name) in stdout) + self._reset_output_streams() + class TestResourceCommand(unittest.TestCase): def __init__(self, *args, **kwargs): @@ -387,6 +436,67 @@ def test_command_delete_failed(self): args = self.parser.parse_args(["fakeresource", "delete", "cba"]) self.assertRaises(Exception, self.branch.commands["delete"].run, args) + @mock.patch.object( + models.ResourceManager, + "get_by_name", + mock.MagicMock(return_value=base.FakeResource(**base.RESOURCES[0])), + ) + @mock.patch.object( + httpclient.HTTPClient, + "delete", + mock.MagicMock(return_value=base.FakeResponse("", 204, "NO CONTENT")), + ) + def test_command_delete_multiple(self): + args = self.parser.parse_args(["fakeresource", "delete", "abc", "def"]) + self.assertEqual(args.func, self.branch.commands["delete"].run_and_print) + deleted_ids, not_found_ids = self.branch.commands["delete"].run(args) + self.assertEqual(deleted_ids, ["abc", "def"]) + self.assertEqual(not_found_ids, []) + self.assertEqual(httpclient.HTTPClient.delete.call_count, 2) + + @mock.patch.object( + models.ResourceManager, + "get_by_name", + mock.MagicMock(side_effect=[base.FakeResource(**base.RESOURCES[0]), None]), + ) + @mock.patch.object( + models.ResourceManager, "get_by_id", mock.MagicMock(return_value=None) + ) + @mock.patch.object( + httpclient.HTTPClient, + "delete", + mock.MagicMock(return_value=base.FakeResponse("", 204, "NO CONTENT")), + ) + def test_command_delete_multiple_partial_failure(self): + # Deleting multiple resources should continue with the remaining resources when one + # of the resources is not found. + args = self.parser.parse_args(["fakeresource", "delete", "abc", "def"]) + deleted_ids, not_found_ids = self.branch.commands["delete"].run(args) + self.assertEqual(deleted_ids, ["abc"]) + self.assertEqual(not_found_ids, ["def"]) + self.assertEqual(httpclient.HTTPClient.delete.call_count, 1) + + @mock.patch.object( + models.ResourceManager, "get_by_name", mock.MagicMock(return_value=None) + ) + @mock.patch.object( + models.ResourceManager, "get_by_id", mock.MagicMock(return_value=None) + ) + def test_command_delete_multiple_not_found(self): + args = self.parser.parse_args(["fakeresource", "delete", "abc", "def"]) + + deleted_ids, not_found_ids = self.branch.commands["delete"].run(args) + self.assertEqual(deleted_ids, []) + self.assertEqual(not_found_ids, ["abc", "def"]) + + # When some of the resources could not be deleted, the command should fail with a + # non-zero exit code. + self.assertRaises( + OperationFailureException, + self.branch.commands["delete"].run_and_print, + args, + ) + @mock.patch.object( models.ResourceManager, "get_by_id",