Skip to content

Commit 8e9d0d0

Browse files
committed
fix(appliance): an expired deadline stops locking the operator out
A verification deadline that is armed disables both the update and the revert control. The comment above the gate already says why, and already says the right rule: "A deadline in flight is the one state that blocks both buttons: a second install would replace the package whose verdict the appliance is still waiting for." The code did not check in flight. It checked whether a deadline file exists. On the appliance that found this, a deadline armed for 0.2.0 sat past its window with the verdict still pending, and nothing ever judged it. Both controls were dead. The documented cure is `systemctl start ems-appliance-manager-verify.service` -- a keyboard at the console, which the ADR names as the backstop rather than the plan, and which an appliance owner often does not have. The timer was enabled, its symlink present in timers.target.wants, and it still never ran; that cause is not diagnosed here. An expired deadline can no longer revert anything: its window is what gave it the right to. Holding both controls shut on it protects nothing that the window could still protect, and takes away the last lever there is. So the gate now blocks while the window has not passed, and lets go when it has, which is what the comment said all along. Judged by the appliance's own clock, not the browser's: that clock belongs to a different machine, and this board has no real-time clock. A clock the console cannot read keeps the lock, because a deadline it cannot place in time is one that may still fire. The card stops lying too. "Waiting for the deadline" and "nothing else can be started until it settles" become "the deadline passed and nothing judged it", saying plainly that nothing was reverted and nothing will be, and naming the timer to look at when it recurs. The clock is a parameter rather than a read of `state`. Taking it from the global broke every test in test_appliance_manager_frontend.py at once, which is that module's whole point: both decisions are derived from backend values alone, so they can be evaluated without a page around them. The signature contract now pins the arity, and two cases pin the rule directly -- one deadline at 999 and 1001 against an epoch of 1000, and a clock of 0, null and "later" that all keep the lock. Nothing about what may be installed changes. The deadline is still armed by the outgoing package, still runs a reverter copied out of it, and still reverts an install that does not prove itself inside the window.
1 parent a34aa05 commit 8e9d0d0

4 files changed

Lines changed: 124 additions & 20 deletions

File tree

