Fix-Welle: Schlussreview-Befunde 1-6 zur Release-Decke
Sechs Befunde aus dem Schlussreview, in einer Welle behoben: - BEFUND 1 (Important): eine leere Auswahl im Festnageln-Feld liess pinRelease() ueber `$this->ceilingChoice ?: null` in setCeiling(null) laufen — die Gegenhandlung (Decke abnehmen) — und meldete dabei die Erfolgsmeldung des Festnagelns. ConfirmPinRelease::confirm() schickt die Version jetzt als Event-Nutzlast (wie ConfirmSaveSecret den Schluessel), und pinRelease(string $version) weist eine leere Version ausdruecklich ab, mit einer eigenen Meldung (release_pin_empty). - BEFUND 2 (Minor, durch 1 miterledigt): Modal und Seite lasen bisher zwei getrennte Eigenschaften ($version vs. $ceilingChoice). Der Fix oben beseitigt die Trennung. - BEFUND 3 (Important, Text only): der Kommentar bei release_tag_exists() in deploy/lib/release.sh und der Fehlerbehandlungs-Abschnitt der Spec behaupteten, ceiling_missing schuetze gegen einen vom Release-Prozess geloeschten Tag. Tut es nicht: `git fetch --tags --force` (ohne --prune-tags, bewusst) entfernt keine lokal bereits geholten Tags, die drueben verschwunden sind. Beide Stellen beschreiben jetzt, wogegen die Pruefung tatsaechlich schuetzt (ein nie geholter oder nie existierender Tag) und wogegen nicht. Kein --prune-tags hinzugefuegt. - BEFUND 4 (Minor): ConfirmPinRelease hatte keinen Test. Zwei neue Tests nach dem Vorbild von ConfirmSaveSecret in IntegrationsPageTest. - BEFUND 5 (Minor): ceilingChoice wurde nie aus dem gesetzten Zustand vorbelegt. UpdateChannel::ceiling() ist jetzt public, Settings::mount() belegt das Feld damit vor. - BEFUND 6 (Minor): eine von Hand geleerte Deckendatei liest die Konsole als "keine Decke" (ceiling() -> null), der Agent meldet dafuer aber ceiling_error. Der "Decke abnehmen"-Knopf stand nur hinter @if($update['ceiling']) und verschwand damit genau in dem Zustand, aus dem er zurueckfuehren muesste. Bedingung erweitert auf ($update['ceiling'] || $update['ceiling_error']). Jeder Befund traegt einen eigenen Test in ReleaseCeilingConsoleTest.php. Volle Suite: 2973 passed (10389 assertions). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feat/versandtakt
nexxo
parent
073e147ff8
commit
24eb7b3d80
|
|
@ -33,7 +33,14 @@ class ConfirmPinRelease extends ModalComponent
|
|||
{
|
||||
$this->authorize('site.manage');
|
||||
|
||||
$this->dispatch('pin-release-confirmed');
|
||||
// Die Version geht als Event-Nutzlast mit, nicht nur als das, was
|
||||
// dieses Modal auf dem Bildschirm zeigt — genau wie
|
||||
// ConfirmSaveSecret::confirm() den Schlüssel mitschickt. Sonst
|
||||
// bestätigt der Klick nur, DASS etwas festgenagelt wird, nicht WAS:
|
||||
// die Seite las bisher ihr eigenes `ceilingChoice` erneut, und eine
|
||||
// leere Auswahl dort landete unbestätigt in setCeiling(null) — der
|
||||
// Gegenhandlung, nicht dem Festnageln.
|
||||
$this->dispatch('pin-release-confirmed', version: $this->version);
|
||||
$this->closeModal();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -167,6 +167,12 @@ class Settings extends Component
|
|||
$this->autoDays = $window->days();
|
||||
$this->autoFrom = $window->from();
|
||||
$this->autoTo = $window->to();
|
||||
|
||||
// Das Auswahlfeld zeigt, was tatsächlich gilt, statt auf "Neueste
|
||||
// Version (nicht festgenagelt)" zu stehen, während die Zeile darunter
|
||||
// "Festgenagelt auf 1.8.0" sagt — zwei Bedienelemente derselben
|
||||
// Karte, die sich sonst widersprechen.
|
||||
$this->ceilingChoice = app(UpdateChannel::class)->ceiling() ?? '';
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -730,9 +736,21 @@ class Settings extends Component
|
|||
* `site.manage` wie `requestUpdate()`: festnageln entscheidet, welche
|
||||
* Fassung dieser Server je bekommt, und ist damit dieselbe Entscheidung
|
||||
* wie sie auszulösen — nur früher.
|
||||
*
|
||||
* Die Version kommt als Parameter aus dem Event, nicht mehr aus
|
||||
* `$this->ceilingChoice`: ConfirmPinRelease schickt mit, was im Modal
|
||||
* tatsächlich bestätigt wurde (genau wie ConfirmSaveSecret den
|
||||
* Schlüssel mitschickt). Eine leere Version wird HIER ausdrücklich
|
||||
* abgewiesen, mit ihrer eigenen Meldung — nicht länger `?: null` in
|
||||
* setCeiling() gereicht, das eine leere Auswahl als "Decke abnehmen"
|
||||
* gelesen und dabei die Festnageln-Erfolgsmeldung gezeigt hätte. Das
|
||||
* Auswahlfeld trägt `<option value="">…nicht festgenagelt</option>`,
|
||||
* und das Einzige, was einen Klick darauf bisher verhinderte, war ein
|
||||
* Alpine-`disabled` — kein Ersatz für eine serverseitige Prüfung, siehe
|
||||
* `requestUpdate()` oben.
|
||||
*/
|
||||
#[On('pin-release-confirmed')]
|
||||
public function pinRelease(): void
|
||||
public function pinRelease(string $version): void
|
||||
{
|
||||
$this->authorize('site.manage');
|
||||
|
||||
|
|
@ -740,7 +758,13 @@ class Settings extends Component
|
|||
return;
|
||||
}
|
||||
|
||||
$accepted = app(UpdateChannel::class)->setCeiling($operator->email, $this->ceilingChoice ?: null);
|
||||
if ($version === '') {
|
||||
$this->dispatch('notify', message: __('admin_settings.release_pin_empty'));
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
$accepted = app(UpdateChannel::class)->setCeiling($operator->email, $version);
|
||||
|
||||
$this->dispatch('notify', message: __($accepted
|
||||
? 'admin_settings.release_pinned'
|
||||
|
|
|
|||
|
|
@ -642,8 +642,14 @@ final class UpdateChannel
|
|||
* ab, während dieser noch liest), wirft `File::get()` eine
|
||||
* FileNotFoundException. Dasselbe Muster wie readJson() und lastLog()
|
||||
* weiter unten: state() darf unter keinen Umständen werfen.
|
||||
*
|
||||
* Öffentlich, weil auch App\Livewire\Admin\Settings::mount() sie
|
||||
* braucht — um `ceilingChoice` mit der tatsächlich gesetzten Decke
|
||||
* vorzubelegen, statt das Auswahlfeld auf einem festgenagelten Server
|
||||
* "Neueste Version (nicht festgenagelt)" zeigen zu lassen, während der
|
||||
* Satz darunter das Gegenteil sagt.
|
||||
*/
|
||||
private function ceiling(): ?string
|
||||
public function ceiling(): ?string
|
||||
{
|
||||
try {
|
||||
$path = storage_path('app/'.self::CEILING);
|
||||
|
|
|
|||
|
|
@ -118,9 +118,21 @@ release_tags_ahead() {
|
|||
# release_tag_exists TAG — true when TAG is a real tag in this checkout.
|
||||
#
|
||||
# Form und Existenz sind zwei Fragen. Eine Decke, die bloss AUSSIEHT wie eine
|
||||
# Version, ist keine: der Release-Prozess loescht einen falschen Tag und
|
||||
# ueberspringt die Nummer, eine Decke kann also ohne Zutun dessen, der sie
|
||||
# gesetzt hat, ins Leere zeigen.
|
||||
# Version, ist keine — ein Tag, den jemand von Hand in die Deckendatei
|
||||
# geschrieben hat und der nie existiert hat, oder einer, den DIESER Wirt
|
||||
# schlicht noch nie geholt hat.
|
||||
#
|
||||
# Was das NICHT abdeckt: einen Tag, der hier schon lag und auf der
|
||||
# Gegenstelle geloescht wurde. Der Agent holt mit
|
||||
# `git fetch --quiet --tags --force origin` (update-agent.sh), und das
|
||||
# ENTFERNT keine lokalen Tags, die drueben verschwunden sind — dafuer
|
||||
# braeuchte es `--prune --prune-tags`. Das ist hier absichtlich NICHT
|
||||
# gesetzt: es waere eine Entscheidung darueber, was jede der zehn
|
||||
# Server-Konsolen mit ihren eigenen lokalen Tags tut, sobald irgendwo einer
|
||||
# zurueckgezogen wird — und die steht dem Besitzer zu, nicht diesem Helfer.
|
||||
# `git rev-parse --verify` beantwortet also auch dann noch mit "ja", wenn
|
||||
# der Tag auf der Gegenstelle laengst weg ist, solange dieser Wirt ihn
|
||||
# irgendwann einmal geholt hatte.
|
||||
release_tag_exists() {
|
||||
[[ -n "${1-}" ]] || return 1
|
||||
git rev-parse -q --verify "refs/tags/${1}^{commit}" >/dev/null 2>&1
|
||||
|
|
|
|||
|
|
@ -154,9 +154,21 @@ weggenagelt hat. Stattdessen: `behind = 0`, nichts wird angeboten, und
|
|||
|
||||
Eine Sicherung, die im Zweifel öffnet, ist keine.
|
||||
|
||||
Dass ein Tag verschwindet, ist hier kein Randfall: der Release-Prozess sieht
|
||||
ausdrücklich vor, dass ein **falscher Tag gelöscht und die Nummer übersprungen**
|
||||
wird.
|
||||
`ceiling_missing` ist **nicht** die Sicherung gegen ein zurückgezogenes
|
||||
Release. Der Agent holt Tags mit `git fetch --quiet --tags --force origin`
|
||||
(deploy/update-agent.sh), und das entfernt **keine** lokalen Tags, die auf
|
||||
der Gegenstelle verschwunden sind — dafür bräuchte es
|
||||
`--prune --prune-tags`, eine Änderung, die hier bewusst nicht gemacht ist:
|
||||
sie würde festlegen, was jede der zehn Server-Konsolen mit ihren eigenen
|
||||
lokalen Tags tut, sobald irgendwo ein Release zurückgezogen wird, und das ist
|
||||
eine Entscheidung des Besitzers, keine dieser Spezifikation. Ein Tag, den ein
|
||||
Wirt einmal geholt hat, bleibt für `release_tag_exists` (deploy/lib/release.sh)
|
||||
also „vorhanden", auch nachdem er drüben gelöscht wurde.
|
||||
|
||||
Sinnvoll bleibt `ceiling_missing` trotzdem — nur gegen einen anderen Fall:
|
||||
eine Decke, die auf einen Tag zeigt, den DIESER Wirt nie geholt hat (ein
|
||||
Tippfehler, eine von Hand in die Datei geschriebene Version) oder der nie
|
||||
existiert hat.
|
||||
|
||||
Weiter:
|
||||
|
||||
|
|
@ -175,7 +187,7 @@ Weiter:
|
|||
|---|---|
|
||||
| `ReleaseComparisonTest` (besteht) | `release_newest_tag`/`release_tags_ahead` mit Decke — die echten Bash-Helfer |
|
||||
| neu, Agent | Echtes Skript gegen ein Wegwerf-Repo mit Tags: Decke gesetzt → `behind` zählt nur bis dorthin, `target_release` **ist** die Decke |
|
||||
| neu, fällt zu | Decke zeigt auf gelöschten Tag → `behind = 0` **und** `ceiling_error`, nicht „neueste" |
|
||||
| neu, fällt zu | Decke zeigt auf einen Tag, den dieser Wirt nie geholt hat → `behind = 0` **und** `ceiling_error`, nicht „neueste" |
|
||||
| neu, Automatik | `clupilot:auto-update` mit gesetzter Decke tut nichts |
|
||||
| neu, Konsole | Festnageln schreibt die Datei, „Decke abnehmen" löscht sie, ohne `site.manage` geht beides nicht |
|
||||
|
||||
|
|
|
|||
|
|
@ -165,6 +165,10 @@ return [
|
|||
'release_pin_confirm' => 'Festnageln',
|
||||
'release_pinned' => 'Festgenagelt.',
|
||||
'release_pin_invalid' => 'Das ist keine gültige Version.',
|
||||
// Eigene Meldung für eine leere Bestätigung — nicht die Erfolgsmeldung
|
||||
// von oben. Eine leere Auswahl darf niemals als "Decke abnehmen" gelten,
|
||||
// nur weil setCeiling(null) genau das bedeutet.
|
||||
'release_pin_empty' => 'Keine Version ausgewählt — nichts wurde geändert.',
|
||||
'release_pinned_at' => 'Festgenagelt auf :version.',
|
||||
'release_pinned_passed' => 'Festgenagelt auf :version — dieser Server ist bereits weiter. Die Decke hält nichts mehr zurück; zurück geht es nicht.',
|
||||
'release_unpin_action' => 'Decke abnehmen',
|
||||
|
|
|
|||
|
|
@ -162,6 +162,10 @@ return [
|
|||
'release_pin_confirm' => 'Pin',
|
||||
'release_pinned' => 'Pinned.',
|
||||
'release_pin_invalid' => 'That is not a valid release.',
|
||||
// Its own message for an empty confirmation — not the success message
|
||||
// above. An empty selection must never count as "remove the ceiling"
|
||||
// just because setCeiling(null) means exactly that.
|
||||
'release_pin_empty' => 'No release selected — nothing was changed.',
|
||||
'release_pinned_at' => 'Pinned to :version.',
|
||||
'release_pinned_passed' => 'Pinned to :version — this server is already past it. The ceiling no longer holds anything back; there is no way back.',
|
||||
'release_unpin_action' => 'Remove ceiling',
|
||||
|
|
|
|||
|
|
@ -207,12 +207,24 @@
|
|||
nicht dieselbe Zeile mit anderer Zahl: wer v1.8.0 festgenagelt hat
|
||||
und v1.8.1 laufen sieht, muss erfahren, dass die Decke nichts mehr
|
||||
tut — sonst liest er sie als Zusicherung, die sie nicht ist. Zurück
|
||||
geht es nicht (update.sh:222). --}}
|
||||
@if ($update['ceiling'])
|
||||
geht es nicht (update.sh:222).
|
||||
|
||||
Der Knopf steht auch OHNE `$update['ceiling']`, sobald
|
||||
`ceiling_error` etwas meldet: eine von Hand geleerte
|
||||
Deckendatei liest die Konsole (UpdateChannel::ceiling())
|
||||
als "keine Decke" — leerer Inhalt wird zu null —, während
|
||||
der Agent dieselbe leere Datei als `ceiling_invalid`
|
||||
zurückmeldet. Ohne diese Bedingung verschwand der einzige
|
||||
Ort mit dem "Decke abnehmen"-Knopf genau in dem Zustand,
|
||||
den nur eine Hand am Wirt erzeugen kann und aus dem die
|
||||
Konsole ohne Kommandozeile zurückführen muss. --}}
|
||||
@if ($update['ceiling'] || $update['ceiling_error'])
|
||||
<p class="mt-3 text-xs text-muted">
|
||||
{{ $update['ceiling_passed']
|
||||
? __('admin_settings.release_pinned_passed', ['version' => ltrim($update['ceiling'], 'v')])
|
||||
: __('admin_settings.release_pinned_at', ['version' => ltrim($update['ceiling'], 'v')]) }}
|
||||
@if ($update['ceiling'])
|
||||
{{ $update['ceiling_passed']
|
||||
? __('admin_settings.release_pinned_passed', ['version' => ltrim($update['ceiling'], 'v')])
|
||||
: __('admin_settings.release_pinned_at', ['version' => ltrim($update['ceiling'], 'v')]) }}
|
||||
@endif
|
||||
<button type="button" wire:click="unpinRelease" class="underline">{{ __('admin_settings.release_unpin_action') }}</button>
|
||||
</p>
|
||||
@endif
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
<?php
|
||||
|
||||
use App\Livewire\Admin\ConfirmPinRelease;
|
||||
use App\Livewire\Admin\Settings;
|
||||
use App\Models\Operator;
|
||||
use App\Services\Deployment\UpdateChannel;
|
||||
|
|
@ -208,7 +209,7 @@ it('pins from the console', function () {
|
|||
Livewire::actingAs($owner, 'operator')
|
||||
->test(Settings::class)
|
||||
->set('ceilingChoice', 'v1.8.0')
|
||||
->call('pinRelease');
|
||||
->call('pinRelease', 'v1.8.0');
|
||||
|
||||
expect(trim(File::get(storage_path('app/deploy/release-ceiling'))))->toBe('v1.8.0');
|
||||
});
|
||||
|
|
@ -238,6 +239,101 @@ it('refuses to pin without site.manage', function () {
|
|||
Livewire::actingAs($staff, 'operator')
|
||||
->test(Settings::class)
|
||||
->set('ceilingChoice', 'v1.8.0')
|
||||
->call('pinRelease')
|
||||
->call('pinRelease', 'v1.8.0')
|
||||
->assertForbidden();
|
||||
});
|
||||
|
||||
it('does not let an empty confirmation take the ceiling off while reporting success', function () {
|
||||
// BEFUND 1: `setCeiling($operator->email, $this->ceilingChoice ?: null)`
|
||||
// liess eine leere Auswahl als "Decke abnehmen" durchgehen — und meldete
|
||||
// dabei `release_pinned`, die Erfolgsmeldung der GEGENHANDLUNG. Erreichbar
|
||||
// war das, weil das Auswahlfeld `<option value="">` traegt und einzig ein
|
||||
// Alpine-`x-bind:disabled` den Klick verhinderte — kein Ersatz fuer eine
|
||||
// serverseitige Pruefung, siehe requestUpdate() im selben Bauteil.
|
||||
//
|
||||
// Die Decke steht bereits auf v1.8.0. Eine leere Bestaetigung darf sie
|
||||
// weder loeschen noch die Festnageln-Erfolgsmeldung zeigen.
|
||||
$owner = Operator::factory()->role('Owner')->create();
|
||||
File::put(storage_path('app/deploy/release-ceiling'), 'v1.8.0');
|
||||
|
||||
Livewire::actingAs($owner, 'operator')
|
||||
->test(Settings::class)
|
||||
->call('pinRelease', '')
|
||||
->assertDispatched('notify', message: __('admin_settings.release_pin_empty'));
|
||||
|
||||
expect(trim(File::get(storage_path('app/deploy/release-ceiling'))))->toBe('v1.8.0');
|
||||
});
|
||||
|
||||
it('prefills the select with the ceiling that is actually set', function () {
|
||||
// BEFUND 5: `ceilingChoice` wurde nie aus dem gesetzten Zustand
|
||||
// vorbelegt. Auf einem festgenagelten Server las das Feld "Neueste
|
||||
// Version (nicht festgenagelt)", waehrend die Zeile darunter
|
||||
// "Festgenagelt auf 1.8.0" sagte — zwei Bedienelemente derselben Karte,
|
||||
// die sich widersprachen.
|
||||
File::put(storage_path('app/deploy/release-ceiling'), 'v1.8.0');
|
||||
$owner = Operator::factory()->role('Owner')->create();
|
||||
|
||||
Livewire::actingAs($owner, 'operator')
|
||||
->test(Settings::class)
|
||||
->assertSet('ceilingChoice', 'v1.8.0');
|
||||
});
|
||||
|
||||
it('leaves the select empty when nothing is pinned', function () {
|
||||
$owner = Operator::factory()->role('Owner')->create();
|
||||
|
||||
Livewire::actingAs($owner, 'operator')
|
||||
->test(Settings::class)
|
||||
->assertSet('ceilingChoice', '');
|
||||
});
|
||||
|
||||
it('keeps the "remove ceiling" button reachable when the file reads back empty', function () {
|
||||
// BEFUND 6: Bash wertet eine leere Deckendatei als `ceiling_invalid`;
|
||||
// PHP (UpdateChannel::ceiling()) liefert dafuer `null` — leerer Inhalt
|
||||
// wird als "keine Decke" gelesen. Die Konsole zeigte dann zwar den
|
||||
// Warnkasten, aber der `@if ($update['ceiling'])`-Block — der einzige
|
||||
// Ort mit dem Knopf "Decke abnehmen" — wurde uebersprungen. Nur eine
|
||||
// Hand am Wirt kann diesen Zustand erzeugen (eine leere Datei), aber die
|
||||
// Konsole muss trotzdem einen Weg zurueck bieten, ohne Kommandozeile.
|
||||
File::put(storage_path('app/deploy/release-ceiling'), '');
|
||||
File::put(storage_path('app/deploy/update-status.json'), json_encode([
|
||||
'state' => 'idle',
|
||||
'ceiling_error' => 'ceiling_invalid',
|
||||
'releases' => ['v1.8.1', 'v1.8.0'],
|
||||
]));
|
||||
$owner = Operator::factory()->role('Owner')->create();
|
||||
|
||||
// Die Konsole selbst liest die leere Datei als "keine Decke" — der Bug
|
||||
// sass genau in diesem Widerspruch zwischen den beiden Seiten.
|
||||
expect(app(UpdateChannel::class)->state()['ceiling'])->toBeNull();
|
||||
|
||||
Livewire::actingAs($owner, 'operator')
|
||||
->test(Settings::class)
|
||||
->assertSeeHtml('wire:click="unpinRelease"')
|
||||
->call('unpinRelease');
|
||||
|
||||
expect(File::exists(storage_path('app/deploy/release-ceiling')))->toBeFalse();
|
||||
});
|
||||
|
||||
it('does not open the pin confirmation without the capability', function () {
|
||||
// Nach dem Vorbild von IntegrationsPageTest::"does not open the
|
||||
// save/forget confirmation without the capability" fuer ConfirmSaveSecret
|
||||
// — ConfirmPinRelease trug dieselbe authorize('site.manage') in mount(),
|
||||
// ohne dass irgendein Test sie je geprueft hat (BEFUND 4).
|
||||
$staff = Operator::factory()->create();
|
||||
|
||||
Livewire::actingAs($staff, 'operator')
|
||||
->test(ConfirmPinRelease::class, ['version' => 'v1.8.0'])
|
||||
->assertForbidden();
|
||||
});
|
||||
|
||||
it('confirms a pin through the modal without writing anything itself', function () {
|
||||
$owner = Operator::factory()->role('Owner')->create();
|
||||
|
||||
Livewire::actingAs($owner, 'operator')
|
||||
->test(ConfirmPinRelease::class, ['version' => 'v1.8.0'])
|
||||
->assertSee('1.8.0')
|
||||
->call('confirm')
|
||||
->assertDispatched('pin-release-confirmed', version: 'v1.8.0');
|
||||
|
||||
expect(File::exists(storage_path('app/deploy/release-ceiling')))->toBeFalse();
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue