Updates to top level Cli

cli/pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "dtaas"
-version = "0.2.0"
+version = "0.2.1"
 description = "DTaaS CLI"
 authors = ["Astitva Sehgal"]
 license = "INTO-CPS-Association"
@@ -9,17 +9,17 @@ packages = [{include = "src"}]
 
 [tool.poetry.dependencies]
 python = "^3.10"
-PyYAML = "^6.0.2"
-click = "^8.1.7"
-tomlkit = "^0.13.2"
+PyYAML = "^6.0.3"
+click = "^8.3.1"
+tomlkit = "^0.13.3"
 
 [tool.poetry.group.dev.dependencies]
-pytest = "^8.3.3"
-pylint = "^3.3.1"
-pytest-cov = "^6.0.0"
-setuptools = "^80.0.0"
-zipp = "^3.19.1"
-safety = "^3.6.2"
+pytest = "^9.0.2"
+pylint = "^4.0.4"
+pytest-cov = "^7.0.0"
+setuptools = "^80.9.0"
+zipp = "^3.23.0"
+safety = "^3.7.0"
 
 [build-system]
 requires = ["poetry-core>=1.0.0", "setuptools>=80.0.0"]

cli/src/cmd.py

@@ -32,9 +32,9 @@ def add():
     Specify the list in dtaas.toml [users].add\n
     """
 
-    configObj = configPkg.Config()
+    config_obj = configPkg.Config()
 
-    err = userPkg.add_users(configObj)
+    err = userPkg.add_users(config_obj)
     if err is not None:
         raise click.ClickException("Error while adding users: " + str(err))
     click.echo("Users added successfully")
@@ -47,9 +47,9 @@ def delete():
     Specify the users in dtaas.toml [users].delete\n
     """
 
-    configObj = configPkg.Config()
+    config_obj = configPkg.Config()
 
-    err = userPkg.delete_user(configObj)
+    err = userPkg.delete_user(config_obj)
     if err is not None:
         raise click.ClickException("Error while deleting users: " + str(err))
     click.echo("User deleted successfully")

cli/src/pkg/users.py

@@ -5,35 +5,60 @@
 from src.pkg import utils
 from src.pkg.constants import COMPOSE_USERS_YML
 
-def get_compose_config(username, server, path, resources):
-    """Makes and returns the config for the user"""
+def _build_config_mapping(user_config, resources):
+    """Build the mapping for config substitution.
 
-    template = {}
+    Args:
+        user_config: Dict with keys 'username', 'path', optionally 'server'
+        resources: Dict with resource limits
+
+    Returns:
+        Mapping dict for config substitution
+    """
     mapping = {
-        "${DTAAS_DIR}": path,
-        "${username}": username,
+        "${DTAAS_DIR}": user_config["path"],
+        "${username}": user_config["username"],
         "${shm_size}": str(resources["shm_size"]),
         "${cpus}": str(resources["cpus"]),
         "${mem_limit}": str(resources["mem_limit"]),
         "${pids_limit}": str(resources["pids_limit"])
-
     }
-    try:
-        if server == utils.LOCALHOST_SERVER:
-            template, err = utils.import_yaml("users.local.yml")
-            utils.check_error(err)
+    if user_config.get("server") is not None:
+        mapping["${SERVER_DNS}"] = user_config["server"]
+    return mapping
+
+
+def _load_template(server):
+    """Load the appropriate template based on server type."""
+    if server == utils.LOCALHOST_SERVER:
+        return utils.import_yaml("users.local.yml")
+    return utils.import_yaml("users.server.yml")
+
 
