Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow spaces and other troublesome characters in environment names #40

Merged
merged 4 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions softpack_core/artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@

ready = 'ready'
queued = 'queued'
failed = 'failed'


class Artifacts:
"""Artifacts repo access class."""

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"
Expand Down Expand Up @@ -131,6 +133,8 @@

if Artifacts.module_file in self.obj:
info["state"] = State.ready
elif Artifacts.builder_out in self.obj:
info["state"] = State.failed

Check warning on line 137 in softpack_core/artifacts.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/artifacts.py#L137

Added line #L137 was not covered by tests
else:
info["state"] = State.queued

Expand Down
22 changes: 16 additions & 6 deletions softpack_core/schemas/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""

import io
import re
from dataclasses import dataclass
from pathlib import Path
from traceback import format_exception_only
Expand Down Expand Up @@ -146,15 +147,24 @@
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 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.
"""
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 not re.fullmatch(r"^[a-zA-Z0-9_-]+$", self.name):
return InvalidInputError(

Check warning on line 163 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L163

Added line #L163 was not covered by tests
message="name must only contain alphanumerics, "
"dash, and underscore"
)

return None

@classmethod
Expand Down Expand Up @@ -239,9 +249,9 @@
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()

Check warning on line 252 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L252

Added line #L252 was not covered by tests
if input_err is not None:
return input_err

Check warning on line 254 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L254

Added line #L254 was not covered by tests

name = env.name
version = 1
Expand All @@ -262,9 +272,9 @@
env.name = name

# Send build request
try:
host = app.settings.builder.host
port = app.settings.builder.port

Check warning on line 277 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L275-L277

Added lines #L275 - L277 were not covered by tests
r = httpx.post(
f"http://{host}:{port}/environments/build",
json={
Expand All @@ -282,9 +292,9 @@
},
},
)
r.raise_for_status()
except Exception as e:
return BuilderError(

Check warning on line 297 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L295-L297

Added lines #L295 - L297 were not covered by tests
message="Connection to builder failed: "
+ "".join(format_exception_only(type(e), e))
)
Expand Down Expand Up @@ -340,9 +350,9 @@
for pkg in env.packages
],
)
ymlData = yaml.dump(softpack_definition)

Check warning on line 353 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L353

Added line #L353 was not covered by tests

tree_oid = cls.artifacts.create_files(

Check warning on line 355 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L355

Added line #L355 was not covered by tests
new_folder_path,
[
(env_type, ""), # e.g. .built_by_softpack
Expand All @@ -354,7 +364,7 @@
tree_oid, "create environment folder"
)
except RuntimeError as e:
return InvalidInputError(

Check warning on line 367 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L367

Added line #L367 was not covered by tests
message="".join(format_exception_only(type(e), e))
)

Expand Down Expand Up @@ -574,7 +584,7 @@
)

except Exception as e:
return InvalidInputError(

Check warning on line 587 in softpack_core/schemas/environment.py

View check run for this annotation

Codecov / codecov/patch

softpack_core/schemas/environment.py#L587

Added line #L587 was not covered by tests
message="".join(format_exception_only(type(e), e))
)

Expand Down
41 changes: 39 additions & 2 deletions tests/integration/test_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,34 @@ 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_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)
assert isinstance(result, InvalidInputError)
Expand Down Expand Up @@ -225,6 +247,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")
)
Expand Down
Loading