Fix IP Allocation Bug: Reserved Range Not Detected

[neddp]

What is this change about?

This PR fixes a bug where BOSH allocates IPs from reserved CIDR ranges, causing CPI failures with "Address is in subnet's reserved address range" errors.

Root Cause: The overlap check in find_next_available_ip was unidirectional. It only checked if the candidate IP includes a blocking IP, but not if a blocking IP includes the candidate.

When allocating a /32 (single IP) and the reserved range is a /30 (4 IPs), the check candidate.include?(blocking) always returns false because a smaller block cannot include a larger one. The reserved range was invisible to the algorithm.

The Bug

# Original (buggy) - only checks one direction
if filtered_ips.any? { |ip| current_prefix.include?(ip) }

Example:

• Candidate: 10.0.11.32/32
• Reserved: 10.0.11.32/30
• Check: 10.0.11.32/32.include?(10.0.11.32/30) → false
• Result: Algorithm thinks IP is available → CPI rejects it

The Fix

# Fixed - checks both directions
blocking_ip = filtered_ips.find do |ip|
  current_prefix.include?(ip) || ip.include?(current_prefix)
end

Now 10.0.11.32/30.include?(10.0.11.32/32) → true → correctly blocked.

Additional Improvement: Deterministic Deduplication

The deduplication logic that removes redundant IPs (e.g., /32 entries already covered by a /30 block) has been refactored to sort IPs before processing. This ensures deterministic behavior regardless of Set iteration order, making the code easier to test and debug.

What tests have you run?

• All existing unit tests pass
• Added edge case tests for overlapping CIDR blocks with different prefix sizes

Release notes

Fixed: IP allocation now correctly detects reserved CIDR ranges. Previously, when allocating single IPs (/32) from a subnet with reserved ranges specified as larger blocks (e.g., /30), the algorithm failed to detect the overlap and attempted to allocate reserved IPs, causing CPI errors.

Breaking change?

No. This is a bug fix.

[fmoehler]

Thanks for looking into this. Are you sure this is a real issue? I understand the issue from the description, but there were tests that already did cover the scenario in question e.g. in ip_repo_spec.rb:299 and they worked. I also checked out this branch, but used the logic from the current "main" branch in ip_repo.rb and all new tests were still green:

The scenario seems to be so basic that I think we should have stumbled across this before.

[fmoehler]

Why is this whole change needed? For performance reasons? Logic wise it seems to get to the same result as the previous code or am I wrong?

[neddp]

The idea of the sorting is to potentially avoid comparisons like this:

10.0.11.32/32.include?(10.0.11.32/30)

That is the main change along with the two way comparison further down:

blocking_ip = filtered_ips.find do |ip|
  current_prefix.include?(ip) || ip.include?(current_prefix)
end

I am not confident enough in the "fix" and we haven't been able to isolate the issue well enough. I will move the PR to draft for now.

[fmoehler]

Please see comment #2657 (comment)

[fmoehler]

Same as above. Why is this needed?

[neddp]

Moving to draft for the moment. Needs further attention.

[s4heid]

@neddp I think you discovered a legitimate bug.

We have noticed sporadic deployment failures of the following kind:

{
  "error":{
    "code":"PrivateIPAddressInReservedRange",
    "message":"Private static IP address 192.168.11.2 falls within reserved IP range of subnet prefix 192.168.11.0/24.",
    "details":[]
  }
}

The subnet prefix in this example was belonging to the following bosh network:

networks:
- name: compilation_network
  subnets:
  - az: z1
    cloud_properties:
      application_security_groups:
      - asg-bosh
      subnet_name: subnet-bosh-z1
      virtual_network_name: vnet-bosh
    dns:
    - 168.63.129.16
    gateway: 192.168.11.1
    range: 192.168.11.0/24
    reserved:
    - 192.168.11.0 - 192.168.11.141
    - 192.168.11.242 - 192.168.11.255

If you look at the first reserved range (192.168.11.0 - 192.168.11.141), you notice that something is off, because bosh attempted to take IP 192.168.11.2, which is part of the reserved range.

Do you plan to continue to work on this change?

[s4heid]

Perhaps, it would also make sense to add an integration / regression test to avoid catching something critical like this in the future again.

[s4heid]

this is very broad. perhaps IPAddr::InvalidAddressError would be sufficient to catch?

[neddp]

Hi @s4heid,

We plan on continuing to work on it, but any help is appreciated in the meantime.

Thanks!