From 287bd9657bef71acccd62f9d181d12aa2ef36a98 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Mon, 3 Aug 2026 17:34:39 +0100 Subject: [PATCH] fix: Fixed paddle relink sub --- admin/internal/api/checkout.go | 72 ++++++-------------------- admin/internal/billing/subscription.go | 8 +-- admin/internal/licensing/link.go | 58 ++++++++++++++++++++- admin/internal/models/backfill.go | 62 ++++++++++++++-------- admin/internal/models/models.go | 15 +++--- 5 files changed, 127 insertions(+), 88 deletions(-) diff --git a/admin/internal/api/checkout.go b/admin/internal/api/checkout.go index 9e8999d..32029a6 100644 --- a/admin/internal/api/checkout.go +++ b/admin/internal/api/checkout.go @@ -1,9 +1,7 @@ package api import ( - "context" "fmt" - "log" "net/http" "strings" "time" @@ -13,6 +11,7 @@ import ( "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/billing" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/catalogue" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/db" + "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/licensing" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/models" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/paddle" "gitea.hostxtra.co.uk/mrhid6/vantage/shared/license" @@ -247,32 +246,26 @@ func claimPlaceholderLink(c *gin.Context) { placeholderID := inst.InstanceID if _, err := db.Admin("admin_instances").UpdateOne(ctx, bson.M{"instance_id": placeholderID}, - bson.M{"$set": bson.M{ - "instance_id": body.InstanceID, - "status": models.StatusActive, - "placeholder": false, - "linked_from_placeholder_id": placeholderID, - }}); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) - return - } - // Re-point the subscription from the placeholder id to the real UUID so - // billing.IssueForInstance (and every later webhook) finds it. - if _, err := db.Admin("subscriptions").UpdateMany(ctx, - bson.M{"instance_id": placeholderID}, - bson.M{"$set": bson.M{"instance_id": body.InstanceID}}); err != nil { + bson.M{ + "$set": bson.M{ + "instance_id": body.InstanceID, + "status": models.StatusActive, + "placeholder": false, + }, + "$addToSet": bson.M{"previous_instance_ids": placeholderID}, + }); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - // Paddle holds its own copy of custom_data, written at checkout, and it still - // names the placeholder. Every later event on this subscription — renewal, - // cancellation, an entitlement change — is decoded from it, so leaving it - // stale means the next webhook resolves to an id no row carries any more. - // Best-effort: the linked_from_placeholder_id alias above is what makes the - // webhook path correct whether or not this call lands, and the customer must - // not be blocked from linking by an outbound API failure. - syncSubscriptionCustomData(ctx, placeholderID, body.InstanceID, inst.AccountID) + // Re-point the subscription rows from the placeholder id to the real UUID so + // billing.IssueForInstance finds it, and rewrite Paddle's own copy of + // custom_data — written at checkout, it still names the placeholder, and every + // later event on this subscription is decoded from it. + if err := licensing.RepointSubscriptions(ctx, placeholderID, body.InstanceID, inst.AccountID); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + return + } if err := billing.IssueForInstance(ctx, body.InstanceID); err != nil { // The link stuck; issuance did not. The reconciler and a retry recover it, @@ -289,37 +282,6 @@ func claimPlaceholderLink(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"instance_id": body.InstanceID}) } -// syncSubscriptionCustomData rewrites custom_data.instance_id at Paddle for -// every subscription that named the placeholder, so future events decode to the -// real UUID. Paddle replaces the whole object on a PATCH, so account_id is sent -// alongside rather than dropped. -// -// Deliberately silent on failure: it is a convergence step, not the correctness -// boundary — handleSubscription resolves a stale id through the instance row's -// linked_from_placeholder_id either way. -func syncSubscriptionCustomData(ctx context.Context, placeholderID, realID, accountID string) { - cur, err := db.Admin("subscriptions").Find(ctx, bson.M{"instance_id": realID}) - if err != nil { - log.Printf("claim link %s: read subscriptions: %v", realID, err) - return - } - var subs []models.Subscription - if err := cur.All(ctx, &subs); err != nil { - log.Printf("claim link %s: decode subscriptions: %v", realID, err) - return - } - for _, s := range subs { - if s.PaddleSubscriptionID == "" { - continue - } - if err := paddle.Get().UpdateSubscriptionCustomData(ctx, s.PaddleSubscriptionID, - map[string]string{"account_id": accountID, "instance_id": realID}); err != nil { - log.Printf("claim link %s: patch custom_data on %s (was %s): %v", - realID, s.PaddleSubscriptionID, placeholderID, err) - } - } -} - // billingPortal mints a Paddle customer-portal URL. The account must already // have a paddle_customer_id, which it learns from its first subscription webhook. func billingPortal(c *gin.Context) { diff --git a/admin/internal/billing/subscription.go b/admin/internal/billing/subscription.go index 9e2567c..0e62b09 100644 --- a/admin/internal/billing/subscription.go +++ b/admin/internal/billing/subscription.go @@ -124,9 +124,9 @@ func handleSubscription(ctx context.Context, ev Event) error { } // resolveInstance finds the instance a webhook's custom_data names, following the -// placeholder alias when the id is one a self-hosted claim has since replaced. -// It returns the instance's CURRENT id, which is the only id anything else should -// be written against. +// identity trail when the id is one a placeholder claim or a relink has since +// replaced. It returns the instance's CURRENT id, which is the only id anything +// else should be written against. func resolveInstance(ctx context.Context, customDataID string) (string, models.Instance, error) { var inst models.Instance err := db.Admin("admin_instances").FindOne(ctx, @@ -138,7 +138,7 @@ func resolveInstance(ctx context.Context, customDataID string) (string, models.I return "", inst, err } if err := db.Admin("admin_instances").FindOne(ctx, - bson.M{"linked_from_placeholder_id": customDataID}).Decode(&inst); err != nil { + bson.M{"previous_instance_ids": customDataID}).Decode(&inst); err != nil { return "", inst, err } return inst.InstanceID, inst, nil diff --git a/admin/internal/licensing/link.go b/admin/internal/licensing/link.go index 41e70b1..e275277 100644 --- a/admin/internal/licensing/link.go +++ b/admin/internal/licensing/link.go @@ -4,11 +4,13 @@ import ( "context" "errors" "fmt" + "log" "time" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/audit" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/db" "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/models" + "gitea.hostxtra.co.uk/mrhid6/vantage/admin/internal/paddle" "gitea.hostxtra.co.uk/mrhid6/vantage/shared/license" "github.com/google/uuid" "go.mongodb.org/mongo-driver/v2/bson" @@ -57,6 +59,48 @@ func LinkInstance(ctx context.Context, accountID, instanceID, name string) (*mod return &inst, nil } +// RepointSubscriptions follows an instance identity rewrite: it moves every +// subscription row from the old id to the new one, then rewrites Paddle's copy +// of custom_data so future webhooks decode to the new id. +// +// The local rewrite is returned as an error — issuance reads the subscription +// back, so a half-moved row is worth failing on. The Paddle patch only logs: the +// customer must not be blocked from linking or relinking by an outbound API +// failure, and the caller has already recorded the old id in +// previous_instance_ids, which is what makes the webhook path correct whether or +// not the patch lands. +func RepointSubscriptions(ctx context.Context, oldID, newID, accountID string) error { + if _, err := db.Admin("subscriptions").UpdateMany(ctx, + bson.M{"instance_id": oldID}, + bson.M{"$set": bson.M{"instance_id": newID}}); err != nil { + return fmt.Errorf("repoint %s -> %s: %w", oldID, newID, err) + } + + cur, err := db.Admin("subscriptions").Find(ctx, bson.M{"instance_id": newID}) + if err != nil { + log.Printf("repoint %s -> %s: read subscriptions: %v", oldID, newID, err) + return nil + } + var subs []models.Subscription + if err := cur.All(ctx, &subs); err != nil { + log.Printf("repoint %s -> %s: decode subscriptions: %v", oldID, newID, err) + return nil + } + for _, s := range subs { + if s.PaddleSubscriptionID == "" { + continue + } + // Paddle replaces the whole custom_data object on a PATCH, so account_id + // is sent alongside rather than dropped. + if err := paddle.Get().UpdateSubscriptionCustomData(ctx, s.PaddleSubscriptionID, + map[string]string{"account_id": accountID, "instance_id": newID}); err != nil { + log.Printf("repoint %s -> %s: patch custom_data on %s: %v", + oldID, newID, s.PaddleSubscriptionID, err) + } + } + return nil +} + // Relink moves a licence to a rebuilt server's new UUID. // // The replacement covers the REMAINING term, not a fresh one — relinking is not @@ -96,13 +140,25 @@ func Relink(ctx context.Context, accountID, oldID, newID string, staff bool) (*m if _, err := db.Admin("admin_instances").UpdateOne(ctx, bson.M{"instance_id": oldID}, - bson.M{"$set": bson.M{"instance_id": newID}, "$inc": bson.M{"relink_count": 1}}); err != nil { + bson.M{ + "$set": bson.M{"instance_id": newID}, + "$inc": bson.M{"relink_count": 1}, + "$addToSet": bson.M{"previous_instance_ids": oldID}, + }); err != nil { if mongo.IsDuplicateKeyError(err) { return nil, ErrAlreadyLinked } return nil, fmt.Errorf("relink: %w", err) } + // A relink rewrites the instance's identity exactly as a placeholder claim + // does, so the same two things have to follow it: the subscription rows that + // named the old id, and Paddle's own copy of custom_data. Without this a + // renewal after a relink cannot find its instance and the term never extends. + if err := RepointSubscriptions(ctx, oldID, newID, accountID); err != nil { + return nil, err + } + actor := accountID if staff { actor = "staff" diff --git a/admin/internal/models/backfill.go b/admin/internal/models/backfill.go index 50926d4..b6e9018 100644 --- a/admin/internal/models/backfill.go +++ b/admin/internal/models/backfill.go @@ -13,6 +13,7 @@ import ( "github.com/google/uuid" "go.mongodb.org/mongo-driver/v2/bson" "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" ) // MigrateLegacyPlans re-keys the pre-spec-7 plan rows and MUST run before @@ -212,23 +213,40 @@ func Backfill(ctx context.Context) error { return err } - // Pass 6: instances claimed before linked_from_placeholder_id existed have no - // alias, and Paddle's custom_data still names the placeholder they were - // claimed from — so their next webhook resolves to nothing. The claim wrote an - // audit entry naming both ids, which is the only surviving record of the - // placeholder, so reconstruct the alias from it. - if err := backfillPlaceholderAliases(ctx); err != nil { + // Pass 6: instances whose identity was rewritten before previous_instance_ids + // existed carry no trail, and Paddle's custom_data still names the id they + // were rewritten FROM — so their next webhook resolves to nothing. Both + // rewrites wrote an audit entry naming the old id, which is the only surviving + // record of it, so reconstruct the trail from those. + if err := backfillInstanceIDHistory(ctx); err != nil { return err } return nil } -// backfillPlaceholderAliases rebuilds linked_from_placeholder_id from the -// instance.placeholder_linked audit entries. Idempotent by filtering on the -// absence of the field. -func backfillPlaceholderAliases(ctx context.Context) error { +// backfillInstanceIDHistory rebuilds previous_instance_ids from the audit entries +// the two identity rewrites leave behind: a placeholder claim +// ("instance.placeholder_linked", detail "from placeholder ") and a relink +// ("instance.relinked", detail "was "). +// +// $addToSet is what makes it idempotent, and it also means a chain of relinks +// accumulates rather than the last one winning. Entries are walked NEWEST first, +// matching on the current id or an already-recovered one: an instance relinked +// A→B→C answers to neither A nor B by the time this runs, so the C entry has to +// record B before the B entry has anything to attach A to. +func backfillInstanceIDHistory(ctx context.Context) error { + prefixes := map[string]string{ + "instance.placeholder_linked": "from placeholder ", + "instance.relinked": "was ", + } + actions := make(bson.A, 0, len(prefixes)) + for action := range prefixes { + actions = append(actions, action) + } + cur, err := db.Admin("admin_audit").Find(ctx, - bson.M{"action": "instance.placeholder_linked"}) + bson.M{"action": bson.M{"$in": actions}}, + options.Find().SetSort(bson.D{{Key: "created_at", Value: -1}})) if err != nil { return err } @@ -237,27 +255,29 @@ func backfillPlaceholderAliases(ctx context.Context) error { return err } - const prefix = "from placeholder " - linked := 0 + recorded := 0 for _, e := range entries { + prefix := prefixes[e.Action] if e.Target == "" || !strings.HasPrefix(e.Detail, prefix) { continue } - placeholderID := strings.TrimSpace(strings.TrimPrefix(e.Detail, prefix)) - if placeholderID == "" || placeholderID == e.Target { + oldID := strings.TrimSpace(strings.TrimPrefix(e.Detail, prefix)) + if oldID == "" || oldID == e.Target { continue } res, err := db.Admin("admin_instances").UpdateOne(ctx, - bson.M{"instance_id": e.Target, - "linked_from_placeholder_id": bson.M{"$exists": false}}, - bson.M{"$set": bson.M{"linked_from_placeholder_id": placeholderID}}) + bson.M{"$or": bson.A{ + bson.M{"instance_id": e.Target}, + bson.M{"previous_instance_ids": e.Target}, + }}, + bson.M{"$addToSet": bson.M{"previous_instance_ids": oldID}}) if err != nil { return err } - linked += int(res.ModifiedCount) + recorded += int(res.ModifiedCount) } - if linked > 0 { - log.Printf("backfill: recovered %d placeholder aliases from the audit log", linked) + if recorded > 0 { + log.Printf("backfill: recovered %d instance id rewrites from the audit log", recorded) } return nil } diff --git a/admin/internal/models/models.go b/admin/internal/models/models.go index 7c4abc1..e505b19 100644 --- a/admin/internal/models/models.go +++ b/admin/internal/models/models.go @@ -147,13 +147,14 @@ type Instance struct { // checkout has something to attach custom_data to, before the customer has // pasted their install's real UUID. Cleared when the instance is linked. Placeholder bool `bson:"placeholder,omitempty" json:"placeholder,omitempty"` - // LinkedFromPlaceholderID is the id this row carried while it was a - // self-hosted placeholder, kept forever after the claim rewrote InstanceID to - // the install's real UUID. Paddle's copy of custom_data still names the - // placeholder on any subscription created before the claim (and on any webhook - // delivered while the outbound patch was failing), so this is what lets a - // later webhook resolve to the right instance instead of erroring as unknown. - LinkedFromPlaceholderID string `bson:"linked_from_placeholder_id,omitempty" json:"-"` + // PreviousInstanceIDs is every id this row has carried before its current one. + // A self-hosted row's identity is rewritten twice over its life — once when a + // paid placeholder is claimed, and again on each relink to a rebuilt server — + // and Paddle keeps its own copy of custom_data written at checkout. That copy + // is patched on each rewrite, but the patch is best-effort and any event + // already in flight still names an old id, so this is what lets a webhook + // resolve to the right instance instead of erroring as unknown. + PreviousInstanceIDs []string `bson:"previous_instance_ids,omitempty" json:"-"` // PendingOwnerUserID is the customer_user who bought a paid-cloud placeholder, // remembered so the confirmed-payment webhook can provision the instance with // them as owner. Cleared once provisioned. Only ever set on a cloud placeholder.