diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c5421460ef..17421827a5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,15 @@ in development Changed ~~~~~~~ +* Modified action delete api. Action delete api removes related action/workflow files on disk + along with de-registering them from database. Prompts on CLI for user permission before + removing disk files. + + ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and + will delete related files on disk without prompting for user permission. #5304 + + Contributed by @mahesh-orch. + * Removed --python3 deprecated flag from st2client. #5305 Contributed by Amanda McGuinness (@amanda11 Ammeon Solutions) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index c78667076d..1e8bf7c7b5 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -42,6 +42,7 @@ from st2common.content.utils import get_pack_base_path from st2common.content.utils import get_pack_resource_file_abs_path from st2common.content.utils import get_relative_path_to_pack_file +from st2common.services.packs import delete_action_files_from_pack from st2common.transport.reactor import TriggerDispatcher from st2common.util.system_info import get_host_info import st2common.validators.api.action as action_validator @@ -236,6 +237,10 @@ def delete(self, ref_or_id, requester_user): action_db, ) + pack_name = action_db["pack"] + entry_point = action_db["entry_point"] + metadata_file = action_db["metadata_file"] + try: Action.delete(action_db) except Exception as e: @@ -247,6 +252,20 @@ def delete(self, ref_or_id, requester_user): ) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) return + try: + delete_action_files_from_pack( + pack_name=pack_name, + entry_point=entry_point, + metadata_file=metadata_file, + ) + except Exception as e: + LOG.error( + "Exception encountered during deleting resource files from disk." + "Exception was %s", + e, + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + return extra = {"action_db": action_db} LOG.audit("Action deleted. Action.id=%s" % (action_db.id), extra=extra) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 17a0627871..29963fd2de 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -278,7 +278,43 @@ class ActionDisableCommand(resource.ContentPackResourceDisableCommand): class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): - pass + def __init__(self, resource, *args, **kwargs): + super(ActionDeleteCommand, self).__init__(resource, *args, **kwargs) + + self.parser.add_argument( + "-f", + "--force", + action="store_true", + dest="force", + help="Auto yes flag to delete 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) + instance = self.get_resource(resource_id, **kwargs) + msg = ( + 'Resource with id "%s" has been successfully deleted from database and disk.' + % (resource_id) + ) + user_input = "" + if not args.force: + user_input = input( + "The resource files on disk will be deleted. Do you want to continue? (y/n): " + ) + if args.force or user_input.lower() == "y" or user_input.lower() == "yes": + self.manager.delete(instance, **kwargs) + print(msg) + else: + print("Action is not deleted.") + + 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 ActionRunCommandMixin(object): diff --git a/st2common/st2common/exceptions/content.py b/st2common/st2common/exceptions/content.py index 3b3449c45b..2336e92d89 100644 --- a/st2common/st2common/exceptions/content.py +++ b/st2common/st2common/exceptions/content.py @@ -23,3 +23,7 @@ class UnsupportedMetaException(StackStormBaseException): class ParseException(ValueError): pass + + +class ResourceDiskFilesRemovalError(StackStormBaseException): + pass diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 1d8bd69399..74e97e6424 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -17,12 +17,15 @@ import itertools +import os import requests import six from six.moves import range from oslo_config import cfg from st2common import log as logging +from st2common.content.utils import get_pack_base_path +from st2common.exceptions.content import ResourceDiskFilesRemovalError from st2common.persistence.pack import Pack from st2common.util.misc import lowercase_value from st2common.util.jsonify import json_encode @@ -32,6 +35,7 @@ "fetch_pack_index", "get_pack_from_index", "search_pack_index", + "delete_action_files_from_pack", ] EXCLUDE_FIELDS = ["repo_url", "email"] @@ -215,3 +219,74 @@ def search_pack_index( break return list(itertools.chain.from_iterable(matches)) + + +def delete_action_files_from_pack(pack_name, entry_point, metadata_file): + """ + Prepares the path for entry_point file and metadata file of action and + deletes them from disk. + """ + + pack_base_path = get_pack_base_path(pack_name=pack_name) + action_entrypoint_file_path = os.path.join(pack_base_path, "actions", entry_point) + action_metadata_file_path = os.path.join(pack_base_path, metadata_file) + + if os.path.isfile(action_entrypoint_file_path): + try: + os.remove(action_entrypoint_file_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" file', + action_entrypoint_file_path, + ) + msg = 'No permission to delete "%s" file from disk' % ( + action_entrypoint_file_path + ) + raise PermissionError(msg) + except Exception as e: + LOG.error( + 'Unable to delete "%s" file. Exception was "%s"', + action_entrypoint_file_path, + e, + ) + msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (action_entrypoint_file_path) + ) + raise ResourceDiskFilesRemovalError(msg) + else: + LOG.warning( + 'The action entry point file "%s" does not exists on disk.', + action_entrypoint_file_path, + ) + + if os.path.isfile(action_metadata_file_path): + try: + os.remove(action_metadata_file_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" file', + action_metadata_file_path, + ) + msg = 'No permission to delete "%s" file from disk' % ( + action_metadata_file_path + ) + raise PermissionError(msg) + except Exception as e: + LOG.error( + 'Could not delete "%s" file. Exception was "%s"', + action_metadata_file_path, + e, + ) + msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (action_metadata_file_path) + ) + raise ResourceDiskFilesRemovalError(msg) + else: + LOG.warning( + 'The action metadata file "%s" does not exists on disk.', + action_metadata_file_path, + ) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py new file mode 100644 index 0000000000..04e68dee51 --- /dev/null +++ b/st2common/tests/unit/services/test_packs.py @@ -0,0 +1,243 @@ +# -*- coding: utf-8 -*- + +# Copyright 2020 The StackStorm Authors. +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +import os +import mock +import unittest2 + +import st2tests + +from st2common.services.packs import delete_action_files_from_pack + +TEST_PACK = "dummy_pack_1" +TEST_PACK_PATH = os.path.join( + st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_PACK +) + + +class DeleteActionFilesTest(unittest2.TestCase): + def test_delete_action_files_from_pack(self): + """ + Test that the action files present in the pack and removed + on the call of delete_action_files_from_pack function. + """ + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating entry_point file in dummy pack + with open(entry_point, "w") as f: + f.write("# entry point file to be removed") + + # creating metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") + + # asserting both entry_point and metadata files exists + self.assertTrue(os.path.exists(entry_point)) + self.assertTrue(os.path.exists(metadata_file)) + + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + # asserting both entry_point and metadata files removed and they doesn't exist + self.assertFalse(os.path.exists(entry_point)) + self.assertFalse(os.path.exists(metadata_file)) + + def test_entry_point_file_does_not_exists(self): + """ + Tests that entry_point file doesn't exists at the path and if action delete + api calls delete_action_files_from_pack function, it doesn't affect. + """ + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating only metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") + + # asserting entry_point file doesn't exist + self.assertFalse(os.path.exists(entry_point)) + + # asserting metadata files exists + self.assertTrue(os.path.exists(metadata_file)) + + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + # asserting both entry_point and metadata files don't exist + self.assertFalse(os.path.exists(entry_point)) + self.assertFalse(os.path.exists(metadata_file)) + + def test_metadata_file_does_not_exists(self): + """ + Tests that metadata file doesn't exists at the path and if action delete + api calls delete_action_files_from_pack function, it doesn't affect. + """ + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating only entry_point file in dummy pack + with open(entry_point, "w") as f: + f.write("# entry point file to be removed") + + # asserting metadata file doesn't exist + self.assertFalse(os.path.exists(metadata_file)) + + # asserting entry_point file exists + self.assertTrue(os.path.exists(entry_point)) + + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + # asserting both entry_point and metadata files don't exist + self.assertFalse(os.path.exists(entry_point)) + self.assertFalse(os.path.exists(metadata_file)) + + +class DeleteActionEntryPointFilesErrorTest(unittest2.TestCase): + """ + Testing that exceptions are thrown by delete_action_files_from_pack function + for entry point file. Here only entry point file is created and metadata + file doesn't exist. + """ + + def setUp(self): + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + + # creating entry_point file in dummy pack + with open(entry_point, "w") as f: + f.write("# entry point file to be removed") + + def tearDown(self): + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + + # removing entry_point file from dummy pack + os.remove(entry_point) + + @mock.patch.object(os, "remove") + def test_permission_error_to_remove_resource_entry_point_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = PermissionError("No permission to delete file from disk") + + # asserting entry_point file exists + self.assertTrue(os.path.exists(entry_point)) + + # asserting metadata file doesn't exist + self.assertFalse(os.path.exists(metadata_file)) + + expected_msg = 'No permission to delete "%s" file from disk' % (entry_point) + + # asserting PermissionError with message on call of delete_action_files_from_pack + # to delete entry_point file + with self.assertRaisesRegexp(PermissionError, expected_msg): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + @mock.patch.object(os, "remove") + def test_exception_to_remove_resource_entry_point_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = Exception("Another exception occured") + + # asserting entry_point file exists + self.assertTrue(os.path.exists(entry_point)) + + # asserting metadata file doesn't exist + self.assertFalse(os.path.exists(metadata_file)) + + expected_msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (entry_point) + ) + + # asserting exception with message on call of delete_action_files_from_pack + # to delete entry_point file + with self.assertRaisesRegexp(Exception, expected_msg): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + +class DeleteActionMetadataFilesErrorTest(unittest2.TestCase): + """ + Testing that exceptions are thrown by delete_action_files_from_pack function for + metadata file. Here only metadata file is created and metadata file doesn't exist. + """ + + def setUp(self): + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") + + def tearDown(self): + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # removing metadata file from dummy pack + os.remove(metadata_file) + + @mock.patch.object(os, "remove") + def test_permission_error_to_remove_resource_metadata_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = PermissionError("No permission to delete file from disk") + + # asserting metadata file exists + self.assertTrue(os.path.exists(metadata_file)) + + # asserting entry_point file doesn't exist + self.assertFalse(os.path.exists(entry_point)) + + expected_msg = 'No permission to delete "%s" file from disk' % (metadata_file) + + # asserting PermissionError with message on call of delete_action_files_from_pack + # to delete metadata file + with self.assertRaisesRegexp(PermissionError, expected_msg): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + @mock.patch.object(os, "remove") + def test_exception_to_remove_resource_metadata_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = Exception("Another exception occured") + + # asserting metadata file exists + self.assertTrue(os.path.exists(metadata_file)) + + # asserting entry_point file doesn't exist + self.assertFalse(os.path.exists(entry_point)) + + expected_msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (metadata_file) + ) + + # asserting exception with message on call of delete_action_files_from_pack + # to delete metadata file + with self.assertRaisesRegexp(Exception, expected_msg): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file)