From cb968db68a2b9c4139422313e8cb4812a63091d9 Mon Sep 17 00:00:00 2001 From: ionutboangiu Date: Tue, 1 Jul 2025 21:31:04 +0300 Subject: [PATCH] ips: keep IPProfile read-only during pool filtering --- ips/apis.go | 36 +++++++++++++++++++++----------- ips/ips.go | 59 ++++++++++++++++++++++++++++++----------------------- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/ips/apis.go b/ips/apis.go index 383043a01..62ba0891d 100644 --- a/ips/apis.go +++ b/ips/apis.go @@ -90,9 +90,9 @@ func (s *IPService) V1AuthorizeIP(ctx *context.Context, args *utils.CGREvent, re } var allocID string - if allocID, err = engine.GetStringOpts(ctx, args.Tenant, args.AsDataProvider(), nil, s.fltrs, s.cfg.IPsCfg().Opts.AllocationID, - utils.OptsIPsAllocationID); err != nil { - return + if allocID, err = engine.GetStringOpts(ctx, args.Tenant, args.AsDataProvider(), nil, s.fltrs, + s.cfg.IPsCfg().Opts.AllocationID, utils.OptsIPsAllocationID); err != nil { + return err } if allocID == utils.EmptyString { return utils.NewErrMandatoryIeMissing(utils.AllocationID) @@ -128,8 +128,14 @@ func (s *IPService) V1AuthorizeIP(ctx *context.Context, args *utils.CGREvent, re } defer allocs.Unlock() + var poolIDs []string + if poolIDs, err = filterAndSortPools(ctx, tnt, allocs.Config().Pools, s.fltrs, + args.AsDataProvider()); err != nil { + return err + } + var allocIP *utils.AllocatedIP - if allocIP, err = s.allocateFromPools(allocs, allocID, true); err != nil { + if allocIP, err = s.allocateFromPools(allocs, allocID, poolIDs, true); err != nil { if errors.Is(err, utils.ErrIPAlreadyAllocated) { return utils.ErrIPUnauthorized } @@ -137,7 +143,7 @@ func (s *IPService) V1AuthorizeIP(ctx *context.Context, args *utils.CGREvent, re } *reply = *allocIP - return + return nil } // V1AllocateIP allocates an IP address for the given event. @@ -152,7 +158,7 @@ func (s *IPService) V1AllocateIP(ctx *context.Context, args *utils.CGREvent, rep var allocID string if allocID, err = engine.GetStringOpts(ctx, args.Tenant, args.AsDataProvider(), nil, s.fltrs, s.cfg.IPsCfg().Opts.AllocationID, utils.OptsIPsAllocationID); err != nil { - return + return err } if allocID == utils.EmptyString { return utils.NewErrMandatoryIeMissing(utils.AllocationID) @@ -188,14 +194,20 @@ func (s *IPService) V1AllocateIP(ctx *context.Context, args *utils.CGREvent, rep } defer allocs.Unlock() + var poolIDs []string + if poolIDs, err = filterAndSortPools(ctx, tnt, allocs.Config().Pools, s.fltrs, + args.AsDataProvider()); err != nil { + return err + } + var allocIP *utils.AllocatedIP - if allocIP, err = s.allocateFromPools(allocs, allocID, false); err != nil { + if allocIP, err = s.allocateFromPools(allocs, allocID, poolIDs, false); err != nil { return err } // index it for storing if err = s.storeMatchedIPAllocations(ctx, allocs); err != nil { - return + return err } *reply = *allocIP return nil @@ -213,7 +225,7 @@ func (s *IPService) V1ReleaseIP(ctx *context.Context, args *utils.CGREvent, repl var allocID string if allocID, err = engine.GetStringOpts(ctx, args.Tenant, args.AsDataProvider(), nil, s.fltrs, s.cfg.IPsCfg().Opts.AllocationID, utils.OptsIPsAllocationID); err != nil { - return + return err } if allocID == utils.EmptyString { return utils.NewErrMandatoryIeMissing(utils.AllocationID) @@ -245,7 +257,7 @@ func (s *IPService) V1ReleaseIP(ctx *context.Context, args *utils.CGREvent, repl var allocs *utils.IPAllocations if allocs, err = s.matchingIPAllocationsForEvent(ctx, tnt, args, allocID); err != nil { - return + return err } defer allocs.Unlock() @@ -256,11 +268,11 @@ func (s *IPService) V1ReleaseIP(ctx *context.Context, args *utils.CGREvent, repl // Handle storing if err = s.storeMatchedIPAllocations(ctx, allocs); err != nil { - return + return err } *reply = utils.OK - return + return nil } // V1GetIPAllocations returns a resource configuration diff --git a/ips/ips.go b/ips/ips.go index 616db5ce3..e29ad539e 100644 --- a/ips/ips.go +++ b/ips/ips.go @@ -267,18 +267,10 @@ func (s *IPService) matchingIPAllocationsForEvent(ctx *context.Context, tnt stri return nil, err } allocs.Lock(lkID) - - // Clone profile to avoid modifying cached version during pool sorting. - prfl := matchedPrfl.Clone() - if err = filterAndSortPools(ctx, prfl, s.fltrs, evNm); err != nil { + if err = allocs.ComputeUnexported(matchedPrfl); err != nil { allocs.Unlock() return nil, err } - if err = allocs.ComputeUnexported(prfl); err != nil { - allocs.Unlock() - return nil, err - } - if err = engine.Cache.Set(ctx, utils.CacheEventIPs, evUUID, // TODO: check if we still should rely on caching previously matched @@ -296,9 +288,13 @@ func (s *IPService) matchingIPAllocationsForEvent(ctx *context.Context, tnt stri // Continues to next pool only if current pool returns ErrIPAlreadyAllocated. // Returns first successful allocation or the last allocation error. func (s *IPService) allocateFromPools(allocs *utils.IPAllocations, allocID string, - dryRun bool) (*utils.AllocatedIP, error) { + poolIDs []string, dryRun bool) (*utils.AllocatedIP, error) { var err error - for _, pool := range allocs.Config().Pools { + for _, poolID := range poolIDs { + pool := findPoolByID(allocs.Config().Pools, poolID) + if pool == nil { + return nil, fmt.Errorf("pool %q: %w", poolID, utils.ErrNotFound) + } var result *utils.AllocatedIP if result, err = allocs.AllocateIPOnPool(allocID, pool, dryRun); err == nil { return result, nil @@ -310,23 +306,34 @@ func (s *IPService) allocateFromPools(allocs *utils.IPAllocations, allocID strin return nil, err } -// filterAndSortPools filters pools by their FilterIDs, sorts by weight (highest first), -// and truncates at the first blocking pool. -func filterAndSortPools(ctx *context.Context, prfl *utils.IPProfile, fltrs *engine.FilterS, - ev utils.DataProvider) error { +func findPoolByID(pools []*utils.IPPool, id string) *utils.IPPool { + for _, pool := range pools { + if pool.ID == id { + return pool + } + } + return nil +} + +// filterAndSortPools filters pools by their FilterIDs, sorts by weight +// (highest first), and truncates at the first blocking pool. +// TODO: check whether pre-allocating filteredPools & poolIDs improves +// performance or wastes memory when filtering is aggressive. +func filterAndSortPools(ctx *context.Context, tenant string, pools []*utils.IPPool, + fltrs *engine.FilterS, ev utils.DataProvider) ([]string, error) { var filteredPools []*utils.IPPool weights := make(map[string]float64) // stores sorting weights by pool ID - for _, pool := range prfl.Pools { - pass, err := fltrs.Pass(ctx, prfl.Tenant, pool.FilterIDs, ev) + for _, pool := range pools { + pass, err := fltrs.Pass(ctx, tenant, pool.FilterIDs, ev) if err != nil { - return err + return nil, err } if !pass { continue } - weight, err := engine.WeightFromDynamics(ctx, pool.Weights, fltrs, prfl.Tenant, ev) + weight, err := engine.WeightFromDynamics(ctx, pool.Weights, fltrs, tenant, ev) if err != nil { - return err + return nil, err } weights[pool.ID] = weight filteredPools = append(filteredPools, pool) @@ -337,16 +344,16 @@ func filterAndSortPools(ctx *context.Context, prfl *utils.IPProfile, fltrs *engi return cmp.Compare(weights[b.ID], weights[a.ID]) }) - for i, pool := range filteredPools { - block, err := engine.BlockerFromDynamics(ctx, pool.Blockers, fltrs, prfl.Tenant, ev) + var poolIDs []string + for _, pool := range filteredPools { + block, err := engine.BlockerFromDynamics(ctx, pool.Blockers, fltrs, tenant, ev) if err != nil { - return err + return nil, err } + poolIDs = append(poolIDs, pool.ID) if block { - filteredPools = filteredPools[0 : i+1] break } } - prfl.Pools = filteredPools - return nil + return poolIDs, nil }