More refactoring, support hiera and multi site mode for Puppet

tests/test_harvest.py

@@ -5,21 +5,28 @@ import pytest
 from pathlib import Path
 
 import enroll.harvest as harvest
+import enroll.system_paths as system_paths
 from enroll.platform import PlatformInfo
 from enroll.systemd import UnitInfo
 from enroll.pathfilter import PathFilter
-from enroll.harvest import (
-    _is_confish,
-    _hint_names,
-    _topdirs_for_package,
-    _iter_matching_files,
-    _parse_apt_signed_by,
-    _capture_link,
-    _capture_file,
-    ManagedFile,
-    ManagedLink,
-    ExcludedFile,
-    IgnorePolicy,
+import enroll.capture as capture
+from enroll.capture import (
+    capture_file as _capture_file,
+    capture_link as _capture_link,
+    capture_user_shell_dotfiles,
+    files_differ,
+)
+from enroll.harvest_types import ExcludedFile, ManagedFile, ManagedLink
+from enroll.ignore import IgnorePolicy
+from enroll.package_hints import (
+    add_pkgs_from_etc_topdirs,
+    hint_names as _hint_names,
+)
+from enroll.system_paths import (
+    is_confish as _is_confish,
+    iter_matching_files as _iter_matching_files,
+    parse_apt_signed_by as _parse_apt_signed_by,
+    topdirs_for_package as _topdirs_for_package,
 )
 
 from unittest.mock import MagicMock
@@ -249,6 +256,7 @@ def test_harvest_dedup_manual_packages_and_builds_etc_custom(
         return ("root", "root", "0644")
 
     monkeypatch.setattr(harvest, "stat_triplet", fake_stat_triplet)
+    monkeypatch.setattr(capture, "stat_triplet", fake_stat_triplet)
 
     # Avoid needing source files on disk by implementing our own bundle copier
     def fake_copy(bundle_dir: str, role_name: str, abs_path: str, src_rel: str):
@@ -256,7 +264,7 @@ def test_harvest_dedup_manual_packages_and_builds_etc_custom(
         dst.parent.mkdir(parents=True, exist_ok=True)
         dst.write_bytes(files.get(abs_path, b""))
 
-    monkeypatch.setattr(harvest, "_copy_into_bundle", fake_copy)
+    monkeypatch.setattr(capture, "copy_into_bundle", fake_copy)
 
     state_path = harvest.harvest(str(bundle), policy=AllowAllPolicy())
     st = json.loads(Path(state_path).read_text(encoding="utf-8"))
@@ -327,8 +335,8 @@ def test_shared_cron_snippet_prefers_matching_role_over_lexicographic(
 
     # Only include the cron snippet in the system capture set.
     monkeypatch.setattr(
-        harvest,
-        "_iter_system_capture_paths",
+        system_paths,
+        "iter_system_capture_paths",
         lambda: [("/etc/cron.d/ntpsec", "system_cron")],
     )
 
@@ -392,6 +400,7 @@ def test_shared_cron_snippet_prefers_matching_role_over_lexicographic(
     monkeypatch.setattr(harvest, "get_backend", lambda info=None: backend)
 
     monkeypatch.setattr(harvest, "stat_triplet", lambda p: ("root", "root", "0644"))
+    monkeypatch.setattr(capture, "stat_triplet", lambda p: ("root", "root", "0644"))
     monkeypatch.setattr(harvest, "collect_non_system_users", lambda: [])
 
     def fake_copy(bundle_dir: str, role_name: str, abs_path: str, src_rel: str):
@@ -399,7 +408,7 @@ def test_shared_cron_snippet_prefers_matching_role_over_lexicographic(
         dst.parent.mkdir(parents=True, exist_ok=True)
         dst.write_bytes(files[abs_path])
 
-    monkeypatch.setattr(harvest, "_copy_into_bundle", fake_copy)
+    monkeypatch.setattr(capture, "copy_into_bundle", fake_copy)
 
     state_path = harvest.harvest(str(bundle), policy=AllowAllPolicy())
     st = json.loads(Path(state_path).read_text(encoding="utf-8"))
@@ -421,7 +430,7 @@ def test_files_differ_binary(tmp_path: Path):
     file2 = tmp_path / "file2.bin"
     file1.write_bytes(b"\x00\x01\x02\x03")
     file2.write_bytes(b"\x00\x01\x02\x03")
-    assert harvest._files_differ(str(file1), str(file2)) is False
+    assert files_differ(str(file1), str(file2)) is False
 
 
 def test_files_differ_binary_different(tmp_path: Path):
@@ -429,7 +438,7 @@ def test_files_differ_binary_different(tmp_path: Path):
     file2 = tmp_path / "file2.bin"
     file1.write_bytes(b"\x00\x01\x02\x03")
     file2.write_bytes(b"\x00\x01\x02\x04")
-    assert harvest._files_differ(str(file1), str(file2)) is True
+    assert files_differ(str(file1), str(file2)) is True
 
 
 def test_files_differ_non_regular_a(tmp_path: Path):
@@ -437,14 +446,14 @@ def test_files_differ_non_regular_a(tmp_path: Path):
     directory.mkdir()
     file1 = tmp_path / "file1.txt"
     file1.write_text("content", encoding="utf-8")
-    assert harvest._files_differ(str(directory), str(file1)) is True
+    assert files_differ(str(directory), str(file1)) is True
 
 
 def test_topdirs_for_package_with_multiple_paths():
     pkg_to_etc_paths = {
         "nginx": ["/etc/nginx/nginx.conf", "/etc/nginx/sites-enabled/default"],
     }
-    result = harvest._topdirs_for_package("nginx", pkg_to_etc_paths)
+    result = _topdirs_for_package("nginx", pkg_to_etc_paths)
     assert result == {"nginx"}
 
 
@@ -452,12 +461,12 @@ def test_topdirs_for_package_with_multiple_topdirs():
     pkg_to_etc_paths = {
         "multi": ["/etc/nginx/nginx.conf", "/etc/ssh/sshd_config"],
     }
-    result = harvest._topdirs_for_package("multi", pkg_to_etc_paths)
+    result = _topdirs_for_package("multi", pkg_to_etc_paths)
     assert result == {"nginx", "ssh"}
 
 
 def test_topdirs_for_package_empty():
-    result = harvest._topdirs_for_package("empty", {})
+    result = _topdirs_for_package("empty", {})
     assert result == set()
 
 
@@ -465,7 +474,7 @@ def test_topdirs_for_package_no_etc():
     pkg_to_etc_paths = {
         "other": ["/usr/share/doc/file"],
     }
-    result = harvest._topdirs_for_package("other", pkg_to_etc_paths)
+    result = _topdirs_for_package("other", pkg_to_etc_paths)
     assert result == set()
 
 
@@ -475,7 +484,7 @@ def test_files_differ_same_content(tmp_path: Path):
     file_b = tmp_path / "b.txt"
     file_a.write_text("same content", encoding="utf-8")
     file_b.write_text("same content", encoding="utf-8")
-    assert harvest._files_differ(str(file_a), str(file_b)) is False
+    assert files_differ(str(file_a), str(file_b)) is False
 
 
 def test_files_differ_different_content(tmp_path: Path):
@@ -484,7 +493,7 @@ def test_files_differ_different_content(tmp_path: Path):
     file_b = tmp_path / "b.txt"
     file_a.write_text("content a", encoding="utf-8")
     file_b.write_text("content b", encoding="utf-8")
-    assert harvest._files_differ(str(file_a), str(file_b)) is True
+    assert files_differ(str(file_a), str(file_b)) is True
 
 
 def test_files_differ_missing_file(tmp_path: Path):
@@ -492,7 +501,7 @@ def test_files_differ_missing_file(tmp_path: Path):
     file_a = tmp_path / "a.txt"
     file_a.write_text("content", encoding="utf-8")
     file_b = tmp_path / "b.txt"
-    assert harvest._files_differ(str(file_a), str(file_b)) is True
+    assert files_differ(str(file_a), str(file_b)) is True
 
 
 def test_files_differ_both_missing(tmp_path: Path):
@@ -500,7 +509,7 @@ def test_files_differ_both_missing(tmp_path: Path):
     file_a = tmp_path / "a.txt"
     file_b = tmp_path / "b.txt"
     # Both missing - should return True (they differ in the sense that neither exists)
-    assert harvest._files_differ(str(file_a), str(file_b)) is True
+    assert files_differ(str(file_a), str(file_b)) is True
 
 
 def test_files_differ_non_regular_b(tmp_path: Path):
@@ -510,7 +519,7 @@ def test_files_differ_non_regular_b(tmp_path: Path):
     link_b = tmp_path / "link"
     link_b.symlink_to(file_a)
     # Symlinks are followed, so content is the same
-    assert harvest._files_differ(str(file_a), str(link_b)) is False
+    assert files_differ(str(file_a), str(link_b)) is False
 
 
 def test_files_differ_oserror_on_read(tmp_path: Path, monkeypatch):
@@ -524,7 +533,7 @@ def test_files_differ_oserror_on_read(tmp_path: Path, monkeypatch):
         raise OSError("Permission denied")
 
     monkeypatch.setattr("builtins.open", fake_open, raising=False)
-    assert harvest._files_differ(str(file_a), str(file_b)) is True
+    assert files_differ(str(file_a), str(file_b)) is True
 
 
 def test_files_differ_large_file_returns_true(tmp_path: Path):
@@ -536,7 +545,7 @@ def test_files_differ_large_file_returns_true(tmp_path: Path):
     file_a.write_bytes(data)
     file_b.write_bytes(data)
     # Should return True because files are too large
-    assert harvest._files_differ(str(file_a), str(file_b), max_bytes=1_000_000) is True
+    assert files_differ(str(file_a), str(file_b), max_bytes=1_000_000) is True
 
 
 def test_files_differ_size_mismatch(tmp_path: Path):
@@ -545,7 +554,7 @@ def test_files_differ_size_mismatch(tmp_path: Path):
     file_b = tmp_path / "b.txt"
     file_a.write_text("short", encoding="utf-8")
     file_b.write_text("much longer content here", encoding="utf-8")
-    assert harvest._files_differ(str(file_a), str(file_b)) is True
+    assert files_differ(str(file_a), str(file_b)) is True
 
 
 def test_files_differ_large_files(tmp_path: Path):
@@ -556,12 +565,12 @@ def test_files_differ_large_files(tmp_path: Path):
     data = b"x" * 10000
     file_a.write_bytes(data)
     file_b.write_bytes(data)
-    assert harvest._files_differ(str(file_a), str(file_b)) is False
+    assert files_differ(str(file_a), str(file_b)) is False
 
 
 def test_hint_names_with_unit_and_packages():
     """Test _hint_names extracts hints from unit and packages."""
-    result = harvest._hint_names("nginx.service", {"nginx-common", "nginx-core"})
+    result = _hint_names("nginx.service", {"nginx-common", "nginx-core"})
     assert "nginx" in result
     assert "nginx-common" in result
     assert "nginx-core" in result
@@ -569,20 +578,20 @@ def test_hint_names_with_unit_and_packages():
 
 def test_hint_names_with_template_unit():
     """Test _hint_names handles template units."""
-    result = harvest._hint_names("getty@tty1.service", set())
+    result = _hint_names("getty@tty1.service", set())
     assert "getty" in result
     assert "getty@tty1" in result
 
 
 def test_hint_names_with_dotted_unit():
     """Test _hint_names handles dotted unit names."""
-    result = harvest._hint_names("nginx.service", set())
+    result = _hint_names("nginx.service", set())
     assert "nginx" in result
 
 
 def test_hint_names_empty():
     """Test _hint_names with empty inputs."""
-    result = harvest._hint_names("", set())
+    result = _hint_names("", set())
     assert result == set()
 
 
@@ -594,7 +603,7 @@ def test_add_pkgs_from_etc_topdirs():
         "ssh": {"openssh-server"},
     }
     pkgs = set()
-    harvest._add_pkgs_from_etc_topdirs(hints, topdir_to_pkgs, pkgs)
+    add_pkgs_from_etc_topdirs(hints, topdir_to_pkgs, pkgs)
     # Should add packages from matching topdirs
     assert "nginx-common" in pkgs or "nginx-core" in pkgs
 
@@ -604,7 +613,7 @@ def test_add_pkgs_from_etc_topdirs_empty():
     hints = set()
     topdir_to_pkgs = {}
     pkgs = set()
-    harvest._add_pkgs_from_etc_topdirs(hints, topdir_to_pkgs, pkgs)
+    add_pkgs_from_etc_topdirs(hints, topdir_to_pkgs, pkgs)
     assert pkgs == set()
 
 
@@ -612,47 +621,47 @@ def test_is_confish_with_conf(tmp_path: Path):
     """Test _is_confish recognizes .conf files."""
     file1 = tmp_path / "test.conf"
     file1.write_text("[Unit]", encoding="utf-8")
-    assert harvest._is_confish(str(file1)) is True
+    assert _is_confish(str(file1)) is True
 
 
 def test_is_confish_with_yaml(tmp_path: Path):
     """Test _is_confish recognizes .yaml files."""
     file1 = tmp_path / "test.yaml"
     file1.write_text("key: value", encoding="utf-8")
-    assert harvest._is_confish(str(file1)) is True
+    assert _is_confish(str(file1)) is True
 
 
 def test_is_confish_with_json(tmp_path: Path):
     """Test _is_confish recognizes .json files."""
     file1 = tmp_path / "test.json"
     file1.write_text('{"key": "value"}', encoding="utf-8")
-    assert harvest._is_confish(str(file1)) is True
+    assert _is_confish(str(file1)) is True
 
 
 def test_is_confish_with_service(tmp_path: Path):
     """Test _is_confish recognizes .service files."""
     file1 = tmp_path / "test.service"
     file1.write_text("[Unit]", encoding="utf-8")
-    assert harvest._is_confish(str(file1)) is True
+    assert _is_confish(str(file1)) is True
 
 
 def test_is_confish_with_extensionless(tmp_path: Path):
     """Test _is_confish recognizes extensionless config files."""
     file1 = tmp_path / "default"
     file1.write_text("OPTIONS=", encoding="utf-8")
-    assert harvest._is_confish(str(file1)) is True
+    assert _is_confish(str(file1)) is True
 
 
 def test_is_confish_not_config(tmp_path: Path):
     """Test _is_confish rejects non-config files."""
     file1 = tmp_path / "test.log"
     file1.write_text("log", encoding="utf-8")
-    assert harvest._is_confish(str(file1)) is False
+    assert _is_confish(str(file1)) is False
 
 
 def test_is_confish_nonexistent():
     """Test _is_confish returns False for nonexistent files."""
-    assert harvest._is_confish("/nonexistent/file.xyz") is False
+    assert _is_confish("/nonexistent/file.xyz") is False
 
 
 """Additional coverage tests for harvest.py"""
@@ -1065,7 +1074,7 @@ def test_user_shell_dotfiles_are_not_auto_captured_without_dangerous(tmp_path: P
     managed: list[ManagedFile] = []
     excluded: list[ExcludedFile] = []
 
-    captured = harvest._capture_user_shell_dotfiles(
+    captured = capture_user_shell_dotfiles(
         bundle_dir=str(tmp_path / "bundle"),
         role_name="users",
         home=str(home),
@@ -1106,7 +1115,7 @@ def test_user_shell_dotfiles_dangerous_captures_changed_files_only(tmp_path: Pat
     managed: list[ManagedFile] = []
     excluded: list[ExcludedFile] = []
 
-    captured = harvest._capture_user_shell_dotfiles(
+    captured = capture_user_shell_dotfiles(
         bundle_dir=str(tmp_path / "bundle"),
         role_name="users",
         home=str(home),

tests/test_harvest_collectors.py

@@ -1,13 +1,10 @@
 from __future__ import annotations
 
-from enroll.harvest import (
-    FirewallRuntimeSnapshot,
-    HarvestContext,
-    IgnorePolicy,
-    PathFilter,
-    RuntimeStateCollector,
-    SysctlSnapshot,
-)
+from enroll.harvest_collectors.context import HarvestContext
+from enroll.harvest_collectors.runtime import RuntimeStateCollector
+from enroll.harvest_types import FirewallRuntimeSnapshot, SysctlSnapshot
+from enroll.ignore import IgnorePolicy
+from enroll.pathfilter import PathFilter
 
 
 class _Backend:

tests/test_harvest_cron_logrotate.py

@@ -4,6 +4,8 @@ import json
 from pathlib import Path
 
 import enroll.harvest as h
+import enroll.capture as capture
+import enroll.harvest_collectors.cron_logrotate as cron_logrotate
 from enroll.platform import PlatformInfo
 from enroll.systemd import UnitInfo
 
@@ -89,7 +91,7 @@ def test_harvest_unifies_cron_and_logrotate_into_dedicated_package_roles(
         }
         return list(mapping.get(spec, []))[:cap]
 
-    monkeypatch.setattr(h, "_iter_matching_files", fake_iter_matching)
+    monkeypatch.setattr(cron_logrotate, "iter_matching_files", fake_iter_matching)
 
     # Avoid real system probing.
     monkeypatch.setattr(
@@ -128,7 +130,7 @@ def test_harvest_unifies_cron_and_logrotate_into_dedicated_package_roles(
     )
     monkeypatch.setattr(h, "collect_non_system_users", lambda: [])
     monkeypatch.setattr(
-        h,
+        capture,
         "stat_triplet",
         lambda p: ("alice" if "alice" in p else "root", "root", "0644"),
     )
@@ -139,7 +141,7 @@ def test_harvest_unifies_cron_and_logrotate_into_dedicated_package_roles(
         dst.parent.mkdir(parents=True, exist_ok=True)
         dst.write_bytes(files.get(abs_path, b""))
 
-    monkeypatch.setattr(h, "_copy_into_bundle", fake_copy)
+    monkeypatch.setattr(capture, "copy_into_bundle", fake_copy)
 
     state_path = h.harvest(str(bundle), policy=AllowAllPolicy())
     st = json.loads(Path(state_path).read_text(encoding="utf-8"))

tests/test_harvest_helpers.py

@@ -4,6 +4,8 @@ import os
 from pathlib import Path
 
 import enroll.harvest as h
+import enroll.system_paths as sp
+from enroll.package_hints import role_name_from_pkg, role_name_from_unit
 
 
 def test_iter_matching_files_skips_symlinks_and_walks_dirs(monkeypatch, tmp_path: Path):
@@ -24,12 +26,12 @@ def test_iter_matching_files_skips_symlinks_and_walks_dirs(monkeypatch, tmp_path
         str(root / "link"): "link",
     }
 
-    monkeypatch.setattr(h.glob, "glob", lambda spec: [str(root), str(root / "link")])
-    monkeypatch.setattr(h.os.path, "islink", lambda p: paths.get(p) == "link")
-    monkeypatch.setattr(h.os.path, "isfile", lambda p: paths.get(p) == "file")
-    monkeypatch.setattr(h.os.path, "isdir", lambda p: paths.get(p) == "dir")
+    monkeypatch.setattr(sp.glob, "glob", lambda spec: [str(root), str(root / "link")])
+    monkeypatch.setattr(sp.os.path, "islink", lambda p: paths.get(p) == "link")
+    monkeypatch.setattr(sp.os.path, "isfile", lambda p: paths.get(p) == "file")
+    monkeypatch.setattr(sp.os.path, "isdir", lambda p: paths.get(p) == "dir")
     monkeypatch.setattr(
-        h.os,
+        sp.os,
         "walk",
         lambda p: [
             (str(root), ["sub"], ["real.txt", "link"]),
@@ -37,7 +39,7 @@ def test_iter_matching_files_skips_symlinks_and_walks_dirs(monkeypatch, tmp_path
         ],
     )
 
-    out = h._iter_matching_files("/whatever/*", cap=100)
+    out = sp.iter_matching_files("/whatever/*", cap=100)
     assert str(root / "real.txt") in out
     assert str(root / "sub" / "nested.txt") in out
     assert str(root / "link") not in out
@@ -57,7 +59,7 @@ def test_parse_apt_signed_by_extracts_keyrings(tmp_path: Path):
     f3 = tmp_path / "c.sources"
     f3.write_text("Signed-By: | /bin/echo nope\n", encoding="utf-8")
 
-    out = h._parse_apt_signed_by([str(f1), str(f2), str(f3)])
+    out = sp.parse_apt_signed_by([str(f1), str(f2), str(f3)])
     assert "/usr/share/keyrings/foo.gpg" in out
     assert "/etc/apt/keyrings/bar.gpg" in out
     assert "/usr/share/keyrings/baz.gpg" in out
@@ -74,9 +76,9 @@ def test_iter_apt_capture_paths_includes_signed_by_keyring(monkeypatch):
         "/usr/share/keyrings/ext.gpg": "file",
     }
 
-    monkeypatch.setattr(h.os.path, "isdir", lambda p: p in {"/etc/apt"})
+    monkeypatch.setattr(sp.os.path, "isdir", lambda p: p in {"/etc/apt"})
     monkeypatch.setattr(
-        h.os,
+        sp.os,
         "walk",
         lambda root: [
             ("/etc/apt", ["apt.conf.d", "sources.list.d"], []),
@@ -84,8 +86,8 @@ def test_iter_apt_capture_paths_includes_signed_by_keyring(monkeypatch):
             ("/etc/apt/sources.list.d", [], ["test.list"]),
         ],
     )
-    monkeypatch.setattr(h.os.path, "islink", lambda p: False)
-    monkeypatch.setattr(h.os.path, "isfile", lambda p: files.get(p) == "file")
+    monkeypatch.setattr(sp.os.path, "islink", lambda p: False)
+    monkeypatch.setattr(sp.os.path, "isfile", lambda p: files.get(p) == "file")
 
     # Only treat the sources glob as having a hit.
     def fake_iter_matching(spec: str, cap: int = 10000):
@@ -93,7 +95,7 @@ def test_iter_apt_capture_paths_includes_signed_by_keyring(monkeypatch):
             return ["/etc/apt/sources.list.d/test.list"]
         return []
 
-    monkeypatch.setattr(h, "_iter_matching_files", fake_iter_matching)
+    monkeypatch.setattr(sp, "iter_matching_files", fake_iter_matching)
 
     # Provide file contents for the sources file.
     real_open = open
@@ -105,10 +107,10 @@ def test_iter_apt_capture_paths_includes_signed_by_keyring(monkeypatch):
 
     # Easier: patch _parse_apt_signed_by directly to avoid filesystem reads.
     monkeypatch.setattr(
-        h, "_parse_apt_signed_by", lambda sfs: {"/usr/share/keyrings/ext.gpg"}
+        sp, "parse_apt_signed_by", lambda sfs: {"/usr/share/keyrings/ext.gpg"}
     )
 
-    out = h._iter_apt_capture_paths()
+    out = sp.iter_apt_capture_paths()
     paths = {p for p, _r in out}
     reasons = {p: r for p, r in out}
     assert "/etc/apt/apt.conf.d/00test" in paths
@@ -138,19 +140,23 @@ def test_iter_dnf_capture_paths(monkeypatch):
             return [("/etc/pki/rpm-gpg", [], ["RPM-GPG-KEY"])]
         return []
 
-    monkeypatch.setattr(h.os.path, "isdir", isdir)
-    monkeypatch.setattr(h.os, "walk", walk)
-    monkeypatch.setattr(h.os.path, "islink", lambda p: False)
-    monkeypatch.setattr(h.os.path, "isfile", lambda p: files.get(p) == "file")
-    monkeypatch.setattr(
-        h,
-        "_iter_matching_files",
-        lambda spec, cap=10000: (
-            ["/etc/yum.repos.d/test.repo"] if spec.endswith("*.repo") else []
-        ),
-    )
+    monkeypatch.setattr(sp.os.path, "isdir", isdir)
+    monkeypatch.setattr(sp.os, "walk", walk)
+    monkeypatch.setattr(sp.os.path, "islink", lambda p: False)
+    monkeypatch.setattr(sp.os.path, "isfile", lambda p: files.get(p) == "file")
 
-    out = h._iter_dnf_capture_paths()
+    def fake_iter_matching(spec: str, cap: int = 10000):
+        if spec == "/etc/yum.conf":
+            return ["/etc/yum.conf"]
+        if spec.endswith("*.repo"):
+            return ["/etc/yum.repos.d/test.repo"]
+        if spec == "/etc/pki/rpm-gpg/*":
+            return ["/etc/pki/rpm-gpg/RPM-GPG-KEY"]
+        return []
+
+    monkeypatch.setattr(sp, "iter_matching_files", fake_iter_matching)
+
+    out = sp.iter_dnf_capture_paths()
     paths = {p for p, _r in out}
     assert "/etc/dnf/dnf.conf" in paths
     assert "/etc/yum/yum.conf" in paths
@@ -160,13 +166,13 @@ def test_iter_dnf_capture_paths(monkeypatch):
 
 
 def test_iter_system_capture_paths_dedupes_first_reason(monkeypatch):
-    monkeypatch.setattr(h, "_SYSTEM_CAPTURE_GLOBS", [("/a", "r1"), ("/b", "r2")])
+    monkeypatch.setattr(sp, "_SYSTEM_CAPTURE_GLOBS", [("/a", "r1"), ("/b", "r2")])
     monkeypatch.setattr(
-        h,
-        "_iter_matching_files",
+        sp,
+        "iter_matching_files",
         lambda spec, cap=10000: ["/dup"] if spec in {"/a", "/b"} else [],
     )
-    out = h._iter_system_capture_paths()
+    out = sp.iter_system_capture_paths()
     assert out == [("/dup", "r1")]
 
 
@@ -289,20 +295,16 @@ def test_collect_firewall_runtime_snapshot_is_per_family_fallback(
 
 
 def test_package_role_names_do_not_collide_with_singleton_roles():
-    from enroll.harvest import _role_name_from_pkg
-
-    assert _role_name_from_pkg("flatpak") == "package_flatpak"
-    assert _role_name_from_pkg("snap") == "package_snap"
-    assert _role_name_from_pkg("users") == "package_users"
-    assert _role_name_from_pkg("nginx") == "nginx"
+    assert role_name_from_pkg("flatpak") == "package_flatpak"
+    assert role_name_from_pkg("snap") == "package_snap"
+    assert role_name_from_pkg("users") == "package_users"
+    assert role_name_from_pkg("nginx") == "nginx"
 
 
 def test_service_role_names_do_not_collide_with_singleton_roles():
-    from enroll.harvest import _role_name_from_unit
-
-    assert _role_name_from_unit("flatpak.service") == "service_flatpak"
-    assert _role_name_from_unit("users.service") == "service_users"
-    assert _role_name_from_unit("nginx.service") == "nginx"
+    assert role_name_from_unit("flatpak.service") == "service_flatpak"
+    assert role_name_from_unit("users.service") == "service_users"
+    assert role_name_from_unit("nginx.service") == "nginx"
 
 
 def test_parse_sysctl_a_output_keeps_persistable_values(monkeypatch):

tests/test_harvest_symlinks.py

@@ -2,6 +2,8 @@ import json
 from pathlib import Path
 
 import enroll.harvest as h
+import enroll.harvest_collectors.services as services
+import enroll.capture as capture
 from enroll.platform import PlatformInfo
 from enroll.systemd import UnitInfo
 
@@ -78,7 +80,7 @@ def _base_monkeypatches(monkeypatch, *, unit: str):
 
     # Avoid walking the real filesystem.
     monkeypatch.setattr(h.os, "walk", lambda root: iter(()))
-    monkeypatch.setattr(h, "_copy_into_bundle", lambda *a, **k: None)
+    monkeypatch.setattr(capture, "copy_into_bundle", lambda *a, **k: None)
 
     # Default to a "no files exist" view of the world unless a test overrides.
     monkeypatch.setattr(h.os.path, "isfile", lambda p: False)
@@ -119,7 +121,7 @@ def test_harvest_captures_nginx_enabled_symlinks(monkeypatch, tmp_path: Path):
             return ["/etc/nginx/modules-enabled/mod-http"]
         return []
 
-    monkeypatch.setattr(h.glob, "glob", fake_glob)
+    monkeypatch.setattr(services.glob, "glob", fake_glob)
 
     state_path = h.harvest(str(bundle), policy=AllowAllPolicy())
     st = json.loads(Path(state_path).read_text(encoding="utf-8"))
@@ -158,7 +160,7 @@ def test_harvest_does_not_capture_enabled_symlinks_without_role(
         },
     )
     monkeypatch.setattr(
-        h.glob, "glob", lambda pat: ["/etc/nginx/sites-enabled/default"]
+        services.glob, "glob", lambda pat: ["/etc/nginx/sites-enabled/default"]
     )
     monkeypatch.setattr(h.os.path, "islink", lambda p: True)
     monkeypatch.setattr(h.os, "readlink", lambda p: "../sites-available/default")
@@ -186,7 +188,7 @@ def test_harvest_symlink_capture_respects_ignore_policy(monkeypatch, tmp_path: P
     monkeypatch.setattr(h.os.path, "islink", lambda p: p in links)
     monkeypatch.setattr(h.os, "readlink", lambda p: links[p])
     monkeypatch.setattr(
-        h.glob,
+        services.glob,
         "glob",
         lambda pat: (
             sorted(list(links.keys())) if pat == "/etc/nginx/sites-enabled/*" else []
@@ -251,7 +253,7 @@ def test_harvest_captures_apache2_enabled_symlinks(monkeypatch, tmp_path: Path):
             return ["/etc/apache2/conf-enabled/security.conf"]
         return []
 
-    monkeypatch.setattr(h.glob, "glob", fake_glob)
+    monkeypatch.setattr(services.glob, "glob", fake_glob)
 
     state_path = h.harvest(str(bundle), policy=AllowAllPolicy())
     st = json.loads(Path(state_path).read_text(encoding="utf-8"))

tests/test_manifest_puppet.py

@@ -3,6 +3,8 @@ from __future__ import annotations
 import json
 from pathlib import Path
 
+import yaml
+
 from enroll import manifest
 
 
@@ -160,36 +162,55 @@ def test_manifest_puppet_writes_control_repo_style_output(tmp_path: Path):
     manifest.manifest(str(bundle), str(out), target="puppet", fqdn="test.example")
 
     site_pp = (out / "manifests" / "site.pp").read_text(encoding="utf-8")
-    assert site_pp == (
-        "node 'test.example' {\n"
-        "  include curl\n"
-        "  include foo\n"
-        "  include users\n"
-        "  include sysctl\n"
-        "}\n"
+    assert "node 'test.example' {" in site_pp
+    assert "lookup('enroll::classes'" in site_pp
+    assert "$enroll_classes.each" in site_pp
+    assert "include $enroll_class" in site_pp
+    assert "node default {" in site_pp
+
+    assert (out / "hiera.yaml").exists()
+    node_data = yaml.safe_load(
+        (out / "data" / "nodes" / "test.example.yaml").read_text(encoding="utf-8")
+    )
+    assert node_data["enroll::classes"] == ["curl", "foo", "users", "sysctl"]
+    assert node_data["curl::packages"] == ["curl"]
+    assert node_data["foo::packages"] == ["foo"]
+    assert node_data["foo::files"]["/etc/foo/foo.conf"]["source"] == (
+        "puppet:///modules/foo/nodes/test.example/etc/foo.conf"
+    )
+    assert node_data["foo::services"]["foo.service"] == {
+        "ensure": "running",
+        "enable": True,
+    }
+    assert node_data["users::users"]["alice"]["comment"] == "Alice Example"
+    assert node_data["users::users"]["alice"]["groups"] == ["docker"]
+    assert node_data["sysctl::files"]["/etc/sysctl.d/99-enroll.conf"]["source"] == (
+        "puppet:///modules/sysctl/nodes/test.example/sysctl/99-enroll.conf"
     )
 
     curl_pp = (out / "modules" / "curl" / "manifests" / "init.pp").read_text(
         encoding="utf-8"
     )
     assert "class curl" in curl_pp
-    assert "package { 'curl':" in curl_pp
+    assert "Array[String] $packages = []" in curl_pp
+    assert "package { $package_name:" in curl_pp
+    assert "package { 'curl':" not in curl_pp
 
     foo_pp = (out / "modules" / "foo" / "manifests" / "init.pp").read_text(
         encoding="utf-8"
     )
     assert "class foo" in foo_pp
-    assert "package { 'foo':" in foo_pp
-    assert "file { '/etc/foo/foo.conf':" in foo_pp
-    assert "source => 'puppet:///modules/foo/etc/foo.conf'" in foo_pp
-    assert "service { 'foo.service':" in foo_pp
+    assert "Hash[String, Hash] $files = {}" in foo_pp
+    assert "* => $attrs" in foo_pp
+    assert "package { 'foo':" not in foo_pp
+    assert "file { '/etc/foo/foo.conf':" not in foo_pp
 
     users_pp = (out / "modules" / "users" / "manifests" / "init.pp").read_text(
         encoding="utf-8"
     )
     assert "class users" in users_pp
-    assert "group { 'docker':" in users_pp
-    assert "user { 'alice':" in users_pp
+    assert "Hash[String, Hash] $users = {}" in users_pp
+    assert "user { 'alice':" not in users_pp
 
     sysctl_pp = (out / "modules" / "sysctl" / "manifests" / "init.pp").read_text(
         encoding="utf-8"
@@ -198,11 +219,162 @@ def test_manifest_puppet_writes_control_repo_style_output(tmp_path: Path):
     assert "Boolean $sysctl_apply = true" in sysctl_pp
     assert "Boolean $sysctl_ignore_apply_errors = true" in sysctl_pp
     assert "exec { 'enroll-apply-sysctl':" in sysctl_pp
-    assert "command     => $sysctl_ignore_apply_errors ? {" in sysctl_pp
-    assert "sysctl -e -p /etc/sysctl.d/99-enroll.conf || true" in sysctl_pp
+    assert "$files.has_key('/etc/sysctl.d/99-enroll.conf')" in sysctl_pp
 
-    assert (out / "modules" / "foo" / "files" / "etc" / "foo.conf").exists()
-    assert (out / "modules" / "sysctl" / "files" / "sysctl" / "99-enroll.conf").exists()
+    assert (
+        out
+        / "modules"
+        / "foo"
+        / "files"
+        / "nodes"
+        / "test.example"
+        / "etc"
+        / "foo.conf"
+    ).exists()
+    assert (
+        out
+        / "modules"
+        / "sysctl"
+        / "files"
+        / "nodes"
+        / "test.example"
+        / "sysctl"
+        / "99-enroll.conf"
+    ).exists()
+
+
+def test_manifest_puppet_fqdn_mode_can_accumulate_separate_node_data(
+    tmp_path: Path,
+):
+    out = tmp_path / "puppet"
+
+    def write_bundle(name: str, content: str) -> Path:
+        bundle = tmp_path / name
+        artifact = bundle / "artifacts" / "foo" / "etc" / "foo.conf"
+        artifact.parent.mkdir(parents=True, exist_ok=True)
+        artifact.write_text(content, encoding="utf-8")
+        _write_state(
+            bundle,
+            {
+                "schema_version": 3,
+                "host": {"hostname": name, "os": "debian", "pkg_backend": "dpkg"},
+                "inventory": {"packages": {}},
+                "roles": {
+                    "services": [
+                        {
+                            "unit": "foo.service",
+                            "role_name": "foo",
+                            "packages": ["foo"],
+                            "active_state": "active",
+                            "unit_file_state": "enabled",
+                            "managed_dirs": [],
+                            "managed_files": [
+                                {
+                                    "path": "/etc/foo/foo.conf",
+                                    "src_rel": "etc/foo.conf",
+                                    "owner": "root",
+                                    "group": "root",
+                                    "mode": "0644",
+                                }
+                            ],
+                            "managed_links": [],
+                        }
+                    ],
+                    "packages": [],
+                    "users": {
+                        "role_name": "users",
+                        "users": [],
+                        "managed_dirs": [],
+                        "managed_files": [],
+                    },
+                    "apt_config": {
+                        "role_name": "apt_config",
+                        "managed_dirs": [],
+                        "managed_files": [],
+                    },
+                    "dnf_config": {
+                        "role_name": "dnf_config",
+                        "managed_dirs": [],
+                        "managed_files": [],
+                    },
+                    "sysctl": {
+                        "role_name": "sysctl",
+                        "managed_dirs": [],
+                        "managed_files": [],
+                    },
+                    "firewall_runtime": {
+                        "role_name": "firewall_runtime",
+                        "packages": [],
+                    },
+                    "etc_custom": {
+                        "role_name": "etc_custom",
+                        "managed_dirs": [],
+                        "managed_files": [],
+                    },
+                    "usr_local_custom": {
+                        "role_name": "usr_local_custom",
+                        "managed_dirs": [],
+                        "managed_files": [],
+                    },
+                    "extra_paths": {
+                        "role_name": "extra_paths",
+                        "managed_dirs": [],
+                        "managed_files": [],
+                        "managed_links": [],
+                    },
+                },
+            },
+        )
+        return bundle
+
+    first = write_bundle("first", "first = true\n")
+    second = write_bundle("second", "second = true\n")
+
+    manifest.manifest(str(first), str(out), target="puppet", fqdn="first.example")
+    manifest.manifest(str(second), str(out), target="puppet", fqdn="second.example")
+
+    assert (out / "data" / "nodes" / "first.example.yaml").exists()
+    assert (out / "data" / "nodes" / "second.example.yaml").exists()
+
+    site_pp = (out / "manifests" / "site.pp").read_text(encoding="utf-8")
+    assert "node 'first.example' {" in site_pp
+    assert "node 'second.example' {" in site_pp
+
+    first_artifact = (
+        out
+        / "modules"
+        / "foo"
+        / "files"
+        / "nodes"
+        / "first.example"
+        / "etc"
+        / "foo.conf"
+    )
+    second_artifact = (
+        out
+        / "modules"
+        / "foo"
+        / "files"
+        / "nodes"
+        / "second.example"
+        / "etc"
+        / "foo.conf"
+    )
+    assert first_artifact.read_text(encoding="utf-8") == "first = true\n"
+    assert second_artifact.read_text(encoding="utf-8") == "second = true\n"
+
+    first_data = yaml.safe_load(
+        (out / "data" / "nodes" / "first.example.yaml").read_text(encoding="utf-8")
+    )
+    second_data = yaml.safe_load(
+        (out / "data" / "nodes" / "second.example.yaml").read_text(encoding="utf-8")
+    )
+    assert first_data["foo::files"]["/etc/foo/foo.conf"]["source"] == (
+        "puppet:///modules/foo/nodes/first.example/etc/foo.conf"
+    )
+    assert second_data["foo::files"]["/etc/foo/foo.conf"]["source"] == (
+        "puppet:///modules/foo/nodes/second.example/etc/foo.conf"
+    )
 
 
 def test_manifest_puppet_uses_default_node_and_common_package_modules(tmp_path: Path):