-        else:
-            template, err = utils.import_yaml("users.server.yml")
-            utils.check_error(err)
-            mapping["${SERVER_DNS}"] = server
+def get_compose_config(username, config):
+    """Makes and returns the config for the user
 
-        config, err = utils.replace_all(template, mapping)
+    Args:
+        username: Username for the config
+        config: Dict with 'server', 'path', 'resources' keys
+
+    Returns:
+        Tuple of (user config dict, error if any)
+    """
+    try:
+        template, err = _load_template(config["server"])
+        utils.check_error(err)
+        user_config = {
+            "username": username,
+            "path": config["path"],
+            "server": config["server"] if config["server"] != utils.LOCALHOST_SERVER else None
+        }
+        mapping = _build_config_mapping(user_config, config["resources"])
+        result, err = utils.replace_all(template, mapping)
         utils.check_error(err)
     except Exception as e:
         return None, e
-
-    return config, None
+    return result, None
 
 
 def create_user_files(users, file_path):
@@ -44,27 +69,30 @@ def create_user_files(users, file_path):
         )
 
 
-def add_users_to_compose(users, compose, server, path, resources):
-    """Adds all the users config to the compose dictionary"""
+def add_users_to_compose(users, compose, config):
+    """Adds all the users config to the compose dictionary
+    Args:
+        users: List of usernames
+        compose: Compose dict to update
+        config: Dict with 'server', 'path', 'resources' keys
+    """
     for username in users:
-        config, err = get_compose_config(username, server, path, resources)
+        user_conf, err = get_compose_config(username, config)
         if err is not None:
             return err
-        compose["services"][username] = config
+        compose["services"][username] = user_conf
     return None
 
 
 def start_user_containers(users):
     """Starts all the user containers in the 'users' list"""
-
     cmd = "docker compose -f compose.users.yml up -d"
     err = run_command_for_containers(cmd, users)
     return err
 
 
 def stop_user_containers(users):
     """Stops all the user containers in the 'users' list"""
-
     cmd = "docker compose -f compose.users.yml down"
     err = run_command_for_containers(cmd, users)
     return err
@@ -75,51 +103,75 @@ def run_command_for_containers(command, containers):
     cmd = [command]
     for name in containers:
         cmd.append(name)
-
     cmd_str = " ".join(cmd)
     result = subprocess.run(cmd_str, shell=True, check=False)
     if result.returncode != 0:
         return Exception(f"failed to run '{cmd_str}' command")
     return None
 
 
+def _setup_compose_structure(compose):
+    """Ensure compose has required structure for services."""
+    if "version" not in compose:
+        compose["version"] = "3"
+    if "services" not in compose:
+        compose["services"] = {}
+    if "networks" not in compose:
+        compose["networks"] = {"users": {"name": "dtaas-users", "external": True}}
+
+
+def _get_add_users_config(config_obj):
+    """Retrieve configuration needed for adding users."""
+    user_list, err = config_obj.get_add_users_list()
+    utils.check_error(err)
+    server, err = config_obj.get_server_dns()
+    utils.check_error(err)
+    path, err = config_obj.get_path()
+    utils.check_error(err)
+    resources, err = config_obj.get_resource_limits()
+    utils.check_error(err)
+    return user_list, server, path, resources
+
+
+def _finalize_compose(compose):
+    """Export and start user containers."""
+    err = utils.export_yaml(compose, COMPOSE_USERS_YML)
+    utils.check_error(err)
+    users_list = list(compose["services"].keys())
+    err = start_user_containers(users_list)
+    utils.check_error(err)
+
+
 def add_users(config_obj):
     """add cli command handler"""
     try:
         compose, err = utils.import_yaml(COMPOSE_USERS_YML)
         utils.check_error(err)
-        user_list, err = config_obj.get_add_users_list()
-        utils.check_error(err)
-        server, err = config_obj.get_server_dns()
-        utils.check_error(err)
-        path, err = config_obj.get_path()
-        utils.check_error(err)
+        user_list, server, path, resources = _get_add_users_config(config_obj)
     except Exception as e:
         return e
 
-    if "version" not in compose:
-        compose["version"] = "3"
-    if "services" not in compose:
-        compose["services"] = {}
-    if "networks" not in compose:
-        compose["networks"] = {"users": {"name": "dtaas-users", "external": True}}
+    _setup_compose_structure(compose)
 
     try:
         create_user_files(user_list, path + "/files")
