Skip to content

Commit

Permalink
Disallow run n and run x as flow names (cylc#4526)
Browse files Browse the repository at this point in the history
* prevent cylc install being OK with runN and run\d+ folder names

* changelog

* response to review

* fix broken tests

* Apply suggestions from code review

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

* fix tests

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
  • Loading branch information
wxtim and MetRonnie authored Nov 29, 2021
1 parent a8127f9 commit 9652260
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ First Release Candidate for Cylc 8.

### Enhancements

[#4526](https://github.com/cylc/cylc-flow/pull/4526) - Prevent runN and run\d+
being allowed as installation target names.

[#4442](https://github.com/cylc/cylc-flow/pull/4442) - Prevent installation
of workflows inside other installed workflows.

Expand Down
12 changes: 10 additions & 2 deletions cylc/flow/workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,7 @@ def _parse_src_reg(reg: Path, cur_dir_only: bool = False) -> Tuple[Path, Path]:
return (reg, abs_path)


def validate_workflow_name(name: str) -> None:
def validate_workflow_name(name: str, runNcheck=False) -> None:
"""Check workflow name is valid and not an absolute path.
Raise WorkflowFilesError if not valid.
Expand All @@ -1333,6 +1333,14 @@ def validate_workflow_name(name: str) -> None:
"Workflow name cannot be a path that points to the cylc-run "
"directory or above"
)
if runNcheck and any(
re.match(r'^run(N|\d+)$', dir_name)
for dir_name in Path(name).parts
):
raise WorkflowFilesError(
"Workflow name cannot contain a folder called 'runN' or "
"'run<number>'."
)


def infer_latest_run(
Expand Down Expand Up @@ -1628,7 +1636,7 @@ def install_workflow(
source = Path(expand_path(source))
if not workflow_name:
workflow_name = source.name
validate_workflow_name(workflow_name)
validate_workflow_name(workflow_name, runNcheck=True)
if run_name in WorkflowFiles.RESERVED_NAMES:
raise WorkflowFilesError(
f'Run name cannot be "{run_name}": That name is reserved.')
Expand Down
12 changes: 10 additions & 2 deletions tests/unit/test_workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,17 @@ def test_check_nested_dirs_install_dirs(
('$HOME/alone', WorkflowFilesError, "invalid workflow name"),
('./foo', WorkflowFilesError, "invalid workflow name"),
('meow/..', WorkflowFilesError,
"cannot be a path that points to the cylc-run directory or above")]
"cannot be a path that points to the cylc-run directory or above"),
('run6', WorkflowFilesError, "cannot contain a folder called 'runN'"),
('e/run6', WorkflowFilesError, "cannot contain a folder called 'runN'"),
('runN', WorkflowFilesError, "cannot contain a folder called 'runN'"),
('e/runN', WorkflowFilesError, "cannot contain a folder called 'runN'")]
)
def test_validate_workflow_name(reg, expected_err, expected_msg):
if expected_err:
with pytest.raises(expected_err) as exc:
workflow_files.validate_workflow_name(reg)
runNcheck = 'cannot contain a folder called' in expected_msg
workflow_files.validate_workflow_name(reg, runNcheck=runNcheck)
if expected_msg:
assert expected_msg in str(exc.value)
else:
Expand Down Expand Up @@ -1628,6 +1633,7 @@ def mocked_remote_clean_cmd_side_effect(reg, platform, rm_dirs, timeout):
for p_name in failed_platforms:
assert f"{p_name} - {PlatformError.MSG_TIDY}" in caplog.text


@pytest.mark.parametrize(
'rm_dirs, expected_args',
[
Expand Down Expand Up @@ -1809,6 +1815,7 @@ def test_check_flow_file(
else:
assert check_flow_file(tmp_path) == tmp_path.joinpath(expected_file)


def test_detect_both_flow_and_suite(tmp_path):
"""Test flow.cylc and suite.rc together in dir raises error."""
tmp_path.joinpath(WorkflowFiles.FLOW_FILE).touch()
Expand All @@ -1824,6 +1831,7 @@ def test_detect_both_flow_and_suite(tmp_path):
"#backward-compatibility"
)


@pytest.mark.parametrize(
'flow_file_target, suiterc_exists, err, expected_file',
[
Expand Down

0 comments on commit 9652260

Please sign in to comment.