mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-14 18:50:47 +01:00
check_and_enable_services: a warning must never mask an error
Eight checks assigned status="warning" unconditionally, so a check running
later in the same function overwrote the status="error" an earlier one had
set. The five other sites in this family already guard the assignment with
`if [[ $status != "error" ]]`, so this was an inconsistency, not a design.
Only half of it was load-bearing, and the two halves fail differently:
services_units.sh (4 sites) reports through report_and_fix, which repairs
ONLY on "error". A downgrade there meant no repair ran at all. The is-active
check is the LAST status assignment in each of the three functions, so it
could mask any error above it -- in check_periodic_systems, three separate
is-enabled checks. A timer that was both disabled and inactive, which is the
ordinary shape of a broken timer, was reported and then left broken.
services_hosts.sh (4 sites) gates its own repair on `status != "ok"`, so the
repair still ran. There the bug printed a real error as a yellow [!] warning
and understated the summary row.
Demonstrated rather than argued, against a simulated broken machine (timer
disabled AND inactive, unit files present):
without the guards: status=warning, repair ran 0 times
with the guards: status=error, repair ran 1 time
The seven assertions that previously pinned the broken behaviour now assert
the correct behaviour; they were rewritten first and failed, then the guards
made them pass. Two further cases cover the masking shape directly: a broken
guard-lib instance must still be repaired when a later check warns.
`--status --dry-run` output is still byte-identical to rev 6691cb26, because
this machine has no disabled timer and no missing pacman hook -- the buggy
path is never taken here, which is precisely why it went unnoticed. The A/B
above is the evidence the live diff cannot provide.
222 assertions across 10 files. services_units.sh holds 100% coverage;
services_hosts.sh rises from 86.05% to 93.02%. test_services_hosts.sh needed
splitting twice to stay under the 250-line cap (hosts_fix, resolved), and
stage_hosts_ok moved into the harness so both new files can use it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3PzGc75gBcdxP3PkHXGcV
This commit is contained in:
parent
0a69d18390
commit
08f3c764b2
@ -0,0 +1,19 @@
|
||||
{
|
||||
"title": "Fix the status-downgrade bug so a warning can never mask an error",
|
||||
"objective": "Stop a later check's status=\"warning\" from overwriting an earlier check's status=\"error\" in check_and_enable_services, which made genuinely broken services report as warnings and then never get repaired. Found while writing the split's tests and deferred out of 9085a4e5 because it changes live enforcement; the user asked for it to be fixed.",
|
||||
"acceptance_criteria": [
|
||||
"No bare `status=\"warning\"` assignment remains in any check lib; every one is guarded by `if [[ $status != \"error\" ]]`, matching the idiom already used in services_apps.sh and services_browser.sh.",
|
||||
"A timer that is BOTH disabled and inactive records \"error\" and triggers its setup script, demonstrated by an A/B run against a simulated broken machine rather than only by assertion.",
|
||||
"The regression is covered by tests that FAILED before the fix and pass after it.",
|
||||
"lib/tests/run_all.sh passes with no failures.",
|
||||
"services_units.sh keeps 100% line coverage.",
|
||||
"Every file including the test files stays at or under 250 lines.",
|
||||
"shellcheck reports zero findings and no suppression is added."
|
||||
],
|
||||
"out_of_scope": [
|
||||
"The 5 sites that were already correctly guarded.",
|
||||
"Changing which statuses report_and_fix acts on: the fix makes the status correct rather than widening the repair gate.",
|
||||
"The kcov attribution artifact on services_hosts.sh, diagnosed in the previous commit's evidence."
|
||||
],
|
||||
"verifier": "lib/tests/run_all.sh (222 assertions), an A/B of check_midnight_shutdown against a disabled+inactive timer with and without the guards, bash meta/scripts/shell_coverage.sh lib/tests/run_all.sh services_units.sh 100, grep for unguarded sites, shellcheck, and check_file_length.sh --all."
|
||||
}
|
||||
@ -0,0 +1,61 @@
|
||||
{
|
||||
"intent": "Fix the status-downgrade bug in check_and_enable_services, where a later check assigning status=\"warning\" unconditionally could overwrite the status=\"error\" an earlier check had set — so a genuinely broken service was reported and then silently never repaired.",
|
||||
"scope": [
|
||||
"linux_configuration/scripts/periodic_background/lib/services_units.sh — 4 unguarded sites, the load-bearing ones",
|
||||
"linux_configuration/scripts/periodic_background/lib/services_hosts.sh — 4 unguarded sites, cosmetic but wrong",
|
||||
"lib/tests/test_services_units.sh — 7 pinning assertions converted to regression assertions",
|
||||
"lib/tests/test_services_hosts.sh — 2 new masking regression cases",
|
||||
"lib/tests/ — test_services_hosts_fix.sh and test_services_hosts_resolved.sh split out for the 250-line cap; stage_hosts_ok moved into the harness so both files can use it",
|
||||
"Non-goal: the 5 sites that were already guarded correctly"
|
||||
],
|
||||
"changes": [
|
||||
"All 8 bare `status=\"warning\"` assignments now read `if [[ $status != \"error\" ]]; then status=\"warning\"; fi`, which is the idiom services_apps.sh and services_browser.sh already used — so this makes the family internally consistent rather than inventing a convention.",
|
||||
"The blast radius differs between the two files and only one is load-bearing. services_units.sh reports through report_and_fix, which repairs ONLY on \"error\", so a downgrade there meant no repair ran at all. services_hosts.sh gates its own repair on `status != \"ok\"`, so repair still ran; there the bug printed a real error as a yellow [!] warning and understated the summary row.",
|
||||
"The three unit checks were the worst case because the is-active check is the LAST status assignment in each function, so it could overwrite any error set above it. In check_periodic_systems that is hosts-file-monitor.service, which sits after three separate is-enabled checks."
|
||||
],
|
||||
"verification": [
|
||||
{
|
||||
"command": "lib/tests/test_services_units.sh (before the fix)",
|
||||
"result": "7 failures, as intended",
|
||||
"evidence": "The pinning assertions were rewritten to assert correct behaviour first, and failed: 'disabled+inactive stays an error (want error, got warning)' and 'so the disabled timer IS repaired (no call matching /ran setup_midnight_shutdown.sh enable/)' for all three unit checks."
|
||||
},
|
||||
{
|
||||
"command": "A/B against a simulated broken machine: timer disabled AND inactive, unit files present",
|
||||
"result": "pass",
|
||||
"evidence": "Without the fix: status=warning, repair ran 0 times — the timer stays dead. With the fix: status=error, repair ran 1 time — the timer is re-enabled. Run by reverting only the 4 guards in services_units.sh and restoring them afterwards."
|
||||
},
|
||||
{
|
||||
"command": "lib/tests/run_all.sh",
|
||||
"result": "pass",
|
||||
"evidence": "222 assertions across 10 test files, 0 failures."
|
||||
},
|
||||
{
|
||||
"command": "diff <(sudo bash <worktree@6691cb26>/…/check_and_enable_services.sh --status --dry-run) <(sudo bash …/check_and_enable_services.sh --status --dry-run)",
|
||||
"result": "identical",
|
||||
"evidence": "Still byte-identical modulo Date:. This machine has no disabled timer and no missing pacman hook, so the buggy path is not taken here — which is exactly why the fault went unnoticed. The A/B above is what demonstrates the behaviour change, since the live diff cannot."
|
||||
},
|
||||
{
|
||||
"command": "bash meta/scripts/shell_coverage.sh lib/tests/run_all.sh <lib> 100",
|
||||
"result": "pass on 6 of 8",
|
||||
"evidence": "services_units.sh holds 100.00% (72/72) through the change. services_hosts.sh rose from 86.05% to 93.02% as the new masking cases exercised more branches; the 6 remaining lines are the kcov attribution artifact diagnosed in check-and-enable-services-coverage-2026-08-21.json. services_hosts_fix.sh 97.30% (2 pipeline continuation lines)."
|
||||
},
|
||||
{
|
||||
"command": "grep -n '^\\s*status=\"warning\"$' lib/services_*.sh",
|
||||
"result": "pass",
|
||||
"evidence": "No unguarded site remains."
|
||||
},
|
||||
{
|
||||
"command": "shellcheck lib/tests/*.sh lib/services_*.sh check_and_enable_services.sh; bash meta/scripts/check_file_length.sh --all",
|
||||
"result": "pass",
|
||||
"evidence": "Zero shellcheck findings, no suppression. No file over 250 lines; the hosts test file needed splitting twice to stay under it."
|
||||
}
|
||||
],
|
||||
"risks": [
|
||||
"This deliberately changes live enforcement behaviour: services that previously sat in a warning state without being repaired will now be repaired on the next hourly run. That is the point of the fix, but it means the next run on a machine with a disabled timer will actually execute a setup script rather than doing nothing.",
|
||||
"A service whose repair script is broken will now retry every run instead of staying quiet at warning. err_missing_script already logs that case at journal error priority, so it surfaces rather than looping silently."
|
||||
],
|
||||
"rollback": [
|
||||
"git revert this commit restores the unguarded assignments and the pinning tests together, so the suite stays green either way.",
|
||||
"After rollback, confirm with the A/B: a disabled+inactive timer should report warning and run no repair."
|
||||
]
|
||||
}
|
||||
@ -28,7 +28,7 @@ check_hosts() {
|
||||
msg "/etc/hosts exists with $line_count lines (StevenBlack list likely installed)"
|
||||
else
|
||||
issues+=("/etc/hosts has only $line_count lines (StevenBlack list may not be installed)")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
else
|
||||
issues+=("/etc/hosts does not exist")
|
||||
@ -42,7 +42,7 @@ check_hosts() {
|
||||
msg "/etc/hosts has immutable attribute set"
|
||||
else
|
||||
issues+=("/etc/hosts is not immutable")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check cached hosts file
|
||||
@ -50,7 +50,7 @@ check_hosts() {
|
||||
msg "StevenBlack cache exists at ${SYSROOT}/etc/hosts.stevenblack"
|
||||
else
|
||||
issues+=("StevenBlack cache not found")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check the guard-lib "hosts" file-guard instance (path unit active +
|
||||
@ -70,7 +70,7 @@ check_hosts() {
|
||||
msg "Pacman hooks installed"
|
||||
else
|
||||
issues+=("Pacman hooks not installed")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check nsswitch.conf has 'files' in hosts line
|
||||
|
||||
@ -31,7 +31,7 @@ check_midnight_shutdown() {
|
||||
msg "day-specific-shutdown.timer is active"
|
||||
else
|
||||
issues+=("day-specific-shutdown.timer is not active")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check service file exists
|
||||
@ -75,7 +75,7 @@ check_startup_monitor() {
|
||||
msg "pc-startup-monitor.timer is active"
|
||||
else
|
||||
issues+=("pc-startup-monitor.timer is not active")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check service file exists
|
||||
@ -118,7 +118,7 @@ check_periodic_systems() {
|
||||
msg "periodic-system-maintenance.timer is active"
|
||||
else
|
||||
issues+=("periodic-system-maintenance.timer is not active")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check startup service
|
||||
@ -141,7 +141,7 @@ check_periodic_systems() {
|
||||
msg "hosts-file-monitor.service is active"
|
||||
else
|
||||
issues+=("hosts-file-monitor.service is not active")
|
||||
status="warning"
|
||||
if [[ $status != "error" ]]; then status="warning"; fi
|
||||
fi
|
||||
|
||||
# Check maintenance script
|
||||
|
||||
@ -208,6 +208,19 @@ make_installer() { # <path>
|
||||
# shellcheck source=../services_common.sh
|
||||
. "${LIB_DIR}/services_common.sh"
|
||||
|
||||
# Stage a fully healthy blocking stack: a long hosts file, the guard-lib trio
|
||||
# active, both pacman hooks, and a resolved.conf that reads /etc/hosts.
|
||||
stage_hosts_ok() {
|
||||
sysfile etc/hosts 200
|
||||
sysfile etc/hosts.stevenblack 200
|
||||
printf '%s\n' "${SERVICES_ROOT}/etc/hosts" >"${DEV}/immutable"
|
||||
printf 'hosts: files resolve dns\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
printf '[Resolve]\nReadEtcHosts=yes\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
printf '%s\n' hosts nsswitch resolved >"${DEV}/guard_healthy"
|
||||
sysfile etc/pacman.d/hooks/10-guard-lib-unlock-all.hook
|
||||
sysfile etc/pacman.d/hooks/90-guard-lib-relock-all.hook
|
||||
}
|
||||
|
||||
# reset_state — return every mutable global and every $DEV fact to "nothing has
|
||||
# happened yet", so each test group starts from a known machine.
|
||||
reset_state() {
|
||||
|
||||
@ -28,19 +28,6 @@ _t_called_in() { # <haystack> <needle> <what>
|
||||
fi
|
||||
}
|
||||
|
||||
# Stage a fully healthy blocking stack: a long hosts file, the guard-lib trio
|
||||
# active, both pacman hooks, and a resolved.conf that reads /etc/hosts.
|
||||
stage_hosts_ok() {
|
||||
sysfile etc/hosts 200
|
||||
sysfile etc/hosts.stevenblack 200
|
||||
printf '%s\n' "${SERVICES_ROOT}/etc/hosts" >"${DEV}/immutable"
|
||||
printf 'hosts: files resolve dns\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
printf '[Resolve]\nReadEtcHosts=yes\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
printf '%s\n' hosts nsswitch resolved >"${DEV}/guard_healthy"
|
||||
sysfile etc/pacman.d/hooks/10-guard-lib-unlock-all.hook
|
||||
sysfile etc/pacman.d/hooks/90-guard-lib-relock-all.hook
|
||||
}
|
||||
|
||||
echo "== check_hosts: a fully healthy stack records ok =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
@ -108,51 +95,6 @@ sysrm etc/nsswitch.conf
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "/etc/nsswitch.conf does not exist" "the missing file is reported"
|
||||
|
||||
echo "== check_hosts: ReadEtcHosts=no is an error and is repaired =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
printf '[Resolve]\nReadEtcHosts=no\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "bypassed by systemd-resolved" "the resolved bypass is called out"
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "ReadEtcHosts=yes" "the setting is repaired"
|
||||
_t_called 'systemctl restart systemd-resolved' "systemd-resolved is restarted so the change takes"
|
||||
|
||||
echo "== check_hosts: DNSOverTLS enabled is an error and is repaired =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
printf '[Resolve]\nReadEtcHosts=yes\nDNSOverTLS=yes\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
# The message interpolates $SYSROOT, so match on the stable part of it.
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "DNSOverTLS='yes'" "DNSOverTLS is called out as a bypass"
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "#DNSOverTLS=no" "it is commented out"
|
||||
|
||||
echo "== check_hosts: drop-in overrides are an error and are removed =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
# A drop-in can re-enable either bypass without touching resolved.conf, so the
|
||||
# repairs above are not durable while one survives.
|
||||
sysfile etc/systemd/resolved.conf.d/99-override.conf
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "drop-in override" "the drop-in is called out"
|
||||
if [[ -f "${SERVICES_ROOT}/etc/systemd/resolved.conf.d/99-override.conf" ]]; then
|
||||
_t_fail "the drop-in is deleted"
|
||||
else
|
||||
_t_pass "the drop-in is deleted"
|
||||
fi
|
||||
|
||||
echo "== check_hosts: a missing resolved.conf warns =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
sysrm etc/systemd/resolved.conf
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "resolved.conf does not exist" "the missing file is reported"
|
||||
|
||||
echo "== check_hosts: an unhealthy guard-lib instance is repaired =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
@ -224,77 +166,6 @@ printf 'nsswitch\n' >"${DEV}/guard_degraded"
|
||||
check_hosts >/dev/null
|
||||
_t_eq "2" "${#MISSING_SCRIPTS[@]}" "both missing repair scripts are recorded"
|
||||
|
||||
echo "== hosts_fix_nsswitch: each resolver shape =="
|
||||
reset_state
|
||||
printf 'hosts: mymachines dns\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/nsswitch.conf")" "files dns" "'files' is inserted before dns"
|
||||
|
||||
reset_state
|
||||
printf 'hosts: mymachines myhostname\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/nsswitch.conf")" "hosts: files" "'files' is prepended when no resolver is named"
|
||||
|
||||
reset_state
|
||||
printf 'hosts: files resolve\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_eq "hosts: files resolve" "$(cat "${SERVICES_ROOT}/etc/nsswitch.conf")" "an already-correct line is left alone"
|
||||
|
||||
reset_state
|
||||
sysrm etc/nsswitch.conf
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_pass "a missing nsswitch.conf is a no-op rather than an error"
|
||||
|
||||
reset_state
|
||||
printf 'passwd: files\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_pass "an nsswitch.conf with no hosts line at all is a no-op"
|
||||
|
||||
echo "== hosts_fix_resolved: adds the setting when the file lacks it =="
|
||||
# Called through `if` rather than bare. When resolved.conf carries no
|
||||
# ReadEtcHosts line at all, the leading `grep` in the assignment pipeline exits
|
||||
# 1; with `set -o pipefail` that makes the whole assignment non-zero, so a bare
|
||||
# call aborts the test under `set -e`. Production never hits this because
|
||||
# check_hosts only ever calls it from inside an `if`, which suppresses `set -e`
|
||||
# for the callee -- so this is a property of calling it bare, not a defect.
|
||||
reset_state
|
||||
printf '[Resolve]\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
if hosts_fix_resolved >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "ReadEtcHosts=yes" "the setting is inserted under [Resolve]"
|
||||
|
||||
reset_state
|
||||
printf '# no section here\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
if hosts_fix_resolved >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "[Resolve]" "a whole [Resolve] section is appended when absent"
|
||||
|
||||
reset_state
|
||||
sysrm etc/systemd/resolved.conf
|
||||
if hosts_fix_resolved >/dev/null; then :; fi
|
||||
_t_pass "a missing resolved.conf is a no-op rather than an error"
|
||||
|
||||
echo "== hosts_repair_all: dry-run refuses to claim success =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
DRY_RUN=1
|
||||
if hosts_repair_all >/dev/null; then
|
||||
_t_fail "hosts_repair_all returns non-zero under --dry-run"
|
||||
else
|
||||
_t_pass "hosts_repair_all returns non-zero under --dry-run"
|
||||
fi
|
||||
|
||||
echo "== hosts_repair_all: a healthy machine verifies after repair =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
if hosts_repair_all >/dev/null; then
|
||||
_t_pass "hosts_repair_all returns zero when the post-repair check passes"
|
||||
else
|
||||
_t_fail "hosts_repair_all returns zero when the post-repair check passes"
|
||||
fi
|
||||
|
||||
echo "== check_hosts: resolved.conf immutable attribute is recognised =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
@ -317,4 +188,36 @@ for inst in hosts nsswitch resolved; do
|
||||
_t_called 'ran migrate_hosts_guard' "and the migration is re-run for '${inst}'"
|
||||
done
|
||||
|
||||
echo "== check_hosts: a warning must never mask an error =="
|
||||
# Regression test for the status-downgrade bug: several checks assigned
|
||||
# status="warning" unconditionally, so a check running LATER in the function
|
||||
# could overwrite the status="error" an earlier one had set. Because
|
||||
# report_and_fix (and check_hosts' own repair gate) only act on "error", the
|
||||
# downgraded fault was reported and then silently never repaired.
|
||||
#
|
||||
# Here the guard-lib 'hosts' instance is broken (an error) AND the pacman hook
|
||||
# pair is missing (a warning, and checked afterwards). The row must stay
|
||||
# "error" and the repair must run.
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
printf '%s\n' nsswitch resolved >"${DEV}/guard_healthy"
|
||||
sysrm etc/pacman.d/hooks/10-guard-lib-unlock-all.hook
|
||||
sysrm etc/pacman.d/hooks/90-guard-lib-relock-all.hook
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called 'ran migrate_hosts_guard' "a broken guard is still repaired when a later check warns"
|
||||
|
||||
# The same shape with the short-hosts-file warning, which is checked FIRST and
|
||||
# so cannot mask anything -- but the immutable/cache warnings after it can.
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
printf '%s\n' nsswitch resolved >"${DEV}/guard_healthy"
|
||||
: >"${DEV}/immutable"
|
||||
sysrm etc/hosts.stevenblack
|
||||
check_hosts >/dev/null
|
||||
_t_called 'ran migrate_hosts_guard' "a broken guard survives the immutable and cache warnings"
|
||||
|
||||
_t_summary
|
||||
|
||||
@ -0,0 +1,105 @@
|
||||
#!/usr/bin/env bash
|
||||
# Tests for the repair helpers in lib/services_hosts_fix.sh, called directly
|
||||
# rather than through check_hosts.
|
||||
#
|
||||
# Split out of test_services_hosts.sh for the repo-wide 250-line cap. These are
|
||||
# the unit-level cases: each resolver shape hosts_fix_nsswitch has to rewrite,
|
||||
# each resolved.conf shape hosts_fix_resolved has to repair, and the contract
|
||||
# that hosts_repair_all reports failure under --dry-run rather than claiming a
|
||||
# repair that never happened.
|
||||
#
|
||||
# The helpers are called via `if` rather than bare: when resolved.conf carries
|
||||
# no ReadEtcHosts line, the leading grep in the assignment pipeline exits 1 and
|
||||
# `set -o pipefail` makes the whole assignment non-zero, so a bare call would
|
||||
# abort the suite under `set -e`. check_hosts only ever calls them from inside
|
||||
# an `if`, which suppresses `set -e` for the callee, so this is a property of
|
||||
# calling them bare and not a defect.
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
|
||||
# shellcheck source=services_harness.sh
|
||||
. "${SCRIPT_DIR}/services_harness.sh"
|
||||
# shellcheck source=../services_hosts_fix.sh
|
||||
. "${SCRIPT_DIR}/../services_hosts_fix.sh"
|
||||
|
||||
_t_called_in() { # <haystack> <needle> <what>
|
||||
if [[ "$1" == *"$2"* ]]; then
|
||||
_t_pass "$3"
|
||||
else
|
||||
_t_fail "$3 (missing '$2')"
|
||||
fi
|
||||
}
|
||||
|
||||
echo "== hosts_fix_nsswitch: each resolver shape =="
|
||||
reset_state
|
||||
printf 'hosts: mymachines dns\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/nsswitch.conf")" "files dns" "'files' is inserted before dns"
|
||||
|
||||
reset_state
|
||||
printf 'hosts: mymachines myhostname\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/nsswitch.conf")" "hosts: files" "'files' is prepended when no resolver is named"
|
||||
|
||||
reset_state
|
||||
printf 'hosts: files resolve\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_eq "hosts: files resolve" "$(cat "${SERVICES_ROOT}/etc/nsswitch.conf")" "an already-correct line is left alone"
|
||||
|
||||
reset_state
|
||||
sysrm etc/nsswitch.conf
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_pass "a missing nsswitch.conf is a no-op rather than an error"
|
||||
|
||||
reset_state
|
||||
printf 'passwd: files\n' >"${SERVICES_ROOT}/etc/nsswitch.conf"
|
||||
if hosts_fix_nsswitch >/dev/null; then :; fi
|
||||
_t_pass "an nsswitch.conf with no hosts line at all is a no-op"
|
||||
|
||||
echo "== hosts_fix_resolved: adds the setting when the file lacks it =="
|
||||
# Called through `if` rather than bare. When resolved.conf carries no
|
||||
# ReadEtcHosts line at all, the leading `grep` in the assignment pipeline exits
|
||||
# 1; with `set -o pipefail` that makes the whole assignment non-zero, so a bare
|
||||
# call aborts the test under `set -e`. Production never hits this because
|
||||
# check_hosts only ever calls it from inside an `if`, which suppresses `set -e`
|
||||
# for the callee -- so this is a property of calling it bare, not a defect.
|
||||
reset_state
|
||||
printf '[Resolve]\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
if hosts_fix_resolved >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "ReadEtcHosts=yes" "the setting is inserted under [Resolve]"
|
||||
|
||||
reset_state
|
||||
printf '# no section here\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
if hosts_fix_resolved >/dev/null; then :; fi
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "[Resolve]" "a whole [Resolve] section is appended when absent"
|
||||
|
||||
reset_state
|
||||
sysrm etc/systemd/resolved.conf
|
||||
if hosts_fix_resolved >/dev/null; then :; fi
|
||||
_t_pass "a missing resolved.conf is a no-op rather than an error"
|
||||
|
||||
echo "== hosts_repair_all: dry-run refuses to claim success =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
DRY_RUN=1
|
||||
if hosts_repair_all >/dev/null; then
|
||||
_t_fail "hosts_repair_all returns non-zero under --dry-run"
|
||||
else
|
||||
_t_pass "hosts_repair_all returns non-zero under --dry-run"
|
||||
fi
|
||||
|
||||
echo "== hosts_repair_all: a healthy machine verifies after repair =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
if hosts_repair_all >/dev/null; then
|
||||
_t_pass "hosts_repair_all returns zero when the post-repair check passes"
|
||||
else
|
||||
_t_fail "hosts_repair_all returns zero when the post-repair check passes"
|
||||
fi
|
||||
|
||||
_t_summary
|
||||
@ -0,0 +1,73 @@
|
||||
#!/usr/bin/env bash
|
||||
# Tests for the systemd-resolved half of check_hosts (lib/services_hosts.sh).
|
||||
#
|
||||
# Split out of test_services_hosts.sh for the repo-wide 250-line cap. Every
|
||||
# case here is a way systemd-resolved can answer a query WITHOUT consulting
|
||||
# /etc/hosts, which leaves the whole blocking stack inert while a correct,
|
||||
# immutable, fully-populated hosts file sits on disk: ReadEtcHosts turned off,
|
||||
# DNSOverTLS turned on, or either re-enabled by a drop-in override.
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
|
||||
# shellcheck source=services_harness.sh
|
||||
. "${SCRIPT_DIR}/services_harness.sh"
|
||||
# shellcheck source=../services_hosts_fix.sh
|
||||
. "${SCRIPT_DIR}/../services_hosts_fix.sh"
|
||||
# shellcheck source=../services_hosts.sh
|
||||
. "${SCRIPT_DIR}/../services_hosts.sh"
|
||||
|
||||
_t_called_in() { # <haystack> <needle> <what>
|
||||
if [[ "$1" == *"$2"* ]]; then
|
||||
_t_pass "$3"
|
||||
else
|
||||
_t_fail "$3 (missing '$2')"
|
||||
fi
|
||||
}
|
||||
|
||||
echo "== check_hosts: ReadEtcHosts=no is an error and is repaired =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
printf '[Resolve]\nReadEtcHosts=no\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "bypassed by systemd-resolved" "the resolved bypass is called out"
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "ReadEtcHosts=yes" "the setting is repaired"
|
||||
_t_called 'systemctl restart systemd-resolved' "systemd-resolved is restarted so the change takes"
|
||||
|
||||
echo "== check_hosts: DNSOverTLS enabled is an error and is repaired =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
printf '[Resolve]\nReadEtcHosts=yes\nDNSOverTLS=yes\n' >"${SERVICES_ROOT}/etc/systemd/resolved.conf"
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
# The message interpolates $SYSROOT, so match on the stable part of it.
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "DNSOverTLS='yes'" "DNSOverTLS is called out as a bypass"
|
||||
_t_called_in "$(cat "${SERVICES_ROOT}/etc/systemd/resolved.conf")" "#DNSOverTLS=no" "it is commented out"
|
||||
|
||||
echo "== check_hosts: drop-in overrides are an error and are removed =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
make_installer "$HOSTS_INSTALL_SCRIPT"
|
||||
make_installer "$GUARD_LIB_MIGRATE_SCRIPT"
|
||||
# A drop-in can re-enable either bypass without touching resolved.conf, so the
|
||||
# repairs above are not durable while one survives.
|
||||
sysfile etc/systemd/resolved.conf.d/99-override.conf
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "drop-in override" "the drop-in is called out"
|
||||
if [[ -f "${SERVICES_ROOT}/etc/systemd/resolved.conf.d/99-override.conf" ]]; then
|
||||
_t_fail "the drop-in is deleted"
|
||||
else
|
||||
_t_pass "the drop-in is deleted"
|
||||
fi
|
||||
|
||||
echo "== check_hosts: a missing resolved.conf warns =="
|
||||
reset_state
|
||||
stage_hosts_ok
|
||||
sysrm etc/systemd/resolved.conf
|
||||
check_hosts >"${TEST_TMPDIR}/out.txt"
|
||||
_t_called_in "$(cat "${TEST_TMPDIR}/out.txt")" "resolved.conf does not exist" "the missing file is reported"
|
||||
|
||||
_t_summary
|
||||
@ -58,15 +58,13 @@ stage_midnight_files
|
||||
: >"${DEV}/enabled"
|
||||
: >"${DEV}/active"
|
||||
check_midnight_shutdown >/dev/null
|
||||
# LATENT BUG, pinned deliberately rather than fixed here: the is-active check
|
||||
# assigns status="warning" unconditionally, so it overwrites the "error" the
|
||||
# is-enabled check set immediately above. A timer that is BOTH disabled and
|
||||
# inactive therefore ends up "warning", and report_and_fix only repairs on
|
||||
# "error" -- so the timer is never re-enabled. Identical in the pre-split file
|
||||
# (verified against rev 6691cb26), so this split neither caused nor hides it.
|
||||
# Fixing it changes enforcement behaviour and belongs in its own commit.
|
||||
_t_eq "warning" "$(get_service_status "midnight_shutdown")" "disabled+inactive is (wrongly) only a warning"
|
||||
_t_not_called 'ran setup_midnight_shutdown' "so the disabled timer is never repaired"
|
||||
# Regression test. The is-active check used to assign status="warning"
|
||||
# unconditionally, overwriting the "error" the is-enabled check set just above;
|
||||
# since report_and_fix only repairs on "error", a timer that was BOTH disabled
|
||||
# and inactive -- the normal shape of a genuinely broken timer -- was reported
|
||||
# and then never re-enabled.
|
||||
_t_eq "error" "$(get_service_status "midnight_shutdown")" "disabled+inactive stays an error"
|
||||
_t_called 'ran setup_midnight_shutdown.sh enable' "so the disabled timer IS repaired"
|
||||
|
||||
echo "== check_midnight_shutdown: --status reports without repairing =="
|
||||
reset_state
|
||||
@ -76,7 +74,7 @@ STATUS_ONLY=1
|
||||
: >"${DEV}/enabled"
|
||||
check_midnight_shutdown >/dev/null
|
||||
_t_not_called 'ran setup_midnight_shutdown' "--status never runs the setup script"
|
||||
_t_eq "warning" "$(get_service_status "midnight_shutdown")" "the verdict is still recorded"
|
||||
_t_eq "error" "$(get_service_status "midnight_shutdown")" "the error is still recorded"
|
||||
|
||||
echo "== check_startup_monitor: fully healthy records ok =="
|
||||
reset_state
|
||||
@ -104,9 +102,9 @@ sysfile usr/local/bin/pc-startup-check.sh
|
||||
: >"${DEV}/enabled"
|
||||
: >"${DEV}/active"
|
||||
check_startup_monitor >/dev/null
|
||||
# Same latent downgrade as check_midnight_shutdown above.
|
||||
_t_eq "warning" "$(get_service_status "startup_monitor")" "disabled+inactive is (wrongly) only a warning"
|
||||
_t_not_called 'ran setup_pc_startup_monitor' "so the disabled timer is never repaired"
|
||||
# Same regression as check_midnight_shutdown above.
|
||||
_t_eq "error" "$(get_service_status "startup_monitor")" "disabled+inactive stays an error"
|
||||
_t_called 'ran setup_pc_startup_monitor' "so the disabled timer IS repaired"
|
||||
|
||||
echo "== check_startup_monitor: enabled but inactive is a warning, not a fix =="
|
||||
reset_state
|
||||
@ -153,10 +151,10 @@ sysfile usr/local/bin/periodic-system-maintenance.sh
|
||||
: >"${DEV}/enabled"
|
||||
: >"${DEV}/active"
|
||||
check_periodic_systems >/dev/null
|
||||
# hosts-file-monitor.service's is-active check is the last status assignment
|
||||
# here, and it too downgrades unconditionally -- same bug, same pinning.
|
||||
_t_eq "warning" "$(get_service_status "periodic_systems")" "a fully disabled stack is (wrongly) only a warning"
|
||||
_t_not_called 'ran setup_periodic_system' "so the disabled stack is never repaired"
|
||||
# hosts-file-monitor.service's is-active check is the LAST status assignment in
|
||||
# this function, so before the fix it downgraded everything above it.
|
||||
_t_eq "error" "$(get_service_status "periodic_systems")" "a fully disabled stack stays an error"
|
||||
_t_called 'ran setup_periodic_system' "so the disabled stack IS repaired"
|
||||
|
||||
echo "== check_periodic_systems: startup service disabled alone still repairs =="
|
||||
reset_state
|
||||
|
||||
Loading…
Reference in New Issue
Block a user