diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc9385..500e7c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # 0.7.0 * Add support for detecting flatpaks and snaps - * BREAKING CHANGE: Group all package and systemd-unit roles into Debian Section/RPM Group roles by default, including managed config files and unit state. This mode is not used if `--fqdn` or `--no-common-roles` is set, in which case, the traditional behaviour of preserving one role per package/unit is used instead. + * Add --merge-simple-packages to reduce the number of roles, for packages that have no config files or services to maintain. # 0.6.0 diff --git a/README.md b/README.md index f462471..96e228f 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ Generate Ansible output from an existing harvest bundle. **Common flags** - `--fqdn `: enables **multi-site** output style -- `--no-common-roles`: disables the default grouping of package and systemd-unit roles into Debian Section/RPM Group roles, preserving one generated role per package/unit. `--fqdn` implies this behaviour. +- `--merge-simple-packages`: Puts all packages that don't have config files or services to maintain, in a `common_packages` role, to reduce the number of overall Ansible roles to run. **Role tags** Generated playbooks tag each role so you can target just the parts you need: @@ -149,7 +149,7 @@ Convenience wrapper that runs **harvest → manifest** in one command. Use this when you want “get me something workable ASAP”. -Supports the same general flags as harvest/manifest, including `--fqdn`, `--no-common-roles`, remote harvest flags, and `--sops`. +Supports the same general flags as harvest/manifest, including `--fqdn`, remote harvest flags, `--merge-simple-packages`, and `--sops`. --- diff --git a/enroll/cli.py b/enroll/cli.py index a621928..2483a24 100644 --- a/enroll/cli.py +++ b/enroll/cli.py @@ -312,14 +312,6 @@ def _add_common_manifest_args(p: argparse.ArgumentParser) -> None: "--fqdn", help="Host FQDN/name for site-mode output (creates inventory/, inventory/host_vars/, playbooks/).", ) - p.add_argument( - "--no-common-roles", - action="store_true", - help=( - "Do not group package and systemd-unit roles into common section/group roles. " - "This preserves one generated role per package/unit. --fqdn implies this." - ), - ) g = p.add_mutually_exclusive_group() g.add_argument( "--jinjaturtle", @@ -511,6 +503,11 @@ def main() -> None: "(binary) using the given GPG fingerprint(s). Requires `sops` on PATH." ), ) + m.add_argument( + "--merge-simple-packages", + action="store_true", + help="Merge packages with no configuration files into a single 'common_packages' role.", + ) _add_common_manifest_args(m) s = sub.add_parser( @@ -573,6 +570,11 @@ def main() -> None: "or a file path." ), ) + s.add_argument( + "--merge-simple-packages", + action="store_true", + help="Merge packages with no configuration files into a single 'common_packages' role.", + ) _add_common_manifest_args(s) d = sub.add_parser("diff", help="Compare two harvests and report differences") @@ -919,7 +921,7 @@ def main() -> None: fqdn=args.fqdn, jinjaturtle=_jt_mode(args), sops_fingerprints=getattr(args, "sops", None), - no_common_roles=bool(getattr(args, "no_common_roles", False)), + merge_simple_packages=getattr(args, "merge_simple_packages", False), ) if getattr(args, "sops", None) and out_enc: print(str(out_enc)) @@ -1057,7 +1059,9 @@ def main() -> None: fqdn=args.fqdn, jinjaturtle=_jt_mode(args), sops_fingerprints=list(sops_fps), - no_common_roles=bool(getattr(args, "no_common_roles", False)), + merge_simple_packages=getattr( + args, "merge_simple_packages", False + ), ) if not args.harvest: print(str(out_file)) @@ -1088,7 +1092,9 @@ def main() -> None: args.out, fqdn=args.fqdn, jinjaturtle=_jt_mode(args), - no_common_roles=bool(getattr(args, "no_common_roles", False)), + merge_simple_packages=getattr( + args, "merge_simple_packages", False + ), ) # For usability (when --harvest wasn't provided), print the harvest path. if not args.harvest: @@ -1119,7 +1125,9 @@ def main() -> None: fqdn=args.fqdn, jinjaturtle=_jt_mode(args), sops_fingerprints=list(sops_fps), - no_common_roles=bool(getattr(args, "no_common_roles", False)), + merge_simple_packages=getattr( + args, "merge_simple_packages", False + ), ) if not args.harvest: print(str(out_file)) @@ -1139,7 +1147,9 @@ def main() -> None: args.out, fqdn=args.fqdn, jinjaturtle=_jt_mode(args), - no_common_roles=bool(getattr(args, "no_common_roles", False)), + merge_simple_packages=getattr( + args, "merge_simple_packages", False + ), ) except RemoteSudoPasswordRequired: raise SystemExit( diff --git a/enroll/debian.py b/enroll/debian.py index 6d0ca06..9bf847e 100644 --- a/enroll/debian.py +++ b/enroll/debian.py @@ -69,7 +69,7 @@ def list_installed_packages() -> Dict[str, List[Dict[str, str]]]: Uses dpkg-query and is expected to work on Debian/Ubuntu-like systems. Output format: - {"pkg": [{"version": "...", "arch": "...", "section": "..."}, ...], ...} + {"pkg": [{"version": "...", "arch": "..."}, ...], ...} """ try: @@ -77,7 +77,7 @@ def list_installed_packages() -> Dict[str, List[Dict[str, str]]]: [ "dpkg-query", "-W", - "-f=${Package}\t${Version}\t${Architecture}\t${Section}\n", + "-f=${Package}\t${Version}\t${Architecture}\n", ], text=True, capture_output=True, @@ -97,10 +97,7 @@ def list_installed_packages() -> Dict[str, List[Dict[str, str]]]: name, ver, arch = parts[0].strip(), parts[1].strip(), parts[2].strip() if not name: continue - instance = {"version": ver, "arch": arch} - if len(parts) >= 4 and parts[3].strip(): - instance["section"] = parts[3].strip() - out.setdefault(name, []).append(instance) + out.setdefault(name, []).append({"version": ver, "arch": arch}) # Stable ordering for deterministic JSON dumps. for k in list(out.keys()): diff --git a/enroll/harvest.py b/enroll/harvest.py index b79c24c..01d3ff7 100644 --- a/enroll/harvest.py +++ b/enroll/harvest.py @@ -86,7 +86,6 @@ class ServiceSnapshot: class PackageSnapshot: package: str role_name: str - section: Optional[str] = None managed_dirs: List[ManagedDir] = field(default_factory=list) managed_files: List[ManagedFile] = field(default_factory=list) managed_links: List[ManagedLink] = field(default_factory=list) @@ -390,30 +389,6 @@ def _role_name_from_pkg(pkg: str) -> str: return avoid_reserved_role_name(_safe_name(pkg), prefix="package") -def _package_section_from_installations( - installs: List[Dict[str, str]], -) -> Optional[str]: - """Return a stable package grouping label from installed package metadata. - - Debian exposes this as ``Section``. RPM-family distributions have a broadly - similar ``Group`` tag, although modern Fedora/RHEL packages may omit it or - set it to ``Unspecified``. - """ - - values: Set[str] = set() - for inst in installs or []: - value = (inst.get("section") or inst.get("group") or "").strip() - if not value: - continue - if value.lower() in {"(none)", "none", "unspecified"}: - continue - values.add(value) - - if not values: - return None - return sorted(values)[0] - - def _copy_into_bundle( bundle_dir: str, role_name: str, abs_path: str, src_rel: str ) -> None: @@ -1304,9 +1279,6 @@ def harvest( cron_snapshot = PackageSnapshot( package=cron_pkg, role_name=cron_role_name, - section=_package_section_from_installations( - installed_pkgs.get(cron_pkg, []) - ), managed_files=cron_managed, excluded=cron_excluded, notes=cron_notes, @@ -1342,9 +1314,6 @@ def harvest( logrotate_snapshot = PackageSnapshot( package=logrotate_pkg, role_name=logrotate_role_name, - section=_package_section_from_installations( - installed_pkgs.get(logrotate_pkg, []) - ), managed_files=lr_managed, excluded=lr_excluded, notes=lr_notes, @@ -1763,11 +1732,11 @@ def harvest( seen_global=captured_global, ) - has_config = bool(managed or excluded) + has_config = bool(pkg_to_etc_paths.get(pkg, []) or managed) if not has_config: notes.append( - "No changed or custom configuration detected for this package." + "No /etc files or custom configuration detected for this package." ) simple_packages.append(pkg) @@ -1775,9 +1744,6 @@ def harvest( PackageSnapshot( package=pkg, role_name=role, - section=_package_section_from_installations( - installed_pkgs.get(pkg, []) - ), managed_files=managed, managed_links=[], excluded=excluded, @@ -2521,7 +2487,6 @@ def harvest( arches = sorted({i.get("arch") for i in installs if i.get("arch")}) vers = sorted({i.get("version") for i in installs if i.get("version")}) version: Optional[str] = vers[0] if len(vers) == 1 else None - section = _package_section_from_installations(installs) observed: List[Dict[str, str]] = [] if pkg in manual_set: @@ -2544,7 +2509,6 @@ def harvest( "version": version, "arches": arches, "installations": installs, - "section": section, "observed_via": observed, "roles": roles, } diff --git a/enroll/manifest.py b/enroll/manifest.py index 1e8ab9b..55c3baa 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -80,78 +80,6 @@ def _yaml_dump_mapping(obj: Dict[str, Any], *, sort_keys: bool = True) -> str: ) -def _role_id(raw: str) -> str: - """Return an Ansible-safe role identifier from an arbitrary label.""" - - s = re.sub(r"[^A-Za-z0-9]+", "_", raw or "misc") - s = re.sub(r"([a-z0-9])([A-Z])", r"\1_\2", s) - s = s.lower() - s = re.sub(r"_+", "_", s).strip("_") - if not s: - s = "misc" - if not re.match(r"^[a-z_]", s): - s = "r_" + s - return s - - -def _package_section_label( - package_role: Dict[str, Any], inventory_packages: Dict[str, Any] -) -> str: - """Return the Debian Section/RPM Group label for a package role.""" - - pkg = str(package_role.get("package") or "") - inv = inventory_packages.get(pkg) or {} - candidates: List[str] = [] - - for value in (package_role.get("section"), inv.get("section")): - if isinstance(value, str) and value.strip(): - candidates.append(value.strip()) - - for inst in inv.get("installations", []) or []: - if not isinstance(inst, dict): - continue - for key in ("section", "group"): - value = inst.get(key) - if isinstance(value, str) and value.strip(): - candidates.append(value.strip()) - - for value in candidates: - if value.lower() in {"(none)", "none", "unspecified"}: - continue - return value - return "misc" - - -def _section_label_for_packages( - packages: List[str], inventory_packages: Dict[str, Any] -) -> str: - """Return a stable section/group label for a set of packages. - - Service roles can involve more than one package. Prefer the first - package with a concrete Debian Section/RPM Group and fall back to misc - when no package metadata is available. - """ - - for pkg in packages or []: - label = _package_section_label({"package": pkg}, inventory_packages) - if label and label.lower() != "misc": - return label - return "misc" - - -def _section_role_name(label: str, occupied_roles: Set[str]) -> str: - """Create a stable section role name, avoiding generated-role collisions.""" - - base = avoid_reserved_role_name(_role_id(label), prefix="section") - role = base if base not in occupied_roles else f"section_{base}" - n = 2 - while role in occupied_roles: - role = f"section_{base}_{n}" - n += 1 - occupied_roles.add(role) - return role - - def _merge_mappings_overwrite( existing: Dict[str, Any], incoming: Dict[str, Any] ) -> Dict[str, Any]: @@ -722,39 +650,6 @@ def _render_install_packages_tasks(role: str, var_prefix: str) -> str: """ -def _render_grouped_systemd_tasks(var_prefix: str) -> str: - """Render tasks to manage multiple systemd units in a common role.""" - - return f"""- name: Probe whether grouped systemd units exist and are manageable - ansible.builtin.systemd: - name: "{{{{ item.name }}}}" - check_mode: true - loop: "{{{{ {var_prefix}_systemd_units | default([]) }}}}" - register: _enroll_unit_probes - failed_when: false - changed_when: false - when: item.manage | default(false) - -- name: Ensure grouped unit enablement matches harvest - ansible.builtin.systemd: - name: "{{{{ item.item.name }}}}" - enabled: "{{{{ item.item.enabled | bool }}}}" - loop: "{{{{ _enroll_unit_probes.results | default([]) }}}}" - when: - - item.item.manage | default(false) - - not (item.failed | default(false)) - -- name: Ensure grouped unit running state matches harvest - ansible.builtin.systemd: - name: "{{{{ item.item.name }}}}" - state: "{{{{ item.item.state }}}}" - loop: "{{{{ _enroll_unit_probes.results | default([]) }}}}" - when: - - item.item.manage | default(false) - - not (item.failed | default(false)) -""" - - def _render_firewall_runtime_tasks(var_prefix: str) -> str: """Render tasks for live ipset/iptables snapshots.""" return f"""- name: Ensure firewall runtime snapshot directory exists @@ -998,16 +893,13 @@ def _manifest_from_bundle_dir( *, fqdn: Optional[str] = None, jinjaturtle: str = "auto", # auto|on|off - no_common_roles: bool = False, + merge_simple_packages: bool = False, ) -> None: state_path = os.path.join(bundle_dir, "state.json") with open(state_path, "r", encoding="utf-8") as f: state = json.load(f) roles: Dict[str, Any] = state.get("roles") or {} - inventory_packages: Dict[str, Any] = (state.get("inventory") or {}).get( - "packages" - ) or {} services: List[Dict[str, Any]] = roles.get("services", []) package_roles: List[Dict[str, Any]] = roles.get("packages", []) @@ -1022,7 +914,6 @@ def _manifest_from_bundle_dir( extra_paths_snapshot: Dict[str, Any] = roles.get("extra_paths", {}) site_mode = fqdn is not None and fqdn != "" - use_common_roles = (not site_mode) and (not no_common_roles) jt_exe = find_jinjaturtle_cmd() jt_enabled = False @@ -1063,20 +954,6 @@ def _manifest_from_bundle_dir( manifested_extra_paths_roles: List[str] = [] manifested_service_roles: List[str] = [] manifested_pkg_roles: List[str] = [] - common_role_groups: Dict[str, List[Dict[str, Any]]] = {} - common_tail_roles: List[str] = [] - - if use_common_roles: - for svc in services: - label = _section_label_for_packages( - svc.get("packages", []) or [], inventory_packages - ) - common_role_groups.setdefault(label, []).append( - {"kind": "service", "snapshot": svc} - ) - services_to_manifest: List[Dict[str, Any]] = [] - else: - services_to_manifest = services # ------------------------- # Users role (non-system users) @@ -2493,7 +2370,7 @@ User-requested extra file harvesting. # ------------------------- # Service roles # ------------------------- - for svc in services_to_manifest: + for svc in services: source_role = svc["role_name"] role = avoid_reserved_role_name(source_role, prefix="service") unit = svc["unit"] @@ -2672,200 +2549,64 @@ Generated from `{unit}`. manifested_service_roles.append(role) # ------------------------- - # Common package section/group roles + # Merge simple packages (if --merge-simple-packages is set) # - # Outside --fqdn/site mode, package and systemd-unit roles are grouped by - # Debian Section or RPM Group by default. Managed config and unit state can - # live in those section roles too; --no-common-roles preserves the historic - # one-role-per-package/unit output, and --fqdn implies that mode because - # grouped role contents would be unsafe across multiple harvested hosts. + # Packages with no configuration files, systemd units, or cron jobs + # are merged into a single 'common_packages' role to reduce role count. # ------------------------- - if use_common_roles: + simple_packages_list: List[str] = [] + if merge_simple_packages: + filtered_package_roles: List[Dict[str, Any]] = [] for pr in package_roles: - label = _package_section_label(pr, inventory_packages) - common_role_groups.setdefault(label, []).append( - {"kind": "package", "snapshot": pr} - ) - package_roles = [] + has_config = pr.get("has_config", True) + managed_files = pr.get("managed_files", []) or [] + # A package is "simple" if it has no config files AND no managed files + if not has_config and not managed_files: + pkg = pr.get("package") + if pkg: + simple_packages_list.append(pkg) + else: + filtered_package_roles.append(pr) + package_roles = filtered_package_roles # ------------------------- # Manually installed package roles # ------------------------- - occupied_roles: Set[str] = set( - manifested_apt_config_roles - + manifested_dnf_config_roles - + manifested_users_roles - + manifested_flatpak_roles - + manifested_snap_roles - + manifested_service_roles - + manifested_firewall_runtime_roles - + manifested_etc_custom_roles - + manifested_usr_local_custom_roles - + manifested_extra_paths_roles - ) - for pr in package_roles: - occupied_roles.add( - avoid_reserved_role_name(str(pr.get("role_name") or ""), prefix="package") - ) - - for section_label, entries in sorted(common_role_groups.items()): - role = _section_role_name(section_label, occupied_roles) + # First, create the common_packages role if we have simple packages to merge + if simple_packages_list: + role = "common_packages" role_dir = os.path.join(roles_root, role) _write_role_scaffold(role_dir) var_prefix = role - packages_set: Set[str] = set() + + # No managed files for common_packages - just package installation files_var: List[Dict[str, Any]] = [] - dirs_var: List[Dict[str, Any]] = [] links_var: List[Dict[str, Any]] = [] - systemd_units: List[Dict[str, Any]] = [] - excluded_all: List[Dict[str, Any]] = [] - notes_all: List[str] = [] - origin_lines: List[str] = [] - jt_combined: Dict[str, Any] = {} - - seen_files: Set[tuple] = set() - seen_dirs: Set[tuple] = set() - seen_links: Set[tuple] = set() - seen_units: Set[str] = set() - - for entry in entries: - kind = entry.get("kind") or "package" - snap = entry.get("snapshot") or {} - source_role = str(snap.get("role_name") or "") - managed_files = snap.get("managed_files", []) or [] - managed_dirs = snap.get("managed_dirs", []) or [] - managed_links = snap.get("managed_links", []) or [] - excluded = snap.get("excluded", []) or [] - notes = snap.get("notes", []) or [] - - if kind == "service": - pkgs = snap.get("packages", []) or [] - unit = str(snap.get("unit") or "") - origin_lines.append(f"service `{unit}` from role `{source_role}`") - else: - pkg = str(snap.get("package") or "") - pkgs = [pkg] if pkg else [] - origin_lines.append(f"package `{pkg}` from role `{source_role}`") - - for pkg in pkgs: - if pkg: - packages_set.add(str(pkg)) - - templated: Set[str] = set() - jt_vars = "" - if managed_files and source_role: - templated, jt_vars = _jinjify_managed_files( - bundle_dir, - source_role, - role_dir, - managed_files, - jt_exe=jt_exe, - jt_enabled=jt_enabled, - overwrite_templates=True, - ) - - _copy_artifacts( - bundle_dir, - source_role, - os.path.join(role_dir, "files"), - exclude_rels=templated, - ) - - notify_other = "Restart managed services" if kind == "service" else None - for item in _build_managed_files_var( - managed_files, - templated, - notify_other=notify_other, - notify_systemd="Run systemd daemon-reload", - ): - key = (item.get("dest"), item.get("src_rel"), item.get("kind")) - if key not in seen_files: - seen_files.add(key) - files_var.append(item) - - for item in _build_managed_dirs_var(managed_dirs): - key = ( - item.get("dest"), - item.get("owner"), - item.get("group"), - item.get("mode"), - ) - if key not in seen_dirs: - seen_dirs.add(key) - dirs_var.append(item) - - for item in _build_managed_links_var(managed_links): - key = (item.get("dest"), item.get("src")) - if key not in seen_links: - seen_links.add(key) - links_var.append(item) - - if kind == "service": - unit = str(snap.get("unit") or "") - if unit and unit not in seen_units: - seen_units.add(unit) - unit_file_state = str(snap.get("unit_file_state") or "") - enabled_at_harvest = unit_file_state in ( - "enabled", - "enabled-runtime", - ) - desired_state = ( - "started" if snap.get("active_state") == "active" else "stopped" - ) - systemd_units.append( - { - "name": unit, - "manage": True, - "enabled": bool(enabled_at_harvest), - "state": desired_state, - } - ) - - excluded_all.extend(excluded) - notes_all.extend(str(n) for n in notes) - - jt_map = _yaml_load_mapping(jt_vars) if jt_vars.strip() else {} - jt_combined = _merge_mappings_overwrite(jt_combined, jt_map) - - packages = sorted(packages_set) - files_var = sorted(files_var, key=lambda x: str(x.get("dest") or "")) - dirs_var = sorted(dirs_var, key=lambda x: str(x.get("dest") or "")) - links_var = sorted(links_var, key=lambda x: str(x.get("dest") or "")) - systemd_units = sorted(systemd_units, key=lambda x: str(x.get("name") or "")) + dirs_var: List[Dict[str, Any]] = [] base_vars: Dict[str, Any] = { - f"{var_prefix}_packages": packages, + f"{var_prefix}_packages": simple_packages_list, f"{var_prefix}_managed_files": files_var, f"{var_prefix}_managed_dirs": dirs_var, f"{var_prefix}_managed_links": links_var, - f"{var_prefix}_systemd_units": systemd_units, } - base_vars = _merge_mappings_overwrite(base_vars, jt_combined) - _write_role_defaults(role_dir, base_vars) + if site_mode: + _write_role_defaults( + role_dir, + { + f"{var_prefix}_packages": [], + f"{var_prefix}_managed_files": [], + f"{var_prefix}_managed_dirs": [], + f"{var_prefix}_managed_links": [], + }, + ) + _write_hostvars(out_dir, fqdn or "", role, base_vars) + else: + _write_role_defaults(role_dir, base_vars) - if {"cron", "logrotate"}.intersection(packages_set): - common_tail_roles.append(role) - - handlers = ( - """--- -- name: Run systemd daemon-reload - ansible.builtin.systemd: - daemon_reload: true - -- name: Restart managed services - ansible.builtin.service: - name: "{{ item.name }}" - state: restarted - loop: "{{ """ - + f"{var_prefix}_systemd_units" - + """ | default([]) }}" - when: - - item.manage | default(false) - - (item.state | default('stopped')) == 'started' -""" - ) + handlers = "---\n" with open( os.path.join(role_dir, "handlers", "main.yml"), "w", encoding="utf-8" ) as f: @@ -2873,10 +2614,6 @@ Generated from `{unit}`. task_parts: List[str] = [] task_parts.append("---\n" + _render_install_packages_tasks(role, var_prefix)) - task_parts.append( - _render_generic_files_tasks(var_prefix, include_restart_notify=True) - ) - task_parts.append(_render_grouped_systemd_tasks(var_prefix)) tasks = "\n".join(task_parts).rstrip() + "\n" with open( @@ -2891,28 +2628,14 @@ Generated from `{unit}`. readme = f"""# {role} -Common role for package section/group `{section_label}`. +Common packages with no configuration files. -## Origin roles -{os.linesep.join("- " + line for line in sorted(origin_lines)) or "- (none)"} +This role was created by merging simple packages using the `--merge-simple-packages` flag. ## Packages -{os.linesep.join("- " + p for p in packages) or "- (none)"} +{os.linesep.join("- " + p for p in simple_packages_list) or "- (none)"} -## Managed files -{os.linesep.join("- " + mf["dest"] for mf in files_var) or "- (none)"} - -## Managed symlinks -{os.linesep.join("- " + ml["dest"] + " -> " + ml["src"] for ml in links_var) or "- (none)"} - -## Systemd units -{os.linesep.join("- " + u["name"] + " (enabled=" + str(u["enabled"]).lower() + ", state=" + u["state"] + ")" for u in systemd_units) or "- (none)"} - -## Excluded (possible secrets / unsafe) -{os.linesep.join("- " + e.get("path", "") + " (" + e.get("reason", "") + ")" for e in excluded_all) or "- (none)"} - -## Notes -{os.linesep.join("- " + n for n in notes_all) or "- (none)"} +> Note: This role only installs packages; it does not manage any configuration files or services. """ with open(os.path.join(role_dir, "README.md"), "w", encoding="utf-8") as f: f.write(readme) @@ -3053,9 +2776,6 @@ Generated for package `{pkg}`. for r in ("cron", "logrotate"): if r in manifested_pkg_roles: tail_roles.append(r) - for r in common_tail_roles: - if r in manifested_pkg_roles and r not in tail_roles: - tail_roles.append(r) main_pkg_roles = [r for r in manifested_pkg_roles if r not in set(tail_roles)] @@ -3089,7 +2809,7 @@ def manifest( fqdn: Optional[str] = None, jinjaturtle: str = "auto", # auto|on|off sops_fingerprints: Optional[List[str]] = None, - no_common_roles: bool = False, + merge_simple_packages: bool = False, ) -> Optional[str]: """Render an Ansible manifest from a harvest. @@ -3122,7 +2842,7 @@ def manifest( out, fqdn=fqdn, jinjaturtle=jinjaturtle, - no_common_roles=no_common_roles, + merge_simple_packages=merge_simple_packages, ) return None @@ -3142,7 +2862,7 @@ def manifest( str(tmp_out), fqdn=fqdn, jinjaturtle=jinjaturtle, - no_common_roles=no_common_roles, + merge_simple_packages=merge_simple_packages, ) enc = _encrypt_manifest_out_dir_to_sops( diff --git a/enroll/rpm.py b/enroll/rpm.py index a036814..0314670 100644 --- a/enroll/rpm.py +++ b/enroll/rpm.py @@ -148,7 +148,7 @@ def list_installed_packages() -> Dict[str, List[Dict[str, str]]]: Uses `rpm -qa` and is expected to work on RHEL/Fedora-like systems. Output format: - {"pkg": [{"version": "...", "arch": "...", "group": "..."}, ...], ...} + {"pkg": [{"version": "...", "arch": "..."}, ...], ...} The version string is formatted as: - "-" for typical packages @@ -161,7 +161,7 @@ def list_installed_packages() -> Dict[str, List[Dict[str, str]]]: "rpm", "-qa", "--qf", - "%{NAME}\t%{EPOCHNUM}\t%{VERSION}\t%{RELEASE}\t%{ARCH}\t%{GROUP}\n", + "%{NAME}\t%{EPOCHNUM}\t%{VERSION}\t%{RELEASE}\t%{ARCH}\n", ], allow_fail=False, merge_err=True, @@ -190,11 +190,7 @@ def list_installed_packages() -> Dict[str, List[Dict[str, str]]]: if epoch and epoch.isdigit() and epoch != "0": v = f"{epoch}:{v}" - instance = {"version": v, "arch": arch} - if len(parts) >= 6 and parts[5].strip(): - instance["group"] = parts[5].strip() - - pkgs.setdefault(name, []).append(instance) + pkgs.setdefault(name, []).append({"version": v, "arch": arch}) for k in list(pkgs.keys()): pkgs[k] = sorted( diff --git a/enroll/schema/state.schema.json b/enroll/schema/state.schema.json index 11f8672..482b014 100644 --- a/enroll/schema/state.schema.json +++ b/enroll/schema/state.schema.json @@ -117,14 +117,6 @@ "minLength": 1, "type": "string" }, - "group": { - "minLength": 1, - "type": "string" - }, - "section": { - "minLength": 1, - "type": "string" - }, "version": { "minLength": 1, "type": "string" @@ -372,12 +364,6 @@ }, "type": "array" }, - "section": { - "type": [ - "string", - "null" - ] - }, "version": { "type": [ "string", @@ -408,12 +394,6 @@ "has_config": { "type": "boolean", "default": true - }, - "section": { - "type": [ - "string", - "null" - ] } }, "required": [ diff --git a/tests.sh b/tests.sh index 2435b6a..2677496 100755 --- a/tests.sh +++ b/tests.sh @@ -44,12 +44,20 @@ poetry run \ --format json | jq DEBIAN_FRONTEND=noninteractive apt-get remove -y --purge cowsay -# No common roles mode (tested later) - poetry run \ - enroll manifest \ - --harvest "${BUNDLE_DIR}2" \ - --out "${ANSIBLE_DIR}2" \ - --no-common-roles +# Ensure some flatpaks are installed +DEBIAN_FRONTEND=noninteractive apt-get install -y flatpak +flatpak remote-add --if-not-exists flathub https://dl.flathub.org/repo/flathub.flatpakrepo +flatpak install -y flathub org.onionshare.OnionShare + +poetry run \ + enroll manifest \ + --harvest "${BUNDLE_DIR}2" \ + --out "${ANSIBLE_DIR}2" + +# Test the presence of OnionShare +builtin cd "${ANSIBLE_DIR}2" +grep -r org.onionshare.OnionShare "${ANSIBLE_DIR}2/roles" +ansible-playbook playbook.yml -i "localhost," -c local --check --diff --tags role_flatpak # Ansible test builtin cd "${ANSIBLE_DIR}" @@ -58,8 +66,3 @@ ansible-lint "${ANSIBLE_DIR}" # Run ansible-playbook playbook.yml -i "localhost," -c local --check --diff - -# Test the --no-common-roles mode -builtin cd "${ANSIBLE_DIR}2" -ls "${ANSIBLE_DIR}2/roles" -ansible-playbook playbook.yml -i "localhost," -c local --check --diff diff --git a/tests/test_cli.py b/tests/test_cli.py index 6a89c58..dcdb6a7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -47,7 +47,6 @@ def test_cli_manifest_subcommand_calls_manifest(monkeypatch, tmp_path): # Common manifest args should be passed through by the CLI. called["fqdn"] = kwargs.get("fqdn") called["jinjaturtle"] = kwargs.get("jinjaturtle") - called["no_common_roles"] = kwargs.get("no_common_roles") monkeypatch.setattr(cli, "manifest", fake_manifest) monkeypatch.setattr( @@ -68,36 +67,6 @@ def test_cli_manifest_subcommand_calls_manifest(monkeypatch, tmp_path): assert called["out"] == str(tmp_path / "ansible") assert called["fqdn"] is None assert called["jinjaturtle"] == "auto" - assert called["no_common_roles"] is False - - -def test_cli_manifest_no_common_roles_is_forwarded(monkeypatch, tmp_path): - called = {} - - def fake_manifest(harvest_dir: str, out_dir: str, **kwargs): - called["harvest"] = harvest_dir - called["out"] = out_dir - called["no_common_roles"] = kwargs.get("no_common_roles") - - monkeypatch.setattr(cli, "manifest", fake_manifest) - monkeypatch.setattr( - sys, - "argv", - [ - "enroll", - "manifest", - "--harvest", - str(tmp_path / "bundle"), - "--out", - str(tmp_path / "ansible"), - "--no-common-roles", - ], - ) - - cli.main() - assert called["harvest"] == str(tmp_path / "bundle") - assert called["out"] == str(tmp_path / "ansible") - assert called["no_common_roles"] is True def test_cli_enroll_subcommand_runs_harvest_then_manifest(monkeypatch, tmp_path): diff --git a/tests/test_debian.py b/tests/test_debian.py index 64fe420..ed9df7a 100644 --- a/tests/test_debian.py +++ b/tests/test_debian.py @@ -169,7 +169,7 @@ def test_list_installed_packages_parses_output(): original_run = d.subprocess.run def fake_run(cmd, text, capture_output, check): - return P(0, "nginx\t1.18.0\tamd64\tweb\nvim\t8.2\tamd64\teditors\n") + return P(0, "nginx\t1.18.0\tamd64\nvim\t8.2\tamd64\n") d.subprocess.run = fake_run try: @@ -177,7 +177,6 @@ def test_list_installed_packages_parses_output(): assert "nginx" in result assert result["nginx"][0]["version"] == "1.18.0" assert result["nginx"][0]["arch"] == "amd64" - assert result["nginx"][0]["section"] == "web" assert "vim" in result finally: d.subprocess.run = original_run diff --git a/tests/test_harvest.py b/tests/test_harvest.py index a54ce98..1b50da5 100644 --- a/tests/test_harvest.py +++ b/tests/test_harvest.py @@ -202,12 +202,7 @@ def test_harvest_dedup_manual_packages_and_builds_etc_custom( owned_etc = {"/etc/openvpn/server.conf"} etc_owner_map = {"/etc/openvpn/server.conf": "openvpn"} topdir_to_pkgs = {"openvpn": {"openvpn"}} - # curl has a package-owned /etc path, but no changed/custom harvested - # artifacts. That should still be considered a simple package role. - pkg_to_etc_paths = { - "openvpn": ["/etc/openvpn/server.conf"], - "curl": ["/etc/curl/curlrc"], - } + pkg_to_etc_paths = {"openvpn": ["/etc/openvpn/server.conf"], "curl": []} backend = FakeBackend( name="dpkg", @@ -269,9 +264,6 @@ def test_harvest_dedup_manual_packages_and_builds_etc_custom( pkg_roles = st["roles"]["packages"] assert all(pr["package"] != "openvpn" for pr in pkg_roles) assert any(pr["package"] == "curl" for pr in pkg_roles) - curl_role = next(pr for pr in pkg_roles if pr["package"] == "curl") - assert curl_role["has_config"] is False - assert any("No changed or custom configuration" in n for n in curl_role["notes"]) # Inventory provenance: openvpn should be observed via systemd unit. openvpn_obs = inv["openvpn"]["observed_via"] diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 3accec9..a8eaf0f 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -9,80 +9,6 @@ import pytest import enroll.manifest as manifest -def _minimal_package_state(packages): - return { - "schema_version": 3, - "host": {"hostname": "test", "os": "debian", "pkg_backend": "dpkg"}, - "inventory": { - "packages": { - p["package"]: { - "version": "1.0", - "arches": ["amd64"], - "installations": [ - { - "version": "1.0", - "arch": "amd64", - "section": p.get("section") or "misc", - } - ], - "section": p.get("section") or "misc", - "observed_via": [{"kind": "package_role", "ref": p["role_name"]}], - "roles": [p["role_name"]], - } - for p in packages - } - }, - "roles": { - "users": { - "role_name": "users", - "users": [], - "managed_files": [], - "excluded": [], - "notes": [], - }, - "services": [], - "packages": packages, - "apt_config": { - "role_name": "apt_config", - "managed_files": [], - "excluded": [], - "notes": [], - }, - "dnf_config": { - "role_name": "dnf_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 _write_state(bundle: Path, state: dict) -> None: - bundle.mkdir(parents=True, exist_ok=True) - (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") - - def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): bundle = tmp_path / "bundle" out = tmp_path / "ansible" @@ -255,7 +181,7 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): bundle / "artifacts" / "usr_local_custom" / "usr" / "local" / "bin" / "myscript" ).write_text("#!/bin/sh\necho hi\n", encoding="utf-8") - manifest.manifest(str(bundle), str(out), no_common_roles=True) + manifest.manifest(str(bundle), str(out)) # Service role: systemd management should be gated on foo_manage_unit and a probe. tasks = (out / "roles" / "foo" / "tasks" / "main.yml").read_text(encoding="utf-8") @@ -287,365 +213,6 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): assert "role: foo" in pb -def test_manifest_groups_simple_packages_by_section_by_default(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - state = _minimal_package_state( - [ - { - "package": "curl", - "role_name": "curl", - "section": "net", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - { - "package": "rsync", - "role_name": "rsync", - "section": "net", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - { - "package": "vim", - "role_name": "vim", - "section": "editors", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - { - "package": "nginx", - "role_name": "nginx", - "section": "httpd", - "has_config": True, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - ] - ) - _write_state(bundle, state) - - manifest.manifest(str(bundle), str(out)) - - assert (out / "roles" / "net").exists() - assert (out / "roles" / "editors").exists() - assert (out / "roles" / "httpd").exists() - assert not (out / "roles" / "curl").exists() - assert not (out / "roles" / "rsync").exists() - assert not (out / "roles" / "vim").exists() - assert not (out / "roles" / "nginx").exists() - - net_defaults = (out / "roles" / "net" / "defaults" / "main.yml").read_text( - encoding="utf-8" - ) - assert "- curl" in net_defaults - assert "- rsync" in net_defaults - - pb = (out / "playbook.yml").read_text(encoding="utf-8") - assert "role: net" in pb - assert "role: editors" in pb - assert "role: httpd" in pb - - -def test_manifest_no_common_roles_preserves_package_roles(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - state = _minimal_package_state( - [ - { - "package": "curl", - "role_name": "curl", - "section": "net", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - { - "package": "vim", - "role_name": "vim", - "section": "editors", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - ] - ) - _write_state(bundle, state) - - manifest.manifest(str(bundle), str(out), no_common_roles=True) - - assert (out / "roles" / "curl").exists() - assert (out / "roles" / "vim").exists() - assert not (out / "roles" / "net").exists() - assert not (out / "roles" / "editors").exists() - - -def test_manifest_groups_excluded_package_paths_into_common_roles(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - state = _minimal_package_state( - [ - { - "package": "secret-agent", - "role_name": "secret_agent", - "section": "net", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [ - {"path": "/etc/secret-agent/key", "reason": "possible_secret"} - ], - "notes": [], - } - ] - ) - _write_state(bundle, state) - - manifest.manifest(str(bundle), str(out)) - - assert (out / "roles" / "net").exists() - assert not (out / "roles" / "secret_agent").exists() - readme = (out / "roles" / "net" / "README.md").read_text(encoding="utf-8") - assert "/etc/secret-agent/key" in readme - - -def test_manifest_groups_managed_package_config_into_common_role(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - (bundle / "artifacts" / "nginx" / "etc" / "nginx").mkdir( - parents=True, exist_ok=True - ) - (bundle / "artifacts" / "nginx" / "etc" / "nginx" / "nginx.conf").write_text( - "worker_processes auto;\n", encoding="utf-8" - ) - state = _minimal_package_state( - [ - { - "package": "nginx", - "role_name": "nginx", - "section": "httpd", - "has_config": True, - "managed_files": [ - { - "path": "/etc/nginx/nginx.conf", - "src_rel": "etc/nginx/nginx.conf", - "owner": "root", - "group": "root", - "mode": "0644", - "reason": "modified_conffile", - } - ], - "managed_dirs": [ - { - "path": "/etc/nginx", - "owner": "root", - "group": "root", - "mode": "0755", - "reason": "parent_of_managed_file", - } - ], - "managed_links": [], - "excluded": [], - "notes": [], - } - ] - ) - _write_state(bundle, state) - - manifest.manifest(str(bundle), str(out)) - - assert (out / "roles" / "httpd").exists() - assert not (out / "roles" / "nginx").exists() - defaults = (out / "roles" / "httpd" / "defaults" / "main.yml").read_text( - encoding="utf-8" - ) - assert "- nginx" in defaults - assert "dest: /etc/nginx/nginx.conf" in defaults - assert (out / "roles" / "httpd" / "files" / "etc" / "nginx" / "nginx.conf").exists() - - -def test_manifest_groups_systemd_units_into_common_role(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - (bundle / "artifacts" / "network_manager" / "etc" / "NetworkManager").mkdir( - parents=True, exist_ok=True - ) - ( - bundle - / "artifacts" - / "network_manager" - / "etc" - / "NetworkManager" - / "NetworkManager.conf" - ).write_text("[main]\n", encoding="utf-8") - - state = { - "schema_version": 3, - "host": {"hostname": "test", "os": "debian", "pkg_backend": "dpkg"}, - "inventory": { - "packages": { - "network-manager": { - "version": "1.0", - "arches": ["amd64"], - "installations": [ - {"version": "1.0", "arch": "amd64", "section": "net"} - ], - "section": "net", - "observed_via": [ - {"kind": "systemd_unit", "ref": "NetworkManager.service"} - ], - "roles": ["network_manager", "network_manager_dispatcher"], - } - } - }, - "roles": { - "users": { - "role_name": "users", - "users": [], - "managed_files": [], - "excluded": [], - "notes": [], - }, - "services": [ - { - "unit": "NetworkManager.service", - "role_name": "network_manager", - "packages": ["network-manager"], - "active_state": "active", - "sub_state": "running", - "unit_file_state": "enabled", - "condition_result": "yes", - "managed_files": [ - { - "path": "/etc/NetworkManager/NetworkManager.conf", - "src_rel": "etc/NetworkManager/NetworkManager.conf", - "owner": "root", - "group": "root", - "mode": "0644", - "reason": "modified_conffile", - } - ], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - { - "unit": "NetworkManager-dispatcher.service", - "role_name": "network_manager_dispatcher", - "packages": ["network-manager"], - "active_state": "inactive", - "sub_state": "dead", - "unit_file_state": "enabled", - "condition_result": "no", - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - }, - ], - "packages": [], - "apt_config": { - "role_name": "apt_config", - "managed_files": [], - "excluded": [], - "notes": [], - }, - "dnf_config": { - "role_name": "dnf_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": [], - }, - }, - } - _write_state(bundle, state) - - manifest.manifest(str(bundle), str(out)) - - assert (out / "roles" / "net").exists() - assert not (out / "roles" / "network_manager").exists() - assert not (out / "roles" / "network_manager_dispatcher").exists() - defaults = (out / "roles" / "net" / "defaults" / "main.yml").read_text( - encoding="utf-8" - ) - assert "- network-manager" in defaults - assert "name: NetworkManager.service" in defaults - assert "name: NetworkManager-dispatcher.service" in defaults - assert "dest: /etc/NetworkManager/NetworkManager.conf" in defaults - tasks = (out / "roles" / "net" / "tasks" / "main.yml").read_text(encoding="utf-8") - assert "Ensure grouped unit enablement matches harvest" in tasks - - -def test_manifest_fqdn_implies_no_common_roles(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - state = _minimal_package_state( - [ - { - "package": "curl", - "role_name": "curl", - "section": "net", - "has_config": False, - "managed_files": [], - "managed_dirs": [], - "managed_links": [], - "excluded": [], - "notes": [], - } - ] - ) - _write_state(bundle, state) - - manifest.manifest(str(bundle), str(out), fqdn="host1.example.test") - - assert (out / "roles" / "curl").exists() - assert not (out / "roles" / "net").exists() - - def test_manifest_site_mode_creates_host_inventory_and_raw_files(tmp_path: Path): """In --fqdn mode, host-specific state goes into inventory/host_vars.""" @@ -1064,10 +631,10 @@ def test_manifest_orders_cron_and_logrotate_at_playbook_tail(tmp_path: Path): ln.strip().removeprefix("- ").strip() for ln in pb if ln.startswith(" - ") ] - # Ensure the grouped role containing cron/logrotate is still ordered after users. - assert roles[-1] == "role: misc" - assert roles.index("role: users") < roles.index("role: misc") + # Ensure tail ordering. + 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): @@ -1800,7 +1367,7 @@ def test_manifest_avoids_package_role_collision_with_flatpak_singleton(tmp_path) bundle.mkdir(parents=True, exist_ok=True) (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") - manifest.manifest(str(bundle), str(out), no_common_roles=True) + manifest.manifest(str(bundle), str(out)) flatpak_defaults = (out / "roles" / "flatpak" / "defaults" / "main.yml").read_text( encoding="utf-8" diff --git a/tests/test_rpm.py b/tests/test_rpm.py index ff8c1e7..3aeccc8 100644 --- a/tests/test_rpm.py +++ b/tests/test_rpm.py @@ -149,9 +149,9 @@ def test_list_manual_packages_uses_yum_fallback(monkeypatch): def test_list_installed_packages_parses_epoch_and_sorts(monkeypatch): out = ( - "bash\t0\t5.2.26\t1.el9\tx86_64\tSystem Environment/Shells\n" - "bash\t1\t5.2.26\t1.el9\taarch64\tSystem Environment/Shells\n" - "coreutils\t(none)\t9.1\t2.el9\tx86_64\tSystem Environment/Base\n" + "bash\t0\t5.2.26\t1.el9\tx86_64\n" + "bash\t1\t5.2.26\t1.el9\taarch64\n" + "coreutils\t(none)\t9.1\t2.el9\tx86_64\n" ) monkeypatch.setattr( rpm, "_run", lambda cmd, allow_fail=False, merge_err=False: (0, out) @@ -159,7 +159,6 @@ def test_list_installed_packages_parses_epoch_and_sorts(monkeypatch): pkgs = rpm.list_installed_packages() assert pkgs["bash"][0]["arch"] == "aarch64" # sorted by arch then version assert pkgs["bash"][0]["version"].startswith("1:") - assert pkgs["bash"][0]["group"] == "System Environment/Shells" assert pkgs["coreutils"][0]["version"] == "9.1-2.el9"