Fixes #21249 - Check MS DHCP range versus start/end address #545
Conversation
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
achevalet
changed the title
achevalet
force-pushed
the
ms_dhcp_range
branch
from
ac836fc
to
9be2ed7
Compare
|
I wonder if this should be implemented at a higher level so all DHCP providers can benefit from this validation. |
|
@ekohl: There's already code to validate subnet ranges in a general case. This change is ms-dhcp specific. |
| @@ -78,6 +78,41 @@ def unused_ip(subnet_address, mac_address, from_address, to_address) | |||
| raise Proxy::DHCP::NotImplemented.new("DhcpsApi::Server#get_free_ip_address is not available on Windows Server 2008 and earlier versions.") | |||
| end | |||
|
|
|||
| dhcp_range = dhcpsapi.list_subnet_elements(subnet_address, DhcpsApi::DHCP_SUBNET_ELEMENT_TYPE::DhcpIpRanges) | |||
| dhcp_start_addr = dhcp_range.map {|r| r[:element][:start_address]}.first | |||
There was a problem hiding this comment.
It's possible to create a subnet without a subnet range, so both dhcp_start_addr and dhcp_end_addr can come back as nils.
There was a problem hiding this comment.
Well, this is the range on the MS DHCP, afaik we can't create a dhcp scope without a range?
There was a problem hiding this comment.
I think it's possible when api is used to create a subnet.
| dhcp_start_addr = dhcp_range.map {|r| r[:element][:start_address]}.first | ||
| dhcp_end_addr = dhcp_range.map {|r| r[:element][:end_address]}.first | ||
|
|
||
| if from_address.to_s.empty? |
There was a problem hiding this comment.
from_address and to_address will be nils if the caller skipped these parameters. from_address.nil? is better check in this case.
There was a problem hiding this comment.
Right, I will update it.
| dhcp_end_addr_ip = IPAddr.new(dhcp_end_addr, Socket::AF_INET) | ||
| end | ||
|
|
||
| if foreman_start_addr_ip.to_i > dhcp_end_addr_ip.to_i |
There was a problem hiding this comment.
There's a PR open (which should be merged in soon) that introduces a dedicated validator for range checks (https://github.com/theforeman/smart-proxy/pull/543/files#diff-42cb95eb87ed37db7fd2fc99a3064f06R67). I would suggest using it instead.
There was a problem hiding this comment.
There are 2 different use cases here:
- foreman range is out of dhcp scope = warning message and exit
- foreman range is out of dhcp scope but dhcp scope is inside foreman range, in that case we just update the from/to to match the dhcp scope and we continue.
Not sure how I can use the validate_subnet_range for that cases.
There was a problem hiding this comment.
Now that I think of it, I'd say treat #2 as an error and let the user to correct the data/scope (as it may have impact on other parts of Foreman) instead of massaging it.
achevalet
force-pushed
the
ms_dhcp_range
branch
from
9be2ed7
to
04a4eff
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
I have no clue how tests work, any help would be much appreciated! |
|
Please see my comment re: tests in your other PR. |
achevalet
force-pushed
the
ms_dhcp_range
branch
2 times, most recently
from
99e0550
to
c7ba03b
Compare
achevalet
force-pushed
the
ms_dhcp_range
branch
from
c7ba03b
to
71d8616
Compare
|
@witlessbird I've amended the tests, please let me know if that's ok. |
|
Merged, thanks @achevalet! |
Tests done:
MS DHCP range: 10.3.28.100-10.3.28.150
Foreman range: 50-99
WARN -- : End of IP range 10.3.28.99 is out of DHCP range (10.3.28.100-10.3.28.150)
Foreman range: 50-100
DEBUG -- : Start of IP range 10.3.28.50 is out of DHCP range (10.3.28.100-10.3.28.150), using 10.3.28.100
Foreman range: 50-150
DEBUG -- : Start of IP range 10.3.28.50 is out of DHCP range (10.3.28.100-10.3.28.150), using 10.3.28.100
Foreman range: 100-200
DEBUG -- : End of IP range 10.3.28.150 is out of DHCP range (10.3.28.100-10.3.28.150), using 10.3.28.150
Foreman range: 50-200
DEBUG -- : Start of IP range 10.3.28.50 is out of DHCP range (10.3.28.100-10.3.28.150), using 10.3.28.100
DEBUG -- : End of IP range 10.3.28.200 is out of DHCP range (10.3.28.100-10.3.28.150), using 10.3.28.150