diff options
| -rwxr-xr-x | scripts/post-rebuild-check | 25 | ||||
| -rw-r--r-- | tests/post-rebuild-check/test_post_rebuild_check.py | 65 |
2 files changed, 87 insertions, 3 deletions
diff --git a/scripts/post-rebuild-check b/scripts/post-rebuild-check index c18ae8f..24e99a4 100755 --- a/scripts/post-rebuild-check +++ b/scripts/post-rebuild-check @@ -56,6 +56,8 @@ # the special value MISSING = binary absent # PRC_NTP_SOURCES newline list of configured NTP server addresses; # the special value MISSING = no NTP daemon active +# PRC_CHRONY_CONF path to chrony.conf (a fixture, under test) -- the +# confdir it names is what decides which drop-ins count # PRC_SYSTEMCTL path to the systemctl binary (a fake, under test) # PRC_SYSTEMCTL_TIMEOUT seconds to allow each systemctl call (default 5) # @@ -101,6 +103,7 @@ ntp_missing="" # own outage, and the machine most in need of checking is the one it hangs on. # A timeout yields empty output and a non-zero status, and both are already # handled as findings, so bounding the call is all that is needed to fail closed. +CHRONY_CONF=${PRC_CHRONY_CONF:-/etc/chrony.conf} SCTL_TIMEOUT=${PRC_SYSTEMCTL_TIMEOUT:-5} SYSTEMCTL=${PRC_SYSTEMCTL:-systemctl} @@ -435,9 +438,25 @@ if [ -n "${PRC_NTP_SOURCES+set}" ]; then ntp_missing=1 fi elif sctl is-active chronyd >/dev/null 2>&1; then - # Both the main file and any drop-in: the IP-addressed source belongs in a - # drop-in, so reading only chrony.conf would miss every correct machine. - ntp_sources=$(cat /etc/chrony.conf /etc/chrony.d/*.conf 2>/dev/null \ + # The main file, plus any drop-in directory chrony.conf actually names. + # + # The confdir read is the load-bearing part. A drop-in is inert unless + # chrony.conf points at its directory, and Arch's stock chrony.conf points + # at none -- so globbing /etc/chrony.d unconditionally would find the + # IP-addressed source, report the machine healthy, and be describing a file + # chrony never opens. That is a false pass on exactly the misconfiguration + # this check exists to catch, so the sources are read only from files + # chrony is actually told to read. + ntp_conf_files=$CHRONY_CONF + for ntp_dir in $(awk '$1 == "confdir" || $1 == "sourcedir" { print $2 }' \ + "$CHRONY_CONF" 2>/dev/null); do + for ntp_f in "$ntp_dir"/*.conf "$ntp_dir"/*.sources; do + [ -f "$ntp_f" ] && ntp_conf_files="$ntp_conf_files $ntp_f" + done + done + # Unquoted on purpose: the accumulated list is several paths, and none of + # this script's own paths contain spaces. + ntp_sources=$(cat $ntp_conf_files 2>/dev/null \ | awk '$1 == "server" || $1 == "pool" { print $2 }') elif sctl is-active systemd-timesyncd >/dev/null 2>&1; then ntp_sources=$(awk -F= '/^[[:space:]]*NTP=/ { print $2 }' \ diff --git a/tests/post-rebuild-check/test_post_rebuild_check.py b/tests/post-rebuild-check/test_post_rebuild_check.py index 757039b..bad337e 100644 --- a/tests/post-rebuild-check/test_post_rebuild_check.py +++ b/tests/post-rebuild-check/test_post_rebuild_check.py @@ -30,6 +30,7 @@ Run from repo root: """ import os +import shutil import subprocess import tempfile import time @@ -124,6 +125,70 @@ class NtpBootstrap(unittest.TestCase): self.assertIn("no NTP sources are configured", r.stdout) self.assertEqual(r.returncode, 1) + # --- the confdir false pass ------------------------------------------ + # + # A drop-in is inert unless chrony.conf names its directory, and Arch's + # stock chrony.conf names none. Reading the drop-in without checking for + # confdir would find the IP-addressed source, call the machine healthy, and + # be describing a file chrony never opens — a false pass on exactly the + # misconfiguration this check exists to catch. + + def _chrony_fixture(self, main_lines, dropin_lines=None): + """Write a chrony.conf (plus an adjacent drop-in dir) and return its path.""" + d = tempfile.mkdtemp(prefix="prc-chrony-") + self.addCleanup(shutil.rmtree, d, True) + dropin_dir = os.path.join(d, "chrony.d") + os.makedirs(dropin_dir) + if dropin_lines is not None: + with open(os.path.join(dropin_dir, "10-bootstrap-ip-ntp.conf"), "w") as f: + f.write(dropin_lines) + conf = os.path.join(d, "chrony.conf") + with open(conf, "w") as f: + f.write(main_lines.replace("@DROPIN@", dropin_dir)) + return conf + + def _run_real_probe(self, chrony_conf): + """Run with PRC_NTP_SOURCES unset so the real chrony reader runs.""" + env = dict(os.environ) + env.update({"PRC_FAILED_UNITS": "", "PRC_UNIT_STATES": "", + "PRC_LOCAL_SCAN_ROOTS": "", "PRC_PROJECT_ROOTS": "", + "PRC_SIGNAL_ACCOUNTS": "+15045551234", + "PRC_CHRONY_CONF": chrony_conf}) + env.pop("PRC_NTP_SOURCES", None) + return subprocess.run(["sh", CHECK], capture_output=True, text=True, + timeout=30, env=env) + + def test_dropin_without_confdir_does_not_count(self): + # The regression. The IP-addressed source is present on disk but + # chrony.conf never points at it, so the machine is still deadlock-prone + # and the check has to say so. + conf = self._chrony_fixture("pool 2.arch.pool.ntp.org iburst\n", + "server 162.159.200.1 iburst\n") + r = self._run_real_probe(conf) + if "no NTP implementation is active" in r.stdout: + self.skipTest("no chronyd on this host — the reader branch can't run") + self.assertIn("every NTP source is named by hostname", r.stdout) + + def test_dropin_with_confdir_counts(self): + # The same two files, with chrony.conf actually naming the directory. + conf = self._chrony_fixture( + "pool 2.arch.pool.ntp.org iburst\nconfdir @DROPIN@\n", + "server 162.159.200.1 iburst\n") + r = self._run_real_probe(conf) + if "no NTP implementation is active" in r.stdout: + self.skipTest("no chronyd on this host — the reader branch can't run") + self.assertIn("check 6/6: NTP bootstrap — ok", r.stdout) + + def test_confdir_naming_an_empty_directory_is_not_a_pass(self): + # confdir present, nothing behind it: the sources are the hostname-only + # main file, so the finding stands. + conf = self._chrony_fixture( + "pool 2.arch.pool.ntp.org iburst\nconfdir @DROPIN@\n", None) + r = self._run_real_probe(conf) + if "no NTP implementation is active" in r.stdout: + self.skipTest("no chronyd on this host — the reader branch can't run") + self.assertIn("every NTP source is named by hostname", r.stdout) + def test_unset_seam_falls_through_to_the_real_probe(self): # Same contract as every other seam: unset means "really look", so a # caller who forgets the variable cannot silently skip the check. |