Skip to content
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

Allow passing hostnames to $dhcp::host::ip #227

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Allow passing hostnames to $dhcp::host::ip #227

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a4093b3
Removing IP validation in order to get dns lookups to work
dwest-galois Jul 17, 2019
fa70ca1
Adding a max-lease-time option to the pool scope
dwest-galois Jul 17, 2019
e411433
setting max_lease_time as optional if undef
dwest-galois Jul 17, 2019
7f7155e
Matches a valid host which could be a valid ipv4, ipv6 or fqdn
dwest-galois Jul 18, 2019
edf6dc5
Merge pull request #1 from dwest-galois/dwest/dev
dwest-galois Jul 18, 2019
aa0eabf
Adding a max-lease-time parameter to the pool scope. Allow `fqdn` as…
dwest-galois Jul 17, 2019
435f15d
Merge branch 'dwest/dev' of github.com:dwest-galois/puppet-dhcp into …
dwest-galois Jul 18, 2019
c86b5d8
Merge branch 'dwest/dev'
dwest-galois Jul 18, 2019
26034cd
Merge branch 'master' of https://github.com/voxpupuli/puppet-dhcp int…
dwest-galois Jan 6, 2021
4061b78
Merge pull request #2 from dwest-galois/dwest/dev
dwest-galois Feb 24, 2021
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
2 changes: 1 addition & 1 deletion manifests/host.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# == Define: dhcp::host
#
define dhcp::host (
Optional[Stdlib::IP::Address] $ip = undef,
Optional[Stdlib::Host] $ip = undef,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming of the parameter must also be changed in my opinion. From a user point of view still having to specify ip parameter when I can also sent FQDN towards it is a bit confusing. This does however causes some backwards compatability issue's. My suggestion would be to implement a new parameter called $host which is of type stdlib::host make a check, which checks if either $ip or $host is set. If $ip is set raise a deprication warning that $ip will be removed in the next version.

What do others think of this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zyronix agreed, don't forget to update the documentation!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that as a user this would confuse me and a rename is in order.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for taking so long to respond. I agree that the $ip parameter should be changed to something like $host. I originally left it as-is since it would probably break a lot of users dhcp::host: configuration. I can provide some unit tests once I get those setup. Due to time constraints, it might take me until next week.

Dhcp::Mac $mac,
String $ddns_hostname = $name,
Hash $options = {},
Expand Down
1 change: 1 addition & 0 deletions manifests/pool.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
Optional[Integer] $mtu = undef,
String $domain_name = '',
$ignore_unknown = undef,
Optional[Integer] $max_lease_time = undef,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this parameter isn't related to changing the above datatype, right? In that case, can you please submit it as a new pull request?

) {
include dhcp::params

Expand Down
3 changes: 3 additions & 0 deletions templates/dhcpd.pool.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ subnet <%= @network %> netmask <%= @mask %> {
<% if @ignore_unknown == true -%>
ignore unknown-clients ;
<% end -%>
<% if @max_lease_time -%>
max-lease-time <%= @max_lease_time %>;
<% end -%>
<% if @range and @range.is_a? Array -%>
<% @range.each do |r| -%>
range <%= r %>;
Expand Down