Skip to content

IFC-2555: Fix prefix pool duplication#9299

Open
solababs wants to merge 2 commits into
stablefrom
fix-prefix-pool-dup-ifc-2555
Open

IFC-2555: Fix prefix pool duplication#9299
solababs wants to merge 2 commits into
stablefrom
fix-prefix-pool-dup-ifc-2555

Conversation

@solababs
Copy link
Copy Markdown
Contributor

@solababs solababs commented May 20, 2026

Why

When a prefix pool's resource prefix has the same length as the requested allocation size (e.g., allocating /30 subnets from a /30 parent), the pool returns the same prefix on every request instead of signalling exhaustion. This causes generators to silently produce duplicate IP allocations, resulting in routing failures.

Root cause: IPPrefixSubnetFetchFree and IPv6PrefixSubnetFetchFree filter occupied child ranges with av.prefixlen > $maxprefixlen (strict greater-than). When the allocated child has prefixlen == parent.prefixlen, the condition 30 > 30 is false — the child is invisible to subsequent queries, which then return the same free_start again.

Closes opsmill/infrahub-sdk-python#981

How to test

# New edge-case tests (reproduce the bug before the fix)
uv run pytest backend/tests/component/core/resource_manager/test_prefix_pool.py \
    -k "same_prefixlen" -v

Checklist

  • Tests added/updated
  • Changelog entry added (changelog/+fix-prefix-pool-dup.fixed.md)
  • External docs updated (if user-facing or ops-facing change)
  • Internal .md docs updated (internal knowledge and AI code tools knowledge)
  • I have reviewed AI generated content

Summary by cubic

Fixes duplicate allocations from prefix pools when the pool resource has the same prefix length as the requested size. Pools now allocate once and then correctly signal exhaustion (IPv4 and IPv6), addressing IFC-2555’s “no more resource available” expectation.

  • Bug Fixes
    • Count occupied ranges where prefixlen >= maxprefixlen (was >), for IPv4 and IPv6 queries.
    • Exclude the parent pool resource itself from occupied results via pfx.uuid <> $exclude_uuid.
    • Pass parent_uuid through the allocator to the IPAM queries.
    • Added tests for /30-from-/30, /127-from-/127, and mixed-resource allocation/exhaustion.

Written for commit a27694a. Summary will update on new commits. Review in cubic

@solababs solababs requested a review from a team as a code owner May 20, 2026 06:38
@github-actions github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label May 20, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would require human review. This PR modifies core IPAM allocation logic and database queries (changing comparison operators and adding uuid filters), which has a high blast radius on network prefix allocation correctness; despite comprehensive tests, such critical business logic changes require a human reviewer's judgment.

Re-trigger cubic

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 20, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing fix-prefix-pool-dup-ifc-2555 (a27694a) with stable (72c3033)

Open in CodSpeed

Comment thread backend/infrahub/core/query/ipam.py
self.obj = obj
self.target_prefixlen = target_prefixlen
self.namespace_id = _get_namespace_id(namespace)
self.parent_uuid = parent_uuid
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to use obj instead of adding the parent_uuid parameter to do the same thing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this tests locally that passed and I think it would be worth adding, or maybe replacing the tests with it

  • start with a resource pool that includes two /30 prefixes. provision a /31 (success), provision a /30 (success), provision another /30 (failure), provision another /31 (success)

@solababs solababs requested a review from ajtmccarty May 25, 2026 10:31
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Shadow auto-approve: would require human review. This change modifies a core IPAM query condition (av.prefixlen > to >=) and adds an exclusion filter, which affects prefix pool allocation logic for both IPv4 and IPv6; while the fix is well-tested, it touches critical business logic where a subtle bug could cause data integrity issues, so...

Re-trigger cubic

Comment on lines +336 to +337
# obj is an IPNetworkType (Python network object); parent_uuid is the database UUID of
# the pool-resource node. They represent different things and cannot substitute each other.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is necessary

target_prefixlen: int,
namespace: Node | str | None = None,
# obj is an IPNetworkType (Python network object); parent_uuid is the database UUID of
# the pool-resource node. They represent different things and cannot substitute each other.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: prefix pool assigning same prefix multiple times

2 participants