Stop a clean migration reporting domains it never lost

The post-migration comparison had the two versions counting domains
differently, and yesterday's wiring turned that into a gate: `run` would have
failed a migration that lost nothing.

The 0.15 side added every domain appearing in any account's address on top of
the domain principals - the fallback's own comment says "if the instance has
no explicit domain principals", but the loop ran unconditionally. The 0.16
side did the reverse, listing only domains some account calls its primary,
discarding the full Domain list it had already fetched. An instance with
three declared domains and accounts aliased across nine reported nine before
and three after. INBUXA is exactly that shape, and this was the account/domain
over-count noted as undiagnosed.

Both sides now mean "the domains this server holds". A domain that still goes
missing is reported as a warning rather than failing the run: what the two
versions call a domain differs across this boundary in ways we have now been
caught by once, and a missing account - which is compared with a local-part
fallback and is what actually matters - still fails.

Narrowing OK() also made String() return before printing the domain lines,
so the new warning would have been silent. Caught by its own test.
This commit is contained in:
2026-08-24 13:26:15 -07:00
parent 28128633ef
commit 3adaee3bc6
9 changed files with 156 additions and 24 deletions
+10 -2
View File
@@ -185,14 +185,22 @@ func (c *Client) accountSnapshotJMAP(ctx context.Context) (*Snapshot, error) {
if err != nil {
return nil, fmt.Errorf("stalwartapi: resolve domain names (an unresolved id would make every domain look missing): %w", err)
}
// Every domain the server holds, not only those some account calls its
// primary. Deriving the list from accounts dropped alias domains and
// domains with no accounts, so comparing it against a "before" list
// that did include them reported losses that had not happened.
domainSet := map[string]bool{}
for _, name := range domainNames {
if name != "" {
domainSet[name] = true
}
}
for _, a := range accounts {
if a.DomainID == "" {
continue
}
if name, ok := domainNames[a.DomainID]; ok && name != "" {
domainSet[name] = true
continue
continue // already counted above
}
// Keep the raw id rather than dropping the domain entirely: a
// domain that can't be named is still a domain that exists.
+15 -6
View File
@@ -180,12 +180,21 @@ func (c *Client) principalSnapshotREST(ctx context.Context) (*Snapshot, error) {
domainSet[d.Name] = true
}
}
// Fall back to the domains implied by account addresses if the
// instance has no explicit domain principals.
for _, p := range individuals {
for _, email := range p.Emails {
if at := strings.LastIndex(email, "@"); at >= 0 && at+1 < len(email) {
domainSet[email[at+1:]] = true
// Fall back to the domains implied by account addresses only when the
// instance declares no domain principals at all.
//
// This loop used to run unconditionally, which quietly inflated the
// list with every alias domain. That matters because this snapshot is
// the "before" side of the post-migration comparison, and the 0.16 side
// reports the domains the server actually holds: an alias domain that
// was never its own principal would be present here, absent there, and
// reported as lost by a migration that lost nothing.
if len(domainSet) == 0 {
for _, p := range individuals {
for _, email := range p.Emails {
if at := strings.LastIndex(email, "@"); at >= 0 && at+1 < len(email) {
domainSet[email[at+1:]] = true
}
}
}
}
+52
View File
@@ -356,3 +356,55 @@ func TestTenantNamesReportsTenantPrincipals(t *testing.T) {
t.Error("TenantNames returned nil without an error; want a (possibly empty) list")
}
}
// The "before" side of the post-migration comparison. It used to add every
// domain appearing in any account's address on top of the domain principals,
// so an instance with three declared domains and accounts aliased across nine
// reported nine - and the 0.16 side, which lists the domains the server
// actually holds, then looked as though six had been lost by a migration that
// lost nothing. INBUXA is exactly that shape.
func TestRESTSnapshotDoesNotInflateDomainsWithAliases(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
typ := r.URL.Query().Get("types")
w.Header().Set("content-type", "application/json")
switch typ {
case "domain":
_, _ = w.Write([]byte(`{"data":{"items":[{"type":"domain","name":"example.org"}],"total":1}}`))
default:
_, _ = w.Write([]byte(`{"data":{"items":[{"type":"individual","name":"[email protected]",` +
`"emails":["[email protected]","[email protected]","[email protected]"],"usedQuota":1}],"total":1}}`))
}
}))
defer srv.Close()
snap, err := (&Client{BaseURL: srv.URL, Username: "admin", Password: "pw", HTTPClient: srv.Client()}).principalSnapshotREST(context.Background())
if err != nil {
t.Fatalf("principalSnapshotREST: %v", err)
}
if len(snap.Domains) != 1 || snap.Domains[0] != "example.org" {
t.Fatalf("Domains = %v, want just the declared domain principal", snap.Domains)
}
}
// The fallback still exists for an instance that declares none, where an
// address is the only evidence a domain is in use.
func TestRESTSnapshotFallsBackToAddressesWhenNoDomainPrincipals(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("content-type", "application/json")
if r.URL.Query().Get("types") == "domain" {
_, _ = w.Write([]byte(`{"data":{"items":[],"total":0}}`))
return
}
_, _ = w.Write([]byte(`{"data":{"items":[{"type":"individual","name":"[email protected]",` +
`"emails":["[email protected]"],"usedQuota":1}],"total":1}}`))
}))
defer srv.Close()
snap, err := (&Client{BaseURL: srv.URL, Username: "admin", Password: "pw", HTTPClient: srv.Client()}).principalSnapshotREST(context.Background())
if err != nil {
t.Fatalf("principalSnapshotREST: %v", err)
}
if len(snap.Domains) != 1 || snap.Domains[0] != "example.org" {
t.Fatalf("Domains = %v, want the domain implied by the address", snap.Domains)
}
}