Skip to content

Commit

Permalink
Added PB126 (#4349)
Browse files Browse the repository at this point in the history
* Added PB126

* added changelog

* fixed cr comments

* More fixes

* More fixes
  • Loading branch information
ShahafBenYakir committed Jun 18, 2024
1 parent 2fa7ee2 commit 62fa385
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 2 deletions.
4 changes: 4 additions & 0 deletions .changelog/4349.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- description: Added PB126 to the new validation format. Ensure that conditional tasks have more than path which is not the default one
type: internal
pr_number: 4349
4 changes: 2 additions & 2 deletions demisto_sdk/commands/validate/sdk_validation_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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", "PB118", "PB123",
"PB100", "PB101","PB103","PB104", "PB105", "PB118", "PB123", "PB126",
"DO100", "DO101", "DO102", "DO103", "DO104", "DO105", "DO106",
"DS100", "DS101", "DS105", "DS106", "DS107",
"SC100", "SC105", "SC106", "SC109",
Expand All @@ -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", "PB123",
"PB100", "PB101","PB103","PB104", "PB105", "PB123", "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",
Expand Down
44 changes: 44 additions & 0 deletions demisto_sdk/commands/validate/tests/PB_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
from demisto_sdk.commands.validate.validators.PB_validators.PB123_is_conditional_task_has_unhandled_reply_options import (
IsAskConditionHasUnhandledReplyOptionsValidator,
)
from demisto_sdk.commands.validate.validators.PB_validators.PB126_is_default_not_only_condition import (
IsDefaultNotOnlyConditionValidator,
)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -361,3 +364,44 @@ def test_does_playbook_have_unconnected_tasks_not_valid():
validation_result = DoesPlaybookHaveUnconnectedTasks().is_valid([playbook])
assert validation_result
assert validation_result[0].message == ERROR_MSG.format(orphan_tasks=orphan_tasks)


def test_IsDefaultNotOnlyConditionValidator():
"""
Given:
Case a: playbook with no conditional tasks.
Case b: playbook with conditional tasks that has two reply options - yes/no.
Case c: playbook with conditional tasks that has one reply options - #default#.
When: Validating the playbook tasks to have more than a default option (IsDefaultNotOnlyConditionValidator).
Then:
Case a: The validation passes (result list of invalid items is empty)
Case b: The validation passes (result list of invalid items is empty)
Case c: The validation fails (result list of invalid items contains the invalid playbook)
"""
playbook = create_playbook_object()
assert not IsDefaultNotOnlyConditionValidator().is_valid([playbook])
playbook.tasks = {
"0": TaskConfig(
**{
"id": "0",
"type": "condition",
"taskid": "27b9c747-b883-4878-8b60-7f352098a631",
"message": {"replyOptions": ["yes", "no"]},
"task": {"id": "27b9c747-b883-4878-8b60-7f352098a63c"},
}
)
}

assert not IsDefaultNotOnlyConditionValidator().is_valid([playbook])
playbook.tasks = {
"0": TaskConfig(
**{
"id": "0",
"type": "condition",
"taskid": "27b9c747-b883-4878-8b60-7f352098a631",
"message": {"replyOptions": ["#default#"]},
"task": {"id": "27b9c747-b883-4878-8b60-7f352098a63c"},
}
)
}
assert IsDefaultNotOnlyConditionValidator().is_valid([playbook])
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from __future__ import annotations

from typing import Iterable, List

from demisto_sdk.commands.common.constants import PlaybookTaskType
from demisto_sdk.commands.content_graph.objects.playbook import Playbook
from demisto_sdk.commands.validate.validators.base_validator import (
BaseValidator,
ValidationResult,
)

ContentTypes = Playbook


class IsDefaultNotOnlyConditionValidator(BaseValidator[ContentTypes]):
error_code = "PB126"
description = (
"Ensure that conditional tasks have an execution path besides for the default."
)
rationale = (
"We want to ensure that conditional tasks have more than path which is not the default one o/w it "
"make no sense to have such."
)
error_message = (
"The following playbook conditional tasks only have a default option: {}. Please remove these tasks or add "
"another non-default option to each task."
)
related_field = "conditions"
is_auto_fixable = False

def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]:
validation_results = []
for content_item in content_items:
failed_tasks = []
for task in content_item.tasks.values():
if (
task.type == PlaybookTaskType.CONDITION
and task.message
and task.message.get("replyOptions")
):
reply_options = set(
map(str.upper, task.message.get("replyOptions", []))
)
if len(reply_options) == 1 and "#default#".upper() in reply_options:
failed_tasks.append(f"Task: {task.id}")
if failed_tasks:
validation_results.append(
ValidationResult(
validator=self,
message=self.error_message.format(failed_tasks),
content_object=content_item,
)
)
return validation_results

0 comments on commit 62fa385

Please sign in to comment.