installer 1.24.0: a PRE-EXISTING backup target is granted too (R-185)
gates / gates (push) Successful in 7s
gates / gates (push) Successful in 7s
configure_backup_target has two arms and only one granted. Case A creates the
storage and grants in the same breath; the Scenario-F arm ('the target already
exists') returned without granting. A box whose felhom-backup pre-dated the
install therefore pointed local_backup_target at a storage its own token could
not read — measured on BOTH demo boxes: {"data":[]} through the token while root
lists three archives. That tier was never restore-tested and nothing said so,
because an empty listing is also what a brand-new tier returns.
The reuse arm now ensures the ACL through the same guarded wrapper. Scenario F is
unviolated: the storage DEFINITION is untouched, and pveum acl modify is
idempotent. BACKUP_TARGET_ID is deliberately NOT added to PVE_STORAGES — that
list is granted a step before the target is resolved, and --acl-storages entries
are preflight-checked for existence; the comment now says so.
A gate asserts it: every arm that resolves the target must also grant on it.
Red-proved by reverting the arm.
This commit is contained in:
@@ -1,3 +1,32 @@
|
||||
## 1.24.0 — a pre-existing backup target is granted too (2026-08-03, R-185)
|
||||
|
||||
**`configure_backup_target` has two arms and only one of them granted.** The Case A arm creates the
|
||||
storage and calls `felhom-backup-target-apply grant` in the same breath — a box that builds its own
|
||||
target has always been fine. The **Scenario-F arm** — *"the target already exists, leave it exactly as
|
||||
it is"* — returned without ever granting.
|
||||
|
||||
So a box whose `felhom-backup` pre-dated the install (created by the vzdump-target-move runbook, or
|
||||
surviving a reinstall) ended up with `local_backup_target: felhom-backup` while its token held
|
||||
`FelhomAgentStore` on only `local`, `local-lvm` and `felhom-pbs`. Measured on **both** demo boxes
|
||||
2026-08-03: the content API answers `{"data":[]}` through the agent's token while root lists three
|
||||
archives. That tier was invisible to the agent and never restore-tested — and nothing said so,
|
||||
because an empty listing is also what a brand-new tier returns.
|
||||
|
||||
The reuse arm now ensures the ACL through the same guarded wrapper, so both arms leave the box in the
|
||||
same state. **Scenario F is unviolated:** the storage DEFINITION is still untouched — granting the
|
||||
role the agent is supposed to have on the target this script is about to write into `agent.json` is
|
||||
finishing the job, not retargeting the box. `pveum acl modify` is idempotent, so a box that already
|
||||
has the grant is unchanged and a box whose token was rotated gets it back.
|
||||
|
||||
`$BACKUP_TARGET_ID` is deliberately **not** added to `PVE_STORAGES`, and the comment now says why: that
|
||||
list is granted in step 4/5, before the target has been resolved in step 6, and `--acl-storages`
|
||||
entries are preflight-checked for existence. The grant belongs with the resolution, which is where it
|
||||
already was for a newly created target.
|
||||
|
||||
**A gate now asserts it** (`hostinstall_gates.py`): every arm of `configure_backup_target` that
|
||||
resolves the target must also grant on it. Red-proved by reverting the reuse arm — `resolves the
|
||||
backup target in 2 place(s) but grants in only 1`.
|
||||
|
||||
## v1.23.0 — the installer is published, not pushed (2026-08-03, R-110 + R-183)
|
||||
|
||||
**Two channels moved off `main` in the same change, because either one left behind makes the other
|
||||
|
||||
@@ -184,7 +184,7 @@
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_VERSION="1.23.0" # the SINGLE version source (F-1): -h and the run banners follow it.
|
||||
SCRIPT_VERSION="1.24.0" # the SINGLE version source (F-1): -h and the run banners follow it.
|
||||
# The hub used to carry a copy for its Setup tab; R-94 DELETED it
|
||||
# (2026-08-02) because the hub cannot know which version a box runs —
|
||||
# the Setup command fetches this script at run time. scripts/
|
||||
@@ -307,6 +307,14 @@ PVE_PRIVS_STORE="Datastore.Allocate Datastore.AllocateSpace"
|
||||
PVE_PRIVS_BASE="Sys.Audit SDN.Use Datastore.Audit"
|
||||
# Storages the agent reads/writes (archive+dump=local, restore=local-lvm, offsite DR=felhom-pbs). The
|
||||
# offsite felhom-pbs MUST be included or the agent's DR backup 403s (SPIKE residual #1). --acl-storages overrides.
|
||||
#
|
||||
# `$BACKUP_TARGET_ID` is deliberately NOT in this list, and the reason matters because the obvious
|
||||
# tidy-up is to add it (R-185, 2026-08-03). This list is granted in STEP 4/5, before
|
||||
# configure_backup_target has run in STEP 6 — so at this point the target may not exist yet, and
|
||||
# --acl-storages entries are preflight-checked for existence. The target's grant therefore belongs
|
||||
# with the target's RESOLUTION, where it already is for a freshly created one, and now also for a
|
||||
# pre-existing one. Adding it here would grant on a storage that may not exist and would still leave
|
||||
# the resolution path as the single owner of that decision, split across two places.
|
||||
PVE_STORAGES=(local local-lvm felhom-pbs)
|
||||
# E-2: the whole-guest backup target storage id, and what configure_backup_target resolved to.
|
||||
# BACKUP_TARGET_RESOLVED feeds agent.json's local_backup_target; "local" means DEGRADED (Case B).
|
||||
@@ -652,8 +660,31 @@ configure_backup_target() {
|
||||
# existing id is an error, and repointing a live target is exactly the silent retarget this
|
||||
# whole arc closes.
|
||||
if pvesm status --storage "$BACKUP_TARGET_ID" >/dev/null 2>&1; then
|
||||
log_skip " backup target '$BACKUP_TARGET_ID' already exists — leaving it exactly as it is (Scenario F)"
|
||||
log_skip " backup target '$BACKUP_TARGET_ID' already exists — leaving its DEFINITION exactly as it is (Scenario F)"
|
||||
BACKUP_TARGET_RESOLVED="$BACKUP_TARGET_ID"
|
||||
# R-185: …but STILL ensure the ACL. "The storage already exists" says nothing about whether
|
||||
# the agent may READ it, and this early return is where the two came apart.
|
||||
#
|
||||
# THE DEFECT THIS CLOSES, measured on both demo boxes 2026-08-03. The CASE A path below
|
||||
# creates the storage and grants in the same breath, so a box that built its own target is
|
||||
# fine. A box whose target ALREADY existed — created by the vzdump-target-move runbook, or
|
||||
# surviving a reinstall — returned here and never granted. The result: `local_backup_target`
|
||||
# pointed at `felhom-backup` while the token held FelhomAgentStore only on local, local-lvm
|
||||
# and felhom-pbs, so the API answered `{"data":[]}` for that storage while root saw three
|
||||
# archives. The tier was invisible to the agent and never restore-tested, and nothing said so
|
||||
# — because an empty listing is also what a brand-new tier returns.
|
||||
#
|
||||
# Scenario F is UNVIOLATED: the storage definition is still untouched. Granting a role the
|
||||
# agent is supposed to have on the target this same script is about to write into
|
||||
# agent.json is not "touching the box's target", it is finishing the job. `pveum acl modify`
|
||||
# is idempotent, so a box that already has the grant is unchanged, and a box that had its
|
||||
# token rotated gets it back.
|
||||
if $DRY_RUN; then
|
||||
log_dry "felhom-backup-target-apply grant $BACKUP_TARGET_ID # R-185: ACL on a pre-existing target"
|
||||
else
|
||||
/usr/local/sbin/felhom-backup-target-apply grant "$BACKUP_TARGET_ID" \
|
||||
|| die "backup target grant failed on the pre-existing target — the agent could not read its own backup tier (R-185)"
|
||||
fi
|
||||
return 0
|
||||
fi
|
||||
local mp
|
||||
|
||||
@@ -202,6 +202,37 @@ if wp is not None:
|
||||
fail("manifests/webpage.yaml no longer tracks main for the website — pinning the SITE to "
|
||||
"the installer tag turns every copy edit into a release")
|
||||
|
||||
# ── R-185: every path that RESOLVES the backup target must also grant on it ──────────────────
|
||||
#
|
||||
# THE DEFECT THIS WOULD HAVE CAUGHT, measured on both demo boxes 2026-08-03. `configure_backup_target`
|
||||
# has two arms. The CASE A arm creates the storage and grants in the same breath. The Scenario-F arm —
|
||||
# "the target already exists, leave it alone" — returned WITHOUT granting, so a box whose target
|
||||
# pre-dated the install pointed `local_backup_target` at a storage its own token could not read. The
|
||||
# API answered `{"data":[]}` while root saw three archives, and nothing said so, because an empty
|
||||
# listing is also what a brand-new tier returns.
|
||||
#
|
||||
# The assertion is deliberately about the FUNCTION, not about PVE_STORAGES: the target's grant belongs
|
||||
# with the target's resolution (PVE_STORAGES is granted a step earlier, before the target exists), so
|
||||
# what must hold is that no arm of that function can resolve a target and skip the grant.
|
||||
fn = re.search(r'^configure_backup_target\(\)\s*\{(.*?)^\}', src, re.S | re.M)
|
||||
if not fn:
|
||||
fail("cannot find configure_backup_target() — the backup-target ACL assertion cannot run, and a "
|
||||
"check that cannot run must never report OK (R-185)")
|
||||
else:
|
||||
body = fn.group(1)
|
||||
resolutions = len(re.findall(r'BACKUP_TARGET_RESOLVED="\$BACKUP_TARGET_ID"', body))
|
||||
grants = len(re.findall(r'felhom-backup-target-apply grant', body))
|
||||
if resolutions == 0:
|
||||
fail("configure_backup_target no longer resolves BACKUP_TARGET_ID anywhere — re-read it")
|
||||
elif grants >= resolutions:
|
||||
ok("every arm that resolves the backup target also grants on it (%d resolution(s), %d grant(s))"
|
||||
% (resolutions, grants))
|
||||
else:
|
||||
fail("configure_backup_target resolves the backup target in %d place(s) but grants in only %d "
|
||||
"— an arm resolves a target the agent may not READ. That is R-185: the tier's archives are "
|
||||
"invisible to the agent, it is never restore-tested, and an empty listing looks exactly "
|
||||
"like a brand-new tier." % (resolutions, grants))
|
||||
|
||||
print()
|
||||
if fails:
|
||||
print("hostinstall gates: %d FAILURE(S)" % len(fails))
|
||||
|
||||
Reference in New Issue
Block a user