fix(station): Manifest-Merge robust gegen Fehlform, Version pro Eintrag
Fixrunde 2 zu "Migrationsimages beim Deploy signieren":
- N1 (Important): das Merge-Skript im SSH-Heredoc von deploy.sh fing beim
Bestandsmanifest nur Lesefehler und kaputtes JSON ab (OSError/ValueError).
War die Datei gueltiges JSON mit falscher Form ("[]", "null", ein String,
oder ein images-Eintrag, der selbst kein Objekt ist), krachte .get() mit
AttributeError - und das VOR der Installation von
server.py/products.py/CLI/Unit und vor dem Neustart, liess einen Pi also
mit neuen Images, aber altem Stationscode zurueck. Jetzt wird alte wie
neue Manifest-Struktur konsequent auf isinstance(dict)/isinstance(list)
geprueft, jede Fehlform faellt auf ein leeres Manifest zurueck statt
abzustuerzen. Neues Szenario 21 deckt alle vier Faelle aus der
Review-Tabelle ab (kaputt, leer, falsch geformtes JSON, kaputter Eintrag).
- N2 (Important): das zusammengefuehrte Manifest hat "ninlight_version"
bisher blind vom neuen Manifest uebernommen, obwohl uebernommene
Alt-Eintraege (deren Images unveraendert liegen bleiben, siehe Fixrunde 1
I1) eine andere Firmware-Version tragen koennen - eine konkrete falsche
Version beim Protokollieren ist schlechter als das vorherige "unbekannt".
Die Version wandert jetzt PRO EINTRAG ins Manifest (deploy.sh erzeugt sie
dort, der Merge behaelt sie); das manifestweite Feld bleibt nur als
Bestandskompatibilitaet und wird nur gesetzt, wenn wirklich alle Eintraege
uebereinstimmen. server.py liest beim Ausliefern jetzt die Version DES
AUSGELIEFERTEN EINTRAGS (Station.image_version(target, variant) ersetzt
manifest_version()) und faellt nur auf das manifestweite Feld zurueck,
wenn der Eintrag selbst keine eigene traegt. test_server.py bekommt einen
neuen Test fuer den Vorrang des Eintrags vor dem manifestweiten Feld,
Szenario 14 in test_deploy.sh prueft jetzt zusaetzlich unterschiedliche
Versionen zwischen zusammengefuehrten Eintraegen und dass das
manifestweite Feld dann fehlt statt falsch zu sein. Die Spec
(docs/superpowers/specs/.../design.md, Abschnitt 4, lokal/nicht
versioniert) ist nachgezogen. Die dadurch ueberfluessig gewordene
"sorted(versions)[0]"-Stelle aus Fixrunde 1 ist entfallen.
- Kleinigkeit: die Remote-Meldung, wenn migrate/ von diesem Rechner leer
ist, behauptet nicht mehr "kein Schluesselverzeichnis" (das stimmt im
Fall "Verzeichnis vorhanden, aber leer" nicht) - sie nennt jetzt keinen
bestimmten Grund mehr, nur die Konsequenz.
- Kleinigkeit: --exclude=manifest.json ist jetzt --exclude=/manifest.json,
konsistent mit dem daneben verankerten --exclude=/migrate.
Alle drei Aenderungen (N1, N2, beide Kleinigkeiten) sind vor dem Fix per
Rot-Probe an einer Kopie von deploy.sh verifiziert: mit den urspruenglichen
Fehlern schlagen Szenario 21 (N1) bzw. Szenario 14 (N2) zuverlaessig fehl.
Bewusst nicht angefasst (Review-Vorgabe): der fehlende HOME-Testfall, und
dass der Merge kein os.path.exists() prueft.
This commit is contained in:
Michael Sauer
+53
-21
@@ -136,8 +136,12 @@ if [ -d "$SIGN_DIR" ]; then
|
||||
# berechnet statt aus dem Signierschritt uebernommen, damit die Datei die Wahrheit
|
||||
# ueber das tatsaechlich abgelegte Image sagt. Form haelt sich an die Spec
|
||||
# (docs/superpowers/spec, Abschnitt 4) und damit an das, was server.py tatsaechlich
|
||||
# liest: "ninlight_version" auf oberster Ebene, nicht pro Eintrag (Fixrunde 1, C1 -
|
||||
# der Server las vorher immer "unbekannt").
|
||||
# liest. Fixrunde 2, N2: "ninlight_version" steht PRO EINTRAG, nicht (mehr nur)
|
||||
# manifestweit - ein einziges Feld fuer das ganze Manifest war schon falsch, sobald
|
||||
# der Merge unten (bei nur einem Schluessel oder gezieltem Deploy) alte und neue
|
||||
# Eintraege mit unterschiedlichen Firmware-Versionen zusammenfuehrt. Das
|
||||
# manifestweite Feld bleibt nur als Bestandskompatibilitaet erhalten und wird nur
|
||||
# gesetzt, wenn wirklich alle Eintraege dieselbe Version tragen.
|
||||
python3 - "$STAGE/migrate" "${VERSION_ARGS[@]}" <<'PY'
|
||||
import hashlib
|
||||
import json
|
||||
@@ -163,23 +167,18 @@ for name in sorted(os.listdir(migrate_dir)):
|
||||
"file": name,
|
||||
"bytes": len(data),
|
||||
"sha256": hashlib.sha256(data).hexdigest(),
|
||||
"ninlight_version": version_map.get(target, ""),
|
||||
})
|
||||
|
||||
# Die Spec kennt nur EINE Firmware-Version pro Manifest, nicht eine pro Ziel. Im
|
||||
# Regelfall bauen alle Ziele eines Deploys aus demselben Firmware-Stand, weichen die
|
||||
# Versionen trotzdem ab, ist das kein Grund zum Abbrechen - nur eine Warnung, und die
|
||||
# alphabetisch erste Version gewinnt (deterministisch, nachvollziehbar).
|
||||
versions = sorted({version_map.get(e["target"], "") for e in entries})
|
||||
if len(versions) > 1:
|
||||
print(f"Warnung: unterschiedliche Firmware-Versionen in diesem Deploy {versions} "
|
||||
f"- manifest.json verwendet {versions[0]}", file=sys.stderr)
|
||||
ninlight_version = versions[0] if versions else ""
|
||||
|
||||
manifest = {
|
||||
"built": datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"),
|
||||
"ninlight_version": ninlight_version,
|
||||
"images": entries,
|
||||
}
|
||||
versions = {e["ninlight_version"] for e in entries}
|
||||
if len(versions) == 1:
|
||||
(version,) = versions
|
||||
if version:
|
||||
manifest["ninlight_version"] = version
|
||||
with open(os.path.join(migrate_dir, "manifest.json"), "w") as fh:
|
||||
json.dump(manifest, fh, indent=2, sort_keys=True)
|
||||
fh.write("\n")
|
||||
@@ -266,7 +265,7 @@ if [ -d /tmp/ninlight-images/migrate ]; then
|
||||
# sondern mit dem, was schon da ist, zusammengefuehrt (Fixrunde 1, I1 - sonst
|
||||
# verschwinden die Eintraege aller Ziele/Varianten, die dieser Lauf nicht neu
|
||||
# gebaut hat, obwohl ihre Images unveraendert liegen bleiben).
|
||||
sudo rsync -a -c --exclude=manifest.json /tmp/ninlight-images/migrate/ "$ROOT_DIR/images/migrate/"
|
||||
sudo rsync -a -c --exclude=/manifest.json /tmp/ninlight-images/migrate/ "$ROOT_DIR/images/migrate/"
|
||||
if [ -f /tmp/ninlight-images/migrate/manifest.json ]; then
|
||||
sudo python3 - "$ROOT_DIR/images/migrate/manifest.json" /tmp/ninlight-images/migrate/manifest.json <<'MERGE'
|
||||
import json
|
||||
@@ -275,19 +274,48 @@ import sys
|
||||
manifest_path, new_manifest_path = sys.argv[1], sys.argv[2]
|
||||
with open(new_manifest_path) as fh:
|
||||
new_manifest = json.load(fh)
|
||||
if not isinstance(new_manifest, dict):
|
||||
new_manifest = {}
|
||||
try:
|
||||
with open(manifest_path) as fh:
|
||||
old_manifest = json.load(fh)
|
||||
except (OSError, ValueError):
|
||||
old_manifest = {"images": []}
|
||||
by_key = {(e.get("target"), e.get("variant")): e for e in old_manifest.get("images", [])}
|
||||
for e in new_manifest.get("images", []):
|
||||
by_key[(e.get("target"), e.get("variant"))] = e
|
||||
old_manifest = {}
|
||||
# Fixrunde 2, N1: ein BESTEHENDES manifest.json kann gueltiges JSON sein und trotzdem
|
||||
# die falsche Form haben (z.B. "[]", "null", ein String, oder ein Eintrag, der selbst
|
||||
# kein Objekt ist) - das darf niemals mitten im Deploy mit AttributeError abbrechen,
|
||||
# denn dieser Merge steht VOR der Installation von server.py/products.py/CLI/Unit und
|
||||
# vor dem Neustart: ein Abbruch hier liesse den Pi mit neuen Images, aber altem
|
||||
# Stationscode zurueck.
|
||||
if not isinstance(old_manifest, dict):
|
||||
old_manifest = {}
|
||||
old_images = old_manifest.get("images")
|
||||
if not isinstance(old_images, list):
|
||||
old_images = []
|
||||
by_key = {(e.get("target"), e.get("variant")): e for e in old_images if isinstance(e, dict)}
|
||||
new_images = new_manifest.get("images")
|
||||
if not isinstance(new_images, list):
|
||||
new_images = []
|
||||
for e in new_images:
|
||||
if isinstance(e, dict):
|
||||
by_key[(e.get("target"), e.get("variant"))] = e
|
||||
merged_images = sorted(by_key.values(), key=lambda e: (e.get("target") or "", e.get("variant") or ""))
|
||||
merged = {
|
||||
"built": new_manifest.get("built"),
|
||||
"ninlight_version": new_manifest.get("ninlight_version"),
|
||||
"images": sorted(by_key.values(), key=lambda e: (e.get("target") or "", e.get("variant") or "")),
|
||||
"images": merged_images,
|
||||
}
|
||||
# Fixrunde 2, N2: das manifestweite "ninlight_version" darf nach dem Merge nicht mehr
|
||||
# einfach vom neuen Manifest uebernommen werden - die uebernommenen Alt-Eintraege
|
||||
# koennen eine andere Version tragen (das war die alte Version dieses Bugs: ein
|
||||
# Voll-Deploy mit nur dem dev-Schluessel hat die neue Firmware-Version ueber ein
|
||||
# unveraendert liegen gebliebenes altes _prod.img geschrieben). Jeder Eintrag traegt
|
||||
# seine eigene Version; das Top-Level-Feld ist nur noch eine Bequemlichkeit fuer den
|
||||
# Fall, dass wirklich alle Eintraege uebereinstimmen.
|
||||
versions = {e.get("ninlight_version") for e in merged_images}
|
||||
if len(versions) == 1:
|
||||
(version,) = versions
|
||||
if isinstance(version, str) and version:
|
||||
merged["ninlight_version"] = version
|
||||
with open(manifest_path, "w") as fh:
|
||||
json.dump(merged, fh, indent=2, sort_keys=True)
|
||||
fh.write("\n")
|
||||
@@ -295,7 +323,11 @@ MERGE
|
||||
fi
|
||||
fi
|
||||
else
|
||||
echo " kein Schluesselverzeichnis auf dem Entwicklungsrechner - migrate/ auf dem Pi bleibt unangetastet"
|
||||
# Weder "kein Schluesselverzeichnis" noch "Verzeichnis vorhanden, aber leer" laesst
|
||||
# sich von hier aus unterscheiden - beide Faelle liefern ein leeres migrate/ von
|
||||
# diesem Rechner. Die lokale Meldung (deploy.sh, "Hinweis: ...") ist bereits genau;
|
||||
# diese hier behauptet absichtlich keinen bestimmten Grund (Fixrunde 2, Kleinigkeit).
|
||||
echo " keine Migrationsimages von diesem Rechner - migrate/ auf dem Pi bleibt unangetastet"
|
||||
fi
|
||||
sudo install -m 0755 /tmp/server.py "$ROOT_DIR/server.py"
|
||||
sudo install -m 0644 /tmp/products.py "$ROOT_DIR/products.py"
|
||||
|
||||
Reference in New Issue
Block a user