From bee405bd528e05989cf0615f32e2cf6122564748 Mon Sep 17 00:00:00 2001 From: sudolulo Date: Mon, 15 Jun 2026 03:15:08 +0000 Subject: [PATCH] Fix six quality findings from code review sitecustomize.py: - Restore _record_status count barrier: write the status file only after all patches have reported. middlewared.plugins.cloud_backup.restic is imported lazily (only when a backup task runs), so without this barrier verify would declare "all patches active" based solely on the B2 patch that fires at startup. Barrier now gates on _Finder._targets rather than the removed _PATCHES dict. - Add comment in exec_module noting the if/elif must stay in sync with _Finder._targets, making the coupling visible. patch_ui.py: - Merge find_webui into find_bundle: previously find_bundle(None) would crash with os.walk(None) if main()'s guard were removed. Merged function returns a 3-tuple (webui_dir, path, content); webui_dir=None means no candidate directory found, path=None means directory found but pattern absent. main() still produces distinct messages for each failure mode. create_task.py: - Split triple-chained .get() in cmd_list_tasks into two lines; the or {} handling for None credentials was buried inside a one-liner. uninstall.sh: - Fix find loop: replace "for x in $(find ...)" with "while IFS= read -r" to handle paths containing spaces or newlines. - Add #!/usr/bin/env shebang form to Python detection, matching apply.sh. Without this, uninstall on a system where middlewared uses the env form would silently leave sitecustomize.py in the wrong site-packages. --- patch/create_task.py | 3 ++- patch/patch_ui.py | 30 ++++++++++++++---------------- patch/sitecustomize.py | 3 +++ uninstall.sh | 8 +++++--- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/patch/create_task.py b/patch/create_task.py index 107e78b..2839e69 100755 --- a/patch/create_task.py +++ b/patch/create_task.py @@ -139,7 +139,8 @@ def cmd_list_tasks(client, _args): print(f"{'ID':>4} {'Enabled':<8} {'Provider':<14} Name") print("─" * 60) for t in sorted(tasks, key=lambda x: x["id"]): - ptype = (t.get("credentials") or {}).get("provider", {}).get("type", "?") + creds = t.get("credentials") or {} + ptype = creds.get("provider", {}).get("type", "?") enabled = "yes" if t.get("enabled") else "no" print(f"{t['id']:>4} {enabled:<8} {ptype:<14} {t.get('description', '')}") diff --git a/patch/patch_ui.py b/patch/patch_ui.py index c0cafe7..79efb69 100644 --- a/patch/patch_ui.py +++ b/patch/patch_ui.py @@ -40,19 +40,17 @@ REPLACE = r'\1["STORJ_IX","S3","B2"]' MARKER = '"STORJ_IX","S3","B2"' -def find_webui(): - for d in WEBUI_CANDIDATES: - if os.path.isdir(d): - return d - return None - - -def find_bundle(webui): +def find_bundle(): """ - Walk the webui directory looking for the JS file that contains the - filterByProviders binding. Returns (path, content) or (None, None). - Only .js files are read; binary files and permission errors are skipped. + Search WEBUI_CANDIDATES for the JS bundle containing the filterByProviders + binding. Returns (webui_dir, path, content). webui_dir is None if no + candidate directory exists; path is None if the directory exists but the + pattern is not found in any bundle. """ + webui = next((d for d in WEBUI_CANDIDATES if os.path.isdir(d)), None) + if webui is None: + return None, None, None + matches = [] for root, _dirs, names in os.walk(webui): for name in sorted(names): # deterministic order @@ -68,7 +66,7 @@ def find_bundle(webui): continue if not matches: - return None, None + return webui, None, None if len(matches) > 1: # Unexpected — log all matches so the operator can investigate. @@ -79,19 +77,19 @@ def find_bundle(webui): for p, _ in matches: print(f"[truecloud-patch] {p}") - return matches[0] + path, content = matches[0] + return webui, path, content def main(): - webui = find_webui() - if not webui: + webui, path, content = find_bundle() + if webui is None: print( "[truecloud-patch] WARNING: webui directory not found; skipping UI patch.\n" "[truecloud-patch] Searched: " + ", ".join(WEBUI_CANDIDATES) ) return - path, content = find_bundle(webui) if path is None: print( "[truecloud-patch] WARNING: filterByProviders pattern not found in any JS bundle.\n" diff --git a/patch/sitecustomize.py b/patch/sitecustomize.py index f034c5f..938aea8 100644 --- a/patch/sitecustomize.py +++ b/patch/sitecustomize.py @@ -105,6 +105,7 @@ class _Loader: self._finder._mark_done(fullname) # exec_module succeeded — apply our patch. + # Must stay in sync with _Finder._targets. try: if fullname == "middlewared.rclone.remote.b2": _patch_b2(module) @@ -187,6 +188,8 @@ def _record_status(fullname: str, ok: bool, detail: str = "") -> None: if fullname in _hook_status: return # idempotent: first call wins _hook_status[fullname] = {"ok": ok, "detail": detail} + if len(_hook_status) < len(_Finder._targets): + return # wait until all patches have reported before writing payload = { "patched_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), diff --git a/uninstall.sh b/uninstall.sh index a63a625..431cfd7 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -53,6 +53,8 @@ if [ -x /usr/bin/middlewared ]; then shebang=$(dd if=/usr/bin/middlewared bs=256 count=1 2>/dev/null | head -1 || true) if [[ "$shebang" =~ ^'#!'(/[^[:space:]]+python[^[:space:]]*) ]]; then PYTHON="${BASH_REMATCH[1]}" + elif [[ "$shebang" =~ ^'#!/usr/bin/env '(python[^[:space:]]*) ]]; then + PYTHON=$(command -v "${BASH_REMATCH[1]}" 2>/dev/null || echo "python3") fi fi @@ -81,13 +83,13 @@ echo "" echo "Restoring UI bundle backup ..." RESTORED=0 -for backup in $(find /usr/share/truenas /var/www/truenas \ - -name "*.js.pre-truecloud-patch" 2>/dev/null); do +while IFS= read -r backup; do original="${backup%.pre-truecloud-patch}" mv "$backup" "$original" echo " Restored: $original" RESTORED=1 -done +done < <(find /usr/share/truenas /var/www/truenas \ + -name "*.js.pre-truecloud-patch" 2>/dev/null) if [ "$RESTORED" -eq 0 ]; then echo " No backup files found."