Get rid of unscoped variables in dhcp::server, fix template

5 files changed, 97 insertions, 20 deletions

diff --git a/manifests/init.pp b/manifests/init.pp
index 8a14b13..e1e05b3 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -1,7 +1,14 @@
 class dhcp (
   $server = true,
+  $server_ddns_update = undef,
+  $server_authoritative = undef,
+  $server_opts = undef,
 ) {
   if $server {
-    class { '::dhcp::server': }
+    class { '::dhcp::server':
+      ddns_update   => $server_ddns_update,
+      authoritative => $server_authoritative,
+      opts          => $server_opts,
+    }
   }
 }
diff --git a/manifests/server.pp b/manifests/server.pp
index ceffa14..d83c26f 100644
--- a/manifests/server.pp
+++ b/manifests/server.pp
@@ -5,15 +5,17 @@
 #   module "common": git://github.com/camptocamp/puppet-common.git
 #
 # facultative argument:
-#   *$dhcpd_ddns_update*   : ddns-update-style option (default to none)
-#   *$dhcpd_authoritative* : set it if you want that your DHCP server is
-#                             authoritative (default to no)
-#   *$dhcpd_opts*          : any other DHCPD valid options
+#   *$ddns_update*   : ddns-update-style option (default to none)
+#   *$authoritative* : set it if you want that your DHCP server is
+#                      authoritative (default to no)
+#   *$opts*          : any other DHCPD valid options
 #
 # Example:
 # node "dhcp.toto.ltd" {
-#   $dhcpd_opts = ['domain-name "toto.ltd"', "domain-name-servers 192.168.21.1"]
-#   include dhcp::server
+#   class { 'dhcp::server':
+#     opts => ['domain-name "toto.ltd"',
+#              'domain-name-servers 192.168.21.1'],
+#   }
 #
 #   dhcp::subnet {"10.27.20.0":
 #     ensure     => present,
@@ -29,7 +31,11 @@
 #   }
 # }
 #
-class dhcp::server {
+class dhcp::server (
+  $ddns_update = 'none',
+  $authoritative = false,
+  $opts = [],
+) {
   class { '::dhcp::server::packages': } ->
   class { '::dhcp::server::config': } ~>
   class { '::dhcp::server::service': }
diff --git a/manifests/server/config.pp b/manifests/server/config.pp
index 4898a60..0c1a74a 100644
--- a/manifests/server/config.pp
+++ b/manifests/server/config.pp
@@ -11,6 +11,10 @@ class dhcp::server::config {
   validate_string($dhcp::params::server_template)
   validate_re($dhcp::params::server_template, '^\S+$')
 
+  validate_string($dhcp::server::ddns_update)
+  validate_bool($dhcp::server::authoritative)
+  validate_array($dhcp::server::opts)
+
   concat {"${dhcp::params::config_dir}/dhcpd.conf":
     owner => root,
     group => root,
diff --git a/spec/classes/dhcp_spec.rb b/spec/classes/dhcp_spec.rb
index c833c52..ce9228e 100644
--- a/spec/classes/dhcp_spec.rb
+++ b/spec/classes/dhcp_spec.rb
@@ -34,6 +34,13 @@ describe 'dhcp' do
       :mode  => '0644'
     ) }
 
+    it { should contain_concat__fragment('00.dhcp.server.base').with(
+      :ensure  => 'present',
+      :target  => '/etc/dhcp3/dhcpd.conf',
+      :content => /log-facility/
+      ).with_content(/ddns-update-style none;/).with_content(/#authoritative/)
+    }
+
     # Service
     it { should contain_service('dhcpd').with(
       :ensure  => 'running',
@@ -62,6 +69,13 @@ describe 'dhcp' do
       :mode  => '0644'
     ) }
 
+    it { should contain_concat__fragment('00.dhcp.server.base').with(
+        :ensure  => 'present',
+        :target  => '/etc/dhcp/dhcpd.conf',
+        :content => /log-facility/
+      ).with_content(/ddns-update-style none;/).with_content(/#authoritative/)
+    }
+
     # Service
     it { should contain_service('dhcpd').with(
       :ensure  => 'running',
@@ -70,4 +84,58 @@ describe 'dhcp' do
       :pattern => '/usr/sbin/dhcpd'
     ) }
   end
+
+  context 'When passing ddns_update' do
+    let (:facts) { {
+      :operatingsystem => 'Debian',
+      :osfamily        => 'Debian',
+      :lsbdistcodename => 'squeeze'
+    } }
+    let (:params) { {
+      :server_ddns_update => 'foo'
+    } }
+
+    it { should contain_concat__fragment('00.dhcp.server.base').with(
+        :ensure  => 'present',
+        :target  => '/etc/dhcp/dhcpd.conf',
+        :content => /log-facility/
+      ).with_content(/ddns-update-style foo;/).with_content(/#authoritative/)
+    }
+  end
+
+  context 'When passing authoritative' do
+    let (:facts) { {
+      :operatingsystem => 'Debian',
+      :osfamily        => 'Debian',
+      :lsbdistcodename => 'squeeze'
+    } }
+    let (:params) { {
+      :server_authoritative => true
+    } }
+
+    it { should contain_concat__fragment('00.dhcp.server.base').with(
+        :ensure  => 'present',
+        :target  => '/etc/dhcp/dhcpd.conf',
+        :content => /log-facility/
+      ).with_content(/ddns-update-style none;/).with_content(/[^#]authoritative/)
+    }
+  end
+
+  context 'When passing opts' do
+    let (:facts) { {
+      :operatingsystem => 'Debian',
+      :osfamily        => 'Debian',
+      :lsbdistcodename => 'squeeze'
+    } }
+    let (:params) { {
+      :server_opts => ['foo', 'bar', 'baz']
+    } }
+
+    it { should contain_concat__fragment('00.dhcp.server.base').with(
+        :ensure  => 'present',
+        :target  => '/etc/dhcp/dhcpd.conf',
+        :content => /log-facility/
+      ).with_content(/ddns-update-style none;/).with_content(/#authoritative/).with_content(/foo;\nbar;\nbaz;\n/)
+    }
+  end
 end
diff --git a/templates/dhcpd.conf.debian.erb b/templates/dhcpd.conf.debian.erb
index ee95bee..7dfd168 100644
--- a/templates/dhcpd.conf.debian.erb
+++ b/templates/dhcpd.conf.debian.erb
@@ -4,22 +4,14 @@
 # attempt to do a DNS update when a lease is confirmed. We default to the
 # behavior of the version 2 packages ('none', since DHCP v2 didn't
 # have support for DDNS.)
-<% if has_variable?('dhcpd_ddns_update') -%>
-ddns-update-style <%=dhcpd_ddns_update%>;
-<% else -%>
-ddns-update-style none;
-<% end -%>
+ddns-update-style <%= @ddns_update %>;
 
 # If this DHCP server is the official DHCP server for the local
 # network, the authoritative directive should be uncommented.
-<% if has_variable?('dhcpd_authoritative') -%>
-authoritative;
-<% else -%>
-#authoritative;
-<% end -%>
+<% unless @authoritative -%>#<%- end -%>authoritative;
 
-<% if has_variable?('dhcpd_opts') and not dhcpd_opts.empty? -%>
-<%= dhcpd_opts.join(";\n") %>
+<% unless @opts.empty? -%>
+<%= @opts.join(";\n") %>;
 <% end -%>
 
 # Use this to send dhcp log messages to a different log file (you also