Skip to content

Commit

Permalink
validation: issue warning for extra keys in REANA specs
Browse files Browse the repository at this point in the history
When validating the REANA specification, whitelist some keys in the JSON
schema (`additionalProperties`) that, when not correctly validated, will
 issue a warning rather than raising a critical error.
 The dictionary of warnings is returned by the validator function.

Closes reanahub/reana-client#660
  • Loading branch information
giuseppe-steduto committed Aug 24, 2023
1 parent dd3d0ba commit dfd53e3
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 7 deletions.
9 changes: 9 additions & 0 deletions reana_commons/openapi_specifications/reana_server.json
Original file line number Diff line number Diff line change
Expand Up @@ -582,13 +582,22 @@
"message": {
"type": "string"
},
"validation_warnings": {
"description": "Dictionary of validation warnings, if any. Each key is a property that was not correctly validated.",
"type": "object"
},
"workflow_id": {
"type": "string"
},
"workflow_name": {
"type": "string"
}
},
"required": [
"workflow_id",
"workflow_name",
"message"
],
"type": "object"
}
},
Expand Down
66 changes: 60 additions & 6 deletions reana_commons/validation/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
import json
import logging
import os
import re
from typing import Dict, List

from jsonschema import ValidationError, validate
from jsonschema import ValidationError
from jsonschema.exceptions import best_match
from jsonschema.validators import validator_for

from reana_commons.config import (
REANA_WORKFLOW_NAME_ILLEGAL_CHARACTERS,
Expand All @@ -23,7 +26,29 @@
from reana_commons.errors import REANAValidationError


def validate_reana_yaml(reana_yaml: Dict) -> None:
def _parse_schema_validation_error(error: ValidationError) -> List:
"""Parse a schema validation error.
When validating the REANA specification file against the REANA specification
schema, the validator can return many ValidationError object. This function parses
the error and returns a list of strings or other objects that, together with the reason
of the error, can be used to extract more info about the error.
"""
if error.validator == "additionalProperties":
# If the error is about additional properties, we want to return the
# name(s) of the additional properties in a list.
# There is no easy way to extract the name of the additional properties,
# so we parse the error message. See https://github.com/reanahub/reana-commons/pull/405

# The error message is of the form:
# "Additional properties are not allowed ('<property>' was unexpected)"
# "Additional properties are not allowed ('<property1>', '<property2>' were unexpected)"
content_inside_parentheses = re.search(r"\((.*?)\)", error.message).group(1)
return re.findall(r"'(.*?)'", content_inside_parentheses or "")
return [error.message]

Check warning on line 48 in reana_commons/validation/utils.py

View check run for this annotation

Codecov / codecov/patch

reana_commons/validation/utils.py#L48

Added line #L48 was not covered by tests


def validate_reana_yaml(reana_yaml: Dict) -> Dict:
"""Validate REANA specification file according to jsonschema.
:param reana_yaml: Dictionary which represents REANA specification file.
Expand All @@ -32,18 +57,47 @@ def validate_reana_yaml(reana_yaml: Dict) -> None:
"""
try:
with open(reana_yaml_schema_file_path, "r") as f:
# Create validator from REANA specification schema
reana_yaml_schema = json.loads(f.read())
validate(reana_yaml, reana_yaml_schema)
validator_class = validator_for(reana_yaml_schema)
validator_class.check_schema(reana_yaml_schema)
validator = validator_class(reana_yaml_schema)

# Collect all validation errors
errors = [e for e in validator.iter_errors(reana_yaml)]

non_critical_validators = ["additionalProperties"]
# Depending on whether a validator is critical or not,
# separate errors into 'critical' and 'warnings'
critical_errors = []
# The warning dictionary has as keys the properties that are not
# respected, and as values, a list of strings that invalidates the property
# or describe the error
warnings = {}
for e in errors:
if e.validator in non_critical_validators:
warnings.setdefault(e.validator, []).extend(
_parse_schema_validation_error(e)
)
else:
critical_errors.append(e)

# If there are critical errors, log and raise exception
if critical_errors:
err = best_match(critical_errors)
logging.error(
"Invalid REANA specification: {error}".format(error=err.message)
)
raise REANAValidationError(err)

return warnings
except IOError as e:
logging.info(
"Something went wrong when reading REANA validation schema from "
"{filepath} : \n"
"{error}".format(filepath=reana_yaml_schema_file_path, error=e.strerror)
)
raise e
except ValidationError as e:
logging.info("Invalid REANA specification: {error}".format(error=e.message))
raise e


def validate_workflow_name(workflow_name: str) -> str:
Expand Down
46 changes: 45 additions & 1 deletion tests/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
"""REANA-Commons validation testing."""

import pytest
from jsonschema.exceptions import ValidationError

from reana_commons.errors import REANAValidationError
from reana_commons.validation.utils import validate_reana_yaml


Expand All @@ -27,4 +29,46 @@ def test_validation_retention_days(yadage_workflow_spec_loaded, retention_days):
"""Test the validation of ``retention_days`` section of ``reana.yaml``."""
reana_yaml = yadage_workflow_spec_loaded
reana_yaml.setdefault("workspace", {}).update({"retention_days": retention_days})
validate_reana_yaml(reana_yaml)
warnings = validate_reana_yaml(reana_yaml)
assert warnings == {}


@pytest.mark.parametrize(
"extra_keys,expected_warnings",
[
(["wrong_key"], {"additionalProperties": ["wrong_key"]}),
(
["wrong_key", "wrong_key2"],
{"additionalProperties": ["wrong_key", "wrong_key2"]},
),
([], {}),
],
)
def test_warnings_reana_yaml(
yadage_workflow_spec_loaded, extra_keys, expected_warnings
):
"""Test the validation of the ``reana.yaml`` file.
Check that the validation returns the expected warnings when there is an
unexpected key in the specification.
"""
reana_yaml = yadage_workflow_spec_loaded
for key in extra_keys:
reana_yaml[key] = "value"
warnings = validate_reana_yaml(reana_yaml)
assert set(expected_warnings.keys()) == set(warnings.keys())
for key, value in expected_warnings.items():
assert set(value) == set(warnings[key])


def test_critical_errors_reana_yaml(yadage_workflow_spec_loaded):
"""Test the validation of the ``reana.yaml`` file.
Test that the validation raises an error when a required key
is missing in the specification (critical error).
"""
# Delete a required key
reana_yaml = yadage_workflow_spec_loaded
del reana_yaml["workflow"]
with pytest.raises(REANAValidationError):
validate_reana_yaml(reana_yaml)

0 comments on commit dfd53e3

Please sign in to comment.