Skip to content

Commit

Permalink
Merge 04778cf into 419a05f
Browse files Browse the repository at this point in the history
  • Loading branch information
DinaMeylakh committed Jun 20, 2024
2 parents 419a05f + 04778cf commit 243780b
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 7 deletions.
4 changes: 4 additions & 0 deletions .changelog/4342.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- description: Added the PA114 validation to the new **validate** command.
type: internal
pr_number: 4342
4 changes: 2 additions & 2 deletions demisto_sdk/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ def zip_packs(ctx, **kwargs) -> int:
@click.option(
"--skip-pack-dependencies",
is_flag=True,
help="Relevant only for the old validate flow and will be removed in a future release. Skip validation of pack dependencies.",
help="Skip validation of pack dependencies.",
)
@click.option(
"--create-id-set",
Expand Down Expand Up @@ -856,7 +856,6 @@ def validate(ctx, config, file_paths: str, **kwargs):
"print_ignored_files",
"no_docker_checks",
"silence_init_prints",
"skip_pack_dependencies",
"id_set_path",
"create_id_set",
"skip_schema_check",
Expand Down Expand Up @@ -920,6 +919,7 @@ def validate(ctx, config, file_paths: str, **kwargs):
prev_ver=kwargs["prev_ver"],
file_path=file_path,
all_files=kwargs.get("validate_all"),
skip_pack_dependencies=kwargs.get("skip_pack_dependencies"),
)
validator_v2 = ValidateManager(
file_path=file_path,
Expand Down
4 changes: 2 additions & 2 deletions demisto_sdk/commands/validate/default_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ select = [
"BA101", "BA105", "BA106", "BA116", "BA118",
"DO104",
"IN108", "IN130",
"PA107", "PA108", "PA111", "PA115", "PA128", "PA130",
"PA107", "PA108", "PA111", "PA114", "PA115", "PA128", "PA130",
"RP101",
]
warning = []
Expand All @@ -32,7 +32,7 @@ warning = []
[xsoar_best_practices_use_git]
select = [
"BA100", "BA101", "BA105", "BA106", "BA110", "BA111", "BA116", "BA118", "BA124", "BA126",
"PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120",
"PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA114", "PA115", "PA117", "PA118", "PA119", "PA120",
"PA121", "PA123", "PA125", "PA127", "PA128", "PA130", "PA131", "PA132",
"BC100", "BC105", "BC108",
"GR106",
Expand Down
46 changes: 46 additions & 0 deletions demisto_sdk/commands/validate/initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@
PLAYBOOKS_DIR,
RELEASE_NOTES_DIR,
SCRIPTS_DIR,
SKIP_RELEASE_NOTES_FOR_TYPES,
GitStatuses,
PathLevel,
)
from demisto_sdk.commands.common.content import Content
from demisto_sdk.commands.common.logger import logger
from demisto_sdk.commands.common.tools import (
detect_file_level,
find_type,
get_file_by_status,
get_pack_name,
get_relative_path_from_packs_dir,
is_external_repo,
pack_name_to_path,
specify_files_from_directory,
)
from demisto_sdk.commands.content_graph.objects.base_content import BaseContent
Expand All @@ -56,13 +60,15 @@ def __init__(
prev_ver=None,
file_path=None,
all_files=False,
skip_pack_dependencies=False,
):
self.staged = staged
self.use_git = use_git
self.file_path = file_path
self.all_files = all_files
self.committed_only = committed_only
self.prev_ver = prev_ver
self.skip_pack_dependencies = skip_pack_dependencies

def validate_git_installed(self):
"""Initialize git util."""
Expand Down Expand Up @@ -299,6 +305,40 @@ def specify_files_by_status(

return filtered_modified_files, filtered_added_files, filtered_renamed_files

def get_associated_pack_metadata(
self,
content_files: Set[BaseContent],
) -> Set[BaseContent]:
"""Get associated pack metadata files even if they were not changed/given explicitly.
Args:
content_files (Set[BaseContent]): The files to get their metadata pairs.
Return:
(Set[BaseContent]): A set of the relevant pack metadata files.
"""
pack_names = []
pack_metadatas: Set[BaseContent] = set()

if self.skip_pack_dependencies:
logger.debug("Skipping pack metadata files validation.")
return set()

for content_file in content_files:
if find_type(str(content_file.path)) not in SKIP_RELEASE_NOTES_FOR_TYPES:
pack_name = get_pack_name(content_file.path)
pack_path = pack_name_to_path(pack_name)
pack_names.append(pack_name)
pack_metadata = BaseContent.from_path(
Path(pack_path) / PACKS_PACK_META_FILE_NAME, metadata_only=True
)
if pack_metadata:
pack_metadatas.add(pack_metadata)

if pack_metadatas:
logger.info(f"Running on pack metadata files in packs: {pack_names}")
return pack_metadatas

def gather_objects_to_run_on(
self,
) -> Tuple[Set[BaseContent], Set[Path]]:
Expand All @@ -317,6 +357,9 @@ def gather_objects_to_run_on(
invalid_content_items,
non_content_items,
) = self.get_files_using_git()
content_objects_to_run = content_objects_to_run.union(
self.get_associated_pack_metadata(content_objects_to_run)
)
elif self.file_path:
(
content_objects_to_run,
Expand All @@ -339,6 +382,9 @@ def gather_objects_to_run_on(
invalid_content_items,
non_content_items,
) = self.get_files_using_git()
content_objects_to_run = content_objects_to_run.union(
self.get_associated_pack_metadata(content_objects_to_run)
)
if not self.use_git:
content_objects_to_run_with_packs: Set[
BaseContent
Expand Down
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 @@ -5,7 +5,7 @@ ignorable_errors = [
"IF100", "IF106", "IF113", "IF115", "IF116",
"IN107", "IN109", "IN110", "IN122", "IN123", "IN126", "IN128", "IN135", "IN136", "IN139", "IN144", "IN145", "IN150", "IN153", "IN154", "IN161",
"MP106",
"PA113", "PA116", "PA124", "PA125", "PA127", "PA129",
"PA113", "PA114", "PA116", "PA124", "PA125", "PA127", "PA129",
"PB104", "PB105", "PB106", "PB107", "PB110", "PB111", "PB114", "PB115", "PB116", "PB118", "PB119", "PB121",
"RM100", "RM102", "RM104", "RM106", "RM110", "RM112", "RM113",
"RP102", "RP104",
Expand All @@ -23,7 +23,7 @@ ignorable_errors = [
[use_git]
select = [
"BA100", "BA101", "BA105", "BA106", "BA108", "BA109", "BA110", "BA111", "BA114", "BA116", "BA118", "BA119", "BA124", "BA125", "BA126",
"PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA115", "PA117", "PA118", "PA119", "PA120",
"PA100", "PA101", "PA102", "PA103", "PA104", "PA105", "PA107", "PA108", "PA109", "PA111", "PA113", "PA114", "PA115", "PA117", "PA118", "PA119", "PA120",
"PA121", "PA123", "PA125", "PA127", "PA128", "PA130", "PA131", "PA132",
"BC100", "BC101", "BC102", "BC103", "BC104", "BC105", "BC106", "BC107", "BC108", "BC110", "BC111", "BC112",
"IN100", "IN101", "IN102", "IN104", "IN106", "IN107", "IN108", "IN109", "IN110", "IN112", "IN113", "IN114", "IN115", "IN117", "IN118", "IN121", "IN122", "IN123", "IN124", "IN125", "IN126", "IN127", "IN130",
Expand Down
32 changes: 32 additions & 0 deletions demisto_sdk/commands/validate/tests/PA_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
from demisto_sdk.commands.validate.validators.PA_validators.PA113_is_url_or_email_exists import (
IsURLOrEmailExistsValidator,
)
from demisto_sdk.commands.validate.validators.PA_validators.PA114_pack_metadata_version_should_be_raised import (
PackMetadataVersionShouldBeRaisedValidator,
)
from demisto_sdk.commands.validate.validators.PA_validators.PA115_is_created_field_in_iso_format import (
IsCreatedFieldInISOFormatValidator,
)
Expand Down Expand Up @@ -1613,3 +1616,32 @@ def test_PackFilesValidator_fix(file_attribute: str):

assert meta_file.file_path.exists()
assert meta_file.exist # changed in the fix


@pytest.mark.parametrize(
"old_version, current_version, expected_invalid",
[("1.0.0", "1.0.1", 0), ("1.0.0", "1.0.0", 1), ("1.1.0", "1.0.1", 1)],
)
def test_PackMetadataVersionShouldBeRaisedValidator(
mocker, old_version, current_version, expected_invalid
):
"""
Given: A previous pack version and a current pack version.
When: Running PackMetadataVersionShouldBeRaisedValidator validator.
Then: Assure the validation succeeds if the current version <= previous version.
Cases:
1) current version > previous version: 0 validation errors.
2) current version = previous version: 1 validation errors.
3) current version < previous version: 1 validation errors.
"""
pack = create_pack_object(["currentVersion"], [current_version])
integration = create_integration_object()
pack.content_items.integration.extend(integration)
mocker.patch(
"demisto_sdk.commands.validate.validators.PA_validators"
".PA114_pack_metadata_version_should_be_raised.get_remote_file",
return_value={"currentVersion": old_version},
)
version_bump_validator = PackMetadataVersionShouldBeRaisedValidator()
version_bump_validator.external_args["prev_ver"] = "origin/master"
assert len(version_bump_validator.is_valid([pack])) == expected_invalid
49 changes: 48 additions & 1 deletion demisto_sdk/commands/validate/tests/validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@
from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH
from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json
from demisto_sdk.commands.content_graph.common import ContentType
from demisto_sdk.commands.content_graph.objects.base_content import BaseContent
from demisto_sdk.commands.content_graph.tests.test_tools import load_yaml
from demisto_sdk.commands.validate.config_reader import (
ConfigReader,
ConfiguredValidations,
)
from demisto_sdk.commands.validate.initializer import Initializer
from demisto_sdk.commands.validate.tests.test_tools import create_integration_object
from demisto_sdk.commands.validate.tests.test_tools import (
create_integration_object,
create_pack_object,
)
from demisto_sdk.commands.validate.validate_manager import ValidateManager
from demisto_sdk.commands.validate.validation_results import ResultWriter
from demisto_sdk.commands.validate.validators.BA_validators.BA101_id_should_equal_name import (
Expand All @@ -40,6 +44,9 @@
from demisto_sdk.commands.validate.validators.PA_validators.PA108_pack_metadata_name_not_valid import (
PackMetadataNameValidator,
)
from demisto_sdk.commands.validate.validators.PA_validators.PA114_pack_metadata_version_should_be_raised import (
PackMetadataVersionShouldBeRaisedValidator,
)
from TestSuite.test_tools import str_in_call_args_list

INTEGRATION = create_integration_object()
Expand Down Expand Up @@ -655,3 +662,43 @@ def test_description():
assert not [
validator for validator in get_all_validators() if not validator.description
]


def test_check_metadata_version_bump_on_content_changes(mocker, repo):
"""
Given: pack with newly added integration.
When: Initializing ValidateManager using git
Then: Ensure PackMetadataVersionShouldBeRaisedValidator is initialized and the external args are properly passed.
"""
pack = create_pack_object(["currentVersion"], ["1.1.1"])
integration = create_integration_object()
pack.content_items.integration.extend(integration)
validation_results = ResultWriter()
config_reader = ConfigReader(category_to_run="use_git")
mocker.patch.object(
Initializer,
"get_files_using_git",
return_value=({BaseContent.from_path(Path(integration.path))}, {}, {}),
)
mocker.patch.object(
BaseContent,
"from_path",
return_value=BaseContent.from_path(Path(pack.path), metadata_only=True),
)
initializer = Initializer(prev_ver="some_prev_ver")

validate_manager = ValidateManager(
validation_results=validation_results,
config_reader=config_reader,
initializer=initializer,
)

version_bump_validator = None
for validator in validate_manager.validators:
if isinstance(validator, PackMetadataVersionShouldBeRaisedValidator):
version_bump_validator = validator

# Assert the PA114 validation will run
assert version_bump_validator
# Assert external args were propagated correctly.
assert version_bump_validator.external_args["prev_ver"] == "some_prev_ver"
9 changes: 9 additions & 0 deletions demisto_sdk/commands/validate/validate_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def __init__(
)
)
self.validators = self.filter_validators()
self.update_validators_external_args()

def run_validations(self) -> int:
"""
Expand Down Expand Up @@ -122,6 +123,14 @@ def filter_validators(self) -> List[BaseValidator]:
if validator.error_code in self.configured_validations.validations_to_run
]

def update_validators_external_args(self):
"""Update the validators with args from the initializer."""
for validator in self.validators:
if validator.external_args:
for external_arg_name in validator.external_args.keys():
external_arg_value = getattr(self.initializer, external_arg_name)
validator.external_args[external_arg_name] = external_arg_value

def add_invalid_content_items(self):
"""Create results for all the invalid_content_items.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
from __future__ import annotations

from typing import Iterable, List

from packaging.version import Version

from demisto_sdk.commands.common.constants import (
DEMISTO_GIT_PRIMARY_BRANCH,
PACKS_PACK_META_FILE_NAME,
)
from demisto_sdk.commands.common.tools import get_remote_file
from demisto_sdk.commands.content_graph.objects.pack import Pack
from demisto_sdk.commands.validate.validators.base_validator import (
BaseValidator,
ValidationResult,
)

ContentTypes = Pack


class PackMetadataVersionShouldBeRaisedValidator(BaseValidator[ContentTypes]):
error_code = "PA114"
description = "Ensure that the pack metadata is raised on relevant changes."
rationale = (
"When updating a pack, its version needs to be raised to maintain traceability."
)
error_message = (
"The pack version (currently: {old_version}) needs to be raised - "
"make sure you are merged from master and "
'update the "currentVersion" field in the '
"pack_metadata.json or in case release notes are required run:\n"
"`demisto-sdk update-release-notes -i Packs/{pack} -u "
"(major|minor|revision|documentation)` to "
"generate them according to the new standard."
)
related_field = "currentVersion, name"
is_auto_fixable = False
# Validate manager should populate the external args for the validation.
external_args = {"prev_ver": None}

def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]:
validation_results = []
prev_ver_tag = self.external_args.get("prev_ver", DEMISTO_GIT_PRIMARY_BRANCH)
for content_item in content_items:
old_meta_file_content = get_remote_file(
str(content_item.path / PACKS_PACK_META_FILE_NAME), tag=prev_ver_tag
)
old_version = old_meta_file_content.get("currentVersion", "0.0.0")
if content_item.current_version and Version(old_version) >= Version(
content_item.current_version
):
validation_results.append(
ValidationResult(
validator=self,
message=self.error_message.format(
old_version=old_version, pack=content_item.name
),
content_object=content_item,
)
)
return validation_results
1 change: 1 addition & 0 deletions demisto_sdk/commands/validate/validators/base_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class BaseValidator(ABC, BaseModel, Generic[ContentTypes]):
is_auto_fixable: ClassVar[bool] = False
graph_interface: ClassVar[ContentGraphInterface] = None
related_file_type: ClassVar[Optional[List[RelatedFileType]]] = None
external_args: ClassVar[dict] = {}

def get_content_types(self):
args = (get_args(self.__orig_bases__[0]) or get_args(self.__orig_bases__[1]))[0] # type: ignore
Expand Down

0 comments on commit 243780b

Please sign in to comment.