Skip to content

Commit

Permalink
Merge branch 'master' into sf-reintroduce-docker-proxy
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelFain authored Dec 22, 2024
2 parents 0115b8f + 019501f commit 120d4e3
Show file tree
Hide file tree
Showing 13 changed files with 303 additions and 6 deletions.
4 changes: 4 additions & 0 deletions .changelog/4670.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- description: Added validation to ensure every silent trigger points to a silent playbook, and vice versa.
type: feature
pr_number: 4670
4 changes: 4 additions & 0 deletions .changelog/4706.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- description: Added support for CaseLayouts and CaseFields content items paths to the `validate-content-paths` **pre-commit** hook.
type: feature
pr_number: 4706
4 changes: 4 additions & 0 deletions .changelog/4717.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- description: Fixed an issue where RM114 falsely failed when it concatenated "Packs/" twice to the file path.
type: fix
pr_number: 4717
4 changes: 4 additions & 0 deletions .changelog/4720.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- description: Exclude silent items from release notes validation.
type: feature
pr_number: 4720
2 changes: 2 additions & 0 deletions demisto_sdk/commands/validate/sdk_validation_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ select = [
"PB126",
"PB127",
"PB130",
"PB131",
"PB114",
"BA100",
"BA101",
Expand Down Expand Up @@ -334,6 +335,7 @@ select = [
"PB119",
"PB127",
"PB130",
"PB131",
"BC100",
"BC101",
"BC102",
Expand Down
159 changes: 159 additions & 0 deletions demisto_sdk/commands/validate/tests/PB_validators_test.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import pytest

from demisto_sdk.commands.content_graph.objects.base_playbook import TaskConfig
from demisto_sdk.commands.content_graph.objects.pack_content_items import (
PackContentItems,
)
from demisto_sdk.commands.content_graph.objects.playbook import Playbook
from demisto_sdk.commands.validate.tests.test_tools import (
create_playbook_object,
create_trigger_object,
)
from demisto_sdk.commands.validate.validators.PB_validators.PB100_is_no_rolename import (
IsNoRolenameValidator,
Expand Down Expand Up @@ -66,6 +70,9 @@
from demisto_sdk.commands.validate.validators.PB_validators.PB130_is_silent_playbook import (
IsSilentPlaybookValidator,
)
from demisto_sdk.commands.validate.validators.PB_validators.PB131_is_silent_playbook_relationships import (
IsSilentPlaybookRelationshipsValidator,
)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -1423,3 +1430,155 @@ def test_IsSilentPlaybookValidator(name, id, is_silent, result_len):
[playbook]
)
assert result_len == len(invalid_content_items)


class Pack:
content_items = PackContentItems()


@pytest.mark.parametrize(
"playbook_id, playbook_is_silent, trigger_playbook_id, trigger_is_silent, result_len",
[
(
"test",
True,
"test",
False,
1,
),
(
"test",
True,
"test",
True,
0,
),
(
"test1",
True,
"test2",
False,
1,
),
(
"test1",
True,
"test2",
True,
1,
),
(
"test1",
False,
"test1",
True,
0,
),
],
)
def test_IsSilentPlaybookRelationshipsValidator(
playbook_id, playbook_is_silent, trigger_playbook_id, trigger_is_silent, result_len
):
"""
Given:
- case 1: A silent trigger that points on a non-silent playbook.
- case 2: A silent trigger that points on a silent playbook.
- case 3: A silent trigger that points on a non-silent playbook that is not found.
- case 4: A silent trigger that points on a silent playbook that is not found.
- case 5: A non-silent trigger that points on a silent playbook.
When:
- Calling IsSilentPlaybookRelationshipsValidator for playbooks.
Then:
- Validate that only invalid items are returned.
"""
playbook_item = create_playbook_object()
playbook_item.pack = Pack()
playbook_item.pack.content_items.trigger.extend([create_trigger_object()])

playbook_item.data["id"] = playbook_id
playbook_item.is_silent = playbook_is_silent

playbook_item.pack.content_items.trigger[0].data["playbook_id"] = (
trigger_playbook_id
)
playbook_item.pack.content_items.trigger[0].is_silent = trigger_is_silent

invalid_content_items = (
IsSilentPlaybookRelationshipsValidator().obtain_invalid_content_items(
[playbook_item]
)
)
assert result_len == len(invalid_content_items)


@pytest.mark.parametrize(
"trigger_playbook_id, trigger_is_silent, playbook_id, playbook_is_silent, result_len",
[
(
"test",
True,
"test",
False,
1,
),
(
"test",
True,
"test",
True,
0,
),
(
"test1",
True,
"test2",
False,
1,
),
(
"test1",
True,
"test2",
True,
1,
),
(
"test1",
False,
"test1",
True,
0,
),
],
)
def test_IsSilentTriggerRelationshipsValidator(
trigger_playbook_id, trigger_is_silent, playbook_id, playbook_is_silent, result_len
):
"""
Given:
- case 1: A silent playbook that corresponds to a non-silent trigger.
- case 2: A silent playbook that corresponds to a silent trigger.
- case 3: A silent playbook that corresponds to a non-silent trigger that is not found.
- case 4: A silent playbook that corresponds to a silent trigger that is not found.
- case 5: A non-silent playbook that corresponds to a silent trigger.
When:
- Calling IsSilentPlaybookRelationshipsValidator for playbooks.
Then:
- Validate that only invalid items are returned.
"""
trigger_item = create_trigger_object()
trigger_item.pack = Pack()
trigger_item.pack.content_items.playbook.extend([create_playbook_object()])

trigger_item.data["playbook_id"] = trigger_playbook_id
trigger_item.is_silent = trigger_is_silent

trigger_item.pack.content_items.playbook[0].data["id"] = playbook_id
trigger_item.pack.content_items.playbook[0].is_silent = playbook_is_silent

invalid_content_items = (
IsSilentPlaybookRelationshipsValidator().obtain_invalid_content_items(
[trigger_item]
)
)
assert result_len == len(invalid_content_items)
36 changes: 36 additions & 0 deletions demisto_sdk/commands/validate/tests/RM_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import more_itertools
import pytest
from click.exceptions import BadParameter

from demisto_sdk.commands.common.tools import find_pack_folder
from demisto_sdk.commands.validate.tests.test_tools import (
Expand Down Expand Up @@ -343,6 +344,41 @@ def test_IsImageExistsInReadmeValidator_obtain_invalid_content_items(
)


def test_IsImageExistsInReadmeValidator_invalid_image_paths(mocker):
"""
Given:
- A pack name and a list of image paths, some with a missing prefix and others already valid.
When:
- Running the `get_invalid_image_paths` function to validate image paths.
Then:
- Ensure that initially invalid paths are identified.
- Ensure that removing the prefix allows paths to be validated successfully.
"""
import click

pack_name = "MyPack"
image_paths = [
"../doc_files/valid_image.png", # Will be valid after adding prefix
]

# Mocking click.Path.convert to simulate file validation
def mock_first_convert(path, param, ctx):
# Simulate failed validation for all paths during second call
raise BadParameter

def mock_second_convert(path, param, ctx):
# Simulate successful validation for all paths during second call
pass

mocker.patch.object(
click.Path, "convert", side_effect=[mock_first_convert, mock_second_convert]
)
invalid_paths = IsImageExistsInReadmeValidator.get_invalid_image_paths(
pack_name, image_paths
)
assert not invalid_paths


def test_IsPackReadmeNotEqualPackDescriptionValidator_not_valid():
"""
Given:
Expand Down
8 changes: 6 additions & 2 deletions demisto_sdk/commands/validate/tests/RN_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ def mock_is_missing_rn_validator(mocker):
]:
mocker.patch(
f"demisto_sdk.commands.validate.validators.RN_validators.{rn_validator}.should_skip_rn_check",
side_effect=lambda c: isinstance(c, TestPlaybook),
side_effect=lambda c: isinstance(c, TestPlaybook) or c.is_silent,
)
mocker.patch(
f"demisto_sdk.commands.validate.validators.RN_validators.{rn_validator}.was_rn_added",
Expand Down Expand Up @@ -691,7 +691,7 @@ def test_IsMissingReleaseNotes_for_api_module_dependents(
def test_IsMissingReleaseNoteEntries(mock_is_missing_rn_validator):
"""
Given:
- pack1 with a modified integration, a RM entry exists
- pack1 with a modified integration and silent playbook, a RN entry exists only for the integration
- pack2 with modified script, playbook and TPB, a RN entry exists only for the script
- pack3 with a modified script, a RN file does not exist
- pack4 with a new mapper, a RN exists
Expand All @@ -709,6 +709,9 @@ def test_IsMissingReleaseNoteEntries(mock_is_missing_rn_validator):
release_note_content=f"#### Integrations\n##### {integ.display_name}\n- Added x y z",
)
integ.pack = pack1
silent_playbook = create_playbook_object()
silent_playbook.is_silent = True
silent_playbook.pack = pack1

script1 = create_script_object()
playbook = create_playbook_object()
Expand Down Expand Up @@ -740,6 +743,7 @@ def test_IsMissingReleaseNoteEntries(mock_is_missing_rn_validator):
content_items = [
pack1,
integ,
silent_playbook,
pack2,
script1,
playbook,
Expand Down
3 changes: 3 additions & 0 deletions demisto_sdk/commands/validate/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ def should_skip_rn_check(content_item: ContentItem) -> bool:
- A modeling rule is considered modified if its XIF and schema files are modified
- A movement between packs is considered a modification
- Test content items shouldn't have RNs
- Silent content items shouldn't have RNs
Args:
content_item (ContentItem): A content item object
Expand All @@ -279,6 +280,8 @@ def should_skip_rn_check(content_item: ContentItem) -> bool:
"""
if isinstance(content_item, (TestPlaybook, TestScript)):
return True
if content_item.is_silent:
return True
if isinstance(content_item, Integration):
return (
not content_item.git_status and not content_item.description_file.git_status
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from __future__ import annotations

from typing import Iterable, List, Union

from demisto_sdk.commands.content_graph.common import ContentType
from demisto_sdk.commands.content_graph.objects.content_item import ContentItem
from demisto_sdk.commands.content_graph.objects.playbook import Playbook
from demisto_sdk.commands.content_graph.objects.trigger import Trigger
from demisto_sdk.commands.validate.validators.base_validator import (
BaseValidator,
ValidationResult,
)

ContentTypes = Union[Playbook, Trigger]


class IsSilentPlaybookRelationshipsValidator(BaseValidator[ContentTypes]):
error_code = "PB131"
description = "Every silent trigger must correspond to a silent playbook in the same pack, and vice versa."
rationale = "To ensure the effective operation of the silent playbook items."
error_message = (
"The {} is silent, but does not correspond to a silent {} in the pack."
)
related_field = "isSilent"
is_auto_fixable = False

def obtain_invalid_content_items(
self, content_items: Iterable[ContentTypes]
) -> List[ValidationResult]:
def get_types_for_err_msg(c: ContentItem) -> tuple[ContentType, ContentType]:
if isinstance(c, Trigger):
return ContentType.TRIGGER, ContentType.PLAYBOOK
return ContentType.PLAYBOOK, ContentType.TRIGGER

return [
ValidationResult(
validator=self,
message=self.error_message.format(*get_types_for_err_msg(content_item)),
content_object=content_item,
)
for content_item in content_items
if not is_valid_relationship(content_item)
]


def is_valid_relationship(content_item: ContentTypes) -> bool:
"""
Validates the relationship between a content item and its associated playbook/trigger.
"""
if not content_item.is_silent:
# If the content item is not marked as silent, it's automatically valid
return True

if content_item.content_type == ContentType.PLAYBOOK:
return any(
trigger.data.get("playbook_id") == content_item.data.get("id")
and trigger.is_silent
for trigger in content_item.pack.content_items.trigger
)

if content_item.content_type == ContentType.TRIGGER:
return any(
playbook.data.get("id") == content_item.data.get("playbook_id")
and playbook.is_silent
for playbook in content_item.pack.content_items.playbook
)

# Default case if content type is not PLAYBOOK or TRIGGER
return True
Loading

0 comments on commit 120d4e3

Please sign in to comment.