fix(inbounds): delete an inbound's dependent rows on every delete path

The node sync's sweep of an inbound the node stopped reporting deleted
the inbound but left its host rows, and the depleted-clients delete
left its sub-balancer references. SQLite hands a freed inbound id out
again once the table empties, so a node's host overrides could bind to
a new inbound it never hosted and point that inbound's subscribers at
an address, port and TLS settings the node chose.

delInbound's cleanup moves into deleteInboundRow (detach clients,
delete the inbound, its host rows and its sub-balancer references),
which the sweep and the depleted-clients delete now call as well.
This commit is contained in:
MHSanaei
2026-11-01 00:36:45 +01:00
parent 776ca554bd
commit 152c84efa2
5 changed files with 93 additions and 35 deletions
+32 -23
View File
@@ -1689,31 +1689,9 @@ func (s *InboundService) delInbound(id int) (bool, func(), error) {
}
if err := runSerializedTx(func(tx *gorm.DB) error {
if err := s.clientService.DetachInbound(tx, id); err != nil {
if err := s.deleteInboundRow(tx, id); err != nil {
return err
}
if err := tx.Delete(model.Inbound{}, id).Error; err != nil {
return err
}
if _, err := database.DeleteHosts(tx, "inbound_id = ?", id); err != nil {
return err
}
// Drop the deleted inbound from any sub-balancer that selects it; a
// dangling id would emit a member no subscriber can resolve (#5648).
var balancers []model.SubBalancer
if err := tx.Find(&balancers).Error; err != nil {
return err
}
for i := range balancers {
before := balancers[i].InboundIds
balancers[i].InboundIds = slices.DeleteFunc(before, func(b int) bool { return b == id })
if len(balancers[i].InboundIds) == len(before) {
continue
}
if err := tx.Save(&balancers[i]).Error; err != nil {
return err
}
}
if loadErr == nil && ib.NodeID != nil {
return (&NodeService{}).MarkNodeDirtyTx(tx, *ib.NodeID)
}
@@ -1749,6 +1727,37 @@ func (s *InboundService) delInbound(id int) (bool, func(), error) {
return needRestart, nodePush, nil
}
// deleteInboundRow removes an inbound and every row keyed by its id; SQLite
// hands a freed id out again, so anything left behind binds to the next inbound.
func (s *InboundService) deleteInboundRow(tx *gorm.DB, id int) error {
if err := s.clientService.DetachInbound(tx, id); err != nil {
return err
}
if err := tx.Delete(model.Inbound{}, id).Error; err != nil {
return err
}
if _, err := database.DeleteHosts(tx, "inbound_id = ?", id); err != nil {
return err
}
// Drop the deleted inbound from any sub-balancer that selects it; a
// dangling id would emit a member no subscriber can resolve (#5648).
var balancers []model.SubBalancer
if err := tx.Find(&balancers).Error; err != nil {
return err
}
for i := range balancers {
before := balancers[i].InboundIds
balancers[i].InboundIds = slices.DeleteFunc(before, func(b int) bool { return b == id })
if len(balancers[i].InboundIds) == len(before) {
continue
}
if err := tx.Save(&balancers[i]).Error; err != nil {
return err
}
}
return nil
}
type BulkDelInboundResult struct {
Deleted int `json:"deleted"`
Skipped []BulkDelInboundReport `json:"skipped,omitempty"`
+1 -5
View File
@@ -1010,11 +1010,7 @@ func (s *InboundService) setRemoteTrafficLocked(nodeID int, snap *runtime.Traffi
}
}
}
if err := s.clientService.DetachInbound(tx, c.Id); err != nil {
return false, err
}
if err := tx.Where("id = ?", c.Id).
Delete(&model.Inbound{}).Error; err != nil {
if err := s.deleteInboundRow(tx, c.Id); err != nil {
return false, err
}
delete(tagToCentral, c.Tag)
+1 -7
View File
@@ -961,13 +961,7 @@ func (s *InboundService) DelDepletedClients(id int) (err error) {
}
if len(newClients) == 0 {
deletedInbounds = append(deletedInbounds, *inbound)
if err := s.clientService.DetachInbound(tx, inbound.Id); err != nil {
return err
}
if _, err := database.DeleteHosts(tx, "inbound_id = ?", inbound.Id); err != nil {
return err
}
if err := tx.Delete(model.Inbound{}, inbound.Id).Error; err != nil {
if err := s.deleteInboundRow(tx, inbound.Id); err != nil {
return err
}
if inbound.NodeID != nil {
@@ -75,3 +75,52 @@ func TestSetRemoteTraffic_AdoptsNodeHostRows(t *testing.T) {
t.Fatalf("total host rows = %d, want 1 (group member for un-adopted node inbound 99 must not materialize)", total)
}
}
func TestSetRemoteTraffic_SweptInboundTakesItsDependentsAlong(t *testing.T) {
setupConflictDB(t)
db := database.GetDB()
const nodeID = 6
if err := db.Create(&model.Node{Id: nodeID, Name: "host-node", Address: "10.0.0.6", Port: 2053, ApiToken: "t", Guid: "host-node-guid"}).Error; err != nil {
t.Fatalf("create node: %v", err)
}
inbound := func(id, port int, tag string) *model.Inbound {
return &model.Inbound{Id: id, Tag: tag, Enable: true, Port: port, Protocol: model.VLESS, Settings: `{"clients":[]}`}
}
group := func(groupId string, inboundId int) *entity.HostGroup {
return &entity.HostGroup{GroupId: groupId, InboundIds: []int{inboundId}, Hosts: []string{"cdn.example.com:8443"}, Remark: "cdn"}
}
svc := InboundService{}
if _, err := svc.setRemoteTrafficLocked(nodeID, &runtime.TrafficSnapshot{
Inbounds: []*model.Inbound{inbound(1, 443, "swept-443"), inbound(2, 444, "kept-444")},
HostGroups: []*entity.HostGroup{group("g-swept", 1), group("g-kept", 2)},
}, false, false); err != nil {
t.Fatalf("adopting merge: %v", err)
}
swept, kept := loadInboundByTag(t, "swept-443"), loadInboundByTag(t, "kept-444")
balancer := &model.SubBalancer{Remark: "both", InboundIds: []int{swept.Id, kept.Id}, Enabled: true}
if err := db.Create(balancer).Error; err != nil {
t.Fatalf("create balancer: %v", err)
}
if _, err := svc.setRemoteTrafficLocked(nodeID, &runtime.TrafficSnapshot{
Inbounds: []*model.Inbound{inbound(2, 444, "kept-444")},
}, false, false); err != nil {
t.Fatalf("sweeping merge: %v", err)
}
var groups []string
if err := db.Model(&model.Host{}).Order("group_id").Pluck("group_id", &groups).Error; err != nil {
t.Fatalf("load hosts: %v", err)
}
if len(groups) != 1 || groups[0] != "g-kept" {
t.Fatalf("host groups after the sweep = %v, want [g-kept]", groups)
}
if err := db.First(balancer, balancer.Id).Error; err != nil {
t.Fatalf("reload balancer: %v", err)
}
if len(balancer.InboundIds) != 1 || balancer.InboundIds[0] != kept.Id {
t.Fatalf("balancer members after the sweep = %v, want [%d]", balancer.InboundIds, kept.Id)
}
}
@@ -76,6 +76,10 @@ func TestDelDepletedClientsCleansRuntimeAfterCommit(t *testing.T) {
Updates(map[string]any{"up": 100, "total": 100, "reset": 0}).Error; err != nil {
t.Fatalf("deplete traffic: %v", err)
}
balancer := &model.SubBalancer{Remark: "depleted", InboundIds: []int{ib.Id}, Enabled: true}
if err := database.GetDB().Create(balancer).Error; err != nil {
t.Fatalf("create balancer: %v", err)
}
if err := (&InboundService{}).DelDepletedClients(-1); err != nil {
t.Fatalf("DelDepletedClients: %v", err)
@@ -83,4 +87,10 @@ func TestDelDepletedClientsCleansRuntimeAfterCommit(t *testing.T) {
if got := fake.delInbound.Load(); got != 1 {
t.Fatalf("runtime DelInbound calls = %d, want 1 after commit", got)
}
if err := database.GetDB().First(balancer, balancer.Id).Error; err != nil {
t.Fatalf("reload balancer: %v", err)
}
if len(balancer.InboundIds) != 0 {
t.Fatalf("balancer members after the inbound went = %v, want none", balancer.InboundIds)
}
}