Skip to content

B #197: VR prefixlen is not correctly inferred for VIPs and IPs #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rsmontero merged 3 commits into from
Feb 17, 2025
Merged

B #197: VR prefixlen is not correctly inferred for VIPs and IPs #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ca8da2
B #197: Update vrouter unit tests
jaimecb Feb 12, 2025
e40f2c8
B #197: Update infer_pfxlen and tests
jaimecb Feb 13, 2025
3744b91
B #197: Make sure pfxlen is never 0 (paranoid)
sk4zuzu Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions appliances/VRouter/Keepalived/tests.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ def clear_vars(object)
expect(keepalived_vars[:by_nic]['eth0'][:interval]).to eq '1'
expect(keepalived_vars[:by_nic]['eth0'][:priority]).to eq '100'
expect(keepalived_vars[:by_nic]['eth0'][:vrid]).to eq '11'
expect(keepalived_vars[:by_nic]['eth0'][:vips][0]).to eq '10.2.11.69/32'
expect(keepalived_vars[:by_nic]['eth0'][:vips][1]).to eq '10.2.11.86/32'
expect(keepalived_vars[:by_nic]['eth0'][:vips][0]).to eq '10.2.11.69/8'
expect(keepalived_vars[:by_nic]['eth0'][:vips][1]).to eq '10.2.11.86/8'
expect(keepalived_vars[:by_nic]['eth0'][:noip]).to be true

expect(keepalived_vars[:by_nic]['eth1'][:interval]).to eq '1'
expect(keepalived_vars[:by_nic]['eth1'][:priority]).to eq '100'
expect(keepalived_vars[:by_nic]['eth1'][:vrid]).to eq '11'
expect(keepalived_vars[:by_nic]['eth1'][:vips][0]).to eq '10.2.12.69/32'
expect(keepalived_vars[:by_nic]['eth1'][:vips][1]).to eq '10.2.12.86/32'
expect(keepalived_vars[:by_nic]['eth1'][:vips][0]).to eq '10.2.12.69/8'
expect(keepalived_vars[:by_nic]['eth1'][:vips][1]).to eq '10.2.12.86/8'
expect(keepalived_vars[:by_nic]['eth1'][:noip]).to be false

expect(keepalived_vars[:by_vrid]['11'].keys).to eq %w[eth0 eth1]
Expand Down Expand Up @@ -112,11 +112,11 @@ def clear_vars(object)
expect(Service::Keepalived.instance_variable_get(:@interfaces).keys).to eq %w[eth0 eth1]

expect(keepalived_vars[:by_nic]['eth0'][:vrid]).to eq '21'
expect(keepalived_vars[:by_nic]['eth0'][:vips][0]).to eq '10.2.21.69/32'
expect(keepalived_vars[:by_nic]['eth0'][:vips][0]).to eq '10.2.21.69/8'
expect(keepalived_vars[:by_nic]['eth0'][:noip]).to be true

expect(keepalived_vars[:by_nic]['eth1'][:vrid]).to eq '21'
expect(keepalived_vars[:by_nic]['eth1'][:vips][0]).to eq '10.2.22.69/32'
expect(keepalived_vars[:by_nic]['eth1'][:vips][0]).to eq '10.2.22.69/8'
expect(keepalived_vars[:by_nic]['eth1'][:noip]).to be false