-        resources, err = config_obj.get_resource_limits()
-        utils.check_error(err)
-        err = add_users_to_compose(user_list, compose, server, path, resources)
-        utils.check_error(err)
-        err = utils.export_yaml(compose, COMPOSE_USERS_YML)
-        utils.check_error(err)
-        err = start_user_containers(user_list)
+        config = {"server": server, "path": path, "resources": resources}
+        err = add_users_to_compose(user_list, compose, config)
         utils.check_error(err)
+        _finalize_compose(compose)
     except Exception as e:
         return e
 
     return None
 
 
+def _remove_users_from_compose(compose, user_list):
+    """Remove users from compose configuration."""
+    for username in user_list:
+        if "services" in compose and username in compose["services"]:
+            del compose["services"][username]
+
+
 def delete_user(config_obj):
     """delete cli command handler"""
     try:
@@ -129,14 +181,9 @@ def delete_user(config_obj):
         utils.check_error(err)
         err = stop_user_containers(user_list)
         utils.check_error(err)
-
-        for username in user_list:
-            if "services" in compose and username in compose["services"]:
-                del compose["services"][username]
-
+        _remove_users_from_compose(compose, user_list)
         err = utils.export_yaml(compose, COMPOSE_USERS_YML)
         utils.check_error(err)
-
     except Exception as e:
         return e
 

cli/src/pkg/utils.py

@@ -25,7 +25,14 @@ def export_yaml(data, filename):
     """This function is used to export to a yaml file safely"""
     try:
         with open(filename, "w") as file:
-            yaml.safe_dump(data, file, sort_keys=False)
+            yaml.safe_dump(
+                data,
+                file,
+                sort_keys=False,
+                default_flow_style=False,
+                allow_unicode=True,
+                indent=2,
+            )
     except Exception as err:
         return Exception(f"Error while writing yaml to file: {filename}, " + str(err))
     return None
@@ -61,12 +68,14 @@ def replace_all(obj, mapping):
 
 
 def replace_string(s, mapping):
+    """Replaces all placeholders in the string with values from the mapping"""
     for key in mapping:
         s = s.replace(key, mapping[key])
     return s, None
 
 
 def replace_list(arr, mapping):
+    """Replaces all placeholders in the list with values from the mapping"""
     for ind, val in enumerate(arr):
         arr[ind], err = replace_all(val, mapping)
         if err is not None:
@@ -75,6 +84,7 @@ def replace_list(arr, mapping):
 
 
 def replace_dict(dictionary, mapping):
+    """Replaces all placeholders in the dictionary with values from the mapping"""
     for key in dictionary:
         if not isinstance(key, str):
             return None, Exception("Config substitution failed: Key is not a string")
@@ -85,5 +95,6 @@ def replace_dict(dictionary, mapping):
 
 
 def check_error(err):
+    """Checks if error is not None and raises it"""
     if err is not None:
         raise err

cli/tests/test_cli.py

@@ -1,3 +1,5 @@
+"""Integration tests for DTaaS CLI commands."""
+
 import subprocess
 from pathlib import Path
 import sys
@@ -11,6 +13,7 @@ def test_add_user_cli():
         capture_output=True,
         text=True,
         cwd=Path(__file__).parent.parent,
+        check=False,
     )
     assert result.returncode == 0, f"Command failed: {result.stderr}\n{result.stdout}"
 
@@ -22,5 +25,6 @@ def test_delete_user_cli():
         capture_output=True,
         text=True,
         cwd=Path(__file__).parent.parent,
+        check=False,
     )
     assert result.returncode == 0, f"Command failed: {result.stderr}\n{result.stdout}"

cli/tests/test_cmd.py

@@ -1,7 +1,10 @@
+"""Tests for admin user commands in the CLI."""
+
+from unittest.mock import patch
 import pytest
 from click.testing import CliRunner
 from src.cmd import dtaas
-from unittest.mock import patch
+# pylint: disable=redefined-outer-name
 
 
 @pytest.fixture