Merge pull request 'clan/create: api fixes and unit tests' (#4449) from api-fixes into main

Reviewed-on: https://git.clan.lol/clan/clan-core/pulls/4449

pkgs/clan-app/ui/src/routes/Onboarding/Onboarding.tsx

@@ -220,16 +220,8 @@ export const Onboarding: Component<RouteSectionProps> = (props) => {
         // todo allow users to select a template
         template: "minimal",
         initial: {
-          meta: {
-            name: name,
-            description: description,
-            // todo it tries to 'delete' icon if it's not provided
-            // this logic is unexpected, and needs reviewed.
-            icon: null,
-          },
-          machines: {},
-          instances: {},
-          services: {},
+          name,
+          description,
         },
       },
     });

pkgs/clan-cli/clan_lib/clan/create.py

@@ -1,4 +1,5 @@
 import logging
+from collections.abc import Callable
 from dataclasses import dataclass
 from pathlib import Path
 
@@ -8,8 +9,11 @@ from clan_lib.dirs import clan_templates
 from clan_lib.errors import ClanError
 from clan_lib.flake import Flake
 from clan_lib.nix import nix_command, nix_metadata, nix_shell
-from clan_lib.persist.inventory_store import InventorySnapshot, InventoryStore
+from clan_lib.nix_models.clan import InventoryMeta
+from clan_lib.persist.inventory_store import InventoryStore
+from clan_lib.persist.util import merge_objects, set_value_by_path
 from clan_lib.templates.handler import clan_template
+from clan_lib.validator.hostname import hostname
 
 log = logging.getLogger(__name__)
 
@@ -21,9 +25,26 @@ class CreateOptions:
 
     src_flake: Flake | None = None
     setup_git: bool = True
-    initial: InventorySnapshot | None = None
+    initial: InventoryMeta | None = None
     update_clan: bool = True
 
+    # -- Internal use only --
+    #
+    # Post-processing hook to make flakes offline testable
+    _postprocess_flake_hook: Callable[[Path], None] | None = None
+
+    def validate(self) -> None:
+        if self.initial and "name" in self.initial:
+            try:
+                hostname(self.initial["name"])
+            except ClanError as e:
+                msg = "must be a valid hostname."
+                raise ClanError(
+                    msg,
+                    location="name",
+                    description="The 'name' field must be a valid hostname.",
+                ) from e
+
 
 def git_command(directory: Path, *args: str) -> list[str]:
     return nix_shell(["git"], ["git", "-C", str(directory), *args])
