Capture the pre-migration snapshot from 0.15.x, and stop claiming counts match when none were compared
Found by running preflight against a real Stalwart 0.15.5 in a VM. Two defects, the second worse than the first. 1. AccountSnapshot could not read the version this tool migrates FROM. 0.15.5 advertises no urn:stalwart:jmap capability and POST /api returns 404 - the JMAP management API and x:Account are 0.16 features. 0.15.x exposes a REST API at GET /api/principal instead. So preflight's account-snapshot check warned and moved on, and every run against a real source instance had no "before" data at all. AccountSnapshot now dispatches on the capability the session document advertises - a positive signal, not an inference from a failed call - and internal/stalwartapi/principal.go implements the 0.15.x REST path, including its 1-based page/limit pagination so an install larger than one page isn't silently truncated. 2. With no "before" counts, the content-integrity comparison iterated an empty map, checked nothing, and reported "all message counts match". That is the strongest claim this tool makes - ARCHITECTURE 4.7 calls it the actual no-data-loss guarantee - made vacuously, and it would have passed on a migration that lost every message. The comparison now derives its account set from whatever the source could report, verifies every account and domain survived either way, and carries MessageCountsCompared so the report says plainly "MESSAGE COUNTS NOT COMPARED ... no-data-loss is NOT verified here" rather than implying otherwise. What can and cannot be checked across the 0.15/0.16 boundary, now that a real server has answered: 0.15.x has no per-mailbox message count at any endpoint, and the impersonation login 0.16 offers returns 401 there, so before/after message counts are impossible for the boundary migration this tool exists for. Both versions do report per-account used quota (usedQuota in 0.15's REST list, usedDiskQuota on 0.16's x:Account), so that is captured on both sides. It is recorded and reported, not asserted on: 4.5 notes the 0.16 migration resets quotas to zero pending recalculation, so comparing those bytes across the boundary would be a false alarm generator. Test servers across preflight, validate and stalwartapi now advertise urn:stalwart:jmap, since they stand in for 0.16 instances and that capability is what says so. Verified end to end against the smoke VM: all nine preflight checks pass, and the checkpoint records 2 accounts, 1 domain and per-account used quota where it previously recorded nothing.
This commit is contained in:
@@ -23,30 +23,54 @@ type MailboxDelta struct {
|
||||
}
|
||||
|
||||
// ContentIntegrityResult is the outcome of comparing a pre-migration
|
||||
// snapshot against a freshly captured post-migration one - the actual
|
||||
// snapshot against a freshly captured post-migration one - the
|
||||
// no-data-loss check described in ARCHITECTURE.md §4.7.
|
||||
//
|
||||
// MessageCountsCompared is the field that decides how much this result is
|
||||
// worth, and it is not always true. Stalwart 0.15.x exposes no per-mailbox
|
||||
// message counts at any endpoint, and its impersonation login (which 0.16
|
||||
// offers) returns 401 - so on the 0.15/0.16 boundary migration there are no
|
||||
// "before" counts to compare and this check can only assert that every
|
||||
// account and domain survived. Saying so is the whole point: an earlier
|
||||
// version of this comparison iterated the before-counts map, found it
|
||||
// empty, and reported "all message counts match" having checked nothing.
|
||||
type ContentIntegrityResult struct {
|
||||
AccountsChecked int
|
||||
MailboxesChecked int
|
||||
MissingAccounts []string // present before, not found after (even accounting for the email-address rewrite)
|
||||
MessageCountMismatches []MailboxDelta // present both before and after, but with a different message count
|
||||
MissingDomains []string // present before, absent after
|
||||
MessageCountsCompared bool // false when the source version could not report counts
|
||||
}
|
||||
|
||||
// OK reports whether every account and mailbox the pre-migration snapshot
|
||||
// knew about was found afterward with an identical message count.
|
||||
// OK reports whether everything this comparison was able to check matched.
|
||||
// Read it together with MessageCountsCompared: OK with that false means
|
||||
// "the directory survived", not "no mail was lost".
|
||||
func (r ContentIntegrityResult) OK() bool {
|
||||
return len(r.MissingAccounts) == 0 && len(r.MessageCountMismatches) == 0
|
||||
return len(r.MissingAccounts) == 0 && len(r.MessageCountMismatches) == 0 && len(r.MissingDomains) == 0
|
||||
}
|
||||
|
||||
func (r ContentIntegrityResult) String() string {
|
||||
if r.OK() {
|
||||
return fmt.Sprintf("content integrity: %d account(s), %d mailbox(es) checked, all message counts match", r.AccountsChecked, r.MailboxesChecked)
|
||||
}
|
||||
var b strings.Builder
|
||||
fmt.Fprintf(&b, "content integrity: %d account(s), %d mailbox(es) checked", r.AccountsChecked, r.MailboxesChecked)
|
||||
if r.MessageCountsCompared {
|
||||
fmt.Fprintf(&b, "content integrity: %d account(s), %d mailbox(es) checked", r.AccountsChecked, r.MailboxesChecked)
|
||||
} else {
|
||||
fmt.Fprintf(&b, "content integrity: %d account(s) and their domains checked; MESSAGE COUNTS NOT COMPARED "+
|
||||
"(this migration's source version reports no per-mailbox counts, so no-data-loss is NOT verified here - "+
|
||||
"only that every account and domain survived)", r.AccountsChecked)
|
||||
}
|
||||
if r.OK() {
|
||||
if r.MessageCountsCompared {
|
||||
b.WriteString(", all message counts match")
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
for _, a := range r.MissingAccounts {
|
||||
fmt.Fprintf(&b, "; MISSING ACCOUNT %s", a)
|
||||
}
|
||||
for _, d := range r.MissingDomains {
|
||||
fmt.Fprintf(&b, "; MISSING DOMAIN %s", d)
|
||||
}
|
||||
for _, d := range r.MessageCountMismatches {
|
||||
fmt.Fprintf(&b, "; MESSAGE COUNT MISMATCH %s/%s: %d before, %d after", d.Account, d.Mailbox, d.Before, d.After)
|
||||
}
|
||||
@@ -66,16 +90,54 @@ func compareContentIntegrity(ctx context.Context, client *stalwartapi.Client, be
|
||||
return nil, fmt.Errorf("capture post-migration snapshot: %w", err)
|
||||
}
|
||||
|
||||
result := &ContentIntegrityResult{}
|
||||
result := &ContentIntegrityResult{MessageCountsCompared: len(before.MailboxCounts) > 0}
|
||||
|
||||
beforeAccounts := make([]string, 0, len(before.MailboxCounts))
|
||||
// The set of accounts to verify comes from whichever pre-migration
|
||||
// facts the source version was able to report: mailbox counts when it
|
||||
// had them (0.16+), otherwise the used-quota map, which the 0.15.x REST
|
||||
// principal list does populate. Deriving it from MailboxCounts alone
|
||||
// would silently check nothing on a 0.15 source.
|
||||
beforeAccountSet := map[string]bool{}
|
||||
for a := range before.MailboxCounts {
|
||||
beforeAccountSet[a] = true
|
||||
}
|
||||
for a := range before.UsedQuota {
|
||||
beforeAccountSet[a] = true
|
||||
}
|
||||
beforeAccounts := make([]string, 0, len(beforeAccountSet))
|
||||
for a := range beforeAccountSet {
|
||||
beforeAccounts = append(beforeAccounts, a)
|
||||
}
|
||||
sort.Strings(beforeAccounts)
|
||||
|
||||
// Likewise for the "after" side: an account that exists but whose
|
||||
// mailboxes couldn't be read still counts as present.
|
||||
afterAccounts := map[string]bool{}
|
||||
for a := range after.MailboxCounts {
|
||||
afterAccounts[a] = true
|
||||
}
|
||||
for a := range after.UsedQuota {
|
||||
afterAccounts[a] = true
|
||||
}
|
||||
for a := range after.MailboxErrors {
|
||||
afterAccounts[a] = true
|
||||
}
|
||||
|
||||
for _, d := range before.Domains {
|
||||
if !containsDomain(after.Domains, d) {
|
||||
result.MissingDomains = append(result.MissingDomains, d)
|
||||
}
|
||||
}
|
||||
|
||||
for _, beforeAccount := range beforeAccounts {
|
||||
result.AccountsChecked++
|
||||
if !accountPresent(afterAccounts, beforeAccount) {
|
||||
result.MissingAccounts = append(result.MissingAccounts, beforeAccount)
|
||||
continue
|
||||
}
|
||||
if !result.MessageCountsCompared {
|
||||
continue // nothing to compare counts against; presence is all this source could give
|
||||
}
|
||||
afterMailboxes, found := after.MailboxCounts[beforeAccount]
|
||||
if !found {
|
||||
afterMailboxes, found = findByLocalPart(after.MailboxCounts, beforeAccount)
|
||||
@@ -105,6 +167,33 @@ func compareContentIntegrity(ctx context.Context, client *stalwartapi.Client, be
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// accountPresent matches an account against the post-migration set the
|
||||
// same way findByLocalPart does, so the v0.16 rewrite of bare usernames
|
||||
// into full addresses doesn't read as every account having vanished.
|
||||
func accountPresent(afterAccounts map[string]bool, beforeAccount string) bool {
|
||||
if afterAccounts[beforeAccount] {
|
||||
return true
|
||||
}
|
||||
local := strings.SplitN(beforeAccount, "@", 2)[0]
|
||||
for a := range afterAccounts {
|
||||
if strings.SplitN(a, "@", 2)[0] == local {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// containsDomain matches domains exactly; unlike account names, the
|
||||
// migration does not rewrite them.
|
||||
func containsDomain(domains []string, want string) bool {
|
||||
for _, d := range domains {
|
||||
if d == want {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func findByLocalPart(mailboxCounts map[string][]stalwartapi.MailboxCount, beforeAccount string) ([]stalwartapi.MailboxCount, bool) {
|
||||
local := strings.SplitN(beforeAccount, "@", 2)[0]
|
||||
for afterAccount, mb := range mailboxCounts {
|
||||
|
||||
@@ -29,6 +29,15 @@ func fakeManagementServer(t *testing.T, accounts []map[string]any, mailboxesByEm
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodGet && r.URL.Path == "/.well-known/jmap" {
|
||||
user, _, _ := r.BasicAuth()
|
||||
if !strings.Contains(user, "%") {
|
||||
// This instance is a migrated 0.16 one, which is what the
|
||||
// urn:stalwart:jmap capability says.
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"apiUrl": apiURL,
|
||||
"capabilities": map[string]any{"urn:ietf:params:jmap:core": map[string]any{}, "urn:stalwart:jmap": map[string]any{}},
|
||||
})
|
||||
return
|
||||
}
|
||||
target := strings.SplitN(user, "%", 2)[0]
|
||||
if _, ok := mailboxesByEmail[target]; !ok {
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
@@ -166,3 +175,70 @@ func TestCompareContentIntegrityMultipleMailboxesPerAccount(t *testing.T) {
|
||||
t.Errorf("mismatch = %+v, want Archive 199->200", m)
|
||||
}
|
||||
}
|
||||
|
||||
// The bug this guards against was found by running preflight against a real
|
||||
// Stalwart 0.15.5: it reports no per-mailbox counts, so the "before"
|
||||
// snapshot has none, and the comparison used to iterate that empty map,
|
||||
// check nothing, and report "all message counts match" - the strongest
|
||||
// claim this tool makes, made vacuously.
|
||||
func TestCompareContentIntegrityDoesNotClaimCountsMatchWhenSourceHadNone(t *testing.T) {
|
||||
srv := fakeManagementServer(t,
|
||||
[]map[string]any{{"id": "a1", "name": "[email protected]", "domainId": "smoke.test"}},
|
||||
map[string][]map[string]any{"[email protected]": {{"name": "Inbox", "totalEmails": 3}}},
|
||||
)
|
||||
defer srv.Close()
|
||||
|
||||
// A 0.15.x-shaped snapshot: accounts and used-quota, no mailbox counts.
|
||||
before := &checkpoint.PreflightSnapshot{
|
||||
AccountCount: 1,
|
||||
Domains: []string{"smoke.test"},
|
||||
UsedQuota: map[string]int64{"[email protected]": 9207},
|
||||
}
|
||||
client := &stalwartapi.Client{BaseURL: srv.URL, Username: "admin", Password: "x"}
|
||||
result, err := compareContentIntegrity(context.Background(), client, before)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.MessageCountsCompared {
|
||||
t.Error("MessageCountsCompared = true, but the source snapshot had no counts")
|
||||
}
|
||||
if result.AccountsChecked != 1 {
|
||||
t.Errorf("AccountsChecked = %d, want 1 - the account set must still be verified", result.AccountsChecked)
|
||||
}
|
||||
if strings.Contains(result.String(), "all message counts match") {
|
||||
t.Errorf("report claims counts match when none were compared:\n%s", result)
|
||||
}
|
||||
if !strings.Contains(result.String(), "MESSAGE COUNTS NOT COMPARED") {
|
||||
t.Errorf("report must say plainly that no-data-loss was not verified:\n%s", result)
|
||||
}
|
||||
}
|
||||
|
||||
// Presence checking still has to work on that path, or it would be no
|
||||
// better than the vacuous pass it replaced.
|
||||
func TestCompareContentIntegrityDetectsLostAccountWithoutCounts(t *testing.T) {
|
||||
srv := fakeManagementServer(t,
|
||||
[]map[string]any{{"id": "a1", "name": "[email protected]", "domainId": "smoke.test"}},
|
||||
map[string][]map[string]any{"[email protected]": {{"name": "Inbox", "totalEmails": 3}}},
|
||||
)
|
||||
defer srv.Close()
|
||||
|
||||
before := &checkpoint.PreflightSnapshot{
|
||||
AccountCount: 2,
|
||||
Domains: []string{"smoke.test", "gone.example"},
|
||||
UsedQuota: map[string]int64{"[email protected]": 9207, "[email protected]": 5380},
|
||||
}
|
||||
client := &stalwartapi.Client{BaseURL: srv.URL, Username: "admin", Password: "x"}
|
||||
result, err := compareContentIntegrity(context.Background(), client, before)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.OK() {
|
||||
t.Fatalf("want a failing result when an account and a domain vanished:\n%s", result)
|
||||
}
|
||||
if len(result.MissingAccounts) != 1 || result.MissingAccounts[0] != "[email protected]" {
|
||||
t.Errorf("MissingAccounts = %v, want [[email protected]]", result.MissingAccounts)
|
||||
}
|
||||
if len(result.MissingDomains) != 1 || result.MissingDomains[0] != "gone.example" {
|
||||
t.Errorf("MissingDomains = %v, want [gone.example]", result.MissingDomains)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -60,7 +60,10 @@ func runFakeStalwartServer() {
|
||||
})
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"apiUrl": "http://127.0.0.1:" + port + "/api",
|
||||
"capabilities": map[string]any{"urn:ietf:params:jmap:core": map[string]any{}, "urn:stalwart:jmap": map[string]any{}},
|
||||
})
|
||||
case r.Method == http.MethodPost && r.URL.Path == "/api":
|
||||
var body map[string]any
|
||||
json.NewDecoder(r.Body).Decode(&body)
|
||||
|
||||
Reference in New Issue
Block a user