Conversation
bastelfreak
force-pushed
the
cidr_to_range
branch
2 times, most recently
from
873aaa1 to
ce21f86
Compare
| ips = IPAddr.new(ip).to_range.map(&:to_s) | ||
| ips.shift | ||
| ips.pop | ||
| ips |
There was a problem hiding this comment.
This is only really true for IPv4, not IPv6.
There was a problem hiding this comment.
I'm not sure if I can follow. I tested this locally and also wrote a test that passes for IPv6. This works for IPv4 and IPv6?
There was a problem hiding this comment.
Popping the last address isn't really needed AFAIK because IPv6 doesn't have a broadcast address. I think the same is true for shifting due to a network address.
There was a problem hiding this comment.
The first address in an IPv6 subnet is reserved as the subnet router anycast address: https://tools.ietf.org/html/rfc4291#section-2.6.1
But the last address in a subnet is usable.
| end | ||
|
|
||
| def cidr_to_range(ip) | ||
| ips = IPAddr.new(ip).to_range.map(&:to_s) |
There was a problem hiding this comment.
Also, this can be slow and memory consuming for huge IPv6 networks like a /64 (or larger). Why would you want this?
There was a problem hiding this comment.
yes, depending on the subnet size it might take some time.
I know at least one person that uses https://github.com/Yelp/puppet-netstdlib and I require those functions as well. Yelp abandoned the module and I had the impression it's a good idea to add them to extlib. I thought for a hand full of short functions it doesnt make much sense to maintain a whole module.
There was a problem hiding this comment.
what is this used for? eespecially in the case of IPv6?
There was a problem hiding this comment.
Yeah, not sure why you would want to enumerate every address of a subnet (you're definitely not doing this for most IPv6 networks), but I think it would be more useful to have a function that gets one of the addresses in a subnet as I mentioned here: #165 (comment)
You could have a loop around such a function if you really wanted to do something with every address.
There was a problem hiding this comment.
this would make sense if we returned lazy structures
|
Maybe these functions can be imported https://forge.puppet.com/yelp/netstdlib |
bastelfreak
force-pushed
the
cidr_to_range
branch
from
ce21f86 to
5537255
Compare
|
@SimonHoenscheid that's what I'm doing :) |
| end | ||
|
|
||
| context 'when called with an IPv6 CIDR' do | ||
| it { is_expected.to run.with_params('fe80::5054:ff:fe47:4a37/126').and_return(['fe80::5054:ff:fe47:4a35', 'fe80::5054:ff:fe47:4a36']) } |
There was a problem hiding this comment.
I always like to use example IP ranges. In https://community.theforeman.org/t/using-ips-and-domains-in-tests-documentation/16600 I keep a list of references.
| end | ||
|
|
||
| context 'when called with an IPv6 CIDR' do | ||
| it { is_expected.to run.with_params('fe80::5054:ff:fe47:4a37/126').and_return(['fe80::5054:ff:fe47:4a35', 'fe80::5054:ff:fe47:4a36']) } |
There was a problem hiding this comment.
Indeed this is not correct for IPv6, the last address fe80::5054:ff:fe47:4a37 is usable.
| ###### calling the function | ||
|
|
||
| ```puppet | ||
| extlib::cidr_to_range('127.0.0.1/8') |
There was a problem hiding this comment.
I sincerely hope no one calls this with a /8...
There was a problem hiding this comment.
not sure that's much worse than the standard /64 IPv6 address range…
There was a problem hiding this comment.
Well, IPv4 /8 is 2**24 while IPv6 /64 is 2**64. Every IPv4 address is 32 bits. 32 bits * 2**24 = 67 MB (assuming optimal storage, which it isn't). For IPv6 it's 128 bits * 2**64 = 256 EiB. If my math is correct, you really don't want to do this for IPv6.
|
Dear @bastelfreak, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
6 participants
Pull Request (PR) description
This Pull Request (PR) fixes the following issues