expect(keepalived_vars[:by_vrid]['21'].keys).to eq %w[eth0 eth1]
Expand Down Expand Up @@ -185,10 +185,10 @@ def clear_vars(object)
priority 100
advert_int 1
virtual_ipaddress {
10.2.30.69/32 dev eth0
10.2.30.86/32 dev eth0
10.2.31.69/32 dev eth1
10.2.31.86/32 dev eth1
10.2.30.69/8 dev eth0
10.2.30.86/8 dev eth0
10.2.31.69/8 dev eth1
10.2.31.86/8 dev eth1
}
virtual_routes {
0.0.0.0/0 via 10.2.30.1
Expand All @@ -201,7 +201,7 @@ def clear_vars(object)
priority 100
advert_int 1
virtual_ipaddress {
10.2.33.69/32 dev eth3
10.2.33.69/8 dev eth3
}
virtual_routes {
}
Expand Down
194 changes: 151 additions & 43 deletions appliances/VRouter/tests.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,54 +36,162 @@ def clear_env
end

RSpec.describe 'detect_addrs' do
it 'should parse IP variables' do
clear_env

ENV['ETH0_IP'] = '1.2.3.4'
ENV['ETH0_MASK'] = '255.255.0.0'
ENV['ETH1_IP'] = '2.3.4.5'
ENV['ETH1_MASK'] = '255.255.255.0'

expect(detect_addrs).to eq ({
'eth0' => { 'ETH0_IP0' => '1.2.3.4/16' },
'eth1' => { 'ETH1_IP0' => '2.3.4.5/24' }
})
end
it 'should parse IP variables with mask' do
clear_env

ENV['ETH0_IP'] = '1.2.3.4'
ENV['ETH0_MASK'] = '255.255.0.0'
ENV['ETH1_IP'] = '2.3.4.5'
ENV['ETH1_MASK'] = '255.255.255.0'

expect(detect_addrs)
.to eq({
'eth0' => { 'ETH0_IP0' => '1.2.3.4/16' },
'eth1' => { 'ETH1_IP0' => '2.3.4.5/24' }
})
end

it 'should parse IP variables with network address' do
clear_env

ENV['ETH0_IP'] = '10.2.3.4'
ENV['ETH0_NETWORK'] = '10.0.0.0'
ENV['ETH1_IP'] = '172.16.3.4'
ENV['ETH1_NETWORK'] = '172.16.0.0'

expect(detect_addrs)
.to eq({
'eth0' => { 'ETH0_IP0' => '10.2.3.4/8' },
'eth1' => { 'ETH1_IP0' => '172.16.3.4/16' }
})
end

it 'should handle private network ranges without mask or network' do
clear_env

ENV['ETH0_IP'] = '10.2.3.4'
ENV['ETH1_IP'] = '172.16.3.4'
ENV['ETH2_IP'] = '192.168.1.2'

expect(detect_addrs)
.to eq({
'eth0' => { 'ETH0_IP0' => '10.2.3.4/8' },
'eth1' => { 'ETH1_IP0' => '172.16.3.4/16' },
'eth2' => { 'ETH2_IP0' => '192.168.1.2/24' }
})
end
end

RSpec.describe 'detect_vips' do
it 'should parse VIP variables' do
clear_env

ENV['ETH0_MASK'] = '255.255.0.0'
ENV['ETH0_VROUTER_IP'] = '1.2.3.4'
ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '2.3.4.5/24'
ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '3.4.5.6'

expect(detect_vips).to eq ({
'eth0' => { 'ETH0_VIP0' => '1.2.3.4/16',
'ETH0_VIP1' => '2.3.4.5/24' },
'eth1' => { 'ETH1_VIP0' => '3.4.5.6/32' }
})
end
it 'should parse VIP variables with mask' do
clear_env

ENV['ETH0_MASK'] = '255.255.0.0'
ENV['ETH0_VROUTER_IP'] = '1.2.3.4'
ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '2.3.4.5/24'
ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '3.4.5.6'

expect(detect_vips)
.to eq({
'eth0' => { 'ETH0_VIP0' => '1.2.3.4/16',
'ETH0_VIP1' => '2.3.4.5/24' },
'eth1' => { 'ETH1_VIP0' => '3.4.5.6/24' }
})
end

it 'should parse VIP variables with network address' do
clear_env

ENV['ETH0_NETWORK'] = '10.0.0.0'
ENV['ETH0_VROUTER_IP'] = '10.2.3.4'
ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '10.2.4.5'
ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '172.16.5.6'
ENV['ETH1_NETWORK'] = '172.16.0.0'

expect(detect_vips)
.to eq({
'eth0' => { 'ETH0_VIP0' => '10.2.3.4/8',
'ETH0_VIP1' => '10.2.4.5/8' },
'eth1' => { 'ETH1_VIP0' => '172.16.5.6/16' }
})
end

it 'should default to /24 for public VIPs without network or mask' do
clear_env

ENV['ETH0_VROUTER_IP'] = '1.2.3.4'
ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '2.3.4.5'

expect(detect_vips)
.to eq({
'eth0' => { 'ETH0_VIP0' => '1.2.3.4/24',
'ETH0_VIP1' => '2.3.4.5/24' }
})
end

it 'should infer prefix for private range VIPs without mask or network' do
clear_env

ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '172.16.5.9'
ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '10.5.6.7'
ENV['ONEAPP_VROUTER_ETH2_VIP0'] = '192.168.1.3'

expect(detect_vips)
.to eq({
'eth0' => { 'ETH0_VIP0' => '172.16.5.9/16' },
'eth1' => { 'ETH1_VIP0' => '10.5.6.7/8' },
'eth2' => { 'ETH2_VIP0' => '192.168.1.3/24' }
})
end
end

RSpec.describe 'detect_endpoints' do
it 'should merge IP and VIP variables correctly' do
clear_env

ENV['ETH0_IP'] = '1.2.3.4'
ENV['ETH0_MASK'] = '255.255.0.0'
ENV['ETH1_IP'] = '2.3.4.5'
ENV['ETH1_MASK'] = '255.255.255.0'

ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '3.4.5.6'

expect(detect_endpoints).to eq ({
'eth0' => { 'ETH0_EP0' => '1.2.3.4/16' },
'eth1' => { 'ETH1_EP0' => '3.4.5.6/24' }
})
end
it 'should merge IP and VIP variables correctly (with mask)' do
clear_env

ENV['ETH0_IP'] = '1.2.3.4'
ENV['ETH0_MASK'] = '255.255.0.0'
ENV['ETH1_IP'] = '2.3.4.5'
ENV['ETH1_MASK'] = '255.255.255.0'

ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '3.4.5.6'

expect(detect_endpoints)
.to eq({
'eth0' => { 'ETH0_EP0' => '1.2.3.4/16' },
'eth1' => { 'ETH1_EP0' => '3.4.5.6/24' }
})
end

it 'should use network address when mask is not available' do
clear_env

ENV['ETH0_IP'] = '1.2.3.4'
ENV['ETH0_NETWORK'] = '1.2.0.0'
ENV['ETH1_IP'] = '2.3.4.5'
ENV['ETH1_NETWORK'] = '2.3.0.0'
ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '3.4.5.6'

expect(detect_endpoints)
.to eq({
'eth0' => { 'ETH0_EP0' => '1.2.3.4/16' },
'eth1' => { 'ETH1_EP0' => '3.4.5.6/16' }
})
end

it 'should handle private network ranges without mask or network' do
clear_env

ENV['ETH0_IP'] = '10.2.3.4'
ENV['ETH1_IP'] = '172.16.3.4'
ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '172.16.5.6'

expect(detect_endpoints)
.to eq({
'eth0' => { 'ETH0_EP0' => '10.2.3.4/8' },
'eth1' => { 'ETH1_EP0' => '172.16.5.6/16' }
})
end
end

RSpec.describe 'parse_interfaces' do
Expand Down Expand Up @@ -322,7 +430,7 @@ def clear_env

{ '1.2.3.4/24' => '1.2.3.0/24',
'2.3.4.5/16' => '2.3.0.0/16',
'6.7.8.9/32' => '6.7.8.9/32' } ]
'6.7.8.9/24' => '6.7.8.0/24' } ]
]
tests.each do |nics, vips, output|
expect(vips_to_subnets(nics, vips)).to eq output
Expand Down
34 changes: 19 additions & 15 deletions appliances/VRouter/vrouter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,30 +107,34 @@ def detect_mgmt_nics
end
end

