From e664e99055cd556d94770154b9586b7205d38116 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 29 Jan 2024 12:13:54 +0000 Subject: [PATCH 1/4] Disallow spaces in environment names --- softpack_core/schemas/environment.py | 18 ++++++++++++------ tests/integration/test_environment.py | 12 ++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index d7f76b7..401ecbd 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -146,15 +146,21 @@ class EnvironmentInput: description: str packages: list[PackageInput] - def validate(cls) -> Union[None, InvalidInputError]: - """Validate that all values have been supplied. + def validate(self) -> Union[None, InvalidInputError]: + """Validate all values. + + Checks that name has no spaces. + Checks all values have been supplied. Returns: None if good, or InvalidInputError if not all values supplied. """ - if any(len(value) == 0 for value in vars(cls).values()): + if any(len(value) == 0 for value in vars(self).values()): return InvalidInputError(message="all fields must be filled in") + if " " in self.name: + return InvalidInputError(message="name must not contain spaces") + return None @classmethod @@ -239,9 +245,9 @@ def create(cls, env: EnvironmentInput) -> CreateResponse: # type: ignore Returns: A message confirming the success or failure of the operation. """ - result = env.validate() - if result is not None: - return result + input_err = env.validate() + if input_err is not None: + return input_err name = env.name version = 1 diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 15a0f23..9514600 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -93,12 +93,20 @@ def test_create(httpx_post, testable_env_input: EnvironmentInput) -> None: ) assert file_in_remote(path) - orig_name = testable_env_input.name + +def test_create_name_empty_disallowed(httpx_post, testable_env_input): testable_env_input.name = "" result = Environment.create(testable_env_input) assert isinstance(result, InvalidInputError) - testable_env_input.name = orig_name + +def test_create_name_spaces_disallowed(httpx_post, testable_env_input): + testable_env_input.name = "names cannot have spaces" + result = Environment.create(testable_env_input) + assert isinstance(result, InvalidInputError) + + +def test_create_path_invalid_disallowed(httpx_post, testable_env_input): testable_env_input.path = "invalid/path" result = Environment.create(testable_env_input) assert isinstance(result, InvalidInputError) From 86020654f32a495b439cad4b0f9e8faefa8ed0f7 Mon Sep 17 00:00:00 2001 From: ash Date: Tue, 30 Jan 2024 09:41:27 +0000 Subject: [PATCH 2/4] Mark failed environments as failed --- softpack_core/artifacts.py | 4 ++++ tests/integration/test_environment.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/softpack_core/artifacts.py b/softpack_core/artifacts.py index 3cf4426..cfd9029 100644 --- a/softpack_core/artifacts.py +++ b/softpack_core/artifacts.py @@ -53,6 +53,7 @@ class State(Enum): ready = 'ready' queued = 'queued' + failed = 'failed' class Artifacts: @@ -60,6 +61,7 @@ class Artifacts: environments_root = "environments" environments_file = "softpack.yml" + builder_out = "builder.out" module_file = "module" readme_file = "README.md" built_by_softpack_file = ".built_by_softpack" @@ -131,6 +133,8 @@ def spec(self) -> Box: if Artifacts.module_file in self.obj: info["state"] = State.ready + elif Artifacts.builder_out in self.obj: + info["state"] = State.failed else: info["state"] = State.queued diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 9514600..67ff7df 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -233,6 +233,21 @@ async def test_states(httpx_post, testable_env_input): assert env.type == Artifacts.built_by_softpack assert env.state == State.queued + upload = UploadFile( + filename=Artifacts.builder_out, file=io.BytesIO(b"some output") + ) + + result = await Environment.write_artifact( + file=upload, + folder_path=f"{testable_env_input.path}/{testable_env_input.name}-1", + file_name=upload.filename, + ) + assert isinstance(result, WriteArtifactSuccess) + + env = get_env_from_iter(testable_env_input.name + "-1") + assert env is not None + assert env.state == State.failed + upload = UploadFile( filename=Artifacts.module_file, file=io.BytesIO(b"#%Module") ) From ccdc191727fb193168accef43e192a4db3c32fc9 Mon Sep 17 00:00:00 2001 From: ash Date: Tue, 30 Jan 2024 09:58:03 +0000 Subject: [PATCH 3/4] Disallow troublesome characters in environment names --- softpack_core/schemas/environment.py | 8 ++++++-- tests/integration/test_environment.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 401ecbd..5949f78 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -5,6 +5,7 @@ """ import io +import re from dataclasses import dataclass from pathlib import Path from traceback import format_exception_only @@ -158,8 +159,11 @@ def validate(self) -> Union[None, InvalidInputError]: if any(len(value) == 0 for value in vars(self).values()): return InvalidInputError(message="all fields must be filled in") - if " " in self.name: - return InvalidInputError(message="name must not contain spaces") + if not re.fullmatch(r"^[a-zA-Z0-9_-]+$", self.name): + return InvalidInputError( + message="name must only contain alphanumerics, " + "dash, and underscore" + ) return None diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py index 67ff7df..78b1f57 100644 --- a/tests/integration/test_environment.py +++ b/tests/integration/test_environment.py @@ -106,6 +106,20 @@ def test_create_name_spaces_disallowed(httpx_post, testable_env_input): assert isinstance(result, InvalidInputError) +def test_create_name_slashes_disallowed(httpx_post, testable_env_input): + testable_env_input.name = "names/cannot/have/slashes" + result = Environment.create(testable_env_input) + assert isinstance(result, InvalidInputError) + + +def test_create_name_dashes_and_number_first_allowed( + httpx_post, testable_env_input +): + testable_env_input.name = "7-zip_piz-7" + result = Environment.create(testable_env_input) + assert isinstance(result, CreateEnvironmentSuccess) + + def test_create_path_invalid_disallowed(httpx_post, testable_env_input): testable_env_input.path = "invalid/path" result = Environment.create(testable_env_input) From 626538685cdee3c7bbbdae946a9938ef1cfdcc7f Mon Sep 17 00:00:00 2001 From: ash Date: Wed, 31 Jan 2024 09:30:45 +0000 Subject: [PATCH 4/4] Update docs for EnvironmentInput.validate --- softpack_core/schemas/environment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py index 5949f78..9bf55bc 100644 --- a/softpack_core/schemas/environment.py +++ b/softpack_core/schemas/environment.py @@ -150,8 +150,8 @@ class EnvironmentInput: def validate(self) -> Union[None, InvalidInputError]: """Validate all values. - Checks that name has no spaces. Checks all values have been supplied. + Checks that name consists only of alphanumerics, dash, and underscore. Returns: None if good, or InvalidInputError if not all values supplied.