Skip to content

Commit

Permalink
Merge pull request #288 from crim-ca/block-unknown-processes
Browse files Browse the repository at this point in the history
Block unknown process type deployment (fixes #276)
  • Loading branch information
fmigneault authored Sep 1, 2021
2 parents 0e0449a + a586cb7 commit a80f343
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 26 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ Changes:
<https://github.com/opengeospatial/ogcapi-processes/blob/master/core/openapi/schemas/format.yaml>`_) such that only
URL formatted strings are allowed, or alternatively and explicit JSON definition. Previous definitions that would
indicate an empty string schema are dropped since ``schema`` is optional.
- Block unknown and ``builtin`` process types during deployment from the API
(fixes `#276 <https://github.com/crim-ca/weaver/issues/276>`_).
Type ``builtin`` can only be registered by `Weaver` itself at startup. Other unknown types that have
no indication for mapping to an appropriate `Process` implementation are preemptively validated.

Fixes:
------
Expand Down
77 changes: 77 additions & 0 deletions tests/functional/test_wps_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
get_cwl_file_format
)
from weaver.processes.constants import CWL_REQUIREMENT_APP_DOCKER, CWL_REQUIREMENT_INIT_WORKDIR
from weaver.processes.types import PROCESS_BUILTIN
from weaver.utils import get_any_value

if TYPE_CHECKING:
Expand Down Expand Up @@ -508,6 +509,82 @@ def test_mediatype_io_format_references(self):
assert desc["outputs"]["wps_format_mimeType"]["formats"][0]["mediaType"] == CONTENT_TYPE_APP_JSON
assert desc["outputs"]["wps_format_mediaType"]["formats"][0]["mediaType"] == CONTENT_TYPE_APP_JSON

def test_block_builtin_processes_from_api(self):
"""
Test to validates if ``builtin`` process type is explicitly blocked during deploymemt from API.
"""
cwl = {
"cwlVersion": "v1.0",
"class": "CommandLineTool",
"baseCommand": ["python3"],
"inputs": {
"stringInput": "string"
},
"requirements": {
CWL_REQUIREMENT_APP_DOCKER: {
"dockerPull": "python:3.7-alpine"
},
},
"outputs": [],
}
body = {
"processDescription": {
"process": {
"id": self._testMethodName,
"title": "some title",
"abstract": "this is a test",
"type": PROCESS_BUILTIN,
},
},
"deploymentProfileName": "http://www.opengis.net/profiles/eoc/wpsApplication",
"executionUnit": [{"unit": cwl}],
}
with contextlib.ExitStack() as stack_exec:
for mock_exec in mocked_execute_process():
stack_exec.enter_context(mock_exec)
resp = mocked_sub_requests(self.app, "post_json", "/processes", data=body, timeout=5,
headers=self.json_headers, only_local=True, expect_errors=True)
assert resp.status_code == 400

def test_block_unknown_processes(self):
"""
Test to validates that any process that cannot be resolved against one of
known :py:data:`weaver.processes.constants.CWL_REQUIREMENT_APP_TYPES` is explicitly blocked.
"""
cwl = {
"cwlVersion": "v1.0",
"class": "CommandLineTool",
"baseCommand": ["python3"],
"inputs": {
"stringInput": "string"
},
"requirements": {
CWL_REQUIREMENT_APP_DOCKER: {"dockerPull": "python:3.7-alpine"},
"InlineJavascriptRequirement": {},
"ResourceRequirement": {"ramMin": 10240, "coresMin": 3}

},
"outputs": [],
}
body = {
"processDescription": {
"process": {
"id": self._testMethodName,
"title": "some title",
"abstract": "this is a test",
},
},
"deploymentProfileName": "http://www.opengis.net/profiles/eoc/wpsApplication",
"executionUnit": [{"unit": cwl}],
}

with contextlib.ExitStack() as stack_exec:
for mock_exec in mocked_execute_process():
stack_exec.enter_context(mock_exec)
resp = mocked_sub_requests(self.app, "post_json", "/processes", data=body, timeout=5,
headers=self.json_headers, only_local=True, expect_errors=True)
assert resp.status_code == 422

