diff --git a/CHANGELOG.md b/CHANGELOG.md index 39a3a55..26525ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,32 @@ # Changelog +## v0.3.2 — 2026-07-13 + +### Fixed + +- **`install.sh --disable-nested-snapshots` did not actually disable anything + until the next reboot.** `apply.sh` only ever *added* patches — there was no + revert path. Disabling removed the opt-in marker and then merely *skipped* + re-applying, but the overlay persists for the whole boot, so the previously + patched `plugins/cloud/{snapshot,crud}.py`, `plugins/cloud_backup/sync.py` and + `_truecloud_nested.py` were all still sitting there — and middlewared + re-imported them on the restart `install.sh` performs. + + It printed *"DISABLED (stock guard restored)"* while the feature kept running. + Someone turning it off *because they were worried about it* would have believed + it was off. + + `apply.sh` now actively reverts: it removes the module first (every injected + block is guarded by `if _tc_nested is not None`, so the stock guard is restored + even if a later step fails), then strips its appended blocks from the three + patched files. `restic.py` also carries a `TRUECLOUD_PATCH` block but belongs to + the *providers* module and is deliberately left alone — reverting it would break + B2 backups. `install.sh --disable` also tears down any staging tree first, since + those bind mounts pin ZFS snapshots that could otherwise never be destroyed. + + Updating **without** the flag was always correct and is unchanged: the nested + module is never installed into middleware unless it is explicitly enabled. + ## v0.3.1 — 2026-07-13 ### Added diff --git a/install.sh b/install.sh index b23aee0..c998823 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ set -euo pipefail -VERSION="0.3.1" +VERSION="0.3.2" # The directory containing install.sh is the permanent install location. PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" @@ -160,10 +160,17 @@ case "$_nested_choice" in off) if [ -f "$_NESTED_MARKER" ]; then rm -f "$_NESTED_MARKER" - echo "Nested-dataset snapshots: DISABLED (stock guard restored)." + # Tear down any staging tree first: those bind mounts PIN their ZFS + # snapshots, so leaving them would block those snapshots from ever + # being destroyed. apply.sh (below) then reverts the patched files. + python3 "$PATCH_DIR/patch/truecloud_nested.py" cleanup || \ + echo " WARNING: staging mounts remain; unmount them manually." + echo "Nested-dataset snapshots: DISABLED." + echo " apply.sh will revert the patched middleware files and the stock" + echo " guard is restored when middlewared restarts (this script does that)." echo " Any task that already has snapshot=true on a nested dataset will" echo " fail validation on its next edit. Turn the option off on those" - echo " tasks, or re-run with --enable-nested-snapshots." + echo " tasks first, or re-run with --enable-nested-snapshots." else echo "Nested-dataset snapshots: already disabled." fi diff --git a/patch/apply.sh b/patch/apply.sh index ebfd23d..7c4cc09 100755 --- a/patch/apply.sh +++ b/patch/apply.sh @@ -32,7 +32,7 @@ # Derive PATCH_DIR from this script's location (parent of the patch/ directory). PATCH_DIR="$(cd "$(dirname "$0")/.." && pwd)" LOG="$PATCH_DIR/apply.log" -VERSION="0.3.1" +VERSION="0.3.2" # Rotate log at 512 KB to avoid unbounded growth on a system volume. # Keep two prior generations (.1 and .2) so the last three boots are always available. @@ -477,6 +477,57 @@ def patch_file(path, block): with open(path, "w", encoding="utf-8") as fh: fh.write(base.rstrip("\n") + "\n" + block) + +def unpatch_file(path): + """Strip our appended block, restoring the stock file. True if it was patched.""" + try: + with open(path, encoding="utf-8") as fh: + content = fh.read() + except OSError: + return False + idx = content.find("\n# TRUECLOUD_PATCH") + if idx == -1: + return False + try: + with open(path, "w", encoding="utf-8") as fh: + fh.write(content[:idx].rstrip("\n") + "\n") + except OSError: + return False + return True + + +def revert_nested(cloud_dir, sync_path): + """Undo the nested patch. Returns the names of what was actually reverted. + + Skipping the patch is NOT enough to disable the feature. The overlay persists + for the whole boot, so an earlier run this boot may already have written the + patched files -- and middlewared re-imports them on the restart that + install.sh performs. Without this, `install.sh --disable-nested-snapshots` + would report "disabled" while the feature kept running until the next reboot. + """ + reverted = [] + + # Remove the module FIRST. Every injected block is guarded by + # `if _tc_nested is not None`, so once it is gone they all no-op even if a + # later step here fails -- the guard is restored no matter what. + try: + os.unlink(os.path.join(cloud_dir, '_truecloud_nested.py')) + reverted.append('_truecloud_nested.py') + except OSError: + pass + + # NB: restic.py also carries a TRUECLOUD_PATCH block, but that belongs to the + # providers module. Only these three are ours to revert. + for name, path in ( + ('crud.py', os.path.join(cloud_dir, 'crud.py')), + ('sync.py', sync_path), + ('snapshot.py', os.path.join(cloud_dir, 'snapshot.py')), + ): + if unpatch_file(path): + reverted.append(name) + + return reverted + b2_ok = restic_ok = False nested_ok = False @@ -517,16 +568,28 @@ else: # guard LAST. If anything fails partway, the guard is still in place and the # option stays unavailable -- we never expose "guard removed, traversal missing". nested_detail = '' -if not nested_enabled: - nested_detail = 'disabled (opt-in; enable with: install.sh --enable-nested-snapshots)' - print('INFO: Nested-dataset snapshot support is disabled (opt-in feature).') - print('INFO: Enable with: bash install.sh --enable-nested-snapshots') -elif nested_native: - nested_detail = 'superseded: TrueNAS handles nested-dataset snapshots natively' - print('INFO: Nested module skipped — TrueNAS now handles nesting natively.') -elif not nested_needed: - nested_detail = 'not needed' - print('INFO: Nested module skipped.') +if not nested_needed: + # Not just "skip": actively revert. The overlay lives for the whole boot, so a + # previously-applied patch is still sitting there and middlewared would + # re-import it on restart. See revert_nested(). + if not nested_enabled: + nested_detail = 'disabled (opt-in; enable with: install.sh --enable-nested-snapshots)' + print('INFO: Nested-dataset snapshot support is disabled (opt-in feature).') + elif nested_native: + nested_detail = 'superseded: TrueNAS handles nested-dataset snapshots natively' + print('INFO: Nested module skipped — TrueNAS now handles nesting natively.') + else: + nested_detail = 'not needed' + print('INFO: Nested module skipped.') + + reverted = revert_nested(cloud_dir, sync_path) + if reverted: + print('OK: Reverted a previously-applied nested patch (' + ', '.join(reverted) + ').') + print(' The stock nesting guard is restored once middlewared restarts.') + nested_detail += ' — previous patch reverted' + + if not nested_enabled: + print('INFO: Enable with: bash install.sh --enable-nested-snapshots') else: try: snapshot_py = os.path.join(cloud_dir, 'snapshot.py') diff --git a/recover.sh b/recover.sh index a479664..78870a8 100755 --- a/recover.sh +++ b/recover.sh @@ -17,7 +17,7 @@ # bash /mnt/tank/truenas-truecloud-patch/patch/apply.sh # systemctl restart middlewared -VERSION="0.3.1" +VERSION="0.3.2" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" diff --git a/tests/test_apply_blocks.py b/tests/test_apply_blocks.py index d8fe6d5..f0a67af 100644 --- a/tests/test_apply_blocks.py +++ b/tests/test_apply_blocks.py @@ -263,10 +263,57 @@ class TestOptIn: def test_patching_is_skipped_entirely_when_disabled(self): # The guard-relaxing crud.py patch must be inside the enabled branch. src = heredoc_source() - gate = src.index("if not nested_enabled:") + gate = src.index("if not nested_needed:") crud = src.index("patch_file(crud_py, CRUD_BLOCK)") assert gate < crud, "crud.py patch must sit inside the opt-in branch" + def test_disabling_REVERTS_the_patch_rather_than_merely_skipping_it(self): + """Skipping is not disabling. + + The overlay persists for the whole boot, so a patch applied by an earlier + run this boot is still on disk — and middlewared re-imports it on the + restart install.sh performs. Without an active revert, + `--disable-nested-snapshots` reports "disabled" while the feature keeps + running until the next reboot. + """ + src = heredoc_source() + assert "def unpatch_file(" in src + assert "def revert_nested(" in src + # The revert must run on every not-needed path (opt-out, superseded). + gate = src.index("if not nested_needed:") + revert = src.index("reverted = revert_nested(") + patch = src.index("patch_file(crud_py, CRUD_BLOCK)") + assert gate < revert < patch, "revert belongs in the not-needed branch" + + def test_revert_removes_the_module_before_unpatching_files(self): + # Every injected block is guarded by `if _tc_nested is not None`, so + # deleting the module first means the guard is restored even if a later + # unpatch step fails. + src = heredoc_source() + body = src[src.index("def revert_nested("):src.index("def patch_file(") if + src.index("def patch_file(") > src.index("def revert_nested(") else len(src)] + body = src[src.index("def revert_nested("):] + body = body[:body.index("\n\n\n")] if "\n\n\n" in body else body + assert body.index("_truecloud_nested.py") < body.index("crud.py") + + def test_revert_never_touches_the_providers_patch(self): + # restic.py also carries a TRUECLOUD_PATCH block, but it belongs to the + # providers module. Reverting it would silently break B2 backups. + src = heredoc_source() + body = src[src.index("def revert_nested("):] + body = body[:body.index("return reverted")] + # Comments legitimately *mention* restic.py to explain why it is excluded; + # what matters is that no code line touches it. + code = "\n".join( + ln for ln in body.splitlines() if not ln.lstrip().startswith("#") + ) + assert "restic" not in code + assert "b2.py" not in code + # It must only ever revert these three, plus the module itself. + assert "crud.py" in code + assert "sync_path" in code + assert "snapshot.py" in code + def test_guard_is_relaxed_only_after_traversal_is_installed(): # Ordering in apply.sh is a safety property: copy module -> patch snapshot.py diff --git a/uninstall.sh b/uninstall.sh index a24ea15..1a1436b 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -3,7 +3,7 @@ set -euo pipefail -VERSION="0.3.1" +VERSION="0.3.2" PATCH_DIR="$(cd "$(dirname "$0")" && pwd)" _HOOK_COMMENT='TrueCloud provider patch (S3/B2)'