mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-12 17:59:06 +02:00
[management] Prevent deleting groups referenced by agent network budget rules (#7450)
`validateDeleteGroup` already refuses to delete a group that is still used by routes, policies, nameservers, setup keys, users, network routers, reverse proxy services, and agent network policies. Account-level agent network budget rules also store group IDs in `TargetGroups`, but that check was missing. Deleting such a group left a dangling ID on the budget rule. `budgetRuleApplies` then never matched callers by group, so the spend cap silently stopped applying. This adds `isGroupLinkedToAgentNetworkBudgetRule` and uses it in `validateDeleteGroup`, matching the existing helpers.
This commit is contained in:
@@ -774,6 +774,14 @@ func validateDeleteGroup(ctx context.Context, transaction store.Store, group *ty
|
||||
return &GroupLinkError{"agent network policy", linkedPolicy.Name}
|
||||
}
|
||||
|
||||
isLinked, linkedRule, err := isGroupLinkedToAgentNetworkBudgetRule(ctx, transaction, group.AccountID, group.ID)
|
||||
if err != nil {
|
||||
return status.Errorf(status.Internal, "failed to check agent network budget rules")
|
||||
}
|
||||
if isLinked {
|
||||
return &GroupLinkError{"agent network budget rule", linkedRule.Name}
|
||||
}
|
||||
|
||||
return checkGroupLinkedToSettings(ctx, transaction, group)
|
||||
}
|
||||
|
||||
@@ -945,6 +953,26 @@ func isGroupLinkedToAgentNetworkPolicy(ctx context.Context, transaction store.St
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// isGroupLinkedToAgentNetworkBudgetRule checks if a group is a target of any
|
||||
// account-level agent network budget rule.
|
||||
func isGroupLinkedToAgentNetworkBudgetRule(ctx context.Context, transaction store.Store, accountID string, groupID string) (bool, *agentNetworkTypes.AccountBudgetRule, error) {
|
||||
rules, err := transaction.GetAccountAgentNetworkBudgetRules(ctx, store.LockingStrengthNone, accountID)
|
||||
if err != nil {
|
||||
log.WithContext(ctx).Errorf("error retrieving agent network budget rules while checking group linkage: %v", err)
|
||||
return false, nil, err
|
||||
}
|
||||
|
||||
for _, rule := range rules {
|
||||
if rule == nil {
|
||||
continue
|
||||
}
|
||||
if slices.Contains(rule.TargetGroups, groupID) {
|
||||
return true, rule, nil
|
||||
}
|
||||
}
|
||||
return false, nil, nil
|
||||
}
|
||||
|
||||
// areGroupChangesAffectPeers checks if any changes to the specified groups will affect peers.
|
||||
// It fetches each collection once and checks all groupIDs against them in memory.
|
||||
func areGroupChangesAffectPeers(ctx context.Context, transaction store.Store, accountID string, groupIDs []string) (bool, error) {
|
||||
|
||||
@@ -132,6 +132,11 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
|
||||
"grp-for-agent-network-policy",
|
||||
"agent network policy",
|
||||
},
|
||||
{
|
||||
"agent network budget rule",
|
||||
"grp-for-agent-network-budget-rule",
|
||||
"agent network budget rule",
|
||||
},
|
||||
{
|
||||
"reverse proxy private service access group",
|
||||
"grp-for-rp-private",
|
||||
@@ -152,6 +157,16 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
|
||||
return
|
||||
}
|
||||
|
||||
group, getErr := am.GetGroup(context.Background(), account.Id, testCase.groupID, groupAdminUserID)
|
||||
if getErr != nil {
|
||||
t.Errorf("group %s should still exist after failed deletion: %s", testCase.groupID, getErr)
|
||||
return
|
||||
}
|
||||
if group == nil {
|
||||
t.Errorf("group %s was deleted despite the failed deletion", testCase.groupID)
|
||||
return
|
||||
}
|
||||
|
||||
var sErr *status.Error
|
||||
if errors.As(err, &sErr) {
|
||||
if sErr.Message != testCase.expectedReason {
|
||||
@@ -240,6 +255,12 @@ func TestDefaultAccountManager_DeleteGroups(t *testing.T) {
|
||||
groupIDs: []string{"grp-for-agent-network-policy"},
|
||||
expectedReasons: []string{"agent network policy"},
|
||||
},
|
||||
{
|
||||
name: "agent network budget rule",
|
||||
groupIDs: []string{"grp-for-agent-network-budget-rule"},
|
||||
expectedReasons: []string{"agent network budget rule"},
|
||||
expectedNotDeleted: []string{"grp-for-agent-network-budget-rule"},
|
||||
},
|
||||
{
|
||||
name: "reverse proxy services",
|
||||
groupIDs: []string{"grp-for-rp-private", "grp-for-rp-bearer"},
|
||||
@@ -501,6 +522,14 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
|
||||
Peers: make([]string, 0),
|
||||
}
|
||||
|
||||
groupForAgentNetworkBudgetRule := &types.Group{
|
||||
ID: "grp-for-agent-network-budget-rule",
|
||||
AccountID: "account-id",
|
||||
Name: "Group for agent network budget rules",
|
||||
Issued: types.GroupIssuedAPI,
|
||||
Peers: make([]string, 0),
|
||||
}
|
||||
|
||||
groupForRPPrivate := &types.Group{
|
||||
ID: "grp-for-rp-private",
|
||||
AccountID: "account-id",
|
||||
@@ -573,6 +602,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
|
||||
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForUsers)
|
||||
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration)
|
||||
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkPolicy)
|
||||
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkBudgetRule)
|
||||
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForRPPrivate)
|
||||
_ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForRPBearer)
|
||||
|
||||
@@ -587,6 +617,20 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
budgetRuleDecoy := agentNetworkTypes.NewAccountBudgetRule(accountID)
|
||||
budgetRuleDecoy.Name = "Unrelated agent network budget rule"
|
||||
budgetRuleDecoy.TargetGroups = []string{"unrelated-group"}
|
||||
if err := am.Store.SaveAgentNetworkBudgetRule(context.Background(), budgetRuleDecoy); err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
budgetRule := agentNetworkTypes.NewAccountBudgetRule(accountID)
|
||||
budgetRule.Name = "Example agent network budget rule"
|
||||
budgetRule.TargetGroups = []string{groupForAgentNetworkBudgetRule.ID}
|
||||
if err := am.Store.SaveAgentNetworkBudgetRule(context.Background(), budgetRule); err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
// The decoy services are created first so the linkage check has to scan
|
||||
// past services that do not reference the groups under test.
|
||||
rpServices := []*rpservice.Service{
|
||||
|
||||
Reference in New Issue
Block a user