def test_complex_io_with_multiple_formats_and_defaults(self):
"""
Test validates that different format types are set on different input variations simultaneously:
Expand Down
7 changes: 6 additions & 1 deletion weaver/processes/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
ServiceNotFound,
log_unhandled_exceptions
)
from weaver.processes.types import PROCESS_APPLICATION, PROCESS_WORKFLOW
from weaver.processes.types import PROCESS_APPLICATION, PROCESS_BUILTIN, PROCESS_WORKFLOW
from weaver.store.base import StoreProcesses, StoreServices
from weaver.utils import get_sane_name, get_settings, get_url_without_query
from weaver.visibility import VISIBILITY_PRIVATE, VISIBILITY_PUBLIC
Expand Down Expand Up @@ -196,6 +196,11 @@ def deploy_process_from_payload(payload, container, overwrite=False):
else:
raise HTTPBadRequest("Missing one of required parameters [href, owsContext, deploymentProfileName].")

if process_info.get("type", "") == PROCESS_BUILTIN:
raise HTTPBadRequest(
"Invalid process type resolved from package: [{0}]. Deployment of {0} process is not allowed."
.format(PROCESS_BUILTIN))

# obtain updated process information using WPS process offering, CWL/WPS reference or CWL package definition
process_info = _get_deploy_process_info(process_info, reference, package)

Expand Down
59 changes: 34 additions & 25 deletions weaver/processes/wps_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
CWL_REQUIREMENT_APP_ESGF_CWT,
CWL_REQUIREMENT_APP_TYPES,
CWL_REQUIREMENT_APP_WPS1,
CWL_REQUIREMENT_INIT_WORKDIR,
WPS_INPUT,
WPS_OUTPUT
)
Expand Down Expand Up @@ -613,6 +614,35 @@ def _generate_process_with_cwl_from_reference(reference):
return cwl_package, process_info


def get_application_requirement(package):
# type: (CWL) -> Dict[str, Any]
"""
Retrieve the principal requirement that allows mapping to the appropriate process implementation.
Obtains the first item in `CWL` package ``requirements`` or ``hints`` that corresponds to a `Weaver`-specific
application type as defined in :py:data:`CWL_REQUIREMENT_APP_TYPES`.
:returns: dictionary that minimally has ``class`` field, and optionally other parameters from that requirement.
"""
# package can define requirements and/or hints,
# if it's an application, only one CWL_REQUIREMENT_APP_TYPES is allowed,
# workflow can have multiple, but they are not explicitly handled
reqs = package.get("requirements", {})
hints = package.get("hints", {})
all_hints = _get_package_requirements_as_class_list(reqs) + _get_package_requirements_as_class_list(hints)
app_hints = list(filter(lambda h: any(h["class"].endswith(t) for t in CWL_REQUIREMENT_APP_TYPES), all_hints))
if len(app_hints) > 1:
raise ValueError("Package 'requirements' and/or 'hints' define too many conflicting values: {}, "
"only one permitted amongst {}.".format(list(app_hints), list(CWL_REQUIREMENT_APP_TYPES)))
requirement = app_hints[0] if app_hints else {"class": ""}

cwl_supported_reqs = [item for item in CWL_REQUIREMENT_APP_TYPES] + [CWL_REQUIREMENT_INIT_WORKDIR]
if not all(item.get("class") in cwl_supported_reqs for item in all_hints):
raise PackageTypeError("Invalid requirement, the requirements supported are {0}".format(cwl_supported_reqs))

return requirement


def get_process_definition(process_offering, reference=None, package=None, data_source=None):
# type: (JSON, Optional[str], Optional[CWL], Optional[str]) -> JSON
"""
Expand Down Expand Up @@ -675,6 +705,8 @@ def try_or_raise_package_error(call, reason):
lambda: _merge_package_inputs_outputs(process_inputs, package_inputs, process_outputs, package_outputs),
reason="Merging of inputs/outputs")

try_or_raise_package_error(lambda: get_application_requirement(package), reason="Validate requirements and hints")

# obtain any retrieved process id if not already provided from upstream process offering, and clean it
process_id = get_sane_name(get_any_id(process_info), assert_invalid=False)
if not process_id:
Expand Down Expand Up @@ -1143,7 +1175,7 @@ def must_fetch(self, input_ref):
"""
if self.remote_execution or self.package_type == PROCESS_WORKFLOW:
return False
app_req = self.get_application_requirement()
app_req = get_application_requirement(self.package)
if app_req["class"] not in [CWL_REQUIREMENT_APP_BUILTIN, CWL_REQUIREMENT_APP_DOCKER]:
if input_ref.startswith("s3://"):
return True
Expand Down Expand Up @@ -1313,29 +1345,6 @@ def make_tool(self, toolpath_object, loading_context):
from weaver.processes.wps_workflow import default_make_tool
return default_make_tool(toolpath_object, loading_context, self.get_job_process_definition)

def get_application_requirement(self):
# type: () -> Dict[str, Any]
"""
Retrieve the principal requirement that allows mapping to the appropriate process implementation.
Obtains the first item in `CWL` package ``requirements`` or ``hints`` that corresponds to a `Weaver`-specific
application type as defined in :py:data:`CWL_REQUIREMENT_APP_TYPES`.
:returns: dictionary that minimally has ``class`` field, and optionally other parameters from that requirement.
"""
# package can define requirements and/or hints,
# if it's an application, only one CWL_REQUIREMENT_APP_TYPES is allowed,
# workflow can have multiple, but they are not explicitly handled
reqs = self.package.get("requirements", {})
hints = self.package.get("hints", {})
all_hints = _get_package_requirements_as_class_list(reqs) + _get_package_requirements_as_class_list(hints)
app_hints = list(filter(lambda h: any(h["class"].endswith(t) for t in CWL_REQUIREMENT_APP_TYPES), all_hints))
if len(app_hints) > 1:
raise ValueError("Package 'requirements' and/or 'hints' define too many conflicting values: {}, "
"only one permitted amongst {}.".format(list(app_hints), list(CWL_REQUIREMENT_APP_TYPES)))
requirement = app_hints[0] if app_hints else {"class": ""}
return requirement

def get_job_process_definition(self, jobname, joborder, tool): # noqa: E811
# type: (str, JSON, CWL) -> WpsPackage
"""
Expand Down Expand Up @@ -1388,7 +1397,7 @@ def _get_wps1_params(_requirement):
_wps_params[_param] = _requirement[_param]
return _wps_params

requirement = self.get_application_requirement()
requirement = get_application_requirement(self.package)
req_class = requirement["class"]

if req_class.endswith(CWL_REQUIREMENT_APP_WPS1):
Expand Down

0 comments on commit a80f343

Please sign in to comment.