@@ -39,6 +60,7 @@ def create_clan(opts: CreateOptions) -> None:
         ClanError: If the source flake is not a valid flake or if the destination
             directory already exists.
     """
+    opts.validate()
 
     dest = opts.dest.resolve()
 
@@ -56,8 +78,14 @@ def create_clan(opts: CreateOptions) -> None:
         opts.src_flake = Flake(str(clan_templates()))
 
     with clan_template(
-        opts.src_flake, template_ident=opts.template, dst_dir=opts.dest
+        opts.src_flake,
+        template_ident=opts.template,
+        dst_dir=opts.dest,
+        # _postprocess_flake_hook must be private to avoid leaking it to the public API
+        post_process=opts._postprocess_flake_hook,  # noqa: SLF001
     ) as _clan_dir:
+        flake = Flake(str(opts.dest))
+
         if opts.setup_git:
             run(git_command(dest, "init"))
             run(git_command(dest, "add", "."))
@@ -77,13 +105,18 @@ def create_clan(opts: CreateOptions) -> None:
 
         if opts.update_clan:
             run(nix_command(["flake", "update"]), RunOpts(cwd=dest))
+            flake.invalidate_cache()
 
         if opts.setup_git:
             run(git_command(dest, "add", "."))
             run(git_command(dest, "commit", "-m", "Initial commit"))
 
-    if opts.initial:
-        inventory_store = InventoryStore(flake=Flake(str(opts.dest)))
-        inventory_store.write(opts.initial, message="Init inventory")
+        if opts.initial:
+            inventory_store = InventoryStore(flake)
+            inventory = inventory_store.read()
+            curr_meta = inventory.get("meta", {})
+            new_meta = merge_objects(curr_meta, opts.initial)
+            set_value_by_path(inventory, "meta", new_meta)
+            inventory_store.write(inventory, message="Init inventory")
 
     return

pkgs/clan-cli/clan_lib/clan/create_test.py

@@ -0,0 +1,130 @@
+from pathlib import Path
+from typing import Any
+
+import pytest
+
+from clan_lib.clan.create import CreateOptions, create_clan
+from clan_lib.errors import ClanError
+from clan_lib.flake import Flake
+from clan_lib.persist.inventory_store import InventoryStore
+
+
+@pytest.mark.with_core
+def test_create_simple(tmp_path: Path, offline_flake_hook: Any) -> None:
+    """
+    Template = 'default'
+    # All default params
+    """
+    dest = tmp_path / "test_clan"
+
+    opts = CreateOptions(
+        dest=dest, template="default", _postprocess_flake_hook=offline_flake_hook
+    )
+
+    create_clan(opts)
+
+    assert dest.exists()
+    assert dest.is_dir()
+
+    flake = Flake(str(dest))
+
+    inventory_store = InventoryStore(flake)
+    inventory = inventory_store.read()
+
+    # Smoke check that some inventory data is present
+    assert isinstance(inventory, dict)
+    assert "meta" in inventory
+    assert "machines" in inventory
+    assert "instances" in inventory
+
+
+@pytest.mark.with_core
+def test_create_with_name(tmp_path: Path, offline_flake_hook: Any) -> None:
+    """
+    Template = 'default'
+    # All default params
+    """
+    dest = tmp_path / "test_clan"
+
+    opts = CreateOptions(
+        dest=dest,
+        template="minimal",  # important the default template is not writable
+        initial={
+            "name": "test-clan",  # invalid hostname
+            "description": "Test description",
+            # Note: missing "icon", should be okay and should not get persisted
+        },
+        _postprocess_flake_hook=offline_flake_hook,
+    )
+    create_clan(opts)
+
+    assert dest.exists()
+    assert dest.is_dir()
+
+    flake = Flake(str(dest))
+
+    inventory_store = InventoryStore(flake)
+    inventory = inventory_store.read()
+
+    # Smoke check that some inventory data is present
+    assert isinstance(inventory, dict)
+    assert "meta" in inventory
+    assert "machines" in inventory
+    assert "instances" in inventory
+
+    meta = inventory.get("meta", {})
+    assert meta.get("name") == "test-clan"
+    assert meta.get("description") == "Test description"
+    assert meta.get("icon") is None, "Icon should not be set if not provided"
+
+
+# When using the 'default' template, the name is set in nix
+# Which means we cannot set it via initial data
+# This test ensures that we cannot set nix values
+# We might want to change this in the future
+@pytest.mark.with_core
+def test_create_cannot_set_name(tmp_path: Path, offline_flake_hook: Any) -> None:
+    """
+    Template = 'default'
+    # All default params
+    """
+    dest = tmp_path / "test_clan"
+
+    opts = CreateOptions(
+        dest=dest,
+        template="default",  # The default template currently has a non-writable 'name'
+        initial={
+            "name": "test-clan",
+        },
+        _postprocess_flake_hook=offline_flake_hook,
+    )
+    with pytest.raises(ClanError) as exc_info:
+        create_clan(opts)
+
+    assert (
+        "Key 'meta.name' is not writeable. It seems its value is statically defined in nix."
+        in str(exc_info.value)
+    )
+
+
+@pytest.mark.with_core
+def test_create_invalid_name(tmp_path: Path, offline_flake_hook: Any) -> None:
+    """
+    Template = 'default'
+    # All default params
+    """
+    dest = tmp_path / "test_clan"
+
+    opts = CreateOptions(
+        dest=dest,
+        template="default",  # The default template currently has a non-writable 'name'
+        initial={
+            "name": "test clan",  # spaces are not allowed in hostnames
+        },
+        _postprocess_flake_hook=offline_flake_hook,
+    )
+    with pytest.raises(ClanError) as exc_info:
+        create_clan(opts)
+
+    assert "must be a valid hostname." in str(exc_info.value)
+    assert "name" in str(exc_info.value.location)

pkgs/clan-cli/clan_lib/conftest.py

@@ -3,4 +3,5 @@ pytest_plugins = [
     "clan_cli.tests.hosts",
     "clan_cli.tests.sshd",
     "clan_cli.tests.runtime",
+    "clan_lib.fixtures.flake_hooks",
 ]

pkgs/clan-cli/clan_lib/fixtures/flake_hooks.py

@@ -0,0 +1,31 @@
+from collections.abc import Callable
+from pathlib import Path
+
+import pytest
+from clan_lib.cmd import RunOpts, run
+from clan_lib.nix import nix_command
+
+
+def substitute_flake_inputs(clan_dir: Path, clan_core_path: Path) -> None:
+    flake_nix = clan_dir / "flake.nix"
+    assert flake_nix.exists()
+
+    content = flake_nix.read_text()
+    content = content.replace(
+        "https://git.clan.lol/clan/clan-core/archive/main.tar.gz",
+        f"path://{clan_core_path}",
+    )
+    flake_nix.write_text(content)
+
+    run(nix_command(["flake", "update"]), RunOpts(cwd=clan_dir))
+
+    flake_lock = clan_dir / "flake.lock"
+    assert flake_lock.exists(), "flake.lock should exist after flake update"
+
+
+@pytest.fixture
+def offline_flake_hook(clan_core: Path) -> Callable[[Path], None]:
+    def patch(clan_dir: Path) -> None:
+        substitute_flake_inputs(clan_dir, clan_core)
+
+    return patch

pkgs/clan-cli/clan_lib/templates/handler.py

@@ -1,6 +1,6 @@
 import logging
 import shutil
-from collections.abc import Iterator
+from collections.abc import Callable, Iterator
 from contextlib import contextmanager
 from pathlib import Path
 
@@ -102,7 +102,12 @@ def machine_template(
 
 
 @contextmanager
-def clan_template(flake: Flake, template_ident: str, dst_dir: Path) -> Iterator[Path]:
+def clan_template(
+    flake: Flake,
+    template_ident: str,
+    dst_dir: Path,
+    post_process: Callable[[Path], None] | None = None,
+) -> Iterator[Path]:
     """
     Create a clan from a template.
     This function will copy the template files to a new clan directory
