Do not call it data loss when the instance would not show us the accounts

The dress rehearsal reported MISSING ACCOUNT [email protected] after a
migration that lost nothing: SMTP on the migrated instance accepts mail for
it, while an address that does not exist is refused, so the account is there.

Enumeration is permission-scoped. The "before" snapshot was read by an
account with full rights; the "after" snapshot by one whose admin role the
migration had not carried across - proven one step earlier, where the quota
rebuild was refused as unauthorised. It was shown a fraction of the accounts
and the comparison called the remainder lost.

"Missing" and "not permitted to see" are different findings and only one of
them is about the data. When the migrated instance shows fewer accounts than
existed, the result now says COULD NOT VERIFY and names the reader, rather
than asserting a loss the evidence does not support. It still fails the run:
an unverified migration is not a verified one, and the fix is to re-check
with an admin account, which --resume makes cheap.

Counting accounts is not a strong enough signal to tell the two cases apart,
so it does not try to - it reports the ambiguity instead of guessing.

Also fixes the test fake, which ignored ?types= and returned domains as
individuals, making every account count in these tests wrong.
This commit is contained in:
2026-08-24 16:02:31 -07:00
parent ce0e237051
commit 019696bce7
3 changed files with 103 additions and 2 deletions
+70 -2
View File
@@ -190,8 +190,76 @@ func fakeInstance(t *testing.T, domains []string, accounts map[string]float64) *
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) {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Honour ?types= the way the real REST API does: asking for
// individuals must not hand back domains, or every count is wrong.
want := r.URL.Query().Get("types")
out := items
if want != "" {
out = nil
for _, it := range items {
if it["type"] == want {
out = append(out, it)
}
}
}
w.Header().Set("content-type", "application/json")
_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"items": items, "total": len(items)}})
_ = json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"items": out, "total": len(out)}})
}))
}
// Enumeration is permission-scoped. An account that authenticates but holds
// no management role is shown only what it may see, and a v0.16 migration
// does not always carry an admin role across - so the account that read the
// "before" side may have less reach afterwards. Observed on a clone of
// production: every account survived, SMTP confirmed it, and the comparison
// still called one lost because the reader could no longer see it.
func TestRunLiveWillNotCallItLossWhenItCouldNotLook(t *testing.T) {
// The migrated instance shows only the reader's own account.
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: "[email protected]", AdminPassword: "pw", HTTPClient: srv.Client(),
Before: &checkpoint.PreflightSnapshot{
Domains: []string{"example.org"},
UsedQuota: map[string]int64{"[email protected]": 1, "[email protected]": 2, "[email protected]": 3},
},
})
if err != nil {
t.Fatalf("RunLive: %v", err)
}
// It still fails: an unverified migration is not a verified one. What it
// must not do is call it data loss, which is a different claim.
if !report.Blocking() {
t.Fatal("an unverifiable result must still stop the run")
}
detail := report.Results[0].Detail
for _, want := range []string{"COULD NOT VERIFY", "not the same as data loss", "permission-scoped", "1 of 3"} {
if !strings.Contains(detail, want) {
t.Fatalf("detail should contain %q, got %q", want, detail)
}
}
}
// The other side of it: when the instance shows everything and an account is
// genuinely absent, that is still a failure.
func TestRunLiveStillFailsWhenTheInstanceShowedEverything(t *testing.T) {
srv := fakeInstance(t, []string{"example.org"}, map[string]float64{
"[email protected]": 10, "[email protected]": 20, "[email protected]": 30,
})
defer srv.Close()
store, rs := newRun(t)
report, _ := RunLive(context.Background(), store, rs, LiveOptions{
AdminURL: srv.URL, AdminUser: "[email protected]", AdminPassword: "pw", HTTPClient: srv.Client(),
Before: &checkpoint.PreflightSnapshot{
Domains: []string{"example.org"},
UsedQuota: map[string]int64{"[email protected]": 1, "[email protected]": 2},
},
})
if !report.Blocking() {
t.Fatalf("a genuine loss must still block, got: %s", report.String())
}
}