mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 10:29:06 +00:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cf521bdfd8 |
@@ -62,6 +62,9 @@ class StartupValidator:
|
||||
# Validate plugins if plugin manager is available
|
||||
if self.plugin_manager:
|
||||
self._validate_plugins()
|
||||
|
||||
# Warn when the running systemd unit no longer matches the repo's
|
||||
self._validate_systemd_units()
|
||||
|
||||
is_valid = len(self.errors) == 0
|
||||
|
||||
@@ -74,6 +77,71 @@ class StartupValidator:
|
||||
|
||||
return (is_valid, self.errors.copy(), self.warnings.copy())
|
||||
|
||||
#: Units this project installs, and where each is installed to.
|
||||
_UNITS = (
|
||||
("systemd/ledmatrix.service", "/etc/systemd/system/ledmatrix.service"),
|
||||
("systemd/ledmatrix-web.service", "/etc/systemd/system/ledmatrix-web.service"),
|
||||
)
|
||||
|
||||
def _validate_systemd_units(self) -> None:
|
||||
"""Warn when an installed unit has drifted from the repo's template.
|
||||
|
||||
Nothing re-applies these after the first install. `git pull` -- which is
|
||||
what the web UI's update button runs -- brings a new template into the
|
||||
checkout, but nothing copies it to /etc/systemd/system and nothing runs
|
||||
`systemctl daemon-reload`, so the unit that actually runs is whatever
|
||||
first_time_install.sh wrote on day one.
|
||||
|
||||
That makes every hardening added to a unit inert on existing installs.
|
||||
Measured on one rig: the installed unit was thirteen days older than the
|
||||
repo's and differed in content, so a MemoryMax the repo had specified
|
||||
was not being enforced at all -- `systemctl show` reported
|
||||
MemoryMax=infinity.
|
||||
|
||||
A warning rather than an error, and certainly not a silent rewrite:
|
||||
editing files under /etc and restarting services is the installer's job,
|
||||
not something a display process should do to a machine while it boots.
|
||||
The remedy is to re-run scripts/install/install_service.sh.
|
||||
"""
|
||||
try:
|
||||
project_root = Path(__file__).resolve().parent.parent
|
||||
for template_rel, installed_path in self._UNITS:
|
||||
template = project_root / template_rel
|
||||
installed = Path(installed_path)
|
||||
if not template.is_file() or not installed.is_file():
|
||||
continue
|
||||
|
||||
# The template carries placeholders the installer substitutes,
|
||||
# so compare the substituted form rather than the raw file.
|
||||
expected = template.read_text(encoding="utf-8")
|
||||
expected = expected.replace("__PROJECT_ROOT_DIR__", str(project_root))
|
||||
expected = expected.replace("__USER__", "root")
|
||||
|
||||
try:
|
||||
actual = installed.read_text(encoding="utf-8")
|
||||
except PermissionError:
|
||||
continue
|
||||
|
||||
if self._unit_body(expected) != self._unit_body(actual):
|
||||
self.warnings.append(
|
||||
f"{installed.name} differs from {template_rel}; the "
|
||||
"installed unit is not refreshed by an update, so "
|
||||
"settings added to the template are not in effect. "
|
||||
"Re-run scripts/install/install_service.sh to apply them."
|
||||
)
|
||||
except OSError as e:
|
||||
self.logger.debug("Could not compare systemd units: %s", e)
|
||||
|
||||
@staticmethod
|
||||
def _unit_body(text: str) -> str:
|
||||
"""A unit's meaningful lines: no comments, no blanks, no ordering noise."""
|
||||
lines = []
|
||||
for line in text.splitlines():
|
||||
line = line.strip()
|
||||
if line and not line.startswith("#"):
|
||||
lines.append(line)
|
||||
return "\n".join(sorted(lines))
|
||||
|
||||
def _validate_config(self) -> None:
|
||||
"""Validate configuration files."""
|
||||
try:
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
"""An installed unit that no longer matches the repo's must be reported.
|
||||
|
||||
Nothing re-applies systemd units after the first install. `git pull` -- what
|
||||
the web UI's update button runs -- brings a new template into the checkout, but
|
||||
no code in web_interface/ or src/ copies it to /etc/systemd/system or runs
|
||||
`systemctl daemon-reload`. The unit that actually runs is whatever
|
||||
first_time_install.sh wrote on day one.
|
||||
|
||||
So every hardening added to a unit is inert on existing installs. Measured on a
|
||||
live rig: the installed unit was dated 2026-08-06 and the repo's 2026-08-19,
|
||||
and they differed -- with the result that a MemoryMax=85% present in the repo's
|
||||
template was not being enforced at all. `systemctl show` reported
|
||||
MemoryMax=infinity.
|
||||
|
||||
This is a warning, not an error, and deliberately not a silent rewrite:
|
||||
editing files under /etc and restarting services is the installer's job, not
|
||||
something a display process should do to a machine while it boots.
|
||||
"""
|
||||
import logging
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from src.startup_validator import StartupValidator
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def validator():
|
||||
v = StartupValidator(config_manager=MagicMock())
|
||||
v.logger = logging.getLogger("test")
|
||||
v.warnings = []
|
||||
v.errors = []
|
||||
return v
|
||||
|
||||
|
||||
def test_a_matching_unit_produces_no_warning(validator, tmp_path):
|
||||
"""The installed unit, substituted exactly as the installer would."""
|
||||
project_root = Path("src/startup_validator.py").resolve().parent.parent
|
||||
template_rel = "systemd/ledmatrix.service"
|
||||
template = project_root / template_rel
|
||||
if not template.is_file():
|
||||
pytest.skip("repo unit template not present")
|
||||
|
||||
installed = tmp_path / "ledmatrix.service"
|
||||
installed.write_text(
|
||||
template.read_text(encoding="utf-8")
|
||||
.replace("__PROJECT_ROOT_DIR__", str(project_root))
|
||||
.replace("__USER__", "root"),
|
||||
encoding="utf-8")
|
||||
|
||||
validator._UNITS = ((template_rel, str(installed)),)
|
||||
validator._validate_systemd_units()
|
||||
assert not validator.warnings, f"a matching unit warned: {validator.warnings}"
|
||||
assert not validator.errors
|
||||
|
||||
|
||||
def test_comments_and_blank_lines_are_not_drift():
|
||||
"""Otherwise every comment the repo adds would look like a changed unit."""
|
||||
a = "[Service]\n# explains a setting\nExecStart=/x\nRestart=always\n"
|
||||
b = "[Service]\nExecStart=/x\n\nRestart=always\n"
|
||||
assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b)
|
||||
|
||||
|
||||
def test_a_changed_directive_is_drift():
|
||||
a = "[Service]\nExecStart=/x\nMemoryMax=85%\n"
|
||||
b = "[Service]\nExecStart=/x\n"
|
||||
assert StartupValidator._unit_body(a) != StartupValidator._unit_body(b)
|
||||
|
||||
|
||||
def test_reordered_directives_are_not_drift():
|
||||
"""systemd does not care about order within a section, so neither should this."""
|
||||
a = "[Service]\nExecStart=/x\nRestart=always\n"
|
||||
b = "[Service]\nRestart=always\nExecStart=/x\n"
|
||||
assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b)
|
||||
|
||||
|
||||
def test_cosmetic_differences_do_not_warn(validator, tmp_path):
|
||||
"""Through the real comparison, not the helper.
|
||||
|
||||
The repo's template carries explanatory comments the installed copy may not
|
||||
have, and the installer does not preserve ordering or blank lines. If those
|
||||
counted as drift, every boot would warn and the warning would be ignored.
|
||||
Asserting this on _unit_body alone would not catch a comparison that stopped
|
||||
calling it -- which is exactly what a careless edit does.
|
||||
"""
|
||||
project_root = Path("src/startup_validator.py").resolve().parent.parent
|
||||
template_rel = "systemd/ledmatrix.service"
|
||||
template = project_root / template_rel
|
||||
if not template.is_file():
|
||||
pytest.skip("repo unit template not present")
|
||||
|
||||
substituted = (template.read_text(encoding="utf-8")
|
||||
.replace("__PROJECT_ROOT_DIR__", str(project_root))
|
||||
.replace("__USER__", "root"))
|
||||
# Same directives, stripped of comments and blank lines and reordered.
|
||||
directives = sorted(line.strip() for line in substituted.splitlines()
|
||||
if line.strip() and not line.strip().startswith("#"))
|
||||
installed = tmp_path / "ledmatrix.service"
|
||||
installed.write_text("\n".join(reversed(directives)) + "\n", encoding="utf-8")
|
||||
|
||||
validator._UNITS = ((template_rel, str(installed)),)
|
||||
validator._validate_systemd_units()
|
||||
assert not validator.warnings, (
|
||||
f"cosmetic-only difference reported as drift: {validator.warnings}")
|
||||
|
||||
|
||||
def test_drift_is_reported_as_a_warning(validator, tmp_path):
|
||||
"""The whole point: a real difference must surface, and only as a warning."""
|
||||
installed = tmp_path / "ledmatrix.service"
|
||||
installed.write_text("[Service]\nExecStart=/usr/bin/python3 /x/run.py\n")
|
||||
|
||||
project_root = Path("src/startup_validator.py").resolve().parent.parent
|
||||
template_rel = "systemd/ledmatrix.service"
|
||||
template = project_root / template_rel
|
||||
if not template.is_file():
|
||||
pytest.skip("repo unit template not present")
|
||||
|
||||
validator._UNITS = ((template_rel, str(installed)),)
|
||||
validator._validate_systemd_units()
|
||||
|
||||
assert validator.warnings, "a differing unit produced no warning"
|
||||
assert "install_service.sh" in validator.warnings[0], (
|
||||
"the warning does not tell the user how to fix it")
|
||||
assert not validator.errors, "drift must not be fatal at startup"
|
||||
|
||||
|
||||
def test_a_missing_installed_unit_is_silent(validator, tmp_path):
|
||||
"""Development checkouts have no /etc/systemd unit; that is not drift."""
|
||||
validator._UNITS = (("systemd/ledmatrix.service", str(tmp_path / "absent.service")),)
|
||||
validator._validate_systemd_units()
|
||||
assert not validator.warnings
|
||||
assert not validator.errors
|
||||
@@ -1,84 +0,0 @@
|
||||
"""A pull that changed nothing on the running system is not an applied update.
|
||||
|
||||
git_pull replaces files on disk and restarts nothing -- there is no systemctl
|
||||
call anywhere in the handler. The display and web services keep running the
|
||||
code they loaded at boot, so the user is told "Code updated successfully" and
|
||||
sees no change until they happen to reboot. The response now says whether a
|
||||
restart is owed, and the UI raises the existing restart-pending banner.
|
||||
"""
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from web_interface.blueprints import api_v3 as mod # noqa: E402
|
||||
from web_interface.blueprints.api_v3 import api_v3 # noqa: E402
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
app = Flask(__name__)
|
||||
app.config['TESTING'] = True
|
||||
app.register_blueprint(api_v3, url_prefix='/api/v3')
|
||||
# The handler consults these after a successful pull; None is the
|
||||
# "not wired up" case it already guards for.
|
||||
api_v3.plugin_store_manager = None
|
||||
api_v3.config_manager = None
|
||||
return app.test_client()
|
||||
|
||||
|
||||
def _git(heads, pull_rc=0, pull_out='Updating a1b2c3..d4e5f6\n'):
|
||||
"""Fake git. `heads` are the successive answers to rev-parse HEAD."""
|
||||
seq = list(heads)
|
||||
|
||||
def run(args, **kwargs):
|
||||
def ok(stdout='', rc=0, b=False):
|
||||
return subprocess.CompletedProcess(
|
||||
args, rc, stdout=(stdout.encode() if b else stdout),
|
||||
stderr=(b'' if b else ''))
|
||||
if args[:2] == ['git', 'rev-parse'] and args[-1] == 'HEAD':
|
||||
return ok(seq.pop(0) + '\n' if seq else 'deadbeef\n')
|
||||
if 'symbolic-full-name' in args or '@{u}' in args:
|
||||
return ok('origin/main\n')
|
||||
if args[:2] == ['git', 'status']:
|
||||
return ok('')
|
||||
if args[:2] == ['git', 'diff']:
|
||||
return ok('')
|
||||
if args[:2] == ['git', 'pull']:
|
||||
return ok(pull_out, pull_rc)
|
||||
return ok('')
|
||||
return run
|
||||
|
||||
|
||||
def _pull(client):
|
||||
return client.post('/api/v3/system/action',
|
||||
json={'action': 'git_pull'}).get_json()
|
||||
|
||||
|
||||
class TestRestartIsRequestedWhenCodeChanged:
|
||||
def test_a_pull_that_moved_head_asks_for_a_restart(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'success'
|
||||
assert data['restart_required'] is True, (
|
||||
"new code on disk, services still running the old code, and "
|
||||
"nothing told the user to restart")
|
||||
|
||||
def test_already_up_to_date_does_not(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'success'
|
||||
assert data['restart_required'] is False, (
|
||||
"prompting after a no-op update trains users to ignore the prompt")
|
||||
|
||||
def test_a_failed_pull_does_not(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'error'
|
||||
assert data['restart_required'] is False
|
||||
@@ -1996,11 +1996,6 @@ def execute_system_action():
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning("git rev-parse timed out before pull")
|
||||
|
||||
# Whether the pull actually brought new code in. "Already up to
|
||||
# date" is a success too, and prompting for a restart then would
|
||||
# train users to ignore the prompt.
|
||||
code_changed = False
|
||||
|
||||
# Perform the git pull. Branches without an upstream were given
|
||||
# an explicit "origin <branch>" above so the update still works.
|
||||
result = subprocess.run(
|
||||
@@ -2044,7 +2039,6 @@ def execute_system_action():
|
||||
capture_output=True, text=True, timeout=10, cwd=project_dir)
|
||||
new_head = _post.stdout.strip() if _post.returncode == 0 else None
|
||||
if old_head and new_head and old_head != new_head:
|
||||
code_changed = True
|
||||
diff = subprocess.run(
|
||||
['git', 'diff', '--name-only', f'{old_head}..{new_head}'],
|
||||
capture_output=True, text=True, timeout=15, cwd=project_dir)
|
||||
@@ -2104,14 +2098,9 @@ def execute_system_action():
|
||||
if ln.strip()), '')
|
||||
pull_message = f"Update failed: {detail}" if detail else "Update failed; check logs for details"
|
||||
|
||||
# Nothing here restarts anything: the pull replaces files on
|
||||
# disk while the display and web services keep running the code
|
||||
# they loaded at boot. Without this the user is told the update
|
||||
# succeeded and sees no change until they happen to reboot.
|
||||
return jsonify({
|
||||
'status': 'success' if result.returncode == 0 else 'error',
|
||||
'message': pull_message,
|
||||
'restart_required': bool(result.returncode == 0 and code_changed),
|
||||
})
|
||||
elif action == 'checkout_branch':
|
||||
# Switch branches from the Tools tab. Needed because a checkout
|
||||
|
||||
@@ -116,25 +116,14 @@ document.body.addEventListener('htmx:afterRequest', function(event) {
|
||||
// ===== Restart-pending banner =====
|
||||
// Shown after restart-requiring saves; persists across tab switches (and
|
||||
// reloads, via sessionStorage) until the display restarts or it's dismissed.
|
||||
window.showRestartPending = function(message) {
|
||||
try {
|
||||
sessionStorage.setItem('ledmatrix-restart-pending', '1');
|
||||
// Persisted alongside the flag: a code update and a config save want
|
||||
// different wording, and the banner outlives the page that raised it.
|
||||
if (message) sessionStorage.setItem('ledmatrix-restart-pending-text', message);
|
||||
else sessionStorage.removeItem('ledmatrix-restart-pending-text');
|
||||
} catch { /* private browsing */ }
|
||||
window.showRestartPending = function() {
|
||||
try { sessionStorage.setItem('ledmatrix-restart-pending', '1'); } catch { /* private browsing */ }
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
const text = document.getElementById('restart-pending-text');
|
||||
if (text && message) text.textContent = message;
|
||||
if (banner) banner.style.display = 'block';
|
||||
};
|
||||
|
||||
window.dismissRestartPending = function() {
|
||||
try {
|
||||
sessionStorage.removeItem('ledmatrix-restart-pending');
|
||||
sessionStorage.removeItem('ledmatrix-restart-pending-text');
|
||||
} catch { /* no-op */ }
|
||||
try { sessionStorage.removeItem('ledmatrix-restart-pending'); } catch { /* no-op */ }
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
if (banner) banner.style.display = 'none';
|
||||
};
|
||||
@@ -162,9 +151,6 @@ document.addEventListener('DOMContentLoaded', function() {
|
||||
try {
|
||||
if (sessionStorage.getItem('ledmatrix-restart-pending') === '1') {
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
const saved = sessionStorage.getItem('ledmatrix-restart-pending-text');
|
||||
const text = document.getElementById('restart-pending-text');
|
||||
if (text && saved) text.textContent = saved;
|
||||
if (banner) banner.style.display = 'block';
|
||||
}
|
||||
} catch { /* no-op */ }
|
||||
|
||||
@@ -413,8 +413,7 @@
|
||||
<div class="flex items-center justify-between">
|
||||
<div class="flex items-center space-x-3">
|
||||
<i class="fas fa-rotate text-lg"></i>
|
||||
<span class="text-sm font-medium" aria-live="polite"
|
||||
id="restart-pending-text">
|
||||
<span class="text-sm font-medium" aria-live="polite">
|
||||
Configuration saved — restart the display to apply the changes
|
||||
</span>
|
||||
</div>
|
||||
@@ -1147,13 +1146,6 @@
|
||||
if (data.status === 'success') {
|
||||
document.getElementById('update-banner').style.display = 'none';
|
||||
try { sessionStorage.removeItem('update-sha-dismissed'); } catch(e) {}
|
||||
// The pull replaced files on disk; the running services still
|
||||
// hold the code they loaded at boot. Ask for the restart that
|
||||
// makes the update actually take effect.
|
||||
if (data.restart_required && typeof window.showRestartPending === 'function') {
|
||||
window.showRestartPending(
|
||||
'Update installed \u2014 restart the display to run the new code');
|
||||
}
|
||||
}
|
||||
if (typeof showNotification === 'function') {
|
||||
showNotification(data.message || 'Update complete', data.status || 'success');
|
||||
|
||||
Reference in New Issue
Block a user