diff --git a/CHANGES.rst b/CHANGES.rst index 80cc9a9c..e0a30a31 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,8 @@ Version 0.9.4 (UNRELEASED) - Changes the REANA specification schema to use the ``draft-07`` version of the JSON schema specification. - Changes validation of REANA specification to expose functions for loading workflow input parameters and workflow specifications. +- Changes the validation schema of the REANA specification to make the ``environment`` property mandatory for the steps of serial workflows. +- Changes the validation schema of the REANA specification to raise a warning for unexpected properties for the steps of serial workflows. - Changes CVMFS support to allow users to automatically mount any available repository. - Fixes the mounting of CVMFS volumes for the REANA deployments that use non-default Kubernetes namespace. diff --git a/reana_commons/validation/schemas/reana_analysis_schema.json b/reana_commons/validation/schemas/reana_analysis_schema.json index 496d5a96..44ab4af2 100644 --- a/reana_commons/validation/schemas/reana_analysis_schema.json +++ b/reana_commons/validation/schemas/reana_analysis_schema.json @@ -109,7 +109,8 @@ "type": "boolean", "title": "Kerberos authentication for the whole workflow." } - } + }, + "additionalProperties": false } }, "anyOf": [ @@ -200,5 +201,127 @@ } } } + }, + "if": { + "properties": { + "workflow": { + "properties": { + "type": { + "const": "serial" + } + } + } + } + }, + "then": { + "properties": { + "workflow": { + "properties": { + "specification": { + "type": "object", + "title": "Serial workflow specification.", + "description": "Serial workflow specification.", + "additionalProperties": false, + "properties": { + "steps": { + "type": "array", + "title": "Serial workflow steps.", + "description": "List of steps which represent the workflow.", + "items": { + "type": "object", + "title": "Serial workflow step.", + "description": "Serial workflow step.", + "additionalProperties": false, + "properties": { + "commands": { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "title": "Step commands", + "description": "List of commands to be run in the step." + }, + "compute_backend": { + "type": "string", + "enum": [ + "kubernetes", + "htcondor", + "htcondorcern", + "slurm", + "slurmcern" + ], + "title": "Compute backend" + }, + "environment": { + "type": "string", + "title": "Container image for the step", + "description": "Image to be used by the container in which the step should be run." + }, + "htcondor_accounting_group": { + "type": "string", + "title": "HTCondor accounting group" + }, + "htcondor_max_runtime": { + "type": "string", + "title": "HTCondor maximum runtime" + }, + "kerberos": { + "type": "boolean", + "title": "Use Kerberos authentication", + "description": "Whether to use Kerberos authentication for the step. This would require you to upload a valid Kerberos ticket as a REANA secret." + }, + "kubernetes_job_timeout": { + "type": "integer", + "title": "Kubernetes job timeout", + "description": "Maximum time for the step to run (number of seconds)" + }, + "kubernetes_memory_limit": { + "type": "string", + "title": "Kubernetes memory limit", + "description": "Kubernetes memory limit (e.g. 256Mi - read more about the expected memory values on the official Kubernetes documentation: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#meaning-of-memory)" + }, + "name": { + "type": "string", + "title": "Step name" + }, + "rucio": { + "type": "boolean", + "title": "Rucio integration", + "description": "Whether to use Rucio integration." + }, + "unpacked_img": { + "type": "boolean", + "title": "Unpacked container image", + "description": "Whether to use an unpacked container image. Useful for Singularity images stored on CVMFS" + }, + "voms_proxy": { + "type": "boolean", + "title": "VOMS proxy", + "description": "Whether to use a VOMS proxy for the step. This would require you to upload a valid VOMS proxy as a REANA secret." + } + }, + "required": [ + "commands", + "environment" + ] + } + } + } + } + } + } + } + }, + "else": { + "properties": { + "workflow": { + "properties": { + "file": { + "type": "string" + } + } + } + } } } \ No newline at end of file diff --git a/reana_commons/validation/utils.py b/reana_commons/validation/utils.py index 6038e635..dec9c0da 100644 --- a/reana_commons/validation/utils.py +++ b/reana_commons/validation/utils.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # # This file is part of REANA. -# Copyright (C) 2022 CERN. +# Copyright (C) 2022, 2023 CERN. # # REANA is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. @@ -12,10 +12,11 @@ import logging import os import re +from collections import deque from typing import Dict, List from jsonschema import ValidationError -from jsonschema.exceptions import best_match +from jsonschema.exceptions import best_match, ErrorTree from jsonschema.validators import validator_for from reana_commons.config import ( @@ -46,8 +47,12 @@ def _get_schema_validation_warnings(errors: List[ValidationError]) -> Dict: # or describe the error warnings = {} for e in errors: + # Get the path of the error (where in reana.yaml it occurred). + # The `path` property of a ValidationError is only relative to its `parent`. + error_path = e.absolute_path + error_path = ".".join(map(str, error_path)) if e.validator in non_critical_validators: - warning_value = [e.message] + warning_value = [{"message": e.message, "path": error_path}] if e.validator == "additionalProperties": # If the error is about additional properties, we want to return the # name(s) of the additional properties in a list. @@ -58,8 +63,14 @@ def _get_schema_validation_warnings(errors: List[ValidationError]) -> Dict: # "Additional properties are not allowed ('' was unexpected)" # "Additional properties are not allowed ('', '' were unexpected)" content_inside_parentheses = re.search(r"\((.*?)\)", e.message).group(1) - warning_value = re.findall(r"'(.*?)'", content_inside_parentheses or "") - warning_key = validator_to_warning.get(e.validator, e.validator) + additional_properties = re.findall( + r"'(.*?)'", content_inside_parentheses or "" + ) + warning_value = [ + {"property": additional_property, "path": error_path} + for additional_property in additional_properties + ] + warning_key = validator_to_warning.get(str(e.validator), str(e.validator)) warnings.setdefault(warning_key, []).extend(warning_value) else: critical_errors.append(e) @@ -77,6 +88,8 @@ def validate_reana_yaml(reana_yaml: Dict) -> Dict: """Validate REANA specification file according to jsonschema. :param reana_yaml: Dictionary which represents REANA specification file. + :returns: Dictionary of non-critical warnings, in the form of + {warning_key: [warning_value1, warning_value2, ...]}. :raises ValidationError: Given REANA spec file does not validate against REANA specification schema. """ diff --git a/tests/test_validation.py b/tests/test_validation.py index 88eb60a3..29fc96f1 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -7,6 +7,7 @@ # under the terms of the MIT License; see LICENSE file for more details. """REANA-Commons validation testing.""" +import operator import pytest from jsonschema.exceptions import ValidationError @@ -35,10 +36,18 @@ def test_validation_retention_days(yadage_workflow_spec_loaded, retention_days): @pytest.mark.parametrize( "extra_keys,expected_warnings", [ - (["wrong_key"], {"additional_properties": ["wrong_key"]}), + ( + ["wrong_key"], + {"additional_properties": [{"property": "wrong_key", "path": ""}]}, + ), ( ["wrong_key", "wrong_key2"], - {"additional_properties": ["wrong_key", "wrong_key2"]}, + { + "additional_properties": [ + {"property": "wrong_key", "path": ""}, + {"property": "wrong_key2", "path": ""}, + ] + }, ), ([], {}), ], @@ -57,7 +66,12 @@ def test_warnings_reana_yaml( 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]) + if isinstance(value, list): + assert len(value) == len(warnings[key]) + for warning_value in value: + assert warning_value in warnings[key] + else: + assert value == warnings[key] def test_critical_errors_reana_yaml(yadage_workflow_spec_loaded):