diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 7e03df39c6..37e72e9aa8 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -96,17 +96,22 @@ def try_to_allocate_dynamic_ip(reservation, subnet) addresses_we_cant_allocate.delete_if { |ipaddr| ipaddr.ipv6? } end - addresses_we_cant_allocate.reject! do |ip| - addresses_we_cant_allocate.any? do |other_ip| - includes = other_ip.include?(ip) - includes && other_ip.prefix < ip.prefix + # Sort by address first, then by prefix (smaller prefix = larger block = earlier) + sorted_ips = addresses_we_cant_allocate.sort_by { |ip| [ip.to_i, ip.prefix] } + + # Remove IPs contained within larger CIDR blocks + sorted_ips = sorted_ips.reject.with_index do |ip, index| + sorted_ips[0...index].any? do |other_ip| + other_ip.prefix < ip.prefix && other_ip.include?(ip) + rescue StandardError + false end end - ip_address_cidr = find_next_available_ip(addresses_we_cant_allocate, first_range_address, subnet.prefix) + ip_address_cidr = find_next_available_ip(sorted_ips, first_range_address, subnet.prefix) - if !(subnet.range == ip_address_cidr || subnet.range.include?(ip_address_cidr)) - raise NoMoreIPsAvailableAndStopRetrying + unless subnet.range == ip_address_cidr || subnet.range.include?(ip_address_cidr) + raise NoMoreIPsAvailableAndStopRetrying end save_ip(ip_address_cidr, reservation, false) @@ -114,31 +119,34 @@ def try_to_allocate_dynamic_ip(reservation, subnet) ip_address_cidr end - def find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix) + def find_next_available_ip(sorted_blocking_ips, first_range_address, prefix) # Remove IPs that are below subnet range - filtered_ips = addresses_we_cant_allocate.sort_by { |ip| ip.to_i }.reject { |ip| ip.to_i < first_range_address.to_i } + filtered_ips = sorted_blocking_ips.reject { |ip| ip.to_i < first_range_address.to_i } current_ip = to_ipaddr(first_range_address.to_i + 1) - found = false - while found == false + loop do current_prefix = to_ipaddr("#{current_ip.base_addr}/#{prefix}") - if filtered_ips.any? { |ip| current_prefix.include?(ip) } - filtered_ips.reject! { |ip| ip.to_i < current_prefix.to_i } - actual_ip_prefix = filtered_ips.first.count - if actual_ip_prefix > current_prefix.count - current_ip = to_ipaddr(current_ip.to_i + actual_ip_prefix) - else - current_ip = to_ipaddr(current_ip.to_i + current_prefix.count) - end + # Check both directions for overlap: candidate includes blocking IP, or blocking IP includes candidate + blocking_ip = filtered_ips.find do |ip| + (current_prefix.include?(ip) rescue false) || + (ip.include?(current_prefix) rescue false) + end + + return current_prefix if blocking_ip.nil? + + if blocking_ip.count > current_prefix.count + # Blocking range is larger, skip past its entire range + current_ip = to_ipaddr(blocking_ip.to_i + blocking_ip.count) else - found_cidr = current_prefix - found = true + # Blocking IP is smaller or same size, try next aligned position + current_ip = to_ipaddr(current_prefix.to_i + current_prefix.count) end - end - found_cidr + # Clean up blocking IPs that we've passed + filtered_ips.reject! { |ip| ip.to_i + ip.count < current_ip.to_i } + end end def try_to_allocate_vip_ip(reservation, subnet) diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb index 52e2924bcd..51d4b3b62d 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/manual_network_subnet.rb @@ -69,13 +69,18 @@ def self.parse(network_name, subnet_spec, availability_zones, managed = false) end end - restricted_ips.reject! do |ip| - restricted_ips.any? do |other_ip| - includes = other_ip.include?(ip) - includes && other_ip.prefix < ip.prefix + sorted_restricted_ips = restricted_ips.to_a.sort_by { |ip| [ip.to_i, ip.prefix] } + + deduplicated_ips = sorted_restricted_ips.reject.with_index do |ip, index| + sorted_restricted_ips[0...index].any? do |other_ip| + other_ip.prefix < ip.prefix && other_ip.include?(ip) + rescue StandardError + false end end + restricted_ips.replace(deduplicated_ips) + each_ip(static_property, false) do |ip| if ip_in_array?(ip, restricted_ips) raise NetworkStaticIpOutOfRange, "Static IP '#{ip}' is in network '#{network_name}' reserved range" diff --git a/src/bosh-director/spec/unit/bosh/director/deployment_plan/ip_provider/ip_repo_spec.rb b/src/bosh-director/spec/unit/bosh/director/deployment_plan/ip_provider/ip_repo_spec.rb index f35f026c19..df7b30e1b1 100644 --- a/src/bosh-director/spec/unit/bosh/director/deployment_plan/ip_provider/ip_repo_spec.rb +++ b/src/bosh-director/spec/unit/bosh/director/deployment_plan/ip_provider/ip_repo_spec.rb @@ -492,6 +492,121 @@ def fail_saving_ips(ips, fail_error) it_behaves_like :retries_on_race_condition end end + + context 'when handling CIDR blocks and overlapping ranges' do + def save_ip_string(ip_string) + ip_addr = to_ipaddr(ip_string) + Bosh::Director::Models::IpAddress.new( + address_str: ip_addr.to_s, + network_name: 'my-manual-network', + instance: instance_model, + task_id: Bosh::Director::Config.current_job.task_id + ).save + end + + context 'when database has individual IPs that are contained in a reserved CIDR block' do + it 'deduplicates and skips the entire CIDR block' do + network_spec['subnets'].first['range'] = '10.0.11.32/27' + network_spec['subnets'].first['gateway'] = '10.0.11.33' + network_spec['subnets'].first['reserved'] = ['10.0.11.32 - 10.0.11.35', '10.0.11.63'] + network_spec['subnets'].first['static'] = ['10.0.11.36', '10.0.11.37', '10.0.11.38', '10.0.11.39', '10.0.11.40'] + + # Simulate database IPs that overlap with reserved range + save_ip_string('10.0.11.32/32') + save_ip_string('10.0.11.33/32') + + # Should skip entire /30 block (32-35) and statics (36-40), allocate 41 + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet) + expect(ip_address).to eq(cidr_ip('10.0.11.41')) + end + end + + context 'when multiple overlapping CIDR blocks exist' do + it 'deduplicates to largest block only' do + network_spec['subnets'].first['range'] = '192.168.1.0/24' + network_spec['subnets'].first['gateway'] = '192.168.1.1' + network_spec['subnets'].first['reserved'] = [ + '192.168.1.0 - 192.168.1.15', # /28 + '192.168.1.0 - 192.168.1.3', # /30 + '192.168.1.4 - 192.168.1.7', # /30 + '192.168.1.8', # /32 + ] + + # Should skip entire /28 block (0-15), allocate 16 + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet) + expect(ip_address).to eq(cidr_ip('192.168.1.16')) + end + end + + context 'when nested CIDR blocks exist' do + it 'deduplicates to outermost block' do + network_spec['subnets'].first['range'] = '192.168.1.0/24' + network_spec['subnets'].first['gateway'] = '192.168.1.1' + network_spec['subnets'].first['reserved'] = [ + '192.168.1.0/24', # Entire range + '192.168.1.0/26', # First quarter + '192.168.1.0/28', # First 16 + ] + + # Should skip entire /24 block + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet) + expect(ip_address).to be_nil + end + end + + context 'when adjacent non-overlapping CIDR blocks exist' do + it 'preserves all blocks and skips each correctly' do + network_spec['subnets'].first['range'] = '10.0.0.0/24' + network_spec['subnets'].first['gateway'] = '10.0.0.1' + network_spec['subnets'].first['reserved'] = [ + '10.0.0.0 - 10.0.0.3', # /30 (0-3) + '10.0.0.4 - 10.0.0.7', # /30 (4-7) + '10.0.0.8 - 10.0.0.11', # /30 (8-11) + ] + + # Should skip all three /30 blocks, allocate 12 + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet) + expect(ip_address).to eq(cidr_ip('10.0.0.12')) + end + end + + context 'when large CIDR block contains scattered individual IPs' do + it 'deduplicates scattered IPs within the block' do + network_spec['subnets'].first['range'] = '10.1.1.0/24' + network_spec['subnets'].first['gateway'] = '10.1.1.1' + network_spec['subnets'].first['reserved'] = ['10.1.1.0/24'] + network_spec['subnets'].first['static'] = [] + + # Save individual IPs that are all within the /24 + save_ip_string('10.1.1.5/32') + save_ip_string('10.1.1.50/32') + save_ip_string('10.1.1.100/32') + save_ip_string('10.1.1.200/32') + + # Should skip entire /24, no IPs available + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet) + expect(ip_address).to be_nil + end + end + + context 'when handling AWS reserved IP ranges' do + it 'correctly skips reserved ranges with database IPs' do + network_spec['subnets'].first['range'] = '10.0.11.32/27' + network_spec['subnets'].first['gateway'] = '10.0.11.33' + network_spec['subnets'].first['reserved'] = ['10.0.11.32 - 10.0.11.35', '10.0.11.63'] + network_spec['subnets'].first['static'] = [] + + # Simulate AWS reserved range scenario with partial database state + save_ip_string('10.0.11.32/32') + save_ip_string('10.0.11.33/32') + save_ip_string('10.0.11.34/32') + + # Should deduplicate /32s, keep /30, skip entire reserved range (32-35) + ip_address = ip_repo.allocate_dynamic_ip(reservation, subnet) + expect(ip_address).to eq(cidr_ip('10.0.11.36')) + end + end + end end describe :allocate_vip_ip do