From 229d8e0aa9cf1437ece562531b5209c70831f9ab Mon Sep 17 00:00:00 2001 From: TalZich Date: Tue, 17 Dec 2024 17:07:54 +0200 Subject: [PATCH 01/13] Naive implementation --- .../strict_objects/integration.py | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index b107773d229..472ca2e6908 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -1,6 +1,7 @@ +from enum import Enum from typing import Any, List, Optional -from pydantic import Field +from pydantic import Field, conlist, validator from demisto_sdk.commands.common.constants import ( TYPE_PYTHON2, @@ -40,7 +41,7 @@ class _Configuration(BaseStrictModel): display: Optional[str] = None - section: Optional[str] = None + section: str advanced: Optional[str] = None default_value: Optional[Any] = Field(None, alias="defaultvalue") name: str @@ -136,14 +137,19 @@ class _CommonFieldsIntegration(BaseStrictModel): ), ) +class SectionOrderValues(str, Enum): + CONNECT = "connect", + COLLECT = "collect", + OPTIMIZE = "optimize" + class _StrictIntegration(BaseStrictModel): common_fields: CommonFieldsIntegration = Field(..., alias="commonfields") # type:ignore[valid-type] display: str beta: Optional[bool] = None category: str - section_order_pascal_case: Optional[List[str]] = Field(None, alias="sectionOrder") - section_order_lower_case: Optional[List[str]] = Field(None, alias="sectionorder") + section_order: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionorder") + section_order_camel_case: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionOrder") image: Optional[str] = None description: str default_mapper_in: Optional[str] = Field(None, alias="defaultmapperin") @@ -161,6 +167,21 @@ class _StrictIntegration(BaseStrictModel): script_not_visible: Optional[bool] = Field(None, alias="scriptNotVisible") hybrid: Optional[bool] = None + def __init__(self, **data): + if 'section_order_camel_case' in data: + data['section_order'] = data.pop('section_order_camel_case') + super().__init__(**data) + + @validator('configuration') + def validate_sections(cls, configuration, values): + section_order = values.get('section_order') + for field_config in configuration: + assert field_config.secion in section_order + return configuration + + + + StrictIntegration = create_model( model_name="StrictIntegration", From 32998fccb54f1c3ce9ba3b82c029d34102c8e668 Mon Sep 17 00:00:00 2001 From: TalZich Date: Sun, 22 Dec 2024 13:25:49 +0200 Subject: [PATCH 02/13] Address rshunim's advice --- .../strict_objects/integration.py | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index 472ca2e6908..b549b3bc399 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -138,9 +138,9 @@ class _CommonFieldsIntegration(BaseStrictModel): ) class SectionOrderValues(str, Enum): - CONNECT = "connect", - COLLECT = "collect", - OPTIMIZE = "optimize" + CONNECT = "Connect", + COLLECT = "Collect", + OPTIMIZE = "Optimize" class _StrictIntegration(BaseStrictModel): @@ -148,8 +148,8 @@ class _StrictIntegration(BaseStrictModel): display: str beta: Optional[bool] = None category: str - section_order: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionorder") - section_order_camel_case: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionOrder") + section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = Field(alias="sectionorder") + section_order_camel_case: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = Field(alias="sectionOrder") image: Optional[str] = None description: str default_mapper_in: Optional[str] = Field(None, alias="defaultmapperin") @@ -158,7 +158,8 @@ class _StrictIntegration(BaseStrictModel): detailed_description: Optional[str] = Field(None, alias="detaileddescription") auto_config_instance: Optional[bool] = Field(None, alias="autoconfiginstance") support_level_header: MarketplaceVersions = Field(None, alias="supportlevelheader") - configuration: List[Configuration] # type:ignore[valid-type] + # configurations: List[Configuration] = Field(..., alias="configuration") # type:ignore[valid-type] + configuration: List[Configuration] # type:ignore[valid-type] script: Script # type:ignore[valid-type] hidden: Optional[bool] = None videos: Optional[List[str]] = None @@ -168,15 +169,19 @@ class _StrictIntegration(BaseStrictModel): hybrid: Optional[bool] = None def __init__(self, **data): - if 'section_order_camel_case' in data: + if 'section_order_camel_case' in data and not 'section_order' in data: data['section_order'] = data.pop('section_order_camel_case') + elif 'section_order_camel_case' in data and 'section_order' in data: + data['section_order'] = list(set(data['section_order']) | set(data['section_order_camel_case'])) super().__init__(**data) @validator('configuration') def validate_sections(cls, configuration, values): section_order = values.get('section_order') - for field_config in configuration: - assert field_config.secion in section_order + assert section_order, 'section_order may not be None or empty list' + for config in configuration: + assert config.section in section_order,\ + f'section {config.section} of {config.display} is not present in section_order {config.section_order}' return configuration From 111ba66c2e33d55c6b1b00351dc49ad7d2af1cde Mon Sep 17 00:00:00 2001 From: TalZich Date: Wed, 25 Dec 2024 14:26:08 +0200 Subject: [PATCH 03/13] Adjust base yml file --- .../strict_objects/integration.py | 23 +++++++++---------- .../tests/test_data/integration.yml | 4 ++++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index b549b3bc399..ebd1b240a39 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -148,8 +148,7 @@ class _StrictIntegration(BaseStrictModel): display: str beta: Optional[bool] = None category: str - section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = Field(alias="sectionorder") - section_order_camel_case: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = Field(alias="sectionOrder") + section_order: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionorder") image: Optional[str] = None description: str default_mapper_in: Optional[str] = Field(None, alias="defaultmapperin") @@ -158,8 +157,7 @@ class _StrictIntegration(BaseStrictModel): detailed_description: Optional[str] = Field(None, alias="detaileddescription") auto_config_instance: Optional[bool] = Field(None, alias="autoconfiginstance") support_level_header: MarketplaceVersions = Field(None, alias="supportlevelheader") - # configurations: List[Configuration] = Field(..., alias="configuration") # type:ignore[valid-type] - configuration: List[Configuration] # type:ignore[valid-type] + configurations: List[Configuration] = Field(..., alias="configuration")# type:ignore[valid-type] script: Script # type:ignore[valid-type] hidden: Optional[bool] = None videos: Optional[List[str]] = None @@ -169,20 +167,21 @@ class _StrictIntegration(BaseStrictModel): hybrid: Optional[bool] = None def __init__(self, **data): - if 'section_order_camel_case' in data and not 'section_order' in data: - data['section_order'] = data.pop('section_order_camel_case') - elif 'section_order_camel_case' in data and 'section_order' in data: - data['section_order'] = list(set(data['section_order']) | set(data['section_order_camel_case'])) + if 'sectionOrder' in data and not 'sectionorder' in data: + data['sectionorder'] = data.pop('sectionOrder') + elif 'sectionOrder' in data and 'sectionorder' in data: + data['sectionorder'] = list(set(data['section_order']) | set(data['section_order_camel_case'])) super().__init__(**data) - @validator('configuration') - def validate_sections(cls, configuration, values): + @validator('configurations') + def validate_sections(cls, configurations, values): + print(f'{configurations=}, {values=}') section_order = values.get('section_order') assert section_order, 'section_order may not be None or empty list' - for config in configuration: + for config in configurations: assert config.section in section_order,\ f'section {config.section} of {config.display} is not present in section_order {config.section_order}' - return configuration + return configurations diff --git a/demisto_sdk/commands/content_graph/tests/test_data/integration.yml b/demisto_sdk/commands/content_graph/tests/test_data/integration.yml index 3abcbdd4371..dd0dda65f7e 100644 --- a/demisto_sdk/commands/content_graph/tests/test_data/integration.yml +++ b/demisto_sdk/commands/content_graph/tests/test_data/integration.yml @@ -7,17 +7,21 @@ fromversion: 5.0.0 category: Forensics & Malware Analysis description: Analyze files using the malwr sandbox detaileddescription: This integration uses the Marlwr sandbox API to submit, analyze and detonate files. +sectionorder: + - Connect configuration: - display: URL name: server defaultvalue: https://malwr.com type: 0 required: true + section: Connect - display: Credentials name: credentials defaultvalue: "" type: 9 required: true + section: Connect script: type: python subtype: python3 From daf9291249774ae8f629a4d8fe26844e318195cc Mon Sep 17 00:00:00 2001 From: TalZich Date: Thu, 26 Dec 2024 15:24:58 +0200 Subject: [PATCH 04/13] Finish implementation and write UT --- .../strict_objects/base_strict_model.py | 2 +- .../content_graph/strict_objects/common.py | 2 +- .../strict_objects/integration.py | 11 +- .../validate/tests/ST_validators_test.py | 101 ++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py b/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py index 37c8bd76bb7..23e6c714c9f 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py +++ b/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py @@ -119,7 +119,7 @@ class StructureError(BaseStrictModel): def __str__(self): field_name = ",".join(more_itertools.always_iterable(self.field_name)) if self.error_type == "assertion_error": - error_message = f"The field {field_name} is not required, but should not be None if it exists" + error_message = self.error_message or f"An assertion error occurred for field {field_name}" elif self.error_type == "value_error.extra": error_message = f"The field {field_name} is extra and {self.error_message}" elif self.error_type == "value_error.missing": diff --git a/demisto_sdk/commands/content_graph/strict_objects/common.py b/demisto_sdk/commands/content_graph/strict_objects/common.py index 221a33a33e5..06cb98dcc18 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/common.py +++ b/demisto_sdk/commands/content_graph/strict_objects/common.py @@ -71,7 +71,7 @@ def prevent_none(cls, value, field): "breaking_changes_notes", # release-notes-config }: # The assertion is caught by pydantic and converted to a pydantic.ValidationError - assert value is not None, f"{value} may not be None" + assert value is not None, f"The field {field.name} is not required, but should not be None if it exists" return value diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index ebd1b240a39..7813fd1bb21 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -175,12 +175,13 @@ def __init__(self, **data): @validator('configurations') def validate_sections(cls, configurations, values): - print(f'{configurations=}, {values=}') - section_order = values.get('section_order') - assert section_order, 'section_order may not be None or empty list' + section_order_field = values.get('section_order') + if not section_order_field: + return None + integration_sections = [section_name.value for section_name in section_order_field] for config in configurations: - assert config.section in section_order,\ - f'section {config.section} of {config.display} is not present in section_order {config.section_order}' + assert config.section in integration_sections,\ + f'section {config.section} of {config.display} is not present in section_order {integration_sections}' return configurations diff --git a/demisto_sdk/commands/validate/tests/ST_validators_test.py b/demisto_sdk/commands/validate/tests/ST_validators_test.py index 64651e36104..0edd8dcde66 100644 --- a/demisto_sdk/commands/validate/tests/ST_validators_test.py +++ b/demisto_sdk/commands/validate/tests/ST_validators_test.py @@ -240,3 +240,104 @@ def test_pack_parser_errors_check(pack: Pack): "value is not a valid enumeration member; permitted: 'xsoar', 'partner', 'community', 'developer'", } == error_messages assert {"type_error.bool", "type_error.enum"} == error_types + + +def test_invalid_section_order(pack: Pack): + """ + Given: + - an integration which contains invalid section order + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + integration.yml.update({'sectionorder': ["Connect", "Run"]}) + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = SchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " + "value is not a valid enumeration member; permitted: 'Connect', 'Collect', 'Optimize'" + ) + +def test_missing_section_order(pack: Pack): + """ + Given: + - an integration with a missing section order + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + integration.yml.delete_key("sectionorder") + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = SchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (value_error.missing) in field sectionorder of integration_0.yml: " + "The field sectionorder is required but missing" + ) + +def test_invalid_section(pack: Pack): + """ + Given: + - an integration which contains invalid section clause in one of its configuration objects + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + curr_config = integration.yml.read_dict()['configuration'] + curr_config[0]['section'] = "Run" + integration.yml.update({'configuration': curr_config}) + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = SchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (assertion_error) in field configuration of integration_0.yml: " + "section Run of URL is not present in section_order ['Connect']" + ) + +def test_missing_section(pack: Pack): + """ + Given: + - an integration with a missing section clause in one of its configuration objects + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + curr_config = integration.yml.read_dict()['configuration'] + curr_config[0].pop("section") + integration.yml.update({'configuration': curr_config}) + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = SchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (value_error.missing) in field configuration,0,section of integration_0.yml: " + "The field configuration,0,section is required but missing" + ) From 6acef1be578e77ffb3f9a2d62e06b9dd6c30f143 Mon Sep 17 00:00:00 2001 From: TalZich Date: Thu, 26 Dec 2024 15:33:36 +0200 Subject: [PATCH 05/13] Add documentation --- .../content_graph/strict_objects/integration.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index 7813fd1bb21..e5723645c14 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -167,6 +167,10 @@ class _StrictIntegration(BaseStrictModel): hybrid: Optional[bool] = None def __init__(self, **data): + """ + Initializes the _StrictIntegration object. + Using this custom init function to support two aliases for the section_order field. + """ if 'sectionOrder' in data and not 'sectionorder' in data: data['sectionorder'] = data.pop('sectionOrder') elif 'sectionOrder' in data and 'sectionorder' in data: @@ -175,6 +179,12 @@ def __init__(self, **data): @validator('configurations') def validate_sections(cls, configurations, values): + """ + Validates each configuration object has a valid section clause. + A valid section clause is a section which is included in the list of the integration's section_order. + Even if the section is an allowed value (currently Collect, Connect or Optimize),it could be invalid if the + specific value is not present in section_order. + """ section_order_field = values.get('section_order') if not section_order_field: return None From fb6be6d9972d3f8e46cb15879271e56727b871e3 Mon Sep 17 00:00:00 2001 From: TalZich Date: Thu, 26 Dec 2024 15:59:51 +0200 Subject: [PATCH 06/13] Add changelog --- .changelog/4739.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/4739.yml diff --git a/.changelog/4739.yml b/.changelog/4739.yml new file mode 100644 index 00000000000..a03ea7538df --- /dev/null +++ b/.changelog/4739.yml @@ -0,0 +1,4 @@ +changes: +- description: This PR adds to the ST-110 validation which will now validate the sectionOrder and individual sections. + type: feature +pr_number: 4739 From 264a7e03eb5181638463fe7eb900e9ce46cec783 Mon Sep 17 00:00:00 2001 From: TalZich Date: Tue, 31 Dec 2024 14:30:41 +0200 Subject: [PATCH 07/13] Build section_order custom validator --- .../strict_objects/integration.py | 12 +- .../validate/sdk_validation_config.toml | 1 + .../validate/tests/ST_validators_test.py | 135 ++++++++++++++---- .../ST_validators/ST110_is_valid_scheme.py | 17 ++- .../ST111_no_exclusions_schema.py | 88 ++++++++++++ 5 files changed, 221 insertions(+), 32 deletions(-) create mode 100644 demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index e5723645c14..212c5ac8746 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -148,6 +148,7 @@ class _StrictIntegration(BaseStrictModel): display: str beta: Optional[bool] = None category: str + configurations: List[Configuration] = Field(..., alias="configuration")# type:ignore[valid-type] section_order: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionorder") image: Optional[str] = None description: str @@ -157,7 +158,6 @@ class _StrictIntegration(BaseStrictModel): detailed_description: Optional[str] = Field(None, alias="detaileddescription") auto_config_instance: Optional[bool] = Field(None, alias="autoconfiginstance") support_level_header: MarketplaceVersions = Field(None, alias="supportlevelheader") - configurations: List[Configuration] = Field(..., alias="configuration")# type:ignore[valid-type] script: Script # type:ignore[valid-type] hidden: Optional[bool] = None videos: Optional[List[str]] = None @@ -177,23 +177,23 @@ def __init__(self, **data): data['sectionorder'] = list(set(data['section_order']) | set(data['section_order_camel_case'])) super().__init__(**data) - @validator('configurations') - def validate_sections(cls, configurations, values): + + @validator('section_order') + def validate_sections(cls, section_order_field, values): """ Validates each configuration object has a valid section clause. A valid section clause is a section which is included in the list of the integration's section_order. Even if the section is an allowed value (currently Collect, Connect or Optimize),it could be invalid if the specific value is not present in section_order. """ - section_order_field = values.get('section_order') + configurations = values.get('configurations') if not section_order_field: return None integration_sections = [section_name.value for section_name in section_order_field] for config in configurations: assert config.section in integration_sections,\ f'section {config.section} of {config.display} is not present in section_order {integration_sections}' - return configurations - + return section_order_field diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index f1c10cba7b2..21911633ea3 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -490,6 +490,7 @@ select = [ "MR107", "MR108", "ST110", + "ST111", "RN103", "RN105", "RN106", diff --git a/demisto_sdk/commands/validate/tests/ST_validators_test.py b/demisto_sdk/commands/validate/tests/ST_validators_test.py index 0edd8dcde66..ee585193b95 100644 --- a/demisto_sdk/commands/validate/tests/ST_validators_test.py +++ b/demisto_sdk/commands/validate/tests/ST_validators_test.py @@ -16,6 +16,7 @@ SchemaValidator, ) from TestSuite.pack import Pack +from demisto_sdk.commands.validate.validators.ST_validators.ST111_no_exclusions_schema import StrictSchemaValidator def test_sanity_SchemaValidator(): @@ -259,12 +260,8 @@ def test_invalid_section_order(pack: Pack): ) results = SchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 1 - assert ( - results[0].message - == "Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " - "value is not a valid enumeration member; permitted: 'Connect', 'Collect', 'Optimize'" - ) + assert len(results) == 0 + def test_missing_section_order(pack: Pack): """ @@ -283,12 +280,8 @@ def test_missing_section_order(pack: Pack): ) results = SchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 1 - assert ( - results[0].message - == "Structure error (value_error.missing) in field sectionorder of integration_0.yml: " - "The field sectionorder is required but missing" - ) + assert len(results) == 0 + def test_invalid_section(pack: Pack): """ @@ -309,12 +302,8 @@ def test_invalid_section(pack: Pack): ) results = SchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 1 - assert ( - results[0].message - == "Structure error (assertion_error) in field configuration of integration_0.yml: " - "section Run of URL is not present in section_order ['Connect']" - ) + assert len(results) == 0 + def test_missing_section(pack: Pack): """ @@ -335,9 +324,107 @@ def test_missing_section(pack: Pack): ) results = SchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 1 - assert ( - results[0].message - == "Structure error (value_error.missing) in field configuration,0,section of integration_0.yml: " - "The field configuration,0,section is required but missing" - ) + assert len(results) == 0 + + +class TestST111: + def test_invalid_section_order(self, pack: Pack): + """ + Given: + - an integration which contains invalid section order + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + integration.yml.update({'sectionorder': ["Connect", "Run"]}) + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " + "value is not a valid enumeration member; permitted: 'Connect', 'Collect', 'Optimize'" + ) + + def test_missing_section_order(self, pack: Pack): + """ + Given: + - an integration with a missing section order + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + integration.yml.delete_key("sectionorder") + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (value_error.missing) in field sectionorder of integration_0.yml: " + "The field sectionorder is required but missing" + ) + + def test_invalid_section(self, pack: Pack): + """ + Given: + - an integration which contains invalid section clause in one of its configuration objects + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + curr_config = integration.yml.read_dict()['configuration'] + curr_config[0]['section'] = "Run" + integration.yml.update({'configuration': curr_config}) + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (assertion_error) in field sectionorder of integration_0.yml: " + "section Run of URL is not present in section_order ['Connect']" + ) + + + def test_missing_section(self, pack: Pack): + """ + Given: + - an integration with a missing section clause in one of its configuration objects + When: + - executing the IntegrationParser + Then: + - the integration is invalid and the correct error message is returned + """ + integration = pack.create_integration(yml=load_yaml("integration.yml")) + curr_config = integration.yml.read_dict()['configuration'] + curr_config[0].pop("section") + integration.yml.update({'configuration': curr_config}) + + integration_parser = IntegrationParser( + Path(integration.path), list(MarketplaceVersions) + ) + + results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) + assert len(results) == 1 + assert ( + results[0].message + == "Structure error (value_error.missing) in field configuration,0,section of integration_0.yml: " + "The field configuration,0,section is required but missing" + ) diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py b/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py index d3a1e7cdc97..dd44a80938d 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py @@ -35,6 +35,7 @@ from demisto_sdk.commands.content_graph.objects.integration import Integration from demisto_sdk.commands.content_graph.objects.list import List as ListObject from demisto_sdk.commands.content_graph.objects.script import Script +from demisto_sdk.commands.content_graph.strict_objects.base_strict_model import StructureError from demisto_sdk.commands.validate.validators.base_validator import ( BaseValidator, ValidationResult, @@ -76,10 +77,15 @@ AssetsModelingRule, ] +EXCLUDED_FIELDS = [ + "section", + "sectionorder" +] + class SchemaValidator(BaseValidator[ContentTypes]): error_code = "ST110" - description = "Validate that the scheme's structure is valid." + description = "Validate that the scheme's structure is valid, while excluding certain fields." rationale = "Maintain valid structure for content items." def obtain_invalid_content_items( @@ -99,4 +105,11 @@ def obtain_invalid_content_items( ] def is_invalid_schema(self, content_item: ContentTypes) -> bool: - return bool(content_item.structure_errors) + return bool(content_item.structure_errors) and not self.is_error_excluded(content_item.structure_errors) + + def is_error_excluded(self, structure_errors: List[StructureError]) -> bool: + for error in structure_errors: + for field_name in error.field_name: + if field_name and field_name in EXCLUDED_FIELDS: + return True + return False \ No newline at end of file diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py new file mode 100644 index 00000000000..887b1b31982 --- /dev/null +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py @@ -0,0 +1,88 @@ +from typing import Union +from demisto_sdk.commands.common.constants import GitStatuses +from demisto_sdk.commands.validate.validators.ST_validators.ST110_is_valid_scheme import SchemaValidator +from demisto_sdk.commands.content_graph.objects import ( + AssetsModelingRule, + CaseField, + CaseLayout, + CaseLayoutRule, + Classifier, + CorrelationRule, + Dashboard, + GenericDefinition, + GenericField, + GenericModule, + GenericType, + IncidentField, + IncidentType, + IndicatorField, + IndicatorType, + Job, + Layout, + LayoutRule, + Mapper, + ModelingRule, + Pack, + ParsingRule, + Playbook, + PreProcessRule, + Report, + Widget, + Wizard, + XDRCTemplate, + XSIAMDashboard, + XSIAMReport, +) +from demisto_sdk.commands.content_graph.objects.integration import Integration +from demisto_sdk.commands.content_graph.objects.list import List as ListObject +from demisto_sdk.commands.content_graph.objects.script import Script +from demisto_sdk.commands.content_graph.strict_objects.base_strict_model import StructureError +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +ContentTypes = Union[ + Integration, + Script, + IncidentField, + IndicatorField, + IncidentType, + GenericType, + Classifier, + Layout, + LayoutRule, + Playbook, + CorrelationRule, + Dashboard, + GenericDefinition, + GenericField, + GenericModule, + Job, + ListObject, + Mapper, + ParsingRule, + PreProcessRule, + Report, + Widget, + Wizard, + XDRCTemplate, + XSIAMDashboard, + XSIAMReport, + IndicatorType, + CaseField, + CaseLayout, + CaseLayoutRule, + Pack, + ModelingRule, + AssetsModelingRule, +] + +class StrictSchemaValidator(SchemaValidator): + error_code = "ST111" + description = "Validate that the scheme's structure is valid, no fields excluded." + rationale = "Maintain valid structure for content items." + expected_git_statuses = [GitStatuses.MODIFIED, GitStatuses.ADDED] + + def is_invalid_schema(self, content_item: ContentTypes) -> bool: + return bool(content_item.structure_errors) \ No newline at end of file From 5ba4fb38560e721ae4d3e4a6b087cacdf1a554ca Mon Sep 17 00:00:00 2001 From: TalZich Date: Wed, 1 Jan 2025 15:34:46 +0200 Subject: [PATCH 08/13] Refactor validation structure --- .../strict_objects/integration.py | 16 +-- .../validate/sdk_validation_config.toml | 1 + .../validate/tests/ST_validators_test.py | 81 ++++--------- .../ST_validators/ST110_is_valid_scheme.py | 15 +-- .../ST111_no_exclusions_schema.py | 107 ++++++------------ 5 files changed, 68 insertions(+), 152 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index 212c5ac8746..f4d3334f9ab 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -41,7 +41,7 @@ class _Configuration(BaseStrictModel): display: Optional[str] = None - section: str + section: Optional[str] advanced: Optional[str] = None default_value: Optional[Any] = Field(None, alias="defaultvalue") name: str @@ -148,8 +148,8 @@ class _StrictIntegration(BaseStrictModel): display: str beta: Optional[bool] = None category: str + section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = Field(alias="sectionorder") configurations: List[Configuration] = Field(..., alias="configuration")# type:ignore[valid-type] - section_order: conlist(SectionOrderValues, min_items=1, max_items=3) = Field(alias="sectionorder") image: Optional[str] = None description: str default_mapper_in: Optional[str] = Field(None, alias="defaultmapperin") @@ -178,22 +178,24 @@ def __init__(self, **data): super().__init__(**data) - @validator('section_order') - def validate_sections(cls, section_order_field, values): + @validator('configurations') + def validate_sections(cls, configurations, values): """ Validates each configuration object has a valid section clause. A valid section clause is a section which is included in the list of the integration's section_order. Even if the section is an allowed value (currently Collect, Connect or Optimize),it could be invalid if the specific value is not present in section_order. """ - configurations = values.get('configurations') + section_order_field = values.get('section_order') if not section_order_field: - return None + return configurations integration_sections = [section_name.value for section_name in section_order_field] for config in configurations: + if not config.section: + return configurations assert config.section in integration_sections,\ f'section {config.section} of {config.display} is not present in section_order {integration_sections}' - return section_order_field + return configurations diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index 3fb0e3b17bd..26652479a06 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -241,6 +241,7 @@ select = [ "MR107", "MR108", "ST110", + "ST111", "TR100" ] warning = [ diff --git a/demisto_sdk/commands/validate/tests/ST_validators_test.py b/demisto_sdk/commands/validate/tests/ST_validators_test.py index ee585193b95..4b7944128c4 100644 --- a/demisto_sdk/commands/validate/tests/ST_validators_test.py +++ b/demisto_sdk/commands/validate/tests/ST_validators_test.py @@ -16,7 +16,8 @@ SchemaValidator, ) from TestSuite.pack import Pack -from demisto_sdk.commands.validate.validators.ST_validators.ST111_no_exclusions_schema import StrictSchemaValidator +from demisto_sdk.commands.validate.validators.ST_validators.ST111_no_exclusions_schema import StrictSchemaValidator, \ + ALLOWED_SECTIONS def test_sanity_SchemaValidator(): @@ -260,7 +261,7 @@ def test_invalid_section_order(pack: Pack): ) results = SchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 0 + assert len(results) == 1 def test_missing_section_order(pack: Pack): @@ -302,7 +303,7 @@ def test_invalid_section(pack: Pack): ) results = SchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 0 + assert len(results) == 1 def test_missing_section(pack: Pack): @@ -328,7 +329,7 @@ def test_missing_section(pack: Pack): class TestST111: - def test_invalid_section_order(self, pack: Pack): + def test_invalid_section_order(self): """ Given: - an integration which contains invalid section order @@ -337,22 +338,12 @@ def test_invalid_section_order(self, pack: Pack): Then: - the integration is invalid and the correct error message is returned """ - integration = pack.create_integration(yml=load_yaml("integration.yml")) - integration.yml.update({'sectionorder': ["Connect", "Run"]}) + integration = create_integration_object(paths=['sectionorder'], values=[["Connect", "Run"]]) + results = StrictSchemaValidator().obtain_invalid_content_items([integration]) - integration_parser = IntegrationParser( - Path(integration.path), list(MarketplaceVersions) - ) + assert len(results) == 0 - results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 1 - assert ( - results[0].message - == "Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " - "value is not a valid enumeration member; permitted: 'Connect', 'Collect', 'Optimize'" - ) - - def test_missing_section_order(self, pack: Pack): + def test_missing_section_order(self): """ Given: - an integration with a missing section order @@ -361,22 +352,14 @@ def test_missing_section_order(self, pack: Pack): Then: - the integration is invalid and the correct error message is returned """ - integration = pack.create_integration(yml=load_yaml("integration.yml")) - integration.yml.delete_key("sectionorder") - - integration_parser = IntegrationParser( - Path(integration.path), list(MarketplaceVersions) - ) + integration = create_integration_object() + integration.data.pop("sectionorder") + results = StrictSchemaValidator().obtain_invalid_content_items([integration]) - results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) assert len(results) == 1 - assert ( - results[0].message - == "Structure error (value_error.missing) in field sectionorder of integration_0.yml: " - "The field sectionorder is required but missing" - ) + assert results[0].message == 'Missing section order' - def test_invalid_section(self, pack: Pack): + def test_invalid_section(self): """ Given: - an integration which contains invalid section clause in one of its configuration objects @@ -385,22 +368,14 @@ def test_invalid_section(self, pack: Pack): Then: - the integration is invalid and the correct error message is returned """ - integration = pack.create_integration(yml=load_yaml("integration.yml")) - curr_config = integration.yml.read_dict()['configuration'] + integration = create_integration_object() + curr_config = integration.data['configuration'] curr_config[0]['section'] = "Run" - integration.yml.update({'configuration': curr_config}) + integration.data['configuration'] = curr_config - integration_parser = IntegrationParser( - Path(integration.path), list(MarketplaceVersions) - ) - results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) - assert len(results) == 1 - assert ( - results[0].message - == "Structure error (assertion_error) in field sectionorder of integration_0.yml: " - "section Run of URL is not present in section_order ['Connect']" - ) + results = StrictSchemaValidator().obtain_invalid_content_items([integration]) + assert len(results) == 0 def test_missing_section(self, pack: Pack): @@ -412,19 +387,11 @@ def test_missing_section(self, pack: Pack): Then: - the integration is invalid and the correct error message is returned """ - integration = pack.create_integration(yml=load_yaml("integration.yml")) - curr_config = integration.yml.read_dict()['configuration'] + integration = create_integration_object() + curr_config = integration.data['configuration'] curr_config[0].pop("section") - integration.yml.update({'configuration': curr_config}) - - integration_parser = IntegrationParser( - Path(integration.path), list(MarketplaceVersions) - ) + integration.data['configuration'] = curr_config - results = StrictSchemaValidator().obtain_invalid_content_items([integration_parser]) + results = StrictSchemaValidator().obtain_invalid_content_items([integration]) assert len(results) == 1 - assert ( - results[0].message - == "Structure error (value_error.missing) in field configuration,0,section of integration_0.yml: " - "The field configuration,0,section is required but missing" - ) + assert results[0].message == f'Missing section for configuration {curr_config[0].get("name")}' diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py b/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py index dd44a80938d..9845f0e3852 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py @@ -77,12 +77,6 @@ AssetsModelingRule, ] -EXCLUDED_FIELDS = [ - "section", - "sectionorder" -] - - class SchemaValidator(BaseValidator[ContentTypes]): error_code = "ST110" description = "Validate that the scheme's structure is valid, while excluding certain fields." @@ -105,11 +99,4 @@ def obtain_invalid_content_items( ] def is_invalid_schema(self, content_item: ContentTypes) -> bool: - return bool(content_item.structure_errors) and not self.is_error_excluded(content_item.structure_errors) - - def is_error_excluded(self, structure_errors: List[StructureError]) -> bool: - for error in structure_errors: - for field_name in error.field_name: - if field_name and field_name in EXCLUDED_FIELDS: - return True - return False \ No newline at end of file + return bool(content_item.structure_errors) \ No newline at end of file diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py index 887b1b31982..59b07cc9abc 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py @@ -1,88 +1,47 @@ -from typing import Union +from typing import Union, Iterable from demisto_sdk.commands.common.constants import GitStatuses -from demisto_sdk.commands.validate.validators.ST_validators.ST110_is_valid_scheme import SchemaValidator -from demisto_sdk.commands.content_graph.objects import ( - AssetsModelingRule, - CaseField, - CaseLayout, - CaseLayoutRule, - Classifier, - CorrelationRule, - Dashboard, - GenericDefinition, - GenericField, - GenericModule, - GenericType, - IncidentField, - IncidentType, - IndicatorField, - IndicatorType, - Job, - Layout, - LayoutRule, - Mapper, - ModelingRule, - Pack, - ParsingRule, - Playbook, - PreProcessRule, - Report, - Widget, - Wizard, - XDRCTemplate, - XSIAMDashboard, - XSIAMReport, -) from demisto_sdk.commands.content_graph.objects.integration import Integration -from demisto_sdk.commands.content_graph.objects.list import List as ListObject -from demisto_sdk.commands.content_graph.objects.script import Script -from demisto_sdk.commands.content_graph.strict_objects.base_strict_model import StructureError from demisto_sdk.commands.validate.validators.base_validator import ( BaseValidator, ValidationResult, ) -ContentTypes = Union[ - Integration, - Script, - IncidentField, - IndicatorField, - IncidentType, - GenericType, - Classifier, - Layout, - LayoutRule, - Playbook, - CorrelationRule, - Dashboard, - GenericDefinition, - GenericField, - GenericModule, - Job, - ListObject, - Mapper, - ParsingRule, - PreProcessRule, - Report, - Widget, - Wizard, - XDRCTemplate, - XSIAMDashboard, - XSIAMReport, - IndicatorType, - CaseField, - CaseLayout, - CaseLayoutRule, - Pack, - ModelingRule, - AssetsModelingRule, +ContentTypes = Integration + +ALLOWED_SECTIONS = [ + "Connect", + "Collect", + "Optimize", ] -class StrictSchemaValidator(SchemaValidator): +class StrictSchemaValidator(BaseValidator[ContentTypes]): error_code = "ST111" description = "Validate that the scheme's structure is valid, no fields excluded." rationale = "Maintain valid structure for content items." expected_git_statuses = [GitStatuses.MODIFIED, GitStatuses.ADDED] - def is_invalid_schema(self, content_item: ContentTypes) -> bool: - return bool(content_item.structure_errors) \ No newline at end of file + def obtain_invalid_content_items( + self, + content_items: Iterable[ContentTypes], + ) -> list[ValidationResult]: + invalid_content_items = [] + for content_item in content_items: + error_message = self.get_error_message(content_item) + if error_message: + invalid_content_items.append(ValidationResult( + validator=self, + message=error_message, + content_object=content_item, + )) + return invalid_content_items + + def get_error_message(self, content_item: ContentTypes) -> str: + section_order = content_item.data.get('sectionorder') or content_item.data.get('sectionOrder') + if not section_order: + return 'Missing section order' + configurations = content_item.data.get('configuration') + for configuration in configurations: + section = configuration.get('section') + if not section: + return f'Missing section for configuration {configuration.get("name")}' + return '' \ No newline at end of file From 9ffecd9b627fac8b4a73b9662045a7dee7e6ecd7 Mon Sep 17 00:00:00 2001 From: TalZich Date: Wed, 1 Jan 2025 16:18:29 +0200 Subject: [PATCH 09/13] pre-commit auto fix --- .../strict_objects/base_strict_model.py | 5 ++- .../content_graph/strict_objects/common.py | 4 +- .../strict_objects/integration.py | 37 +++++++++-------- .../validate/tests/ST_validators_test.py | 40 ++++++++++--------- .../ST_validators/ST110_is_valid_scheme.py | 8 ++-- .../ST111_no_exclusions_schema.py | 32 +++++++++------ 6 files changed, 74 insertions(+), 52 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py b/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py index 23e6c714c9f..f9984b03486 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py +++ b/demisto_sdk/commands/content_graph/strict_objects/base_strict_model.py @@ -119,7 +119,10 @@ class StructureError(BaseStrictModel): def __str__(self): field_name = ",".join(more_itertools.always_iterable(self.field_name)) if self.error_type == "assertion_error": - error_message = self.error_message or f"An assertion error occurred for field {field_name}" + error_message = ( + self.error_message + or f"An assertion error occurred for field {field_name}" + ) elif self.error_type == "value_error.extra": error_message = f"The field {field_name} is extra and {self.error_message}" elif self.error_type == "value_error.missing": diff --git a/demisto_sdk/commands/content_graph/strict_objects/common.py b/demisto_sdk/commands/content_graph/strict_objects/common.py index 06cb98dcc18..4f9aab0c2a9 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/common.py +++ b/demisto_sdk/commands/content_graph/strict_objects/common.py @@ -71,7 +71,9 @@ def prevent_none(cls, value, field): "breaking_changes_notes", # release-notes-config }: # The assertion is caught by pydantic and converted to a pydantic.ValidationError - assert value is not None, f"The field {field.name} is not required, but should not be None if it exists" + assert ( + value is not None + ), f"The field {field.name} is not required, but should not be None if it exists" return value diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index f4d3334f9ab..eb637b25007 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -137,9 +137,10 @@ class _CommonFieldsIntegration(BaseStrictModel): ), ) + class SectionOrderValues(str, Enum): - CONNECT = "Connect", - COLLECT = "Collect", + CONNECT = ("Connect",) + COLLECT = ("Collect",) OPTIMIZE = "Optimize" @@ -148,8 +149,10 @@ class _StrictIntegration(BaseStrictModel): display: str beta: Optional[bool] = None category: str - section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = Field(alias="sectionorder") - configurations: List[Configuration] = Field(..., alias="configuration")# type:ignore[valid-type] + section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = ( + Field(alias="sectionorder") + ) + configurations: List[Configuration] = Field(..., alias="configuration") # type:ignore[valid-type] image: Optional[str] = None description: str default_mapper_in: Optional[str] = Field(None, alias="defaultmapperin") @@ -171,14 +174,15 @@ def __init__(self, **data): Initializes the _StrictIntegration object. Using this custom init function to support two aliases for the section_order field. """ - if 'sectionOrder' in data and not 'sectionorder' in data: - data['sectionorder'] = data.pop('sectionOrder') - elif 'sectionOrder' in data and 'sectionorder' in data: - data['sectionorder'] = list(set(data['section_order']) | set(data['section_order_camel_case'])) + if "sectionOrder" in data and "sectionorder" not in data: + data["sectionorder"] = data.pop("sectionOrder") + elif "sectionOrder" in data and "sectionorder" in data: + data["sectionorder"] = list( + set(data["section_order"]) | set(data["section_order_camel_case"]) + ) super().__init__(**data) - - @validator('configurations') + @validator("configurations") def validate_sections(cls, configurations, values): """ Validates each configuration object has a valid section clause. @@ -186,20 +190,21 @@ def validate_sections(cls, configurations, values): Even if the section is an allowed value (currently Collect, Connect or Optimize),it could be invalid if the specific value is not present in section_order. """ - section_order_field = values.get('section_order') + section_order_field = values.get("section_order") if not section_order_field: return configurations - integration_sections = [section_name.value for section_name in section_order_field] + integration_sections = [ + section_name.value for section_name in section_order_field + ] for config in configurations: if not config.section: return configurations - assert config.section in integration_sections,\ - f'section {config.section} of {config.display} is not present in section_order {integration_sections}' + assert ( + config.section in integration_sections + ), f"section {config.section} of {config.display} is not present in section_order {integration_sections}" return configurations - - StrictIntegration = create_model( model_name="StrictIntegration", base_models=( diff --git a/demisto_sdk/commands/validate/tests/ST_validators_test.py b/demisto_sdk/commands/validate/tests/ST_validators_test.py index 4b7944128c4..4d6cb690df8 100644 --- a/demisto_sdk/commands/validate/tests/ST_validators_test.py +++ b/demisto_sdk/commands/validate/tests/ST_validators_test.py @@ -15,9 +15,10 @@ from demisto_sdk.commands.validate.validators.ST_validators.ST110_is_valid_scheme import ( SchemaValidator, ) +from demisto_sdk.commands.validate.validators.ST_validators.ST111_no_exclusions_schema import ( + StrictSchemaValidator, +) from TestSuite.pack import Pack -from demisto_sdk.commands.validate.validators.ST_validators.ST111_no_exclusions_schema import StrictSchemaValidator, \ - ALLOWED_SECTIONS def test_sanity_SchemaValidator(): @@ -254,7 +255,7 @@ def test_invalid_section_order(pack: Pack): - the integration is invalid and the correct error message is returned """ integration = pack.create_integration(yml=load_yaml("integration.yml")) - integration.yml.update({'sectionorder': ["Connect", "Run"]}) + integration.yml.update({"sectionorder": ["Connect", "Run"]}) integration_parser = IntegrationParser( Path(integration.path), list(MarketplaceVersions) @@ -294,9 +295,9 @@ def test_invalid_section(pack: Pack): - the integration is invalid and the correct error message is returned """ integration = pack.create_integration(yml=load_yaml("integration.yml")) - curr_config = integration.yml.read_dict()['configuration'] - curr_config[0]['section'] = "Run" - integration.yml.update({'configuration': curr_config}) + curr_config = integration.yml.read_dict()["configuration"] + curr_config[0]["section"] = "Run" + integration.yml.update({"configuration": curr_config}) integration_parser = IntegrationParser( Path(integration.path), list(MarketplaceVersions) @@ -316,9 +317,9 @@ def test_missing_section(pack: Pack): - the integration is invalid and the correct error message is returned """ integration = pack.create_integration(yml=load_yaml("integration.yml")) - curr_config = integration.yml.read_dict()['configuration'] + curr_config = integration.yml.read_dict()["configuration"] curr_config[0].pop("section") - integration.yml.update({'configuration': curr_config}) + integration.yml.update({"configuration": curr_config}) integration_parser = IntegrationParser( Path(integration.path), list(MarketplaceVersions) @@ -338,7 +339,9 @@ def test_invalid_section_order(self): Then: - the integration is invalid and the correct error message is returned """ - integration = create_integration_object(paths=['sectionorder'], values=[["Connect", "Run"]]) + integration = create_integration_object( + paths=["sectionorder"], values=[["Connect", "Run"]] + ) results = StrictSchemaValidator().obtain_invalid_content_items([integration]) assert len(results) == 0 @@ -357,7 +360,7 @@ def test_missing_section_order(self): results = StrictSchemaValidator().obtain_invalid_content_items([integration]) assert len(results) == 1 - assert results[0].message == 'Missing section order' + assert results[0].message == "Missing section order" def test_invalid_section(self): """ @@ -369,15 +372,13 @@ def test_invalid_section(self): - the integration is invalid and the correct error message is returned """ integration = create_integration_object() - curr_config = integration.data['configuration'] - curr_config[0]['section'] = "Run" - integration.data['configuration'] = curr_config - + curr_config = integration.data["configuration"] + curr_config[0]["section"] = "Run" + integration.data["configuration"] = curr_config results = StrictSchemaValidator().obtain_invalid_content_items([integration]) assert len(results) == 0 - def test_missing_section(self, pack: Pack): """ Given: @@ -388,10 +389,13 @@ def test_missing_section(self, pack: Pack): - the integration is invalid and the correct error message is returned """ integration = create_integration_object() - curr_config = integration.data['configuration'] + curr_config = integration.data["configuration"] curr_config[0].pop("section") - integration.data['configuration'] = curr_config + integration.data["configuration"] = curr_config results = StrictSchemaValidator().obtain_invalid_content_items([integration]) assert len(results) == 1 - assert results[0].message == f'Missing section for configuration {curr_config[0].get("name")}' + assert ( + results[0].message + == f'Missing section for configuration {curr_config[0].get("name")}' + ) diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py b/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py index 9845f0e3852..ff4d1ab8151 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST110_is_valid_scheme.py @@ -35,7 +35,6 @@ from demisto_sdk.commands.content_graph.objects.integration import Integration from demisto_sdk.commands.content_graph.objects.list import List as ListObject from demisto_sdk.commands.content_graph.objects.script import Script -from demisto_sdk.commands.content_graph.strict_objects.base_strict_model import StructureError from demisto_sdk.commands.validate.validators.base_validator import ( BaseValidator, ValidationResult, @@ -77,9 +76,12 @@ AssetsModelingRule, ] + class SchemaValidator(BaseValidator[ContentTypes]): error_code = "ST110" - description = "Validate that the scheme's structure is valid, while excluding certain fields." + description = ( + "Validate that the scheme's structure is valid, while excluding certain fields." + ) rationale = "Maintain valid structure for content items." def obtain_invalid_content_items( @@ -99,4 +101,4 @@ def obtain_invalid_content_items( ] def is_invalid_schema(self, content_item: ContentTypes) -> bool: - return bool(content_item.structure_errors) \ No newline at end of file + return bool(content_item.structure_errors) diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py index 59b07cc9abc..8e96589fe62 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py @@ -1,4 +1,5 @@ -from typing import Union, Iterable +from typing import Iterable + from demisto_sdk.commands.common.constants import GitStatuses from demisto_sdk.commands.content_graph.objects.integration import Integration from demisto_sdk.commands.validate.validators.base_validator import ( @@ -14,6 +15,7 @@ "Optimize", ] + class StrictSchemaValidator(BaseValidator[ContentTypes]): error_code = "ST111" description = "Validate that the scheme's structure is valid, no fields excluded." @@ -21,27 +23,31 @@ class StrictSchemaValidator(BaseValidator[ContentTypes]): expected_git_statuses = [GitStatuses.MODIFIED, GitStatuses.ADDED] def obtain_invalid_content_items( - self, - content_items: Iterable[ContentTypes], + self, + content_items: Iterable[ContentTypes], ) -> list[ValidationResult]: invalid_content_items = [] for content_item in content_items: error_message = self.get_error_message(content_item) if error_message: - invalid_content_items.append(ValidationResult( - validator=self, - message=error_message, - content_object=content_item, - )) + invalid_content_items.append( + ValidationResult( + validator=self, + message=error_message, + content_object=content_item, + ) + ) return invalid_content_items def get_error_message(self, content_item: ContentTypes) -> str: - section_order = content_item.data.get('sectionorder') or content_item.data.get('sectionOrder') + section_order = content_item.data.get("sectionorder") or content_item.data.get( + "sectionOrder" + ) if not section_order: - return 'Missing section order' - configurations = content_item.data.get('configuration') + return "Missing section order" + configurations = content_item.data.get("configuration") for configuration in configurations: - section = configuration.get('section') + section = configuration.get("section") if not section: return f'Missing section for configuration {configuration.get("name")}' - return '' \ No newline at end of file + return "" From 03f06aef263150b3cc18407304fb440a4347c7d4 Mon Sep 17 00:00:00 2001 From: TalZich Date: Wed, 1 Jan 2025 16:32:40 +0200 Subject: [PATCH 10/13] Address pre-commit issues --- .../commands/content_graph/strict_objects/integration.py | 2 +- .../validators/ST_validators/ST111_no_exclusions_schema.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index eb637b25007..b9c83ffe6a2 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -149,7 +149,7 @@ class _StrictIntegration(BaseStrictModel): display: str beta: Optional[bool] = None category: str - section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = ( + section_order: Optional[conlist(SectionOrderValues, min_items=1, max_items=3)] = ( # type:ignore[valid-type] Field(alias="sectionorder") ) configurations: List[Configuration] = Field(..., alias="configuration") # type:ignore[valid-type] diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py index 8e96589fe62..7d844fbf352 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py @@ -46,7 +46,7 @@ def get_error_message(self, content_item: ContentTypes) -> str: if not section_order: return "Missing section order" configurations = content_item.data.get("configuration") - for configuration in configurations: + for configuration in configurations: # type:ignore[union-attr] section = configuration.get("section") if not section: return f'Missing section for configuration {configuration.get("name")}' From 84c5308bb7ee52be4e87457e4f8baa1ff3f16314 Mon Sep 17 00:00:00 2001 From: TalZich Date: Thu, 2 Jan 2025 10:50:12 +0200 Subject: [PATCH 11/13] Use StrEnum instead of Enum --- .../commands/content_graph/strict_objects/integration.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index b9c83ffe6a2..48bfda79c4b 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -1,4 +1,3 @@ -from enum import Enum from typing import Any, List, Optional from pydantic import Field, conlist, validator @@ -8,6 +7,7 @@ TYPE_PYTHON3, MarketplaceVersions, ) +from demisto_sdk.commands.common.StrEnum import StrEnum from demisto_sdk.commands.content_graph.strict_objects.base_strict_model import ( Argument, BaseIntegrationScript, @@ -138,9 +138,9 @@ class _CommonFieldsIntegration(BaseStrictModel): ) -class SectionOrderValues(str, Enum): - CONNECT = ("Connect",) - COLLECT = ("Collect",) +class SectionOrderValues(StrEnum): + CONNECT = "Connect" + COLLECT = "Collect" OPTIMIZE = "Optimize" From 3dfc1d2224f9329504eb75c0869a1af63770c121 Mon Sep 17 00:00:00 2001 From: TalZich Date: Thu, 2 Jan 2025 15:03:45 +0200 Subject: [PATCH 12/13] Address CR --- .../commands/validate/sdk_validation_config.toml | 1 - .../commands/validate/tests/ST_validators_test.py | 13 +++++++++---- .../ST_validators/ST111_no_exclusions_schema.py | 2 -- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index 26652479a06..3fb0e3b17bd 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -241,7 +241,6 @@ select = [ "MR107", "MR108", "ST110", - "ST111", "TR100" ] warning = [ diff --git a/demisto_sdk/commands/validate/tests/ST_validators_test.py b/demisto_sdk/commands/validate/tests/ST_validators_test.py index 4d6cb690df8..83c51190d3c 100644 --- a/demisto_sdk/commands/validate/tests/ST_validators_test.py +++ b/demisto_sdk/commands/validate/tests/ST_validators_test.py @@ -263,6 +263,9 @@ def test_invalid_section_order(pack: Pack): results = SchemaValidator().obtain_invalid_content_items([integration_parser]) assert len(results) == 1 + assert results[0].message == ("Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " + "value is not a valid enumeration member; permitted: " + "'Connect', 'Collect', 'Optimize'") def test_missing_section_order(pack: Pack): @@ -272,7 +275,7 @@ def test_missing_section_order(pack: Pack): When: - executing the IntegrationParser Then: - - the integration is invalid and the correct error message is returned + - the validation does not fail as it is only addressed in ST111 """ integration = pack.create_integration(yml=load_yaml("integration.yml")) integration.yml.delete_key("sectionorder") @@ -305,6 +308,8 @@ def test_invalid_section(pack: Pack): results = SchemaValidator().obtain_invalid_content_items([integration_parser]) assert len(results) == 1 + assert results[0].message == ("Structure error (assertion_error) in field configuration of integration_0.yml: " + "section Run of URL is not present in section_order ['Connect']") def test_missing_section(pack: Pack): @@ -314,7 +319,7 @@ def test_missing_section(pack: Pack): When: - executing the IntegrationParser Then: - - the integration is invalid and the correct error message is returned + - the validation does not fail as it is only addressed in ST111 """ integration = pack.create_integration(yml=load_yaml("integration.yml")) curr_config = integration.yml.read_dict()["configuration"] @@ -337,7 +342,7 @@ def test_invalid_section_order(self): When: - executing the IntegrationParser Then: - - the integration is invalid and the correct error message is returned + - the validation does not fail as it is only addressed in ST110 """ integration = create_integration_object( paths=["sectionorder"], values=[["Connect", "Run"]] @@ -369,7 +374,7 @@ def test_invalid_section(self): When: - executing the IntegrationParser Then: - - the integration is invalid and the correct error message is returned + - the validation does not fail as it is only addressed in ST110 """ integration = create_integration_object() curr_config = integration.data["configuration"] diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py index 7d844fbf352..f73a1fecd0a 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py @@ -1,6 +1,5 @@ from typing import Iterable -from demisto_sdk.commands.common.constants import GitStatuses from demisto_sdk.commands.content_graph.objects.integration import Integration from demisto_sdk.commands.validate.validators.base_validator import ( BaseValidator, @@ -20,7 +19,6 @@ class StrictSchemaValidator(BaseValidator[ContentTypes]): error_code = "ST111" description = "Validate that the scheme's structure is valid, no fields excluded." rationale = "Maintain valid structure for content items." - expected_git_statuses = [GitStatuses.MODIFIED, GitStatuses.ADDED] def obtain_invalid_content_items( self, From 67de9f3c01ce428b55329b93d03bfd49c1f1cedc Mon Sep 17 00:00:00 2001 From: TalZich Date: Thu, 2 Jan 2025 16:16:49 +0200 Subject: [PATCH 13/13] Address CR and pre-commit --- .changelog/4739.yml | 3 ++- .../content_graph/strict_objects/integration.py | 2 +- .../commands/validate/tests/ST_validators_test.py | 14 +++++++++----- .../ST_validators/ST111_no_exclusions_schema.py | 5 ++--- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.changelog/4739.yml b/.changelog/4739.yml index a03ea7538df..dde131fb0cb 100644 --- a/.changelog/4739.yml +++ b/.changelog/4739.yml @@ -1,4 +1,5 @@ changes: -- description: This PR adds to the ST-110 validation which will now validate the sectionOrder and individual sections. +- description: "This PR adds to the ST-110 validation which will now validate the sectionOrder and individual sections, + as well as adding a new validation, ST-111, which will validate the existence of the same fields." type: feature pr_number: 4739 diff --git a/demisto_sdk/commands/content_graph/strict_objects/integration.py b/demisto_sdk/commands/content_graph/strict_objects/integration.py index 48bfda79c4b..55d8ebda94a 100644 --- a/demisto_sdk/commands/content_graph/strict_objects/integration.py +++ b/demisto_sdk/commands/content_graph/strict_objects/integration.py @@ -41,7 +41,7 @@ class _Configuration(BaseStrictModel): display: Optional[str] = None - section: Optional[str] + section: Optional[str] = None advanced: Optional[str] = None default_value: Optional[Any] = Field(None, alias="defaultvalue") name: str diff --git a/demisto_sdk/commands/validate/tests/ST_validators_test.py b/demisto_sdk/commands/validate/tests/ST_validators_test.py index 83c51190d3c..aa1e4e4ea69 100644 --- a/demisto_sdk/commands/validate/tests/ST_validators_test.py +++ b/demisto_sdk/commands/validate/tests/ST_validators_test.py @@ -263,9 +263,11 @@ def test_invalid_section_order(pack: Pack): results = SchemaValidator().obtain_invalid_content_items([integration_parser]) assert len(results) == 1 - assert results[0].message == ("Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " - "value is not a valid enumeration member; permitted: " - "'Connect', 'Collect', 'Optimize'") + assert results[0].message == ( + "Structure error (type_error.enum) in field sectionorder,1 of integration_0.yml: " + "value is not a valid enumeration member; permitted: " + "'Connect', 'Collect', 'Optimize'" + ) def test_missing_section_order(pack: Pack): @@ -308,8 +310,10 @@ def test_invalid_section(pack: Pack): results = SchemaValidator().obtain_invalid_content_items([integration_parser]) assert len(results) == 1 - assert results[0].message == ("Structure error (assertion_error) in field configuration of integration_0.yml: " - "section Run of URL is not present in section_order ['Connect']") + assert results[0].message == ( + "Structure error (assertion_error) in field configuration of integration_0.yml: " + "section Run of URL is not present in section_order ['Connect']" + ) def test_missing_section(pack: Pack): diff --git a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py index f73a1fecd0a..f812df74be3 100644 --- a/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py +++ b/demisto_sdk/commands/validate/validators/ST_validators/ST111_no_exclusions_schema.py @@ -26,8 +26,7 @@ def obtain_invalid_content_items( ) -> list[ValidationResult]: invalid_content_items = [] for content_item in content_items: - error_message = self.get_error_message(content_item) - if error_message: + if error_message := self.is_missing_section_fields(content_item): invalid_content_items.append( ValidationResult( validator=self, @@ -37,7 +36,7 @@ def obtain_invalid_content_items( ) return invalid_content_items - def get_error_message(self, content_item: ContentTypes) -> str: + def is_missing_section_fields(self, content_item: ContentTypes) -> str: section_order = content_item.data.get("sectionorder") or content_item.data.get( "sectionOrder" )