def infer_pfxlen(eth_index, ip, infer_class: true)
unless (pfxlen = ip.split(%[/])[1]).nil?
return pfxlen.to_i
end
def infer_pfxlen(eth_index, ip)
pfxlen = ip.then do |ip|
unless (pfxlen = ip.split(%[/])[1]).nil?
next pfxlen.to_i
end

unless (mask = env("ETH#{eth_index}_MASK", nil)).nil?
return IPAddr.new("#{ip}/#{mask}").prefix.to_i
end
unless (mask = env("ETH#{eth_index}_MASK", nil)).nil?
next IPAddr.new("#{ip}/#{mask}").prefix.to_i
end

unless (network = env("ETH#{eth_index}_NETWORK", nil)).nil?
next 32 - 8 * network.split(%[.]).map(&:to_i).reverse.take_while(&:zero?).count
end

if infer_class
case (ip = IPAddr.new(ip)).family
when Socket::AF_INET
return 8 if ip.to_i & 0xff00_0000 == 0x0a00_0000 # A 10.x.y.z/8
return 16 if ip.to_i & 0xfff0_0000 == 0xac10_0000 # B 172.16.x.y/16
return 24 if ip.to_i & 0xffff_0000 == 0xc0a8_0000 # C 192.168.x.y/24
next 8 if ip.to_i & 0xff00_0000 == 0x0a00_0000 # A 10.x.y.z/8
next 16 if ip.to_i & 0xfff0_0000 == 0xac10_0000 # B 172.16.x.y/16
next 24 if ip.to_i & 0xffff_0000 == 0xc0a8_0000 # C 192.168.x.y/24
end
return 24 # guess/fallback
end

return 32 # VIPs
next 24 # guess/fallback
end
return pfxlen.zero? ? 32 : pfxlen
end

def append_pfxlen(eth_index, ip)
return "#{ip.split(%[/])[0]}/#{infer_pfxlen(eth_index, ip, infer_class: false)}"
return "#{ip.split(%[/])[0]}/#{infer_pfxlen(eth_index, ip)}"
end

def detect_addrs
Expand Down