fix: Harden instance rename against interleaving and lost unwinds
This commit is contained in:
+21
-14
@@ -107,34 +107,41 @@ func RenameSlug(name, currentSlug string) (string, error) {
|
||||
|
||||
// RenameInstance changes an instance's name and re-derives its slug from it.
|
||||
//
|
||||
// It returns the name and slug the control plane held BEFORE the write, and
|
||||
// those are the only correct values to unwind with. The caller's own copy of the
|
||||
// instance may be stale, and admin's copy stores slug with `omitempty`, so an
|
||||
// unwind driven from there can write an empty slug — which either mis-restores
|
||||
// the tenant host or trips the unique index against every other slugless row.
|
||||
//
|
||||
// The count-then-update is racy on its own, and is safe for the same reason
|
||||
// CreateInstanceWithID's loop is: instances.slug carries a unique index, so a
|
||||
// lost race surfaces as a duplicate-key error. Unlike creation there is nothing
|
||||
// to retry with — the caller asked for one specific name — so it becomes
|
||||
// ErrSlugTaken. Do not remove the duplicate-key branch, and do not remove the
|
||||
// index.
|
||||
func RenameInstance(ctx context.Context, db *mongo.Database, instanceID, name string) (*models.Instance, error) {
|
||||
var inst models.Instance
|
||||
func RenameInstance(ctx context.Context, db *mongo.Database, instanceID, name string) (inst *models.Instance, prevName, prevSlug string, err error) {
|
||||
var cur models.Instance
|
||||
if err := db.Collection("instances").FindOne(ctx,
|
||||
bson.M{"instance_id": instanceID}).Decode(&inst); err != nil {
|
||||
return nil, err
|
||||
bson.M{"instance_id": instanceID}).Decode(&cur); err != nil {
|
||||
return nil, "", "", err
|
||||
}
|
||||
prevName, prevSlug = cur.Name, cur.Slug
|
||||
|
||||
slug, err := RenameSlug(name, inst.Slug)
|
||||
slug, err := RenameSlug(name, cur.Slug)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, prevName, prevSlug, err
|
||||
}
|
||||
|
||||
if slug != inst.Slug {
|
||||
if slug != cur.Slug {
|
||||
n, err := db.Collection("instances").CountDocuments(ctx, bson.M{
|
||||
"slug": slug,
|
||||
"instance_id": bson.M{"$ne": instanceID},
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, prevName, prevSlug, err
|
||||
}
|
||||
if n > 0 {
|
||||
return nil, fmt.Errorf("%w: %s", ErrSlugTaken, slug)
|
||||
return nil, prevName, prevSlug, fmt.Errorf("%w: %s", ErrSlugTaken, slug)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -142,14 +149,14 @@ func RenameInstance(ctx context.Context, db *mongo.Database, instanceID, name st
|
||||
bson.M{"instance_id": instanceID},
|
||||
bson.M{"$set": bson.M{"name": name, "slug": slug}}); err != nil {
|
||||
if mongo.IsDuplicateKeyError(err) {
|
||||
return nil, fmt.Errorf("%w: %s", ErrSlugTaken, slug)
|
||||
return nil, prevName, prevSlug, fmt.Errorf("%w: %s", ErrSlugTaken, slug)
|
||||
}
|
||||
return nil, err
|
||||
return nil, prevName, prevSlug, err
|
||||
}
|
||||
|
||||
inst.Name = name
|
||||
inst.Slug = slug
|
||||
return &inst, nil
|
||||
cur.Name = name
|
||||
cur.Slug = slug
|
||||
return &cur, prevName, prevSlug, nil
|
||||
}
|
||||
|
||||
// RestoreInstanceIdentity writes an exact name and slug back, unwinding a rename
|
||||
|
||||
Reference in New Issue
Block a user