diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..85d522b --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,29 @@ +name: Tests + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + pytest: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.14"] + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + - name: Install + run: | + python -m pip install -U pip + python -m pip install ".[test]" + + - name: Run tests + run: pytest -q diff --git a/README.md b/README.md index 632a76d..3140500 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,16 @@ An instance is a name with three files behind it: `/systemd/instances/ .yaml`, and their deployed copies under `/etc/`. Deployed means the `.env` is in `/etc//instances/`, which is what the template unit reads. +## Tests + +```bash +uv run --extra test pytest +``` + +They build a throwaway host tree and exercise real directory reads, stubbing +only systemd. The rendering tests assert that no `@placeholder@` survives, +which is what catches a template gaining a variable nobody wired up. + ## Host setup ` install`, as root, converges a host: it creates the ops group and the diff --git a/pyproject.toml b/pyproject.toml index fb28055..7349353 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,9 @@ dependencies = [ "pyyaml", ] +[project.optional-dependencies] +test = ["pytest"] + [tool.setuptools.packages.find] where = ["src"] include = ["instrumentctl"] @@ -19,6 +22,10 @@ include = ["instrumentctl"] [tool.setuptools.package-data] "instrumentctl" = ["templates/*"] +[tool.pytest.ini_options] +pythonpath = ["src"] +testpaths = ["tests"] + [tool.pylint.format] max-line-length = 100 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..e2ca2fb --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,74 @@ +"""A throwaway host tree, so the tests exercise real directory reads.""" +from __future__ import annotations + +import pathlib +import textwrap + +import pytest + +from instrumentctl.instances import Paths +from instrumentctl.product import Product + +INSTRUMENT_TOML = """\ +[instrument] +name = "demo" +display_name = "Demo" +user = "demouser" +ops_group = "demo-ops" +env_prefix = "DEMO" +docs_url = "https://example.invalid/demo" + +[secrets] +key_suffix = "_env" + +[legacy] +unit = "demo-daemon@.service" +polkit_rule = "49-demo-daemons.rules" +""" + +# name -> (host it belongs to, config body) +INSTANCES = { + "red_adc": ("thishost", "hardware:\n password_env: DEMO_RED_PASSWORD\n"), + "red_fw": ("thishost", "hardware:\n port: 9000\n"), + "blue_fw": ("thishost", "nested:\n deep:\n token_env: DEMO_TOKEN\n"), + "elsewhere_arm": ("otherhost", "hardware:\n port: 1\n"), +} + + +@pytest.fixture(name="product") +def product_fixture(tmp_path: pathlib.Path) -> Product: + config = tmp_path / "instrument.toml" + config.write_text(INSTRUMENT_TOML, encoding="utf-8") + return Product.load(config) + + +@pytest.fixture(name="paths") +def paths_fixture(tmp_path: pathlib.Path, product: Product) -> Paths: + """A repo with four instances, two of them deployed.""" + repo, etc = tmp_path / "repo", tmp_path / "etc" + (repo / "systemd" / "instances").mkdir(parents=True) + (repo / "config" / "arm").mkdir(parents=True) + (repo / "daemons" / "arm").mkdir(parents=True) + (repo / "daemons" / "arm" / "runner.py").touch() + (etc / "instances").mkdir(parents=True) + (etc / "host").write_text("thishost\n", encoding="utf-8") + + for name, (host, body) in INSTANCES.items(): + (repo / "systemd" / "instances" / f"{name}.env").write_text(textwrap.dedent(f"""\ + DEMO_DAEMON=arm/runner.py + DEMO_CONFIG={etc}/{name}.yaml + DEMO_HOST={host} + """), encoding="utf-8") + (repo / "config" / "arm" / f"{name}.yaml").write_text(body, encoding="utf-8") + + for name in ("red_adc", "blue_fw"): + (etc / "instances" / f"{name}.env").write_text( + (repo / "systemd" / "instances" / f"{name}.env").read_text(), encoding="utf-8") + (etc / f"{name}.yaml").write_text( + (repo / "config" / "arm" / f"{name}.yaml").read_text(), encoding="utf-8") + + return Paths.from_env(product, environ={ + "DEMO_REPO_DIR": str(repo), + "DEMO_ETC_DIR": str(etc), + "DEMO_VENV_DIR": str(tmp_path / "venv"), + }) diff --git a/tests/test_deploy.py b/tests/test_deploy.py new file mode 100644 index 0000000..8c997a1 --- /dev/null +++ b/tests/test_deploy.py @@ -0,0 +1,79 @@ +"""Deploy planning: what gets copied, and what is refused before anything is.""" +from __future__ import annotations + +import argparse + +import pytest + +from instrumentctl import commands +from instrumentctl.instances import Paths + + +def plan_args(**overrides: object) -> argparse.Namespace: + settings = dict(names=[], new=False, force=False, any_host=False, + no_start=True, no_enable=True, dry_run=True) + settings.update(overrides) + return argparse.Namespace(**settings) + + +def test_a_subsystem_deploys_every_instance_in_it(paths: Paths, capsys) -> None: + assert commands.deploy(paths, plan_args(names=["red"])) == 0 + out = capsys.readouterr().out + assert "red_adc" in out and "red_fw" in out + + +def test_an_instance_assigned_elsewhere_is_refused(paths: Paths, capsys) -> None: + assert commands.deploy(paths, plan_args(names=["elsewhere_arm"])) == 1 + assert "belongs on 'otherhost'" in capsys.readouterr().err + + +def test_any_host_overrides_the_assignment(paths: Paths) -> None: + assert commands.deploy(paths, plan_args(names=["elsewhere_arm"], + any_host=True)) == 0 + + +def test_one_bad_target_copies_nothing(paths: Paths, capsys) -> None: + """A typo in the third name must not leave the first two half-applied.""" + assert commands.deploy(paths, plan_args(names=["red_adc", "nonexistent"])) == 1 + assert "red_adc" not in capsys.readouterr().out + + +def test_an_instance_whose_config_is_missing_is_refused(paths: Paths, capsys) -> None: + (paths.repo / "config" / "arm" / "red_fw.yaml").unlink() + assert commands.deploy(paths, plan_args(names=["red_fw"])) == 1 + assert "expected one" in capsys.readouterr().err + + +def test_deploy_with_no_targets_asks_for_one(paths: Paths, capsys) -> None: + assert commands.deploy(paths, plan_args()) == 2 + assert "name the instances to deploy" in capsys.readouterr().err + + +def test_new_deploys_only_what_is_not_here_yet(paths: Paths, capsys) -> None: + assert commands.deploy(paths, plan_args(new=True, any_host=True)) == 0 + out = capsys.readouterr().out + # red_adc and blue_fw are already deployed by the fixture + assert "red_fw" in out and "elsewhere_arm" in out + assert "would copy" in out + + +def test_a_deployed_config_that_differs_is_kept(paths: Paths, capsys) -> None: + """It holds this host's real hardware values; the repo copy does not.""" + (paths.etc / "red_adc.yaml").write_text("hardware:\n port: 9999\n", encoding="utf-8") + commands.deploy(paths, plan_args(names=["red_adc"], dry_run=False)) + assert "kept" in capsys.readouterr().out + assert "9999" in (paths.etc / "red_adc.yaml").read_text() + + +def test_force_replaces_it(paths: Paths) -> None: + (paths.etc / "red_adc.yaml").write_text("hardware:\n port: 9999\n", encoding="utf-8") + commands.deploy(paths, plan_args(names=["red_adc"], force=True, dry_run=False)) + assert "9999" not in (paths.etc / "red_adc.yaml").read_text() + + +def test_the_instance_file_always_follows_the_repo(paths: Paths) -> None: + """It only points at things, so a host has no reason to diverge.""" + deployed = paths.instances / "red_adc.env" + deployed.write_text("DEMO_DAEMON=stale/thing.py\n", encoding="utf-8") + commands.deploy(paths, plan_args(names=["red_adc"], dry_run=False)) + assert "stale/thing.py" not in deployed.read_text() diff --git a/tests/test_product.py b/tests/test_product.py new file mode 100644 index 0000000..8314593 --- /dev/null +++ b/tests/test_product.py @@ -0,0 +1,38 @@ +"""Product: what an instrument declares, and what follows from it.""" +from __future__ import annotations + +import pathlib + +from instrumentctl.product import Product + + +def test_names_and_paths_follow_from_the_product(product: Product) -> None: + assert product.unit("red_adc") == "demo@red_adc.service" + assert product.env_key("DAEMON") == "DEMO_DAEMON" + assert product.unit_template == pathlib.Path("/etc/systemd/system/demo@.service") + assert product.polkit_rule == pathlib.Path("/etc/polkit-1/rules.d/49-demo.rules") + assert product.sudoers == pathlib.Path("/etc/sudoers.d/demo-ops") + assert product.enable_helper == pathlib.Path("/usr/local/sbin/demo-enable") + + +def test_instance_of_ignores_units_that_are_not_ours(product: Product) -> None: + assert product.instance_of("demo@red_adc.service") == "red_adc" + assert product.instance_of("sshd.service") is None + # A different instrument's units must not be claimed + assert product.instance_of("other@red_adc.service") is None + + +def test_optional_keys_default_from_the_name(tmp_path: pathlib.Path) -> None: + config = tmp_path / "minimal.toml" + config.write_text('[instrument]\nname = "bare"\n', encoding="utf-8") + bare = Product.load(config) + assert (bare.display_name, bare.user, bare.ops_group) == ("bare", "bare", "bare-ops") + assert bare.env_prefix == "BARE" + assert bare.secret_key_suffix == "_env" + assert bare.legacy_unit_path is None + + +def test_legacy_paths_appear_only_when_declared(product: Product) -> None: + assert product.legacy_unit_path is not None + assert product.legacy_unit_path.name == "demo-daemon@.service" + assert product.legacy_polkit_path.name == "49-demo-daemons.rules" diff --git a/tests/test_render.py b/tests/test_render.py new file mode 100644 index 0000000..a9194ab --- /dev/null +++ b/tests/test_render.py @@ -0,0 +1,65 @@ +"""Rendering host artefacts: the files that reach a host as root.""" +from __future__ import annotations + +import re + +import pytest + +from instrumentctl.installer import _render +from instrumentctl.instances import Paths +from instrumentctl.product import Product + +TEMPLATES = ("unit.service", "polkit.rules", "sudoers", + "front-door.sh", "enable-helper.sh") + +PLACEHOLDER = re.compile(r"@[a-z_]+@") + + +@pytest.mark.parametrize("template", TEMPLATES) +def test_no_placeholder_survives_rendering(template: str, product: Product, + paths: Paths) -> None: + """A template gaining a variable nobody wired up would otherwise reach a + host as a literal @user@ inside a unit file.""" + rendered = _render(template, product, paths) + assert not PLACEHOLDER.search(rendered), PLACEHOLDER.search(rendered).group() + + +@pytest.mark.parametrize("template", TEMPLATES) +def test_no_other_instrument_leaks_in(template: str, product: Product, + paths: Paths) -> None: + assert "hispec" not in _render(template, product, paths).lower() + + +def test_the_unit_runs_the_right_thing_as_the_right_user(product: Product, + paths: Paths) -> None: + unit = _render("unit.service", product, paths) + assert "Description=Demo hardware daemon (%i)" in unit + assert "User=demouser\nGroup=demouser" in unit + assert f"EnvironmentFile={paths.etc}/instances/%i.env" in unit + # Optional, so a host with no secrets file still starts + assert f"EnvironmentFile=-{paths.secrets}" in unit + assert (f"ExecStart={paths.venv}/bin/python3 {paths.repo}/daemons/" + "${DEMO_DAEMON} -c ${DEMO_CONFIG}") in unit + + +def test_the_polkit_rule_is_scoped_to_our_units_and_group(product: Product, + paths: Paths) -> None: + rule = _render("polkit.rules", product, paths) + assert 'subject.isInGroup("demo-ops")' in rule + assert 'unit.indexOf("demo@") == 0' in rule + + +def test_the_sudoers_line_is_scoped_to_the_one_helper(product: Product, + paths: Paths) -> None: + line = [l for l in _render("sudoers", product, paths).splitlines() + if l.startswith("%")] + assert line == ["%demo-ops ALL=(root) NOPASSWD: /usr/local/sbin/demo-enable"] + + +def test_the_enable_helper_only_touches_our_units(product: Product, + paths: Paths) -> None: + helper = _render("enable-helper.sh", product, paths) + assert f"INSTANCE_DIR={paths.etc}/instances" in helper + assert '"demo@$name.service"' in helper + # It must refuse a name that is not a deployed instance + assert "deploy the instance file first" in helper diff --git a/tests/test_resolve.py b/tests/test_resolve.py new file mode 100644 index 0000000..7a37820 --- /dev/null +++ b/tests/test_resolve.py @@ -0,0 +1,40 @@ +"""Turning what an operator typed into instance names.""" +from __future__ import annotations + +import pytest + +from instrumentctl.instances import TargetError, resolve + +NAMES = ["red_adc", "red_fw", "blue_fw"] + + +def test_a_shared_prefix_selects_the_subsystem() -> None: + assert resolve(["red"], NAMES) == ["red_adc", "red_fw"] + + +def test_all_selects_everything_in_order() -> None: + assert resolve(["all"], NAMES) == NAMES + + +def test_an_exact_name_wins_over_prefix_matching() -> None: + assert resolve(["red_adc"], NAMES) == ["red_adc"] + + +def test_targets_keep_command_line_order() -> None: + # `stop power fei` must not silently become `stop fei power` + assert resolve(["blue", "red"], NAMES) == ["blue_fw", "red_adc", "red_fw"] + + +def test_duplicates_are_dropped_but_order_survives() -> None: + assert resolve(["red", "red_adc"], NAMES) == ["red_adc", "red_fw"] + + +def test_an_unmatched_target_raises_rather_than_selecting_nothing() -> None: + with pytest.raises(TargetError): + resolve(["green"], NAMES) + + +def test_a_partial_word_does_not_match_a_longer_name() -> None: + # "re" is not a subsystem; matching it would start daemons nobody asked for + with pytest.raises(TargetError): + resolve(["re"], NAMES) diff --git a/tests/test_secrets.py b/tests/test_secrets.py new file mode 100644 index 0000000..6371313 --- /dev/null +++ b/tests/test_secrets.py @@ -0,0 +1,62 @@ +"""Finding the credentials a host needs, which is what doctor reports.""" +from __future__ import annotations + +import os + +import pytest + +from instrumentctl.instances import Paths, required_secrets, secrets_set + + +def test_credential_keys_are_found_at_any_depth(paths: Paths) -> None: + needed = required_secrets(paths) + # red_adc declares one at the top of hardware:, blue_fw two levels down + assert "DEMO_RED_PASSWORD" in needed + assert "DEMO_TOKEN" in needed + + +def test_each_variable_records_why_it_is_needed(paths: Paths) -> None: + assert needed_reason(paths, "DEMO_RED_PASSWORD") == "red_adc (hardware.password_env)" + + +def needed_reason(paths: Paths, variable: str) -> str: + return required_secrets(paths)[variable][0] + + +def test_instances_that_are_not_deployed_are_not_scanned(paths: Paths) -> None: + # elsewhere_arm and red_fw exist in the repo but are not deployed here + assert all("elsewhere_arm" not in reason + for reasons in required_secrets(paths).values() + for reason in reasons) + + +def test_a_config_with_no_credentials_contributes_nothing(paths: Paths) -> None: + assert not any("red_fw" in reason + for reasons in required_secrets(paths).values() + for reason in reasons) + + +def test_an_empty_value_counts_as_unset(paths: Paths) -> None: + paths.secrets.write_text("DEMO_TOKEN=\nDEMO_RED_PASSWORD=x\n", encoding="utf-8") + present = secrets_set(paths) + assert not present["DEMO_TOKEN"] + assert present["DEMO_RED_PASSWORD"] == "x" + + +@pytest.mark.skipif(os.geteuid() == 0, reason="root reads an unreadable file") +def test_an_unreadable_secrets_file_is_unknown_not_empty(paths: Paths) -> None: + """Reporting every credential as missing would be worse than saying so.""" + paths.secrets.write_text("DEMO_TOKEN=x\n", encoding="utf-8") + paths.secrets.chmod(0o000) + try: + assert secrets_set(paths) is None + finally: + paths.secrets.chmod(0o600) + + +def test_quotes_and_comments_are_handled(paths: Paths) -> None: + paths.secrets.write_text( + '# a comment\nDEMO_TOKEN="quoted value"\n\nDEMO_OTHER=bare\n', encoding="utf-8") + present = secrets_set(paths) + assert present["DEMO_TOKEN"] == "quoted value" + assert present["DEMO_OTHER"] == "bare"