diff --git a/cwltool/checker.py b/cwltool/checker.py index d81642e1d..6e5822ce8 100644 --- a/cwltool/checker.py +++ b/cwltool/checker.py @@ -557,3 +557,16 @@ def loop_checker(steps: Iterator[MutableMapping[str, Any]]) -> None: ) if exceptions: raise ValidationException("\n".join(exceptions)) + + +def resreq_minmax_checker(requirement: CWLObjectType) -> None: + """ + Check if the minResource is lesser than the maxResource in resource requirements. + + :raises ValidationException: If the minResource is greater than the maxResource. + """ + for a in ("cores", "ram", "tmpdir", "outdir"): + mn = cast(Union[int, float, None], requirement.get(a + "Min")) + mx = cast(Union[int, float, None], requirement.get(a + "Max")) + if mn is not None and mx is not None and mx < mn: + raise ValidationException(f"{a}Min cannot be greater than {a}Max.") diff --git a/cwltool/command_line_tool.py b/cwltool/command_line_tool.py index a9d5e57e2..ce5f24cb2 100644 --- a/cwltool/command_line_tool.py +++ b/cwltool/command_line_tool.py @@ -39,6 +39,7 @@ content_limit_respected_read_bytes, substitute, ) +from .checker import resreq_minmax_checker from .context import LoadingContext, RuntimeContext, getdefault from .docker import DockerCommandLineJob, PodmanCommandLineJob from .errors import UnsupportedRequirement, WorkflowException @@ -410,6 +411,10 @@ def __init__(self, toolpath_object: CommentedMap, loadingContext: LoadingContext else PathCheckingMode.STRICT ) + resource_req = self.get_requirement("ResourceRequirement")[0] + if getdefault(loadingContext.do_validate, True) and resource_req: + resreq_minmax_checker(resource_req) + def make_job_runner(self, runtimeContext: RuntimeContext) -> type[JobBase]: """Return the correct CommandLineJob class given the container settings.""" dockerReq, dockerRequired = self.get_requirement("DockerRequirement") diff --git a/cwltool/process.py b/cwltool/process.py index 75069b859..cf35a149a 100644 --- a/cwltool/process.py +++ b/cwltool/process.py @@ -1027,9 +1027,11 @@ def evalResources( elif mx is None: mx = mn - if mn is not None: + if mn is not None and mx is not None: + if mx < mn: + raise ValidationException(f"{a}Min cannot be greater than {a}Max.") request[a + "Min"] = mn - request[a + "Max"] = cast(Union[int, float], mx) + request[a + "Max"] = mx request_evaluated = cast(dict[str, Union[int, float]], request) if runtimeContext.select_resources is not None: diff --git a/cwltool/workflow.py b/cwltool/workflow.py index fe34c75ce..e28b58319 100644 --- a/cwltool/workflow.py +++ b/cwltool/workflow.py @@ -13,7 +13,12 @@ from schema_salad.sourceline import SourceLine, indent from . import command_line_tool, context, procgenerator -from .checker import circular_dependency_checker, loop_checker, static_checker +from .checker import ( + circular_dependency_checker, + loop_checker, + resreq_minmax_checker, + static_checker, +) from .context import LoadingContext, RuntimeContext, getdefault from .cwlprov.provenance_profile import ProvenanceProfile from .cwlprov.writablebagfile import create_job @@ -133,6 +138,10 @@ def __init__( circular_dependency_checker(step_inputs) loop_checker(step.tool for step in self.steps) + resource_req = self.get_requirement("ResourceRequirement")[0] + if resource_req: + resreq_minmax_checker(resource_req) + def make_workflow_step( self, toolpath_object: CommentedMap, diff --git a/tests/test_examples.py b/tests/test_examples.py index f371f45ae..a0f1ad08a 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -1841,3 +1841,18 @@ def test_make_template() -> None: ] ) assert exit_code == 0, stderr + + +@pytest.mark.parametrize( + "file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"] +) +def test_invalid_resource_requirements(file: str) -> None: + """Ensure an error with an invalid resource requirement.""" + exit_code, stdout, stderr = get_main_output( + [ + "--disable-validate", + get_data(file), + ] + ) + assert exit_code == 1 + assert "cannot be greater than" in stderr diff --git a/tests/test_validate.py b/tests/test_validate.py index 8e664d9aa..6d9b20fba 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -4,6 +4,8 @@ import logging import re +import pytest + from .util import get_data, get_main_output @@ -125,3 +127,18 @@ def test_validate_custom_logger() -> None: assert "tests/CometAdapter.cwl#out' previously defined" not in stdout assert "tests/CometAdapter.cwl#out' previously defined" not in stderr assert "tests/CometAdapter.cwl#out' previously defined" in custom_log_text + + +@pytest.mark.parametrize( + "file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"] +) +def test_validate_with_invalid_requirements(file: str) -> None: + """Ensure that --validate returns an error with an invalid resource requirement.""" + exit_code, stdout, stderr = get_main_output( + [ + "--validate", + get_data(file), + ] + ) + assert exit_code == 1 + assert "cannot be greater than" in stdout diff --git a/tests/wf/bad_resreq_mnmx_clt.cwl b/tests/wf/bad_resreq_mnmx_clt.cwl new file mode 100644 index 000000000..50ca836e8 --- /dev/null +++ b/tests/wf/bad_resreq_mnmx_clt.cwl @@ -0,0 +1,14 @@ +#!/usr/bin/env cwl-runner +class: CommandLineTool +cwlVersion: v1.2 + +requirements: + ResourceRequirement: + coresMin: 4 + coresMax: 2 + +inputs: [] +outputs: [] + +baseCommand: ["echo", "Hello World"] + diff --git a/tests/wf/bad_resreq_mnmx_wf.cwl b/tests/wf/bad_resreq_mnmx_wf.cwl new file mode 100755 index 000000000..64b1e1c9a --- /dev/null +++ b/tests/wf/bad_resreq_mnmx_wf.cwl @@ -0,0 +1,25 @@ +#!/usr/bin/env cwl-runner +class: Workflow +cwlVersion: v1.2 + +requirements: + ResourceRequirement: + ramMin: 128 + ramMax: 64 + +inputs: [] +outputs: [] + +steps: + hello_world: + requirements: + ResourceRequirement: + ramMin: 64 + ramMax: 128 + run: + class: CommandLineTool + baseCommand: [ "echo", "Hello World" ] + inputs: [ ] + outputs: [ ] + out: [ ] + in: [ ]