From 5fb78d283f420c27f3e0b129f8b167e8f27dda65 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Fri, 27 Aug 2021 16:49:36 +0000 Subject: [PATCH 01/40] st2 API and CLI command added for actions/workflows clone operation --- st2api/st2api/controllers/v1/actions.py | 85 +++++++++ st2client/st2client/commands/resource.py | 95 +++++++++++ st2client/st2client/models/core.py | 25 +++ st2common/st2common/openapi.yaml.j2 | 49 ++++++ st2common/st2common/services/packs.py | 93 ++++++++++ st2common/st2common/validators/api/misc.py | 6 + st2common/tests/unit/services/test_packs.py | 161 ++++++++++++++++++ .../actions/workflows/__init__.py | 0 8 files changed, 514 insertions(+) create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 1e8bf7c7b5..bea7a354fc 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -28,6 +28,7 @@ from st2api.controllers import resource from st2api.controllers.v1.action_views import ActionViewsController from st2common import log as logging +from st2common.bootstrap.actionsregistrar import register_actions from st2common.constants.triggers import ACTION_FILE_WRITTEN_TRIGGER from st2common.exceptions.action import InvalidActionParameterException from st2common.exceptions.apivalidation import ValueValidationException @@ -39,10 +40,12 @@ from st2common.router import abort from st2common.router import Response from st2common.validators.api.misc import validate_not_part_of_system_pack +from st2common.validators.api.misc import validate_not_part_of_system_pack_by_name 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.services.packs import clone_action from st2common.transport.reactor import TriggerDispatcher from st2common.util.system_info import get_host_info import st2common.validators.api.action as action_validator @@ -271,6 +274,88 @@ def delete(self, ref_or_id, requester_user): LOG.audit("Action deleted. Action.id=%s" % (action_db.id), extra=extra) return Response(status=http_client.NO_CONTENT) + def clone( + self, + source_pack, + source_action, + dest_pack, + dest_action, + requester_user, + overwrite=False, + ): + """ + Clone an action. + Handles requests: + POST /actions/{sorce_pack}/{source_action}/{dest_pack}/{dest_action} + """ + + source_ref = "%s.%s" % (source_pack, source_action) + source_action_db = self._get_by_ref(resource_ref=source_ref) + msg = "The requested source for cloning operation doesn't exists" + if not source_action_db: + abort(http_client.BAD_REQUEST, six.text_type(msg)) + + permission_type = PermissionType.ACTION_CREATE + rbac_utils = get_rbac_backend().get_utils_class() + rbac_utils.assert_user_has_resource_db_permission( + user_db=requester_user, + resource_db=source_action_db, + permission_type=permission_type, + ) + + LOG.debug( + "POST /actions/ lookup with ref=%s found object: %s", + source_ref, + source_action_db, + ) + + source_pack = source_action_db["pack"] + source_entry_point = source_action_db["entry_point"] + source_metadata_file = source_action_db["metadata_file"] + source_pack_base_path = get_pack_base_path(pack_name=source_pack) + dest_pack_base_path = get_pack_base_path(pack_name=dest_pack) + if not os.path.isdir(dest_pack_base_path): + raise ValueError('Destination pack "%s" doesn\'t exist' % (dest_pack)) + + dest_ref = "%s.%s" % (dest_pack, dest_action) + dest_action_db = self._get_by_ref(resource_ref=dest_ref) + + try: + validate_not_part_of_system_pack_by_name(dest_pack) + except ValueValidationException as e: + abort(http_client.BAD_REQUEST, six.text_type(e)) + + if dest_action_db: + if overwrite: + pass + else: + msg = "The requested destination action already exists" + abort(http_client.BAD_REQUEST, six.text_type(msg)) + try: + clone_action( + source_pack_base_path=source_pack_base_path, + source_metadata_file=source_metadata_file, + source_entry_point=source_entry_point, + dest_pack_base_path=dest_pack_base_path, + dest_pack=dest_pack, + dest_action=dest_action, + ) + register_actions(pack_dir=dest_pack_base_path) + except Exception as e: + LOG.error( + "Exception encountered during cloning resource." "Exception was %s", + e, + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + return + + dest_action_db = self._get_by_ref(resource_ref=dest_ref) + extra = {"cloned_acion_db": dest_action_db} + LOG.audit("Action cloned. Action.id=%s" % (dest_action_db.id), extra=extra) + cloned_action_api = ActionAPI.from_model(dest_action_db) + + return Response(json=cloned_action_api, status=http_client.CREATED) + def _handle_data_files(self, pack_ref, data_files): """ Method for handling action data files. diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index d66e990a93..ee7796f85c 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -102,6 +102,7 @@ def __init__( "delete": ResourceDeleteCommand, "enable": ResourceEnableCommand, "disable": ResourceDisableCommand, + "clone": ResourceCloneCommand, } for cmd, cmd_class in cmd_map.items(): if cmd not in commands: @@ -116,6 +117,7 @@ def __init__( self.commands["create"] = commands["create"](*args) self.commands["update"] = commands["update"](*args) self.commands["delete"] = commands["delete"](*args) + self.commands["clone"] = commands["clone"](*args) if has_disable: self.commands["enable"] = commands["enable"](*args) @@ -758,6 +760,99 @@ class ContentPackResourceDeleteCommand(ResourceDeleteCommand): pk_argument_name = "ref_or_id" +class ResourceCloneCommand(ResourceCommand): + pk_source_pack = "source_pack" + pk_source_action = "source_action" + pk_dest_pack = "destination_pack" + pk_dest_action = "destination_action" + + def __init__(self, resource, *args, **kwargs): + super(ResourceCloneCommand, self).__init__( + resource, + "clone", + "Clone a new %s." % resource.get_display_name().lower(), + *args, + **kwargs, + ) + + pk_list = [ + self.pk_source_pack, + self.pk_source_action, + self.pk_dest_pack, + self.pk_dest_action, + ] + + for var in pk_list: + metavar = self._get_metavar_for_argument(argument=var) + helparg = self._get_help_for_argument(resource=resource, argument=var) + self.parser.add_argument(var, metavar=metavar, help=helparg) + + self.parser.add_argument( + "-f", + "--force", + action="store_true", + dest="force", + help="Auto yes flag to overwrite action files on disk if destination exists.", + ) + + @add_auth_token_to_kwargs_from_cli + def run(self, args, **kwargs): + source_pack = getattr(args, self.pk_source_pack, None) + source_action = getattr(args, self.pk_source_action, None) + dest_pack = getattr(args, self.pk_dest_pack, None) + dest_action = getattr(args, self.pk_dest_action, None) + source_ref = "%s.%s" % (source_pack, source_action) + instance = self.get_resource(source_ref, **kwargs) + msg = 'Action with name "%s" has been successfully cloned in "%s" pack.' % ( + dest_action, + dest_pack, + ) + + dest_ref = "%s.%s" % (dest_pack, dest_action) + try: + dest_instance = self.get_resource(dest_ref, **kwargs) + except ResourceNotFoundError: + dest_instance = None + + if dest_instance: + user_input = "" + if not args.force: + user_input = input( + "The destination action already exists. Do you want to overwrite? (y/n): " + ) + if args.force or user_input.lower() == "y" or user_input.lower() == "yes": + self.manager.clone( + instance, + source_pack, + source_action, + dest_pack, + dest_action, + overwrite=True, + **kwargs, + ) + print(msg) + else: + print("Action is not cloned.") + else: + self.manager.clone( + instance, + source_pack, + source_action, + dest_pack, + dest_action, + overwrite=False, + **kwargs, + ) + print(msg) + + def run_and_print(self, args, **kwargs): + resource_id = getattr(args, self.pk_source_pack) + try: + self.run(args, **kwargs) + except ResourceNotFoundError: + self.print_not_found(resource_id) + + def load_meta_file(file_path): if not os.path.isfile(file_path): raise Exception('File "%s" does not exist.' % file_path) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 7f94cc94f8..33d8009f89 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -376,6 +376,31 @@ def delete(self, instance, **kwargs): return True + @add_auth_token_to_kwargs_from_env + def clone( + self, + instance, + source_pack, + source_action, + dest_pack, + dest_action, + overwrite, + **kwargs, + ): + url = "/%s/%s/%s/%s/%s?overwrite=%s" % ( + self.resource.get_url_path_name(), + source_pack, + source_action, + dest_pack, + dest_action, + overwrite, + ) + response = self.client.post(url, instance.serialize(), **kwargs) + if response.status_code != http_client.OK: + self.handle_error(response) + instance = self.resource.deserialize(parse_api_response(response)) + return instance + @add_auth_token_to_kwargs_from_env def delete_by_id(self, instance_id, **kwargs): url = "/%s/%s" % (self.resource.get_url_path_name(), instance_id) diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index b41602587b..98c07cd219 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -421,6 +421,55 @@ paths: description: Unexpected error schema: $ref: '#/definitions/Error' + /api/v1/actions/{source_pack}/{source_action}/{dest_pack}/{dest_action}: + post: + operationId: st2api.controllers.v1.actions:actions_controller.clone + description: | + Clone one action. + parameters: + - name: source_pack + in: path + description: Source pack name + type: string + required: true + - name: source_action + in: path + description: Source action name + type: string + required: true + - name: dest_pack + in: path + description: Destination pack name + type: string + required: true + - name: dest_action + in: path + description: Destination action name + type: string + required: true + - name: overwrite + in: query + description: Force clone action if destination already exists + type: boolean + default: false + x-parameters: + - name: user + in: context + x-as: requester_user + description: User performing the operation. + responses: + '201': + description: Single action being cloned + schema: + $ref: '#/definitions/Action' + examples: + application/json: + ref: 'core.local' + # and stuff + default: + description: Unexpected error + schema: + $ref: '#/definitions/Error' /api/v1/actions/views/parameters/{action_id}: get: operationId: st2api.controllers.v1.action_views:parameters_view_controller.get_one diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 74e97e6424..1aad373028 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -22,6 +22,7 @@ import six from six.moves import range from oslo_config import cfg +import yaml from st2common import log as logging from st2common.content.utils import get_pack_base_path @@ -290,3 +291,95 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): 'The action metadata file "%s" does not exists on disk.', action_metadata_file_path, ) + + +def clone_content_to_destination_file(source_file, destination_file): + try: + with open(source_file, "r") as sf: + with open(destination_file, "w") as df: + for line in sf: + df.write(line) + except PermissionError: + LOG.error( + 'No write permission for "%s" file', + destination_file, + ) + msg = 'No permission to write in "%s" file' % (destination_file) + raise PermissionError(msg) + except FileNotFoundError: + LOG.error('No "workflows" directory at path: "%s"', destination_file) + msg = 'Please make sure "workflows" directory present at path: "%s"' % ( + destination_file + ) + raise FileNotFoundError(msg) + except Exception as e: + LOG.error( + 'Could not copy to "%s" file. Exception was "%s"', + destination_file, + e, + ) + msg = ( + 'Could not copy to "%s" file, please check the logs ' + "or ask your StackStorm administrator to check and clone " + "the actions files manually" % (destination_file) + ) + raise Exception(msg) + + +def clone_action( + source_pack_base_path, + source_metadata_file, + source_entry_point, + dest_pack_base_path, + dest_pack, + dest_action, +): + """ + Prepares the path for entry point and metadata files for source and destination. + Clones the content from source action files to destination action files. + """ + + source_metadata_file_path = os.path.join( + source_pack_base_path, source_metadata_file + ) + dest_metadata_file_name = "%s.yaml" % (dest_action) + dest_metadata_file_path = os.path.join( + dest_pack_base_path, "actions", dest_metadata_file_name + ) + + if source_entry_point: + source_entry_point_file_path = os.path.join( + source_pack_base_path, "actions", source_entry_point + ) + if str(source_entry_point).endswith("py"): + dest_entry_point_file_name = "%s.py" % (dest_action) + elif str(source_entry_point).startswith("workflows"): + dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) + else: + dest_entry_point_file_name = dest_action + + dest_entrypoint_file_path = os.path.join( + dest_pack_base_path, "actions", dest_entry_point_file_name + ) + clone_content_to_destination_file( + source_file=source_entry_point_file_path, + destination_file=dest_entrypoint_file_path, + ) + + else: + dest_entry_point_file_name = "" + + clone_content_to_destination_file( + source_file=source_metadata_file_path, destination_file=dest_metadata_file_path + ) + + with open(dest_metadata_file_path) as df: + doc = yaml.load(df, Loader=yaml.FullLoader) + + doc["name"] = dest_action + if "pack" in doc: + doc["pack"] = dest_pack + doc["entry_point"] = dest_entry_point_file_name + + with open(dest_metadata_file_path, "w") as df: + yaml.dump(doc, df, default_flow_style=False, sort_keys=False) diff --git a/st2common/st2common/validators/api/misc.py b/st2common/st2common/validators/api/misc.py index 215afc5501..d80bf12dde 100644 --- a/st2common/st2common/validators/api/misc.py +++ b/st2common/st2common/validators/api/misc.py @@ -37,3 +37,9 @@ def validate_not_part_of_system_pack(resource_db): raise ValueValidationException(msg) return resource_db + + +def validate_not_part_of_system_pack_by_name(pack_name): + if pack_name == SYSTEM_PACK_NAME: + msg = "Resources belonging to system level packs can't be manipulated" + raise ValueValidationException(msg) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 04e68dee51..6b3155d0fe 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -24,12 +24,28 @@ import st2tests from st2common.services.packs import delete_action_files_from_pack +from st2common.services.packs import clone_action TEST_PACK = "dummy_pack_1" TEST_PACK_PATH = os.path.join( st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_PACK ) +TEST_SOURCE_PACK = "core" +TEST_SOURCE_PACK_PATH = os.path.join( + st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_SOURCE_PACK +) + +TEST_SOURCE_WORKFLOW_PACK = "orquesta_tests" +TEST_SOURCE_WORKFLOW_PACK_PATH = os.path.join( + st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_SOURCE_WORKFLOW_PACK +) + +TEST_DEST_PACK = "dummy_pack_23" +TEST_DEST_PACK_PATH = os.path.join( + st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_DEST_PACK +) + class DeleteActionFilesTest(unittest2.TestCase): def test_delete_action_files_from_pack(self): @@ -241,3 +257,148 @@ def test_exception_to_remove_resource_metadata_file(self, remove): # to delete metadata file with self.assertRaisesRegexp(Exception, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + +class CloneActionsTest(unittest2.TestCase): + @classmethod + def tearDownClass(cls): + action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + workflow_files_path = os.path.join(action_files_path, "workflows") + for file in os.listdir(action_files_path): + if os.path.isfile(os.path.join(action_files_path, file)): + os.remove(os.path.join(action_files_path, file)) + for file in os.listdir(workflow_files_path): + os.remove(os.path.join(workflow_files_path, file)) + + def test_clone_action_with_python_script_runner(self): + clone_action( + TEST_SOURCE_PACK_PATH, + "actions/inject_trigger.yaml", + "inject_trigger.py", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "action_1", + ) + cloned_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_1.yaml" + ) + cloned_action_entry_point_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_1.py" + ) + self.assertTrue(os.path.exists(cloned_action_metadata_file_path)) + self.assertTrue(os.path.exists(cloned_action_entry_point_file_path)) + + def test_clone_action_with_shell_script_runner(self): + clone_action( + TEST_SOURCE_PACK_PATH, + "actions/sendmail.yaml", + "send_mail/send_mail", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "action_2", + ) + cloned_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_2.yaml" + ) + cloned_action_entry_point_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_2" + ) + self.assertTrue(os.path.exists(cloned_action_metadata_file_path)) + self.assertTrue(os.path.exists(cloned_action_entry_point_file_path)) + + def test_clone_action_with_local_shell_cmd_runner(self): + clone_action( + TEST_SOURCE_PACK_PATH, + "actions/echo.yaml", + "", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "action_3", + ) + cloned_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_3.yaml" + ) + self.assertTrue(os.path.exists(cloned_action_metadata_file_path)) + + def test_clone_workflow(self): + clone_action( + TEST_SOURCE_WORKFLOW_PACK_PATH, + "actions/data-flow.yaml", + "workflows/data-flow.yaml", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "workflow_1", + ) + cloned_workflow_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "workflow_1.yaml" + ) + cloned_workflow_entry_point_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "workflows", "workflow_1.yaml" + ) + self.assertTrue(os.path.exists(cloned_workflow_metadata_file_path)) + self.assertTrue(os.path.exists(cloned_workflow_entry_point_file_path)) + + @mock.patch("builtins.open", create=True) + def test_permission_error_to_write_in_destination_file(self, mock_open): + mock_open.side_effect = PermissionError("No permission to write in file") + cloned_action_entry_point_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_4.py" + ) + expected_msg = 'No permission to write in "%s" file' % ( + cloned_action_entry_point_file_path + ) + + with self.assertRaisesRegexp(PermissionError, expected_msg): + clone_action( + TEST_SOURCE_PACK_PATH, + "actions/inject_trigger.yaml", + "inject_trigger.py", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "action_4", + ) + + @mock.patch("builtins.open", create=True) + def test_file_not_found_error_for_destination_file(self, mock_open): + mock_open.side_effect = FileNotFoundError("No such file or directory") + cloned_action_entry_point_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_5.py" + ) + expected_msg = ( + 'Please make sure "workflows" directory present at path: "%s"' + % (cloned_action_entry_point_file_path) + ) + + with self.assertRaisesRegexp(FileNotFoundError, expected_msg): + clone_action( + TEST_SOURCE_PACK_PATH, + "actions/inject_trigger.yaml", + "inject_trigger.py", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "action_5", + ) + + @mock.patch("builtins.open", create=True) + def test_exceptions_to_write_in_destination_file(self, mock_open): + mock_open.side_effect = Exception( + "Exception encoutntered during writing in destination action file" + ) + cloned_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_6.yaml" + ) + expected_msg = ( + 'Could not copy to "%s" file, please check the logs or ask your ' + "StackStorm administrator to check and clone the actions files manually" + % cloned_action_metadata_file_path + ) + + with self.assertRaisesRegexp(Exception, expected_msg): + clone_action( + TEST_SOURCE_PACK_PATH, + "actions/echo.yaml", + "", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "action_6", + ) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py new file mode 100644 index 0000000000..e69de29bb2 From 411f38f6a037df320eaeb57c4e072ceb09eba1ea Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Fri, 27 Aug 2021 17:39:54 +0000 Subject: [PATCH 02/40] Clone api for actions/workflows- adding openapi.yaml file --- st2common/st2common/openapi.yaml | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 723c6f5235..5dad4cc1f3 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -425,6 +425,55 @@ paths: description: Unexpected error schema: $ref: '#/definitions/Error' + /api/v1/actions/{source_pack}/{source_action}/{dest_pack}/{dest_action}: + post: + operationId: st2api.controllers.v1.actions:actions_controller.clone + description: | + Clone one action. + parameters: + - name: source_pack + in: path + description: Source pack name + type: string + required: true + - name: source_action + in: path + description: Source action name + type: string + required: true + - name: dest_pack + in: path + description: Destination pack name + type: string + required: true + - name: dest_action + in: path + description: Destination action name + type: string + required: true + - name: overwrite + in: query + description: Force clone action if destination already exists + type: boolean + default: false + x-parameters: + - name: user + in: context + x-as: requester_user + description: User performing the operation. + responses: + '201': + description: Single action being cloned + schema: + $ref: '#/definitions/Action' + examples: + application/json: + ref: 'core.local' + # and stuff + default: + description: Unexpected error + schema: + $ref: '#/definitions/Error' /api/v1/actions/views/parameters/{action_id}: get: operationId: st2api.controllers.v1.action_views:parameters_view_controller.get_one From 89a94a470c056b2fc2b3138dfd7f28f9b596f684 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Tue, 14 Sep 2021 14:18:51 +0000 Subject: [PATCH 03/40] Modifications done as per code review comments --- st2api/st2api/controllers/v1/actions.py | 113 +++++++--- .../tests/unit/controllers/v1/test_actions.py | 208 ++++++++++++++++++ st2client/st2client/commands/action.py | 87 ++++++++ st2client/st2client/commands/resource.py | 81 +------ st2client/st2client/models/core.py | 18 +- st2client/tests/unit/test_models.py | 25 +++ st2common/st2common/openapi.yaml | 50 +++-- st2common/st2common/openapi.yaml.j2 | 50 +++-- st2common/st2common/services/packs.py | 78 ++++++- st2common/st2common/validators/api/misc.py | 3 +- st2common/tests/unit/services/test_packs.py | 30 ++- 11 files changed, 559 insertions(+), 184 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index bea7a354fc..410223aabc 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -28,7 +28,6 @@ from st2api.controllers import resource from st2api.controllers.v1.action_views import ActionViewsController from st2common import log as logging -from st2common.bootstrap.actionsregistrar import register_actions from st2common.constants.triggers import ACTION_FILE_WRITTEN_TRIGGER from st2common.exceptions.action import InvalidActionParameterException from st2common.exceptions.apivalidation import ValueValidationException @@ -46,6 +45,8 @@ from st2common.content.utils import get_relative_path_to_pack_file from st2common.services.packs import delete_action_files_from_pack from st2common.services.packs import clone_action +from st2common.services.packs import clone_action_db +from st2common.services.packs import remove_unnecessary_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 @@ -274,28 +275,19 @@ def delete(self, ref_or_id, requester_user): LOG.audit("Action deleted. Action.id=%s" % (action_db.id), extra=extra) return Response(status=http_client.NO_CONTENT) - def clone( - self, - source_pack, - source_action, - dest_pack, - dest_action, - requester_user, - overwrite=False, - ): + def clone(self, dest_data, ref_or_id, requester_user): """ - Clone an action. + Clone an action from source pack to destination pack. Handles requests: - POST /actions/{sorce_pack}/{source_action}/{dest_pack}/{dest_action} + POST /actions/{ref_or_id}/clone """ - source_ref = "%s.%s" % (source_pack, source_action) - source_action_db = self._get_by_ref(resource_ref=source_ref) - msg = "The requested source for cloning operation doesn't exists" + source_action_db = self._get_by_ref_or_id(ref_or_id=ref_or_id) if not source_action_db: + msg = "The requested source for cloning operation doesn't exists" abort(http_client.BAD_REQUEST, six.text_type(msg)) - permission_type = PermissionType.ACTION_CREATE + permission_type = PermissionType.ACTION_VIEW rbac_utils = get_rbac_backend().get_utils_class() rbac_utils.assert_user_has_resource_db_permission( user_db=requester_user, @@ -305,54 +297,109 @@ def clone( LOG.debug( "POST /actions/ lookup with ref=%s found object: %s", - source_ref, + ref_or_id, source_action_db, ) source_pack = source_action_db["pack"] source_entry_point = source_action_db["entry_point"] + source_runner_type = source_action_db["runner_type"]["name"] source_metadata_file = source_action_db["metadata_file"] source_pack_base_path = get_pack_base_path(pack_name=source_pack) - dest_pack_base_path = get_pack_base_path(pack_name=dest_pack) + dest_pack_base_path = get_pack_base_path(pack_name=dest_data.dest_pack) if not os.path.isdir(dest_pack_base_path): - raise ValueError('Destination pack "%s" doesn\'t exist' % (dest_pack)) + raise ValueError( + "Destination pack '%s' doesn't exist" % (dest_data.dest_pack) + ) - dest_ref = "%s.%s" % (dest_pack, dest_action) + dest_ref = ".".join([dest_data.dest_pack, dest_data.dest_action]) dest_action_db = self._get_by_ref(resource_ref=dest_ref) try: - validate_not_part_of_system_pack_by_name(dest_pack) + validate_not_part_of_system_pack_by_name(dest_data.dest_pack) except ValueValidationException as e: abort(http_client.BAD_REQUEST, six.text_type(e)) if dest_action_db: - if overwrite: - pass - else: + permission_type = PermissionType.ACTION_CREATE + rbac_utils = get_rbac_backend().get_utils_class() + rbac_utils.assert_user_has_resource_api_permission( + user_db=requester_user, + resource_api=dest_action_db, + permission_type=permission_type, + ) + if not dest_data.overwrite: msg = "The requested destination action already exists" abort(http_client.BAD_REQUEST, six.text_type(msg)) + else: + try: + Action.delete(dest_action_db) + except Exception as e: + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + + try: + cloned_dest_action_db = clone_action_db( + source_action_db=source_action_db, + dest_pack=dest_data.dest_pack, + dest_action=dest_data.dest_action, + ) + dest_runner_type = dest_action_db["runner_type"]["name"] + dest_entry_point_file = dest_action_db["entry_point"] + if source_runner_type != dest_runner_type: + remove_unnecessary_files_from_pack( + dest_pack_base_path, dest_entry_point_file + ) + except Exception as e: + LOG.error( + "Exception encountered during overwriting resource. " + "Exception was %s", + e, + ) + dest_action_db.id = None + Action.add_or_update(dest_action_db) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + else: + cloned_dest_action_db = clone_action_db( + source_action_db=source_action_db, + dest_pack=dest_data.dest_pack, + dest_action=dest_data.dest_action, + ) + + permission_type = PermissionType.ACTION_CREATE + rbac_utils = get_rbac_backend().get_utils_class() + rbac_utils.assert_user_has_resource_api_permission( + user_db=requester_user, + resource_api=cloned_dest_action_db, + permission_type=permission_type, + ) + try: clone_action( source_pack_base_path=source_pack_base_path, source_metadata_file=source_metadata_file, source_entry_point=source_entry_point, + source_runner_type=source_runner_type, dest_pack_base_path=dest_pack_base_path, - dest_pack=dest_pack, - dest_action=dest_action, + dest_pack=dest_data.dest_pack, + dest_action=dest_data.dest_action, ) - register_actions(pack_dir=dest_pack_base_path) except Exception as e: LOG.error( - "Exception encountered during cloning resource." "Exception was %s", + "Exception encountered during cloning action. Exception was %s", e, ) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) - return - dest_action_db = self._get_by_ref(resource_ref=dest_ref) - extra = {"cloned_acion_db": dest_action_db} - LOG.audit("Action cloned. Action.id=%s" % (dest_action_db.id), extra=extra) - cloned_action_api = ActionAPI.from_model(dest_action_db) + try: + cloned_dest_action_db = Action.add_or_update(cloned_dest_action_db) + except Exception as e: + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + + extra = {"cloned_acion_db": cloned_dest_action_db} + LOG.audit( + "Action cloned. Action.id=%s" % (cloned_dest_action_db.id), extra=extra + ) + cloned_action_api = ActionAPI.from_model(cloned_dest_action_db) return Response(json=cloned_action_api, status=http_client.CREATED) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index cd1d8b555c..c2ce5b686f 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -262,6 +262,40 @@ }, } +ACTION_16 = { + "name": "st2.dummy.source_action", + "description": "test description", + "enabled": True, + "pack": "sourcepack", + "entry_point": "/tmp/test/source_action.sh", + "runner_type": "local-shell-script", + "parameters": { + "x": {"type": "string", "default": "A1"}, + "y": {"type": "string", "default": "B1"}, + }, + "tags": [ + {"name": "tag1", "value": "dont-care1"}, + {"name": "tag2", "value": "dont-care2"}, + ], +} + +ACTION_17 = { + "name": "st2.dummy.clone_action", + "description": "test description", + "enabled": True, + "pack": "clonepack", + "entry_point": "/tmp/test/clone_action.sh", + "runner_type": "local-shell-script", + "parameters": { + "x": {"type": "string", "default": "A1"}, + "y": {"type": "string", "default": "B1"}, + }, + "tags": [ + {"name": "tag1", "value": "dont-care1"}, + {"name": "tag2", "value": "dont-care2"}, + ], +} + ACTION_WITH_NOTIFY = { "name": "st2.dummy.action_notify_test", "description": "test description", @@ -757,6 +791,168 @@ def test_delete_action_belonging_to_system_pack(self): del_resp = self.__do_delete(action_id, expect_errors=True) self.assertEqual(del_resp.status_int, 400) + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone(self, mock_clone_action): + source_post_resp = self.__do_post(ACTION_16) + self.assertEqual(source_post_resp.status_int, 201) + dest_post_resp = self.__do_post(ACTION_17) + self.assertEqual(dest_post_resp.status_int, 201) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": "clone_action_2", + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) + self.assertEqual(clone_resp.status_int, 201) + self.__do_delete(self.__get_action_id(source_post_resp)) + self.__do_delete(self.__get_action_id(dest_post_resp)) + self.__do_delete(self.__get_action_id(clone_resp)) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_overwrite(self, mock_clone_action): + source_post_resp = self.__do_post(ACTION_16) + self.assertEqual(source_post_resp.status_int, 201) + dest_post_resp = self.__do_post(ACTION_17) + self.assertEqual(dest_post_resp.status_int, 201) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": ACTION_17["name"], + "overwrite": True, + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) + self.assertEqual(clone_resp.status_int, 201) + self.__do_delete(self.__get_action_id(source_post_resp)) + self.__do_delete(self.__get_action_id(clone_resp)) + + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_source_does_not_exist(self, mock_clone_action): + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": ACTION_17["name"], + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone( + dest_data_body, + source_ref_or_id, + expect_errors=True, + ) + self.assertEqual(clone_resp.status_int, 404) + msg = 'Resource with a reference or id "%s" not found' % source_ref_or_id + self.assertEqual(clone_resp.json["faultstring"], msg) + + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_destination_pack_does_not_exist(self, mock_clone_action): + source_post_resp = self.__do_post(ACTION_16) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": ACTION_17["name"], + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone( + dest_data_body, + source_ref_or_id, + expect_errors=True, + ) + self.assertEqual(clone_resp.status_int, 400) + msg = "Destination pack '%s' doesn't exist" % ACTION_17["pack"] + self.assertEqual(clone_resp.json["faultstring"], msg) + self.__do_delete(self.__get_action_id(source_post_resp)) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_destination_already_exist(self, mock_clone_action): + source_post_resp = self.__do_post(ACTION_16) + dest_post_resp = self.__do_post(ACTION_17) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": ACTION_17["name"], + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone( + dest_data_body, + source_ref_or_id, + expect_errors=True, + ) + self.assertEqual(clone_resp.status_int, 400) + msg = "The requested destination action already exists" + self.assertEqual(clone_resp.json["faultstring"], msg) + self.__do_delete(self.__get_action_id(source_post_resp)) + self.__do_delete(self.__get_action_id(dest_post_resp)) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_exception(self, mock_clone_action): + msg = "Exception encountered during cloning action." + mock_clone_action.side_effect = Exception(msg) + source_post_resp = self.__do_post(ACTION_16) + dest_post_resp = self.__do_post(ACTION_17) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": "clone_action_3", + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone( + dest_data_body, + source_ref_or_id, + expect_errors=True, + ) + self.assertEqual(clone_resp.status_int, 500) + self.assertEqual(clone_resp.json["faultstring"], msg) + self.__do_delete(self.__get_action_id(source_post_resp)) + self.__do_delete(self.__get_action_id(dest_post_resp)) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action_db") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_overwrite_exception_cloning_db_action_reregistered( + self, mock_clone_action_db + ): + msg = "Exception encountered during overwriting resource." + mock_clone_action_db.side_effect = Exception(msg) + source_post_resp = self.__do_post(ACTION_16) + self.__do_post(ACTION_17) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": ACTION_17["name"], + "overwrite": True, + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone( + dest_data_body, + source_ref_or_id, + expect_errors=True, + ) + self.assertEqual(clone_resp.status_int, 500) + self.assertEqual(clone_resp.json["faultstring"], msg) + get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_17["name"]) + action_id = get_resp.json[0]["id"] + self.__do_delete(action_id) + + self.__do_delete(self.__get_action_id(source_post_resp)) + def _insert_mock_models(self): action_1_id = self.__get_action_id(self.__do_post(ACTION_1)) action_2_id = self.__get_action_id(self.__do_post(ACTION_2)) @@ -802,3 +998,15 @@ def __do_delete(self, action_id, expect_errors=False): return self.app.delete( "/v1/actions/%s" % action_id, expect_errors=expect_errors ) + + def __do_clone( + self, + dest_data, + ref_or_id, + expect_errors=False, + ): + return self.app.post_json( + "/v1/actions/%s/clone" % (ref_or_id), + dest_data, + expect_errors=expect_errors, + ) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 29963fd2de..9680a9c2f6 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -208,6 +208,7 @@ def __init__(self, description, app, subparsers, parent_parser=None): "get": ActionGetCommand, "update": ActionUpdateCommand, "delete": ActionDeleteCommand, + "clone": ActionCloneCommand, }, ) @@ -317,6 +318,92 @@ def run_and_print(self, args, **kwargs): self.print_not_found(resource_id) +class ActionCloneCommand(resource.ContentPackResourceCloneCommand): + source_ref = "source_ref_or_id" + dest_pack = "dest_pack_name" + dest_action = "dest_action_name" + + def __init__(self, resource, *args, **kwargs): + super(ActionCloneCommand, self).__init__(resource, *args, **kwargs) + + args_list = [ + self.source_ref, + self.dest_pack, + self.dest_action, + ] + + for var in args_list: + metavar = self._get_metavar_for_argument(argument=var) + helparg = self._get_help_for_argument(resource=resource, argument=var) + self.parser.add_argument(var, metavar=metavar, help=helparg) + + self.parser.add_argument( + "-f", + "--force", + action="store_true", + dest="force", + help="Overwrite action files on disk if destination exists.", + ) + + @add_auth_token_to_kwargs_from_cli + def run(self, args, **kwargs): + source_ref = getattr(args, self.source_ref, None) + dest_pack = getattr(args, self.dest_pack, None) + dest_action = getattr(args, self.dest_action, None) + dest_ref = "%s.%s" % (dest_pack, dest_action) + self.get_resource_by_ref_or_id(source_ref, **kwargs) + + try: + dest_instance = self.get_resource(dest_ref, **kwargs) + except ResourceNotFoundError: + dest_instance = None + + if dest_instance: + user_input = "" + if not args.force: + user_input = input( + "The destination action already exists. Do you want to overwrite? (y/n): " + ) + if args.force or user_input.lower() == "y" or user_input.lower() == "yes": + return self.manager.clone( + source_ref, + dest_pack, + dest_action, + overwrite=True, + **kwargs, + ) + else: + print("Action is not cloned.") + return + else: + return self.manager.clone( + source_ref, + dest_pack, + dest_action, + overwrite=False, + **kwargs, + ) + + def run_and_print(self, args, **kwargs): + try: + instance = self.run(args, **kwargs) + if not instance: + return + self.print_output( + instance, + table.PropertyValueTable, + attributes=["all"], + json=args.json, + yaml=args.yaml, + ) + except ResourceNotFoundError: + source_ref = getattr(args, self.source_ref, None) + self.print_not_found(source_ref) + except Exception as e: + message = six.text_type(e) + print("ERROR: %s" % (message)) + + class ActionRunCommandMixin(object): """ Mixin class which contains utility functions related to action execution. diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index ee7796f85c..ef276b59f0 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -761,11 +761,6 @@ class ContentPackResourceDeleteCommand(ResourceDeleteCommand): class ResourceCloneCommand(ResourceCommand): - pk_source_pack = "source_pack" - pk_source_action = "source_action" - pk_dest_pack = "destination_pack" - pk_dest_action = "destination_action" - def __init__(self, resource, *args, **kwargs): super(ResourceCloneCommand, self).__init__( resource, @@ -775,82 +770,16 @@ def __init__(self, resource, *args, **kwargs): **kwargs, ) - pk_list = [ - self.pk_source_pack, - self.pk_source_action, - self.pk_dest_pack, - self.pk_dest_action, - ] - - for var in pk_list: - metavar = self._get_metavar_for_argument(argument=var) - helparg = self._get_help_for_argument(resource=resource, argument=var) - self.parser.add_argument(var, metavar=metavar, help=helparg) - - self.parser.add_argument( - "-f", - "--force", - action="store_true", - dest="force", - help="Auto yes flag to overwrite action files on disk if destination exists.", - ) - @add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): - source_pack = getattr(args, self.pk_source_pack, None) - source_action = getattr(args, self.pk_source_action, None) - dest_pack = getattr(args, self.pk_dest_pack, None) - dest_action = getattr(args, self.pk_dest_action, None) - source_ref = "%s.%s" % (source_pack, source_action) - instance = self.get_resource(source_ref, **kwargs) - msg = 'Action with name "%s" has been successfully cloned in "%s" pack.' % ( - dest_action, - dest_pack, - ) + raise NotImplementedError("clone '%s' is not implemented." % args.parser) - dest_ref = "%s.%s" % (dest_pack, dest_action) - try: - dest_instance = self.get_resource(dest_ref, **kwargs) - except ResourceNotFoundError: - dest_instance = None + def run_and_print(self, args, **kwargs): + self.run(args, **kwargs) - if dest_instance: - user_input = "" - if not args.force: - user_input = input( - "The destination action already exists. Do you want to overwrite? (y/n): " - ) - if args.force or user_input.lower() == "y" or user_input.lower() == "yes": - self.manager.clone( - instance, - source_pack, - source_action, - dest_pack, - dest_action, - overwrite=True, - **kwargs, - ) - print(msg) - else: - print("Action is not cloned.") - else: - self.manager.clone( - instance, - source_pack, - source_action, - dest_pack, - dest_action, - overwrite=False, - **kwargs, - ) - print(msg) - def run_and_print(self, args, **kwargs): - resource_id = getattr(args, self.pk_source_pack) - try: - self.run(args, **kwargs) - except ResourceNotFoundError: - self.print_not_found(resource_id) +class ContentPackResourceCloneCommand(ResourceCloneCommand): + pass def load_meta_file(file_path): diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 33d8009f89..e3dc76f954 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -379,23 +379,23 @@ def delete(self, instance, **kwargs): @add_auth_token_to_kwargs_from_env def clone( self, - instance, - source_pack, - source_action, + source_ref, dest_pack, dest_action, overwrite, **kwargs, ): - url = "/%s/%s/%s/%s/%s?overwrite=%s" % ( + url = "/%s/%s/clone/?overwrite=%s" % ( self.resource.get_url_path_name(), - source_pack, - source_action, - dest_pack, - dest_action, + source_ref, overwrite, ) - response = self.client.post(url, instance.serialize(), **kwargs) + payload = { + "dest_pack": dest_pack, + "dest_action": dest_action, + "overwrite": overwrite, + } + response = self.client.post(url, payload, **kwargs) if response.status_code != http_client.OK: self.handle_error(response) instance = self.resource.deserialize(parse_api_response(response)) diff --git a/st2client/tests/unit/test_models.py b/st2client/tests/unit/test_models.py index 8a137afa13..46e329847e 100644 --- a/st2client/tests/unit/test_models.py +++ b/st2client/tests/unit/test_models.py @@ -396,3 +396,28 @@ def side_effect_checking_verify_parameter_is_not(): self.assertEqual(mock_requests.call_args_list[1][0], call_args) self.assertEqual(mock_requests.call_args_list[1][1], call_kwargs) + + @mock.patch.object( + httpclient.HTTPClient, + "post", + mock.MagicMock( + return_value=base.FakeResponse(json.dumps(base.RESOURCES[0]), 200, "OK") + ), + ) + def test_resource_clone(self): + mgr = models.ResourceManager(base.FakeResource, base.FAKE_ENDPOINT) + source_ref = "spack.saction" + resource = mgr.clone(source_ref, "dpack", "daction", False) + self.assertIsNotNone(resource) + + @mock.patch.object( + httpclient.HTTPClient, + "post", + mock.MagicMock( + return_value=base.FakeResponse("", 500, "INTERNAL SERVER ERROR") + ), + ) + def test_resource_clone_failed(self): + mgr = models.ResourceManager(base.FakeResource, base.FAKE_ENDPOINT) + source_ref = "spack.saction" + self.assertRaises(Exception, mgr.clone, source_ref, "dpack", "daction") diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 5dad4cc1f3..d13362e388 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -425,37 +425,23 @@ paths: description: Unexpected error schema: $ref: '#/definitions/Error' - /api/v1/actions/{source_pack}/{source_action}/{dest_pack}/{dest_action}: + /api/v1/actions/{ref_or_id}/clone: post: operationId: st2api.controllers.v1.actions:actions_controller.clone description: | Clone one action. parameters: - - name: source_pack - in: path - description: Source pack name - type: string - required: true - - name: source_action - in: path - description: Source action name - type: string - required: true - - name: dest_pack + - name: ref_or_id in: path - description: Destination pack name + description: Source action reference or id type: string required: true - - name: dest_action - in: path - description: Destination action name - type: string + - name: dest_data + in: body + description: Destination action content + schema: + $ref: '#/definitions/ActionCloneRequest' required: true - - name: overwrite - in: query - description: Force clone action if destination already exists - type: boolean - default: false x-parameters: - name: user in: context @@ -4733,6 +4719,9 @@ definitions: allOf: - $ref: '#/definitions/Action' - $ref: '#/definitions/DataFilesSubSchema' + ActionCloneRequest: + allOf: + - $ref: '#/definitions/ActionCloneSchema' ActionParameters: type: object properties: @@ -5413,6 +5402,23 @@ definitions: additionalProperties: False default: [] # additionalProperties: false + ActionCloneSchema: + type: object + properties: + dest_pack: + type: string + description: Destination pack name for cloning + dest_action: + type: string + description: Destination action name for cloning + overwrite: + type: boolean + description: Force clone action if destination already exists + default: false + required: + - dest_pack + - dest_action + additionalProperties: false NotificationPropertySubSchema: type: object properties: diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index 98c07cd219..b3562a7602 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -421,37 +421,23 @@ paths: description: Unexpected error schema: $ref: '#/definitions/Error' - /api/v1/actions/{source_pack}/{source_action}/{dest_pack}/{dest_action}: + /api/v1/actions/{ref_or_id}/clone: post: operationId: st2api.controllers.v1.actions:actions_controller.clone description: | Clone one action. parameters: - - name: source_pack - in: path - description: Source pack name - type: string - required: true - - name: source_action - in: path - description: Source action name - type: string - required: true - - name: dest_pack + - name: ref_or_id in: path - description: Destination pack name + description: Source action reference or id type: string required: true - - name: dest_action - in: path - description: Destination action name - type: string + - name: dest_data + in: body + description: Destination action content + schema: + $ref: '#/definitions/ActionCloneRequest' required: true - - name: overwrite - in: query - description: Force clone action if destination already exists - type: boolean - default: false x-parameters: - name: user in: context @@ -4729,6 +4715,9 @@ definitions: allOf: - $ref: '#/definitions/Action' - $ref: '#/definitions/DataFilesSubSchema' + ActionCloneRequest: + allOf: + - $ref: '#/definitions/ActionCloneSchema' ActionParameters: type: object properties: @@ -5409,6 +5398,23 @@ definitions: additionalProperties: False default: [] # additionalProperties: false + ActionCloneSchema: + type: object + properties: + dest_pack: + type: string + description: Destination pack name for cloning + dest_action: + type: string + description: Destination action name for cloning + overwrite: + type: boolean + description: Force clone action if destination already exists + default: false + required: + - dest_pack + - dest_action + additionalProperties: false NotificationPropertySubSchema: type: object properties: diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 1aad373028..0b084c1f67 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -15,6 +15,7 @@ from __future__ import absolute_import +import copy import itertools import os @@ -22,6 +23,7 @@ import six from six.moves import range from oslo_config import cfg +import shutil import yaml from st2common import log as logging @@ -293,12 +295,9 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): ) -def clone_content_to_destination_file(source_file, destination_file): +def _clone_content_to_destination_file(source_file, destination_file): try: - with open(source_file, "r") as sf: - with open(destination_file, "w") as df: - for line in sf: - df.write(line) + shutil.copy(src=source_file, dst=destination_file) except PermissionError: LOG.error( 'No write permission for "%s" file', @@ -308,7 +307,7 @@ def clone_content_to_destination_file(source_file, destination_file): raise PermissionError(msg) except FileNotFoundError: LOG.error('No "workflows" directory at path: "%s"', destination_file) - msg = 'Please make sure "workflows" directory present at path: "%s"' % ( + msg = "Please make sure 'workflows' directory present in path: '%s'" % ( destination_file ) raise FileNotFoundError(msg) @@ -330,6 +329,7 @@ def clone_action( source_pack_base_path, source_metadata_file, source_entry_point, + source_runner_type, dest_pack_base_path, dest_pack, dest_action, @@ -351,9 +351,9 @@ def clone_action( source_entry_point_file_path = os.path.join( source_pack_base_path, "actions", source_entry_point ) - if str(source_entry_point).endswith("py"): + if source_runner_type == "python-script": dest_entry_point_file_name = "%s.py" % (dest_action) - elif str(source_entry_point).startswith("workflows"): + elif source_runner_type == "orquesta": dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) else: dest_entry_point_file_name = dest_action @@ -361,7 +361,7 @@ def clone_action( dest_entrypoint_file_path = os.path.join( dest_pack_base_path, "actions", dest_entry_point_file_name ) - clone_content_to_destination_file( + _clone_content_to_destination_file( source_file=source_entry_point_file_path, destination_file=dest_entrypoint_file_path, ) @@ -369,7 +369,7 @@ def clone_action( else: dest_entry_point_file_name = "" - clone_content_to_destination_file( + _clone_content_to_destination_file( source_file=source_metadata_file_path, destination_file=dest_metadata_file_path ) @@ -383,3 +383,61 @@ def clone_action( with open(dest_metadata_file_path, "w") as df: yaml.dump(doc, df, default_flow_style=False, sort_keys=False) + + +def clone_action_db(source_action_db, dest_pack, dest_action): + dest_action_db = copy.deepcopy(source_action_db) + source_runner_type = source_action_db["runner_type"]["name"] + if source_runner_type == "python-script": + dest_entry_point_file_name = "%s.py" % (dest_action) + elif source_runner_type == "orquesta": + dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) + elif source_runner_type == "local-shell-script": + dest_entry_point_file_name = dest_action + else: + dest_entry_point_file_name = "" + dest_action_db["entry_point"] = dest_entry_point_file_name + dest_action_db["metadata_file"] = "actions/%s.yaml" % (dest_action) + dest_action_db["name"] = dest_action + dest_ref = ".".join([dest_pack, dest_action]) + dest_action_db["ref"] = dest_ref + dest_action_db["uid"] = "action:%s:%s" % (dest_pack, dest_action) + if "pack" in dest_action_db: + dest_action_db["pack"] = dest_pack + dest_action_db["id"] = None + return dest_action_db + + +def remove_unnecessary_files_from_pack(dest_pack_base_path, dest_entry_point_file): + """ + While cloning an action, in case of overwrite destination the source runner + type is different than the destination runner type then after cloning operation, + unnecessary entry point file need to be removed from destination pack. + """ + dest_entrypoint_file_path = os.path.join( + dest_pack_base_path, "actions", dest_entry_point_file + ) + if os.path.isfile(dest_entrypoint_file_path): + try: + os.remove(dest_entrypoint_file_path) + except PermissionError: + LOG.error( + 'No permission to delete unnecessary "%s" file', + dest_entrypoint_file_path, + ) + msg = 'No permission to delete unnecessary "%s" file from disk' % ( + dest_entrypoint_file_path + ) + raise PermissionError(msg) + except Exception as e: + LOG.error( + 'Could not delete unnecessary "%s" file. Exception was "%s"', + dest_entrypoint_file_path, + e, + ) + msg = ( + 'The unnecessary 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" % (dest_entrypoint_file_path) + ) + raise Exception(msg) diff --git a/st2common/st2common/validators/api/misc.py b/st2common/st2common/validators/api/misc.py index d80bf12dde..bbf6ec1329 100644 --- a/st2common/st2common/validators/api/misc.py +++ b/st2common/st2common/validators/api/misc.py @@ -15,6 +15,7 @@ from __future__ import absolute_import from st2common.constants.pack import SYSTEM_PACK_NAME +from st2common.constants.pack import SYSTEM_PACK_NAMES from st2common.exceptions.apivalidation import ValueValidationException __all__ = ["validate_not_part_of_system_pack"] @@ -40,6 +41,6 @@ def validate_not_part_of_system_pack(resource_db): def validate_not_part_of_system_pack_by_name(pack_name): - if pack_name == SYSTEM_PACK_NAME: + if pack_name in SYSTEM_PACK_NAMES: msg = "Resources belonging to system level packs can't be manipulated" raise ValueValidationException(msg) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 6b3155d0fe..9ccaf09bd5 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -268,13 +268,15 @@ def tearDownClass(cls): if os.path.isfile(os.path.join(action_files_path, file)): os.remove(os.path.join(action_files_path, file)) for file in os.listdir(workflow_files_path): - os.remove(os.path.join(workflow_files_path, file)) + if os.path.isfile(os.path.join(workflow_files_path, file)): + os.remove(os.path.join(workflow_files_path, file)) def test_clone_action_with_python_script_runner(self): clone_action( TEST_SOURCE_PACK_PATH, "actions/inject_trigger.yaml", "inject_trigger.py", + "python-script", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "action_1", @@ -293,6 +295,7 @@ def test_clone_action_with_shell_script_runner(self): TEST_SOURCE_PACK_PATH, "actions/sendmail.yaml", "send_mail/send_mail", + "local-shell-script", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "action_2", @@ -311,6 +314,7 @@ def test_clone_action_with_local_shell_cmd_runner(self): TEST_SOURCE_PACK_PATH, "actions/echo.yaml", "", + "local-shell-cmd", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "action_3", @@ -325,6 +329,7 @@ def test_clone_workflow(self): TEST_SOURCE_WORKFLOW_PACK_PATH, "actions/data-flow.yaml", "workflows/data-flow.yaml", + "orquesta", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "workflow_1", @@ -338,9 +343,9 @@ def test_clone_workflow(self): self.assertTrue(os.path.exists(cloned_workflow_metadata_file_path)) self.assertTrue(os.path.exists(cloned_workflow_entry_point_file_path)) - @mock.patch("builtins.open", create=True) - def test_permission_error_to_write_in_destination_file(self, mock_open): - mock_open.side_effect = PermissionError("No permission to write in file") + @mock.patch("shutil.copy") + def test_permission_error_to_write_in_destination_file(self, mock_copy): + mock_copy.side_effect = PermissionError("No permission to write in file") cloned_action_entry_point_file_path = os.path.join( TEST_DEST_PACK_PATH, "actions", "action_4.py" ) @@ -353,19 +358,20 @@ def test_permission_error_to_write_in_destination_file(self, mock_open): TEST_SOURCE_PACK_PATH, "actions/inject_trigger.yaml", "inject_trigger.py", + "python-script", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "action_4", ) - @mock.patch("builtins.open", create=True) - def test_file_not_found_error_for_destination_file(self, mock_open): - mock_open.side_effect = FileNotFoundError("No such file or directory") + @mock.patch("shutil.copy") + def test_file_not_found_error_for_destination_file(self, mock_copy): + mock_copy.side_effect = FileNotFoundError("No such file or directory") cloned_action_entry_point_file_path = os.path.join( TEST_DEST_PACK_PATH, "actions", "action_5.py" ) expected_msg = ( - 'Please make sure "workflows" directory present at path: "%s"' + "Please make sure 'workflows' directory present in path: '%s'" % (cloned_action_entry_point_file_path) ) @@ -374,14 +380,15 @@ def test_file_not_found_error_for_destination_file(self, mock_open): TEST_SOURCE_PACK_PATH, "actions/inject_trigger.yaml", "inject_trigger.py", + "python-script", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "action_5", ) - @mock.patch("builtins.open", create=True) - def test_exceptions_to_write_in_destination_file(self, mock_open): - mock_open.side_effect = Exception( + @mock.patch("shutil.copy") + def test_exceptions_to_write_in_destination_file(self, mock_copy): + mock_copy.side_effect = Exception( "Exception encoutntered during writing in destination action file" ) cloned_action_metadata_file_path = os.path.join( @@ -398,6 +405,7 @@ def test_exceptions_to_write_in_destination_file(self, mock_open): TEST_SOURCE_PACK_PATH, "actions/echo.yaml", "", + "local-shell-cmd", TEST_DEST_PACK_PATH, TEST_DEST_PACK, "action_6", From ba17481b6d65ed487c80da24d61421ecc53996f0 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Tue, 14 Sep 2021 16:16:30 +0000 Subject: [PATCH 04/40] Adding RBAC unit tests for clone action API --- st2api/tests/base.py | 100 ++++++ .../unit/controllers/v1/test_actions_rbac.py | 317 ++++++++++++++++++ 2 files changed, 417 insertions(+) create mode 100644 st2api/tests/base.py create mode 100644 st2api/tests/unit/controllers/v1/test_actions_rbac.py diff --git a/st2api/tests/base.py b/st2api/tests/base.py new file mode 100644 index 0000000000..1ea367fa3f --- /dev/null +++ b/st2api/tests/base.py @@ -0,0 +1,100 @@ +# Copyright 2020 The StackStorm Authors. +# Copyright (C) 2020 Extreme Networks, Inc - All Rights Reserved +# +# 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. + +""" +Module containing base classes for API controller RBAC tests. +""" + +from __future__ import absolute_import + +from oslo_config import cfg + +from st2common.rbac.types import SystemRole +from st2common.persistence.auth import User +from st2common.persistence.rbac import UserRoleAssignment +from st2common.models.db.auth import UserDB +from st2common.models.db.rbac import UserRoleAssignmentDB +from st2common.rbac.migrations import run_all as run_all_rbac_migrations +from st2tests.api import BaseFunctionalTest +from st2tests.base import CleanDbTestCase + + +__all__ = ["APIControllerWithRBACTestCase"] + + +class BaseAPIControllerWithRBACTestCase(BaseFunctionalTest, CleanDbTestCase): + """ + Base test case class for testing API controllers with RBAC enabled. + """ + + enable_auth = True + + @classmethod + def setUpClass(cls): + super(BaseAPIControllerWithRBACTestCase, cls).setUpClass() + + # Make sure RBAC is enabeld + cfg.CONF.set_override(name="enable", override=True, group="rbac") + cfg.CONF.set_override(name="backend", override="default", group="rbac") + + @classmethod + def tearDownClass(cls): + super(BaseAPIControllerWithRBACTestCase, cls).tearDownClass() + + def setUp(self): + super(BaseAPIControllerWithRBACTestCase, self).setUp() + + self.users = {} + self.roles = {} + + # Run RBAC migrations + run_all_rbac_migrations() + + # Insert mock users with default role assignments + role_names = [SystemRole.SYSTEM_ADMIN, SystemRole.ADMIN, SystemRole.OBSERVER] + for role_name in role_names: + user_db = UserDB(name=role_name) + user_db = User.add_or_update(user_db) + self.users[role_name] = user_db + + role_assignment_db = UserRoleAssignmentDB( + user=user_db.name, + role=role_name, + source="assignments/%s.yaml" % user_db.name, + ) + UserRoleAssignment.add_or_update(role_assignment_db) + + # Insert a user with no permissions and role assignments + user_1_db = UserDB(name="no_permissions") + user_1_db = User.add_or_update(user_1_db) + self.users["no_permissions"] = user_1_db + + # Insert special system user + user_2_db = UserDB(name="system_user") + user_2_db = User.add_or_update(user_2_db) + self.users["system_user"] = user_2_db + + role_assignment_db = UserRoleAssignmentDB( + user=user_2_db.name, + role=SystemRole.ADMIN, + source="assignments/%s.yaml" % user_2_db.name, + ) + UserRoleAssignment.add_or_update(role_assignment_db) + + +class APIControllerWithRBACTestCase(BaseAPIControllerWithRBACTestCase): + from st2api import app + + app_module = app diff --git a/st2api/tests/unit/controllers/v1/test_actions_rbac.py b/st2api/tests/unit/controllers/v1/test_actions_rbac.py new file mode 100644 index 0000000000..8b90051fe4 --- /dev/null +++ b/st2api/tests/unit/controllers/v1/test_actions_rbac.py @@ -0,0 +1,317 @@ +# Copyright 2020 The StackStorm Authors. +# Copyright (C) 2020 Extreme Networks, Inc - All Rights Reserved +# +# 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. + +import mock +import os +import six + +import st2common.validators.api.action as action_validator +from st2common.rbac.types import PermissionType +from st2common.rbac.types import ResourceType +from st2common.persistence.auth import User +from st2common.persistence.rbac import Role +from st2common.persistence.rbac import UserRoleAssignment +from st2common.persistence.rbac import PermissionGrant +from st2common.models.db.auth import UserDB +from st2common.models.db.rbac import RoleDB +from st2common.models.db.rbac import UserRoleAssignmentDB +from st2common.models.db.rbac import PermissionGrantDB +from st2tests.fixturesloader import FixturesLoader +from st2api.controllers.v1.actions import ActionsController + +from tests.base import APIControllerWithRBACTestCase +from st2tests.api import APIControllerWithIncludeAndExcludeFilterTestCase + +http_client = six.moves.http_client + +__all__ = ["ActionControllerRBACTestCase"] + +FIXTURES_PACK = "generic" +TEST_FIXTURES = { + "runners": ["testrunner1.yaml"], + "actions": ["action1.yaml", "local.yaml"], +} + +ACTION_2 = { + "name": "ma.dummy.action", + "pack": "examples", + "description": "test description", + "enabled": True, + "entry_point": "/tmp/test/action2.py", + "runner_type": "local-shell-script", + "parameters": { + "c": {"type": "string", "default": "C1", "position": 0}, + "d": {"type": "string", "default": "D1", "immutable": True}, + }, +} + +ACTION_3 = { + "name": "ma.dummy.clone_action", + "pack": "clonepack", + "description": "test description", + "enabled": True, + "entry_point": "/tmp/test/clone_action.sh", + "runner_type": "local-shell-script", + "parameters": { + "x": {"type": "string", "default": "A1"}, + "y": {"type": "string", "default": "B1"}, + }, +} + + +class ActionControllerRBACTestCase( + APIControllerWithRBACTestCase, APIControllerWithIncludeAndExcludeFilterTestCase +): + + # Attributes used by APIControllerWithIncludeAndExcludeFilterTestCase + get_all_path = "/v1/actions" + controller_cls = ActionsController + include_attribute_field_name = "parameters" + exclude_attribute_field_name = "parameters" + test_exact_object_count = False + rbac_enabled = True + + fixtures_loader = FixturesLoader() + + def setUp(self): + super(ActionControllerRBACTestCase, self).setUp() + self.models = self.fixtures_loader.save_fixtures_to_db( + fixtures_pack=FIXTURES_PACK, fixtures_dict=TEST_FIXTURES + ) + + # Insert mock users, roles and assignments + # Users + user_2_db = UserDB(name="action_create") + user_2_db = User.add_or_update(user_2_db) + self.users["action_create"] = user_2_db + + # Roles + # action_create grant on parent pack + grant_db = PermissionGrantDB( + resource_uid="pack:examples", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_CREATE], + ) + grant_db = PermissionGrant.add_or_update(grant_db) + permission_grants = [str(grant_db.id)] + role_1_db = RoleDB(name="action_create", permission_grants=permission_grants) + role_1_db = Role.add_or_update(role_1_db) + self.roles["action_create"] = role_1_db + + # Role assignments + user_db = self.users["action_create"] + role_assignment_db = UserRoleAssignmentDB( + user=user_db.name, + role=self.roles["action_create"].name, + source="assignments/%s.yaml" % user_db.name, + ) + UserRoleAssignment.add_or_update(role_assignment_db) + + # creating `action_clone` user with all permissions related to cloning an action + user_3_db = UserDB(name="action_clone") + user_3_db = User.add_or_update(user_3_db) + self.users["action_clone"] = user_3_db + + # roles of action_clone user + grant_db = PermissionGrantDB( + resource_uid="pack:clonepack", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_CREATE], + ) + grant_db = PermissionGrant.add_or_update(grant_db) + grant_db_view = PermissionGrantDB( + resource_uid="pack:examples", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_VIEW], + ) + grant_db_view = PermissionGrant.add_or_update(grant_db_view) + grant_db_create = PermissionGrantDB( + resource_uid="pack:examples", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_CREATE], + ) + grant_db_create = PermissionGrant.add_or_update(grant_db_create) + permission_grants = [ + str(grant_db.id), + str(grant_db_view.id), + str(grant_db_create.id), + ] + role_1_db = RoleDB(name="action_clone", permission_grants=permission_grants) + role_1_db = Role.add_or_update(role_1_db) + self.roles["action_clone"] = role_1_db + + # role assignments for action_clone user + user_db = self.users["action_clone"] + role_assignment_db = UserRoleAssignmentDB( + user=user_db.name, + role=self.roles["action_clone"].name, + source="assignments/%s.yaml" % user_db.name, + ) + UserRoleAssignment.add_or_update(role_assignment_db) + + # creating `no_create_permission` user with action_view permission on source action + # but no create_action permission on destination pack + user_2_db = UserDB(name="no_create_permission") + user_2_db = User.add_or_update(user_2_db) + self.users["no_create_permission"] = user_2_db + + # roles of no_create_permission user + grant_db = PermissionGrantDB( + resource_uid="pack:examples", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_VIEW], + ) + grant_db = PermissionGrant.add_or_update(grant_db) + permission_grants = [str(grant_db.id)] + role_1_db = RoleDB( + name="no_create_permission", permission_grants=permission_grants + ) + role_1_db = Role.add_or_update(role_1_db) + self.roles["no_create_permission"] = role_1_db + + # role assignments for no_create_permission user + user_db = self.users["no_create_permission"] + role_assignment_db = UserRoleAssignmentDB( + user=user_db.name, + role=self.roles["no_create_permission"].name, + source="assignments/%s.yaml" % user_db.name, + ) + UserRoleAssignment.add_or_update(role_assignment_db) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_action_success(self, mock_clone_action): + user_db = self.users["action_clone"] + self.use_user(user_db) + self.__do_post(ACTION_2) + self.__do_post(ACTION_3) + dest_data_body = { + "dest_pack": ACTION_3["pack"], + "dest_action": "clone_action_2", + } + source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) + clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) + self.assertEqual(clone_resp.status_code, http_client.CREATED) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_overwrite_action_success(self, mock_clone_action): + user_db = self.users["action_clone"] + self.use_user(user_db) + self.__do_post(ACTION_2) + self.__do_post(ACTION_3) + dest_data_body = { + "dest_pack": ACTION_3["pack"], + "dest_action": ACTION_3["name"], + "overwrite": True, + } + source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) + clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) + self.assertEqual(clone_resp.status_code, http_client.CREATED) + + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_action_no_source_action_view_permission(self): + user_db = self.users["action_create"] + self.use_user(user_db) + self.__do_post(ACTION_2) + user_db = self.users["no_permissions"] + self.use_user(user_db) + dest_data_body = { + "dest_pack": ACTION_3["pack"], + "dest_action": "clone_action_3", + } + source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) + clone_resp = self.__do_clone( + dest_data_body, source_ref_or_id, expect_errors=True + ) + expected_msg = ( + 'User "no_permissions" doesn\'t have required permission "action_view" ' + 'on resource "action:examples:ma.dummy.action"' + ) + self.assertEqual(clone_resp.status_code, http_client.FORBIDDEN) + self.assertEqual(clone_resp.json["faultstring"], expected_msg) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_action_no_destination_action_create_permission(self): + user_db = self.users["action_clone"] + self.use_user(user_db) + self.__do_post(ACTION_2) + self.__do_post(ACTION_3) + user_db = self.users["no_create_permission"] + self.use_user(user_db) + dest_data_body = { + "dest_pack": ACTION_3["pack"], + "dest_action": "clone_action_4", + } + source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) + clone_resp = self.__do_clone( + dest_data_body, source_ref_or_id, expect_errors=True + ) + expected_msg = ( + 'User "no_create_permission" doesn\'t have required permission "action_create" ' + 'on resource "action:clonepack:clone_action_4"' + ) + self.assertEqual(clone_resp.status_code, http_client.FORBIDDEN) + self.assertEqual(clone_resp.json["faultstring"], expected_msg) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_overwrite_action_no_destination_action_create_permission(self): + user_db = self.users["action_clone"] + self.use_user(user_db) + self.__do_post(ACTION_2) + self.__do_post(ACTION_3) + user_db = self.users["no_create_permission"] + self.use_user(user_db) + dest_data_body = { + "dest_pack": ACTION_3["pack"], + "dest_action": ACTION_3["name"], + "overwrite": True, + } + source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) + clone_resp = self.__do_clone( + dest_data_body, source_ref_or_id, expect_errors=True + ) + expected_msg = ( + 'User "no_create_permission" doesn\'t have required permission "action_create" ' + 'on resource "action:clonepack:ma.dummy.clone_action"' + ) + self.assertEqual(clone_resp.status_code, http_client.FORBIDDEN) + self.assertEqual(clone_resp.json["faultstring"], expected_msg) + + def _insert_mock_models(self): + action_ids = [action["id"] for action in self.models["actions"].values()] + return action_ids + + def __do_post(self, rule): + return self.app.post_json("/v1/actions", rule, expect_errors=True) + + def __do_clone(self, dest_data, action_id, expect_errors=False): + return self.app.post_json( + "/v1/actions/%s/clone" % (action_id), dest_data, expect_errors=expect_errors + ) From 0dedd4a394938f9633d80d9f4e099a9636fda25e Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Wed, 15 Sep 2021 15:10:18 +0000 Subject: [PATCH 05/40] Refactoring clone action function and unit tests --- st2api/st2api/controllers/v1/actions.py | 92 +++++++++---------- .../tests/unit/controllers/v1/test_actions.py | 3 - .../unit/controllers/v1/test_actions_rbac.py | 83 ++++++++++++++++- st2client/st2client/models/core.py | 6 +- st2common/st2common/services/packs.py | 35 ------- 5 files changed, 122 insertions(+), 97 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 410223aabc..f8d292e59f 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -31,6 +31,7 @@ from st2common.constants.triggers import ACTION_FILE_WRITTEN_TRIGGER from st2common.exceptions.action import InvalidActionParameterException from st2common.exceptions.apivalidation import ValueValidationException +from st2common.exceptions.rbac import ResourceAccessDeniedError from st2common.persistence.action import Action from st2common.models.api.action import ActionAPI from st2common.persistence.pack import Pack @@ -46,7 +47,6 @@ from st2common.services.packs import delete_action_files_from_pack from st2common.services.packs import clone_action from st2common.services.packs import clone_action_db -from st2common.services.packs import remove_unnecessary_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 @@ -287,6 +287,8 @@ def clone(self, dest_data, ref_or_id, requester_user): msg = "The requested source for cloning operation doesn't exists" abort(http_client.BAD_REQUEST, six.text_type(msg)) + LOG.debug("Clone source action object found: %s", source_action_db) + permission_type = PermissionType.ACTION_VIEW rbac_utils = get_rbac_backend().get_utils_class() rbac_utils.assert_user_has_resource_db_permission( @@ -295,22 +297,16 @@ def clone(self, dest_data, ref_or_id, requester_user): permission_type=permission_type, ) - LOG.debug( - "POST /actions/ lookup with ref=%s found object: %s", - ref_or_id, - source_action_db, - ) - source_pack = source_action_db["pack"] source_entry_point = source_action_db["entry_point"] source_runner_type = source_action_db["runner_type"]["name"] source_metadata_file = source_action_db["metadata_file"] source_pack_base_path = get_pack_base_path(pack_name=source_pack) dest_pack_base_path = get_pack_base_path(pack_name=dest_data.dest_pack) + if not os.path.isdir(dest_pack_base_path): - raise ValueError( - "Destination pack '%s' doesn't exist" % (dest_data.dest_pack) - ) + msg = "Destination pack '%s' doesn't exist" % (dest_data.dest_pack) + abort(http_client.BAD_REQUEST, six.text_type(msg)) dest_ref = ".".join([dest_data.dest_pack, dest_data.dest_action]) dest_action_db = self._get_by_ref(resource_ref=dest_ref) @@ -321,59 +317,50 @@ def clone(self, dest_data, ref_or_id, requester_user): abort(http_client.BAD_REQUEST, six.text_type(e)) if dest_action_db: - permission_type = PermissionType.ACTION_CREATE - rbac_utils = get_rbac_backend().get_utils_class() - rbac_utils.assert_user_has_resource_api_permission( - user_db=requester_user, - resource_api=dest_action_db, - permission_type=permission_type, - ) if not dest_data.overwrite: msg = "The requested destination action already exists" abort(http_client.BAD_REQUEST, six.text_type(msg)) - else: - try: - Action.delete(dest_action_db) - except Exception as e: - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) - - try: - cloned_dest_action_db = clone_action_db( - source_action_db=source_action_db, - dest_pack=dest_data.dest_pack, - dest_action=dest_data.dest_action, - ) - dest_runner_type = dest_action_db["runner_type"]["name"] - dest_entry_point_file = dest_action_db["entry_point"] - if source_runner_type != dest_runner_type: - remove_unnecessary_files_from_pack( - dest_pack_base_path, dest_entry_point_file - ) - except Exception as e: - LOG.error( - "Exception encountered during overwriting resource. " - "Exception was %s", - e, - ) - dest_action_db.id = None - Action.add_or_update(dest_action_db) - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) - else: + + try: + permission_type = PermissionType.ACTION_CREATE + rbac_utils = get_rbac_backend().get_utils_class() + rbac_utils.assert_user_has_resource_db_permission( + user_db=requester_user, + resource_db=dest_action_db, + permission_type=permission_type, + ) + + permission_type = PermissionType.ACTION_DELETE + rbac_utils = get_rbac_backend().get_utils_class() + rbac_utils.assert_user_has_resource_db_permission( + user_db=requester_user, + resource_db=dest_action_db, + permission_type=permission_type, + ) + self.delete(dest_ref, requester_user) + except ResourceAccessDeniedError as e: + abort(http_client.UNAUTHORIZED, six.text_type(e)) + except Exception as e: + LOG.debug( + "Exception encountered during deleting existing destination action. " + "Exception was: %s", + e, + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + + try: cloned_dest_action_db = clone_action_db( source_action_db=source_action_db, dest_pack=dest_data.dest_pack, dest_action=dest_data.dest_action, ) - permission_type = PermissionType.ACTION_CREATE rbac_utils = get_rbac_backend().get_utils_class() - rbac_utils.assert_user_has_resource_api_permission( + rbac_utils.assert_user_has_resource_db_permission( user_db=requester_user, - resource_api=cloned_dest_action_db, + resource_db=cloned_dest_action_db, permission_type=permission_type, ) - - try: clone_action( source_pack_base_path=source_pack_base_path, source_metadata_file=source_metadata_file, @@ -383,6 +370,11 @@ def clone(self, dest_data, ref_or_id, requester_user): dest_pack=dest_data.dest_pack, dest_action=dest_data.dest_action, ) + except PermissionError as e: + LOG.error("No permission to clone action in destination pack.") + abort(http_client.FORBIDDEN, six.text_type(e)) + except ResourceAccessDeniedError as e: + abort(http_client.UNAUTHORIZED, six.text_type(e)) except Exception as e: LOG.error( "Exception encountered during cloning action. Exception was %s", diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index c2ce5b686f..be2ad653b0 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -947,9 +947,6 @@ def test_clone_overwrite_exception_cloning_db_action_reregistered( ) self.assertEqual(clone_resp.status_int, 500) self.assertEqual(clone_resp.json["faultstring"], msg) - get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_17["name"]) - action_id = get_resp.json[0]["id"] - self.__do_delete(action_id) self.__do_delete(self.__get_action_id(source_post_resp)) diff --git a/st2api/tests/unit/controllers/v1/test_actions_rbac.py b/st2api/tests/unit/controllers/v1/test_actions_rbac.py index 8b90051fe4..a5a50cc894 100644 --- a/st2api/tests/unit/controllers/v1/test_actions_rbac.py +++ b/st2api/tests/unit/controllers/v1/test_actions_rbac.py @@ -143,10 +143,17 @@ def setUp(self): permission_types=[PermissionType.ACTION_CREATE], ) grant_db_create = PermissionGrant.add_or_update(grant_db_create) + grant_db_delete = PermissionGrantDB( + resource_uid="pack:clonepack", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_DELETE], + ) + grant_db_delete = PermissionGrant.add_or_update(grant_db_delete) permission_grants = [ str(grant_db.id), str(grant_db_view.id), str(grant_db_create.id), + str(grant_db_delete.id), ] role_1_db = RoleDB(name="action_clone", permission_grants=permission_grants) role_1_db = Role.add_or_update(role_1_db) @@ -174,7 +181,13 @@ def setUp(self): permission_types=[PermissionType.ACTION_VIEW], ) grant_db = PermissionGrant.add_or_update(grant_db) - permission_grants = [str(grant_db.id)] + grant_db_delete = PermissionGrantDB( + resource_uid="pack:clonepack", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_DELETE], + ) + grant_db_delete = PermissionGrant.add_or_update(grant_db_delete) + permission_grants = [str(grant_db.id), str(grant_db_delete.id)] role_1_db = RoleDB( name="no_create_permission", permission_grants=permission_grants ) @@ -190,6 +203,41 @@ def setUp(self): ) UserRoleAssignment.add_or_update(role_assignment_db) + # creating `no_delete_permission` user with action_view permission on source action, + # action_create on destination pack but no create_delete permission on destination pack + user_2_db = UserDB(name="no_delete_permission") + user_2_db = User.add_or_update(user_2_db) + self.users["no_delete_permission"] = user_2_db + + # roles of no_delete_permission user + grant_db_view = PermissionGrantDB( + resource_uid="pack:examples", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_VIEW], + ) + grant_db_view = PermissionGrant.add_or_update(grant_db_view) + grant_db_create = PermissionGrantDB( + resource_uid="pack:clonepack", + resource_type=ResourceType.PACK, + permission_types=[PermissionType.ACTION_CREATE], + ) + grant_db_create = PermissionGrant.add_or_update(grant_db_create) + permission_grants = [str(grant_db_view.id), str(grant_db_create.id)] + role_1_db = RoleDB( + name="no_delete_permission", permission_grants=permission_grants + ) + role_1_db = Role.add_or_update(role_1_db) + self.roles["no_delete_permission"] = role_1_db + + # role assignments for no_delete_permission user + user_db = self.users["no_delete_permission"] + role_assignment_db = UserRoleAssignmentDB( + user=user_db.name, + role=self.roles["no_delete_permission"].name, + source="assignments/%s.yaml" % user_db.name, + ) + UserRoleAssignment.add_or_update(role_assignment_db) + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @mock.patch("st2api.controllers.v1.actions.clone_action") @mock.patch.object( @@ -274,14 +322,14 @@ def test_clone_action_no_destination_action_create_permission(self): 'User "no_create_permission" doesn\'t have required permission "action_create" ' 'on resource "action:clonepack:clone_action_4"' ) - self.assertEqual(clone_resp.status_code, http_client.FORBIDDEN) + self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) self.assertEqual(clone_resp.json["faultstring"], expected_msg) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_overwrite_action_no_destination_action_create_permission(self): + def test_clone_overwrite_no_destination_action_create_permission(self): user_db = self.users["action_clone"] self.use_user(user_db) self.__do_post(ACTION_2) @@ -301,7 +349,34 @@ def test_clone_overwrite_action_no_destination_action_create_permission(self): 'User "no_create_permission" doesn\'t have required permission "action_create" ' 'on resource "action:clonepack:ma.dummy.clone_action"' ) - self.assertEqual(clone_resp.status_code, http_client.FORBIDDEN) + self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) + self.assertEqual(clone_resp.json["faultstring"], expected_msg) + + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_overwrite_no_destination_action_delete_permission(self): + user_db = self.users["action_clone"] + self.use_user(user_db) + self.__do_post(ACTION_2) + self.__do_post(ACTION_3) + user_db = self.users["no_delete_permission"] + self.use_user(user_db) + dest_data_body = { + "dest_pack": ACTION_3["pack"], + "dest_action": ACTION_3["name"], + "overwrite": True, + } + source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) + clone_resp = self.__do_clone( + dest_data_body, source_ref_or_id, expect_errors=True + ) + expected_msg = ( + 'User "no_delete_permission" doesn\'t have required permission "action_delete" ' + 'on resource "action:clonepack:ma.dummy.clone_action"' + ) + self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) self.assertEqual(clone_resp.json["faultstring"], expected_msg) def _insert_mock_models(self): diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index e3dc76f954..38d94416d0 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -385,11 +385,7 @@ def clone( overwrite, **kwargs, ): - url = "/%s/%s/clone/?overwrite=%s" % ( - self.resource.get_url_path_name(), - source_ref, - overwrite, - ) + url = "/%s/%s/clone" % (self.resource.get_url_path_name(), source_ref) payload = { "dest_pack": dest_pack, "dest_action": dest_action, diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 0b084c1f67..01fe225722 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -406,38 +406,3 @@ def clone_action_db(source_action_db, dest_pack, dest_action): dest_action_db["pack"] = dest_pack dest_action_db["id"] = None return dest_action_db - - -def remove_unnecessary_files_from_pack(dest_pack_base_path, dest_entry_point_file): - """ - While cloning an action, in case of overwrite destination the source runner - type is different than the destination runner type then after cloning operation, - unnecessary entry point file need to be removed from destination pack. - """ - dest_entrypoint_file_path = os.path.join( - dest_pack_base_path, "actions", dest_entry_point_file - ) - if os.path.isfile(dest_entrypoint_file_path): - try: - os.remove(dest_entrypoint_file_path) - except PermissionError: - LOG.error( - 'No permission to delete unnecessary "%s" file', - dest_entrypoint_file_path, - ) - msg = 'No permission to delete unnecessary "%s" file from disk' % ( - dest_entrypoint_file_path - ) - raise PermissionError(msg) - except Exception as e: - LOG.error( - 'Could not delete unnecessary "%s" file. Exception was "%s"', - dest_entrypoint_file_path, - e, - ) - msg = ( - 'The unnecessary 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" % (dest_entrypoint_file_path) - ) - raise Exception(msg) From 5a1f1a8dcce82c38c4009f6817deef23ab8f947b Mon Sep 17 00:00:00 2001 From: ashwini-orchestral <72917414+ashwini-orchestral@users.noreply.github.com> Date: Wed, 29 Sep 2021 18:25:36 +0530 Subject: [PATCH 06/40] Refactored clone_actions function to create workflows directory if doesn't exists and retaining action file extensions --- st2common/st2common/services/packs.py | 44 ++++++++++++--------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 01fe225722..7a23642ab9 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -305,12 +305,6 @@ def _clone_content_to_destination_file(source_file, destination_file): ) msg = 'No permission to write in "%s" file' % (destination_file) raise PermissionError(msg) - except FileNotFoundError: - LOG.error('No "workflows" directory at path: "%s"', destination_file) - msg = "Please make sure 'workflows' directory present in path: '%s'" % ( - destination_file - ) - raise FileNotFoundError(msg) except Exception as e: LOG.error( 'Could not copy to "%s" file. Exception was "%s"', @@ -346,18 +340,24 @@ def clone_action( dest_metadata_file_path = os.path.join( dest_pack_base_path, "actions", dest_metadata_file_name ) + _clone_content_to_destination_file( + source_file=source_metadata_file_path, destination_file=dest_metadata_file_path + ) if source_entry_point: - source_entry_point_file_path = os.path.join( - source_pack_base_path, "actions", source_entry_point - ) - if source_runner_type == "python-script": - dest_entry_point_file_name = "%s.py" % (dest_action) - elif source_runner_type == "orquesta": + if source_runner_type in ["orquesta", "action-chain"]: dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) + # creating workflows directory if doesn't exist + wf_dir_path = os.path.join(dest_pack_base_path, "actions", "workflows") + if not os.path.isdir(wf_dir_path): + os.mkdir(path=wf_dir_path) else: - dest_entry_point_file_name = dest_action + old_ext = os.path.splitext(source_entry_point)[1] + dest_entry_point_file_name = dest_action + old_ext + source_entry_point_file_path = os.path.join( + source_pack_base_path, "actions", source_entry_point + ) dest_entrypoint_file_path = os.path.join( dest_pack_base_path, "actions", dest_entry_point_file_name ) @@ -365,14 +365,9 @@ def clone_action( source_file=source_entry_point_file_path, destination_file=dest_entrypoint_file_path, ) - else: dest_entry_point_file_name = "" - _clone_content_to_destination_file( - source_file=source_metadata_file_path, destination_file=dest_metadata_file_path - ) - with open(dest_metadata_file_path) as df: doc = yaml.load(df, Loader=yaml.FullLoader) @@ -388,12 +383,12 @@ def clone_action( def clone_action_db(source_action_db, dest_pack, dest_action): dest_action_db = copy.deepcopy(source_action_db) source_runner_type = source_action_db["runner_type"]["name"] - if source_runner_type == "python-script": - dest_entry_point_file_name = "%s.py" % (dest_action) - elif source_runner_type == "orquesta": - dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) - elif source_runner_type == "local-shell-script": - dest_entry_point_file_name = dest_action + if source_action_db["entry_point"]: + if source_runner_type in ["orquesta", "action-chain"]: + dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) + else: + old_ext = os.path.splitext(source_action_db["entry_point"])[1] + dest_entry_point_file_name = dest_action + old_ext else: dest_entry_point_file_name = "" dest_action_db["entry_point"] = dest_entry_point_file_name @@ -405,4 +400,5 @@ def clone_action_db(source_action_db, dest_pack, dest_action): if "pack" in dest_action_db: dest_action_db["pack"] = dest_pack dest_action_db["id"] = None + return dest_action_db From ef43f5b84dbccc4826eed842c48f17ff86f4b235 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral <72917414+ashwini-orchestral@users.noreply.github.com> Date: Wed, 29 Sep 2021 18:28:40 +0530 Subject: [PATCH 07/40] Added new unit test and refactored existing unit tests related to clone actions --- st2common/tests/unit/services/test_packs.py | 45 +++++++++++---------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 9ccaf09bd5..a9f9821ce7 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -346,11 +346,11 @@ def test_clone_workflow(self): @mock.patch("shutil.copy") def test_permission_error_to_write_in_destination_file(self, mock_copy): mock_copy.side_effect = PermissionError("No permission to write in file") - cloned_action_entry_point_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_4.py" + cloned_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "action_4.yaml" ) expected_msg = 'No permission to write in "%s" file' % ( - cloned_action_entry_point_file_path + cloned_action_metadata_file_path ) with self.assertRaisesRegexp(PermissionError, expected_msg): @@ -365,26 +365,27 @@ def test_permission_error_to_write_in_destination_file(self, mock_copy): ) @mock.patch("shutil.copy") - def test_file_not_found_error_for_destination_file(self, mock_copy): - mock_copy.side_effect = FileNotFoundError("No such file or directory") - cloned_action_entry_point_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_5.py" - ) - expected_msg = ( - "Please make sure 'workflows' directory present in path: '%s'" - % (cloned_action_entry_point_file_path) + def test_workflows_directory_created_if_does_not_exist(self, mock_copy): + action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + workflow_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions", "workflows") + for file in os.listdir(workflow_files_path): + if os.path.isfile(os.path.join(workflow_files_path, file)): + os.remove(os.path.join(workflow_files_path, file)) + # removing workflows directory and asserting it doesn't exist + os.rmdir(workflow_files_path) + self.assertFalse(os.path.exists(workflow_files_path)) + self.assertTrue(os.path.exists(action_files_path)) + clone_action( + TEST_SOURCE_WORKFLOW_PACK_PATH, + "actions/data-flow.yaml", + "workflows/data-flow.yaml", + "orquesta", + TEST_DEST_PACK_PATH, + TEST_DEST_PACK, + "workflow_1", ) - - with self.assertRaisesRegexp(FileNotFoundError, expected_msg): - clone_action( - TEST_SOURCE_PACK_PATH, - "actions/inject_trigger.yaml", - "inject_trigger.py", - "python-script", - TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "action_5", - ) + # workflows directory created and asserting it exists + self.assertTrue(os.path.exists(workflow_files_path)) @mock.patch("shutil.copy") def test_exceptions_to_write_in_destination_file(self, mock_copy): From 63395cc53fc075a24fc3a55a5c5664c6a552de88 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral <72917414+ashwini-orchestral@users.noreply.github.com> Date: Wed, 29 Sep 2021 18:39:35 +0530 Subject: [PATCH 08/40] Refactored action clone API method to reuse existing create and delete action functionalities and done other minor fixes --- st2api/st2api/controllers/v1/actions.py | 102 ++++++++++++------------ 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 3a9d6a3a59..c35ac638b7 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -17,6 +17,7 @@ import os.path import stat import errno +from collections import namedtuple import six from mongoengine import ValidationError @@ -299,25 +300,47 @@ def clone(self, dest_data, ref_or_id, requester_user): LOG.debug("Clone source action object found: %s", source_action_db) - permission_type = PermissionType.ACTION_VIEW - rbac_utils = get_rbac_backend().get_utils_class() - rbac_utils.assert_user_has_resource_db_permission( - user_db=requester_user, - resource_db=source_action_db, - permission_type=permission_type, + try: + permission_type = PermissionType.ACTION_VIEW + rbac_utils = get_rbac_backend().get_utils_class() + rbac_utils.assert_user_has_resource_db_permission( + user_db=requester_user, + resource_db=source_action_db, + permission_type=permission_type, + ) + except ResourceAccessDeniedError as e: + abort(http_client.UNAUTHORIZED, six.text_type(e)) + + cloned_dest_action_db = clone_action_db( + source_action_db=source_action_db, + dest_pack=dest_data.dest_pack, + dest_action=dest_data.dest_action, ) - source_pack = source_action_db["pack"] - source_entry_point = source_action_db["entry_point"] - source_runner_type = source_action_db["runner_type"]["name"] - source_metadata_file = source_action_db["metadata_file"] - source_pack_base_path = get_pack_base_path(pack_name=source_pack) + cloned_action_api = ActionAPI.from_model(cloned_dest_action_db) + + try: + permission_type = PermissionType.ACTION_CREATE + rbac_utils.assert_user_has_resource_api_permission( + user_db=requester_user, + resource_api=cloned_action_api, + permission_type=permission_type, + ) + except ResourceAccessDeniedError as e: + abort(http_client.UNAUTHORIZED, six.text_type(e)) + dest_pack_base_path = get_pack_base_path(pack_name=dest_data.dest_pack) if not os.path.isdir(dest_pack_base_path): msg = "Destination pack '%s' doesn't exist" % (dest_data.dest_pack) abort(http_client.BAD_REQUEST, six.text_type(msg)) + source_pack = source_action_db["pack"] + source_entry_point = source_action_db["entry_point"] + source_runner_type = source_action_db["runner_type"]["name"] + source_metadata_file = source_action_db["metadata_file"] + source_pack_base_path = get_pack_base_path(pack_name=source_pack) + dest_pack_base_path = get_pack_base_path(pack_name=dest_data.dest_pack) dest_ref = ".".join([dest_data.dest_pack, dest_data.dest_action]) dest_action_db = self._get_by_ref(resource_ref=dest_ref) @@ -332,22 +355,15 @@ def clone(self, dest_data, ref_or_id, requester_user): abort(http_client.BAD_REQUEST, six.text_type(msg)) try: - permission_type = PermissionType.ACTION_CREATE - rbac_utils = get_rbac_backend().get_utils_class() - rbac_utils.assert_user_has_resource_db_permission( - user_db=requester_user, - resource_db=dest_action_db, - permission_type=permission_type, - ) - permission_type = PermissionType.ACTION_DELETE - rbac_utils = get_rbac_backend().get_utils_class() rbac_utils.assert_user_has_resource_db_permission( user_db=requester_user, resource_db=dest_action_db, permission_type=permission_type, ) - self.delete(dest_ref, requester_user) + Data = namedtuple("Data", ["remove_files"]) + options = Data(True) + self.delete(options, dest_ref, requester_user) except ResourceAccessDeniedError as e: abort(http_client.UNAUTHORIZED, six.text_type(e)) except Exception as e: @@ -359,18 +375,6 @@ def clone(self, dest_data, ref_or_id, requester_user): abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) try: - cloned_dest_action_db = clone_action_db( - source_action_db=source_action_db, - dest_pack=dest_data.dest_pack, - dest_action=dest_data.dest_action, - ) - permission_type = PermissionType.ACTION_CREATE - rbac_utils = get_rbac_backend().get_utils_class() - rbac_utils.assert_user_has_resource_db_permission( - user_db=requester_user, - resource_db=cloned_dest_action_db, - permission_type=permission_type, - ) clone_action( source_pack_base_path=source_pack_base_path, source_metadata_file=source_metadata_file, @@ -380,31 +384,31 @@ def clone(self, dest_data, ref_or_id, requester_user): dest_pack=dest_data.dest_pack, dest_action=dest_data.dest_action, ) + + post_response = self.post(cloned_action_api, requester_user) + if post_response.status_code != http_client.CREATED: + raise Exception("Could not add cloned action to database.") + + extra = {"cloned_acion_db": cloned_dest_action_db} + LOG.audit( + "Action cloned. Action.id=%s" % (cloned_dest_action_db.id), extra=extra + ) + return post_response except PermissionError as e: - LOG.error("No permission to clone action in destination pack.") + LOG.error("No permission to clone the action. Exception was %s", e) abort(http_client.FORBIDDEN, six.text_type(e)) - except ResourceAccessDeniedError as e: - abort(http_client.UNAUTHORIZED, six.text_type(e)) except Exception as e: LOG.error( "Exception encountered during cloning action. Exception was %s", e, ) + delete_action_files_from_pack( + pack_name=cloned_dest_action_db["pack"], + entry_point=cloned_dest_action_db["entry_point"], + metadata_file=cloned_dest_action_db["metadata_file"], + ) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) - try: - cloned_dest_action_db = Action.add_or_update(cloned_dest_action_db) - except Exception as e: - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) - - extra = {"cloned_acion_db": cloned_dest_action_db} - LOG.audit( - "Action cloned. Action.id=%s" % (cloned_dest_action_db.id), extra=extra - ) - cloned_action_api = ActionAPI.from_model(cloned_dest_action_db) - - return Response(json=cloned_action_api, status=http_client.CREATED) - def _handle_data_files(self, pack_ref, data_files): """ Method for handling action data files. From d34444822e50f224714369e890b6cd3c42d58284 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral <72917414+ashwini-orchestral@users.noreply.github.com> Date: Wed, 29 Sep 2021 18:49:02 +0530 Subject: [PATCH 09/40] Minor fixes in existing unit tests --- .../tests/unit/controllers/v1/test_actions.py | 78 +++++++++++++------ 1 file changed, 55 insertions(+), 23 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index bd76da6a14..0a9ae568cb 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -267,11 +267,11 @@ "description": "test description", "enabled": True, "pack": "sourcepack", - "entry_point": "/tmp/test/source_action.sh", - "runner_type": "local-shell-script", + "entry_point": "/tmp/test/source_action.py", + "runner_type": "python-script", "parameters": { - "x": {"type": "string", "default": "A1"}, - "y": {"type": "string", "default": "B1"}, + "x": {"type": "string", "default": "X1"}, + "y": {"type": "string", "default": "Y1"}, }, "tags": [ {"name": "tag1", "value": "dont-care1"}, @@ -287,13 +287,8 @@ "entry_point": "/tmp/test/clone_action.sh", "runner_type": "local-shell-script", "parameters": { - "x": {"type": "string", "default": "A1"}, - "y": {"type": "string", "default": "B1"}, + "a": {"type": "string", "default": "A1"}, }, - "tags": [ - {"name": "tag1", "value": "dont-care1"}, - {"name": "tag2", "value": "dont-care2"}, - ], } ACTION_WITH_NOTIFY = { @@ -894,17 +889,16 @@ def test_delete_action_belonging_to_system_pack(self): def test_clone(self, mock_clone_action): source_post_resp = self.__do_post(ACTION_16) self.assertEqual(source_post_resp.status_int, 201) - dest_post_resp = self.__do_post(ACTION_17) - self.assertEqual(dest_post_resp.status_int, 201) dest_data_body = { "dest_pack": ACTION_17["pack"], - "dest_action": "clone_action_2", + "dest_action": ACTION_17["name"], } source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) self.assertEqual(clone_resp.status_int, 201) + get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_17["name"]) + self.assertEqual(get_resp.status_int, 200) self.__do_delete(self.__get_action_id(source_post_resp)) - self.__do_delete(self.__get_action_id(dest_post_resp)) self.__do_delete(self.__get_action_id(clone_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @@ -925,6 +919,15 @@ def test_clone_overwrite(self, mock_clone_action): source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) self.assertEqual(clone_resp.status_int, 201) + get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_17["name"]) + expected_params_dict = { + "x": {"type": "string", "default": "X1"}, + "y": {"type": "string", "default": "Y1"}, + } + actual_prams_dict = get_resp.json[0]["parameters"] + self.assertDictEqual(actual_prams_dict, expected_params_dict) + actual_runner_type = get_resp.json[0]["runner_type"] + self.assertNotEqual(actual_runner_type, ACTION_17["runner_type"]) self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(self.__get_action_id(clone_resp)) @@ -973,7 +976,7 @@ def test_clone_destination_pack_does_not_exist(self, mock_clone_action): @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_destination_already_exist(self, mock_clone_action): + def test_clone_destination_action_already_exist(self, mock_clone_action): source_post_resp = self.__do_post(ACTION_16) dest_post_resp = self.__do_post(ACTION_17) dest_data_body = { @@ -992,6 +995,31 @@ def test_clone_destination_already_exist(self, mock_clone_action): self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(self.__get_action_id(dest_post_resp)) + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) + @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_clone_permission_error(self, mock_clone_action): + msg = "No permission to access the files for cloning operation" + mock_clone_action.side_effect = PermissionError(msg) + source_post_resp = self.__do_post(ACTION_16) + dest_post_resp = self.__do_post(ACTION_17) + dest_data_body = { + "dest_pack": ACTION_17["pack"], + "dest_action": "clone_action_3", + } + source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) + clone_resp = self.__do_clone( + dest_data_body, + source_ref_or_id, + expect_errors=True, + ) + self.assertEqual(clone_resp.status_int, 403) + self.assertEqual(clone_resp.json["faultstring"], msg) + self.__do_delete(self.__get_action_id(source_post_resp)) + self.__do_delete(self.__get_action_id(dest_post_resp)) + @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @mock.patch("st2api.controllers.v1.actions.clone_action") @mock.patch.object( @@ -1004,7 +1032,7 @@ def test_clone_exception(self, mock_clone_action): dest_post_resp = self.__do_post(ACTION_17) dest_data_body = { "dest_pack": ACTION_17["pack"], - "dest_action": "clone_action_3", + "dest_action": "clone_action_4", } source_ref_or_id = "%s.%s" % (ACTION_16["pack"], ACTION_16["name"]) clone_resp = self.__do_clone( @@ -1018,15 +1046,15 @@ def test_clone_exception(self, mock_clone_action): self.__do_delete(self.__get_action_id(dest_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action_db") + @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_overwrite_exception_cloning_db_action_reregistered( - self, mock_clone_action_db + def test_clone_overwrite_exception_destination_recovered( + self, mock_clone_overwrite ): - msg = "Exception encountered during overwriting resource." - mock_clone_action_db.side_effect = Exception(msg) + msg = "Exception encountered during overwriting action." + mock_clone_overwrite.side_effect = Exception(msg) source_post_resp = self.__do_post(ACTION_16) self.__do_post(ACTION_17) dest_data_body = { @@ -1041,9 +1069,13 @@ def test_clone_overwrite_exception_cloning_db_action_reregistered( expect_errors=True, ) self.assertEqual(clone_resp.status_int, 500) - self.assertEqual(clone_resp.json["faultstring"], msg) - + dest_get_resp = self.__do_get_actions_by_url_parameter( + "name", ACTION_17["name"] + ) + self.assertEqual(dest_get_resp.status_int, 200) + self.assertEqual(dest_get_resp.json[0]["runner_type"], ACTION_17["runner_type"]) self.__do_delete(self.__get_action_id(source_post_resp)) + self.__do_delete(dest_get_resp.json[0]["id"]) def _insert_mock_models(self): action_1_id = self.__get_action_id(self.__do_post(ACTION_1)) From 0eb5dce3c945f207231e2939cb1c03bcad2ef1a6 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral <72917414+ashwini-orchestral@users.noreply.github.com> Date: Wed, 29 Sep 2021 19:01:43 +0530 Subject: [PATCH 10/40] Minor fix in unit test related to clone response error code --- st2api/tests/unit/controllers/v1/test_actions_rbac.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions_rbac.py b/st2api/tests/unit/controllers/v1/test_actions_rbac.py index a5a50cc894..e178546fd4 100644 --- a/st2api/tests/unit/controllers/v1/test_actions_rbac.py +++ b/st2api/tests/unit/controllers/v1/test_actions_rbac.py @@ -296,7 +296,7 @@ def test_clone_action_no_source_action_view_permission(self): 'User "no_permissions" doesn\'t have required permission "action_view" ' 'on resource "action:examples:ma.dummy.action"' ) - self.assertEqual(clone_resp.status_code, http_client.FORBIDDEN) + self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) self.assertEqual(clone_resp.json["faultstring"], expected_msg) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) From d5003a4e841b496dbeb35e66a08a7c497c42cff3 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral <72917414+ashwini-orchestral@users.noreply.github.com> Date: Mon, 4 Oct 2021 11:33:43 +0530 Subject: [PATCH 11/40] Update st2client/st2client/commands/action.py to deduplicate `self.manager.clone` Co-authored-by: Jacob Floyd --- st2client/st2client/commands/action.py | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index a994e7f1ab..6e2478fe13 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -348,6 +348,8 @@ def run(self, args, **kwargs): except ResourceNotFoundError: dest_instance = None + overwrite = False + if dest_instance: user_input = "" if not args.force: @@ -355,24 +357,18 @@ def run(self, args, **kwargs): "The destination action already exists. Do you want to overwrite? (y/n): " ) if args.force or user_input.lower() == "y" or user_input.lower() == "yes": - return self.manager.clone( - source_ref, - dest_pack, - dest_action, - overwrite=True, - **kwargs, - ) + overwrite = True else: print("Action is not cloned.") return - else: - return self.manager.clone( - source_ref, - dest_pack, - dest_action, - overwrite=False, - **kwargs, - ) + + return self.manager.clone( + source_ref, + dest_pack, + dest_action, + overwrite=overwrite, + **kwargs, + ) def run_and_print(self, args, **kwargs): try: From 0ff59a10c2ec3a6d80f379f01dc7ad6672e5d013 Mon Sep 17 00:00:00 2001 From: W Chan Date: Tue, 5 Oct 2021 18:59:37 +0000 Subject: [PATCH 12/40] Move local generic body class to public GenericRequestParam Move the local generic body class that is private to the Router class and rename it to GenericRequestParam and make it public so that the class can be used when calling specific API handler explicitly. --- st2common/st2common/router.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/st2common/st2common/router.py b/st2common/st2common/router.py index fa9c002354..36a9476ed6 100644 --- a/st2common/st2common/router.py +++ b/st2common/st2common/router.py @@ -145,6 +145,11 @@ class NotFoundException(Exception): pass +class GenericRequestParam(object): + def __init__(self, **entries): + self.__dict__.update(entries) + + class Request(webob.Request): """ Custom Request implementation which uses our custom and faster json serializer and deserializer. @@ -518,11 +523,6 @@ def __call__(self, req): if content_type == "text/plain": kw[argument_name] = data else: - - class Body(object): - def __init__(self, **entries): - self.__dict__.update(entries) - ref = schema.get("$ref", None) if ref: with self.spec_resolver.resolving(ref) as resolved: @@ -553,10 +553,10 @@ def __init__(self, **entries): ) else: LOG.debug( - "Missing x-api-model definition for %s, using generic Body " + "Missing x-api-model definition for %s, using GenericRequestParam " "model." % (endpoint["operationId"]) ) - model = Body + model = GenericRequestParam instance = self._get_model_instance(model_cls=model, data=data) kw[argument_name] = instance From d32b1d0aaeedd408cd37e034d5b52f3570785f79 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 8 Oct 2021 17:40:02 +0530 Subject: [PATCH 13/40] For options (i.e. `remove_files` flag) using GenericRequestParam class from st2common/router.py instead of namedtuple and other minor fixes --- st2api/st2api/controllers/v1/actions.py | 51 ++++++++++++++++--------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index c35ac638b7..8cdbfa60a6 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -17,7 +17,6 @@ import os.path import stat import errno -from collections import namedtuple import six from mongoengine import ValidationError @@ -39,6 +38,7 @@ from st2common.rbac.types import PermissionType from st2common.rbac.backends import get_rbac_backend from st2common.router import abort +from st2common.router import GenericRequestParam from st2common.router import Response from st2common.validators.api.misc import validate_not_part_of_system_pack from st2common.validators.api.misc import validate_not_part_of_system_pack_by_name @@ -46,8 +46,11 @@ 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.services.packs import clone_action +from st2common.services.packs import clone_action_files from st2common.services.packs import clone_action_db +from st2common.services.packs import temp_backup_action_files +from st2common.services.packs import remove_temp_action_files +from st2common.services.packs import restore_temp_action_files from st2common.transport.reactor import TriggerDispatcher from st2common.util.system_info import get_host_info import st2common.validators.api.action as action_validator @@ -298,7 +301,10 @@ def clone(self, dest_data, ref_or_id, requester_user): msg = "The requested source for cloning operation doesn't exists" abort(http_client.BAD_REQUEST, six.text_type(msg)) - LOG.debug("Clone source action object found: %s", source_action_db) + extra = {"action_db": source_action_db} + LOG.audit( + "Source action found. Action.id=%s" % (source_action_db.id), extra=extra + ) try: permission_type = PermissionType.ACTION_VIEW @@ -335,11 +341,6 @@ def clone(self, dest_data, ref_or_id, requester_user): msg = "Destination pack '%s' doesn't exist" % (dest_data.dest_pack) abort(http_client.BAD_REQUEST, six.text_type(msg)) - source_pack = source_action_db["pack"] - source_entry_point = source_action_db["entry_point"] - source_runner_type = source_action_db["runner_type"]["name"] - source_metadata_file = source_action_db["metadata_file"] - source_pack_base_path = get_pack_base_path(pack_name=source_pack) dest_pack_base_path = get_pack_base_path(pack_name=dest_data.dest_pack) dest_ref = ".".join([dest_data.dest_pack, dest_data.dest_action]) dest_action_db = self._get_by_ref(resource_ref=dest_ref) @@ -361,8 +362,12 @@ def clone(self, dest_data, ref_or_id, requester_user): resource_db=dest_action_db, permission_type=permission_type, ) - Data = namedtuple("Data", ["remove_files"]) - options = Data(True) + options = GenericRequestParam(remove_files=True) + dest_metadata_file = dest_action_db["metadata_file"] + dest_entry_point = dest_action_db["entry_point"] + temp_backup_action_files( + dest_pack_base_path, dest_metadata_file, dest_entry_point + ) self.delete(options, dest_ref, requester_user) except ResourceAccessDeniedError as e: abort(http_client.UNAUTHORIZED, six.text_type(e)) @@ -375,14 +380,10 @@ def clone(self, dest_data, ref_or_id, requester_user): abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) try: - clone_action( - source_pack_base_path=source_pack_base_path, - source_metadata_file=source_metadata_file, - source_entry_point=source_entry_point, - source_runner_type=source_runner_type, + clone_action_files( + source_action_db=source_action_db, + dest_action_db=cloned_dest_action_db, dest_pack_base_path=dest_pack_base_path, - dest_pack=dest_data.dest_pack, - dest_action=dest_data.dest_action, ) post_response = self.post(cloned_action_api, requester_user) @@ -393,9 +394,18 @@ def clone(self, dest_data, ref_or_id, requester_user): LOG.audit( "Action cloned. Action.id=%s" % (cloned_dest_action_db.id), extra=extra ) + if dest_action_db: + remove_temp_action_files(dest_pack_base_path) return post_response except PermissionError as e: LOG.error("No permission to clone the action. Exception was %s", e) + if dest_action_db: + restore_temp_action_files( + dest_pack_base_path, dest_metadata_file, dest_entry_point + ) + dest_action_db.id = None + Action.add_or_update(dest_action_db) + remove_temp_action_files(dest_pack_base_path) abort(http_client.FORBIDDEN, six.text_type(e)) except Exception as e: LOG.error( @@ -407,6 +417,13 @@ def clone(self, dest_data, ref_or_id, requester_user): entry_point=cloned_dest_action_db["entry_point"], metadata_file=cloned_dest_action_db["metadata_file"], ) + if dest_action_db: + restore_temp_action_files( + dest_pack_base_path, dest_metadata_file, dest_entry_point + ) + dest_action_db.id = None + Action.add_or_update(dest_action_db) + remove_temp_action_files(dest_pack_base_path) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) def _handle_data_files(self, pack_ref, data_files): From b9484c703476303d49645d821c439c067bffeaa8 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 8 Oct 2021 17:42:59 +0530 Subject: [PATCH 14/40] Refactoring unit tests regarding cleanup partially cloned actions and restoring original dest actions --- .../tests/unit/controllers/v1/test_actions.py | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index 0a9ae568cb..4963d3211e 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -882,7 +882,7 @@ def test_delete_action_belonging_to_system_pack(self): self.assertEqual(del_resp.status_int, 400) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch("st2api.controllers.v1.actions.clone_action_files") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) @@ -898,15 +898,20 @@ def test_clone(self, mock_clone_action): self.assertEqual(clone_resp.status_int, 201) get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_17["name"]) self.assertEqual(get_resp.status_int, 200) + self.assertTrue(mock_clone_action.called) self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(self.__get_action_id(clone_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch("st2api.controllers.v1.actions.remove_temp_action_files") + @mock.patch("st2api.controllers.v1.actions.temp_backup_action_files") + @mock.patch("st2api.controllers.v1.actions.clone_action_files") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_overwrite(self, mock_clone_action): + def test_clone_overwrite( + self, mock_clone_action, mock_temp_backup, mock_clean_backup + ): source_post_resp = self.__do_post(ACTION_16) self.assertEqual(source_post_resp.status_int, 201) dest_post_resp = self.__do_post(ACTION_17) @@ -928,14 +933,16 @@ def test_clone_overwrite(self, mock_clone_action): self.assertDictEqual(actual_prams_dict, expected_params_dict) actual_runner_type = get_resp.json[0]["runner_type"] self.assertNotEqual(actual_runner_type, ACTION_17["runner_type"]) + self.assertTrue(mock_clone_action.called) + self.assertTrue(mock_temp_backup.called) + self.assertTrue(mock_clean_backup.called) self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(self.__get_action_id(clone_resp)) - @mock.patch("st2api.controllers.v1.actions.clone_action") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_source_does_not_exist(self, mock_clone_action): + def test_clone_source_does_not_exist(self): dest_data_body = { "dest_pack": ACTION_17["pack"], "dest_action": ACTION_17["name"], @@ -950,11 +957,10 @@ def test_clone_source_does_not_exist(self, mock_clone_action): msg = 'Resource with a reference or id "%s" not found' % source_ref_or_id self.assertEqual(clone_resp.json["faultstring"], msg) - @mock.patch("st2api.controllers.v1.actions.clone_action") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_destination_pack_does_not_exist(self, mock_clone_action): + def test_clone_destination_pack_does_not_exist(self): source_post_resp = self.__do_post(ACTION_16) dest_data_body = { "dest_pack": ACTION_17["pack"], @@ -972,11 +978,10 @@ def test_clone_destination_pack_does_not_exist(self, mock_clone_action): self.__do_delete(self.__get_action_id(source_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_destination_action_already_exist(self, mock_clone_action): + def test_clone_destination_action_already_exist(self): source_post_resp = self.__do_post(ACTION_16) dest_post_resp = self.__do_post(ACTION_17) dest_data_body = { @@ -996,7 +1001,7 @@ def test_clone_destination_action_already_exist(self, mock_clone_action): self.__do_delete(self.__get_action_id(dest_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch("st2api.controllers.v1.actions.clone_action_files") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) @@ -1021,11 +1026,12 @@ def test_clone_permission_error(self, mock_clone_action): self.__do_delete(self.__get_action_id(dest_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") + @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") + @mock.patch("st2api.controllers.v1.actions.clone_action_files") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_clone_exception(self, mock_clone_action): + def test_clone_exception(self, mock_clone_action, mock_delete_files): msg = "Exception encountered during cloning action." mock_clone_action.side_effect = Exception(msg) source_post_resp = self.__do_post(ACTION_16) @@ -1042,16 +1048,27 @@ def test_clone_exception(self, mock_clone_action): ) self.assertEqual(clone_resp.status_int, 500) self.assertEqual(clone_resp.json["faultstring"], msg) + # asserting delete_action_files_from_pack function called i.e. cloned files are cleaned up + self.assertTrue(mock_delete_files.called) self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(self.__get_action_id(dest_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") + @mock.patch("st2api.controllers.v1.actions.remove_temp_action_files") + @mock.patch("st2api.controllers.v1.actions.restore_temp_action_files") + @mock.patch("st2api.controllers.v1.actions.temp_backup_action_files") + @mock.patch("st2api.controllers.v1.actions.clone_action_files") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) def test_clone_overwrite_exception_destination_recovered( - self, mock_clone_overwrite + self, + mock_clone_overwrite, + mock_backup_files, + mock_restore_files, + mock_remove_backup, + mock_clean_files, ): msg = "Exception encountered during overwriting action." mock_clone_overwrite.side_effect = Exception(msg) @@ -1074,6 +1091,14 @@ def test_clone_overwrite_exception_destination_recovered( ) self.assertEqual(dest_get_resp.status_int, 200) self.assertEqual(dest_get_resp.json[0]["runner_type"], ACTION_17["runner_type"]) + # asserting temp_backup_action_files function called + self.assertTrue(mock_backup_files.called) + # asserting restore_temp_action_files called i.e. original ACTION_17 restored + self.assertTrue(mock_restore_files.called) + # asserting remove_temp_action_files function called + self.assertTrue(mock_remove_backup.called) + # asserting delete_action_files_from_pack called i.e. cloned files are cleaned up + self.assertTrue(mock_clean_files.called) self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(dest_get_resp.json[0]["id"]) From 7a55c94dca6bb43dfdca7dcf54f757c8b5249dd5 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 8 Oct 2021 17:46:11 +0530 Subject: [PATCH 15/40] Added functions related to backup destination action files in case of overwrite and restoring them back to destination in case of exception --- st2common/st2common/services/packs.py | 122 ++++++++++++++++++++------ 1 file changed, 95 insertions(+), 27 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 7a23642ab9..3afb20afa8 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -29,6 +29,7 @@ from st2common import log as logging from st2common.content.utils import get_pack_base_path from st2common.exceptions.content import ResourceDiskFilesRemovalError +from st2common.models.db.stormbase import UIDFieldMixin from st2common.persistence.pack import Pack from st2common.util.misc import lowercase_value from st2common.util.jsonify import json_encode @@ -319,62 +320,53 @@ def _clone_content_to_destination_file(source_file, destination_file): raise Exception(msg) -def clone_action( - source_pack_base_path, - source_metadata_file, - source_entry_point, - source_runner_type, - dest_pack_base_path, - dest_pack, - dest_action, -): +def clone_action_files(source_action_db, dest_action_db, dest_pack_base_path): """ Prepares the path for entry point and metadata files for source and destination. Clones the content from source action files to destination action files. """ + source_pack = source_action_db["pack"] + source_entry_point = source_action_db["entry_point"] + source_metadata_file = source_action_db["metadata_file"] + source_pack_base_path = get_pack_base_path(pack_name=source_pack) source_metadata_file_path = os.path.join( source_pack_base_path, source_metadata_file ) - dest_metadata_file_name = "%s.yaml" % (dest_action) - dest_metadata_file_path = os.path.join( - dest_pack_base_path, "actions", dest_metadata_file_name - ) + dest_metadata_file_name = dest_action_db["metadata_file"] + dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file_name) + _clone_content_to_destination_file( source_file=source_metadata_file_path, destination_file=dest_metadata_file_path ) - if source_entry_point: - if source_runner_type in ["orquesta", "action-chain"]: - dest_entry_point_file_name = "workflows/%s.yaml" % (dest_action) + dest_entry_point = dest_action_db["entry_point"] + dest_runner_type = dest_action_db["runner_type"]["name"] + + if dest_entry_point: + if dest_runner_type in ["orquesta", "action-chain"]: # creating workflows directory if doesn't exist wf_dir_path = os.path.join(dest_pack_base_path, "actions", "workflows") if not os.path.isdir(wf_dir_path): os.mkdir(path=wf_dir_path) - else: - old_ext = os.path.splitext(source_entry_point)[1] - dest_entry_point_file_name = dest_action + old_ext - source_entry_point_file_path = os.path.join( source_pack_base_path, "actions", source_entry_point ) dest_entrypoint_file_path = os.path.join( - dest_pack_base_path, "actions", dest_entry_point_file_name + dest_pack_base_path, "actions", dest_entry_point ) _clone_content_to_destination_file( source_file=source_entry_point_file_path, destination_file=dest_entrypoint_file_path, ) - else: - dest_entry_point_file_name = "" with open(dest_metadata_file_path) as df: doc = yaml.load(df, Loader=yaml.FullLoader) - doc["name"] = dest_action + doc["name"] = dest_action_db["name"] if "pack" in doc: - doc["pack"] = dest_pack - doc["entry_point"] = dest_entry_point_file_name + doc["pack"] = dest_action_db["pack"] + doc["entry_point"] = dest_entry_point with open(dest_metadata_file_path, "w") as df: yaml.dump(doc, df, default_flow_style=False, sort_keys=False) @@ -396,9 +388,85 @@ def clone_action_db(source_action_db, dest_pack, dest_action): dest_action_db["name"] = dest_action dest_ref = ".".join([dest_pack, dest_action]) dest_action_db["ref"] = dest_ref - dest_action_db["uid"] = "action:%s:%s" % (dest_pack, dest_action) + dest_action_db["uid"] = UIDFieldMixin.UID_SEPARATOR.join( + ["action", dest_pack, dest_action] + ) if "pack" in dest_action_db: dest_action_db["pack"] = dest_pack dest_action_db["id"] = None return dest_action_db + + +def temp_backup_action_files(dest_pack_base_path, dest_metadata_file, dest_entry_point): + temp_dir_path = os.path.join(dest_pack_base_path, "temp_dir") + os.mkdir(temp_dir_path) + temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") + dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file) + _clone_content_to_destination_file( + source_file=dest_metadata_file_path, destination_file=temp_metadata_file_path + ) + if dest_entry_point: + old_ext = os.path.splitext(dest_entry_point)[1] + temp_entry_point_file_name = "temp_entry_point_file" + old_ext + temp_entry_point_file_path = os.path.join( + temp_dir_path, temp_entry_point_file_name + ) + dest_entry_point_file_path = os.path.join( + dest_pack_base_path, "actions", dest_entry_point + ) + _clone_content_to_destination_file( + source_file=dest_entry_point_file_path, + destination_file=temp_entry_point_file_path, + ) + + +def restore_temp_action_files( + dest_pack_base_path, dest_metadata_file, dest_entry_point +): + temp_dir_path = os.path.join(dest_pack_base_path, "temp_dir") + temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") + dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file) + _clone_content_to_destination_file( + source_file=temp_metadata_file_path, destination_file=dest_metadata_file_path + ) + if dest_entry_point: + old_ext = os.path.splitext(dest_entry_point)[1] + temp_entry_point_file_name = "temp_entry_point_file" + old_ext + temp_entry_point_file_path = os.path.join( + temp_dir_path, temp_entry_point_file_name + ) + dest_entry_point_file_path = os.path.join( + dest_pack_base_path, "actions", dest_entry_point + ) + _clone_content_to_destination_file( + source_file=temp_entry_point_file_path, + destination_file=dest_entry_point_file_path, + ) + + +def remove_temp_action_files(dest_pack_base_path): + temp_dir_path = os.path.join(dest_pack_base_path, "temp_dir") + + if os.path.isdir(temp_dir_path): + try: + shutil.rmtree(temp_dir_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" directory', + temp_dir_path, + ) + msg = 'No permission to delete the "%s" directory' % (temp_dir_path) + raise PermissionError(msg) + except Exception as e: + LOG.error( + 'Unable to delete "%s" directory. Exception was "%s"', + temp_dir_path, + e, + ) + msg = ( + 'The temporary directory "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the temporary directory manually" % (temp_dir_path) + ) + raise Exception(msg) From 57eb5fec2b4df1aec1ce3b175c6eaf5a4bf0f30c Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 8 Oct 2021 17:48:30 +0530 Subject: [PATCH 16/40] Added unit tests for newly added function for restoring destination action files in case of exception during clone overwrite and done other minor fixes --- st2common/tests/unit/services/test_packs.py | 473 ++++++++++++++++---- 1 file changed, 387 insertions(+), 86 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index a9f9821ce7..68254b8225 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -19,12 +19,18 @@ import os import mock +import shutil import unittest2 import st2tests +from st2common.models.db.stormbase import UIDFieldMixin from st2common.services.packs import delete_action_files_from_pack -from st2common.services.packs import clone_action +from st2common.services.packs import clone_action_files +from st2common.services.packs import clone_action_db +from st2common.services.packs import temp_backup_action_files +from st2common.services.packs import restore_temp_action_files +from st2common.services.packs import remove_temp_action_files TEST_PACK = "dummy_pack_1" TEST_PACK_PATH = os.path.join( @@ -32,20 +38,164 @@ ) TEST_SOURCE_PACK = "core" -TEST_SOURCE_PACK_PATH = os.path.join( - st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_SOURCE_PACK -) TEST_SOURCE_WORKFLOW_PACK = "orquesta_tests" -TEST_SOURCE_WORKFLOW_PACK_PATH = os.path.join( - st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_SOURCE_WORKFLOW_PACK -) TEST_DEST_PACK = "dummy_pack_23" TEST_DEST_PACK_PATH = os.path.join( st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_DEST_PACK ) +SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER = { + "description": "Action which injects a new trigger in the system.", + "enabled": True, + "entry_point": "inject_trigger.py", + "metadata_file": "actions/inject_trigger.yaml", + "name": "inject_trigger", + "notify": {}, + "output_schema": {}, + "pack": TEST_SOURCE_PACK, + "parameters": { + "trigger": { + "type": "string", + "description": "Trigger reference (e.g. mypack.my_trigger).", + "required": True, + }, + "payload": {"type": "object", "description": "Trigger payload."}, + "trace_tag": { + "type": "string", + "description": "Optional trace tag.", + "required": False, + }, + }, + "ref": "core.inject_trigger", + "runner_type": {"name": "python-script"}, + "tags": [], + "uid": "action:core:inject_trigger", +} + +SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER = { + "description": "This sends an email", + "enabled": True, + "entry_point": "send_mail/send_mail", + "metadata_file": "actions/sendmail.yaml", + "name": "sendmail", + "notify": {}, + "output_schema": {}, + "pack": TEST_SOURCE_PACK, + "parameters": { + "sendmail_binary": { + "description": "Optional path to the sendmail binary. If not provided, it uses a system default one.", + "position": 0, + "required": False, + "type": "string", + "default": "None", + }, + "from": { + "description": "Sender email address.", + "position": 1, + "required": False, + "type": "string", + "default": "stanley", + }, + "to": { + "description": "Recipient email address.", + "position": 2, + "required": True, + "type": "string", + }, + "subject": { + "description": "Subject of the email.", + "position": 3, + "required": True, + "type": "string", + }, + "send_empty_body": { + "description": "Send a message even if the body is empty.", + "position": 4, + "required": False, + "type": "boolean", + "default": True, + }, + "content_type": { + "type": "string", + "description": "Content type of message to be sent without the charset (charset is set to UTF-8 inside the script).", + "default": "text/html", + "position": 5, + }, + "body": { + "description": "Body of the email.", + "position": 6, + "required": True, + "type": "string", + }, + "sudo": {"immutable": True}, + "attachments": { + "description": "Array of attachment file paths, comma-delimited.", + "position": 7, + "required": False, + "type": "string", + }, + }, + "ref": "core.sendmail", + "runner_type": {"name": "local-shell-script"}, + "tags": [], + "uid": "action:core:sendmail", +} + +SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER = { + "description": "Action that executes the Linux echo command on the localhost.", + "enabled": True, + "entry_point": "", + "metadata_file": "actions/echo.yaml", + "name": "echo", + "notify": {}, + "output_schema": {}, + "pack": TEST_SOURCE_PACK, + "parameters": { + "message": { + "description": "The message that the command will echo.", + "type": "string", + "required": True, + }, + "cmd": { + "description": "Arbitrary Linux command to be executed on the local host.", + "required": True, + "type": "string", + "default": 'echo "{{message}}"', + "immutable": True, + }, + "kwarg_op": {"immutable": True}, + "sudo": {"default": False, "immutable": True}, + "sudo_password": {"immutable": True}, + }, + "ref": "core.echo", + "runner_type": {"name": "local-shell-cmd"}, + "tags": [], + "uid": "action:core:echo", +} + +SOURCE_WORKFLOW = { + "description": "A basic workflow to demonstrate data flow options.", + "enabled": True, + "entry_point": "workflows/data-flow.yaml", + "metadata_file": "actions/data-flow.yaml", + "name": "data-flow", + "notify": {}, + "output_schema": { + "a6": {"type": "string", "required": True}, + "b6": {"type": "string", "required": True}, + "a7": {"type": "string", "required": True}, + "b7": {"type": "string", "required": True, "secret": "********"}, + }, + "pack": TEST_SOURCE_WORKFLOW_PACK, + "parameters": {"a1": {"required": True, "type": "string"}}, + "ref": "orquesta_tests.data-flow", + "runner_type": {"name": "orquesta"}, + "tags": [], + "uid": "action:orquesta_tests:data-flow", +} + class DeleteActionFilesTest(unittest2.TestCase): def test_delete_action_files_from_pack(self): @@ -259,7 +409,7 @@ def test_exception_to_remove_resource_metadata_file(self, remove): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) -class CloneActionsTest(unittest2.TestCase): +class CloneActionDBAndFilesTestCase(unittest2.TestCase): @classmethod def tearDownClass(cls): action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") @@ -271,74 +421,87 @@ def tearDownClass(cls): if os.path.isfile(os.path.join(workflow_files_path, file)): os.remove(os.path.join(workflow_files_path, file)) - def test_clone_action_with_python_script_runner(self): - clone_action( - TEST_SOURCE_PACK_PATH, - "actions/inject_trigger.yaml", - "inject_trigger.py", - "python-script", - TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "action_1", + def test_clone_action_db(self): + CLONE_ACTION_1 = clone_action_db( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_1" + ) + exptected_uid = UIDFieldMixin.UID_SEPARATOR.join( + ["action", TEST_DEST_PACK, "clone_action_1"] + ) + actual_uid = CLONE_ACTION_1["uid"] + self.assertEqual(actual_uid, exptected_uid) + exptected_parameters = { + "trigger": { + "type": "string", + "description": "Trigger reference (e.g. mypack.my_trigger).", + "required": True, + }, + "payload": {"type": "object", "description": "Trigger payload."}, + "trace_tag": { + "type": "string", + "description": "Optional trace tag.", + "required": False, + }, + } + actual_parameters = CLONE_ACTION_1["parameters"] + self.assertDictEqual(actual_parameters, exptected_parameters) + + def test_clone_files_for_python_script_runner_action(self): + CLONE_ACTION_1 = clone_action_db( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_1" + ) + clone_action_files( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, CLONE_ACTION_1, TEST_DEST_PACK_PATH ) cloned_action_metadata_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_1.yaml" + TEST_DEST_PACK_PATH, "actions", "clone_action_1.yaml" ) cloned_action_entry_point_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_1.py" + TEST_DEST_PACK_PATH, "actions", "clone_action_1.py" ) self.assertTrue(os.path.exists(cloned_action_metadata_file_path)) self.assertTrue(os.path.exists(cloned_action_entry_point_file_path)) - def test_clone_action_with_shell_script_runner(self): - clone_action( - TEST_SOURCE_PACK_PATH, - "actions/sendmail.yaml", - "send_mail/send_mail", - "local-shell-script", - TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "action_2", + def test_clone_files_for_shell_script_runner_action(self): + CLONE_ACTION_2 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_2" + ) + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_2, TEST_DEST_PACK_PATH ) cloned_action_metadata_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_2.yaml" + TEST_DEST_PACK_PATH, "actions", "clone_action_2.yaml" ) cloned_action_entry_point_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_2" + TEST_DEST_PACK_PATH, "actions", "clone_action_2" ) self.assertTrue(os.path.exists(cloned_action_metadata_file_path)) self.assertTrue(os.path.exists(cloned_action_entry_point_file_path)) - def test_clone_action_with_local_shell_cmd_runner(self): - clone_action( - TEST_SOURCE_PACK_PATH, - "actions/echo.yaml", - "", - "local-shell-cmd", + def test_clone_files_for_local_shell_cmd_runner_action(self): + CLONE_ACTION_3 = clone_action_db( + SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER, TEST_DEST_PACK, "clone_action_3" + ) + clone_action_files( + SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER, + CLONE_ACTION_3, TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "action_3", ) cloned_action_metadata_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_3.yaml" + TEST_DEST_PACK_PATH, "actions", "clone_action_3.yaml" ) self.assertTrue(os.path.exists(cloned_action_metadata_file_path)) - def test_clone_workflow(self): - clone_action( - TEST_SOURCE_WORKFLOW_PACK_PATH, - "actions/data-flow.yaml", - "workflows/data-flow.yaml", - "orquesta", - TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "workflow_1", + def test_clone_files_for_workflow_action(self): + CLONE_WORKFLOW = clone_action_db( + SOURCE_WORKFLOW, TEST_DEST_PACK, "clone_workflow" ) + clone_action_files(SOURCE_WORKFLOW, CLONE_WORKFLOW, TEST_DEST_PACK_PATH) cloned_workflow_metadata_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "workflow_1.yaml" + TEST_DEST_PACK_PATH, "actions", "clone_workflow.yaml" ) cloned_workflow_entry_point_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "workflows", "workflow_1.yaml" + TEST_DEST_PACK_PATH, "actions", "workflows", "clone_workflow.yaml" ) self.assertTrue(os.path.exists(cloned_workflow_metadata_file_path)) self.assertTrue(os.path.exists(cloned_workflow_entry_point_file_path)) @@ -347,25 +510,45 @@ def test_clone_workflow(self): def test_permission_error_to_write_in_destination_file(self, mock_copy): mock_copy.side_effect = PermissionError("No permission to write in file") cloned_action_metadata_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_4.yaml" + TEST_DEST_PACK_PATH, "actions", "clone_action_4.yaml" ) expected_msg = 'No permission to write in "%s" file' % ( cloned_action_metadata_file_path ) - + CLONE_ACTION_2 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_4" + ) with self.assertRaisesRegexp(PermissionError, expected_msg): - clone_action( - TEST_SOURCE_PACK_PATH, - "actions/inject_trigger.yaml", - "inject_trigger.py", - "python-script", + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, + CLONE_ACTION_2, TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "action_4", ) @mock.patch("shutil.copy") - def test_workflows_directory_created_if_does_not_exist(self, mock_copy): + def test_exceptions_to_write_in_destination_file(self, mock_copy): + mock_copy.side_effect = Exception( + "Exception encoutntered during writing in destination action file" + ) + CLONE_ACTION_5 = clone_action_db( + SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER, TEST_DEST_PACK, "clone_action_5" + ) + cloned_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", "clone_action_5.yaml" + ) + expected_msg = ( + 'Could not copy to "%s" file, please check the logs or ask your ' + "StackStorm administrator to check and clone the actions files manually" + % cloned_action_metadata_file_path + ) + with self.assertRaisesRegexp(Exception, expected_msg): + clone_action_files( + SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER, + CLONE_ACTION_5, + TEST_DEST_PACK_PATH, + ) + + def test_workflows_directory_created_if_does_not_exist(self): action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") workflow_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions", "workflows") for file in os.listdir(workflow_files_path): @@ -375,39 +558,157 @@ def test_workflows_directory_created_if_does_not_exist(self, mock_copy): os.rmdir(workflow_files_path) self.assertFalse(os.path.exists(workflow_files_path)) self.assertTrue(os.path.exists(action_files_path)) - clone_action( - TEST_SOURCE_WORKFLOW_PACK_PATH, - "actions/data-flow.yaml", - "workflows/data-flow.yaml", - "orquesta", - TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "workflow_1", + CLONE_WORKFLOW = clone_action_db( + SOURCE_WORKFLOW, TEST_DEST_PACK, "clone_workflow" ) + clone_action_files(SOURCE_WORKFLOW, CLONE_WORKFLOW, TEST_DEST_PACK_PATH) # workflows directory created and asserting it exists self.assertTrue(os.path.exists(workflow_files_path)) - @mock.patch("shutil.copy") - def test_exceptions_to_write_in_destination_file(self, mock_copy): - mock_copy.side_effect = Exception( - "Exception encoutntered during writing in destination action file" + +class CloneActionFilesBackupTestCase(unittest2.TestCase): + def setUp(self): + temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + if os.path.isdir(temp_dir_path): + shutil.rmtree(temp_dir_path) + + @classmethod + def tearDownClass(cls): + action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + workflow_files_path = os.path.join(action_files_path, "workflows") + for file in os.listdir(action_files_path): + if os.path.isfile(os.path.join(action_files_path, file)): + os.remove(os.path.join(action_files_path, file)) + for file in os.listdir(workflow_files_path): + if os.path.isfile(os.path.join(workflow_files_path, file)): + os.remove(os.path.join(workflow_files_path, file)) + temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + if os.path.isdir(temp_dir_path): + shutil.rmtree(temp_dir_path) + + def test_temp_backup_action_files(self): + CLONE_ACTION_1 = clone_action_db( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_1" ) - cloned_action_metadata_file_path = os.path.join( - TEST_DEST_PACK_PATH, "actions", "action_6.yaml" + clone_action_files( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, CLONE_ACTION_1, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_1["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_1["entry_point"] + temp_backup_action_files( + TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + ) + temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + self.assertTrue(temp_dir_path) + temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") + self.assertTrue(os.path.exists(temp_metadata_file_path)) + temp_entry_point_file_path = os.path.join( + temp_dir_path, "temp_entry_point_file.py" + ) + self.assertTrue(os.path.exists(temp_entry_point_file_path)) + + def test_restore_temp_action_files(self): + CLONE_ACTION_2 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_2" + ) + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_2, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_2["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_2["entry_point"] + temp_backup_action_files( + TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + ) + dest_action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + # removing destination action files + for file in os.listdir(dest_action_files_path): + if os.path.isfile(os.path.join(dest_action_files_path, file)): + os.remove(os.path.join(dest_action_files_path, file)) + dest_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, dest_action_metadata_file + ) + dest_action_entry_point_file_path = os.path.join( + TEST_DEST_PACK_PATH, "actions", dest_action_entry_point_file ) + # asserting destination action files doesn't exist + self.assertFalse(os.path.isfile(dest_action_metadata_file_path)) + self.assertFalse(os.path.isfile(dest_action_entry_point_file_path)) + # restoring temp backed action files to destination + restore_temp_action_files( + TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + ) + # asserting action files restored at destination + self.assertTrue(os.path.isfile(dest_action_metadata_file_path)) + self.assertTrue(os.path.isfile(dest_action_entry_point_file_path)) + + def test_remove_temp_action_files(self): + CLONE_ACTION_3 = clone_action_db( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_3" + ) + clone_action_files( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, CLONE_ACTION_3, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_3["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_3["entry_point"] + temp_backup_action_files( + TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + ) + temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") + temp_entry_point_file_path = os.path.join( + temp_dir_path, "temp_entry_point_file.py" + ) + # asserting temp_dir and backed action files exits + self.assertTrue(os.path.isdir(temp_dir_path)) + self.assertTrue(os.path.exists(temp_metadata_file_path)) + self.assertTrue(os.path.exists(temp_entry_point_file_path)) + # removing temp_dir and backed action files + remove_temp_action_files(TEST_DEST_PACK_PATH) + # asserting temp_dir and backed action files doesn't exit + self.assertFalse(os.path.isdir(temp_dir_path)) + self.assertFalse(os.path.exists(temp_metadata_file_path)) + self.assertFalse(os.path.exists(temp_entry_point_file_path)) + + @mock.patch("shutil.rmtree") + def test_exception_remove_temp_action_files(self, mock_rmdir): + CLONE_ACTION_4 = clone_action_db( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_4" + ) + clone_action_files( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, CLONE_ACTION_4, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_4["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_4["entry_point"] + temp_backup_action_files( + TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + ) + temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + self.assertTrue(os.path.isdir(temp_dir_path)) + mock_rmdir.side_effect = Exception expected_msg = ( - 'Could not copy to "%s" file, please check the logs or ask your ' - "StackStorm administrator to check and clone the actions files manually" - % cloned_action_metadata_file_path + 'The temporary directory "%s" could not be removed from disk, please check the logs ' + "or ask your StackStorm administrator to check and delete the temporary directory " + "manually" % temp_dir_path ) + with self.assertRaisesRegexp(Exception, expected_msg): + remove_temp_action_files(TEST_DEST_PACK_PATH) + @mock.patch("shutil.rmtree") + def test_permission_error_remove_temp_action_files(self, mock_rmdir): + CLONE_ACTION_5 = clone_action_db( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_5" + ) + clone_action_files( + SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, CLONE_ACTION_5, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_5["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_5["entry_point"] + temp_backup_action_files( + TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + ) + temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + self.assertTrue(os.path.isdir(temp_dir_path)) + mock_rmdir.side_effect = PermissionError + expected_msg = 'No permission to delete the "%s" directory' % temp_dir_path with self.assertRaisesRegexp(Exception, expected_msg): - clone_action( - TEST_SOURCE_PACK_PATH, - "actions/echo.yaml", - "", - "local-shell-cmd", - TEST_DEST_PACK_PATH, - TEST_DEST_PACK, - "action_6", - ) + remove_temp_action_files(TEST_DEST_PACK_PATH) From 83591f38e98f58a6ed23036931a37bf21b031f6a Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 8 Oct 2021 17:52:35 +0530 Subject: [PATCH 17/40] Deleted st2api/tests/base.py as it was temporarily added here for RBAC related unit tests for clone action --- st2api/tests/base.py | 100 ------------------------------------------- 1 file changed, 100 deletions(-) delete mode 100644 st2api/tests/base.py diff --git a/st2api/tests/base.py b/st2api/tests/base.py deleted file mode 100644 index 1ea367fa3f..0000000000 --- a/st2api/tests/base.py +++ /dev/null @@ -1,100 +0,0 @@ -# Copyright 2020 The StackStorm Authors. -# Copyright (C) 2020 Extreme Networks, Inc - All Rights Reserved -# -# 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. - -""" -Module containing base classes for API controller RBAC tests. -""" - -from __future__ import absolute_import - -from oslo_config import cfg - -from st2common.rbac.types import SystemRole -from st2common.persistence.auth import User -from st2common.persistence.rbac import UserRoleAssignment -from st2common.models.db.auth import UserDB -from st2common.models.db.rbac import UserRoleAssignmentDB -from st2common.rbac.migrations import run_all as run_all_rbac_migrations -from st2tests.api import BaseFunctionalTest -from st2tests.base import CleanDbTestCase - - -__all__ = ["APIControllerWithRBACTestCase"] - - -class BaseAPIControllerWithRBACTestCase(BaseFunctionalTest, CleanDbTestCase): - """ - Base test case class for testing API controllers with RBAC enabled. - """ - - enable_auth = True - - @classmethod - def setUpClass(cls): - super(BaseAPIControllerWithRBACTestCase, cls).setUpClass() - - # Make sure RBAC is enabeld - cfg.CONF.set_override(name="enable", override=True, group="rbac") - cfg.CONF.set_override(name="backend", override="default", group="rbac") - - @classmethod - def tearDownClass(cls): - super(BaseAPIControllerWithRBACTestCase, cls).tearDownClass() - - def setUp(self): - super(BaseAPIControllerWithRBACTestCase, self).setUp() - - self.users = {} - self.roles = {} - - # Run RBAC migrations - run_all_rbac_migrations() - - # Insert mock users with default role assignments - role_names = [SystemRole.SYSTEM_ADMIN, SystemRole.ADMIN, SystemRole.OBSERVER] - for role_name in role_names: - user_db = UserDB(name=role_name) - user_db = User.add_or_update(user_db) - self.users[role_name] = user_db - - role_assignment_db = UserRoleAssignmentDB( - user=user_db.name, - role=role_name, - source="assignments/%s.yaml" % user_db.name, - ) - UserRoleAssignment.add_or_update(role_assignment_db) - - # Insert a user with no permissions and role assignments - user_1_db = UserDB(name="no_permissions") - user_1_db = User.add_or_update(user_1_db) - self.users["no_permissions"] = user_1_db - - # Insert special system user - user_2_db = UserDB(name="system_user") - user_2_db = User.add_or_update(user_2_db) - self.users["system_user"] = user_2_db - - role_assignment_db = UserRoleAssignmentDB( - user=user_2_db.name, - role=SystemRole.ADMIN, - source="assignments/%s.yaml" % user_2_db.name, - ) - UserRoleAssignment.add_or_update(role_assignment_db) - - -class APIControllerWithRBACTestCase(BaseAPIControllerWithRBACTestCase): - from st2api import app - - app_module = app From 3c0f50e8abbd3e9e3b173af334d4c262c177d626 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 8 Oct 2021 17:54:32 +0530 Subject: [PATCH 18/40] Deleted test_actions_rbac.py file as these clone action RBAC unit test need to be moved to st2-rbac-backend repo --- .../unit/controllers/v1/test_actions_rbac.py | 392 ------------------ 1 file changed, 392 deletions(-) delete mode 100644 st2api/tests/unit/controllers/v1/test_actions_rbac.py diff --git a/st2api/tests/unit/controllers/v1/test_actions_rbac.py b/st2api/tests/unit/controllers/v1/test_actions_rbac.py deleted file mode 100644 index e178546fd4..0000000000 --- a/st2api/tests/unit/controllers/v1/test_actions_rbac.py +++ /dev/null @@ -1,392 +0,0 @@ -# Copyright 2020 The StackStorm Authors. -# Copyright (C) 2020 Extreme Networks, Inc - All Rights Reserved -# -# 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. - -import mock -import os -import six - -import st2common.validators.api.action as action_validator -from st2common.rbac.types import PermissionType -from st2common.rbac.types import ResourceType -from st2common.persistence.auth import User -from st2common.persistence.rbac import Role -from st2common.persistence.rbac import UserRoleAssignment -from st2common.persistence.rbac import PermissionGrant -from st2common.models.db.auth import UserDB -from st2common.models.db.rbac import RoleDB -from st2common.models.db.rbac import UserRoleAssignmentDB -from st2common.models.db.rbac import PermissionGrantDB -from st2tests.fixturesloader import FixturesLoader -from st2api.controllers.v1.actions import ActionsController - -from tests.base import APIControllerWithRBACTestCase -from st2tests.api import APIControllerWithIncludeAndExcludeFilterTestCase - -http_client = six.moves.http_client - -__all__ = ["ActionControllerRBACTestCase"] - -FIXTURES_PACK = "generic" -TEST_FIXTURES = { - "runners": ["testrunner1.yaml"], - "actions": ["action1.yaml", "local.yaml"], -} - -ACTION_2 = { - "name": "ma.dummy.action", - "pack": "examples", - "description": "test description", - "enabled": True, - "entry_point": "/tmp/test/action2.py", - "runner_type": "local-shell-script", - "parameters": { - "c": {"type": "string", "default": "C1", "position": 0}, - "d": {"type": "string", "default": "D1", "immutable": True}, - }, -} - -ACTION_3 = { - "name": "ma.dummy.clone_action", - "pack": "clonepack", - "description": "test description", - "enabled": True, - "entry_point": "/tmp/test/clone_action.sh", - "runner_type": "local-shell-script", - "parameters": { - "x": {"type": "string", "default": "A1"}, - "y": {"type": "string", "default": "B1"}, - }, -} - - -class ActionControllerRBACTestCase( - APIControllerWithRBACTestCase, APIControllerWithIncludeAndExcludeFilterTestCase -): - - # Attributes used by APIControllerWithIncludeAndExcludeFilterTestCase - get_all_path = "/v1/actions" - controller_cls = ActionsController - include_attribute_field_name = "parameters" - exclude_attribute_field_name = "parameters" - test_exact_object_count = False - rbac_enabled = True - - fixtures_loader = FixturesLoader() - - def setUp(self): - super(ActionControllerRBACTestCase, self).setUp() - self.models = self.fixtures_loader.save_fixtures_to_db( - fixtures_pack=FIXTURES_PACK, fixtures_dict=TEST_FIXTURES - ) - - # Insert mock users, roles and assignments - # Users - user_2_db = UserDB(name="action_create") - user_2_db = User.add_or_update(user_2_db) - self.users["action_create"] = user_2_db - - # Roles - # action_create grant on parent pack - grant_db = PermissionGrantDB( - resource_uid="pack:examples", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_CREATE], - ) - grant_db = PermissionGrant.add_or_update(grant_db) - permission_grants = [str(grant_db.id)] - role_1_db = RoleDB(name="action_create", permission_grants=permission_grants) - role_1_db = Role.add_or_update(role_1_db) - self.roles["action_create"] = role_1_db - - # Role assignments - user_db = self.users["action_create"] - role_assignment_db = UserRoleAssignmentDB( - user=user_db.name, - role=self.roles["action_create"].name, - source="assignments/%s.yaml" % user_db.name, - ) - UserRoleAssignment.add_or_update(role_assignment_db) - - # creating `action_clone` user with all permissions related to cloning an action - user_3_db = UserDB(name="action_clone") - user_3_db = User.add_or_update(user_3_db) - self.users["action_clone"] = user_3_db - - # roles of action_clone user - grant_db = PermissionGrantDB( - resource_uid="pack:clonepack", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_CREATE], - ) - grant_db = PermissionGrant.add_or_update(grant_db) - grant_db_view = PermissionGrantDB( - resource_uid="pack:examples", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_VIEW], - ) - grant_db_view = PermissionGrant.add_or_update(grant_db_view) - grant_db_create = PermissionGrantDB( - resource_uid="pack:examples", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_CREATE], - ) - grant_db_create = PermissionGrant.add_or_update(grant_db_create) - grant_db_delete = PermissionGrantDB( - resource_uid="pack:clonepack", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_DELETE], - ) - grant_db_delete = PermissionGrant.add_or_update(grant_db_delete) - permission_grants = [ - str(grant_db.id), - str(grant_db_view.id), - str(grant_db_create.id), - str(grant_db_delete.id), - ] - role_1_db = RoleDB(name="action_clone", permission_grants=permission_grants) - role_1_db = Role.add_or_update(role_1_db) - self.roles["action_clone"] = role_1_db - - # role assignments for action_clone user - user_db = self.users["action_clone"] - role_assignment_db = UserRoleAssignmentDB( - user=user_db.name, - role=self.roles["action_clone"].name, - source="assignments/%s.yaml" % user_db.name, - ) - UserRoleAssignment.add_or_update(role_assignment_db) - - # creating `no_create_permission` user with action_view permission on source action - # but no create_action permission on destination pack - user_2_db = UserDB(name="no_create_permission") - user_2_db = User.add_or_update(user_2_db) - self.users["no_create_permission"] = user_2_db - - # roles of no_create_permission user - grant_db = PermissionGrantDB( - resource_uid="pack:examples", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_VIEW], - ) - grant_db = PermissionGrant.add_or_update(grant_db) - grant_db_delete = PermissionGrantDB( - resource_uid="pack:clonepack", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_DELETE], - ) - grant_db_delete = PermissionGrant.add_or_update(grant_db_delete) - permission_grants = [str(grant_db.id), str(grant_db_delete.id)] - role_1_db = RoleDB( - name="no_create_permission", permission_grants=permission_grants - ) - role_1_db = Role.add_or_update(role_1_db) - self.roles["no_create_permission"] = role_1_db - - # role assignments for no_create_permission user - user_db = self.users["no_create_permission"] - role_assignment_db = UserRoleAssignmentDB( - user=user_db.name, - role=self.roles["no_create_permission"].name, - source="assignments/%s.yaml" % user_db.name, - ) - UserRoleAssignment.add_or_update(role_assignment_db) - - # creating `no_delete_permission` user with action_view permission on source action, - # action_create on destination pack but no create_delete permission on destination pack - user_2_db = UserDB(name="no_delete_permission") - user_2_db = User.add_or_update(user_2_db) - self.users["no_delete_permission"] = user_2_db - - # roles of no_delete_permission user - grant_db_view = PermissionGrantDB( - resource_uid="pack:examples", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_VIEW], - ) - grant_db_view = PermissionGrant.add_or_update(grant_db_view) - grant_db_create = PermissionGrantDB( - resource_uid="pack:clonepack", - resource_type=ResourceType.PACK, - permission_types=[PermissionType.ACTION_CREATE], - ) - grant_db_create = PermissionGrant.add_or_update(grant_db_create) - permission_grants = [str(grant_db_view.id), str(grant_db_create.id)] - role_1_db = RoleDB( - name="no_delete_permission", permission_grants=permission_grants - ) - role_1_db = Role.add_or_update(role_1_db) - self.roles["no_delete_permission"] = role_1_db - - # role assignments for no_delete_permission user - user_db = self.users["no_delete_permission"] - role_assignment_db = UserRoleAssignmentDB( - user=user_db.name, - role=self.roles["no_delete_permission"].name, - source="assignments/%s.yaml" % user_db.name, - ) - UserRoleAssignment.add_or_update(role_assignment_db) - - @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") - @mock.patch.object( - action_validator, "validate_action", mock.MagicMock(return_value=True) - ) - def test_clone_action_success(self, mock_clone_action): - user_db = self.users["action_clone"] - self.use_user(user_db) - self.__do_post(ACTION_2) - self.__do_post(ACTION_3) - dest_data_body = { - "dest_pack": ACTION_3["pack"], - "dest_action": "clone_action_2", - } - source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) - clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) - self.assertEqual(clone_resp.status_code, http_client.CREATED) - - @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch("st2api.controllers.v1.actions.clone_action") - @mock.patch.object( - action_validator, "validate_action", mock.MagicMock(return_value=True) - ) - def test_clone_overwrite_action_success(self, mock_clone_action): - user_db = self.users["action_clone"] - self.use_user(user_db) - self.__do_post(ACTION_2) - self.__do_post(ACTION_3) - dest_data_body = { - "dest_pack": ACTION_3["pack"], - "dest_action": ACTION_3["name"], - "overwrite": True, - } - source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) - clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) - self.assertEqual(clone_resp.status_code, http_client.CREATED) - - @mock.patch.object( - action_validator, "validate_action", mock.MagicMock(return_value=True) - ) - def test_clone_action_no_source_action_view_permission(self): - user_db = self.users["action_create"] - self.use_user(user_db) - self.__do_post(ACTION_2) - user_db = self.users["no_permissions"] - self.use_user(user_db) - dest_data_body = { - "dest_pack": ACTION_3["pack"], - "dest_action": "clone_action_3", - } - source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) - clone_resp = self.__do_clone( - dest_data_body, source_ref_or_id, expect_errors=True - ) - expected_msg = ( - 'User "no_permissions" doesn\'t have required permission "action_view" ' - 'on resource "action:examples:ma.dummy.action"' - ) - self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) - self.assertEqual(clone_resp.json["faultstring"], expected_msg) - - @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch.object( - action_validator, "validate_action", mock.MagicMock(return_value=True) - ) - def test_clone_action_no_destination_action_create_permission(self): - user_db = self.users["action_clone"] - self.use_user(user_db) - self.__do_post(ACTION_2) - self.__do_post(ACTION_3) - user_db = self.users["no_create_permission"] - self.use_user(user_db) - dest_data_body = { - "dest_pack": ACTION_3["pack"], - "dest_action": "clone_action_4", - } - source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) - clone_resp = self.__do_clone( - dest_data_body, source_ref_or_id, expect_errors=True - ) - expected_msg = ( - 'User "no_create_permission" doesn\'t have required permission "action_create" ' - 'on resource "action:clonepack:clone_action_4"' - ) - self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) - self.assertEqual(clone_resp.json["faultstring"], expected_msg) - - @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch.object( - action_validator, "validate_action", mock.MagicMock(return_value=True) - ) - def test_clone_overwrite_no_destination_action_create_permission(self): - user_db = self.users["action_clone"] - self.use_user(user_db) - self.__do_post(ACTION_2) - self.__do_post(ACTION_3) - user_db = self.users["no_create_permission"] - self.use_user(user_db) - dest_data_body = { - "dest_pack": ACTION_3["pack"], - "dest_action": ACTION_3["name"], - "overwrite": True, - } - source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) - clone_resp = self.__do_clone( - dest_data_body, source_ref_or_id, expect_errors=True - ) - expected_msg = ( - 'User "no_create_permission" doesn\'t have required permission "action_create" ' - 'on resource "action:clonepack:ma.dummy.clone_action"' - ) - self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) - self.assertEqual(clone_resp.json["faultstring"], expected_msg) - - @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) - @mock.patch.object( - action_validator, "validate_action", mock.MagicMock(return_value=True) - ) - def test_clone_overwrite_no_destination_action_delete_permission(self): - user_db = self.users["action_clone"] - self.use_user(user_db) - self.__do_post(ACTION_2) - self.__do_post(ACTION_3) - user_db = self.users["no_delete_permission"] - self.use_user(user_db) - dest_data_body = { - "dest_pack": ACTION_3["pack"], - "dest_action": ACTION_3["name"], - "overwrite": True, - } - source_ref_or_id = "%s.%s" % (ACTION_2["pack"], ACTION_2["name"]) - clone_resp = self.__do_clone( - dest_data_body, source_ref_or_id, expect_errors=True - ) - expected_msg = ( - 'User "no_delete_permission" doesn\'t have required permission "action_delete" ' - 'on resource "action:clonepack:ma.dummy.clone_action"' - ) - self.assertEqual(clone_resp.status_code, http_client.UNAUTHORIZED) - self.assertEqual(clone_resp.json["faultstring"], expected_msg) - - def _insert_mock_models(self): - action_ids = [action["id"] for action in self.models["actions"].values()] - return action_ids - - def __do_post(self, rule): - return self.app.post_json("/v1/actions", rule, expect_errors=True) - - def __do_clone(self, dest_data, action_id, expect_errors=False): - return self.app.post_json( - "/v1/actions/%s/clone" % (action_id), dest_data, expect_errors=expect_errors - ) From b1815a28b267f4333572acfa900175b860315de4 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 11 Oct 2021 22:15:38 +0530 Subject: [PATCH 19/40] Created new function `_restore_action` to reuse the common code and done few minor fixes --- st2api/st2api/controllers/v1/actions.py | 37 ++++++++++++++----------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 8cdbfa60a6..639042a3ba 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -17,6 +17,7 @@ import os.path import stat import errno +import uuid import six from mongoengine import ValidationError @@ -365,8 +366,12 @@ def clone(self, dest_data, ref_or_id, requester_user): options = GenericRequestParam(remove_files=True) dest_metadata_file = dest_action_db["metadata_file"] dest_entry_point = dest_action_db["entry_point"] + temp_sub_dir = str(uuid.uuid4()) temp_backup_action_files( - dest_pack_base_path, dest_metadata_file, dest_entry_point + dest_pack_base_path, + dest_metadata_file, + dest_entry_point, + temp_sub_dir, ) self.delete(options, dest_ref, requester_user) except ResourceAccessDeniedError as e: @@ -385,27 +390,21 @@ def clone(self, dest_data, ref_or_id, requester_user): dest_action_db=cloned_dest_action_db, dest_pack_base_path=dest_pack_base_path, ) - post_response = self.post(cloned_action_api, requester_user) if post_response.status_code != http_client.CREATED: raise Exception("Could not add cloned action to database.") - + cloned_dest_action_db["id"] = post_response.json["id"] extra = {"cloned_acion_db": cloned_dest_action_db} LOG.audit( "Action cloned. Action.id=%s" % (cloned_dest_action_db.id), extra=extra ) if dest_action_db: - remove_temp_action_files(dest_pack_base_path) + remove_temp_action_files(temp_sub_dir) return post_response except PermissionError as e: LOG.error("No permission to clone the action. Exception was %s", e) if dest_action_db: - restore_temp_action_files( - dest_pack_base_path, dest_metadata_file, dest_entry_point - ) - dest_action_db.id = None - Action.add_or_update(dest_action_db) - remove_temp_action_files(dest_pack_base_path) + self._restore_action(dest_action_db, dest_pack_base_path, temp_sub_dir) abort(http_client.FORBIDDEN, six.text_type(e)) except Exception as e: LOG.error( @@ -418,12 +417,7 @@ def clone(self, dest_data, ref_or_id, requester_user): metadata_file=cloned_dest_action_db["metadata_file"], ) if dest_action_db: - restore_temp_action_files( - dest_pack_base_path, dest_metadata_file, dest_entry_point - ) - dest_action_db.id = None - Action.add_or_update(dest_action_db) - remove_temp_action_files(dest_pack_base_path) + self._restore_action(dest_action_db, dest_pack_base_path, temp_sub_dir) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) def _handle_data_files(self, pack_ref, data_files): @@ -537,5 +531,16 @@ def _dispatch_trigger_for_written_data_files(self, action_db, written_data_files } self._trigger_dispatcher.dispatch(trigger=trigger, payload=payload) + def _restore_action(self, action_db, pack_base_path, temp_sub_dir): + restore_temp_action_files( + pack_base_path, + action_db["metadata_file"], + action_db["entry_point"], + temp_sub_dir, + ) + action_db.id = None + Action.add_or_update(action_db) + remove_temp_action_files(temp_sub_dir) + actions_controller = ActionsController() From f795daf0e865f45c46022e82cd5dddd5f2803622 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 11 Oct 2021 22:20:30 +0530 Subject: [PATCH 20/40] Adding check for `actions` directory in pack to create it if doesn't exist Changing tmp actions backup directory to system `/tmp` directory. Cleaning `temp_backup_action_file`, `restore_temp_action_files` and `remove_temp_action_files` functions. Making other minor fixes. --- st2common/st2common/services/packs.py | 59 ++++++++++++--------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 3afb20afa8..be36abd6c7 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -301,21 +301,20 @@ def _clone_content_to_destination_file(source_file, destination_file): shutil.copy(src=source_file, dst=destination_file) except PermissionError: LOG.error( - 'No write permission for "%s" file', + 'Unable to copy file to "%s" due to permission error.', destination_file, ) - msg = 'No permission to write in "%s" file' % (destination_file) + msg = 'Unable to copy file to "%s".' % (destination_file) raise PermissionError(msg) except Exception as e: LOG.error( - 'Could not copy to "%s" file. Exception was "%s"', + 'Unable to copy file to "%s". Exception was "%s".', destination_file, e, ) msg = ( - 'Could not copy to "%s" file, please check the logs ' - "or ask your StackStorm administrator to check and clone " - "the actions files manually" % (destination_file) + 'Unable to copy file to "%s". Please check the logs or ask your ' + "administrator to clone the files manually." % destination_file ) raise Exception(msg) @@ -336,6 +335,10 @@ def clone_action_files(source_action_db, dest_action_db, dest_pack_base_path): dest_metadata_file_name = dest_action_db["metadata_file"] dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file_name) + # creating actions directory if doesn't exist + ac_dir_path = os.path.join(dest_pack_base_path, "actions") + if not os.path.isdir(ac_dir_path): + os.mkdir(path=ac_dir_path) _clone_content_to_destination_file( source_file=source_metadata_file_path, destination_file=dest_metadata_file_path ) @@ -398,22 +401,20 @@ def clone_action_db(source_action_db, dest_pack, dest_action): return dest_action_db -def temp_backup_action_files(dest_pack_base_path, dest_metadata_file, dest_entry_point): - temp_dir_path = os.path.join(dest_pack_base_path, "temp_dir") +def temp_backup_action_files(pack_base_path, metadata_file, entry_point, temp_sub_dir): + temp_dir_path = "/tmp/%s" % temp_sub_dir os.mkdir(temp_dir_path) - temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") - dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file) + os.mkdir(os.path.join(temp_dir_path, "actions")) + os.mkdir(os.path.join(temp_dir_path, "actions", "workflows")) + temp_metadata_file_path = os.path.join(temp_dir_path, metadata_file) + dest_metadata_file_path = os.path.join(pack_base_path, metadata_file) _clone_content_to_destination_file( source_file=dest_metadata_file_path, destination_file=temp_metadata_file_path ) - if dest_entry_point: - old_ext = os.path.splitext(dest_entry_point)[1] - temp_entry_point_file_name = "temp_entry_point_file" + old_ext - temp_entry_point_file_path = os.path.join( - temp_dir_path, temp_entry_point_file_name - ) + if entry_point: + temp_entry_point_file_path = os.path.join(temp_dir_path, "actions", entry_point) dest_entry_point_file_path = os.path.join( - dest_pack_base_path, "actions", dest_entry_point + pack_base_path, "actions", entry_point ) _clone_content_to_destination_file( source_file=dest_entry_point_file_path, @@ -421,23 +422,17 @@ def temp_backup_action_files(dest_pack_base_path, dest_metadata_file, dest_entry ) -def restore_temp_action_files( - dest_pack_base_path, dest_metadata_file, dest_entry_point -): - temp_dir_path = os.path.join(dest_pack_base_path, "temp_dir") - temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") - dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file) +def restore_temp_action_files(pack_base_path, metadata_file, entry_point, temp_sub_dir): + temp_dir_path = "/tmp/%s" % temp_sub_dir + temp_metadata_file_path = os.path.join(temp_dir_path, metadata_file) + dest_metadata_file_path = os.path.join(pack_base_path, metadata_file) _clone_content_to_destination_file( source_file=temp_metadata_file_path, destination_file=dest_metadata_file_path ) - if dest_entry_point: - old_ext = os.path.splitext(dest_entry_point)[1] - temp_entry_point_file_name = "temp_entry_point_file" + old_ext - temp_entry_point_file_path = os.path.join( - temp_dir_path, temp_entry_point_file_name - ) + if entry_point: + temp_entry_point_file_path = os.path.join(temp_dir_path, "actions", entry_point) dest_entry_point_file_path = os.path.join( - dest_pack_base_path, "actions", dest_entry_point + pack_base_path, "actions", entry_point ) _clone_content_to_destination_file( source_file=temp_entry_point_file_path, @@ -445,8 +440,8 @@ def restore_temp_action_files( ) -def remove_temp_action_files(dest_pack_base_path): - temp_dir_path = os.path.join(dest_pack_base_path, "temp_dir") +def remove_temp_action_files(temp_sub_dir): + temp_dir_path = "/tmp/%s" % temp_sub_dir if os.path.isdir(temp_dir_path): try: From 305d50e59ca26722eba8cebf3d2e5399bcc3db9e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 11 Oct 2021 22:23:21 +0530 Subject: [PATCH 21/40] Added unit tests for exceptions for `temp_backup_action_files` and `restore_temp_action_files` functions Consolidated backup, restore, remove temp action files into a single unit test. Refactored other few unit tests. --- st2common/tests/unit/services/test_packs.py | 323 ++++++++++++++------ 1 file changed, 222 insertions(+), 101 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 68254b8225..c4572b0244 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -21,6 +21,7 @@ import mock import shutil import unittest2 +import uuid import st2tests @@ -39,7 +40,7 @@ TEST_SOURCE_PACK = "core" -TEST_SOURCE_WORKFLOW_PACK = "orquesta_tests" +TEST_SOURCE_WORKFLOW_PACK = "examples" TEST_DEST_PACK = "dummy_pack_23" TEST_DEST_PACK_PATH = os.path.join( @@ -178,22 +179,18 @@ SOURCE_WORKFLOW = { "description": "A basic workflow to demonstrate data flow options.", "enabled": True, - "entry_point": "workflows/data-flow.yaml", - "metadata_file": "actions/data-flow.yaml", - "name": "data-flow", + "entry_point": "workflows/orquesta-data-flow.yaml", + "id": "616413f3c05ab0a4329a22aa", + "metadata_file": "actions/orquesta-data-flow.yaml", + "name": "orquesta-data-flow", "notify": {}, - "output_schema": { - "a6": {"type": "string", "required": True}, - "b6": {"type": "string", "required": True}, - "a7": {"type": "string", "required": True}, - "b7": {"type": "string", "required": True, "secret": "********"}, - }, + "output_schema": {}, "pack": TEST_SOURCE_WORKFLOW_PACK, "parameters": {"a1": {"required": True, "type": "string"}}, - "ref": "orquesta_tests.data-flow", + "ref": "examples.orquesta-data-flow", "runner_type": {"name": "orquesta"}, "tags": [], - "uid": "action:orquesta_tests:data-flow", + "uid": "action:examples:orquesta-data-flow", } @@ -410,10 +407,23 @@ def test_exception_to_remove_resource_metadata_file(self, remove): class CloneActionDBAndFilesTestCase(unittest2.TestCase): + @classmethod + def setUpClass(cls): + action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + workflow_files_path = os.path.join(action_files_path, "workflows") + if not os.path.isdir(action_files_path): + os.mkdir(action_files_path) + if not os.path.isdir(workflow_files_path): + os.mkdir(workflow_files_path) + @classmethod def tearDownClass(cls): action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") workflow_files_path = os.path.join(action_files_path, "workflows") + if not os.path.isdir(action_files_path): + os.mkdir(action_files_path) + if not os.path.isdir(workflow_files_path): + os.mkdir(workflow_files_path) for file in os.listdir(action_files_path): if os.path.isfile(os.path.join(action_files_path, file)): os.remove(os.path.join(action_files_path, file)) @@ -430,19 +440,7 @@ def test_clone_action_db(self): ) actual_uid = CLONE_ACTION_1["uid"] self.assertEqual(actual_uid, exptected_uid) - exptected_parameters = { - "trigger": { - "type": "string", - "description": "Trigger reference (e.g. mypack.my_trigger).", - "required": True, - }, - "payload": {"type": "object", "description": "Trigger payload."}, - "trace_tag": { - "type": "string", - "description": "Optional trace tag.", - "required": False, - }, - } + exptected_parameters = SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER["parameters"] actual_parameters = CLONE_ACTION_1["parameters"] self.assertDictEqual(actual_parameters, exptected_parameters) @@ -512,16 +510,16 @@ def test_permission_error_to_write_in_destination_file(self, mock_copy): cloned_action_metadata_file_path = os.path.join( TEST_DEST_PACK_PATH, "actions", "clone_action_4.yaml" ) - expected_msg = 'No permission to write in "%s" file' % ( + expected_msg = 'Unable to copy file to "%s".' % ( cloned_action_metadata_file_path ) - CLONE_ACTION_2 = clone_action_db( + CLONE_ACTION_4 = clone_action_db( SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_4" ) with self.assertRaisesRegexp(PermissionError, expected_msg): clone_action_files( SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, - CLONE_ACTION_2, + CLONE_ACTION_4, TEST_DEST_PACK_PATH, ) @@ -537,8 +535,8 @@ def test_exceptions_to_write_in_destination_file(self, mock_copy): TEST_DEST_PACK_PATH, "actions", "clone_action_5.yaml" ) expected_msg = ( - 'Could not copy to "%s" file, please check the logs or ask your ' - "StackStorm administrator to check and clone the actions files manually" + 'Unable to copy file to "%s". Please check the logs or ask your ' + "administrator to clone the files manually." % cloned_action_metadata_file_path ) with self.assertRaisesRegexp(Exception, expected_msg): @@ -548,30 +546,41 @@ def test_exceptions_to_write_in_destination_file(self, mock_copy): TEST_DEST_PACK_PATH, ) + def test_actions_directory_created_if_does_not_exist(self): + action_dir_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + # removing actions directory and asserting it doesn't exist + shutil.rmtree(action_dir_path) + self.assertFalse(os.path.exists(action_dir_path)) + CLONE_ACTION_6 = clone_action_db( + SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER, TEST_DEST_PACK, "clone_action_6" + ) + clone_action_files( + SOURCE_ACTION_WITH_LOCAL_SHELL_CMD_RUNNER, + CLONE_ACTION_6, + TEST_DEST_PACK_PATH, + ) + # workflows directory created and asserting it exists + self.assertTrue(os.path.exists(action_dir_path)) + wf_dir_path = os.path.join(action_dir_path, "workflows") + if not os.path.isdir(wf_dir_path): + os.mkdir(wf_dir_path) + def test_workflows_directory_created_if_does_not_exist(self): - action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") - workflow_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions", "workflows") - for file in os.listdir(workflow_files_path): - if os.path.isfile(os.path.join(workflow_files_path, file)): - os.remove(os.path.join(workflow_files_path, file)) + action_dir_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + workflows_dir_path = os.path.join(TEST_DEST_PACK_PATH, "actions", "workflows") # removing workflows directory and asserting it doesn't exist - os.rmdir(workflow_files_path) - self.assertFalse(os.path.exists(workflow_files_path)) - self.assertTrue(os.path.exists(action_files_path)) + shutil.rmtree(workflows_dir_path) + self.assertFalse(os.path.exists(workflows_dir_path)) + self.assertTrue(os.path.exists(action_dir_path)) CLONE_WORKFLOW = clone_action_db( SOURCE_WORKFLOW, TEST_DEST_PACK, "clone_workflow" ) clone_action_files(SOURCE_WORKFLOW, CLONE_WORKFLOW, TEST_DEST_PACK_PATH) # workflows directory created and asserting it exists - self.assertTrue(os.path.exists(workflow_files_path)) + self.assertTrue(os.path.exists(workflows_dir_path)) class CloneActionFilesBackupTestCase(unittest2.TestCase): - def setUp(self): - temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") - if os.path.isdir(temp_dir_path): - shutil.rmtree(temp_dir_path) - @classmethod def tearDownClass(cls): action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") @@ -582,11 +591,8 @@ def tearDownClass(cls): for file in os.listdir(workflow_files_path): if os.path.isfile(os.path.join(workflow_files_path, file)): os.remove(os.path.join(workflow_files_path, file)) - temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") - if os.path.isdir(temp_dir_path): - shutil.rmtree(temp_dir_path) - def test_temp_backup_action_files(self): + def test_temp_backup_restore_remove_action_files(self): CLONE_ACTION_1 = clone_action_db( SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_1" ) @@ -595,32 +601,27 @@ def test_temp_backup_action_files(self): ) dest_action_metadata_file = CLONE_ACTION_1["metadata_file"] dest_action_entry_point_file = CLONE_ACTION_1["entry_point"] + temp_sub_dir = str(uuid.uuid4()) + + # creating backup of dest action files at /tmp/ temp_backup_action_files( - TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, ) - temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") - self.assertTrue(temp_dir_path) - temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") - self.assertTrue(os.path.exists(temp_metadata_file_path)) + temp_dir_path = "/tmp/%s" % temp_sub_dir + self.assertTrue(os.path.isdir(temp_dir_path)) + temp_metadata_file_path = os.path.join(temp_dir_path, dest_action_metadata_file) temp_entry_point_file_path = os.path.join( - temp_dir_path, "temp_entry_point_file.py" + temp_dir_path, "actions", dest_action_entry_point_file ) + # asserting backup files exists + self.assertTrue(os.path.exists(temp_metadata_file_path)) self.assertTrue(os.path.exists(temp_entry_point_file_path)) - def test_restore_temp_action_files(self): - CLONE_ACTION_2 = clone_action_db( - SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_2" - ) - clone_action_files( - SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_2, TEST_DEST_PACK_PATH - ) - dest_action_metadata_file = CLONE_ACTION_2["metadata_file"] - dest_action_entry_point_file = CLONE_ACTION_2["entry_point"] - temp_backup_action_files( - TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file - ) - dest_action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") # removing destination action files + dest_action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") for file in os.listdir(dest_action_files_path): if os.path.isfile(os.path.join(dest_action_files_path, file)): os.remove(os.path.join(dest_action_files_path, file)) @@ -633,44 +634,30 @@ def test_restore_temp_action_files(self): # asserting destination action files doesn't exist self.assertFalse(os.path.isfile(dest_action_metadata_file_path)) self.assertFalse(os.path.isfile(dest_action_entry_point_file_path)) + # restoring temp backed action files to destination restore_temp_action_files( - TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, ) # asserting action files restored at destination self.assertTrue(os.path.isfile(dest_action_metadata_file_path)) self.assertTrue(os.path.isfile(dest_action_entry_point_file_path)) - - def test_remove_temp_action_files(self): - CLONE_ACTION_3 = clone_action_db( - SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_3" - ) - clone_action_files( - SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, CLONE_ACTION_3, TEST_DEST_PACK_PATH - ) - dest_action_metadata_file = CLONE_ACTION_3["metadata_file"] - dest_action_entry_point_file = CLONE_ACTION_3["entry_point"] - temp_backup_action_files( - TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file - ) - temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") - temp_metadata_file_path = os.path.join(temp_dir_path, "temp_metadata_file.yaml") - temp_entry_point_file_path = os.path.join( - temp_dir_path, "temp_entry_point_file.py" - ) # asserting temp_dir and backed action files exits self.assertTrue(os.path.isdir(temp_dir_path)) self.assertTrue(os.path.exists(temp_metadata_file_path)) self.assertTrue(os.path.exists(temp_entry_point_file_path)) + # removing temp_dir and backed action files - remove_temp_action_files(TEST_DEST_PACK_PATH) + remove_temp_action_files(temp_sub_dir) # asserting temp_dir and backed action files doesn't exit self.assertFalse(os.path.isdir(temp_dir_path)) self.assertFalse(os.path.exists(temp_metadata_file_path)) self.assertFalse(os.path.exists(temp_entry_point_file_path)) - @mock.patch("shutil.rmtree") - def test_exception_remove_temp_action_files(self, mock_rmdir): + def test_exception_remove_temp_action_files(self): CLONE_ACTION_4 = clone_action_db( SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_4" ) @@ -679,22 +666,28 @@ def test_exception_remove_temp_action_files(self, mock_rmdir): ) dest_action_metadata_file = CLONE_ACTION_4["metadata_file"] dest_action_entry_point_file = CLONE_ACTION_4["entry_point"] + temp_sub_dir = str(uuid.uuid4()) temp_backup_action_files( - TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, ) - temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + temp_dir_path = "/tmp/%s" % temp_sub_dir self.assertTrue(os.path.isdir(temp_dir_path)) - mock_rmdir.side_effect = Exception expected_msg = ( 'The temporary directory "%s" could not be removed from disk, please check the logs ' "or ask your StackStorm administrator to check and delete the temporary directory " "manually" % temp_dir_path ) - with self.assertRaisesRegexp(Exception, expected_msg): - remove_temp_action_files(TEST_DEST_PACK_PATH) + with mock.patch("shutil.rmtree") as mock_rmdir: + mock_rmdir.side_effect = Exception + with self.assertRaisesRegexp(Exception, expected_msg): + remove_temp_action_files(temp_sub_dir) + + remove_temp_action_files(temp_sub_dir) - @mock.patch("shutil.rmtree") - def test_permission_error_remove_temp_action_files(self, mock_rmdir): + def test_permission_error_remove_temp_action_files(self): CLONE_ACTION_5 = clone_action_db( SOURCE_ACTION_WITH_PYTHON_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_5" ) @@ -703,12 +696,140 @@ def test_permission_error_remove_temp_action_files(self, mock_rmdir): ) dest_action_metadata_file = CLONE_ACTION_5["metadata_file"] dest_action_entry_point_file = CLONE_ACTION_5["entry_point"] + temp_sub_dir = str(uuid.uuid4()) temp_backup_action_files( - TEST_DEST_PACK_PATH, dest_action_metadata_file, dest_action_entry_point_file + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, ) - temp_dir_path = os.path.join(TEST_DEST_PACK_PATH, "temp_dir") + temp_dir_path = "/tmp/%s" % temp_sub_dir self.assertTrue(os.path.isdir(temp_dir_path)) - mock_rmdir.side_effect = PermissionError expected_msg = 'No permission to delete the "%s" directory' % temp_dir_path - with self.assertRaisesRegexp(Exception, expected_msg): - remove_temp_action_files(TEST_DEST_PACK_PATH) + with mock.patch("shutil.rmtree") as mock_rmdir: + mock_rmdir.side_effect = PermissionError + with self.assertRaisesRegexp(PermissionError, expected_msg): + remove_temp_action_files(temp_sub_dir) + + remove_temp_action_files(temp_sub_dir) + + def test_exception_temp_backup_action_files(self): + CLONE_ACTION_6 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_6" + ) + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_6, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_6["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_6["entry_point"] + temp_sub_dir = str(uuid.uuid4()) + temp_dir_path = "/tmp/%s" % temp_sub_dir + tmp_action_metadata_file_path = os.path.join( + temp_dir_path, dest_action_metadata_file + ) + expected_msg = ( + 'Unable to copy file to "%s". Please check the logs or ask your ' + "administrator to clone the files manually." % tmp_action_metadata_file_path + ) + with mock.patch("shutil.copy") as mock_copy: + mock_copy.side_effect = Exception + with self.assertRaisesRegexp(Exception, expected_msg): + temp_backup_action_files( + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, + ) + + remove_temp_action_files(temp_sub_dir) + + def test_permission_error_temp_backup_action_files(self): + CLONE_ACTION_7 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_7" + ) + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_7, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_7["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_7["entry_point"] + temp_sub_dir = str(uuid.uuid4()) + temp_dir_path = "/tmp/%s" % temp_sub_dir + tmp_action_metadata_file_path = os.path.join( + temp_dir_path, dest_action_metadata_file + ) + expected_msg = 'Unable to copy file to "%s".' % tmp_action_metadata_file_path + with mock.patch("shutil.copy") as mock_copy: + mock_copy.side_effect = PermissionError + with self.assertRaisesRegexp(PermissionError, expected_msg): + temp_backup_action_files( + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, + ) + + remove_temp_action_files(temp_sub_dir) + + def test_exception_restore_temp_action_files(self): + CLONE_ACTION_8 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_8" + ) + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_8, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_8["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_8["entry_point"] + dest_action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + for file in os.listdir(dest_action_files_path): + if os.path.isfile(os.path.join(dest_action_files_path, file)): + os.remove(os.path.join(dest_action_files_path, file)) + temp_sub_dir = str(uuid.uuid4()) + dest_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, dest_action_metadata_file + ) + expected_msg = ( + 'Unable to copy file to "%s". Please check the logs or ask your ' + "administrator to clone the files manually." + % dest_action_metadata_file_path + ) + with mock.patch("shutil.copy") as mock_copy: + mock_copy.side_effect = Exception + with self.assertRaisesRegexp(Exception, expected_msg): + restore_temp_action_files( + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, + ) + + remove_temp_action_files(temp_sub_dir) + + def test_permission_error_restore_temp_action_files(self): + CLONE_ACTION_9 = clone_action_db( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, TEST_DEST_PACK, "clone_action_9" + ) + clone_action_files( + SOURCE_ACTION_WITH_SHELL_SCRIPT_RUNNER, CLONE_ACTION_9, TEST_DEST_PACK_PATH + ) + dest_action_metadata_file = CLONE_ACTION_9["metadata_file"] + dest_action_entry_point_file = CLONE_ACTION_9["entry_point"] + dest_action_files_path = os.path.join(TEST_DEST_PACK_PATH, "actions") + for file in os.listdir(dest_action_files_path): + if os.path.isfile(os.path.join(dest_action_files_path, file)): + os.remove(os.path.join(dest_action_files_path, file)) + temp_sub_dir = str(uuid.uuid4()) + dest_action_metadata_file_path = os.path.join( + TEST_DEST_PACK_PATH, dest_action_metadata_file + ) + expected_msg = 'Unable to copy file to "%s".' % dest_action_metadata_file_path + with mock.patch("shutil.copy") as mock_copy: + mock_copy.side_effect = PermissionError + with self.assertRaisesRegexp(PermissionError, expected_msg): + restore_temp_action_files( + TEST_DEST_PACK_PATH, + dest_action_metadata_file, + dest_action_entry_point_file, + temp_sub_dir, + ) + + remove_temp_action_files(temp_sub_dir) From 6a250b88a62f70d7c7bc1dfab8751be55e98fa01 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 11 Oct 2021 22:42:35 +0530 Subject: [PATCH 22/40] Refactored test_packs.py to fix failing CI unit test chunk 1 --- st2common/tests/unit/services/test_packs.py | 22 ++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index c4572b0244..b4b09da08c 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -40,7 +40,7 @@ TEST_SOURCE_PACK = "core" -TEST_SOURCE_WORKFLOW_PACK = "examples" +TEST_SOURCE_WORKFLOW_PACK = "orquesta_tests" TEST_DEST_PACK = "dummy_pack_23" TEST_DEST_PACK_PATH = os.path.join( @@ -176,21 +176,25 @@ "uid": "action:core:echo", } -SOURCE_WORKFLOW = { +SOURCE_WORKFLOW = SOURCE_WORKFLOW = { "description": "A basic workflow to demonstrate data flow options.", "enabled": True, - "entry_point": "workflows/orquesta-data-flow.yaml", - "id": "616413f3c05ab0a4329a22aa", - "metadata_file": "actions/orquesta-data-flow.yaml", - "name": "orquesta-data-flow", + "entry_point": "workflows/data-flow.yaml", + "metadata_file": "actions/data-flow.yaml", + "name": "data-flow", "notify": {}, - "output_schema": {}, + "output_schema": { + "a6": {"type": "string", "required": True}, + "b6": {"type": "string", "required": True}, + "a7": {"type": "string", "required": True}, + "b7": {"type": "string", "required": True, "secret": "********"}, + }, "pack": TEST_SOURCE_WORKFLOW_PACK, "parameters": {"a1": {"required": True, "type": "string"}}, - "ref": "examples.orquesta-data-flow", + "ref": "orquesta_tests.data-flow", "runner_type": {"name": "orquesta"}, "tags": [], - "uid": "action:examples:orquesta-data-flow", + "uid": "action:orquesta_tests:data-flow", } From d41453ace5fa358cc83b057552e7196e94b9eb55 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 11 Oct 2021 23:03:01 +0530 Subject: [PATCH 23/40] Minor fix in test_packs.py, removed extra variable name `SOURCE_WORKFLOW` --- st2common/tests/unit/services/test_packs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index b4b09da08c..bbc433aad5 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -176,7 +176,7 @@ "uid": "action:core:echo", } -SOURCE_WORKFLOW = SOURCE_WORKFLOW = { +SOURCE_WORKFLOW = { "description": "A basic workflow to demonstrate data flow options.", "enabled": True, "entry_point": "workflows/data-flow.yaml", From 0ea4b9a4681a2029cbcd75ce1624881819990057 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 13 Oct 2021 18:28:39 +0530 Subject: [PATCH 24/40] Refactored unit test `test_clone_overwrite_exception_destination_recovered` under `test_actions.py` Added few more checks to assert original ACTION_17 files have been restored --- .../tests/unit/controllers/v1/test_actions.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index 4963d3211e..7f8162e94b 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -1086,11 +1086,6 @@ def test_clone_overwrite_exception_destination_recovered( expect_errors=True, ) self.assertEqual(clone_resp.status_int, 500) - dest_get_resp = self.__do_get_actions_by_url_parameter( - "name", ACTION_17["name"] - ) - self.assertEqual(dest_get_resp.status_int, 200) - self.assertEqual(dest_get_resp.json[0]["runner_type"], ACTION_17["runner_type"]) # asserting temp_backup_action_files function called self.assertTrue(mock_backup_files.called) # asserting restore_temp_action_files called i.e. original ACTION_17 restored @@ -1099,6 +1094,19 @@ def test_clone_overwrite_exception_destination_recovered( self.assertTrue(mock_remove_backup.called) # asserting delete_action_files_from_pack called i.e. cloned files are cleaned up self.assertTrue(mock_clean_files.called) + # retrieving reregistered oringinal ACTION_17 from database + dest_get_resp = self.__do_get_actions_by_url_parameter( + "name", ACTION_17["name"] + ) + self.assertEqual(dest_get_resp.status_int, 200) + expected_runner_type = ACTION_17["runner_type"] + actual_runner_type = dest_get_resp.json[0]["runner_type"] + # asserting ACTION_17 has original runner type + self.assertEqual(actual_runner_type, expected_runner_type) + expeted_parameters = ACTION_17["parameters"] + actual_parameters = dest_get_resp.json[0]["parameters"] + # asserting ACTION_17 has original parameters + self.assertDictEqual(actual_parameters, expeted_parameters) self.__do_delete(self.__get_action_id(source_post_resp)) self.__do_delete(dest_get_resp.json[0]["id"]) From 6d736c185f8e3e6876c4dc04549325d0d42ccfbd Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 13 Oct 2021 19:07:00 +0530 Subject: [PATCH 25/40] Minor grammar fix in comment in `test_actions.py` --- st2api/tests/unit/controllers/v1/test_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index 7f8162e94b..6af056ab7f 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -1094,7 +1094,7 @@ def test_clone_overwrite_exception_destination_recovered( self.assertTrue(mock_remove_backup.called) # asserting delete_action_files_from_pack called i.e. cloned files are cleaned up self.assertTrue(mock_clean_files.called) - # retrieving reregistered oringinal ACTION_17 from database + # retrieving oringinal ACTION_17 from db which is reregistered after exception dest_get_resp = self.__do_get_actions_by_url_parameter( "name", ACTION_17["name"] ) From 76bebb8914c62276f579f1e31dcd1a2cf5bdb5db Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 13 Oct 2021 23:09:59 +0530 Subject: [PATCH 26/40] Refactoring unit test `test_clone_overwrite` to use ACTION_16["parameters"] instead of defining params dictionary again --- st2api/tests/unit/controllers/v1/test_actions.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index 6af056ab7f..8200cdc946 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -925,10 +925,7 @@ def test_clone_overwrite( clone_resp = self.__do_clone(dest_data_body, source_ref_or_id) self.assertEqual(clone_resp.status_int, 201) get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_17["name"]) - expected_params_dict = { - "x": {"type": "string", "default": "X1"}, - "y": {"type": "string", "default": "Y1"}, - } + expected_params_dict = ACTION_16["parameters"] actual_prams_dict = get_resp.json[0]["parameters"] self.assertDictEqual(actual_prams_dict, expected_params_dict) actual_runner_type = get_resp.json[0]["runner_type"] From 34326d5c9b941be8ac3f048c01d4f38b174b9f2a Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 13 Oct 2021 23:29:29 +0530 Subject: [PATCH 27/40] Minor change to add comment line in unit test in `test_actions.py` --- st2api/tests/unit/controllers/v1/test_actions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index 8200cdc946..ddb0f11675 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -950,6 +950,7 @@ def test_clone_source_does_not_exist(self): source_ref_or_id, expect_errors=True, ) + # clone operation failed and asserting response status code and error msg self.assertEqual(clone_resp.status_int, 404) msg = 'Resource with a reference or id "%s" not found' % source_ref_or_id self.assertEqual(clone_resp.json["faultstring"], msg) From ab458186abc49e254d23730a3a8d7e0cfb3e3c6a Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 14 Oct 2021 11:24:51 +0530 Subject: [PATCH 28/40] Added comment for source workflow path required for related unit tests --- st2common/tests/unit/services/test_packs.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index bbc433aad5..4186a8d29c 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -176,6 +176,8 @@ "uid": "action:core:echo", } +# source workflow needed from ``/st2tests/fixtures/packs/`` path. When source workflow +# taken from ``/opt/stackstorm/packs/`` path, related unit tests fail SOURCE_WORKFLOW = { "description": "A basic workflow to demonstrate data flow options.", "enabled": True, From 7085db1acc563ce5e359f42af3b1a5a5589d825e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 14 Oct 2021 22:34:52 +0530 Subject: [PATCH 29/40] Fix related to entry_point directories creatinon in `/tmp/` relative to `./pack/actions` directory --- st2common/st2common/services/packs.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index be36abd6c7..a773df4ba1 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -404,15 +404,23 @@ def clone_action_db(source_action_db, dest_pack, dest_action): def temp_backup_action_files(pack_base_path, metadata_file, entry_point, temp_sub_dir): temp_dir_path = "/tmp/%s" % temp_sub_dir os.mkdir(temp_dir_path) - os.mkdir(os.path.join(temp_dir_path, "actions")) - os.mkdir(os.path.join(temp_dir_path, "actions", "workflows")) + actions_dir = os.path.join(temp_dir_path, "actions") + os.mkdir(actions_dir) temp_metadata_file_path = os.path.join(temp_dir_path, metadata_file) dest_metadata_file_path = os.path.join(pack_base_path, metadata_file) _clone_content_to_destination_file( source_file=dest_metadata_file_path, destination_file=temp_metadata_file_path ) if entry_point: - temp_entry_point_file_path = os.path.join(temp_dir_path, "actions", entry_point) + if os.path.split(entry_point)[0] == "": + temp_entry_point_file_path = os.path.join( + temp_dir_path, "actions", entry_point + ) + else: + entry_point_dir = str(os.path.split(entry_point)[0]) + entry_point_dir_path = os.path.join(actions_dir, entry_point_dir) + os.makedirs(entry_point_dir_path) + temp_entry_point_file_path = os.path.join(actions_dir, entry_point) dest_entry_point_file_path = os.path.join( pack_base_path, "actions", entry_point ) From d9d27ae3015109643356bc4650ad2e885034c26b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 14 Oct 2021 22:37:13 +0530 Subject: [PATCH 30/40] Minor fix in clone api function for removing cloned action database entry in case of exception in operation --- st2api/st2api/controllers/v1/actions.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 639042a3ba..6dddb35314 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -385,15 +385,15 @@ def clone(self, dest_data, ref_or_id, requester_user): abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) try: + post_response = self.post(cloned_action_api, requester_user) + if post_response.status_code != http_client.CREATED: + raise Exception("Could not add cloned action to database.") + cloned_dest_action_db["id"] = post_response.json["id"] clone_action_files( source_action_db=source_action_db, dest_action_db=cloned_dest_action_db, dest_pack_base_path=dest_pack_base_path, ) - post_response = self.post(cloned_action_api, requester_user) - if post_response.status_code != http_client.CREATED: - raise Exception("Could not add cloned action to database.") - cloned_dest_action_db["id"] = post_response.json["id"] extra = {"cloned_acion_db": cloned_dest_action_db} LOG.audit( "Action cloned. Action.id=%s" % (cloned_dest_action_db.id), extra=extra @@ -403,6 +403,13 @@ def clone(self, dest_data, ref_or_id, requester_user): return post_response except PermissionError as e: LOG.error("No permission to clone the action. Exception was %s", e) + delete_action_files_from_pack( + pack_name=cloned_dest_action_db["pack"], + entry_point=cloned_dest_action_db["entry_point"], + metadata_file=cloned_dest_action_db["metadata_file"], + ) + if post_response.status_code == http_client.CREATED: + Action.delete(cloned_dest_action_db) if dest_action_db: self._restore_action(dest_action_db, dest_pack_base_path, temp_sub_dir) abort(http_client.FORBIDDEN, six.text_type(e)) @@ -416,8 +423,11 @@ def clone(self, dest_data, ref_or_id, requester_user): entry_point=cloned_dest_action_db["entry_point"], metadata_file=cloned_dest_action_db["metadata_file"], ) + if post_response.status_code == http_client.CREATED: + Action.delete(cloned_dest_action_db) if dest_action_db: self._restore_action(dest_action_db, dest_pack_base_path, temp_sub_dir) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) def _handle_data_files(self, pack_ref, data_files): From 02bac392332a1c790564c403bc67449ca381d018 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 14 Oct 2021 22:40:44 +0530 Subject: [PATCH 31/40] Refactoring unit tests for newly updated clone api function for removing cloned action database entry in case of exception in operation --- st2api/tests/unit/controllers/v1/test_actions.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index ddb0f11675..8225af15e2 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -1007,7 +1007,6 @@ def test_clone_permission_error(self, mock_clone_action): msg = "No permission to access the files for cloning operation" mock_clone_action.side_effect = PermissionError(msg) source_post_resp = self.__do_post(ACTION_16) - dest_post_resp = self.__do_post(ACTION_17) dest_data_body = { "dest_pack": ACTION_17["pack"], "dest_action": "clone_action_3", @@ -1021,7 +1020,6 @@ def test_clone_permission_error(self, mock_clone_action): self.assertEqual(clone_resp.status_int, 403) self.assertEqual(clone_resp.json["faultstring"], msg) self.__do_delete(self.__get_action_id(source_post_resp)) - self.__do_delete(self.__get_action_id(dest_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") @@ -1033,7 +1031,6 @@ def test_clone_exception(self, mock_clone_action, mock_delete_files): msg = "Exception encountered during cloning action." mock_clone_action.side_effect = Exception(msg) source_post_resp = self.__do_post(ACTION_16) - dest_post_resp = self.__do_post(ACTION_17) dest_data_body = { "dest_pack": ACTION_17["pack"], "dest_action": "clone_action_4", @@ -1049,7 +1046,6 @@ def test_clone_exception(self, mock_clone_action, mock_delete_files): # asserting delete_action_files_from_pack function called i.e. cloned files are cleaned up self.assertTrue(mock_delete_files.called) self.__do_delete(self.__get_action_id(source_post_resp)) - self.__do_delete(self.__get_action_id(dest_post_resp)) @mock.patch.object(os.path, "isdir", mock.MagicMock(return_value=True)) @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") From 44fb50e4143e136b3dbd8e1025184c24bba4ff73 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 19 Oct 2021 20:36:56 +0530 Subject: [PATCH 32/40] Deleted unnecessary file __init__.py --- .../fixtures/packs/dummy_pack_23/actions/workflows/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 From a009f8c0a5d986bbe617d7149dbadabbefbd8b48 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 19 Oct 2021 21:11:04 +0530 Subject: [PATCH 33/40] Fixing failing unit tests with `FileNotFoundError`. Creating empty folders and file `/dummy_pack_23/actions/workflows/__init__.py` --- .../fixtures/packs/dummy_pack_23/actions/workflows/__init__.py | 1 + 1 file changed, 1 insertion(+) create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py @@ -0,0 +1 @@ + From 5a8f693c02df2519a6e13fecbeada93b27e36f53 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 19 Oct 2021 22:35:29 +0530 Subject: [PATCH 34/40] Fixing black/flake error - removed blank line From d9a43b31d7f540b49a7d9c4ab35630469c23f80c Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 19 Oct 2021 22:49:48 +0530 Subject: [PATCH 35/40] Fixing black/flake check - added Apache license header and copyright notice --- .../dummy_pack_23/actions/workflows/__init__.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py index 8b13789179..30c83ff44f 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py @@ -1 +1,14 @@ - +# 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 f27900ae87169685d7b7b52cb82604c4f4f017f5 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 19 Oct 2021 23:16:20 +0530 Subject: [PATCH 36/40] Simplified logic of creating entry point folder in /tmp/ directory --- st2common/st2common/services/packs.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index a773df4ba1..1f320b4d92 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -412,15 +412,10 @@ def temp_backup_action_files(pack_base_path, metadata_file, entry_point, temp_su source_file=dest_metadata_file_path, destination_file=temp_metadata_file_path ) if entry_point: - if os.path.split(entry_point)[0] == "": - temp_entry_point_file_path = os.path.join( - temp_dir_path, "actions", entry_point - ) - else: - entry_point_dir = str(os.path.split(entry_point)[0]) - entry_point_dir_path = os.path.join(actions_dir, entry_point_dir) - os.makedirs(entry_point_dir_path) - temp_entry_point_file_path = os.path.join(actions_dir, entry_point) + entry_point_dir = str(os.path.split(entry_point)[0]) + if entry_point_dir != "": + os.makedirs(os.path.join(actions_dir, entry_point_dir)) + temp_entry_point_file_path = os.path.join(actions_dir, entry_point) dest_entry_point_file_path = os.path.join( pack_base_path, "actions", entry_point ) From 91e4c2d0e449f43c0a312a99f5debb60f27f92b5 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 27 Oct 2021 10:48:02 +0530 Subject: [PATCH 37/40] Minor fix for running CIs to fix timeout error in ci/circleci: packages --- st2common/st2common/services/packs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 1f320b4d92..fd4caa61c0 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -445,7 +445,6 @@ def restore_temp_action_files(pack_base_path, metadata_file, entry_point, temp_s def remove_temp_action_files(temp_sub_dir): temp_dir_path = "/tmp/%s" % temp_sub_dir - if os.path.isdir(temp_dir_path): try: shutil.rmtree(temp_dir_path) From 9ad36c4b1e75f9c6b67c6c843d45b1d08bf2d7a4 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 27 Oct 2021 17:45:45 +0530 Subject: [PATCH 38/40] Minor fix in comment for creation actions directory under packs.py --- st2common/st2common/services/packs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index fd4caa61c0..817ad56312 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -335,7 +335,7 @@ def clone_action_files(source_action_db, dest_action_db, dest_pack_base_path): dest_metadata_file_name = dest_action_db["metadata_file"] dest_metadata_file_path = os.path.join(dest_pack_base_path, dest_metadata_file_name) - # creating actions directory if doesn't exist + # creating actions directory in destination pack if doesn't exist ac_dir_path = os.path.join(dest_pack_base_path, "actions") if not os.path.isdir(ac_dir_path): os.mkdir(path=ac_dir_path) From b2cef24cff0a781cdc5603764eb005383618e9e5 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 28 Oct 2021 12:23:36 +0530 Subject: [PATCH 39/40] Adding newly added functions in __all__ list --- st2common/st2common/services/packs.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 817ad56312..6633fbefde 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -40,6 +40,11 @@ "get_pack_from_index", "search_pack_index", "delete_action_files_from_pack", + "clone_action_files", + "clone_action_db", + "temp_backup_action_files", + "restore_temp_action_files", + "remove_temp_action_files", ] EXCLUDE_FIELDS = ["repo_url", "email"] From 91029eb2771dc41f2b6a348995c918970ed5d3dd Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 28 Oct 2021 13:11:37 +0530 Subject: [PATCH 40/40] Adding newly added functions in __all__ list in misc.py --- st2common/st2common/validators/api/misc.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/validators/api/misc.py b/st2common/st2common/validators/api/misc.py index bbf6ec1329..290a81a3bd 100644 --- a/st2common/st2common/validators/api/misc.py +++ b/st2common/st2common/validators/api/misc.py @@ -18,7 +18,10 @@ from st2common.constants.pack import SYSTEM_PACK_NAMES from st2common.exceptions.apivalidation import ValueValidationException -__all__ = ["validate_not_part_of_system_pack"] +__all__ = [ + "validate_not_part_of_system_pack", + "validate_not_part_of_system_pack_by_name", +] def validate_not_part_of_system_pack(resource_db):