Check run name as well as workflow name in Cylc install

Update changes.d/6264.fix.md Co-authored-by: Ronnie Dutta <[email protected]> Apply suggestions from code review Co-authored-by: Ronnie Dutta <[email protected]> Restore workflow_files added tests for reserved dir names
cylc · Aug 1, 2024 · e040d9c · e040d9c
1 parent f68e8b9
commit e040d9c
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 2 deletions.
diff --git a/changes.d/6264.fix.md b/changes.d/6264.fix.md
@@ -0,0 +1 @@
+Fix bug where `cylc install` failed to prevent invalid run names.
diff --git a/cylc/flow/install.py b/cylc/flow/install.py
@@ -285,13 +285,17 @@ 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:
         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)
+        validate_workflow_name(
+            os.path.join(workflow_name, run_name),
+            check_reserved_names=True
+        )
+    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(

diff --git a/tests/unit/test_install.py b/tests/unit/test_install.py
@@ -542,3 +542,48 @@ 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_name_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?')
+
+
+def test_install_workflow_failif_reserved_name(tmp_src_dir):
+    """Reserved names cause install validation failure.
+
+    n.b. manually defined to avoid test dependency on workflow_files.
+    """
+    src_dir = tmp_src_dir('ludlow')
+    is_reserved = '(that filename is reserved)'
+    reserved_names = {
+        'share',
+        'log',
+        'runN',
+        'suite.rc',
+        'work',
+        '_cylc-install',
+        'flow.cylc',
+        # .service fails because starting a workflow/run can't start with "."
+        # And that check fails first.
+        # '.service'
+    }
+    install_workflow(src_dir, workflow_name='ok', run_name='also_ok')
+    for name in reserved_names:
+        with pytest.raises(WorkflowFilesError, match=is_reserved):
+            install_workflow(src_dir, workflow_name='ok', run_name=name)
+        with pytest.raises(WorkflowFilesError, match=is_reserved):
+            install_workflow(src_dir, workflow_name=name)
+        with pytest.raises(WorkflowFilesError, match=is_reserved):
+            install_workflow(src_dir, workflow_name=name, run_name='ok')