From b2d3ad6bf599309f94b73e88a091a7a0e93c797b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=C2=AA=20Fern=C3=A1ndez?= Date: Tue, 4 Jun 2024 20:28:02 +0200 Subject: [PATCH 1/3] Added validation checks to WfExS bootstrapping process. --- wfexs_backend/wfexs_backend.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/wfexs_backend/wfexs_backend.py b/wfexs_backend/wfexs_backend.py index 34d12f3c..1632c866 100644 --- a/wfexs_backend/wfexs_backend.py +++ b/wfexs_backend/wfexs_backend.py @@ -292,6 +292,11 @@ def bootstrap( updated = False + valErrors = config_validate(local_config_ro, cls.CONFIG_SCHEMA) + if len(valErrors) > 0: + logging.error(f"ERROR on incoming local configuration block for bootstrap: {valErrors}") + sys.exit(1) + local_config = cast("WritableWfExSConfigBlock", copy.deepcopy(local_config_ro)) # Getting the config directory @@ -395,6 +400,13 @@ def bootstrap( "Python interpreter does not support scrypt, so encoded crypt4gh keys with that algorithm cannot be used" ) + # Validate, again, as it changed + if updated: + valErrors = config_validate(local_config, cls.CONFIG_SCHEMA) + if len(valErrors) > 0: + logging.error(f"ERROR in bootstrapped updated local configuration block: {valErrors}") + sys.exit(1) + return updated, local_config @classmethod From 3ca4abfba1e83a4c046eeb6d584faa08c820fd2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=C2=AA=20Fern=C3=A1ndez?= Date: Tue, 4 Jun 2024 20:28:27 +0200 Subject: [PATCH 2/3] Fixed bootstrapping issue when `--cacheDir` flag is provided. --- wfexs_backend/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wfexs_backend/__main__.py b/wfexs_backend/__main__.py index b46e4fe0..7bfc69ab 100644 --- a/wfexs_backend/__main__.py +++ b/wfexs_backend/__main__.py @@ -1255,7 +1255,7 @@ def main() -> None: ) if args.cacheDir: - local_config["cache-directory"] = args.cacheDir + local_config["cacheDir"] = args.cacheDir # In any case, assuring the cache directory does exist cacheDir = local_config.get("cacheDir") From dca9ea58ff472592337e532eed27e1f8b61ae554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=C2=AA=20Fern=C3=A1ndez?= Date: Tue, 4 Jun 2024 21:05:16 +0200 Subject: [PATCH 3/3] Fixed (lack of) bootstrapping in corner case. --- tests/pushers/test_cache.py | 4 ++-- wfexs_backend/__main__.py | 15 +++++++++------ wfexs_backend/wfexs_backend.py | 25 +++++++++++++++++-------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/tests/pushers/test_cache.py b/tests/pushers/test_cache.py index 118cc243..4989be82 100644 --- a/tests/pushers/test_cache.py +++ b/tests/pushers/test_cache.py @@ -119,7 +119,7 @@ def test_cache_push(tmpdir) -> "None": # type: ignore[no-untyped-def] # First, instantiate WfExS backend temppath = tmpdir.mkdir("TEMP") - bootstrap_ok, test_local_config = WfExSBackend.bootstrap( + bootstrap_ok, test_local_config, config_directory = WfExSBackend.bootstrap( local_config_ro={ "cacheDir": "CACHE", "workDir": "WORKDIRS", @@ -129,7 +129,7 @@ def test_cache_push(tmpdir) -> "None": # type: ignore[no-untyped-def] ) wfexs = WfExSBackend( local_config=test_local_config, - config_directory=tmpdir.strpath, + config_directory=config_directory, ) # Second, instantiate the cache plugin diff --git a/wfexs_backend/__main__.py b/wfexs_backend/__main__.py index 7bfc69ab..239596ee 100644 --- a/wfexs_backend/__main__.py +++ b/wfexs_backend/__main__.py @@ -1273,11 +1273,12 @@ def main() -> None: # A filename is needed later, in order to initialize installation keys if not localConfigFilename: - localConfigFilename = defaultLocalConfigFilename - - # Hints for the the default path for the Crypt4GH keys - config_directory = os.path.dirname(localConfigFilename) - config_relname = os.path.basename(localConfigFilename) + config_directory = None + config_relname = os.path.basename(defaultLocalConfigFilename) + else: + # Hints for the the default path for the Crypt4GH keys + config_directory = os.path.dirname(localConfigFilename) + config_relname = os.path.basename(localConfigFilename) # Initialize (and create config file) if command in ( @@ -1290,9 +1291,11 @@ def main() -> None: WfExS_Commands.Import, WfExS_Commands.Execute, ): - updated_config, local_config = WfExSBackend.bootstrap( + updated_config, local_config, config_directory = WfExSBackend.bootstrap( local_config, config_directory, key_prefix=config_relname ) + # This is needed because config directory could have been empty + localConfigFilename = os.path.join(config_directory, config_relname) # Last, should config be saved back? if updated_config or not os.path.exists(localConfigFilename): diff --git a/wfexs_backend/wfexs_backend.py b/wfexs_backend/wfexs_backend.py index 1632c866..bd32b259 100644 --- a/wfexs_backend/wfexs_backend.py +++ b/wfexs_backend/wfexs_backend.py @@ -278,7 +278,7 @@ def bootstrap( local_config_ro: "WfExSConfigBlock", config_directory: "Optional[AnyPath]" = None, key_prefix: "Optional[str]" = None, - ) -> "Tuple[bool, WfExSConfigBlock]": + ) -> "Tuple[bool, WfExSConfigBlock, AnyPath]": """ :param local_config: Relevant local configuration, like the cache directory. :param config_directory: The filename to be used to resolve relative paths @@ -294,14 +294,18 @@ def bootstrap( valErrors = config_validate(local_config_ro, cls.CONFIG_SCHEMA) if len(valErrors) > 0: - logging.error(f"ERROR on incoming local configuration block for bootstrap: {valErrors}") + logging.error( + f"ERROR on incoming local configuration block for bootstrap: {valErrors}" + ) sys.exit(1) local_config = cast("WritableWfExSConfigBlock", copy.deepcopy(local_config_ro)) # Getting the config directory if config_directory is None: - config_directory = cast("AbsPath", os.getcwd()) + config_directory = cast( + "AbsPath", tempfile.mkdtemp(prefix="WfExS", suffix="config") + ) if not os.path.isabs(config_directory): config_directory = cast("AbsPath", os.path.abspath(config_directory)) @@ -404,10 +408,12 @@ def bootstrap( if updated: valErrors = config_validate(local_config, cls.CONFIG_SCHEMA) if len(valErrors) > 0: - logging.error(f"ERROR in bootstrapped updated local configuration block: {valErrors}") + logging.error( + f"ERROR in bootstrapped updated local configuration block: {valErrors}" + ) sys.exit(1) - return updated, local_config + return updated, local_config, config_directory @classmethod def FromDescription( @@ -437,7 +443,7 @@ def FromDescription( stacklevel=2, ) - _, updated_local_config = cls.bootstrap( + _, updated_local_config, config_directory = cls.bootstrap( local_config, config_directory=config_directory ) @@ -476,7 +482,8 @@ def __init__( ) if not isinstance(local_config, dict): - local_config = {} + # Minimal bootstrapping for embedded cases + _, local_config, config_directory = self.bootstrap({}, config_directory) # validate the local configuration object valErrors = config_validate(local_config, self.CONFIG_SCHEMA) @@ -1666,7 +1673,9 @@ def listStagedWorkflows( creation, orcids, # TODO: give some use to this instanceRawWorkdir, - ) = self.parseOrCreateRawWorkDir(entry.path, create_ok=False) + ) = self.parseOrCreateRawWorkDir( + cast("AbsPath", entry.path), create_ok=False + ) except: self.logger.warning(f"Skipped {entry.name} on listing") continue