mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-26 13:39:47 +00:00
Compose returned timer discovery repair for effective mask correction
Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot]
@@ -15,6 +15,29 @@
|
||||
|
||||
## Purpose
|
||||
|
||||
### Existing auto-update timers are discovered without changing consent
|
||||
|
||||
Server installer timer discovery is a bounded, exact-unit `systemctl` inventory.
|
||||
It compares the first field of complete rows, including their state/preset
|
||||
columns, without regex matching or truncating custom instance names. A failed
|
||||
inventory's output is not proof of an installed timer.
|
||||
|
||||
All five existing/new-install refresh consumers can therefore replace stale
|
||||
auto-update assets when a timer exists, without enabling/starting it or changing
|
||||
`system.json`. A configured timer or updater masked by a `/dev/null` symlink is
|
||||
an explicit operator stop: refresh retains the mask and all existing assets.
|
||||
Discovery is separate from enablement; version changes still require affirmative
|
||||
consent to set up automatic updates.
|
||||
|
||||
`TestRootInstallTimerDiscoveryReachesConsentPreservingRefresh` runs actual
|
||||
discovery, installer `main` and asset refresh together for version pins, source
|
||||
builds, menu updates/reinstalls and leftover fresh-install timers. Its controls
|
||||
cover enabled/disabled rows, exact-name impostors, long instance names, missing/
|
||||
failed/hung inventory, byte-preserved preferences and both unit masks. Where
|
||||
available, real `systemctl --root` reads only a temporary fixture filesystem.
|
||||
Manager operations and server installation remain doubles: this is source/flow
|
||||
proof, not native systemd activation, published installation or customer recovery.
|
||||
|
||||
### Private, authenticated server-installer inputs
|
||||
|
||||
Release archive downloads and their signature sidecars stay inside the same
|
||||
|
||||
+18
-1
@@ -772,7 +772,15 @@ detect_service_name() {
|
||||
}
|
||||
|
||||
update_timer_exists() {
|
||||
command -v systemctl >/dev/null 2>&1 && systemctl list-unit-files --no-legend 2>/dev/null | grep -q "^${UPDATE_TIMER_UNIT}$"
|
||||
command -v systemctl >/dev/null 2>&1 || return 1
|
||||
local unit_files=""
|
||||
# Rows also contain enablement/preset columns. Match the exact first field,
|
||||
# not the whole row or a regex, and keep long custom unit names untruncated.
|
||||
# A failed inventory must not admit even plausible partial stdout.
|
||||
if ! unit_files=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$UPDATE_TIMER_UNIT" 2>/dev/null); then
|
||||
return 1
|
||||
fi
|
||||
awk -v unit="$UPDATE_TIMER_UNIT" '$1 == unit { found = 1 } END { exit !found }' <<< "$unit_files"
|
||||
}
|
||||
|
||||
update_timer_enabled() {
|
||||
@@ -4731,6 +4739,15 @@ setup_auto_updates() {
|
||||
# Refresh the installed assets unconditionally instead, without changing the
|
||||
# user's choices: system.json and the timer's enabled state stay untouched.
|
||||
refresh_auto_updates() {
|
||||
# Discovering an existing masked unit is not consent to unmask it. The
|
||||
# staged unit rename would otherwise replace its /dev/null symlink.
|
||||
local asset
|
||||
for asset in "${UPDATE_TIMER_PATH:-${PULSE_UPDATE_TIMER_PATH:-}}" "${UPDATE_SERVICE_PATH:-${PULSE_UPDATE_SERVICE_PATH:-}}"; do
|
||||
if [[ -L "$asset" && "$asset" -ef /dev/null ]]; then
|
||||
print_info "Auto-update unit is deliberately masked ($asset); preserving the existing helper and units."
|
||||
return 0
|
||||
fi
|
||||
done
|
||||
print_info "Refreshing the installed auto-update helper..."
|
||||
if ! install_auto_update_assets; then
|
||||
print_warn "Could not refresh the auto-update helper; the previously installed one may be stale."
|
||||
|
||||
@@ -4540,6 +4540,19 @@ func TestRootInstallTemporaryInputsRemainPrivateAndAuthenticated(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Execute discovery and every installer refresh consumer together. systemd
|
||||
// manager operations are confined doubles; real inventory uses a fixture root.
|
||||
func TestRootInstallTimerDiscoveryReachesConsentPreservingRefresh(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
cmd := exec.CommandContext(ctx, "python3", repoFile("scripts", "tests", "test_server_update_timer.py"), "-v")
|
||||
cmd.Env = append(os.Environ(), "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh"))
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err != nil {
|
||||
t.Fatalf("installer timer discovery/refresh controls: %v\n%s", err, out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRootInstallStableReleaseTagRejectsPrereleaseShapes(t *testing.T) {
|
||||
script := `
|
||||
` + extractRootInstallShellFunction(t, "is_stable_release_tag") + `
|
||||
|
||||
@@ -99,11 +99,73 @@ func TestTemperatureGuidanceKeepsExistingSafetyAndSourceMeaning(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, text := range []string{"thermal_zone0` is not necessarily the CPU", "millidegrees", "not physical-disk SMART", "--token-file", "--local-only", "--ssh-known-hosts", "--remove-proxmox-access", "not a signed release asset", "not an empty reading"} {
|
||||
for _, text := range []string{"thermal_zone0` is not necessarily the CPU", "millidegrees", "not physical-disk SMART", "--token-file", "--local-only", "--remove-proxmox-access", "not a signed release asset", "not an empty reading"} {
|
||||
if !strings.Contains(string(doc), text) {
|
||||
t.Fatalf("temperature guide loses existing scope or source distinction %q", text)
|
||||
}
|
||||
}
|
||||
if err := temperatureCleanupRecipeScope(string(doc)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
// The current guide deliberately uses local-only cleanup, not remote key
|
||||
// cleanup or trust enrolment. Check the executable recipe, not whether prose
|
||||
// happens to mention an optional cluster-SSH flag from older instructions.
|
||||
func temperatureCleanupRecipeScope(doc string) error {
|
||||
const helper = `"$HOME/.config/pulse/sensor-proxy-uninstall.sh"`
|
||||
count := 0
|
||||
for _, match := range regexp.MustCompile("(?s)```bash\n(.*?)```").FindAllStringSubmatch(doc, -1) {
|
||||
block := strings.ReplaceAll(match[1], "\\\n", " ")
|
||||
if !strings.Contains(block, "bash "+helper) {
|
||||
continue
|
||||
}
|
||||
count++
|
||||
fields := strings.Fields(block)
|
||||
if len(fields) != 4 || fields[0] != "bash" || fields[1] != helper ||
|
||||
!((fields[2] == "--uninstall" && fields[3] == "--local-only") ||
|
||||
(fields[2] == "--local-only" && fields[3] == "--uninstall")) {
|
||||
return fmt.Errorf("temperature cleanup recipe must preserve local-only scope without extra operations")
|
||||
}
|
||||
}
|
||||
if count != 1 {
|
||||
return fmt.Errorf("temperature guide needs one explicit local-only cleanup recipe, got %d", count)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestTemperatureCleanupRecipeScopeRejectsBroaderOrMissingActions(t *testing.T) {
|
||||
const command = `bash "$HOME/.config/pulse/sensor-proxy-uninstall.sh" --uninstall --local-only`
|
||||
wrap := func(command string) string {
|
||||
return "```bash\n" + command + "\n```\n"
|
||||
}
|
||||
for _, recipe := range []string{
|
||||
command,
|
||||
`bash "$HOME/.config/pulse/sensor-proxy-uninstall.sh" --local-only --uninstall`,
|
||||
"bash \"$HOME/.config/pulse/sensor-proxy-uninstall.sh\" \\\n --uninstall --local-only",
|
||||
} {
|
||||
// Warning prose can name dangerous flags without proposing their use.
|
||||
doc := wrap(recipe) + "Do not add --purge or --remove-proxmox-access.\n"
|
||||
if err := temperatureCleanupRecipeScope(doc); err != nil {
|
||||
t.Fatalf("safe local cleanup rejected: %v", err)
|
||||
}
|
||||
}
|
||||
for name, doc := range map[string]string{
|
||||
"no action": "Use local-only cleanup after planning maintenance.\n",
|
||||
"remote cleanup": wrap(strings.Replace(command, " --local-only", "", 1)),
|
||||
"purge": wrap(command + " --purge"),
|
||||
"remove access": wrap(command + " --remove-proxmox-access"),
|
||||
"cluster trust": wrap(command + " --ssh-known-hosts /path/to/known_hosts"),
|
||||
"replay": wrap(command) + wrap(command),
|
||||
"chained action": wrap(command + " && echo done"),
|
||||
"prose impostor": wrap(strings.Replace(command, " --local-only", "", 1)) + "Keep --local-only.\n",
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
if temperatureCleanupRecipeScope(doc) == nil {
|
||||
t.Fatal("unsafe or absent cleanup recipe passed")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestTemperatureGuidanceReadingCommandsArePrivateAndBounded(t *testing.T) {
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Actual installer timer discovery and refresh, confined to temporary files.
|
||||
|
||||
The server/install producers and systemd manager operations are doubles. The
|
||||
optional real systemctl inventory uses --root, never the host's unit manager.
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[2]
|
||||
INSTALLER = Path(os.environ.get("PULSE_INSTALLER_UNDER_TEST", ROOT / "install.sh"))
|
||||
REAL_SYSTEMCTL = shutil.which("systemctl")
|
||||
|
||||
SYSTEMCTL = r'''#!/usr/bin/env bash
|
||||
set -eu
|
||||
printf '%s\n' "$*" >> "$FIXTURE/calls"
|
||||
case "$1" in
|
||||
list-unit-files)
|
||||
if [[ "$INVENTORY" == real ]]; then
|
||||
exec "$REAL_SYSTEMCTL" --root="$FIXTURE/root" "$@"
|
||||
fi
|
||||
if [[ "$INVENTORY" == hang ]]; then exec sleep 30; fi
|
||||
printf '%s' "$ROWS"
|
||||
exit "$QUERY_EXIT"
|
||||
;;
|
||||
daemon-reload) exit 0 ;;
|
||||
*) echo "Unexpected manager operation: $*" >&2; exit 97 ;;
|
||||
esac
|
||||
'''
|
||||
|
||||
HARNESS = r'''
|
||||
source "$INSTALLER"
|
||||
IN_CONTAINER=true
|
||||
IN_DOCKER=false
|
||||
CURRENT_VERSION=v6.5.0
|
||||
print_header() { :; }
|
||||
print_info() { echo "$*"; }
|
||||
print_warn() { echo "$*"; }
|
||||
print_success() { echo "$*"; }
|
||||
print_error() { echo "$*" >&2; }
|
||||
check_root() { :; }
|
||||
detect_os() { :; }
|
||||
check_docker_environment() { :; }
|
||||
check_existing_installation() { [[ "$FLOW" != fresh ]]; }
|
||||
detect_service_name() { echo "$SERVICE_NAME"; }
|
||||
run_upgrade_readiness_preflight() { :; }
|
||||
backup_existing() { :; }
|
||||
create_user() { :; }
|
||||
install_dependencies() { :; }
|
||||
setup_directories() { :; }
|
||||
download_pulse() { :; }
|
||||
build_from_source() { :; }
|
||||
setup_update_command() { :; }
|
||||
ensure_systemd_service_installed() { :; }
|
||||
install_systemd_service() { :; }
|
||||
start_pulse() { :; }
|
||||
create_marker_file() { :; }
|
||||
print_completion() { :; }
|
||||
resolve_latest_release_tag_for_channel() {
|
||||
if [[ "$1" == stable ]]; then echo v6.6.0; else echo v6.7.0-rc.1; fi
|
||||
}
|
||||
safe_read() {
|
||||
printf '%s\n' "$1" >> "$FIXTURE/prompts"
|
||||
if [[ "$2" == enable_updates ]]; then
|
||||
printf -v "$2" '%s' n
|
||||
elif [[ "$FLOW" == reinstall ]]; then
|
||||
printf -v "$2" '%s' 3
|
||||
else
|
||||
printf -v "$2" '%s' 1
|
||||
fi
|
||||
}
|
||||
case "$FLOW" in
|
||||
query)
|
||||
if update_timer_exists; then echo PRESENT; else echo ABSENT; fi
|
||||
exit 0 ;;
|
||||
unavailable)
|
||||
command() {
|
||||
[[ "$*" != '-v systemctl' ]] || return 1
|
||||
builtin command "$@"
|
||||
}
|
||||
if update_timer_exists; then echo PRESENT; else echo ABSENT; fi
|
||||
exit 0 ;;
|
||||
version) FORCE_VERSION=v6.6.0 ;;
|
||||
source) BUILD_FROM_SOURCE=true ;;
|
||||
update|reinstall|fresh) : ;;
|
||||
*) exit 98 ;;
|
||||
esac
|
||||
main
|
||||
'''
|
||||
|
||||
|
||||
class ServerUpdateTimerTest(unittest.TestCase):
|
||||
def exercise(self, *, flow="query", state="enabled", service="pulse",
|
||||
rows=None, query_exit=0, inventory="double", mask="",
|
||||
config_enabled=True):
|
||||
with tempfile.TemporaryDirectory() as temporary:
|
||||
fixture = Path(temporary)
|
||||
config = fixture / "config"
|
||||
install = fixture / "install"
|
||||
units = fixture / "root/etc/systemd/system"
|
||||
tools = fixture / "tools"
|
||||
for directory in (config, install / "scripts", units, tools):
|
||||
directory.mkdir(parents=True)
|
||||
helper = tools / "auto-update"
|
||||
helper.write_text("#!/bin/bash\necho stale-helper\n")
|
||||
(install / "scripts/pulse-auto-update.sh").write_text(
|
||||
"#!/bin/bash\necho current-helper\n")
|
||||
settings = json.dumps({"autoUpdateEnabled": config_enabled, "updateChannel": "stable"})
|
||||
(config / "system.json").write_text(settings)
|
||||
timer_name = f"{service}-update.timer"
|
||||
timer = units / timer_name
|
||||
updater = units / f"{service}-update.service"
|
||||
timer.write_text("[Timer]\nOnCalendar=daily\n[Install]\nWantedBy=timers.target\n")
|
||||
updater.write_text("[Service]\nExecStart=/bin/true\n")
|
||||
if state == "enabled":
|
||||
wants = units / "timers.target.wants"
|
||||
wants.mkdir()
|
||||
(wants / timer_name).symlink_to(f"../{timer_name}")
|
||||
masked = timer if mask == "timer" else updater if mask == "service" else None
|
||||
if masked:
|
||||
masked.unlink()
|
||||
masked.symlink_to("/dev/null")
|
||||
systemctl = tools / "systemctl"
|
||||
systemctl.write_text(SYSTEMCTL)
|
||||
systemctl.chmod(0o755)
|
||||
if rows is None:
|
||||
rows = f"{timer_name} {state} enabled\n"
|
||||
env = {**os.environ, "INSTALLER": str(INSTALLER), "FIXTURE": str(fixture),
|
||||
"FLOW": flow, "ROWS": rows, "QUERY_EXIT": str(query_exit),
|
||||
"INVENTORY": inventory, "REAL_SYSTEMCTL": REAL_SYSTEMCTL or "",
|
||||
"PATH": f"{tools}:{os.environ['PATH']}", "PULSE_SERVICE_NAME": service,
|
||||
"PULSE_INSTALL_DIR": str(install), "PULSE_CONFIG_DIR": str(config),
|
||||
"PULSE_UPDATE_SERVICE_PATH": str(updater), "PULSE_UPDATE_TIMER_PATH": str(timer),
|
||||
"PULSE_AUTO_UPDATE_DEST": str(helper)}
|
||||
result = subprocess.run(["bash", "-c", HARNESS], env=env,
|
||||
capture_output=True, text=True, timeout=12)
|
||||
def lines(name):
|
||||
path = fixture / name
|
||||
return path.read_text().splitlines() if path.exists() else []
|
||||
return {"result": result, "helper": helper.read_text(),
|
||||
"settings": (config / "system.json").read_text(), "original_settings": settings,
|
||||
"calls": lines("calls"), "prompts": lines("prompts"),
|
||||
"timer_masked": timer.is_symlink() and os.readlink(timer) == "/dev/null",
|
||||
"service_masked": updater.is_symlink() and os.readlink(updater) == "/dev/null",
|
||||
"timer_enabled": (units / "timers.target.wants" / timer_name).is_symlink()}
|
||||
|
||||
def assert_discovery(self, evidence, present):
|
||||
r = evidence["result"]
|
||||
self.assertEqual(r.returncode, 0, r.stdout + r.stderr)
|
||||
self.assertEqual(r.stdout.strip(), "PRESENT" if present else "ABSENT")
|
||||
|
||||
def assert_refreshed(self, evidence, enabled):
|
||||
r = evidence["result"]
|
||||
self.assertEqual(r.returncode, 0, r.stdout + r.stderr)
|
||||
self.assertIn("current-helper", evidence["helper"])
|
||||
self.assertNotIn("stale-helper", evidence["helper"])
|
||||
self.assertEqual(evidence["settings"], evidence["original_settings"])
|
||||
self.assertEqual(evidence["timer_enabled"], enabled)
|
||||
mutations = [call for call in evidence["calls"] if not call.startswith("list-unit-files")]
|
||||
self.assertEqual(mutations, ["daemon-reload"])
|
||||
|
||||
def test_discovers_full_systemctl_rows_regardless_of_enablement(self):
|
||||
for state in ("enabled", "enabled-runtime", "disabled", "static", "indirect",
|
||||
"linked", "linked-runtime", "masked", "masked-runtime"):
|
||||
with self.subTest(state=state):
|
||||
self.assert_discovery(self.exercise(state=state), True)
|
||||
|
||||
def test_exact_unit_field_rejects_prefixes_and_regex_impostors(self):
|
||||
for rows in ("", "pulse-update.timer-other enabled enabled\n",
|
||||
"pulse-updateXtimer enabled enabled\n", "other-update.timer enabled enabled\n"):
|
||||
with self.subTest(rows=rows):
|
||||
self.assert_discovery(self.exercise(rows=rows), False)
|
||||
self.assert_discovery(self.exercise(rows="other.timer enabled enabled\n"
|
||||
"\tpulse-update.timer\tdisabled\tenabled\n"), True)
|
||||
|
||||
def test_query_is_bounded_and_scoped_to_full_exact_instance_name(self):
|
||||
service = "pulse-" + "department-" * 12
|
||||
e = self.exercise(service=service)
|
||||
self.assert_discovery(e, True)
|
||||
self.assertEqual(e["calls"], [f"list-unit-files --no-legend --no-pager --full -- {service}-update.timer"])
|
||||
|
||||
def test_failed_query_never_accepts_its_stdout(self):
|
||||
self.assert_discovery(self.exercise(query_exit=1), False)
|
||||
|
||||
def test_missing_systemctl_never_runs_a_query(self):
|
||||
e = self.exercise(flow="unavailable")
|
||||
self.assert_discovery(e, False)
|
||||
self.assertEqual(e["calls"], [])
|
||||
|
||||
def test_hung_inventory_times_out_without_manager_mutation(self):
|
||||
e = self.exercise(inventory="hang")
|
||||
self.assert_discovery(e, False)
|
||||
self.assertEqual(len(e["calls"]), 1)
|
||||
|
||||
def test_all_install_flows_reach_real_refresh_without_changing_consent(self):
|
||||
for flow in ("version", "source", "update", "reinstall", "fresh"):
|
||||
for state, enabled in (("enabled", True), ("disabled", False)):
|
||||
with self.subTest(flow=flow, state=state):
|
||||
e = self.exercise(flow=flow, state=state, config_enabled=enabled)
|
||||
self.assert_refreshed(e, enabled)
|
||||
self.assertNotIn("Automatic updates are not configured.", e["result"].stdout)
|
||||
if enabled:
|
||||
self.assertFalse(any("Enable auto-updates?" in p for p in e["prompts"]))
|
||||
|
||||
def test_missing_or_failed_inventory_does_not_refresh_or_enable(self):
|
||||
for rows, code in (("", 0), ("pulse-update.timer enabled enabled\n", 1)):
|
||||
e = self.exercise(flow="version", rows=rows, query_exit=code)
|
||||
self.assertEqual(e["result"].returncode, 0)
|
||||
self.assertIn("stale-helper", e["helper"])
|
||||
self.assertEqual(e["settings"], e["original_settings"])
|
||||
self.assertFalse(any(call == "daemon-reload" for call in e["calls"]))
|
||||
|
||||
def test_timer_and_updater_masks_are_not_replaced_during_refresh(self):
|
||||
for mask in ("timer", "service"):
|
||||
with self.subTest(mask=mask):
|
||||
e = self.exercise(flow="version", mask=mask)
|
||||
self.assertEqual(e["result"].returncode, 0)
|
||||
self.assertTrue(e[f"{mask}_masked"])
|
||||
self.assertIn("stale-helper", e["helper"])
|
||||
self.assertIn("masked", e["result"].stdout)
|
||||
self.assertEqual(e["settings"], e["original_settings"])
|
||||
self.assertFalse(any(call == "daemon-reload" for call in e["calls"]))
|
||||
|
||||
@unittest.skipUnless(REAL_SYSTEMCTL, "systemctl filesystem inventory is unavailable")
|
||||
def test_real_systemctl_inventory_and_complete_flow_use_only_fixture_root(self):
|
||||
for state, enabled in (("enabled", True), ("disabled", False)):
|
||||
with self.subTest(state=state):
|
||||
self.assert_discovery(self.exercise(inventory="real", state=state), True)
|
||||
self.assert_refreshed(self.exercise(flow="version", inventory="real", state=state), enabled)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user