From 14704662e70e96c210a65bcfdee6e8dc5dfb6e97 Mon Sep 17 00:00:00 2001 From: Sapir Shuker <49246861+sapirshuker@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:15:25 +0300 Subject: [PATCH 1/4] PB106 (#4360) * PB106 * add changelogs * fixes and add validation config * Update sdk_validation_config.toml * fix CR * fix CR review * fix merge from master * fix merge from master * Update sdk_validation_config.toml * reomve related_file_type = [RelatedFileType.YML] * fixes * fixes --- .changelog/4360.yml | 4 ++ .../validate/sdk_validation_config.toml | 4 +- .../validate/tests/PB_validators_test.py | 69 +++++++++++++++++++ .../PB106_is_playbook_using_an_instance.py | 63 +++++++++++++++++ 4 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 .changelog/4360.yml create mode 100644 demisto_sdk/commands/validate/validators/PB_validators/PB106_is_playbook_using_an_instance.py diff --git a/.changelog/4360.yml b/.changelog/4360.yml new file mode 100644 index 0000000000..90151f4c3c --- /dev/null +++ b/.changelog/4360.yml @@ -0,0 +1,4 @@ +changes: +- description: Converted PB106 validation to new validation format. The validation checks if the playbook uses a specific instance. + type: internal +pr_number: 4360 diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index 7ca1a40638..68e4082d40 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -30,7 +30,7 @@ select = [ "IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160", "IN161", "IN162", "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB118", "PB122", "PB123", "PB125", "PB126", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB118", "PB122", "PB123", "PB126", + "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB118", "PB122", "PB123", "PB126", "PB106", "DO100", "DO101", "DO102", "DO103", "DO104", "DO105", "DO106", "DS100", "DS101", "DS105", "DS106", "DS107", "SC100", "SC105", "SC106", "SC109", @@ -54,7 +54,7 @@ select = [ "IN161", "IN162", "LO107", "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB122", "PB123", "PB125", "PB126", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB122", "PB123", "PB126", + "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB122", "PB123", "PB126", "PB106", "BA100", "BA101", "BA105", "BA106", "BA110", "BA111", "BA113", "BA116", "BA118", "BA119", "BA126", "DS100", "DS101", "DS105", "PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120", diff --git a/demisto_sdk/commands/validate/tests/PB_validators_test.py b/demisto_sdk/commands/validate/tests/PB_validators_test.py index 8befccd388..8f72932565 100644 --- a/demisto_sdk/commands/validate/tests/PB_validators_test.py +++ b/demisto_sdk/commands/validate/tests/PB_validators_test.py @@ -1,6 +1,7 @@ import pytest from demisto_sdk.commands.content_graph.objects.base_playbook import TaskConfig +from demisto_sdk.commands.content_graph.objects.playbook import Playbook from demisto_sdk.commands.validate.tests.test_tools import create_playbook_object from demisto_sdk.commands.validate.validators.PB_validators.PB100_is_no_rolename import ( IsNoRolenameValidator, @@ -18,6 +19,9 @@ from demisto_sdk.commands.validate.validators.PB_validators.PB105_playbook_delete_context_all import ( PlaybookDeleteContextAllValidator, ) +from demisto_sdk.commands.validate.validators.PB_validators.PB106_is_playbook_using_an_instance import ( + IsPlayBookUsingAnInstanceValidator, +) from demisto_sdk.commands.validate.validators.PB_validators.PB108_is_valid_task_id import ( IsValidTaskIdValidator, ) @@ -599,3 +603,68 @@ def test_IsDefaultNotOnlyConditionValidator(): ) } assert IsDefaultNotOnlyConditionValidator().is_valid([playbook]) + + +def test_IsPlayBookUsingAnInstanceValidator_is_valid(): + """ + Given: + - A playbook + Case 1: The playbook is valid. + Case 2: The playbook isn't valid, it has using field. + When: + - calling IsPlayBookUsingAnInstanceValidator.is_valid. + Then: + - The results should be as expected: + Case 1: The playbook is valid + Case 2: The playbook is invalid + """ + # Case 1 + valid_playbook = create_playbook_object() + valid_result = IsPlayBookUsingAnInstanceValidator().is_valid([valid_playbook]) + + # Case 2 + invalid_playbook = create_playbook_object() + for _, task in invalid_playbook.tasks.items(): + task.scriptarguments = {"using": "instance_name"} + results_invalid = IsPlayBookUsingAnInstanceValidator().is_valid([invalid_playbook]) + + assert valid_result == [] + assert results_invalid != [] + assert results_invalid[0].message == ( + "Playbook should not use specific instance for tasks: {0}.".format( + ", ".join([task.taskid for task in invalid_playbook.tasks.values()]) + ) + ) + + +def test_IsPlayBookUsingAnInstanceValidator_fix(): + """ + Given: + - A playbook + Case 1: The playbook isn't valid, it will be fixed. + When: + - calling IsPlayBookUsingAnInstanceValidator.fix. + Then: + - The message appears with the invalid tasks. + """ + + # Case 1 + invalid_playbook = create_playbook_object() + for _, task in invalid_playbook.tasks.items(): + task.scriptarguments = {"using": "instance_name", "some_key": "value"} + validator_invalid_playbook = IsPlayBookUsingAnInstanceValidator() + validator_invalid_playbook.invalid_tasks[invalid_playbook.name] = [ + task for task in invalid_playbook.tasks.values() + ] + fix_validator = validator_invalid_playbook.fix(invalid_playbook) + fix_message = fix_validator.message + fixed_content_item: Playbook = fix_validator.content_object + expected_message = ( + "Removed The 'using' statement from the following tasks tasks: {0}.".format( + ", ".join([task.taskid for task in invalid_playbook.tasks.values()]) + ) + ) + assert fix_message == expected_message + for tasks in fixed_content_item.tasks.values(): + scriptargs = tasks.scriptarguments + assert scriptargs == {"some_key": "value"} diff --git a/demisto_sdk/commands/validate/validators/PB_validators/PB106_is_playbook_using_an_instance.py b/demisto_sdk/commands/validate/validators/PB_validators/PB106_is_playbook_using_an_instance.py new file mode 100644 index 0000000000..28f59e1c91 --- /dev/null +++ b/demisto_sdk/commands/validate/validators/PB_validators/PB106_is_playbook_using_an_instance.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +from typing import ClassVar, Dict, Iterable, List + +from demisto_sdk.commands.content_graph.objects.base_playbook import TaskConfig +from demisto_sdk.commands.content_graph.objects.playbook import Playbook +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + FixResult, + ValidationResult, +) + +ContentTypes = Playbook + + +class IsPlayBookUsingAnInstanceValidator(BaseValidator[ContentTypes]): + invalid_tasks: ClassVar[dict] = {} + error_code = "PB106" + description = "Validate whether the playbook does not use an instance. If the Playbook use an instance it is not valid." + rationale = "If the playbook uses a specific instance it can leads to errors because not all the users have the same instance." + error_message = "Playbook should not use specific instance for tasks: {0}." + fix_message = "Removed The 'using' statement from the following tasks tasks: {0}." + related_field = "using" + is_auto_fixable = True + + def is_playbook_using_an_instance( + self, content_item: ContentTypes + ) -> list[TaskConfig]: + content_item_tasks: Dict[str, TaskConfig] = content_item.tasks + invalid_tasks: list[TaskConfig] = [] + for _, task in content_item_tasks.items(): + scriptargs = task.scriptarguments + if scriptargs and scriptargs.get("using"): + invalid_tasks.append(task) + self.invalid_tasks[content_item.name] = invalid_tasks + return invalid_tasks + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + ", ".join([task.taskid for task in invalid_tasks]) + ), + content_object=content_item, + ) + for content_item in content_items + if (invalid_tasks := self.is_playbook_using_an_instance(content_item)) + ] + + def fix(self, content_item: ContentTypes) -> FixResult: + invalid_tasks = self.invalid_tasks[content_item.name] + for invalid_task in invalid_tasks: + scriptargs = invalid_task.scriptarguments + if scriptargs and scriptargs.get("using", {}): + scriptargs.pop("using") + return FixResult( + validator=self, + message=self.fix_message.format( + ", ".join([task.taskid for task in invalid_tasks]) + ), + content_object=content_item, + ) From 821571882941f74b005dfdba98b81275ab01ca7a Mon Sep 17 00:00:00 2001 From: anas-yousef <44998563+anas-yousef@users.noreply.github.com> Date: Wed, 19 Jun 2024 15:35:37 +0300 Subject: [PATCH 2/4] Added fix for auto update docker image field (#4287) * Added fix * Added chagelog * Added default value and updated UT * Added new line --- .changelog/4287.yml | 4 ++++ .../content_graph/parsers/integration_script.py | 4 +--- .../content_graph/tests/parsers_and_models_test.py | 11 +++++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 .changelog/4287.yml diff --git a/.changelog/4287.yml b/.changelog/4287.yml new file mode 100644 index 0000000000..6d5085cea2 --- /dev/null +++ b/.changelog/4287.yml @@ -0,0 +1,4 @@ +changes: +- description: Added a fix where the `auto_update_docker_image` field was not updated correctly in the the content-graph. + type: fix +pr_number: 4287 diff --git a/demisto_sdk/commands/content_graph/parsers/integration_script.py b/demisto_sdk/commands/content_graph/parsers/integration_script.py index 08add557d5..462e48f52e 100644 --- a/demisto_sdk/commands/content_graph/parsers/integration_script.py +++ b/demisto_sdk/commands/content_graph/parsers/integration_script.py @@ -89,6 +89,4 @@ def subtype(self): @property def auto_update_docker_image(self): - return ( - get_value(self.yml_data, "autoUpdateDockerImage", "") - ).lower() != "false" + return get_value(self.yml_data, "autoUpdateDockerImage", True) diff --git a/demisto_sdk/commands/content_graph/tests/parsers_and_models_test.py b/demisto_sdk/commands/content_graph/tests/parsers_and_models_test.py index 32574aa33a..0128d93e00 100644 --- a/demisto_sdk/commands/content_graph/tests/parsers_and_models_test.py +++ b/demisto_sdk/commands/content_graph/tests/parsers_and_models_test.py @@ -35,7 +35,7 @@ def content_items_to_node_ids( - content_items_dict: Dict[ContentType, List[str]] + content_items_dict: Dict[ContentType, List[str]], ) -> Set[str]: """A helper method that converts a dict of content items to a set of their node ids.""" return { @@ -1272,7 +1272,14 @@ def test_script_parser(self, pack: Pack): @pytest.mark.parametrize( "raw_value, expected_value", - [("false", False), ("true", True), ("tRue", True), ("something", True)], + [ + (False, False), + (True, True), + ("true", True), + ("false", False), + ("tRue", True), + ("faLse", False), + ], ) def test_script_parser_set_autoupdate(self, raw_value, expected_value, pack: Pack): """ From ae3017ab0d8dec9be2528ab1dfe96cbcdd7c8cf2 Mon Sep 17 00:00:00 2001 From: azonenfeld <117573492+aaron1535@users.noreply.github.com> Date: Wed, 19 Jun 2024 16:29:07 +0300 Subject: [PATCH 3/4] new PB115 validate (#4366) * new PB115 validate * CR issues * fix the fix * add config * add description * pre-commit issues --- .changelog/4366.yml | 4 + .../validate/sdk_validation_config.toml | 6 +- .../validate/tests/PB_validators_test.py | 107 ++++++++++++++++++ .../PB115_is_tasks_quiet_mode.py | 94 +++++++++++++++ 4 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 .changelog/4366.yml create mode 100644 demisto_sdk/commands/validate/validators/PB_validators/PB115_is_tasks_quiet_mode.py diff --git a/.changelog/4366.yml b/.changelog/4366.yml new file mode 100644 index 0000000000..b38361f55f --- /dev/null +++ b/.changelog/4366.yml @@ -0,0 +1,4 @@ +changes: +- description: Converted PB115 validation to new validation format. The validation checks if the 'quietmode' field of all tasks in playbook are not in default value. + type: internal +pr_number: 4366 diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index 68e4082d40..2e2ab2bde2 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -29,8 +29,7 @@ select = [ "IN100", "IN101", "IN102", "IN104", "IN106", "IN107", "IN108", "IN109", "IN110", "IN112", "IN113", "IN114", "IN115", "IN117", "IN118", "IN121", "IN122", "IN123", "IN124", "IN125", "IN126", "IN127", "IN130", "IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160", "IN161", "IN162", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB118", "PB122", "PB123", "PB125", "PB126", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB118", "PB122", "PB123", "PB126", "PB106", + "PB100", "PB101", "PB103", "PB104", "PB105", "PB106", "PB108", "PB115", "PB118", "PB122", "PB123", "PB125", "PB126", "DO100", "DO101", "DO102", "DO103", "DO104", "DO105", "DO106", "DS100", "DS101", "DS105", "DS106", "DS107", "SC100", "SC105", "SC106", "SC109", @@ -53,8 +52,7 @@ select = [ "IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160", "IN161", "IN162", "LO107", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB122", "PB123", "PB125", "PB126", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB108", "PB122", "PB123", "PB126", "PB106", + "PB100", "PB101", "PB103", "PB104", "PB105", "PB106", "PB108", "PB115", "PB122", "PB123", "PB125", "PB126", "BA100", "BA101", "BA105", "BA106", "BA110", "BA111", "BA113", "BA116", "BA118", "BA119", "BA126", "DS100", "DS101", "DS105", "PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120", diff --git a/demisto_sdk/commands/validate/tests/PB_validators_test.py b/demisto_sdk/commands/validate/tests/PB_validators_test.py index 8f72932565..a9a17ba0fb 100644 --- a/demisto_sdk/commands/validate/tests/PB_validators_test.py +++ b/demisto_sdk/commands/validate/tests/PB_validators_test.py @@ -25,6 +25,9 @@ from demisto_sdk.commands.validate.validators.PB_validators.PB108_is_valid_task_id import ( IsValidTaskIdValidator, ) +from demisto_sdk.commands.validate.validators.PB_validators.PB115_is_tasks_quiet_mode import ( + IsTasksQuietModeValidator, +) from demisto_sdk.commands.validate.validators.PB_validators.PB118_is_input_key_not_in_tasks import ( IsInputKeyNotInTasksValidator, ) @@ -327,6 +330,110 @@ def test_IsAskConditionHasUnhandledReplyOptionsValidator(): assert IsAskConditionHasUnhandledReplyOptionsValidator().is_valid([playbook]) +def test_IsTasksQuietModeValidator_fail_case(): + """ + Given: + - A invalid playbook with tasks that "quietmode" field is 2 + - An invalid playbook to fix + + When: + - calling IsTasksQuietModeValidator.is_valid. + - calling IsTasksQuietModeValidator.fix + + Then: + - The playbook is invalid + -The playbook becomes valid + """ + playbook = create_playbook_object( + ["inputs", "quiet", "tasks"], + [ + [ + { + "value": {}, + "required": False, + "description": "", + "playbookInputQuery": {"query": "", "queryEntity": "indicators"}, + } + ], + False, + { + "0": { + "id": "test fail task No1", + "taskid": "27b9c747-b883-4878-8b60-7f352098a631", + "type": "condition", + "message": {"replyOptions": ["yes"]}, + "nexttasks": {"no": ["1"]}, + "task": {"id": "27b9c747-b883-4878-8b60-7f352098a63c"}, + "quietmode": 2, + }, + "1": { + "id": "test fail task No2", + "taskid": "27b9c747-b883-4878-8b60-7f352098a631", + "type": "condition", + "message": {"replyOptions": ["yes"]}, + "nexttasks": {"no": ["1"]}, + "task": {"id": "27b9c747-b883-4878-8b60-7f352098a63c"}, + "quietmode": 2, + }, + }, + ], + pack_info={}, + ) + validator = IsTasksQuietModeValidator() + validate_res = validator.is_valid([playbook]) + assert len(validate_res) == 1 + assert ( + (validate_res[0]).message + == "Playbook 'Detonate File - JoeSecurity V2' contains tasks that are not in quiet mode (quietmode: 2) The tasks names is: 'test fail task No1, test fail task No2'." + ) + fix_playbook = validator.fix(playbook).content_object + assert len(validator.is_valid([fix_playbook])) == 0 + + +def test_IsTasksQuietModeValidator_pass_case(): + """ + Given: + - A valid playbook with tasks that "quietmode" field is 1 + + When: + - calling IsTasksQuietModeValidator.is_valid. + - calling IsTasksQuietModeValidator.fix + + Then: + - The playbook is valid + - The playbook doesn't changed + """ + playbook = create_playbook_object( + ["inputs", "quiet", "tasks"], + [ + [ + { + "value": {}, + "required": False, + "description": "", + "playbookInputQuery": {"query": "", "queryEntity": "indicators"}, + } + ], + False, + { + "0": { + "id": "test task", + "taskid": "27b9c747-b883-4878-8b60-7f352098a631", + "type": "condition", + "message": {"replyOptions": ["yes"]}, + "nexttasks": {"no": ["1"]}, + "task": {"id": "27b9c747-b883-4878-8b60-7f352098a63c"}, + "quietmode": 1, + } + }, + ], + ) + validator = IsTasksQuietModeValidator() + assert len(validator.is_valid([playbook])) == 0 + fix_playbook = validator.fix(playbook).content_object + assert fix_playbook == playbook + + def test_PB125_playbook_only_default_next_valid(): """ Given: diff --git a/demisto_sdk/commands/validate/validators/PB_validators/PB115_is_tasks_quiet_mode.py b/demisto_sdk/commands/validate/validators/PB_validators/PB115_is_tasks_quiet_mode.py new file mode 100644 index 0000000000..824419a0b8 --- /dev/null +++ b/demisto_sdk/commands/validate/validators/PB_validators/PB115_is_tasks_quiet_mode.py @@ -0,0 +1,94 @@ +from __future__ import annotations + +from typing import Dict, Iterable, List + +from demisto_sdk.commands.content_graph.objects.base_playbook import TaskConfig +from demisto_sdk.commands.content_graph.objects.playbook import Playbook +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + FixResult, + ValidationResult, +) + +ContentTypes = Playbook + + +class IsTasksQuietModeValidator(BaseValidator[ContentTypes]): + error_code = "PB115" + description = "Checks if the 'quietmode' field of all tasks in playbook are not in default value." + rationale = "Confirmation of turning off quitmode" + error_message = "Playbook '{playbook_name}' contains tasks that are not in quiet mode (quietmode: 2) The tasks names is: '{tasks}'." + fix_message = ( + "Fixed playbook '{playbook_name}' to set tasks '{tasks}' to (quietmode: 0)." + ) + related_field = "tasks" + is_auto_fixable = True + invalid_tasks_in_playbooks: Dict[str, List[TaskConfig]] = {} + + def get_invalid_task_ids(self, content_item: Playbook) -> List[TaskConfig]: + """ + Identify tasks with quietmode == 2 and update self.invalid_tasks_in_playbooks with these tasks. + + Args: + content_item (ContentTypes): The content item to check. + + Returns: + List[str]: List of task IDs where quietmode == 2. + """ + invalid_task_ids = [ + task for task in list(content_item.tasks.values()) if task.quietmode == 2 + ] + self.invalid_tasks_in_playbooks[content_item.name] = invalid_task_ids + return invalid_task_ids + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + """ + Validates that tasks in content_items(playbook) are in quiet mode if they contain an input query for "indicators". + + Args: + content_items (Iterable[ContentTypes]): Content items to validate. + + Returns: + List[ValidationResult]: Validation results for items not meeting criteria. + """ + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + playbook_name=content_item.name, + tasks=", ".join([task.id for task in invalid_tasks]), + ), + content_object=content_item, + ) + for content_item in content_items + if ( + any( + (input.get("playbookInputQuery") or {}).get("queryEntity") + == "indicators" + for input in content_item.data.get("inputs", {}) + ) + and (invalid_tasks := self.get_invalid_task_ids(content_item)) + ) + ] + + def fix(self, content_item: ContentTypes) -> FixResult: + """ + Sets quietmode to 0 for all tasks with quietmode set to 2 in the given content item. + + Args: + content_item (ContentTypes): The content item to fix. + + Returns: + FixResult: The result of the fix operation. + """ + invalid_tasks = self.invalid_tasks_in_playbooks.get(content_item.name, []) + for task in invalid_tasks: + task.quietmode = 0 + return FixResult( + validator=self, + message=self.fix_message.format( + playbook_name=content_item.name, + tasks=", ".join([task.id for task in invalid_tasks]), + ), + content_object=content_item, + ) From d35f9d91e79fda55e7a3e0e3ab30a161738ab2a1 Mon Sep 17 00:00:00 2001 From: noydavidi <77931201+noydavidi@users.noreply.github.com> Date: Wed, 19 Jun 2024 17:08:54 +0300 Subject: [PATCH 4/4] refactored is_taskid_equals_id validation (#4358) * refactored is_taskid_equals_id validation * created changelog file * changes was made after running build * run precommit local and made automatic chanages * deleted PB109 from the select in the Custom Categories * changes was made after code review, change logic of the validation function and changes the unittests * changes after pre commit * changes after build * precommit * changes after second code review * updated config.toml file * Update demisto_sdk/commands/validate/tests/PB_validators_test.py * merged with master --------- Co-authored-by: noy Co-authored-by: Yuval Hayun <70104171+YuvHayun@users.noreply.github.com> --- .changelog/4358.yml | 4 ++ .../validate/sdk_validation_config.toml | 4 +- .../validate/tests/PB_validators_test.py | 42 ++++++++++++++++- .../PB109_is_taskid_equals_id.py | 46 +++++++++++++++++++ 4 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 .changelog/4358.yml create mode 100644 demisto_sdk/commands/validate/validators/PB_validators/PB109_is_taskid_equals_id.py diff --git a/.changelog/4358.yml b/.changelog/4358.yml new file mode 100644 index 0000000000..916d9e5784 --- /dev/null +++ b/.changelog/4358.yml @@ -0,0 +1,4 @@ +changes: +- description: Moved PB109 to the new validate format. The validation checks that taskid field and id field under task field contains equal values. + type: internal +pr_number: 4358 diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index 2e2ab2bde2..438b3fe6fd 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -29,7 +29,7 @@ select = [ "IN100", "IN101", "IN102", "IN104", "IN106", "IN107", "IN108", "IN109", "IN110", "IN112", "IN113", "IN114", "IN115", "IN117", "IN118", "IN121", "IN122", "IN123", "IN124", "IN125", "IN126", "IN127", "IN130", "IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160", "IN161", "IN162", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB106", "PB108", "PB115", "PB118", "PB122", "PB123", "PB125", "PB126", + "PB100", "PB101", "PB103", "PB104", "PB105", "PB106", "PB108", "PB109", "PB115", "PB118", "PB122", "PB123", "PB125", "PB126", "DO100", "DO101", "DO102", "DO103", "DO104", "DO105", "DO106", "DS100", "DS101", "DS105", "DS106", "DS107", "SC100", "SC105", "SC106", "SC109", @@ -52,7 +52,7 @@ select = [ "IN131", "IN134", "IN135", "IN139", "IN141", "IN142", "IN144", "IN145", "IN146", "IN149", "IN150", "IN151", "IN152", "IN153", "IN154", "IN156", "IN158", "IN159", "IN160", "IN161", "IN162", "LO107", - "PB100", "PB101", "PB103", "PB104", "PB105", "PB106", "PB108", "PB115", "PB122", "PB123", "PB125", "PB126", + "PB100", "PB101", "PB103", "PB104", "PB105", "PB106", "PB108", "PB109", "PB115", "PB122", "PB123", "PB125", "PB126", "BA100", "BA101", "BA105", "BA106", "BA110", "BA111", "BA113", "BA116", "BA118", "BA119", "BA126", "DS100", "DS101", "DS105", "PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120", diff --git a/demisto_sdk/commands/validate/tests/PB_validators_test.py b/demisto_sdk/commands/validate/tests/PB_validators_test.py index a9a17ba0fb..33d4a69e63 100644 --- a/demisto_sdk/commands/validate/tests/PB_validators_test.py +++ b/demisto_sdk/commands/validate/tests/PB_validators_test.py @@ -25,6 +25,9 @@ from demisto_sdk.commands.validate.validators.PB_validators.PB108_is_valid_task_id import ( IsValidTaskIdValidator, ) +from demisto_sdk.commands.validate.validators.PB_validators.PB109_is_taskid_equals_id import ( + IsTaskidDifferentFromidValidator, +) from demisto_sdk.commands.validate.validators.PB_validators.PB115_is_tasks_quiet_mode import ( IsTasksQuietModeValidator, ) @@ -696,7 +699,6 @@ def test_IsDefaultNotOnlyConditionValidator(): } ) } - assert not IsDefaultNotOnlyConditionValidator().is_valid([playbook]) playbook.tasks = { "0": TaskConfig( @@ -712,6 +714,44 @@ def test_IsDefaultNotOnlyConditionValidator(): assert IsDefaultNotOnlyConditionValidator().is_valid([playbook]) +def test_IsTaskidDifferentFromidValidator(): + """ + Given: + - A playbook with tasks, taskid and id + Case 1: id equals taskid + Case 2: id not equals taskid + + When: + - Validating the playbook + + Then: + - The results should be as expected: + Case 1: an empty list + Case 2: a list in length 1 because there is one error + """ + playbook = create_playbook_object() + results = IsTaskidDifferentFromidValidator().is_valid([playbook]) + assert len(results) == 0 + playbook.tasks = { + "0": TaskConfig( + **{ + "id": "test", + "taskid": "test1", + "type": "condition", + "message": {"replyOptions": ["yes"]}, + "nexttasks": {"no": ["1"]}, + "task": {"id": "test"}, + } + ) + } + results = IsTaskidDifferentFromidValidator().is_valid([playbook]) + assert len(results) == 1 + assert ( + results[0].message + == "On tasks: 0, the field 'taskid' and the 'id' under the 'task' field must be with equal value." + ) + + def test_IsPlayBookUsingAnInstanceValidator_is_valid(): """ Given: diff --git a/demisto_sdk/commands/validate/validators/PB_validators/PB109_is_taskid_equals_id.py b/demisto_sdk/commands/validate/validators/PB_validators/PB109_is_taskid_equals_id.py new file mode 100644 index 0000000000..7dacf1123f --- /dev/null +++ b/demisto_sdk/commands/validate/validators/PB_validators/PB109_is_taskid_equals_id.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.content_graph.objects.test_playbook import BasePlaybook +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +ContentTypes = BasePlaybook + + +class IsTaskidDifferentFromidValidator(BaseValidator[ContentTypes]): + error_code = "PB109" + description = ( + "Check that taskid field and id field under task field contains equal values." + ) + rationale = "System requirements" + error_message = "On tasks: {}, the field 'taskid' and the 'id' under the 'task' field must be with equal value." + related_field = "tasks" + is_auto_fixable = False + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + error_results = [] + + for content_item in content_items: + tasks: dict = content_item.tasks + not_valid_tasks = [] + + for task_key, task in tasks.items(): + if task.taskid != task.task.id: + not_valid_tasks.append(task_key) + + if not_valid_tasks: + error_results.append( + ValidationResult( + validator=self, + message=self.error_message.format( + ", ".join(not_valid_tasks), + ), + content_object=content_item, + ) + ) + + return error_results