From e4887b7add36f3e926f7362e3e159fd9c523beeb Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Thu, 1 Jan 2026 11:02:30 +1100 Subject: [PATCH 01/48] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d075951..f4920b5 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ sudo apt update sudo apt install enroll ``` -### Fedora 42 +## Fedora ```bash sudo rpm --import https://mig5.net/static/mig5.asc From 09438246ae0557185c3343c0db6e0101f2d75385 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Thu, 1 Jan 2026 15:24:21 +1100 Subject: [PATCH 02/48] Build for Fedora 43 --- Dockerfile.rpmbuild | 8 +++++--- release.sh | 45 ++++++++++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/Dockerfile.rpmbuild b/Dockerfile.rpmbuild index c928cea..f76a673 100644 --- a/Dockerfile.rpmbuild +++ b/Dockerfile.rpmbuild @@ -1,5 +1,6 @@ # syntax=docker/dockerfile:1 -FROM fedora:42 +ARG BASE_IMAGE=fedora:42 +FROM ${BASE_IMAGE} RUN set -eux; \ dnf -y update; \ @@ -34,11 +35,12 @@ SRC="${SRC:-/src}" WORKROOT="${WORKROOT:-/work}" OUT="${OUT:-/out}" DEPS_DIR="${DEPS_DIR:-/deps}" - +VERSION_ID="$(grep VERSION_ID /etc/os-release | cut -d= -f2)" +echo "Version ID is ${VERSION_ID}" # Install jinjaturtle from local rpm # Filter out .src.rpm and debug* subpackages if present. if [ -d "${DEPS_DIR}" ] && compgen -G "${DEPS_DIR}/*.rpm" > /dev/null; then - mapfile -t rpms < <(ls -1 "${DEPS_DIR}"/*.rpm | grep -vE '(\.src\.rpm$|-(debuginfo|debugsource)-)') + mapfile -t rpms < <(ls -1 "${DEPS_DIR}"/*.rpm | grep -vE '(\.src\.rpm$|-(debuginfo|debugsource)-)' | grep "${VERSION_ID}") if [ "${#rpms[@]}" -gt 0 ]; then echo "Installing dependency RPMs from ${DEPS_DIR}:" printf ' - %s\n' "${rpms[@]}" diff --git a/release.sh b/release.sh index fdbe771..0a052c7 100755 --- a/release.sh +++ b/release.sh @@ -44,31 +44,46 @@ for dist in ${DISTS[@]}; do done # RPM -sudo apt-get -y install createrepo-c rpm -docker build -f Dockerfile.rpmbuild -t enroll:f42 --progress=plain . -docker run --rm -v "$PWD":/src -v "$PWD/dist/rpm":/out -v "$HOME/git/jinjaturtle/dist/rpm":/deps:ro enroll:f42 -sudo chown -R "${USER}" "$PWD/dist" - REPO_ROOT="${HOME}/git/repo_rpm" RPM_REPO="${REPO_ROOT}/rpm/x86_64" BUILD_OUTPUT="${HOME}/git/enroll/dist" REMOTE="letessier.mig5.net:/opt/repo_rpm" KEYID="00AE817C24A10C2540461A9C1D7CDE0234DB458D" -echo "==> Updating RPM repo..." mkdir -p "$RPM_REPO" +sudo apt-get -y install createrepo-c rpm -for file in `ls -1 "${BUILD_OUTPUT}/rpm"`; do - rpmsign --addsign "${BUILD_OUTPUT}/rpm/$file" +DISTS=( + fedora:43 + fedora:42 +) + +for dist in ${DISTS[@]}; do + release=$(echo ${dist} | cut -d: -f2) + docker build \ + --no-cache \ + -f Dockerfile.rpmbuild \ + -t enroll-rpm:${release} \ + --progress=plain \ + --build-arg BASE_IMAGE=${dist} \ + . + + docker run --rm -v "$PWD":/src -v "$PWD/dist/rpm":/out -v "$HOME/git/jinjaturtle/dist/rpm":/deps:ro enroll-rpm:${release} + sudo chown -R "${USER}" "$PWD/dist" + + echo "==> Updating RPM repo..." + for file in `ls -1 "${BUILD_OUTPUT}/rpm"`; do + rpmsign --addsign "${BUILD_OUTPUT}/rpm/$file" + done + + cp "${BUILD_OUTPUT}/rpm/"*.rpm "$RPM_REPO/" + + createrepo_c "$RPM_REPO" + + echo "==> Signing repomd.xml..." + qubes-gpg-client --local-user "$KEYID" --detach-sign --armor "$RPM_REPO/repodata/repomd.xml" > "$RPM_REPO/repodata/repomd.xml.asc" done -cp "${BUILD_OUTPUT}/rpm/"*.rpm "$RPM_REPO/" - -createrepo_c "$RPM_REPO" - -echo "==> Signing repomd.xml..." -qubes-gpg-client --local-user "$KEYID" --detach-sign --armor "$RPM_REPO/repodata/repomd.xml" > "$RPM_REPO/repodata/repomd.xml.asc" - echo "==> Syncing repo to server..." rsync -aHPvz --exclude=.git --delete "$REPO_ROOT/" "$REMOTE/" From 781efef4678d4ee1d176a264d62423aefe6680b6 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Fri, 2 Jan 2026 20:19:47 +1100 Subject: [PATCH 03/48] Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook --- CHANGELOG.md | 4 ++++ enroll/manifest.py | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49217f0..8283b5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.2.1 + + * Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook + # 0.2.0 * Add version CLI arg diff --git a/enroll/manifest.py b/enroll/manifest.py index bc629bb..839ebab 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -1551,8 +1551,6 @@ User-requested extra file harvesting. manifested_extra_paths_roles.append(role) - manifested_usr_local_custom_roles.append(role) - # ------------------------- # Service roles # ------------------------- From c88405ef01510b554846b55a5d3dd9593bb46352 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Fri, 2 Jan 2026 21:10:32 +1100 Subject: [PATCH 04/48] Ensure directories in the tree of anything included with --include are defined in the state and manifest so we make dirs before we try to create files --- CHANGELOG.md | 1 + enroll/fsutil.py | 2 +- enroll/harvest.py | 77 ++++++++++++++++++++++++++++++++++++++++++++++ enroll/ignore.py | 30 ++++++++++++++++++ enroll/manifest.py | 65 +++++++++++++++++++++++++++++++++++--- 5 files changed, 170 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8283b5b..3c41210 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # 0.2.1 * Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook + * Ensure directories in the tree of anything included with --include are defined in the state and manifest so we make dirs before we try to create files # 0.2.0 diff --git a/enroll/fsutil.py b/enroll/fsutil.py index 3d18df6..c852b9e 100644 --- a/enroll/fsutil.py +++ b/enroll/fsutil.py @@ -24,7 +24,7 @@ def stat_triplet(path: str) -> Tuple[str, str, str]: mode is a zero-padded octal string (e.g. "0644"). """ st = os.stat(path, follow_symlinks=True) - mode = oct(st.st_mode & 0o777)[2:].zfill(4) + mode = oct(st.st_mode & 0o7777)[2:].zfill(4) import grp import pwd diff --git a/enroll/harvest.py b/enroll/harvest.py index 74ac516..98e1404 100644 --- a/enroll/harvest.py +++ b/enroll/harvest.py @@ -34,6 +34,15 @@ class ManagedFile: reason: str +@dataclass +class ManagedDir: + path: str + owner: str + group: str + mode: str + reason: str + + @dataclass class ExcludedFile: path: str @@ -109,6 +118,7 @@ class ExtraPathsSnapshot: role_name: str include_patterns: List[str] exclude_patterns: List[str] + managed_dirs: List[ManagedDir] managed_files: List[ManagedFile] excluded: List[ExcludedFile] notes: List[str] @@ -1484,12 +1494,78 @@ def harvest( extra_notes: List[str] = [] extra_excluded: List[ExcludedFile] = [] extra_managed: List[ManagedFile] = [] + extra_managed_dirs: List[ManagedDir] = [] + extra_dir_seen: Set[str] = set() + + def _walk_and_capture_dirs(root: str) -> None: + root = os.path.normpath(root) + if not root.startswith("/"): + root = "/" + root + if not os.path.isdir(root) or os.path.islink(root): + return + for dirpath, dirnames, _ in os.walk(root, followlinks=False): + if len(extra_managed_dirs) >= MAX_FILES_CAP: + extra_notes.append( + f"Reached directory cap ({MAX_FILES_CAP}) while scanning {root}." + ) + return + dirpath = os.path.normpath(dirpath) + if not dirpath.startswith("/"): + dirpath = "/" + dirpath + if path_filter.is_excluded(dirpath): + # Prune excluded subtrees. + dirnames[:] = [] + continue + if os.path.islink(dirpath) or not os.path.isdir(dirpath): + dirnames[:] = [] + continue + + if dirpath not in extra_dir_seen: + deny = policy.deny_reason_dir(dirpath) + if not deny: + try: + owner, group, mode = stat_triplet(dirpath) + extra_managed_dirs.append( + ManagedDir( + path=dirpath, + owner=owner, + group=group, + mode=mode, + reason="user_include_dir", + ) + ) + except OSError: + pass + extra_dir_seen.add(dirpath) + + # Prune excluded dirs and symlinks early. + pruned: List[str] = [] + for d in dirnames: + p = os.path.join(dirpath, d) + if os.path.islink(p) or path_filter.is_excluded(p): + continue + pruned.append(d) + dirnames[:] = pruned + extra_role_name = "extra_paths" extra_role_seen = seen_by_role.setdefault(extra_role_name, set()) include_specs = list(include_paths or []) exclude_specs = list(exclude_paths or []) + # If any include pattern points at a directory, capture that directory tree's + # ownership/mode so the manifest can recreate it accurately. + include_pats = path_filter.iter_include_patterns() + for pat in include_pats: + if pat.kind == "prefix": + p = pat.value + if os.path.isdir(p) and not os.path.islink(p): + _walk_and_capture_dirs(p) + elif pat.kind == "glob": + for h in glob.glob(pat.value, recursive=True): + if os.path.isdir(h) and not os.path.islink(h): + _walk_and_capture_dirs(h) + if include_specs: extra_notes.append("User include patterns:") extra_notes.extend([f"- {p}" for p in include_specs]) @@ -1529,6 +1605,7 @@ def harvest( role_name=extra_role_name, include_patterns=include_specs, exclude_patterns=exclude_specs, + managed_dirs=extra_managed_dirs, managed_files=extra_managed, excluded=extra_excluded, notes=extra_notes, diff --git a/enroll/ignore.py b/enroll/ignore.py index 904997f..895c030 100644 --- a/enroll/ignore.py +++ b/enroll/ignore.py @@ -137,3 +137,33 @@ class IgnorePolicy: return "sensitive_content" return None + + def deny_reason_dir(self, path: str) -> Optional[str]: + """Directory-specific deny logic. + + deny_reason() is file-oriented (it rejects directories as "not_regular_file"). + For directory metadata capture (so roles can recreate directory trees), we need + a lighter-weight check: + - apply deny_globs (unless dangerous) + - require the path to be a real directory (no symlink) + - ensure it's stat'able/readable + + No size checks or content scanning are performed for directories. + """ + if not self.dangerous: + for g in self.deny_globs or []: + if fnmatch.fnmatch(path, g): + return "denied_path" + + try: + os.stat(path, follow_symlinks=True) + except OSError: + return "unreadable" + + if os.path.islink(path): + return "symlink" + + if not os.path.isdir(path): + return "not_directory" + + return None diff --git a/enroll/manifest.py b/enroll/manifest.py index 839ebab..a373773 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -344,6 +344,29 @@ def _write_role_defaults(role_dir: str, mapping: Dict[str, Any]) -> None: f.write(out) +def _build_managed_dirs_var( + managed_dirs: List[Dict[str, Any]], +) -> List[Dict[str, Any]]: + """Convert enroll managed_dirs into an Ansible-friendly list of dicts. + + Each dict drives a role task loop and is safe across hosts. + """ + out: List[Dict[str, Any]] = [] + for d in managed_dirs: + dest = d.get("path") or "" + if not dest: + continue + out.append( + { + "dest": dest, + "owner": d.get("owner") or "root", + "group": d.get("group") or "root", + "mode": d.get("mode") or "0755", + } + ) + return out + + def _build_managed_files_var( managed_files: List[Dict[str, Any]], templated_src_rels: Set[str], @@ -390,7 +413,22 @@ def _render_generic_files_tasks( # Using first_found makes roles work in both modes: # - site-mode: inventory/host_vars///.files/... # - non-site: roles//files/... - return f"""- name: Deploy any systemd unit files (templates) + return f"""- name: Ensure managed directories exist (preserve owner/group/mode) + ansible.builtin.file: + path: "{{{{ item.dest }}}}" + state: directory + owner: "{{{{ item.owner }}}}" + group: "{{{{ item.group }}}}" + mode: "{{{{ item.mode }}}}" + loop: "{{{{ {var_prefix}_managed_dirs | default([]) }}}}" + +- name: Ensure destination directories exist + ansible.builtin.file: + path: "{{{{ item.dest | dirname }}}}" + state: directory + loop: "{{{{ {var_prefix}_managed_files | default([]) }}}}" + +- name: Deploy any systemd unit files (templates) ansible.builtin.template: src: "{{{{ item.src_rel }}}}.j2" dest: "{{{{ item.dest }}}}" @@ -1444,13 +1482,17 @@ Unowned /etc config files not attributed to packages or services. # ------------------------- # extra_paths role (user-requested includes) # ------------------------- - if extra_paths_snapshot and extra_paths_snapshot.get("managed_files"): + if extra_paths_snapshot and ( + extra_paths_snapshot.get("managed_files") + or extra_paths_snapshot.get("managed_dirs") + ): role = extra_paths_snapshot.get("role_name", "extra_paths") role_dir = os.path.join(roles_root, role) _write_role_scaffold(role_dir) var_prefix = role + managed_dirs = extra_paths_snapshot.get("managed_dirs", []) or [] managed_files = extra_paths_snapshot.get("managed_files", []) excluded = extra_paths_snapshot.get("excluded", []) notes = extra_paths_snapshot.get("notes", []) @@ -1489,12 +1531,23 @@ Unowned /etc config files not attributed to packages or services. notify_systemd=None, ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} - vars_map: Dict[str, Any] = {f"{var_prefix}_managed_files": files_var} + vars_map: Dict[str, Any] = { + f"{var_prefix}_managed_dirs": dirs_var, + f"{var_prefix}_managed_files": files_var, + } vars_map = _merge_mappings_overwrite(vars_map, jt_map) if site_mode: - _write_role_defaults(role_dir, {f"{var_prefix}_managed_files": []}) + _write_role_defaults( + role_dir, + { + f"{var_prefix}_managed_dirs": [], + f"{var_prefix}_managed_files": [], + }, + ) _write_hostvars(out_dir, fqdn or "", role, vars_map) else: _write_role_defaults(role_dir, vars_map) @@ -1530,6 +1583,10 @@ User-requested extra file harvesting. """ + ("\n".join([f"- {p}" for p in exclude_pats]) or "- (none)") + """\n +## Managed directories +""" + + ("\n".join([f"- {d.get('path')}" for d in managed_dirs]) or "- (none)") + + """\n ## Managed files """ + ("\n".join([f"- {mf.get('path')}" for mf in managed_files]) or "- (none)") From 29b52d451d4d477ea2f9d05fdc5c85fe8f8ecd16 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Fri, 2 Jan 2026 21:29:16 +1100 Subject: [PATCH 05/48] 0.2.1 --- debian/changelog | 7 +++++++ pyproject.toml | 2 +- rpm/enroll.spec | 5 ++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/debian/changelog b/debian/changelog index f050e7f..dbc7548 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +enroll (0.2.1) unstable; urgency=medium + + * Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook + * Ensure directories in the tree of anything included with --include are defined in the state and manifest so we make dirs before we try to create files + + -- Miguel Jacq Fri, 01 Jan 2026 21:30:00 +1100 + enroll (0.2.0) unstable; urgency=medium * Add version CLI arg diff --git a/pyproject.toml b/pyproject.toml index 683a9b2..34f411e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "enroll" -version = "0.2.0" +version = "0.2.1" description = "Enroll a server's running state retrospectively into Ansible" authors = ["Miguel Jacq "] license = "GPL-3.0-or-later" diff --git a/rpm/enroll.spec b/rpm/enroll.spec index 3beac03..8fc8cac 100644 --- a/rpm/enroll.spec +++ b/rpm/enroll.spec @@ -1,4 +1,4 @@ -%global upstream_version 0.2.0 +%global upstream_version 0.2.1 Name: enroll Version: %{upstream_version} @@ -43,6 +43,9 @@ Enroll a server's running state retrospectively into Ansible. %{_bindir}/enroll %changelog +* Fri Jan 01 2026 Miguel Jacq - %{version}-%{release} +- Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook +- Ensure directories in the tree of anything included with --include are defined in the state and manifest so we make dirs before we try to create files * Mon Dec 29 2025 Miguel Jacq - %{version}-%{release} - Add version CLI arg - Add ability to enroll RH-style systems (DNF5/DNF/RPM) From 824010b2ab15865b0c1845d8cc9e67a80c7accf2 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 3 Jan 2026 11:39:57 +1100 Subject: [PATCH 06/48] Several bug fixes and prep for 0.2.2 - Fix stat() of parent directory so that we set directory perms correct on --include paths. - Set pty for remote calls when sudo is required, to help systems with limits on sudo without pty --- CHANGELOG.md | 5 ++ README.md | 2 +- debian/changelog | 7 ++ enroll/harvest.py | 171 +++++++++++++++++++++++++++++++++++++++------ enroll/manifest.py | 68 ++++++++++++++---- enroll/remote.py | 34 +++++---- pyproject.toml | 2 +- release.sh | 16 ++--- rpm/enroll.spec | 5 +- 9 files changed, 249 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c41210..0740cb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.2.2 + + * Fix stat() of parent directory so that we set directory perms correct on --include paths. + * Set pty for remote calls when sudo is required, to help systems with limits on sudo without pty + # 0.2.1 * Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook diff --git a/README.md b/README.md index f4920b5..e399633 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,7 @@ sudo rpm --import https://mig5.net/static/mig5.asc sudo tee /etc/yum.repos.d/mig5.repo > /dev/null << 'EOF' [mig5] name=mig5 Repository -baseurl=https://rpm.mig5.net/rpm/$basearch +baseurl=https://rpm.mig5.net/rpm/$releasever/$basearch enabled=1 gpgcheck=1 repo_gpgcheck=1 diff --git a/debian/changelog b/debian/changelog index dbc7548..8c2f4b9 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +enroll (0.2.2) unstable; urgency=medium + + * Fix stat() of parent directory so that we set directory perms correct on --include paths. + * Set pty for remote calls when sudo is required, to help systems with limits on sudo without pty + + -- Miguel Jacq Sat, 02 Jan 2026 09:56:00 +1100 + enroll (0.2.1) unstable; urgency=medium * Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook diff --git a/enroll/harvest.py b/enroll/harvest.py index 98e1404..7aba7c6 100644 --- a/enroll/harvest.py +++ b/enroll/harvest.py @@ -6,7 +6,7 @@ import os import re import shutil import time -from dataclasses import dataclass, asdict +from dataclasses import dataclass, asdict, field from typing import Dict, List, Optional, Set from .systemd import ( @@ -58,59 +58,66 @@ class ServiceSnapshot: sub_state: Optional[str] unit_file_state: Optional[str] condition_result: Optional[str] - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass class PackageSnapshot: package: str role_name: str - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass class UsersSnapshot: role_name: str users: List[dict] - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass class AptConfigSnapshot: role_name: str - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass class DnfConfigSnapshot: role_name: str - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass class EtcCustomSnapshot: role_name: str - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass class UsrLocalCustomSnapshot: role_name: str - managed_files: List[ManagedFile] - excluded: List[ExcludedFile] - notes: List[str] + managed_dirs: List[ManagedDir] = field(default_factory=list) + managed_files: List[ManagedFile] = field(default_factory=list) + excluded: List[ExcludedFile] = field(default_factory=list) + notes: List[str] = field(default_factory=list) @dataclass @@ -149,6 +156,71 @@ ALLOWED_UNOWNED_EXTS = { MAX_FILES_CAP = 4000 MAX_UNOWNED_FILES_PER_ROLE = 500 + +def _merge_parent_dirs( + existing_dirs: List[ManagedDir], + managed_files: List[ManagedFile], + *, + policy: IgnorePolicy, +) -> List[ManagedDir]: + """Ensure parent directories for managed_files are present in managed_dirs. + + This is used so the Ansible manifest can create destination directories with + explicit owner/group/mode (ansible-lint friendly) without needing a separate + "mkdir without perms" task. + + We only add the immediate parent directory for each managed file. For + explicit directory includes (extra_paths), existing_dirs will already + contain the walked directory tree. + """ + by_path: Dict[str, ManagedDir] = { + d.path: d for d in (existing_dirs or []) if d.path + } + + for mf in managed_files or []: + p = str(mf.path or "").rstrip("/") + if not p: + continue + dpath = os.path.dirname(p) + if not dpath or dpath == "/": + continue + if dpath in by_path: + continue + + # Directory-deny logic: newer IgnorePolicy implementations provide + # deny_reason_dir(). Older/simple policies (including unit tests) may + # only implement deny_reason(), which is file-oriented and may return + # "not_regular_file" for directories. + deny = None + deny_dir = getattr(policy, "deny_reason_dir", None) + if callable(deny_dir): + deny = deny_dir(dpath) + else: + deny = policy.deny_reason(dpath) + if deny in ("not_regular_file", "not_file", "not_regular"): + deny = None + if deny: + # If the file itself was captured, its parent directory is likely safe, + # but still respect deny globs for directories to avoid managing + # sensitive/forbidden trees. + continue + + try: + owner, group, mode = stat_triplet(dpath) + except OSError: + continue + + by_path[dpath] = ManagedDir( + path=dpath, + owner=owner, + group=group, + mode=mode, + reason="parent_of_managed_file", + ) + + return [by_path[k] for k in sorted(by_path)] + + # Directories that are shared across many packages. # Never attribute all unowned files in these trees # to one single package. @@ -1521,7 +1593,14 @@ def harvest( continue if dirpath not in extra_dir_seen: - deny = policy.deny_reason_dir(dirpath) + deny = None + deny_dir = getattr(policy, "deny_reason_dir", None) + if callable(deny_dir): + deny = deny_dir(dirpath) + else: + deny = policy.deny_reason(dirpath) + if deny in ("not_regular_file", "not_file", "not_regular"): + deny = None if not deny: try: owner, group, mode = stat_triplet(dirpath) @@ -1661,6 +1740,52 @@ def harvest( "roles": roles, } + # Ensure every role has explicit managed_dirs for parent directories of managed files. + # This lets the manifest create directories with owner/group/mode (ansible-lint friendly) + # without a separate "mkdir without perms" task. + users_snapshot.managed_dirs = _merge_parent_dirs( + users_snapshot.managed_dirs, users_snapshot.managed_files, policy=policy + ) + for s in service_snaps: + s.managed_dirs = _merge_parent_dirs( + s.managed_dirs, s.managed_files, policy=policy + ) + for p in pkg_snaps: + p.managed_dirs = _merge_parent_dirs( + p.managed_dirs, p.managed_files, policy=policy + ) + + if apt_config_snapshot: + apt_config_snapshot.managed_dirs = _merge_parent_dirs( + apt_config_snapshot.managed_dirs, + apt_config_snapshot.managed_files, + policy=policy, + ) + if dnf_config_snapshot: + dnf_config_snapshot.managed_dirs = _merge_parent_dirs( + dnf_config_snapshot.managed_dirs, + dnf_config_snapshot.managed_files, + policy=policy, + ) + if etc_custom_snapshot: + etc_custom_snapshot.managed_dirs = _merge_parent_dirs( + etc_custom_snapshot.managed_dirs, + etc_custom_snapshot.managed_files, + policy=policy, + ) + if usr_local_custom_snapshot: + usr_local_custom_snapshot.managed_dirs = _merge_parent_dirs( + usr_local_custom_snapshot.managed_dirs, + usr_local_custom_snapshot.managed_files, + policy=policy, + ) + if extra_paths_snapshot: + extra_paths_snapshot.managed_dirs = _merge_parent_dirs( + extra_paths_snapshot.managed_dirs, + extra_paths_snapshot.managed_files, + policy=policy, + ) + state = { "enroll": { "version": get_enroll_version(), diff --git a/enroll/manifest.py b/enroll/manifest.py index a373773..f30e5f3 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -422,12 +422,6 @@ def _render_generic_files_tasks( mode: "{{{{ item.mode }}}}" loop: "{{{{ {var_prefix}_managed_dirs | default([]) }}}}" -- name: Ensure destination directories exist - ansible.builtin.file: - path: "{{{{ item.dest | dirname }}}}" - state: directory - loop: "{{{{ {var_prefix}_managed_files | default([]) }}}}" - - name: Deploy any systemd unit files (templates) ansible.builtin.template: src: "{{{{ item.src_rel }}}}.j2" @@ -983,6 +977,7 @@ Generated non-system user accounts and SSH public material. var_prefix = role managed_files = apt_config_snapshot.get("managed_files", []) + managed_dirs = apt_config_snapshot.get("managed_dirs", []) or [] excluded = apt_config_snapshot.get("excluded", []) notes = apt_config_snapshot.get("notes", []) @@ -1019,12 +1014,20 @@ Generated non-system user accounts and SSH public material. notify_systemd=None, ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} - vars_map: Dict[str, Any] = {f"{var_prefix}_managed_files": files_var} + vars_map: Dict[str, Any] = { + f"{var_prefix}_managed_files": files_var, + f"{var_prefix}_managed_dirs": dirs_var, + } vars_map = _merge_mappings_overwrite(vars_map, jt_map) if site_mode: - _write_role_defaults(role_dir, {f"{var_prefix}_managed_files": []}) + _write_role_defaults( + role_dir, + {f"{var_prefix}_managed_files": [], f"{var_prefix}_managed_dirs": []}, + ) _write_hostvars(out_dir, fqdn or "", role, vars_map) else: _write_role_defaults(role_dir, vars_map) @@ -1134,6 +1137,7 @@ APT configuration harvested from the system (sources, pinning, and keyrings). var_prefix = role managed_files = dnf_config_snapshot.get("managed_files", []) + managed_dirs = dnf_config_snapshot.get("managed_dirs", []) or [] excluded = dnf_config_snapshot.get("excluded", []) notes = dnf_config_snapshot.get("notes", []) @@ -1169,12 +1173,20 @@ APT configuration harvested from the system (sources, pinning, and keyrings). notify_systemd=None, ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} - vars_map: Dict[str, Any] = {f"{var_prefix}_managed_files": files_var} + vars_map: Dict[str, Any] = { + f"{var_prefix}_managed_files": files_var, + f"{var_prefix}_managed_dirs": dirs_var, + } vars_map = _merge_mappings_overwrite(vars_map, jt_map) if site_mode: - _write_role_defaults(role_dir, {f"{var_prefix}_managed_files": []}) + _write_role_defaults( + role_dir, + {f"{var_prefix}_managed_files": [], f"{var_prefix}_managed_dirs": []}, + ) _write_hostvars(out_dir, fqdn or "", role, vars_map) else: _write_role_defaults(role_dir, vars_map) @@ -1285,6 +1297,7 @@ DNF/YUM configuration harvested from the system (repos, config files, and RPM GP var_prefix = role managed_files = etc_custom_snapshot.get("managed_files", []) + managed_dirs = etc_custom_snapshot.get("managed_dirs", []) or [] excluded = etc_custom_snapshot.get("excluded", []) notes = etc_custom_snapshot.get("notes", []) @@ -1321,12 +1334,20 @@ DNF/YUM configuration harvested from the system (repos, config files, and RPM GP notify_systemd="Run systemd daemon-reload", ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} - vars_map: Dict[str, Any] = {f"{var_prefix}_managed_files": files_var} + vars_map: Dict[str, Any] = { + f"{var_prefix}_managed_files": files_var, + f"{var_prefix}_managed_dirs": dirs_var, + } vars_map = _merge_mappings_overwrite(vars_map, jt_map) if site_mode: - _write_role_defaults(role_dir, {f"{var_prefix}_managed_files": []}) + _write_role_defaults( + role_dir, + {f"{var_prefix}_managed_files": [], f"{var_prefix}_managed_dirs": []}, + ) _write_hostvars(out_dir, fqdn or "", role, vars_map) else: _write_role_defaults(role_dir, vars_map) @@ -1395,6 +1416,7 @@ Unowned /etc config files not attributed to packages or services. var_prefix = role managed_files = usr_local_custom_snapshot.get("managed_files", []) + managed_dirs = usr_local_custom_snapshot.get("managed_dirs", []) or [] excluded = usr_local_custom_snapshot.get("excluded", []) notes = usr_local_custom_snapshot.get("notes", []) @@ -1431,12 +1453,20 @@ Unowned /etc config files not attributed to packages or services. notify_systemd=None, ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} - vars_map: Dict[str, Any] = {f"{var_prefix}_managed_files": files_var} + vars_map: Dict[str, Any] = { + f"{var_prefix}_managed_files": files_var, + f"{var_prefix}_managed_dirs": dirs_var, + } vars_map = _merge_mappings_overwrite(vars_map, jt_map) if site_mode: - _write_role_defaults(role_dir, {f"{var_prefix}_managed_files": []}) + _write_role_defaults( + role_dir, + {f"{var_prefix}_managed_files": [], f"{var_prefix}_managed_dirs": []}, + ) _write_hostvars(out_dir, fqdn or "", role, vars_map) else: _write_role_defaults(role_dir, vars_map) @@ -1616,6 +1646,7 @@ User-requested extra file harvesting. unit = svc["unit"] pkgs = svc.get("packages", []) or [] managed_files = svc.get("managed_files", []) or [] + managed_dirs = svc.get("managed_dirs", []) or [] role_dir = os.path.join(roles_root, role) _write_role_scaffold(role_dir) @@ -1660,11 +1691,14 @@ User-requested extra file harvesting. notify_systemd="Run systemd daemon-reload", ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} base_vars: Dict[str, Any] = { f"{var_prefix}_unit_name": unit, f"{var_prefix}_packages": pkgs, f"{var_prefix}_managed_files": files_var, + f"{var_prefix}_managed_dirs": dirs_var, f"{var_prefix}_manage_unit": True, f"{var_prefix}_systemd_enabled": bool(enabled_at_harvest), f"{var_prefix}_systemd_state": desired_state, @@ -1679,6 +1713,7 @@ User-requested extra file harvesting. f"{var_prefix}_unit_name": unit, f"{var_prefix}_packages": [], f"{var_prefix}_managed_files": [], + f"{var_prefix}_managed_dirs": [], f"{var_prefix}_manage_unit": False, f"{var_prefix}_systemd_enabled": False, f"{var_prefix}_systemd_state": "stopped", @@ -1782,6 +1817,7 @@ Generated from `{unit}`. role = pr["role_name"] pkg = pr.get("package") or "" managed_files = pr.get("managed_files", []) or [] + managed_dirs = pr.get("managed_dirs", []) or [] role_dir = os.path.join(roles_root, role) _write_role_scaffold(role_dir) @@ -1823,10 +1859,13 @@ Generated from `{unit}`. notify_systemd="Run systemd daemon-reload", ) + dirs_var = _build_managed_dirs_var(managed_dirs) + jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} base_vars: Dict[str, Any] = { f"{var_prefix}_packages": pkgs, f"{var_prefix}_managed_files": files_var, + f"{var_prefix}_managed_dirs": dirs_var, } base_vars = _merge_mappings_overwrite(base_vars, jt_map) @@ -1836,6 +1875,7 @@ Generated from `{unit}`. { f"{var_prefix}_packages": [], f"{var_prefix}_managed_files": [], + f"{var_prefix}_managed_dirs": [], }, ) _write_hostvars(out_dir, fqdn or "", role, base_vars) diff --git a/enroll/remote.py b/enroll/remote.py index 9618512..b86cd08 100644 --- a/enroll/remote.py +++ b/enroll/remote.py @@ -16,7 +16,6 @@ def _safe_extract_tar(tar: tarfile.TarFile, dest: Path) -> None: Protects against path traversal (e.g. entries containing ../). """ - # Note: tar member names use POSIX separators regardless of platform. dest = dest.resolve() @@ -80,9 +79,18 @@ def _build_enroll_pyz(tmpdir: Path) -> Path: return pyz_path -def _ssh_run(ssh, cmd: str) -> tuple[int, str, str]: - """Run a command over a Paramiko SSHClient.""" - _stdin, stdout, stderr = ssh.exec_command(cmd) +def _ssh_run(ssh, cmd: str, *, get_pty: bool = False) -> tuple[int, str, str]: + """Run a command over a Paramiko SSHClient. + + Paramiko's exec_command runs commands without a TTY by default. + Some hosts have sudoers "requiretty" enabled, which causes sudo to + fail even when passwordless sudo is configured. For those commands, + request a PTY. + + We do not request a PTY for commands that stream binary data + (e.g. tar/gzip output), as a PTY can corrupt the byte stream. + """ + _stdin, stdout, stderr = ssh.exec_command(cmd, get_pty=get_pty) out = stdout.read().decode("utf-8", errors="replace") err = stderr.read().decode("utf-8", errors="replace") rc = stdout.channel.recv_exit_status() @@ -105,7 +113,6 @@ def remote_harvest( Returns the local path to state.json inside local_out_dir. """ - try: import paramiko # type: ignore except Exception as e: @@ -182,34 +189,35 @@ def remote_harvest( for p in exclude_paths or []: argv.extend(["--exclude-path", str(p)]) - _cmd = " ".join(shlex.quote(a) for a in argv) - if not no_sudo: - cmd = f"sudo {_cmd}" - else: - cmd = _cmd - rc, out, err = _ssh_run(ssh, cmd) + _cmd = " ".join(map(shlex.quote, argv)) + cmd = f"sudo {_cmd}" if not no_sudo else _cmd + + # PTY for sudo commands (helps sudoers requiretty). + rc, out, err = _ssh_run(ssh, cmd, get_pty=(not no_sudo)) if rc != 0: raise RuntimeError( "Remote harvest failed.\n" f"Command: {cmd}\n" f"Exit code: {rc}\n" + f"Stdout: {out.strip()}\n" f"Stderr: {err.strip()}" ) if not no_sudo: - # Ensure user can read the files, before we tar it + # Ensure user can read the files, before we tar it. if not resolved_user: raise RuntimeError( "Unable to determine remote username for chown. " "Pass --remote-user explicitly or use --no-sudo." ) cmd = f"sudo chown -R {resolved_user} {rbundle}" - rc, out, err = _ssh_run(ssh, cmd) + rc, out, err = _ssh_run(ssh, cmd, get_pty=True) if rc != 0: raise RuntimeError( "chown of harvest failed.\n" f"Command: {cmd}\n" f"Exit code: {rc}\n" + f"Stdout: {out.strip()}\n" f"Stderr: {err.strip()}" ) diff --git a/pyproject.toml b/pyproject.toml index 34f411e..72dd732 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "enroll" -version = "0.2.1" +version = "0.2.2" description = "Enroll a server's running state retrospectively into Ansible" authors = ["Miguel Jacq "] license = "GPL-3.0-or-later" diff --git a/release.sh b/release.sh index 0a052c7..db3f27b 100755 --- a/release.sh +++ b/release.sh @@ -44,14 +44,11 @@ for dist in ${DISTS[@]}; do done # RPM -REPO_ROOT="${HOME}/git/repo_rpm" -RPM_REPO="${REPO_ROOT}/rpm/x86_64" -BUILD_OUTPUT="${HOME}/git/enroll/dist" -REMOTE="letessier.mig5.net:/opt/repo_rpm" -KEYID="00AE817C24A10C2540461A9C1D7CDE0234DB458D" - -mkdir -p "$RPM_REPO" sudo apt-get -y install createrepo-c rpm +BUILD_OUTPUT="${HOME}/git/enroll/dist" +KEYID="00AE817C24A10C2540461A9C1D7CDE0234DB458D" +REPO_ROOT="${HOME}/git/repo_rpm" +REMOTE="letessier.mig5.net:/opt/repo_rpm" DISTS=( fedora:43 @@ -60,6 +57,10 @@ DISTS=( for dist in ${DISTS[@]}; do release=$(echo ${dist} | cut -d: -f2) + REPO_RELEASE_ROOT="${REPO_ROOT}/fc${release}" + RPM_REPO="${REPO_RELEASE_ROOT}/rpm/x86_64" + mkdir -p "$RPM_REPO" + docker build \ --no-cache \ -f Dockerfile.rpmbuild \ @@ -71,7 +72,6 @@ for dist in ${DISTS[@]}; do docker run --rm -v "$PWD":/src -v "$PWD/dist/rpm":/out -v "$HOME/git/jinjaturtle/dist/rpm":/deps:ro enroll-rpm:${release} sudo chown -R "${USER}" "$PWD/dist" - echo "==> Updating RPM repo..." for file in `ls -1 "${BUILD_OUTPUT}/rpm"`; do rpmsign --addsign "${BUILD_OUTPUT}/rpm/$file" done diff --git a/rpm/enroll.spec b/rpm/enroll.spec index 8fc8cac..12286fa 100644 --- a/rpm/enroll.spec +++ b/rpm/enroll.spec @@ -1,4 +1,4 @@ -%global upstream_version 0.2.1 +%global upstream_version 0.2.2 Name: enroll Version: %{upstream_version} @@ -43,6 +43,9 @@ Enroll a server's running state retrospectively into Ansible. %{_bindir}/enroll %changelog +* Sat Jan 02 2026 Miguel Jacq - %{version}-%{release} +- Fix stat() of parent directory so that we set directory perms correct on --include paths. +- Set pty for remote calls when sudo is required, to help systems with limits on sudo without pty * Fri Jan 01 2026 Miguel Jacq - %{version}-%{release} - Don't accidentally add extra_paths role to usr_local_custom list, resulting in extra_paths appearing twice in manifested playbook - Ensure directories in the tree of anything included with --include are defined in the state and manifest so we make dirs before we try to create files From 6c3275b44a9ca1ebeac4caec02cb650e996837c5 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 3 Jan 2026 11:46:40 +1100 Subject: [PATCH 07/48] Fix tests --- tests/test_cli_config_and_sops.py | 189 +++++++++++++++++ tests/test_more_coverage.py | 323 ++++++++++++++++++++++++++++++ tests/test_remote.py | 18 +- 3 files changed, 525 insertions(+), 5 deletions(-) create mode 100644 tests/test_cli_config_and_sops.py create mode 100644 tests/test_more_coverage.py diff --git a/tests/test_cli_config_and_sops.py b/tests/test_cli_config_and_sops.py new file mode 100644 index 0000000..7e3fe5b --- /dev/null +++ b/tests/test_cli_config_and_sops.py @@ -0,0 +1,189 @@ +from __future__ import annotations + +import argparse +import configparser +import tarfile +from pathlib import Path + + +def test_discover_config_path_precedence(monkeypatch, tmp_path: Path): + from enroll.cli import _discover_config_path + + cfg = tmp_path / "cfg.ini" + cfg.write_text("[enroll]\n", encoding="utf-8") + + # --no-config always wins + monkeypatch.setenv("ENROLL_CONFIG", str(cfg)) + assert _discover_config_path(["--no-config", "harvest"]) is None + + # explicit --config wins + assert _discover_config_path(["--config", str(cfg), "harvest"]) == cfg + + # env var used when present + assert _discover_config_path(["harvest"]) == cfg + + +def test_discover_config_path_finds_local_and_xdg(monkeypatch, tmp_path: Path): + from enroll.cli import _discover_config_path + + # local file in cwd + cwd = tmp_path / "cwd" + cwd.mkdir() + local = cwd / "enroll.ini" + local.write_text("[enroll]\n", encoding="utf-8") + + monkeypatch.chdir(cwd) + monkeypatch.delenv("ENROLL_CONFIG", raising=False) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + assert _discover_config_path(["harvest"]) == local + + # xdg config fallback + monkeypatch.chdir(tmp_path) + xdg = tmp_path / "xdg" + (xdg / "enroll").mkdir(parents=True) + xcfg = xdg / "enroll" / "enroll.ini" + xcfg.write_text("[enroll]\n", encoding="utf-8") + monkeypatch.setenv("XDG_CONFIG_HOME", str(xdg)) + assert _discover_config_path(["harvest"]) == xcfg + + +def test_section_to_argv_supports_bool_append_count_and_unknown(monkeypatch, capsys): + from enroll.cli import _section_to_argv + + ap = argparse.ArgumentParser(add_help=False) + ap.add_argument("--flag", action="store_true") + ap.add_argument("--no-flag", action="store_false", dest="flag2") + ap.add_argument("--item", action="append", default=[]) + ap.add_argument("-v", action="count", default=0) + + cfg = configparser.ConfigParser() + cfg.read_dict( + { + "enroll": { + "flag": "true", + "no_flag": "false", + "item": "a,b", + "v": "2", + "unknown_key": "zzz", + } + } + ) + + argv = _section_to_argv(ap, cfg, "enroll") + + # bools set + assert "--flag" in argv + assert "--no-flag" in argv + + # append expanded + assert argv.count("--item") == 2 + assert "a" in argv and "b" in argv + + # count flag expanded + assert argv.count("-v") == 2 + + # unknown key prints warning + err = capsys.readouterr().err + assert "unknown option" in err + + +def test_inject_config_argv_inserts_global_and_command_tokens(tmp_path: Path): + from enroll.cli import _inject_config_argv + + root = argparse.ArgumentParser(add_help=False) + root.add_argument("--root-flag", action="store_true") + sub = root.add_subparsers(dest="cmd", required=True) + p_h = sub.add_parser("harvest", add_help=False) + p_h.add_argument("--dangerous", action="store_true") + p_h.add_argument("--include-path", action="append", default=[]) + + cfg_path = tmp_path / "enroll.ini" + cfg_path.write_text( + """[enroll] +root-flag = true + +[harvest] +dangerous = true +include-path = /etc/one,/etc/two +""", + encoding="utf-8", + ) + + argv = ["harvest", "--include-path", "/etc/cli"] + injected = _inject_config_argv( + argv, + cfg_path=cfg_path, + root_parser=root, + subparsers={"harvest": p_h}, + ) + + # global inserted before cmd, subcommand tokens right after cmd + assert injected[:2] == ["--root-flag", "harvest"] + # include-path from config inserted before CLI include-path (CLI wins later if duplicates) + joined = " ".join(injected) + assert "--include-path /etc/one" in joined + assert "--include-path /etc/cli" in joined + + +def test_resolve_sops_out_file_and_encrypt_path(monkeypatch, tmp_path: Path): + from enroll import cli + + # directory output should yield harvest.tar.gz.sops inside + out_dir = tmp_path / "o" + out_dir.mkdir() + assert ( + cli._resolve_sops_out_file(str(out_dir), hint="h").name == "harvest.tar.gz.sops" + ) + + # file-like output retained + out_file = tmp_path / "x.sops" + assert cli._resolve_sops_out_file(str(out_file), hint="h") == out_file + + # None uses cache dir + class HC: + def __init__(self, d: Path): + self.dir = d + + monkeypatch.setattr( + cli, "new_harvest_cache_dir", lambda hint: HC(tmp_path / "cache") + ) + p = cli._resolve_sops_out_file(None, hint="h") + assert str(p).endswith("harvest.tar.gz.sops") + + # Cover _tar_dir_to quickly (writes a tarball) + bundle = tmp_path / "bundle" + bundle.mkdir() + (bundle / "state.json").write_text("{}", encoding="utf-8") + tar_path = tmp_path / "b.tar.gz" + cli._tar_dir_to(bundle, tar_path) + assert tar_path.exists() + with tarfile.open(tar_path, "r:gz") as tf: + names = tf.getnames() + assert "state.json" in names or "./state.json" in names + + +def test_encrypt_harvest_dir_to_sops_cleans_up_tmp_tgz(monkeypatch, tmp_path: Path): + from enroll.cli import _encrypt_harvest_dir_to_sops + + bundle = tmp_path / "bundle" + bundle.mkdir() + (bundle / "state.json").write_text("{}", encoding="utf-8") + out_file = tmp_path / "out.sops" + + seen = {} + + def fake_encrypt(src: Path, dst: Path, pgp_fingerprints, mode): # noqa: ARG001 + # write something so we can see output created + seen["src"] = src + dst.write_bytes(b"enc") + + monkeypatch.setattr("enroll.cli.encrypt_file_binary", fake_encrypt) + + # Make os.unlink raise FileNotFoundError to hit the except branch in finally. + monkeypatch.setattr( + "enroll.cli.os.unlink", lambda p: (_ for _ in ()).throw(FileNotFoundError()) + ) + + res = _encrypt_harvest_dir_to_sops(bundle, out_file, fps=["ABC"]) + assert res == out_file + assert out_file.read_bytes() == b"enc" diff --git a/tests/test_more_coverage.py b/tests/test_more_coverage.py new file mode 100644 index 0000000..2c6693a --- /dev/null +++ b/tests/test_more_coverage.py @@ -0,0 +1,323 @@ +from __future__ import annotations + +import json +import os +import subprocess +import sys +import types +from pathlib import Path +from types import SimpleNamespace + +import pytest + + +def test_cache_dir_defaults_to_home_cache(monkeypatch, tmp_path: Path): + # Ensure default path uses ~/.cache when XDG_CACHE_HOME is unset. + from enroll.cache import enroll_cache_dir + + monkeypatch.delenv("XDG_CACHE_HOME", raising=False) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + + p = enroll_cache_dir() + assert str(p).startswith(str(tmp_path)) + assert p.name == "enroll" + + +def test_harvest_cache_state_json_property(tmp_path: Path): + from enroll.cache import HarvestCache + + hc = HarvestCache(tmp_path / "h1") + assert hc.state_json == hc.dir / "state.json" + + +def test_cache_dir_security_rejects_symlink(tmp_path: Path): + from enroll.cache import _ensure_dir_secure + + real = tmp_path / "real" + real.mkdir() + link = tmp_path / "link" + link.symlink_to(real, target_is_directory=True) + + with pytest.raises(RuntimeError, match="Refusing to use symlink"): + _ensure_dir_secure(link) + + +def test_cache_dir_chmod_failures_are_ignored(monkeypatch, tmp_path: Path): + from enroll import cache + + # Make the cache base path deterministic and writable. + monkeypatch.setattr(cache, "enroll_cache_dir", lambda: tmp_path) + + # Force os.chmod to fail to cover the "except OSError: pass" paths. + monkeypatch.setattr( + os, "chmod", lambda *a, **k: (_ for _ in ()).throw(OSError("nope")) + ) + + hc = cache.new_harvest_cache_dir() + assert hc.dir.exists() + assert hc.dir.is_dir() + + +def test_stat_triplet_falls_back_to_numeric_ids(monkeypatch, tmp_path: Path): + from enroll.fsutil import stat_triplet + import pwd + import grp + + p = tmp_path / "x" + p.write_text("x", encoding="utf-8") + + # Force username/group resolution failures. + monkeypatch.setattr( + pwd, "getpwuid", lambda _uid: (_ for _ in ()).throw(KeyError("no user")) + ) + monkeypatch.setattr( + grp, "getgrgid", lambda _gid: (_ for _ in ()).throw(KeyError("no group")) + ) + + owner, group, mode = stat_triplet(str(p)) + assert owner.isdigit() + assert group.isdigit() + assert len(mode) == 4 + + +def test_ignore_policy_iter_effective_lines_removes_block_comments(): + from enroll.ignore import IgnorePolicy + + pol = IgnorePolicy() + data = b"""keep1 +/* +drop me +*/ +keep2 +""" + assert list(pol.iter_effective_lines(data)) == [b"keep1", b"keep2"] + + +def test_ignore_policy_deny_reason_dir_variants(tmp_path: Path): + from enroll.ignore import IgnorePolicy + + pol = IgnorePolicy() + + # denied by glob + assert pol.deny_reason_dir("/etc/shadow") == "denied_path" + + # symlink rejected + d = tmp_path / "d" + d.mkdir() + link = tmp_path / "l" + link.symlink_to(d, target_is_directory=True) + assert pol.deny_reason_dir(str(link)) == "symlink" + + # not a directory + f = tmp_path / "f" + f.write_text("x", encoding="utf-8") + assert pol.deny_reason_dir(str(f)) == "not_directory" + + # ok + assert pol.deny_reason_dir(str(d)) is None + + +def test_run_jinjaturtle_parses_outputs(monkeypatch, tmp_path: Path): + # Fully unit-test enroll.jinjaturtle.run_jinjaturtle by stubbing subprocess.run. + from enroll.jinjaturtle import run_jinjaturtle + + def fake_run(cmd, **kwargs): # noqa: ARG001 + # cmd includes "-d -t