From f2a773c651846dab2fab6dad28561facdff7f1c9 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 28 Nov 2024 17:05:09 +0100 Subject: [PATCH] Rename 'secrets' to 'credentials' in tool parsing and related methods and use the discussed XML format #19196 --- lib/galaxy/tool_util/cwl/parser.py | 4 +- lib/galaxy/tool_util/deps/requirements.py | 125 +++++++++++++++------- lib/galaxy/tool_util/linters/general.py | 8 +- lib/galaxy/tool_util/parser/cwl.py | 4 +- lib/galaxy/tool_util/parser/interface.py | 4 +- lib/galaxy/tool_util/parser/yaml.py | 2 +- lib/galaxy/tool_util/xsd/galaxy.xsd | 103 ++++++++++++++---- lib/galaxy/tools/__init__.py | 17 +-- lib/galaxy/tools/evaluation.py | 37 ++++--- test/unit/tool_util/test_cwl.py | 2 +- test/unit/tool_util/test_parsing.py | 4 +- 11 files changed, 212 insertions(+), 98 deletions(-) diff --git a/lib/galaxy/tool_util/cwl/parser.py b/lib/galaxy/tool_util/cwl/parser.py index d2deec42a874..cc0b9944483a 100644 --- a/lib/galaxy/tool_util/cwl/parser.py +++ b/lib/galaxy/tool_util/cwl/parser.py @@ -219,8 +219,8 @@ def software_requirements(self) -> List: def resource_requirements(self) -> List: return self.hints_or_requirements_of_class("ResourceRequirement") - def secrets(self) -> List: - return self.hints_or_requirements_of_class("Secrets") + def credentials(self) -> List: + return self.hints_or_requirements_of_class("Credentials") class CommandLineToolProxy(ToolProxy): diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index 7b74ec2575be..fba3ecb4830c 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -306,52 +306,99 @@ def resource_requirements_from_list(requirements: Iterable[Dict[str, Any]]) -> L return rr -class SecretsRequirement: +class SecretOrVariable: def __init__( self, type: str, - user_preferences_key: str, + name: str, inject_as_env: str, - label: Optional[str] = "", - required: Optional[bool] = False, + label: str = "", + description: str = "", ) -> None: self.type = type - self.user_preferences_key = user_preferences_key + self.name = name self.inject_as_env = inject_as_env self.label = label - self.required = required - if not self.user_preferences_key: - raise ValueError("Missing user_preferences_key") - seperated_key = user_preferences_key.split("/") - if len(seperated_key) != 2 or not seperated_key[0] or not seperated_key[1]: - raise ValueError("Invalid user_preferences_key") - if self.type not in {"vault"}: - raise ValueError(f"Invalid secret type '{self.type}'") + self.description = description + if self.type not in {"secret", "variable"}: + raise ValueError(f"Invalid credential type '{self.type}'") if not self.inject_as_env: raise ValueError("Missing inject_as_env") def to_dict(self) -> Dict[str, Any]: return { "type": self.type, - "user_preferences_key": self.user_preferences_key, + "name": self.name, "inject_as_env": self.inject_as_env, "label": self.label, - "required": self.required, + "description": self.description, } @classmethod - def from_dict(cls, dict: Dict[str, Any]) -> "SecretsRequirement": + def from_element(cls, elem) -> "SecretOrVariable": + return cls( + type=elem.tag, + name=elem.get("name"), + inject_as_env=elem.get("inject_as_env"), + label=elem.get("label", ""), + description=elem.get("description", ""), + ) + + @classmethod + def from_dict(cls, dict: Dict[str, Any]) -> "SecretOrVariable": type = dict["type"] - user_preferences_key = dict["user_preferences_key"] + name = dict["name"] inject_as_env = dict["inject_as_env"] label = dict.get("label", "") + description = dict.get("description", "") + return cls(type=type, name=name, inject_as_env=inject_as_env, label=label, description=description) + + +class CredentialsRequirement: + def __init__( + self, + name: str, + reference: str, + required: bool = False, + label: str = "", + description: str = "", + secrets_and_variables: Optional[List[SecretOrVariable]] = None, + ) -> None: + self.name = name + self.reference = reference + self.required = required + self.label = label + self.description = description + self.secrets_and_variables = secrets_and_variables if secrets_and_variables is not None else [] + + if not self.reference: + raise ValueError("Missing reference") + + def to_dict(self) -> Dict[str, Any]: + return { + "name": self.name, + "reference": self.reference, + "required": self.required, + "label": self.label, + "description": self.description, + "secrets_and_variables": [s.to_dict() for s in self.secrets_and_variables], + } + + @classmethod + def from_dict(cls, dict: Dict[str, Any]) -> "CredentialsRequirement": + name = dict["name"] + reference = dict["reference"] required = dict.get("required", False) + label = dict.get("label", "") + description = dict.get("description", "") + secrets_and_variables = [SecretOrVariable.from_dict(s) for s in dict.get("secrets_and_variables", [])] return cls( - type=type, - user_preferences_key=user_preferences_key, - inject_as_env=inject_as_env, - label=label, + name=name, + reference=reference, required=required, + label=label, + description=description, + secrets_and_variables=secrets_and_variables, ) @@ -359,13 +406,13 @@ def parse_requirements_from_lists( software_requirements: List[Union[ToolRequirement, Dict[str, Any]]], containers: Iterable[Dict[str, Any]], resource_requirements: Iterable[Dict[str, Any]], - secrets: Iterable[Dict[str, Any]], -) -> Tuple[ToolRequirements, List[ContainerDescription], List[ResourceRequirement], List[SecretsRequirement]]: + credentials: Iterable[Dict[str, Any]], +) -> Tuple[ToolRequirements, List[ContainerDescription], List[ResourceRequirement], List[CredentialsRequirement]]: return ( ToolRequirements.from_list(software_requirements), [ContainerDescription.from_dict(c) for c in containers], resource_requirements_from_list(resource_requirements), - [SecretsRequirement.from_dict(s) for s in secrets], + [CredentialsRequirement.from_dict(s) for s in credentials], ) @@ -413,9 +460,9 @@ def parse_requirements_from_xml(xml_root, parse_resources_and_secrets: bool = Fa if parse_resources_and_secrets: resource_elems = requirements_elem.findall("resource") if requirements_elem is not None else [] resources = [resource_from_element(r) for r in resource_elems] - secret_elems = requirements_elem.findall("secret") if requirements_elem is not None else [] - secrets = [secret_from_element(s) for s in secret_elems] - return requirements, containers, resources, secrets + credentials_elems = requirements_elem.findall("credentials") if requirements_elem is not None else [] + credentials = [credentials_from_element(s) for s in credentials_elems] + return requirements, containers, resources, credentials return requirements, containers @@ -440,16 +487,18 @@ def container_from_element(container_elem) -> ContainerDescription: return container -def secret_from_element(secret_elem) -> SecretsRequirement: - type = secret_elem.get("type") - user_preferences_key = secret_elem.get("user_preferences_key") - inject_as_env = secret_elem.get("inject_as_env") - label = secret_elem.get("label", "") - required = string_as_bool(secret_elem.get("required", "false")) - return SecretsRequirement( - type=type, - user_preferences_key=user_preferences_key, - inject_as_env=inject_as_env, - label=label, +def credentials_from_element(credentials_elem) -> CredentialsRequirement: + name = credentials_elem.get("name") + reference = credentials_elem.get("reference") + required = string_as_bool(credentials_elem.get("required", "false")) + label = credentials_elem.get("label", "") + description = credentials_elem.get("description", "") + secrets_and_variables = [SecretOrVariable.from_element(elem) for elem in credentials_elem.findall("*")] + return CredentialsRequirement( + name=name, + reference=reference, required=required, + label=label, + description=description, + secrets_and_variables=secrets_and_variables, ) diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index 2e13abd0ea53..ddc2c4e6621c 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -183,7 +183,7 @@ class RequirementNameMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, *_ = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -195,7 +195,7 @@ class RequirementVersionMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, *_ = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -207,7 +207,7 @@ class RequirementVersionWhitespace(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, *_ = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -223,7 +223,7 @@ class ResourceRequirementExpression(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + *_, resource_requirements, _ = tool_source.parse_requirements_and_containers() for rr in resource_requirements: if rr.runtime_required: lint_ctx.warn( diff --git a/lib/galaxy/tool_util/parser/cwl.py b/lib/galaxy/tool_util/parser/cwl.py index 171ab7ce817f..540f8edcfbdd 100644 --- a/lib/galaxy/tool_util/parser/cwl.py +++ b/lib/galaxy/tool_util/parser/cwl.py @@ -162,12 +162,12 @@ def parse_requirements_and_containers(self): software_requirements = self.tool_proxy.software_requirements() resource_requirements = self.tool_proxy.resource_requirements() - secrets = self.tool_proxy.secrets() + credentials = self.tool_proxy.credentials() return requirements.parse_requirements_from_lists( software_requirements=[{"name": r[0], "version": r[1], "type": "package"} for r in software_requirements], containers=containers, resource_requirements=resource_requirements, - secrets=secrets, + credentials=credentials, ) def parse_profile(self): diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index d7c26c82b359..2c59751f2ca3 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -34,8 +34,8 @@ if TYPE_CHECKING: from galaxy.tool_util.deps.requirements import ( ContainerDescription, + CredentialsRequirement, ResourceRequirement, - SecretsRequirement, ToolRequirements, ) @@ -309,7 +309,7 @@ def parse_required_files(self) -> Optional["RequiredFiles"]: def parse_requirements_and_containers( self, ) -> Tuple[ - "ToolRequirements", List["ContainerDescription"], List["ResourceRequirement"], List["SecretsRequirement"] + "ToolRequirements", List["ContainerDescription"], List["ResourceRequirement"], List["CredentialsRequirement"] ]: """Return triple of ToolRequirement, ContainerDescription, ResourceRequirement, and SecretsRequirement objects.""" diff --git a/lib/galaxy/tool_util/parser/yaml.py b/lib/galaxy/tool_util/parser/yaml.py index 4de318b44ddd..673e5a2cb418 100644 --- a/lib/galaxy/tool_util/parser/yaml.py +++ b/lib/galaxy/tool_util/parser/yaml.py @@ -115,7 +115,7 @@ def parse_requirements_and_containers(self): software_requirements=[r for r in mixed_requirements if r.get("type") != "resource"], containers=self.root_dict.get("containers", []), resource_requirements=[r for r in mixed_requirements if r.get("type") == "resource"], - secrets=self.root_dict.get("secrets", []), + credentials=self.root_dict.get("credentials", []), ) def parse_input_pages(self) -> PagesSource: diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index db5c621c17f2..33e18c59b975 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -612,7 +612,7 @@ serve as complete descriptions of the runtime of a tool. - + @@ -726,44 +726,109 @@ Read more about configuring Galaxy to run Docker jobs - + - + + + + + ``` ]]> - + + + + + + + The name of the credential set. + + + + + A reference to the source of the credentials. + + + + + The label of the credential set. + + + + + The description of the credential set. + + - The type of secret to inject. Valid value is ``vault`` for now. + Whether the credentials are required for the tool to run. - + + + + + + - The name of the user preference key to store the secret in. + The name of the variable. - The name of the environment variable to inject the secret into. + The environment variable name to inject the value as. - The label of the secret. + The label for the variable. - + - Whether the secret is required to run the tool. + The description for the variable. + + + + + + + + + + The name of the secret. + + + + + The environment variable name to inject the value as. + + + + + The label for the secret. + + + + + The description for the secret. @@ -7890,14 +7955,6 @@ and ``bibtex`` are the only supported options. - - - Type of secret for tool execution. - - - - - Documentation for ToolTypeType diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 83aa5293e2f7..286283e4e0b4 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1216,17 +1216,18 @@ def parse(self, tool_source: ToolSource, guid: Optional[str] = None, dynamic: bo raise Exception(message) # Requirements (dependencies) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, credentials = tool_source.parse_requirements_and_containers() self.requirements = requirements self.containers = containers self.resource_requirements = resource_requirements - self.secrets = secrets - for secret in self.secrets: - preferences = self.app.config.user_preferences_extra["preferences"] - main_key, input_key = secret.user_preferences_key.split("/") - preferences_input = preferences.get(main_key, {}).get("inputs", []) - if not any(input_item.get("name") == input_key for input_item in preferences_input): - raise exceptions.ConfigurationError(f"User preferences key {secret.user_preferences_key} not found") + self.credentials = credentials + # for credential in self.credentials: + # pass + # preferences = self.app.config.user_preferences_extra["preferences"] + # main_key, input_key = credential.user_preferences_key.split("/") + # preferences_input = preferences.get(main_key, {}).get("inputs", []) + # if not any(input_item.get("name") == input_key for input_item in preferences_input): + # raise exceptions.ConfigurationError(f"User preferences key {credential.user_preferences_key} not found") required_files = tool_source.parse_required_files() if required_files is None: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 96be3a6ca028..bbb72308cc1e 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -6,10 +6,9 @@ import string import tempfile from datetime import datetime -from typing import ( +from typing import ( # cast, Any, Callable, - cast, Dict, List, Optional, @@ -29,11 +28,11 @@ ) from galaxy.model.none_like import NoneDataset from galaxy.security.object_wrapper import wrap_with_safe_string -from galaxy.security.vault import UserVaultWrapper -from galaxy.structured_app import ( + +# from galaxy.security.vault import UserVaultWrapper +from galaxy.structured_app import ( # StructuredApp, BasicSharedApp, MinimalToolApp, - StructuredApp, ) from galaxy.tool_util.data import TabularToolDataTable from galaxy.tools.parameters import ( @@ -191,16 +190,24 @@ def set_compute_environment(self, compute_environment: ComputeEnvironment, get_s ) self.execute_tool_hooks(inp_data=inp_data, out_data=out_data, incoming=incoming) - if self.tool.secrets: - app = cast(StructuredApp, self.app) - user_vault = UserVaultWrapper(app.vault, self._user) - for secret in self.tool.secrets: - vault_key = secret.user_preferences_key - secret_value = user_vault.read_secret("preferences/" + vault_key) - if secret_value is not None: - self.environment_variables.append({"name": secret.inject_as_env, "value": secret_value}) - else: - log.warning("Failed to read secret from vault") + if self.tool.credentials: + # app = cast(StructuredApp, self.app) + # user_vault = UserVaultWrapper(app.vault, self._user) + for credentials in self.tool.credentials: + reference = credentials.reference + for secret_or_variable in credentials.secrets_and_variables: + if secret_or_variable.type == "variable": + # variable_value = self.param_dict.get(f"{reference}/{secret_or_variable.name}") + variable_value = f"A variable: {reference}/{secret_or_variable.name}" + self.environment_variables.append( + {"name": secret_or_variable.inject_as_env, "value": variable_value} + ) + elif secret_or_variable.type == "secret": + # secret_value = user_vault.read_secret(f"{reference}/{secret_or_variable.name}") + secret_value = f"A secret: {reference}/{secret_or_variable.name}" + self.environment_variables.append( + {"name": secret_or_variable.inject_as_env, "value": secret_value} + ) def execute_tool_hooks(self, inp_data, out_data, incoming): # Certain tools require tasks to be completed prior to job execution diff --git a/test/unit/tool_util/test_cwl.py b/test/unit/tool_util/test_cwl.py index 5977ec3565d0..8e95793cc8a9 100644 --- a/test/unit/tool_util/test_cwl.py +++ b/test/unit/tool_util/test_cwl.py @@ -281,7 +281,7 @@ def test_load_proxy_simple(): outputs, output_collections = tool_source.parse_outputs(None) assert len(outputs) == 1 - software_requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + software_requirements, containers, resource_requirements, _ = tool_source.parse_requirements_and_containers() assert software_requirements.to_dict() == [] assert len(containers) == 1 assert containers[0].to_dict() == { diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index ae7290179abf..6973606b7f75 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -347,7 +347,7 @@ def test_action(self): assert self._tool_source.parse_action_module() is None def test_requirements(self): - requirements, containers, resource_requirements, secrets = self._tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, _ = self._tool_source.parse_requirements_and_containers() assert requirements[0].type == "package" assert list(containers)[0].identifier == "mycool/bwa" assert resource_requirements[0].resource_type == "cores_min" @@ -533,7 +533,7 @@ def test_action(self): assert self._tool_source.parse_action_module() is None def test_requirements(self): - software_requirements, containers, resource_requirements, secrets = ( + software_requirements, containers, resource_requirements, _ = ( self._tool_source.parse_requirements_and_containers() ) assert software_requirements.to_dict() == [{"name": "bwa", "type": "package", "version": "1.0.1", "specs": []}]