From f692725bed7b4d9a3321d70f3c8f073610aaeb73 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 27 Sep 2026 12:46:49 +0800 Subject: [PATCH] =?UTF-8?q?fix(migrate):=20=E5=85=91=E6=8D=A2=E8=BF=81?= =?UTF-8?q?=E7=A7=BB=E7=A0=81=E5=9C=A8=E5=8F=8C=E6=96=B9=E8=AE=A4=E9=A2=86?= =?UTF-8?q?=E9=94=81=E4=B8=8B=E6=A3=80=E6=9F=A5=E7=9B=AE=E6=A0=87=E9=85=8D?= =?UTF-8?q?=E9=A2=9D=EF=BC=8C=E8=B6=85=E5=87=BA=E8=BF=94=E5=9B=9E=20403=20?= =?UTF-8?q?migrate=5Fquota=5Fexceeded=20=E4=B8=94=E7=A0=81=E4=B8=8D?= =?UTF-8?q?=E6=B6=88=E8=80=97?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/openapi.yaml | 8 + internal/api/api_test.go | 10 + internal/api/handlers_account_migrate.go | 9 + internal/api/handlers_account_migrate_test.go | 61 ++++++ internal/api/pgrepo.go | 61 +++++- internal/api/repo.go | 9 +- internal/pgint/accounts_test.go | 196 +++++++++++++++++- panel/src/i18n/resources/en-US/errors.json | 1 + panel/src/i18n/resources/zh-CN/errors.json | 1 + panel/src/lib/api.test.ts | 6 + panel/src/lib/api.ts | 2 + panel/src/lib/openapi.gen.ts | 9 + 12 files changed, 362 insertions(+), 11 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index f6673ef..d7a9dca 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -5709,6 +5709,14 @@ paths: schema: { $ref: '#/components/schemas/Error' } '401': $ref: '#/components/responses/Unauthorized' + '403': + description: > + The source's servers would push the caller over a quota cap + (migrate_quota_exceeded). Nothing moved and the code is unspent; it redeems once + the quota fits, until it expires. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } /api/v1/me/submissions: post: diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 074889b..da23b5d 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -47,6 +47,9 @@ type fakeRepo struct { // claimQuotaRefuse simulates ClaimServer's atomic quota gate (audit #4) // refusing a name whose advisory pre-check already passed. claimQuotaRefuse map[string]bool + // migrateQuotaRefuse names redeeming targets whose quota RedeemMigration's gate + // finds too small for the servers moving in; it refuses only a source that owns one. + migrateQuotaRefuse map[string]bool // serverResources / resourceUpdates mirror the cached resource columns: // ServerResources is what the resize path reads (to preserve storage), and // UpdateServerResources records the write for assertions. @@ -1471,6 +1474,13 @@ func (f *fakeRepo) RedeemMigration(_ context.Context, targetUserID, codeHash str if mig == nil { return "", nil, ErrLinkCodeInvalid } + if f.migrateQuotaRefuse[targetUserID] { + for _, rec := range f.byName { + if rec.OwnerID == mig.sourceUserID { + return "", nil, ErrQuotaExceeded // before anything moves, as the real gate + } + } + } // Re-point every server the source owns to the target (byName holds pointers). var moved []string for name, rec := range f.byName { diff --git a/internal/api/handlers_account_migrate.go b/internal/api/handlers_account_migrate.go index 2ed1eaf..2d055a0 100644 --- a/internal/api/handlers_account_migrate.go +++ b/internal/api/handlers_account_migrate.go @@ -410,6 +410,11 @@ func (a *API) handleMigrateIssueCode(w http.ResponseWriter, r *http.Request) { var errMigrateNotConfirmed = newError(http.StatusConflict, "not_confirmed", "confirm the migration on this browser first; a confirmation lasts 10 minutes") +// errMigrateQuotaExceeded refuses a redeem whose servers would push the target over +// its quota. The code stays unspent, so raising the quota and redeeming again works. +var errMigrateQuotaExceeded = newError(http.StatusForbidden, "migrate_quota_exceeded", + "the servers this migration moves do not fit this account's quota; ask an admin to raise it, then redeem the same code again before it expires") + // migrateRedeemRequest is the redeem body: the one-time code the target received. type migrateRedeemRequest struct { Code string `json:"code"` @@ -441,6 +446,10 @@ func (a *API) handleMigrateRedeem(w http.ResponseWriter, r *http.Request) { writeError(w, r, newError(http.StatusBadRequest, "invalid_code", "migrate code is invalid or expired")) return } + if errors.Is(err, ErrQuotaExceeded) { + writeError(w, r, errMigrateQuotaExceeded) + return + } writeError(w, r, err) return } diff --git a/internal/api/handlers_account_migrate_test.go b/internal/api/handlers_account_migrate_test.go index a886423..7f6e01b 100644 --- a/internal/api/handlers_account_migrate_test.go +++ b/internal/api/handlers_account_migrate_test.go @@ -315,6 +315,67 @@ func TestMigrateRedeemBinding(t *testing.T) { } } +// TestMigrateRedeemOverQuota: a target whose quota cannot take the source's servers is +// refused with 403 migrate_quota_exceeded, and the refusal leaves everything as it was — +// no server moved, the source live, the migration still code_issued, nothing audited or +// mailed — so the same code redeems once an admin raises the quota. +func TestMigrateRedeemOverQuota(t *testing.T) { + const uuid = "88888888-8888-8888-8888-888888888888" + src := &Principal{UserID: "u1", Email: "old@example.net", Role: "user"} + tgt := &Principal{UserID: "u2", Email: "new@example.net", Role: "user"} + + repo := newFakeRepo() + repo.seedUser(UserView{ID: "u1", Username: "old", Email: "old@example.net", Role: "user"}) + repo.seedUser(UserView{ID: "u2", Username: "new", Email: "new@example.net", Role: "user"}) + repo.links[uuid] = "u1" + repo.byName["alpha"] = &ServerRecord{Name: "alpha", OwnerID: "u1"} + repo.migrateQuotaRefuse = map[string]bool{"u2": true} + + mk, mailer, _ := migrateEnv(repo) + ehSrc := mk(src).ExternalHandler() + if w := startMigrate(t, mk(src).InternalHandler(), uuid); w.Code != http.StatusCreated { + t.Fatalf("start: %d (%s)", w.Code, w.Body.String()) + } + if w := do(ehSrc, "POST", "/api/v1/account/migrate/confirm/otp/start", "", jsonHeader); w.Code != http.StatusAccepted { + t.Fatalf("otp start: %d (%s)", w.Code, w.Body.String()) + } + if w := do(ehSrc, "POST", "/api/v1/account/migrate/confirm/otp/verify", `{"code":"`+mailer.code+`"}`, jsonHeader); w.Code != http.StatusOK { + t.Fatalf("otp verify: %d (%s)", w.Code, w.Body.String()) + } + w := do(ehSrc, "POST", "/api/v1/account/migrate/issue-code", `{"target_user_id":"u2"}`, jsonHeader) + if w.Code != http.StatusCreated { + t.Fatalf("issue-code: %d (%s)", w.Code, w.Body.String()) + } + mcode, _ := acctBody(t, w)["code"].(string) + audits, notices := len(repo.audits), len(mailer.notices) + + ehTgt := mk(tgt).ExternalHandler() + w = do(ehTgt, "POST", "/api/v1/account/migrate/redeem", `{"code":"`+mcode+`"}`, jsonHeader) + if w.Code != http.StatusForbidden || decodeErr(t, w) != "migrate_quota_exceeded" { + t.Fatalf("redeem over quota: code = %d body %s, want 403 migrate_quota_exceeded", w.Code, w.Body.String()) + } + if repo.byName["alpha"].OwnerID != "u1" { + t.Fatalf("server moved on a refused redeem: owner=%s", repo.byName["alpha"].OwnerID) + } + if d, _ := repo.UserDetail(context.Background(), "u1"); d.DeletedAt != nil || d.Disabled { + t.Fatalf("source retired on a refused redeem: %+v", d) + } + if m, _ := repo.MigrationForSource(context.Background(), "u1"); m == nil || m.State != "code_issued" { + t.Fatalf("migration after a refused redeem = %+v, want still code_issued", m) + } + if len(repo.audits) != audits || len(mailer.notices) != notices { + t.Fatalf("a refused redeem audited %v / mailed %q", repo.audits[audits:], mailer.notices[notices:]) + } + + delete(repo.migrateQuotaRefuse, "u2") + if w := do(ehTgt, "POST", "/api/v1/account/migrate/redeem", `{"code":"`+mcode+`"}`, jsonHeader); w.Code != http.StatusOK { + t.Fatalf("redeem once the quota fits: code = %d body %s, want 200", w.Code, w.Body.String()) + } + if repo.byName["alpha"].OwnerID != "u2" { + t.Fatalf("server not moved once the quota fits: owner=%s", repo.byName["alpha"].OwnerID) + } +} + // TestMigrateGuards covers the input/state refusals: an unlinked UUID has no account to // migrate; a code cannot be issued before confirmation; the target may be neither the // source itself nor an unknown account. diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 61fbf6d..05ee4f5 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -396,7 +396,7 @@ func (p *PGRepo) QuotaCheck(ctx context.Context, userID string, excludeName stri return false, err } - return quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum, incoming), nil + return quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum, 1, incoming), nil } // UpdateServerResources updates the resource cache for a server after a spec @@ -498,7 +498,7 @@ func (p *PGRepo) ClaimServer(ctx context.Context, name, userID string) (bool, er return false, err } if !quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum, - ResourceSpec{CPUMilli: cpu, MemoryMB: mem, StorageMB: stor}) { + 1, ResourceSpec{CPUMilli: cpu, MemoryMB: mem, StorageMB: stor}) { return false, ErrQuotaExceeded } @@ -526,13 +526,14 @@ func (p *PGRepo) ClaimServer(ctx context.Context, name, userID string) (bool, er } // quotaAllows applies the four spec §9.3 caps to one per-owner aggregate plus -// the incoming spec. Shared by QuotaCheck (the advisory pre-check) and -// ClaimServer (the atomic gate) so the two can never drift. An invalid (NULL or +// incomingCount servers whose specs sum to incoming. Shared by QuotaCheck (the +// advisory pre-check), ClaimServer (the atomic gate) and RedeemMigration (a +// migration's servers arriving at once) so they can never drift. An invalid (NULL or // missing) cap means unlimited for that dimension; storage is compared in MB // against max_storage_gb × 1024. func quotaAllows(maxServers, maxCPU, maxMem, maxStor sql.NullInt64, - count, cpuSum, memSum, storSum int64, incoming ResourceSpec) bool { - if maxServers.Valid && count >= maxServers.Int64 { + count, cpuSum, memSum, storSum, incomingCount int64, incoming ResourceSpec) bool { + if maxServers.Valid && count+incomingCount > maxServers.Int64 { return false } if maxCPU.Valid && cpuSum+int64(incoming.CPUMilli) > maxCPU.Int64 { @@ -2492,6 +2493,54 @@ func (p *PGRepo) RedeemMigration(ctx context.Context, targetUserID, codeHash str return "", nil, err } + // Take both accounts' claim lanes, ClaimServer's lock, in id order so two + // migrations crossing between the same pair cannot deadlock. The target's makes + // the quota read below and the move one consistent decision against its own + // claims; the source's keeps a claim of its own from landing between the count + // of its servers and their move. + lanes := []string{sourceUserID, targetUserID} + if lanes[1] < lanes[0] { + lanes[0], lanes[1] = lanes[1], lanes[0] + } + for _, id := range lanes { + if _, err := tx.ExecContext(ctx, `SELECT pg_advisory_xact_lock(hashtext($1))`, id); err != nil { + return "", nil, err + } + } + + // The target's four caps (spec §9.3) must hold with every server the source owns + // added in, as a claim of each would. Over any cap, nothing moves and the code + // stays unspent, so the target can have an admin raise its quota and redeem again + // before the code expires. A source that owns nothing moves nothing, and retires + // even into a target already over a cap. + var n, cpu, mem, stor int64 + if err := tx.QueryRowContext(ctx, + `SELECT COUNT(*), COALESCE(SUM(cached_cpu_milli), 0), COALESCE(SUM(cached_memory_mb), 0), COALESCE(SUM(cached_storage_mb), 0) + FROM servers WHERE owner_id = $1 AND deleted_at IS NULL`, + sourceUserID).Scan(&n, &cpu, &mem, &stor); err != nil { + return "", nil, err + } + if n > 0 { + var maxServers, maxCPU, maxMem, maxStor sql.NullInt64 + if err := tx.QueryRowContext(ctx, + `SELECT max_servers, max_cpu_milli, max_memory_mb, max_storage_gb + FROM quotas WHERE user_id = $1`, targetUserID).Scan( + &maxServers, &maxCPU, &maxMem, &maxStor); err != nil && !errors.Is(err, sql.ErrNoRows) { + return "", nil, err + } + var count, cpuSum, memSum, storSum int64 + if err := tx.QueryRowContext(ctx, + `SELECT COUNT(*), COALESCE(SUM(cached_cpu_milli), 0), COALESCE(SUM(cached_memory_mb), 0), COALESCE(SUM(cached_storage_mb), 0) + FROM servers WHERE owner_id = $1 AND deleted_at IS NULL`, + targetUserID).Scan(&count, &cpuSum, &memSum, &storSum); err != nil { + return "", nil, err + } + if !quotaAllows(maxServers, maxCPU, maxMem, maxStor, count, cpuSum, memSum, storSum, + n, ResourceSpec{CPUMilli: int(cpu), MemoryMB: int(mem), StorageMB: int(stor)}) { + return "", nil, ErrQuotaExceeded + } + } + // Re-point every server the source owns to the target, collecting the names for // the audit trail. Server ownership is the only thing that moves. rows, err := tx.QueryContext(ctx, diff --git a/internal/api/repo.go b/internal/api/repo.go index daa69a1..2c0edad 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -825,9 +825,12 @@ type Repo interface { // the migration 'redeemed' — all in one transaction. It returns the source user id // and the moved server names for the audit trail. No matching or expired code, or a // code whose named target is a different user → ErrLinkCodeInvalid (an intercepted - // code is useless to anyone but the named target). Server ownership is the only thing - // moved — the mc_uuid link and web credentials stay with their accounts. now drives - // expiry and the terminal timestamps. + // code is useless to anyone but the named target). When the source owns servers, the + // target's quota must hold with all of them added in, checked under ClaimServer's + // lock for both accounts; over any cap → ErrQuotaExceeded, with nothing moved and the + // code left unspent. Server ownership is the only thing moved — the mc_uuid link and + // web credentials stay with their accounts. now drives expiry and the terminal + // timestamps. RedeemMigration(ctx context.Context, targetUserID, codeHash string, now time.Time) (sourceUserID string, movedServers []string, err error) } diff --git a/internal/pgint/accounts_test.go b/internal/pgint/accounts_test.go index 28c2091..6c151ca 100644 --- a/internal/pgint/accounts_test.go +++ b/internal/pgint/accounts_test.go @@ -339,6 +339,12 @@ func TestMigrationUnderConcurrency(t *testing.T) { // waitForLockWait polls until a statement starting with prefix is waiting on a lock. func waitForLockWait(t *testing.T, prefix string) { + t.Helper() + waitForLockWaiters(t, prefix, 1) +} + +// waitForLockWaiters polls until want statements starting with prefix wait on a lock. +func waitForLockWaiters(t *testing.T, prefix string, want int) { t.Helper() deadline := time.Now().Add(5 * time.Second) for time.Now().Before(deadline) { @@ -348,12 +354,12 @@ func waitForLockWait(t *testing.T, prefix string) { prefix).Scan(&n); err != nil { t.Fatalf("read pg_stat_activity: %v", err) } - if n > 0 { + if n >= want { return } time.Sleep(10 * time.Millisecond) } - t.Fatalf("no statement %q waited on a lock within 5s", prefix) + t.Fatalf("fewer than %d statements %q waited on a lock within 5s", want, prefix) } // A restart that reaches the source while its redeem is in flight must not leave the @@ -400,6 +406,192 @@ func TestStartMigrationBehindARedeem(t *testing.T) { } } +// untouched checks a refused redeem left the source as it was: its servers, the +// account and its session live, and the code still issued. +func untouched(t *testing.T, what, srcID, session string, servers ...string) { + t.Helper() + for _, s := range servers { + if owner := serverOwner(t, s); owner != srcID { + t.Fatalf("%s moved %s to %s", what, s, owner) + } + } + var live, sessionLive bool + if err := db.QueryRow(`SELECT NOT disabled AND deleted_at IS NULL FROM users WHERE id = $1`, srcID).Scan(&live); err != nil { + t.Fatalf("read source: %v", err) + } + if err := db.QueryRow(`SELECT revoked_at IS NULL FROM sessions WHERE token_hash = $1`, session).Scan(&sessionLive); err != nil { + t.Fatalf("read source session: %v", err) + } + if !live || !sessionLive { + t.Fatalf("%s: source live=%v, session live=%v; want both", what, live, sessionLive) + } + if m, err := repo.MigrationForSource(context.Background(), srcID); err != nil || m.State != "code_issued" { + t.Fatalf("%s: migration = %+v, %v; want still code_issued", what, m, err) + } +} + +// A redeem moves the source's servers only when the target's four caps hold with all +// of them added in. Over any cap it changes nothing, and the same code redeems once +// the quota fits. Deleted servers count on neither side. +func TestMigrationRedeemWithinTargetQuota(t *testing.T) { + ctx := context.Background() + now := mustNow() + src := newUser(t, "user", "migq-src") + dst := newUser(t, "user", "migq-dst") + sfx := suffix(t) + a, b := "mqa-"+sfx, "mqb-"+sfx + seedOwnedServer(t, a, src.ID, false) + seedOwnedServer(t, b, src.ID, false) + seedOwnedServer(t, "mqgone-"+sfx, src.ID, true) + seedOwnedServer(t, "mqown-"+sfx, dst.ID, false) + mustExec(t, `UPDATE servers SET cached_storage_mb = 400 WHERE name IN ($1, $2)`, a, b) + mustExec(t, `UPDATE servers SET cached_storage_mb = 225 WHERE name = $1`, "mqown-"+sfx) + seedOwnedServer(t, "mqowngone-"+sfx, dst.ID, true) + session := newSession(t, src.ID, "migq-src", now.Add(time.Hour)) + startToCode(t, src.ID, dst.ID, "h-quota", now, now.Add(10*time.Minute)) + + // With both servers the target would own 3 servers, 300 millicores, 384 MB of + // memory and 1025 MB of storage. Each cap one short of its figure refuses on its + // own; storage is capped in whole GB, and 1 GB is 1024 MB. + n := func(v int) *int { return &v } + for _, c := range []struct { + name string + q api.QuotaInput + }{ + {"servers", api.QuotaInput{MaxServers: n(2)}}, + {"cpu", api.QuotaInput{MaxCPUMilli: n(299)}}, + {"memory", api.QuotaInput{MaxMemoryMB: n(383)}}, + {"storage", api.QuotaInput{MaxStorageGB: n(1)}}, + } { + if _, err := repo.SetQuotas(ctx, dst.ID, c.q, "pgint"); err != nil { + t.Fatalf("SetQuotas(%s): %v", c.name, err) + } + if _, _, err := repo.RedeemMigration(ctx, dst.ID, "h-quota", now); !errors.Is(err, api.ErrQuotaExceeded) { + t.Fatalf("redeem over the %s cap = %v, want ErrQuotaExceeded", c.name, err) + } + untouched(t, "a redeem over the "+c.name+" cap", src.ID, session, a, b) + } + + // Every cap at its figure fits, storage at the next whole GB. + if _, err := repo.SetQuotas(ctx, dst.ID, api.QuotaInput{MaxServers: n(3), MaxCPUMilli: n(300), MaxMemoryMB: n(384), MaxStorageGB: n(2)}, "pgint"); err != nil { + t.Fatalf("SetQuotas(fits): %v", err) + } + _, moved, err := repo.RedeemMigration(ctx, dst.ID, "h-quota", now) + sort.Strings(moved) + if err != nil || strings.Join(moved, ",") != a+","+b { + t.Fatalf("redeem once the quota fits = %v, %v; want [%s %s]", moved, err, a, b) + } + if o := serverOwner(t, a); o != dst.ID { + t.Fatalf("owner of %s after the redeem = %s, want the target", a, o) + } + + // A source that owns nothing brings nothing, so it retires even into a target + // already over a cap. + empty := newUser(t, "user", "migq-empty") + startToCode(t, empty.ID, dst.ID, "h-empty", now, now.Add(10*time.Minute)) + if _, err := repo.SetQuotas(ctx, dst.ID, api.QuotaInput{MaxServers: n(0)}, "pgint"); err != nil { + t.Fatalf("SetQuotas(0): %v", err) + } + if _, moved, err := repo.RedeemMigration(ctx, dst.ID, "h-empty", now); err != nil || len(moved) != 0 { + t.Fatalf("redeem of a source with no servers = %v, %v; want nothing moved", moved, err) + } + var retired bool + if err := db.QueryRow(`SELECT disabled AND deleted_at IS NOT NULL FROM users WHERE id = $1`, empty.ID).Scan(&retired); err != nil || !retired { + t.Fatalf("empty source after its redeem: retired=%v, %v; want retired", retired, err) + } +} + +// holdLane takes a user's claim lane (ClaimServer's advisory lock) in a transaction the +// caller ends. +func holdLane(t *testing.T, userIDs ...string) *sql.Tx { + t.Helper() + tx, err := db.BeginTx(context.Background(), nil) + if err != nil { + t.Fatalf("begin: %v", err) + } + for _, id := range userIDs { + if _, err := tx.Exec(`SELECT pg_advisory_xact_lock(hashtext($1))`, id); err != nil { + tx.Rollback() //nolint:errcheck // the test is failing anyway + t.Fatalf("hold lane: %v", err) + } + } + return tx +} + +// A redeem decides under both accounts' claim lanes: a server either account claims +// while the redeem waits is counted against the target's quota. The target holds one +// server of a two-server cap and the source one, so either claim tips it over. +func TestMigrationRedeemWaitsForClaims(t *testing.T) { + ctx := context.Background() + now := mustNow() + n := func(v int) *int { return &v } + for _, lane := range []string{"target", "source"} { + src := newUser(t, "user", "migw-src") + dst := newUser(t, "user", "migw-dst") + sfx := suffix(t) + mine, claimed := "mwa-"+sfx, "mwc-"+sfx + seedOwnedServer(t, mine, src.ID, false) + seedOwnedServer(t, "mwown-"+sfx, dst.ID, false) + mustExec(t, `INSERT INTO servers (name, cached_cpu_milli, cached_memory_mb, cached_storage_mb) VALUES ($1, 100, 128, 1)`, claimed) + if _, err := repo.SetQuotas(ctx, dst.ID, api.QuotaInput{MaxServers: n(2)}, "pgint"); err != nil { + t.Fatalf("SetQuotas: %v", err) + } + session := newSession(t, src.ID, "migw-src", now.Add(time.Hour)) + startToCode(t, src.ID, dst.ID, "h-wait", now, now.Add(10*time.Minute)) + claimer := dst.ID + if lane == "source" { + claimer = src.ID + } + + hold := holdLane(t, claimer) + redeemed := make(chan error, 1) + go func() { + _, _, err := repo.RedeemMigration(ctx, dst.ID, "h-wait", now) + redeemed <- err + }() + waitForLockWait(t, "SELECT pg_advisory_xact_lock") + if _, err := hold.Exec(`UPDATE servers SET owner_id = $2 WHERE name = $1`, claimed, claimer); err != nil { + t.Fatalf("%s claim: %v", lane, err) + } + if err := hold.Commit(); err != nil { + t.Fatalf("%s claim commit: %v", lane, err) + } + if err := <-redeemed; !errors.Is(err, api.ErrQuotaExceeded) { + t.Fatalf("redeem behind a %s claim = %v, want ErrQuotaExceeded", lane, err) + } + untouched(t, "a redeem behind a "+lane+" claim", src.ID, session, mine) + } +} + +// Two migrations crossing between one pair of accounts take their lanes in one order, +// so neither deadlocks the other: both wait behind a holder of both lanes and, once it +// lets go, both redeem. +func TestCrossingMigrationsDoNotDeadlock(t *testing.T) { + ctx := context.Background() + now := mustNow() + x := newUser(t, "user", "migx-a") + y := newUser(t, "user", "migx-b") + sfx := suffix(t) + seedOwnedServer(t, "mxa-"+sfx, x.ID, false) + seedOwnedServer(t, "mxb-"+sfx, y.ID, false) + startToCode(t, x.ID, y.ID, "h-x", now, now.Add(10*time.Minute)) + startToCode(t, y.ID, x.ID, "h-y", now, now.Add(10*time.Minute)) + + hold := holdLane(t, x.ID, y.ID) + errs := make(chan error, 2) + go func() { _, _, err := repo.RedeemMigration(ctx, y.ID, "h-x", now); errs <- err }() + go func() { _, _, err := repo.RedeemMigration(ctx, x.ID, "h-y", now); errs <- err }() + waitForLockWaiters(t, "SELECT pg_advisory_xact_lock", 2) + if err := hold.Commit(); err != nil { + t.Fatalf("release lanes: %v", err) + } + for i := 0; i < 2; i++ { + if err := <-errs; err != nil { + t.Fatalf("crossing redeem: %v", err) + } + } +} + // ---- setup tokens (migration 0012) -------------------------------------------------- // A setup token redeems once, never at or after expiry, and racing redeems of one diff --git a/panel/src/i18n/resources/en-US/errors.json b/panel/src/i18n/resources/en-US/errors.json index ac19192..7e6548c 100644 --- a/panel/src/i18n/resources/en-US/errors.json +++ b/panel/src/i18n/resources/en-US/errors.json @@ -79,6 +79,7 @@ "account_retired": "This account has been retired — sign in with the account it was migrated to.", "no_migration": "There is no migration in progress.", "not_confirmed": "Confirm it's you in this browser first. A confirmation lasts 10 minutes.", + "migrate_quota_exceeded": "The servers this migration brings over don't fit your quota. Ask an admin to raise it, then redeem the same code again before it expires.", "invalid_quota": "A quota must be a whole number from 0 to 2147483647. Leave it empty for unlimited.", "already_confirmed": "This migration has already been confirmed.", "invalid_target": "The migration target must be a different account.", diff --git a/panel/src/i18n/resources/zh-CN/errors.json b/panel/src/i18n/resources/zh-CN/errors.json index 6a1e57b..b894e93 100644 --- a/panel/src/i18n/resources/zh-CN/errors.json +++ b/panel/src/i18n/resources/zh-CN/errors.json @@ -79,6 +79,7 @@ "account_retired": "该账户已退役——请使用迁移后的账户登录。", "no_migration": "当前没有进行中的迁移。", "not_confirmed": "请先在当前浏览器完成身份确认，确认 10 分钟内有效。", + "migrate_quota_exceeded": "这次迁移带来的服务器超出了你的配额。请管理员调高配额后，在迁移码过期前用同一个码再兑换一次。", "invalid_quota": "配额要填 0 到 2147483647 之间的整数，不限就留空。", "already_confirmed": "该迁移已经确认过了。", "invalid_target": "迁移目标账户不能与来源账户相同。", diff --git a/panel/src/lib/api.test.ts b/panel/src/lib/api.test.ts index 6f67b53..25f2717 100644 --- a/panel/src/lib/api.test.ts +++ b/panel/src/lib/api.test.ts @@ -218,6 +218,12 @@ describe("account migration wire shapes", () => { code: "MIGR-1234", }); }); + + it("says a redeem over quota can be retried with the same code once the quota fits", () => { + expect(humanizeError({ status: 403, code: "migrate_quota_exceeded" })).toBe( + "The servers this migration brings over don't fit your quota. Ask an admin to raise it, then redeem the same code again before it expires.", + ); + }); }); // Pin the GET /fleet wire shape (the SysAdmin cockpit's read). It is the ONLY diff --git a/panel/src/lib/api.ts b/panel/src/lib/api.ts index d8b6bb4..b2ac34e 100644 --- a/panel/src/lib/api.ts +++ b/panel/src/lib/api.ts @@ -1184,6 +1184,8 @@ export function humanizeError(e: unknown): string { return t("no_migration"); case "not_confirmed": return t("not_confirmed"); + case "migrate_quota_exceeded": + return t("migrate_quota_exceeded"); case "invalid_quota": return t("invalid_quota"); case "already_confirmed": diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index 12b9a7c..1be2b46 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -7685,6 +7685,15 @@ export interface operations { }; }; 401: components["responses"]["Unauthorized"]; + /** @description The source's servers would push the caller over a quota cap (migrate_quota_exceeded). Nothing moved and the code is unspent; it redeems once the quota fits, until it expires. */ + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Error"]; + }; + }; }; }; mySubmissions: {