@@ -159,6 +164,17 @@ def clan_template(flake: Flake, template_ident: str, dst_dir: Path) -> Iterator[
 
     copy_from_nixstore(src_path, dst_dir)
 
+    if post_process:
+        try:
+            post_process(dst_dir)
+        except Exception as e:
+            log.error(f"Error during post-processing of clan template: {e}")
+            log.info(f"Removing left-over directory: {dst_dir}")
+            shutil.rmtree(dst_dir, ignore_errors=True)
+            msg = (
+                f"Post-processing of clan template {printable_template_ref} failed: {e}"
+            )
+            raise ClanError(msg) from e
     try:
         yield dst_dir
     except Exception as e:

pkgs/clan-cli/clan_lib/tests/test_create.py

@@ -115,7 +115,7 @@ def test_clan_create_api(
     host_ip = hosts[0].address
     host_user = hosts[0].user
     vm_name = "test-clan"
-    clan_core_dir_var = str(clan_core)
+
     priv_key_var = hosts[0].private_key
     ssh_port_var = str(hosts[0].port)
 
@@ -143,9 +143,8 @@ def test_clan_create_api(
     assert dest_clan_dir.is_dir()
     assert (dest_clan_dir / "flake.nix").is_file()
 
-    clan_core_dir = Path(clan_core_dir_var)
     # TODO: We need a way to generate the lock file for the templates
-    fix_flake_inputs(dest_clan_dir, clan_core_dir)
+    fix_flake_inputs(dest_clan_dir, clan_core)
 
     # ===== CREATE SOPS KEY ======
     sops_keys = maybe_get_admin_public_keys()

pkgs/clan-cli/clan_lib/validator/hostname.py

@@ -0,0 +1,30 @@
+import re
+
+from clan_lib.errors import ClanError
+
+
+def hostname(host: str) -> str:
+    """
+    Validates a hostname according to the expected format in NixOS.
+
+    Usage Example
+
+    @dataclass
+    class Clan:
+        name: str
+
+        def validate(self) -> None:
+            from clan_lib.validator.hostname import hostname
+            try:
+                hostname(self.name)
+            except ValueError as e:
+                raise ClanError(str(e), location="name")
+    """
+
+    # TODO: Generate from nix schema
+    hostname_regex = r"^(?!-)[A-Za-z0-9-]{1,63}(?<!-)$"
+    if not re.fullmatch(hostname_regex, host):
+        msg = "Machine name must be a valid hostname"
+        raise ClanError(msg, location="Create Machine")
+
+    return host