Skip to content

Commit

Permalink
Check run name as well as workflow name in Cylc install
Browse files Browse the repository at this point in the history
  • Loading branch information
wxtim committed Jul 25, 2024
1 parent f68e8b9 commit d92d7c6
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 7 deletions.
1 change: 1 addition & 0 deletions changes.d/6264.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where failed to prevent invalid run names.
7 changes: 6 additions & 1 deletion cylc/flow/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,18 @@ def install_workflow(
source = Path(expand_path(source)).resolve()
if not workflow_name:
workflow_name = get_source_workflow_name(source)
validate_workflow_name(workflow_name, check_reserved_names=True)
if run_name is not None:
validate_workflow_name(
workflow_name + '/' + run_name,
being_validated='workflow and run'
)
if len(Path(run_name).parts) != 1:
raise WorkflowFilesError(
f'Run name cannot be a path. (You used {run_name})'
)
check_reserved_dir_names(run_name)
else:
validate_workflow_name(workflow_name, check_reserved_names=True)
validate_source_dir(source, workflow_name)
run_path_base = Path(get_workflow_run_dir(workflow_name))
relink, run_num, rundir = get_run_dir_info(
Expand Down
16 changes: 10 additions & 6 deletions cylc/flow/workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from typing import (
Callable,
Dict,
Literal,
Optional,
Tuple,
Union,
Expand Down Expand Up @@ -831,9 +832,11 @@ def check_deprecation(path, warn=True, force_compat_mode=False):


def validate_workflow_name(
name: str, check_reserved_names: bool = False
name: str,
check_reserved_names: bool = False,
being_validated: Literal['workflow', 'workflow and run'] = 'workflow'
) -> None:
"""Check workflow name/ID is valid and not an absolute path.
"""Check workflow/run name/ID is valid and not an absolute path.
Args:
name: Workflow name or ID.
Expand All @@ -845,17 +848,18 @@ def validate_workflow_name(
is_valid, message = WorkflowNameValidator.validate(name)
if not is_valid:
raise WorkflowFilesError(
f"invalid workflow name '{name}' - {message}"
f"invalid {being_validated} name '{name}' - {message}"
)
if os.path.isabs(name):
raise WorkflowFilesError(
f"workflow name cannot be an absolute path: {name}"
f"{being_validated.capitalize()}"
f' name cannot be an absolute path: {name}'
)
name = os.path.normpath(name)
if name.startswith(os.curdir):
raise WorkflowFilesError(
"Workflow name cannot be a path that points to the cylc-run "
"directory or above"
f"{being_validated.capitalize()} name cannot be a"
" path that points to the cylc-run directory or above"
)
if check_reserved_names:
check_reserved_dir_names(name)
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,3 +542,19 @@ def test_validate_source_dir(tmp_run_dir: Callable, tmp_src_dir: Callable):
with pytest.raises(WorkflowFilesError) as exc_info:
validate_source_dir(src_dir, 'ajay')
assert "exists in source directory" in str(exc_info.value)


def test_install_workflow_failif_bad_name(tmp_src_dir):
"""If a run_name is given validate_workflow_name is called on
the workflow and the run name in combination.
"""
src_dir: Path = tmp_src_dir('ludlow')
# It only has a workflow name:
with pytest.raises(WorkflowFilesError, match='can only contain'):
install_workflow(src_dir, workflow_name='foo?')
# It only has a run name:
with pytest.raises(WorkflowFilesError, match='can only contain'):
install_workflow(src_dir, run_name='foo?')
# It has a legal workflow name, but an invalid run name:
with pytest.raises(WorkflowFilesError, match='can only contain'):
install_workflow(src_dir, workflow_name='foo', run_name='bar?')

0 comments on commit d92d7c6

Please sign in to comment.