appliance/static/app.js

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,13 +1200,33 @@
12001200
from the backend payload alone. A deadline in flight is the one state that
12011201
blocks both buttons: a second install would replace the package whose
12021202
verdict the appliance is still waiting for. */
1203-
function managerActions(manager) {
1203+
/* The appliance's own clock, which is the only one that may judge its
1204+
deadline: the browser's belongs to a different machine, and this board has
1205+
no real-time clock. Passed in rather than read from state inside
1206+
managerActions, so both decisions stay derived from backend values alone
1207+
and stay testable without a page around them. */
1208+
function applianceNow() {
1209+
return Number((((state.data.status || {}).system || {}).time || {}).epoch) || 0;
1210+
}
1211+
1212+
function managerActions(manager, now) {
12041213
var verify = (manager || {}).verify || {};
12051214
var armed = verify.armed === true;
1215+
/* In flight means the window has not passed, not merely that a deadline is
1216+
on disk. A deadline whose window expired without a verdict is a mechanism
1217+
that did not run: it can no longer revert anything, and locking both
1218+
controls on it removes the operator's last lever on a box whose only
1219+
documented alternative is a keyboard at the console. An unreadable clock
1220+
keeps the lock, because a deadline this cannot place may still fire. */
1221+
var expiry = Number(verify.deadline_epoch) || 0;
1222+
now = Number(now) || 0;
1223+
var inFlight = armed && !(now && expiry && now >= expiry);
12061224
return {
12071225
armed: armed,
1208-
canUpdate: !armed,
1209-
canRevert: (manager || {}).can_revert === true && !armed
1226+
inFlight: inFlight,
1227+
expiredUnjudged: armed && !inFlight,
1228+
canUpdate: !inFlight,
1229+
canRevert: (manager || {}).can_revert === true && !inFlight
12101230
};
12111231
}
12121232

@@ -1239,7 +1259,7 @@
12391259
"data-direction": entry.direction || "",
12401260
title: entry.release_id,
12411261
text: "Install " + managerLabel(entry),
1242-
disabled: !managerActions(manager).canUpdate,
1262+
disabled: !managerActions(manager, applianceNow()).canUpdate,
12431263
onclick: function () {
12441264
planOperation({
12451265
endpoint: "/api/manager/plan-update",
@@ -1265,6 +1285,7 @@
12651285
package to every appliance at once. */
12661286
function renderManagerUpdates(main) {
12671287
var manager = state.data.manager;
1288+
var actions = managerActions(manager, applianceNow());
12681289
if (manager === undefined || manager === null) {
12691290
main.appendChild(el("h2", { class: "section-title", text: "Appliance Manager" }));
12701291
main.appendChild(el("p", { class: "section-hint", text: "Reading the manager state\u2026" }));
@@ -1305,19 +1326,21 @@
13051326
], "manager-kept"),
13061327
card("Last install", [
13071328
el("p", { class: "status-value" }, [
1308-
verify.armed
1329+
actions.inFlight
13091330
? tone("warn", "waiting for the deadline")
1310-
: (verdict.settled
1311-
? tone(MANAGER_VERDICTS[verdict.verdict] ? MANAGER_VERDICTS[verdict.verdict][0] : "warn",
1312-
format(verdict.verdict))
1313-
: tone("ok", "nothing in flight"))
1331+
: actions.expiredUnjudged
1332+
? tone("bad", "deadline expired without a verdict")
1333+
: (verdict.settled
1334+
? tone(MANAGER_VERDICTS[verdict.verdict] ? MANAGER_VERDICTS[verdict.verdict][0] : "warn",
1335+
format(verdict.verdict))
1336+
: tone("ok", "nothing in flight"))
13141337
]),
13151338
fact("Result", (manager.outcome || {}).outcome),
13161339
verify.armed ? fact("Expecting", verify.expected_version) : null
13171340
], "manager-verify")
13181341
]));
13191342

1320-
if (verify.armed) {
1343+
if (actions.inFlight) {
13211344
main.appendChild(el("p", { class: "empty-state", "data-test": "manager-deadline" }, [
13221345
el("strong", { text: "An install is being judged. " }),
13231346
el("span", {
@@ -1326,6 +1349,16 @@
13261349
+ "package is installed again. Nothing else can be started until it settles."
13271350
})
13281351
]));
1352+
} else if (actions.expiredUnjudged) {
1353+
main.appendChild(el("p", { class: "empty-state", "data-test": "manager-deadline-expired" }, [
1354+
el("strong", { text: "The deadline passed and nothing judged it. " }),
1355+
el("span", {
1356+
text: "This appliance armed a deadline for " + format(verify.expected_version)
1357+
+ " and its window has closed without a verdict, so nothing was reverted and nothing "
1358+
+ "will be. Installing or reverting is available again; the next install replaces "
1359+
+ "this deadline. Check ems-appliance-manager-verify.timer if it keeps happening."
1360+
})
1361+
]));
13291362
} else if (verdict.settled && verdict.verdict !== "confirmed") {
13301363
main.appendChild(el("p", { class: "empty-state", "data-test": "manager-verdict" }, [
13311364
el("strong", { text: "The last manager install " }),
@@ -1350,7 +1383,7 @@
13501383
el("button", {
13511384
type: "button", class: "ghost-button compact", "data-test": "manager-plan-revert",
13521385
text: "Reinstall " + (manager.can_revert ? format(kept.version) : "\u2014"),
1353-
disabled: !managerActions(manager).canRevert,
1386+
disabled: !managerActions(manager, applianceNow()).canRevert,
13541387
onclick: function () {
13551388
planOperation({
13561389
endpoint: "/api/manager/plan-revert", body: {},

tests/e2e-appliance/appliance.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,28 @@ test.describe("appliance manager @smoke", () => {
865865
await expect(page.locator('[data-test="manager-plan-revert"]')).toBeDisabled();
866866
});
867867

868+
test("a deadline whose window closed without a verdict stops blocking", async ({
869+
page,
870+
request,
871+
}) => {
872+
// The lockout is for a deadline in flight. One whose window passed without a
873+
// verdict never ran, cannot revert anything any more, and locking the
874+
// controls on it leaves an operator with no lever but a keyboard at the
875+
// console -- which an appliance owner often does not have.
876+
await resetAppliance(request, {
877+
manager_package_kept: true,
878+
manager_deadline_expired: true,
879+
});
880+
await signIn(page);
881+
await openView(page, "updates");
882+
883+
await expect(page.locator('[data-test="manager-deadline"]')).toHaveCount(0);
884+
const notice = page.locator('[data-test="manager-deadline-expired"]');
885+
await expect(notice).toContainText("nothing judged it");
886+
await expect(notice).toContainText("0.3.0");
887+
await expect(page.locator('[data-test="manager-plan-revert"]')).toBeEnabled();
888+
});
889+
868890
test("a reverted install is reported rather than left silent", async ({ page, request }) => {
869891
await resetAppliance(request, { manager_package_kept: true, manager_verdict: "reverted" });
870892
await signIn(page);

tests/e2e-appliance/serve_appliance_test.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def seed_rescue_account(root, *, changed=False, absent=False):
7878
)
7979

8080

81-
def seed_manager_state(services, *, kept=False, verdict="", armed=False):
81+
def seed_manager_state(services, *, kept=False, verdict="", armed=False, deadline_expired=False):
8282
"""What the Appliance Manager card reports, written the way the host does."""
8383

8484
from appliance import manager_retention, manager_verify
@@ -130,7 +130,10 @@ def seed_manager_state(services, *, kept=False, verdict="", armed=False):
130130
"build_id": "20260901000000",
131131
"previous_path": str(packages / manager_retention.PREVIOUS_NAME),
132132
"armed_at": 1787000000,
133-
"deadline_epoch": 4000000000,
133+
# In the past when the window is meant to have closed: the
134+
# console judges "in flight" by the appliance's own clock,
135+
# so the fixture has to move the deadline, not the clock.
136+
"deadline_epoch": 1787000900 if deadline_expired else 4000000000,
134137
"window_seconds": 900,
135138
}
136139
),
@@ -335,7 +338,9 @@ def seed_appliance_state(options=None):
335338
services,
336339
kept=bool(options.get("manager_package_kept")),
337340
verdict=str(options.get("manager_verdict") or ""),
338-
armed=bool(options.get("manager_deadline_armed")),
341+
armed=bool(options.get("manager_deadline_armed"))
342+
or bool(options.get("manager_deadline_expired")),
343+
deadline_expired=bool(options.get("manager_deadline_expired")),
339344
)
340345

341346
seed_appliance_state({})

tests/test_appliance_manager_frontend.py

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,12 @@ def extract(name):
4141
raise AssertionError(f"{name} is not a closed function")
4242

4343

44-
def evaluate(name, payload, *, preamble=""):
44+
def evaluate(name, payload, *arguments, preamble=""):
45+
rendered = ", ".join(json.dumps(value) for value in (payload,) + arguments)
4546
script = (
4647
preamble
4748
+ extract(name)
48-
+ f"\nconsole.log(JSON.stringify({name}("
49-
+ json.dumps(payload)
50-
+ ")));\n"
49+
+ f"\nconsole.log(JSON.stringify({name}({rendered})));\n"
5150
)
5251
result = subprocess.run(
5352
[node, "-"], input=script, capture_output=True, text=True, timeout=120
@@ -79,9 +78,16 @@ def manager(**overrides):
7978

8079

8180
def test_the_action_gate_is_a_named_function():
82-
"""A gate buried in a render call cannot be tested at all."""
81+
"""A gate buried in a render call cannot be tested at all.
8382
84-
assert "function managerActions(manager)" in APP
83+
The clock is a parameter for the same reason. Reading it from ``state``
84+
inside the gate made both decisions depend on a page being around them,
85+
which is exactly what this module exists to prevent -- and it broke every
86+
test here at once.
87+
"""
88+
89+
assert "function managerActions(manager, now)" in APP
90+
assert "function applianceNow()" in APP
8591

8692

8793
@requires_node
@@ -110,6 +116,44 @@ def test_an_armed_deadline_blocks_both_buttons():
110116
assert result["canRevert"] is False
111117

112118

119+
@requires_node
120+
def test_a_deadline_whose_window_closed_stops_blocking():
121+
"""The lockout is for a deadline in flight, and one that expired is not.
122+
123+
It can no longer revert anything, so holding both controls shut on it
124+
protects nothing and leaves the operator with the console as the only way
125+
on -- which is the backstop, not the plan.
126+
"""
127+
128+
armed = manager(
129+
can_revert=True,
130+
verify={"armed": True, "expected_version": "0.2.0", "deadline_epoch": 1000},
131+
)
132+
133+
assert evaluate("managerActions", armed, 999)["canUpdate"] is False
134+
expired = evaluate("managerActions", armed, 1001)
135+
assert expired["armed"] is True
136+
assert expired["inFlight"] is False
137+
assert expired["expiredUnjudged"] is True
138+
assert expired["canUpdate"] is True
139+
assert expired["canRevert"] is True
140+
141+
142+
@requires_node
143+
def test_a_clock_it_cannot_read_keeps_the_deadline_shut():
144+
"""Fail closed: a deadline this cannot place in time may still fire."""
145+
146+
armed = manager(
147+
can_revert=True,
148+
verify={"armed": True, "expected_version": "0.2.0", "deadline_epoch": 1000},
149+
)
150+
151+
for unusable in (0, None, "later"):
152+
result = evaluate("managerActions", armed, unusable)
153+
assert result["canUpdate"] is False, unusable
154+
assert result["canRevert"] is False, unusable
155+
156+
113157
@requires_node
114158
def test_a_missing_payload_enables_nothing_it_cannot_prove():
115159
result = evaluate("managerActions", {})

0 commit comments

Comments
 (0)