From 85345083a3f33a20b1c4c4185d03c60046828256 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Tue, 30 Jun 2026 12:34:26 +1000 Subject: [PATCH] Remove reference to --jinjaturtle auto/on/off (it's a boolean arg). Fix alma python --- .forgejo/workflows/ci.yml | 4 +++- DEVELOPMENT.md | 20 ++++++++++---------- enroll/ansible.py | 6 +++--- enroll/cli.py | 8 ++++---- enroll/jinjaturtle.py | 24 +++++++++++++----------- enroll/manifest.py | 2 +- tests.sh | 39 +++++++++++++++++++++++++++++++++++++-- tests/test_cli.py | 6 +++--- tests/test_jinjaturtle.py | 2 +- tests/test_manifest.py | 2 +- 10 files changed, 76 insertions(+), 37 deletions(-) diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 77a7f9c..758efef 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: image: docker.io/library/debian:13 python: python3 - distro: almalinux - image: docker.io/library/almalinux:10 + image: docker.io/library/almalinux:9 python: python3.11 container: @@ -95,6 +95,8 @@ jobs: chmod +x /usr/local/bin/sops - name: Run test script + env: + PYTHON_BIN: ${{ matrix.python }} run: | ./tests.sh diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 674bac7..fc50d1e 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -777,7 +777,7 @@ manifest( bundle_dir, out, fqdn=None, - jinjaturtle="auto", + jinjaturtle=None, sops_fingerprints=None, no_common_roles=False, target="ansible", @@ -876,7 +876,7 @@ ansible.manifest_from_bundle_dir( bundle_dir, out_dir, fqdn=None, - jinjaturtle="auto", + jinjaturtle=None, no_common_roles=False, ) ``` @@ -992,14 +992,14 @@ File: `jinjaturtle.py` JinjaTurtle mode is resolved by: ```python -resolve_jinjaturtle_mode("auto" | "on" | "off") +resolve_jinjaturtle_mode(None | True | False) ``` Semantics: -- `auto`: use `jinjaturtle` when it exists on `PATH`; otherwise copy raw files. -- `on`: require `jinjaturtle`; error if missing. -- `off`: never use it. +- `None`: use `jinjaturtle` when it exists on `PATH`; otherwise copy raw files. +- `True`: require `jinjaturtle`; error if missing. +- `False`: never use it. Supported path types include structured config suffixes: @@ -1285,12 +1285,12 @@ Disables the default grouping of package/service snapshots by Debian Section or ### 20.3 `--jinjaturtle` / `--no-jinjaturtle` -The CLI maps these to renderer mode strings: +The CLI exposes these as boolean flags, not as options that take values: ```text -no flag -> auto ---jinjaturtle -> on ---no-jinjaturtle -> off +no flag -> use jinjaturtle when it exists on PATH +--jinjaturtle -> require jinjaturtle; fail if it is missing +--no-jinjaturtle -> disable jinjaturtle ``` --- diff --git a/enroll/ansible.py b/enroll/ansible.py index 76399f9..f93dd64 100644 --- a/enroll/ansible.py +++ b/enroll/ansible.py @@ -418,7 +418,7 @@ def _prepare_ansible_context( out_dir: str, *, fqdn: Optional[str], - jinjaturtle: str, + jinjaturtle: Optional[bool], ) -> AnsibleManifestContext: site_mode = fqdn is not None and fqdn != "" jt_exe, jt_enabled = resolve_jinjaturtle_mode(jinjaturtle) @@ -2400,7 +2400,7 @@ class AnsibleManifestRenderer: out_dir: str, *, fqdn: Optional[str] = None, - jinjaturtle: str = "auto", + jinjaturtle: Optional[bool] = None, no_common_roles: bool = False, ) -> None: self.bundle_dir = bundle_dir @@ -2499,7 +2499,7 @@ def manifest_from_bundle_dir( out_dir: str, *, fqdn: Optional[str] = None, - jinjaturtle: str = "auto", + jinjaturtle: Optional[bool] = None, no_common_roles: bool = False, ) -> None: AnsibleManifestRenderer( diff --git a/enroll/cli.py b/enroll/cli.py index 604991e..a123fd1 100644 --- a/enroll/cli.py +++ b/enroll/cli.py @@ -481,12 +481,12 @@ def _add_common_manifest_args(p: argparse.ArgumentParser) -> None: ) -def _jt_mode(args: argparse.Namespace) -> str: +def _jt_mode(args: argparse.Namespace) -> Optional[bool]: if getattr(args, "jinjaturtle", False): - return "on" + return True if getattr(args, "no_jinjaturtle", False): - return "off" - return "auto" + return False + return None def _add_config_args(p: argparse.ArgumentParser) -> None: diff --git a/enroll/jinjaturtle.py b/enroll/jinjaturtle.py index 38c6f61..f6078c2 100644 --- a/enroll/jinjaturtle.py +++ b/enroll/jinjaturtle.py @@ -43,24 +43,26 @@ SUPPORTED_SUFFIXES = { } | SYSTEMD_SUFFIXES -def resolve_jinjaturtle_mode(jinjaturtle: str) -> Tuple[Optional[str], bool]: - """Resolve Enroll's common JinjaTurtle mode flag. +def resolve_jinjaturtle_mode( + jinjaturtle: Optional[bool] = None, +) -> Tuple[Optional[str], bool]: + """Resolve Enroll's common JinjaTurtle flag. - Renderers accept the same values: - - ``auto``: use JinjaTurtle when present on PATH - - ``on``: require it and fail if it is absent - - ``off``: never use it + ``None`` means the default behaviour: use JinjaTurtle when its executable + is present on PATH. ``True`` corresponds to ``--jinjaturtle`` and requires + the executable. ``False`` corresponds to ``--no-jinjaturtle`` and disables + the integration. """ jt_exe = find_jinjaturtle_cmd() - if jinjaturtle not in {"auto", "on", "off"}: - raise ValueError("jinjaturtle must be one of: auto, on, off") - if jinjaturtle == "on": + if jinjaturtle is True: if not jt_exe: raise RuntimeError("jinjaturtle requested but not found on PATH") return jt_exe, True - if jinjaturtle == "auto": + if jinjaturtle is False: + return jt_exe, False + if jinjaturtle is None: return jt_exe, jt_exe is not None - return jt_exe, False + raise TypeError("jinjaturtle must be None, True, or False") def _merge_mappings_overwrite( diff --git a/enroll/manifest.py b/enroll/manifest.py index b607020..de8fe5e 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -189,7 +189,7 @@ def manifest( out: str, *, fqdn: Optional[str] = None, - jinjaturtle: str = "auto", # auto|on|off + jinjaturtle: Optional[bool] = None, sops_fingerprints: Optional[List[str]] = None, no_common_roles: bool = False, ) -> Optional[str]: diff --git a/tests.sh b/tests.sh index ace310b..5b0bc6c 100755 --- a/tests.sh +++ b/tests.sh @@ -137,6 +137,38 @@ is_rpm_family() { esac } +preferred_python_bin() { + if [[ -n "${ENROLL_TEST_PYTHON_BIN:-}" ]]; then + printf '%s' "${ENROLL_TEST_PYTHON_BIN}" + return + fi + if [[ -n "${PYTHON_BIN:-}" ]]; then + printf '%s' "${PYTHON_BIN}" + return + fi + if command -v python3.11 >/dev/null 2>&1; then + printf '%s' "python3.11" + return + fi + printf '%s' "python3" +} + +require_python_for_jinjaturtle() { + local py_bin + py_bin="$(preferred_python_bin)" + command -v "${py_bin}" >/dev/null 2>&1 \ + || fail "Python interpreter '${py_bin}' was not found; set PYTHON_BIN or ENROLL_TEST_PYTHON_BIN." + "${py_bin}" - <<'PY' +import sys +if sys.version_info < (3, 10): + raise SystemExit( + f"Python {sys.version.split()[0]} is too old for JinjaTurtle; " + "install/use Python >= 3.10 (for AlmaLinux 9, set PYTHON_BIN=python3.11)." + ) +PY + printf '%s' "${py_bin}" +} + pkg_update_once() { if is_debian; then if [[ -z "${APT_UPDATED:-}" ]]; then @@ -292,11 +324,14 @@ ensure_jinjaturtle_from_git() { if is_debian; then pkg_install ca-certificates git python3 python3-venv elif is_rpm_family; then - pkg_install ca-certificates git python3 + pkg_install ca-certificates git python3.11 python3.11-pip else fail "Unsupported OS for JinjaTurtle git build: $(os_id)." fi + local jt_python + jt_python="$(require_python_for_jinjaturtle)" + local src="${ENROLL_TEST_JINJATURTLE_SOURCE:-}" local cloned=0 if [[ -z "${src}" ]]; then @@ -316,7 +351,7 @@ ensure_jinjaturtle_from_git() { # the console script on PATH. Enroll only needs shutil.which("jinjaturtle") # to succeed, which a symlink into /usr/local/bin satisfies. rm -rf "${JINJATURTLE_VENV_DIR}" - run python3 -m venv "${JINJATURTLE_VENV_DIR}" + run "${jt_python}" -m venv "${JINJATURTLE_VENV_DIR}" run "${JINJATURTLE_VENV_DIR}/bin/pip" install --upgrade pip run "${JINJATURTLE_VENV_DIR}/bin/pip" install "${src}" diff --git a/tests/test_cli.py b/tests/test_cli.py index a5bed5d..51b4222 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -67,7 +67,7 @@ def test_cli_manifest_subcommand_calls_manifest(monkeypatch, tmp_path): assert called["harvest"] == str(tmp_path / "bundle") assert called["out"] == str(tmp_path / "ansible") assert called["fqdn"] is None - assert called["jinjaturtle"] == "auto" + assert called["jinjaturtle"] is None assert called["no_common_roles"] is False @@ -202,7 +202,7 @@ def test_cli_enroll_subcommand_runs_harvest_then_manifest(monkeypatch, tmp_path) cli.main() assert calls == [ ("harvest", str(tmp_path / "bundle"), False, [], []), - ("manifest", str(tmp_path / "bundle"), str(tmp_path / "ansible"), None, "auto"), + ("manifest", str(tmp_path / "bundle"), str(tmp_path / "ansible"), None, None), ] @@ -491,7 +491,7 @@ def test_cli_manifest_common_args(monkeypatch, tmp_path): cli.main() assert called["fqdn"] == "example.test" - assert called["jinjaturtle"] == "off" + assert called["jinjaturtle"] is False def test_cli_explain_passes_args_and_writes_stdout(monkeypatch, capsys, tmp_path): diff --git a/tests/test_jinjaturtle.py b/tests/test_jinjaturtle.py index 9d43650..1a85976 100644 --- a/tests/test_jinjaturtle.py +++ b/tests/test_jinjaturtle.py @@ -127,7 +127,7 @@ def test_manifest_uses_jinjaturtle_templates_and_does_not_copy_raw( monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", fake_run_jinjaturtle) - manifest_mod.manifest(str(bundle), str(out), jinjaturtle="on") + manifest_mod.manifest(str(bundle), str(out), jinjaturtle=True) role_dir = out / "roles" / "utils" diff --git a/tests/test_manifest.py b/tests/test_manifest.py index a2259f5..1d177d2 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -1541,7 +1541,7 @@ def test_manifest_applies_jinjaturtle_to_jinjifyable_managed_file( monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", lambda *a, **k: _Res()) out_dir = tmp_path / "out" - manifest.manifest(str(bundle), str(out_dir), jinjaturtle="on") + manifest.manifest(str(bundle), str(out_dir), jinjaturtle=True) tmpl = out_dir / "roles" / "apt_config" / "templates" / "etc" / "apt" / "foo.ini.j2" assert tmpl.exists()