Check the migration actually kept everything, and let report say so
internal/validate was written and tested and then never called: `run` ended at cutover, so the tool performed a migration and never confirmed it had carried the data across, and `report` was an error message pointing at the package that would have answered. `run` now compares the migrated instance against the snapshot preflight took and fails if an account or a domain that existed before is missing from it. The comparison runs against the service cutover has just started, which is the instance people will actually use - its real config, its real ports, under its real service manager - and costs no extra downtime; booting a second copy inside the maintenance window would. BootCheck stays as the equivalent for an instance the tool boots itself. The service is left running on a failure. By that point the store has been migrated in place, so stopping it undoes nothing, and only the operator can weigh the finding against their recovery point. A check that could not run is reported as skipped, never as a pass. Preflight only captures the "before" when it has an admin URL, and a run without one has to say it compared nothing rather than imply everything survived - which is the exact failure ARCHITECTURE.md §4.7 warns about. `report <run-id>` re-reads the recorded verdict rather than re-checking: run again next week and you would be asking how the instance looks now, not how it looked when it was migrated. §4.7 said validation ran after cutover while the only implementation booted its own copy, and listed a suite far larger than what exists. It now says which of the two happens, and which checks are real.
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
// SPDX-FileCopyrightText: 2026 LINUXexpert-org
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package validate
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
|
||||
"github.com/LINUXexpert-org/stalwart-migrator/internal/stalwartapi"
|
||||
)
|
||||
|
||||
// LiveOptions describes the migrated instance cutover has just started.
|
||||
type LiveOptions struct {
|
||||
AdminURL string
|
||||
AdminUser string
|
||||
AdminPassword string
|
||||
HTTPClient *http.Client
|
||||
|
||||
// Before is what preflight captured before anything was touched
|
||||
// (checkpoint.RunState.PreflightSnapshot). Nil when preflight had no
|
||||
// admin URL to capture it from, in which case there is nothing to
|
||||
// compare against and the check reports that rather than passing.
|
||||
Before *checkpoint.PreflightSnapshot
|
||||
}
|
||||
|
||||
// CheckLive compares a running instance against the pre-migration snapshot.
|
||||
//
|
||||
// The same comparison BootCheck performs against an instance it booted
|
||||
// itself, aimed instead at the service cutover has already started. That is
|
||||
// the instance people will actually use — its real config, its real ports,
|
||||
// under its real service manager — and checking it costs no extra downtime,
|
||||
// where booting a second copy inside the maintenance window would.
|
||||
func CheckLive(ctx context.Context, client *stalwartapi.Client, before *checkpoint.PreflightSnapshot) (*ContentIntegrityResult, error) {
|
||||
return compareContentIntegrity(ctx, client, before)
|
||||
}
|
||||
|
||||
// RunLive executes the post-cutover content comparison as a checkpointed
|
||||
// step, mirroring how every other phase records itself.
|
||||
//
|
||||
// A missing snapshot or admin URL is reported as skipped, never as a pass:
|
||||
// "every account survived" and "we were unable to look" are different
|
||||
// answers, and ARCHITECTURE.md §4.7 is explicit that this suite must not
|
||||
// imply a guarantee it did not measure.
|
||||
func RunLive(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState, opts LiveOptions) (Report, error) {
|
||||
var report Report
|
||||
|
||||
switch {
|
||||
case opts.AdminURL == "":
|
||||
report.Results = append(report.Results, CheckResult{
|
||||
Name: "content-integrity", Status: StatusSkip,
|
||||
Detail: "no admin URL configured - nothing could be compared against the migrated instance",
|
||||
})
|
||||
return report, nil
|
||||
case opts.Before == nil:
|
||||
report.Results = append(report.Results, CheckResult{
|
||||
Name: "content-integrity", Status: StatusSkip,
|
||||
Detail: "preflight captured no pre-migration snapshot - there is nothing to compare the migrated instance against",
|
||||
})
|
||||
return report, nil
|
||||
}
|
||||
|
||||
client := &stalwartapi.Client{
|
||||
BaseURL: opts.AdminURL, Username: opts.AdminUser, Password: opts.AdminPassword, HTTPClient: opts.HTTPClient,
|
||||
}
|
||||
|
||||
outcome, err := store.RunStep(rs, checkpoint.PhaseValidate, "content-integrity", func() (checkpoint.StepOutcome, error) {
|
||||
r, err := CheckLive(ctx, client, opts.Before)
|
||||
if err != nil {
|
||||
return checkpoint.StepOutcome{}, err
|
||||
}
|
||||
if !r.OK() {
|
||||
// Recorded as a completed step with a failing verdict rather
|
||||
// than an error: the comparison ran, and its answer is the
|
||||
// finding. An error here would read as "we could not look".
|
||||
return checkpoint.StepOutcome{Verdict: string(StatusFail), Detail: r.String()}, nil
|
||||
}
|
||||
return checkpoint.StepOutcome{Detail: r.String()}, nil
|
||||
})
|
||||
if err != nil {
|
||||
report.Results = append(report.Results, CheckResult{
|
||||
Name: "content-integrity", Status: StatusFail,
|
||||
Detail: fmt.Sprintf("could not compare the migrated instance against the pre-migration snapshot: %v", err),
|
||||
})
|
||||
return report, err
|
||||
}
|
||||
|
||||
status := StatusOK
|
||||
if outcome.Verdict == string(StatusFail) {
|
||||
status = StatusFail
|
||||
}
|
||||
report.Results = append(report.Results, CheckResult{Name: "content-integrity", Status: status, Detail: outcome.Detail})
|
||||
return report, nil
|
||||
}
|
||||
@@ -0,0 +1,167 @@
|
||||
// SPDX-FileCopyrightText: 2026 LINUXexpert-org
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package validate
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
|
||||
)
|
||||
|
||||
// A run that never captured a "before" cannot be checked against one. The
|
||||
// point of these two is that such a run reports as unchecked rather than as
|
||||
// passing: ARCHITECTURE.md §4.7 is explicit that this suite must not imply a
|
||||
// guarantee it did not measure.
|
||||
|
||||
func TestRunLiveSkipsWithoutSnapshot(t *testing.T) {
|
||||
store, rs := newRun(t)
|
||||
report, err := RunLive(context.Background(), store, rs, LiveOptions{AdminURL: "https://mail.example.org", Before: nil})
|
||||
if err != nil {
|
||||
t.Fatalf("RunLive: %v", err)
|
||||
}
|
||||
if got := report.Results[0].Status; got != StatusSkip {
|
||||
t.Fatalf("status = %q, want %q", got, StatusSkip)
|
||||
}
|
||||
if report.Blocking() {
|
||||
t.Fatal("a skipped check must not block the run")
|
||||
}
|
||||
if !strings.Contains(report.Results[0].Detail, "nothing to compare") {
|
||||
t.Fatalf("detail should say why it was skipped, got %q", report.Results[0].Detail)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunLiveSkipsWithoutAdminURL(t *testing.T) {
|
||||
store, rs := newRun(t)
|
||||
report, err := RunLive(context.Background(), store, rs, LiveOptions{Before: &checkpoint.PreflightSnapshot{}})
|
||||
if err != nil {
|
||||
t.Fatalf("RunLive: %v", err)
|
||||
}
|
||||
if got := report.Results[0].Status; got != StatusSkip {
|
||||
t.Fatalf("status = %q, want %q", got, StatusSkip)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunLivePassesWhenEverythingSurvived(t *testing.T) {
|
||||
srv := fakeInstance(t, []string{"example.org"}, map[string]float64{"[email protected]": 10, "[email protected]": 20})
|
||||
defer srv.Close()
|
||||
|
||||
store, rs := newRun(t)
|
||||
report, err := RunLive(context.Background(), store, rs, LiveOptions{
|
||||
AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "pw", HTTPClient: srv.Client(),
|
||||
Before: &checkpoint.PreflightSnapshot{
|
||||
Domains: []string{"example.org"},
|
||||
UsedQuota: map[string]int64{"[email protected]": 1, "[email protected]": 2},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("RunLive: %v", err)
|
||||
}
|
||||
if report.Blocking() {
|
||||
t.Fatalf("expected a pass, got: %s", report.String())
|
||||
}
|
||||
if got := report.Results[0].Status; got != StatusOK {
|
||||
t.Fatalf("status = %q, want %q", got, StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunLiveFailsWhenAnAccountIsMissing(t *testing.T) {
|
||||
// bob did not make it across.
|
||||
srv := fakeInstance(t, []string{"example.org"}, map[string]float64{"[email protected]": 10})
|
||||
defer srv.Close()
|
||||
|
||||
store, rs := newRun(t)
|
||||
report, err := RunLive(context.Background(), store, rs, LiveOptions{
|
||||
AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "pw", HTTPClient: srv.Client(),
|
||||
Before: &checkpoint.PreflightSnapshot{
|
||||
Domains: []string{"example.org"},
|
||||
UsedQuota: map[string]int64{"[email protected]": 1, "[email protected]": 2},
|
||||
},
|
||||
})
|
||||
// The comparison ran and found something: that is a finding, not an
|
||||
// error, so the step completes and the report carries the verdict.
|
||||
if err != nil {
|
||||
t.Fatalf("RunLive returned an error for a completed comparison: %v", err)
|
||||
}
|
||||
if !report.Blocking() {
|
||||
t.Fatalf("a missing account must block, got: %s", report.String())
|
||||
}
|
||||
if !strings.Contains(report.Results[0].Detail, "[email protected]") {
|
||||
t.Fatalf("the report should name the missing account, got %q", report.Results[0].Detail)
|
||||
}
|
||||
// And it must be recorded, so `report <run-id>` can say so afterwards.
|
||||
step := rs.Outcome(checkpoint.PhaseValidate, "content-integrity")
|
||||
if step.Verdict != string(StatusFail) {
|
||||
t.Fatalf("checkpoint verdict = %q, want %q", step.Verdict, StatusFail)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunLiveFailsWhenADomainIsMissing(t *testing.T) {
|
||||
srv := fakeInstance(t, []string{"example.org"}, map[string]float64{"[email protected]": 10})
|
||||
defer srv.Close()
|
||||
|
||||
store, rs := newRun(t)
|
||||
report, _ := RunLive(context.Background(), store, rs, LiveOptions{
|
||||
AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "pw", HTTPClient: srv.Client(),
|
||||
Before: &checkpoint.PreflightSnapshot{
|
||||
Domains: []string{"example.org", "vanished.example"},
|
||||
UsedQuota: map[string]int64{"[email protected]": 1},
|
||||
},
|
||||
})
|
||||
if !report.Blocking() {
|
||||
t.Fatalf("a missing domain must block, got: %s", report.String())
|
||||
}
|
||||
if !strings.Contains(report.Results[0].Detail, "vanished.example") {
|
||||
t.Fatalf("the report should name the missing domain, got %q", report.Results[0].Detail)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunLiveFailsWhenTheInstanceCannotBeRead(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
store, rs := newRun(t)
|
||||
report, err := RunLive(context.Background(), store, rs, LiveOptions{
|
||||
AdminURL: srv.URL, AdminUser: "admin", AdminPassword: "pw", HTTPClient: srv.Client(),
|
||||
Before: &checkpoint.PreflightSnapshot{Domains: []string{"example.org"}},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected an error when the instance cannot be read")
|
||||
}
|
||||
if !report.Blocking() {
|
||||
t.Fatalf("being unable to look must not read as a pass, got: %s", report.String())
|
||||
}
|
||||
}
|
||||
|
||||
func newRun(t *testing.T) (*checkpoint.Store, *checkpoint.RunState) {
|
||||
t.Helper()
|
||||
store := checkpoint.NewStore(t.TempDir())
|
||||
rs, err := store.Create("0.15.5", "0.16.14")
|
||||
if err != nil {
|
||||
t.Fatalf("create run: %v", err)
|
||||
}
|
||||
return store, rs
|
||||
}
|
||||
|
||||
// fakeInstance answers the principal listing the snapshot is built from.
|
||||
func fakeInstance(t *testing.T, domains []string, accounts map[string]float64) *httptest.Server {
|
||||
t.Helper()
|
||||
items := make([]map[string]any, 0, len(domains)+len(accounts))
|
||||
for _, d := range domains {
|
||||
items = append(items, map[string]any{"type": "domain", "name": d})
|
||||
}
|
||||
for name, quota := range accounts {
|
||||
items = append(items, map[string]any{"type": "individual", "name": name, "usedQuota": quota})
|
||||
}
|
||||
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("content-type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"items": items, "total": len(items)}})
|
||||
}))
|
||||
}
|
||||
@@ -13,6 +13,11 @@ type Status string
|
||||
const (
|
||||
StatusOK Status = "ok"
|
||||
StatusFail Status = "fail"
|
||||
// StatusSkip is a check that could not be performed. It is deliberately
|
||||
// not StatusOK: "every account survived" and "we were unable to look"
|
||||
// are different answers, and reporting the second as the first is the
|
||||
// failure mode ARCHITECTURE.md §4.7 warns about.
|
||||
StatusSkip Status = "skip"
|
||||
)
|
||||
|
||||
type CheckResult struct {
|
||||
|
||||
Reference in New Issue
Block a user