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

Add ssh_settings_json to user register #722

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
1 change: 1 addition & 0 deletions fractal_client/cmd/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ def user(
"cache_dir",
"slurm_user",
"username",
"ssh_settings_json",
"superuser",
]
function_kwargs = get_kwargs(parameters, kwargs)
Expand Down
55 changes: 35 additions & 20 deletions fractal_client/cmd/_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
*,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions fractal_client/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
47 changes: 47 additions & 0 deletions tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading