mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-06-30 09:34:21 +00:00
fix(nodes): block node delete while inbounds are still attached (#5394)
NodeService.Delete dropped the node row (and its per-node child rows) without checking for inbounds still referencing it via node_id, leaving orphaned inbounds with a dangling node_id that confuse node sync, subscriptions and cleanup. Refuse the delete with a clear error when inbounds are still attached, and remove the per-node child rows before the node row inside one transaction. Delete stays tolerant of a missing node row so it can still clean up orphaned rows. Regression test covers the blocked and clean-delete paths.
This commit is contained in:
@@ -24,6 +24,8 @@ import (
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/util/netsafe"
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/web/runtime"
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/xray"
|
||||
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
type HeartbeatPatch struct {
|
||||
@@ -439,6 +441,17 @@ func FilterNodeSnapshot(n *model.Node, snap *runtime.TrafficSnapshot) {
|
||||
|
||||
func (s *NodeService) Delete(id int) error {
|
||||
db := database.GetDB()
|
||||
// Refuse to delete a node that still owns inbounds: dropping the node row
|
||||
// while inbounds keep its node_id leaves orphaned, dangling references that
|
||||
// confuse node sync, subscriptions and cleanup. The operator must detach or
|
||||
// remove those inbounds first. (DB-002)
|
||||
var attached int64
|
||||
if err := db.Model(&model.Inbound{}).Where("node_id = ?", id).Count(&attached).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
if attached > 0 {
|
||||
return common.NewError(fmt.Sprintf("cannot delete node: %d inbound(s) still attached to it; detach or delete them first", attached))
|
||||
}
|
||||
// Capture the node's guid before deleting the row so we can drop its per-node
|
||||
// IP attribution (NodeClientIp is keyed by guid, not node id).
|
||||
var guid string
|
||||
@@ -446,16 +459,22 @@ func (s *NodeService) Delete(id int) error {
|
||||
if err := db.Select("guid").Where("id = ?", id).First(&n).Error; err == nil {
|
||||
guid = n.Guid
|
||||
}
|
||||
if err := db.Where("id = ?", id).Delete(model.Node{}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
if err := db.Where("node_id = ?", id).Delete(&model.NodeClientTraffic{}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
if guid != "" {
|
||||
if err := db.Where("node_guid = ?", guid).Delete(&model.NodeClientIp{}).Error; err != nil {
|
||||
// Delete the node row and its per-node child rows atomically. Remove the
|
||||
// children (traffic baselines, IP attribution) before the parent node row so
|
||||
// the ordering already matches a future ON DELETE constraint. Delete stays
|
||||
// tolerant of a missing node row so it can still clean up orphaned baselines.
|
||||
if err := db.Transaction(func(tx *gorm.DB) error {
|
||||
if err := tx.Where("node_id = ?", id).Delete(&model.NodeClientTraffic{}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
if guid != "" {
|
||||
if err := tx.Where("node_guid = ?", guid).Delete(&model.NodeClientIp{}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return tx.Where("id = ?", id).Delete(&model.Node{}).Error
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
if mgr := runtime.GetManager(); mgr != nil {
|
||||
mgr.InvalidateNode(id)
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
||||
)
|
||||
|
||||
// TestNodeDelete_BlocksWhenInboundsAttached guards DB-002: a node that still
|
||||
// owns inbounds must not be deletable (which would orphan those inbounds with a
|
||||
// dangling node_id), while a node with none deletes cleanly together with its
|
||||
// traffic baselines.
|
||||
func TestNodeDelete_BlocksWhenInboundsAttached(t *testing.T) {
|
||||
db := initTrafficTestDB(t)
|
||||
svc := &NodeService{}
|
||||
|
||||
node := &model.Node{Name: "n1"}
|
||||
if err := db.Create(node).Error; err != nil {
|
||||
t.Fatalf("create node: %v", err)
|
||||
}
|
||||
createNodeInbound(t, db, node.Id, "n1-in-443", 443)
|
||||
|
||||
// With an inbound attached, Delete must fail and leave node + inbound intact.
|
||||
if err := svc.Delete(node.Id); err == nil {
|
||||
t.Fatal("Delete should fail while an inbound is still attached")
|
||||
}
|
||||
var nodeCnt, ibCnt int64
|
||||
db.Model(&model.Node{}).Where("id = ?", node.Id).Count(&nodeCnt)
|
||||
db.Model(&model.Inbound{}).Where("node_id = ?", node.Id).Count(&ibCnt)
|
||||
if nodeCnt != 1 || ibCnt != 1 {
|
||||
t.Fatalf("after blocked delete: node=%d inbound=%d, want 1/1", nodeCnt, ibCnt)
|
||||
}
|
||||
|
||||
// Detach the inbound and seed a traffic baseline; Delete now succeeds and
|
||||
// cleans the baseline.
|
||||
if err := db.Where("node_id = ?", node.Id).Delete(&model.Inbound{}).Error; err != nil {
|
||||
t.Fatalf("detach inbound: %v", err)
|
||||
}
|
||||
if err := db.Create(&model.NodeClientTraffic{NodeId: node.Id, Email: "gone"}).Error; err != nil {
|
||||
t.Fatalf("seed baseline: %v", err)
|
||||
}
|
||||
if err := svc.Delete(node.Id); err != nil {
|
||||
t.Fatalf("Delete (no inbounds attached): %v", err)
|
||||
}
|
||||
var baseCnt int64
|
||||
db.Model(&model.Node{}).Where("id = ?", node.Id).Count(&nodeCnt)
|
||||
db.Model(&model.NodeClientTraffic{}).Where("node_id = ?", node.Id).Count(&baseCnt)
|
||||
if nodeCnt != 0 || baseCnt != 0 {
|
||||
t.Fatalf("after delete: node=%d baseline=%d, want 0/0", nodeCnt, baseCnt)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user