Merge pull request #722 from fractal-analytics-platform/716-include-m…

…ore-user-settings-attributes-in-fractal-user-register

Add `ssh_settings_json` to user register

CHANGELOG.md

@@ -10,6 +10,7 @@
 * Rename `source` into `label`, for `task collect-custom` command (\#712).
 * Do not refer to obsolete task attributes `source` or `owner` (\#712, \#717).
 * Add `--new-ssh-settings-json` option to `fractal user edit` (\#715).
+* Add `--ssh-settings-json` option to `fractal user register` (\#722).
 * Add `--private` option to task-creating commands (\#717).
 * Drop `task delete` command (\#717).
 * Handle missing server in `fractal version` (\#724).

fractal_client/cmd/__init__.py

@@ -327,6 +327,7 @@ def user(
             "cache_dir",
             "slurm_user",
             "username",
+            "ssh_settings_json",
             "superuser",
         ]
         function_kwargs = get_kwargs(parameters, kwargs)

fractal_client/cmd/_user.py

@@ -10,6 +10,35 @@
 from ..response import check_response
 
 
+def _read_ssh_settings_json(ssh_settings_json: str) -> dict:
+    """
+    Read, validate and return as a dict the user's ssh-settings json file
+    """
+    ssh_settings_json_path = Path(ssh_settings_json)
+    if not ssh_settings_json_path.exists():
+        sys.exit(f"Invalid {ssh_settings_json=}. File does not exist.")
+    with ssh_settings_json_path.open("r") as f:
+        try:
+            ssh_settings = json.load(f)
+        except JSONDecodeError:
+            sys.exit(f"{ssh_settings_json_path} is not a valid JSON.")
+    __ALLOWED_KEYS__ = (
+        "ssh_host",
+        "ssh_username",
+        "ssh_private_key_path",
+        "ssh_tasks_dir",
+        "ssh_jobs_dir",
+    )
+    settings = dict()
+    for key, value in ssh_settings.items():
+        if key in __ALLOWED_KEYS__:
+            settings[key] = value
+        else:
+            sys.exit(f"Invalid {key=} in {ssh_settings_json=}.")
+
+    return settings
+
+
 def user_register(
     client: AuthClient,
     *,
@@ -18,6 +47,7 @@ def user_register(
     slurm_user: Optional[str] = None,
     cache_dir: Optional[str] = None,
     username: Optional[str] = None,
+    ssh_settings_json: Optional[str] = None,
     superuser: bool = False,
     verified: bool = True,  # TODO: this is not currently exposed in the CLI
     batch: bool = False,
@@ -34,6 +64,9 @@ def user_register(
         new_settings["slurm_user"] = slurm_user
     if cache_dir:
         new_settings["cache_dir"] = cache_dir
+    if ssh_settings_json is not None:
+        ssh_settings = _read_ssh_settings_json(ssh_settings_json)
+        new_settings.update(ssh_settings)
 
     res = client.post(
         f"{settings.FRACTAL_SERVER}/auth/register/", json=new_user
@@ -134,26 +167,8 @@ def user_edit(
     if new_slurm_user is not None:
         settings_update["slurm_user"] = new_slurm_user
     if new_ssh_settings_json is not None:
-        new_ssh_settings_json_path = Path(new_ssh_settings_json)
-        if not new_ssh_settings_json_path.exists():
-            sys.exit(f"Invalid {new_ssh_settings_json=}. File does not exist.")
-        with new_ssh_settings_json_path.open("r") as f:
-            try:
-                ssh_settings = json.load(f)
-            except JSONDecodeError:
-                sys.exit(f"{new_ssh_settings_json_path} is not a valid JSON.")
-        __ALLOWED_KEYS__ = (
-            "ssh_host",
-            "ssh_username",
-            "ssh_private_key_path",
-            "ssh_tasks_dir",
-            "ssh_jobs_dir",
-        )
-        for key, value in ssh_settings.items():
-            if key in __ALLOWED_KEYS__:
-                settings_update[key] = value
-            else:
-                sys.exit(f"Invalid {key=} in {new_ssh_settings_json=}.")
+        ssh_settings = _read_ssh_settings_json(new_ssh_settings_json)
+        settings_update.update(ssh_settings)
 
     res = client.patch(
         f"{settings.FRACTAL_SERVER}/auth/users/{user_id}/", json=user_update

fractal_client/parser.py

@@ -800,6 +800,16 @@
     help="Username associated to the user.",
     required=False,
 )
+user_register_parser.add_argument(
+    "--ssh-settings-json",
+    help=(
+        "Path to JSON file with (a subset of) following settings: "
+        "ssh_host, ssh_username, ssh_private_key_path, "
+        "ssh_tasks_dir, ssh_jobs_dir."
+    ),
+    type=str,
+    required=False,
+)
 user_register_parser.add_argument(
     "--superuser",
     help="Give superuser privileges to the new user.",

tests/test_user.py

@@ -40,6 +40,53 @@ def test_register_as_superuser(
     assert res.data["is_verified"]
 
 
+def test_register_with_ssh_settings(invoke_as_superuser, new_name, tmp_path):
+
+    EMAIL_USER = f"{new_name()}@example.org"
+
+    with pytest.raises(SystemExit, match="File does not exist"):
+        invoke_as_superuser(
+            f"user register {EMAIL_USER} {PWD_USER} --ssh-settings-json xy.z"
+        )
+
+    invalid_json = tmp_path / "not-a-json.foo"
+    with invalid_json.open("w") as f:
+        f.write("hello world")
+    with pytest.raises(SystemExit, match="not a valid JSON"):
+        invoke_as_superuser(
+            f"user register {EMAIL_USER} {PWD_USER} "
+            f"--ssh-settings-json {invalid_json}"
+        )
+
+    invalid_key_json = tmp_path / "invalid-key.json"
+    with invalid_key_json.open("w") as f:
+        json.dump(dict(invalid="invalid"), f)
+    with pytest.raises(SystemExit, match="Invalid key"):
+        invoke_as_superuser(
+            f"user register {EMAIL_USER} {PWD_USER} "
+            f"--ssh-settings-json {invalid_key_json}"
+        )
+
+    valid_json = tmp_path / "ssh-config.json"
+    with valid_json.open("w") as f:
+        json.dump(
+            dict(
+                ssh_host="SSH_HOST",
+                ssh_private_key_path="/SSH_PRIVATE_KEY_PATH",
+            ),
+            f,
+        )
+    res = invoke_as_superuser(
+        f"user register {EMAIL_USER} {PWD_USER} "
+        f"--ssh-settings-json {valid_json}"
+    )
+    assert res.retcode == 0
+    assert res.data["settings"]["ssh_host"] == "SSH_HOST"
+    assert res.data["settings"]["ssh_private_key_path"] == (
+        "/SSH_PRIVATE_KEY_PATH"
+    )
+
+
 def test_register_as_superuser_with_batch(invoke_as_superuser, new_name):
     EMAIL_USER = f"{new_name()}@example.org"
     # Register a user with the --batch flag