From 95b784c1a0180127a232bfd9463b101d650086cf Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 10 Jan 2026 11:16:28 +1100 Subject: [PATCH] Fix and add tests --- tests/test_diff_new_features.py | 400 ++++++++++++++++++++++++++++++++ tests/test_manifest.py | 18 +- 2 files changed, 409 insertions(+), 9 deletions(-) create mode 100644 tests/test_diff_new_features.py diff --git a/tests/test_diff_new_features.py b/tests/test_diff_new_features.py new file mode 100644 index 0000000..fd0524f --- /dev/null +++ b/tests/test_diff_new_features.py @@ -0,0 +1,400 @@ +from __future__ import annotations + +import json +import sys +import types +from pathlib import Path + +import pytest + + +def _write_bundle( + root: Path, state: dict, artifacts: dict[str, bytes] | None = None +) -> None: + root.mkdir(parents=True, exist_ok=True) + (root / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") + artifacts = artifacts or {} + for rel, data in artifacts.items(): + p = root / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_bytes(data) + + +def _minimal_roles() -> dict: + """A small roles structure that's sufficient for enroll.diff file indexing.""" + return { + "users": { + "role_name": "users", + "users": [], + "managed_files": [], + "excluded": [], + "notes": [], + }, + "services": [], + "packages": [], + "apt_config": { + "role_name": "apt_config", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "etc_custom": { + "role_name": "etc_custom", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "usr_local_custom": { + "role_name": "usr_local_custom", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "extra_paths": { + "role_name": "extra_paths", + "include_patterns": [], + "exclude_patterns": [], + "managed_files": [], + "excluded": [], + "notes": [], + }, + } + + +def test_diff_ignore_package_versions_suppresses_version_drift(tmp_path: Path): + from enroll.diff import compare_harvests + + old = tmp_path / "old" + new = tmp_path / "new" + + old_state = { + "schema_version": 3, + "host": {"hostname": "h1"}, + "inventory": { + "packages": { + "curl": { + "version": "1.0", + "installations": [{"version": "1.0", "arch": "amd64"}], + } + } + }, + "roles": _minimal_roles(), + } + new_state = { + **old_state, + "inventory": { + "packages": { + "curl": { + "version": "1.1", + "installations": [{"version": "1.1", "arch": "amd64"}], + } + } + }, + } + + _write_bundle(old, old_state) + _write_bundle(new, new_state) + + # Without ignore flag, version drift is reported and counts as changes. + report, has_changes = compare_harvests(str(old), str(new)) + assert has_changes is True + assert report["packages"]["version_changed"] + + # With ignore flag, version drift is suppressed and does not count as changes. + report2, has_changes2 = compare_harvests( + str(old), str(new), ignore_package_versions=True + ) + assert has_changes2 is False + assert report2["packages"]["version_changed"] == [] + assert report2["packages"]["version_changed_ignored_count"] == 1 + assert report2["filters"]["ignore_package_versions"] is True + + +def test_diff_exclude_path_filters_file_drift_and_affects_has_changes(tmp_path: Path): + from enroll.diff import compare_harvests + + old = tmp_path / "old" + new = tmp_path / "new" + + # Only file drift is under /var/anacron, which is excluded. + old_state = { + "schema_version": 3, + "host": {"hostname": "h1"}, + "inventory": {"packages": {}}, + "roles": { + **_minimal_roles(), + "extra_paths": { + **_minimal_roles()["extra_paths"], + "managed_files": [ + { + "path": "/var/anacron/daily.stamp", + "src_rel": "var/anacron/daily.stamp", + "owner": "root", + "group": "root", + "mode": "0644", + "reason": "extra_path", + } + ], + }, + }, + } + new_state = json.loads(json.dumps(old_state)) + + _write_bundle( + old, + old_state, + {"artifacts/extra_paths/var/anacron/daily.stamp": b"yesterday\n"}, + ) + _write_bundle( + new, + new_state, + {"artifacts/extra_paths/var/anacron/daily.stamp": b"today\n"}, + ) + + report, has_changes = compare_harvests( + str(old), str(new), exclude_paths=["/var/anacron"] + ) + assert has_changes is False + assert report["files"]["changed"] == [] + assert report["filters"]["exclude_paths"] == ["/var/anacron"] + + +def test_diff_exclude_path_only_filters_files_not_packages(tmp_path: Path): + from enroll.diff import compare_harvests + + old = tmp_path / "old" + new = tmp_path / "new" + + old_state = { + "schema_version": 3, + "host": {"hostname": "h1"}, + "inventory": {"packages": {"curl": {"version": "1.0"}}}, + "roles": { + **_minimal_roles(), + "extra_paths": { + **_minimal_roles()["extra_paths"], + "managed_files": [ + { + "path": "/var/anacron/daily.stamp", + "src_rel": "var/anacron/daily.stamp", + "owner": "root", + "group": "root", + "mode": "0644", + "reason": "extra_path", + } + ], + }, + }, + } + new_state = { + **old_state, + "inventory": { + "packages": { + "curl": {"version": "1.0"}, + "htop": {"version": "3.0"}, + } + }, + } + + _write_bundle( + old, + old_state, + {"artifacts/extra_paths/var/anacron/daily.stamp": b"yesterday\n"}, + ) + _write_bundle( + new, + new_state, + { + "artifacts/extra_paths/var/anacron/daily.stamp": b"today\n", + }, + ) + + report, has_changes = compare_harvests( + str(old), str(new), exclude_paths=["/var/anacron"] + ) + assert has_changes is True + # File drift is filtered, but package drift remains. + assert report["files"]["changed"] == [] + assert report["packages"]["added"] == ["htop"] + + +def test_enforce_old_harvest_requires_ansible_playbook(monkeypatch, tmp_path: Path): + import enroll.diff as d + + monkeypatch.setattr(d.shutil, "which", lambda name: None) + + old = tmp_path / "old" + _write_bundle(old, {"inventory": {"packages": {}}, "roles": _minimal_roles()}) + + with pytest.raises(RuntimeError, match="ansible-playbook not found"): + d.enforce_old_harvest(str(old)) + + +def test_enforce_old_harvest_runs_ansible_with_tags_from_file_drift( + monkeypatch, tmp_path: Path +): + import enroll.diff as d + import enroll.manifest as mf + + # Pretend ansible-playbook is installed. + monkeypatch.setattr(d.shutil, "which", lambda name: "/usr/bin/ansible-playbook") + + calls: dict[str, object] = {} + + # Stub manifest generation to only create playbook.yml (fast, no real roles needed). + def fake_manifest(_harvest_dir: str, out_dir: str, **_kwargs): + out = Path(out_dir) + (out / "playbook.yml").write_text( + "---\n- hosts: all\n gather_facts: false\n roles: []\n", + encoding="utf-8", + ) + + monkeypatch.setattr(mf, "manifest", fake_manifest) + + def fake_run( + argv, cwd=None, env=None, capture_output=False, text=False, check=False + ): + calls["argv"] = list(argv) + calls["cwd"] = cwd + return types.SimpleNamespace(returncode=0, stdout="ok", stderr="") + + monkeypatch.setattr(d.subprocess, "run", fake_run) + + old = tmp_path / "old" + old_state = { + "schema_version": 3, + "host": {"hostname": "h1"}, + "inventory": {"packages": {}}, + "roles": { + **_minimal_roles(), + "usr_local_custom": { + **_minimal_roles()["usr_local_custom"], + "managed_files": [ + { + "path": "/etc/myapp.conf", + "src_rel": "etc/myapp.conf", + "owner": "root", + "group": "root", + "mode": "0644", + "reason": "custom", + } + ], + }, + }, + } + _write_bundle(old, old_state) + + # Minimal report containing enforceable drift: a baseline file is "removed". + report = { + "packages": {"added": [], "removed": [], "version_changed": []}, + "services": {"enabled_added": [], "enabled_removed": [], "changed": []}, + "users": {"added": [], "removed": [], "changed": []}, + "files": { + "added": [], + "removed": [{"path": "/etc/myapp.conf", "role": "usr_local_custom"}], + "changed": [], + }, + } + + info = d.enforce_old_harvest(str(old), report=report) + assert info["status"] == "applied" + assert "--tags" in info["command"] + assert "role_usr_local_custom" in ",".join(info.get("tags") or []) + + argv = calls.get("argv") + assert argv and argv[0].endswith("ansible-playbook") + assert "--tags" in argv + # Ensure we pass the computed tag. + i = argv.index("--tags") + assert "role_usr_local_custom" in str(argv[i + 1]) + + +def test_cli_diff_forwards_exclude_and_ignore_flags(monkeypatch, capsys): + import enroll.cli as cli + + calls: dict[str, object] = {} + + def fake_compare( + old, new, *, sops_mode=False, exclude_paths=None, ignore_package_versions=False + ): + calls["compare"] = { + "old": old, + "new": new, + "sops_mode": sops_mode, + "exclude_paths": exclude_paths, + "ignore_package_versions": ignore_package_versions, + } + # No changes -> should not try to enforce. + return {"packages": {}, "services": {}, "users": {}, "files": {}}, False + + monkeypatch.setattr(cli, "compare_harvests", fake_compare) + monkeypatch.setattr(cli, "format_report", lambda report, fmt="text": "R\n") + monkeypatch.setattr( + sys, + "argv", + [ + "enroll", + "diff", + "--old", + "/tmp/old", + "--new", + "/tmp/new", + "--exclude-path", + "/var/anacron", + "--ignore-package-versions", + ], + ) + + cli.main() + _ = capsys.readouterr() + assert calls["compare"]["exclude_paths"] == ["/var/anacron"] + assert calls["compare"]["ignore_package_versions"] is True + + +def test_cli_diff_enforce_skips_when_no_enforceable_drift(monkeypatch): + import enroll.cli as cli + + # Drift exists, but is not enforceable (only additions / version changes). + report = { + "packages": {"added": ["htop"], "removed": [], "version_changed": []}, + "services": { + "enabled_added": ["x.service"], + "enabled_removed": [], + "changed": [], + }, + "users": {"added": ["bob"], "removed": [], "changed": []}, + "files": {"added": [{"path": "/tmp/new"}], "removed": [], "changed": []}, + } + + monkeypatch.setattr(cli, "compare_harvests", lambda *a, **k: (report, True)) + monkeypatch.setattr(cli, "has_enforceable_drift", lambda r: False) + called = {"enforce": False} + monkeypatch.setattr( + cli, "enforce_old_harvest", lambda *a, **k: called.update({"enforce": True}) + ) + + captured = {} + + def fake_format(rep, fmt="text"): + captured["report"] = rep + return "R\n" + + monkeypatch.setattr(cli, "format_report", fake_format) + + monkeypatch.setattr( + sys, + "argv", + [ + "enroll", + "diff", + "--old", + "/tmp/old", + "--new", + "/tmp/new", + "--enforce", + ], + ) + + cli.main() + assert called["enforce"] is False + assert captured["report"]["enforcement"]["status"] == "skipped" diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 8b34fcb..073fd6d 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -206,11 +206,11 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): # Playbook should include users, etc_custom, packages, and services pb = (out / "playbook.yml").read_text(encoding="utf-8") - assert "- users" in pb - assert "- etc_custom" in pb - assert "- usr_local_custom" in pb - assert "- curl" in pb - assert "- foo" in pb + assert "role: users" in pb + assert "role: etc_custom" in pb + assert "role: usr_local_custom" in pb + assert "role: curl" in pb + assert "role: foo" in pb def test_manifest_site_mode_creates_host_inventory_and_raw_files(tmp_path: Path): @@ -490,7 +490,7 @@ def test_manifest_includes_dnf_config_role_when_present(tmp_path: Path): manifest.manifest(str(bundle), str(out)) pb = (out / "playbook.yml").read_text(encoding="utf-8") - assert "- dnf_config" in pb + assert "role: dnf_config" in pb tasks = (out / "roles" / "dnf_config" / "tasks" / "main.yml").read_text( encoding="utf-8" @@ -632,9 +632,9 @@ def test_manifest_orders_cron_and_logrotate_at_playbook_tail(tmp_path: Path): ] # Ensure tail ordering. - assert roles[-2:] == ["cron", "logrotate"] - assert "users" in roles - assert roles.index("users") < roles.index("cron") + assert roles[-2:] == ["role: cron", "role: logrotate"] + assert "role: users" in roles + assert roles.index("role: users") < roles.index("role: cron") def test_yaml_helpers_fallback_when_yaml_unavailable(monkeypatch):