From 200ea09157ad0b1fc1eb4cf9c7e2e283ac1ca280 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 11 Jul 2026 21:30:21 +0200 Subject: [PATCH] fix(node): never sweep a node's inbounds before their first adoption Adding a node imports nothing; its pre-existing inbounds only become central rows on the first clean traffic-sync tick. But any save of the node (switching sync mode, picking tags after "Load inbounds from node") marks it config-dirty, and the next tick then ran ReconcileNode before that first adoption: with zero central rows the delete sweep saw every remote tag as undesired and destroyed the node's real inbounds - in "all" mode all of them - disconnecting live clients with no confirmation, and the master then reported "record not found". Track the first completed clean sync in nodes.inbounds_adopted_at and skip the sweep (pushes still run) until it is set, so "absent locally" can no longer be conflated with "deleted on the master". A node that has synced before still sweeps normally, including the offline last-inbound-deleted case. Existing nodes are seeded as adopted on upgrade to keep their behavior unchanged. Closes #5898 --- internal/database/db.go | 19 +++++++++++++- internal/database/model/model.go | 4 +++ internal/web/job/node_traffic_sync_job.go | 5 ++++ internal/web/service/inbound_node.go | 5 ++++ .../service/inbound_node_reconcile_test.go | 26 +++++++++++++++++++ internal/web/service/node.go | 9 +++++++ 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/internal/database/db.go b/internal/database/db.go index 02eef7177..3e09ec518 100644 --- a/internal/database/db.go +++ b/internal/database/db.go @@ -928,7 +928,7 @@ func runSeeders(isUsersEmpty bool) error { } if empty && isUsersEmpty { - seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "ApiTokensHash", "LegacyProxySettingsCleanup", "WireguardPeersToClients", "MtprotoSecretsToClients"} + seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "ApiTokensHash", "LegacyProxySettingsCleanup", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted"} for _, name := range seeders { if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil { return err @@ -1021,6 +1021,12 @@ func runSeeders(isUsersEmpty bool) error { } } + if !slices.Contains(seedersHistory, "NodeInboundsAdopted") { + if err := seedNodeInboundsAdopted(); err != nil { + return err + } + } + if err := seedHostsFromExternalProxy(); err != nil { return err } @@ -1055,6 +1061,17 @@ func runSeeders(isUsersEmpty bool) error { return normalizeSettingPaths() } +// seedNodeInboundsAdopted keeps the pre-existing reconcile behavior for nodes +// that were already syncing before the inbounds_adopted_at gate was introduced. +func seedNodeInboundsAdopted() error { + if err := db.Model(&model.Node{}). + Where("inbounds_adopted_at = 0"). + Update("inbounds_adopted_at", time.Now().Unix()).Error; err != nil { + return err + } + return db.Create(&model.HistoryOfSeeders{SeederName: "NodeInboundsAdopted"}).Error +} + func seedHostGroupIds() error { var history []string if err := db.Model(&model.HistoryOfSeeders{}).Pluck("seeder_name", &history).Error; err != nil { diff --git a/internal/database/model/model.go b/internal/database/model/model.go index ccaf0727d..ca02a9ac1 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -719,6 +719,10 @@ type Node struct { ConfigDirty bool `json:"configDirty" gorm:"default:false"` ConfigDirtyAt int64 `json:"configDirtyAt"` + // InboundsAdoptedAt records the first clean traffic sync that imported the + // node's pre-existing inbounds; reconcile must not sweep remote tags before it. + InboundsAdoptedAt int64 `json:"-" gorm:"column:inbounds_adopted_at;default:0"` + InboundCount int `json:"inboundCount" gorm:"-" example:"5"` ClientCount int `json:"clientCount" gorm:"-" example:"27"` OnlineCount int `json:"onlineCount" gorm:"-" example:"3"` diff --git a/internal/web/job/node_traffic_sync_job.go b/internal/web/job/node_traffic_sync_job.go index 56913f080..50b0e440e 100644 --- a/internal/web/job/node_traffic_sync_job.go +++ b/internal/web/job/node_traffic_sync_job.go @@ -399,6 +399,11 @@ func (j *NodeTrafficSyncJob) syncOne(mgr *runtime.Manager, n *model.Node, doIpSy if changed { j.structural.set() } + if !dirty && n.InboundsAdoptedAt == 0 { + if markErr := j.nodeService.MarkNodeInboundsAdopted(n.Id); markErr != nil { + logger.Warningf("node traffic sync: mark inbounds adopted for %s failed: %v", n.Name, markErr) + } + } active := make([]string, 0, len(snap.OnlineEmails)) active = append(active, snap.OnlineEmails...) diff --git a/internal/web/service/inbound_node.go b/internal/web/service/inbound_node.go index b896f1d55..0a441fec6 100644 --- a/internal/web/service/inbound_node.go +++ b/internal/web/service/inbound_node.go @@ -129,6 +129,11 @@ func (s *InboundService) ReconcileNode(ctx context.Context, rt *runtime.Remote, errs = append(errs, fmt.Errorf("reconcile inbound %q: %w", ib.Tag, err)) } } + // Before the first clean sync adopts the node's inbounds, "absent locally" + // means "not imported yet" — sweeping now would wipe the node at onboarding. + if n.InboundsAdoptedAt == 0 { + return errors.Join(errs...) + } // In "selected" sync mode the panel only manages the selected tags: the // rest were never imported, so their absence from the local DB must not // delete them from the node. Only a selected tag missing locally (the diff --git a/internal/web/service/inbound_node_reconcile_test.go b/internal/web/service/inbound_node_reconcile_test.go index 6f99c21e6..337094949 100644 --- a/internal/web/service/inbound_node_reconcile_test.go +++ b/internal/web/service/inbound_node_reconcile_test.go @@ -87,6 +87,7 @@ func reconcileTestNode(t *testing.T, ts *httptest.Server, name, mode string, tag Status: "online", InboundSyncMode: mode, InboundTags: tags, + InboundsAdoptedAt: 1, } if err := database.GetDB().Create(n).Error; err != nil { t.Fatalf("create node: %v", err) @@ -143,6 +144,31 @@ func TestReconcileNode_AllModeDeletesUndesiredRemoteInbounds(t *testing.T) { } } +// A node whose pre-existing inbounds were never adopted into the central DB +// has zero local rows for legitimate reasons: reconcile before that first +// adoption must not sweep — it would delete every real inbound on the node +// right after onboarding (add node, save it again, watch it get wiped). +func TestReconcileNode_SkipsSweepBeforeFirstAdoption(t *testing.T) { + setupConflictDB(t) + + ts, deletedIDs := fakeNodePanel(t, map[string]int{ + "real-a": 1, + "real-b": 2, + "real-c": 3, + }) + node := reconcileTestNode(t, ts, "fresh-node", "all", nil) + node.InboundsAdoptedAt = 0 + + svc := InboundService{} + if err := svc.ReconcileNode(context.Background(), runtime.NewRemote(node, nil), node); err != nil { + t.Fatalf("ReconcileNode: %v", err) + } + + if got := deletedIDs(); len(got) != 0 { + t.Fatalf("deleted remote ids = %v, want none before first adoption", got) + } +} + // One inbound the node rejects (e.g. a legacy protocol failing the node's // request validation, #5685) must not abort the reconcile: the healthy inbound // is still pushed, the delete sweep still runs, and the returned error names diff --git a/internal/web/service/node.go b/internal/web/service/node.go index f2e6e61a6..2d59d0e9e 100644 --- a/internal/web/service/node.go +++ b/internal/web/service/node.go @@ -771,6 +771,15 @@ func (s *NodeService) ClearNodeDirty(id int, dirtyAt int64) error { Update("config_dirty", false).Error } +func (s *NodeService) MarkNodeInboundsAdopted(id int) error { + if id <= 0 { + return nil + } + return database.GetDB().Model(model.Node{}). + Where("id = ? AND inbounds_adopted_at = 0", id). + Update("inbounds_adopted_at", time.Now().Unix()).Error +} + func (s *NodeService) NodeSyncState(id int) (enabled bool, status string, dirty bool, dirtyAt int64, err error) { if id <= 0 { return false, "", false, 0, errors.New("invalid node id")