From 11a1a7cce03b40e112bab662aede7abc1fd7ca14 Mon Sep 17 00:00:00 2001 From: Jennifer Power Date: Thu, 12 Oct 2023 13:49:21 -0400 Subject: [PATCH] PSCE-239: feat: adds validation for YAML trestle rules (#52) * feat(transformers): initial validation handler from pre-validation before pydantic Signed-off-by: Jennifer Power * chore: fixes spelling error on csv_transformer.py comment Signed-off-by: Jennifer Power * feat(task): adds collection of validation errors in rules task Updates the rules transform task to make sure all errors for a component defintion are collection during transformation and presented back to the user to make errors easier to address at one time. Signed-off-by: Jennifer Power --------- Signed-off-by: Jennifer Power --- .../transformers/test_validations.py | 60 +++++++++++ .../transformers/test_yaml_transformer.py | 19 ++++ trestlebot/tasks/rule_transform_task.py | 12 ++- trestlebot/transformers/csv_transformer.py | 2 +- trestlebot/transformers/validations.py | 100 ++++++++++++++++++ trestlebot/transformers/yaml_transformer.py | 15 ++- 6 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 tests/trestlebot/transformers/test_validations.py create mode 100644 trestlebot/transformers/validations.py diff --git a/tests/trestlebot/transformers/test_validations.py b/tests/trestlebot/transformers/test_validations.py new file mode 100644 index 00000000..1362a682 --- /dev/null +++ b/tests/trestlebot/transformers/test_validations.py @@ -0,0 +1,60 @@ +#!/usr/bin/python + +# Copyright 2023 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Test for Validations.""" +from typing import Any, Dict + +from trestlebot.transformers.validations import ( + ValidationHandler, + ValidationOutcome, + parameter_validation, +) + + +def test_parameter_validation(valid_rule_data: Dict[str, Any]) -> None: + """Test parameter validation with valid data.""" + result: ValidationOutcome = ValidationOutcome(errors=[], valid=True) + parameter_validation(valid_rule_data, result) + assert result.valid + + +def test_parameter_validation_with_error( + invalid_param_rule_data: Dict[str, Any] +) -> None: + """Test parameter validation with invalid parameter.""" + result: ValidationOutcome = ValidationOutcome(errors=[], valid=True) + parameter_validation(invalid_param_rule_data, result) + assert not result.valid + assert len(result.errors) == 1 + assert ( + result.errors[0].error_message + == "Default value must be one of the alternative values" + ) + + +def test_parameter_validation_with_handler( + invalid_param_rule_data: Dict[str, Any] +) -> None: + """Test parameter validation with handler.""" + result: ValidationOutcome = ValidationOutcome(errors=[], valid=True) + handler = ValidationHandler(parameter_validation) + handler.handle(invalid_param_rule_data, result) + assert not result.valid + assert len(result.errors) == 1 + assert ( + result.errors[0].error_message + == "Default value must be one of the alternative values" + ) diff --git a/tests/trestlebot/transformers/test_yaml_transformer.py b/tests/trestlebot/transformers/test_yaml_transformer.py index 55bbb14d..55ee2336 100644 --- a/tests/trestlebot/transformers/test_yaml_transformer.py +++ b/tests/trestlebot/transformers/test_yaml_transformer.py @@ -21,6 +21,7 @@ from tests.testutils import YAML_TEST_DATA_PATH from trestlebot.transformers.base_transformer import RulesTransformerException from trestlebot.transformers.trestle_rule import TrestleRule +from trestlebot.transformers.validations import ValidationHandler, parameter_validation from trestlebot.transformers.yaml_transformer import ( FromRulesYAMLTransformer, ToRulesYAMLTransformer, @@ -88,6 +89,24 @@ def test_rules_transform_with_invalid_rule() -> None: transformer.transform(rule_file_info) +def test_rules_transform_with_additional_validation() -> None: + """Test rules transform with additional validation.""" + # load rule from path and close the file + # get the file info as a string + rule_path = YAML_TEST_DATA_PATH / "test_rule_invalid_params.yaml" + rule_file = open(rule_path, "r") + rule_file_info = rule_file.read() + rule_file.close() + validation_handler_chain = ValidationHandler(parameter_validation) + transformer = ToRulesYAMLTransformer(validation_handler_chain) + + with pytest.raises( + RulesTransformerException, + match=".*Default value must be one of the alternative values", + ): + transformer.transform(rule_file_info) + + def test_read_write_integration(test_rule: TrestleRule) -> None: """Test read/write integration.""" from_rules_transformer = FromRulesYAMLTransformer() diff --git a/trestlebot/tasks/rule_transform_task.py b/trestlebot/tasks/rule_transform_task.py index 7944673e..71003efa 100644 --- a/trestlebot/tasks/rule_transform_task.py +++ b/trestlebot/tasks/rule_transform_task.py @@ -93,6 +93,10 @@ def _transform_components(self, component_definition_path: pathlib.Path) -> None logger.debug( f"Transforming rules for component definition {component_definition_path.name}" ) + + # To report all rule errors at once, we collect them in a list and + # pretty print them in a raised exception + transformation_errors: List[str] = [] for component in self.iterate_models(component_definition_path): for rule_path in self.iterate_models(component): # Load the rule into memory as a stream to process @@ -102,10 +106,16 @@ def _transform_components(self, component_definition_path: pathlib.Path) -> None rule = self._rule_transformer.transform(rule_stream) csv_builder.add_row(rule) except RulesTransformerException as e: - raise TaskException( + transformation_errors.append( f"Failed to transform rule {rule_path.name}: {e}" ) + if len(transformation_errors) > 0: + raise TaskException( + f"Failed to transform rules for component definition {component_definition_path.name}: \ + \n{', '.join(transformation_errors)}" + ) + if csv_builder.row_count == 0: raise TaskException( f"No rules found for component definition {component_definition_path.name}" diff --git a/trestlebot/transformers/csv_transformer.py b/trestlebot/transformers/csv_transformer.py index f3bae64f..3c7f6587 100644 --- a/trestlebot/transformers/csv_transformer.py +++ b/trestlebot/transformers/csv_transformer.py @@ -14,7 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. -"""CSV Tranformer for rule authoring.""" +"""CSV Transformer for rule authoring.""" import csv import json diff --git a/trestlebot/transformers/validations.py b/trestlebot/transformers/validations.py new file mode 100644 index 00000000..96bf885d --- /dev/null +++ b/trestlebot/transformers/validations.py @@ -0,0 +1,100 @@ +#!/usr/bin/python + +# Copyright 2023 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Trestle Validation for rule authoring. + +This is meant to be extensible for future validations. +Base rule validation and utility functions are defined here. +""" + +import logging +from typing import Any, Callable, Dict, List, Optional + +from pydantic import BaseModel + +from trestlebot import const + + +logger = logging.getLogger(__name__) + + +class RuleValidationError(BaseModel): + """RuleValidationError model.""" + + field_name: str + error_message: str + + +class ValidationOutcome(BaseModel): + """ValidationOutcome model.""" + + errors: List[RuleValidationError] + valid: bool + + +class ValidationHandler: + def __init__( # type: ignore + self, validate_fn: Callable[[Any, ValidationOutcome], None], next_handler=None + ) -> None: + self.validate_fn: Callable[[Any, ValidationOutcome], None] = validate_fn + self.next_handler: Optional[ValidationHandler] = next_handler + + def handle(self, data: Any, result: ValidationOutcome) -> None: + self.validate_fn(data, result) + if self.next_handler: + self.next_handler.handle(data, result) + + +# TODO(jpower432): Create a class for Profile validation to ensure unique +# entries exists in the workspace. + + +def parameter_validation(data: Dict[str, Any], result: ValidationOutcome) -> None: + """Parameter logic additions validation.""" + rule_info: Dict[str, Any] = data.get(const.RULE_INFO_TAG, {}) + parameter_data: Dict[str, Any] = rule_info.get(const.PARAMETER, {}) + + if not parameter_data: + logger.debug("No parameter data found") + return # No parameter data, nothing to validate + + default_value = parameter_data.get(const.DEFAULT_VALUE, "") + alternative_values: Dict[str, Any] = parameter_data.get( + const.ALTERNATIVE_VALUES, {} + ) + + if not default_value: + add_validation_error(result, const.PARAMETER, "Default value is required") + + if not alternative_values: + add_validation_error(result, const.PARAMETER, "Alternative values are required") + + default_value_alt = alternative_values.get("default", "") + + if not default_value_alt or default_value_alt != default_value: + add_validation_error( + result, + const.PARAMETER, + "Default value must be one of the alternative values", + ) + + +def add_validation_error(result: ValidationOutcome, field: str, error_msg: str) -> None: + """Add a validation error to the result.""" + validation_error = RuleValidationError(field_name=field, error_message=error_msg) + result.errors.append(validation_error) + result.valid = False diff --git a/trestlebot/transformers/yaml_transformer.py b/trestlebot/transformers/yaml_transformer.py index f03debf0..ef2a1204 100644 --- a/trestlebot/transformers/yaml_transformer.py +++ b/trestlebot/transformers/yaml_transformer.py @@ -18,7 +18,7 @@ import logging import pathlib from io import StringIO -from typing import Any, Dict +from typing import Any, Dict, Optional from pydantic import ValidationError from ruamel.yaml import YAML @@ -35,6 +35,7 @@ Profile, TrestleRule, ) +from trestlebot.transformers.validations import ValidationHandler, ValidationOutcome logger = logging.getLogger(__name__) @@ -43,8 +44,9 @@ class ToRulesYAMLTransformer(ToRulesTransformer): """Interface for YAML transformer to Rules model.""" - def __init__(self) -> None: + def __init__(self, validator: Optional[ValidationHandler] = None) -> None: """Initialize.""" + self.validator: Optional[ValidationHandler] = validator super().__init__() def transform(self, blob: str) -> TrestleRule: @@ -53,6 +55,15 @@ def transform(self, blob: str) -> TrestleRule: yaml = YAML(typ="safe") yaml_data: Dict[str, Any] = yaml.load(blob) + logger.debug("Executing pre-validation on YAML data") + if self.validator is not None: + result = ValidationOutcome(errors=[], valid=True) + self.validator.handle(yaml_data, result) + if not result.valid: + raise RulesTransformerException( + f"Invalid YAML file: {result.errors}" + ) + rule_info_data = yaml_data[const.RULE_INFO_TAG] profile_data = rule_info_data[const.PROFILE]