Add spec for dhcp::subnet and improve template and definition

diff --git a/manifests/subnet.pp b/manifests/subnet.pp
index 18ffdc5c2fb56cc5ef8eb932c05b6337869e5aef..b9826daef41c455e17662938f05cd5997458d071 100644 (file)
--- a/manifests/subnet.pp
+++ b/manifests/subnet.pp
@@ -27,16 +27,29 @@
 #
 define dhcp::subnet(
   $broadcast,
-  $ensure=present,
-  $netmask=false,
-  $routers=false,
-  $subnet_mask=false,
-  $domain_name=false,
-  $other_opts=false,
-  $is_shared=false
+  $ensure = present,
+  $netmask = undef,
+  $routers = [],
+  $subnet_mask = undef,
+  $domain_name = undef,
+  $other_opts = [],
+  $is_shared = false
 ) {
 
-  include dhcp::params
+  include ::dhcp::params
+
+  $ip_re = '^([1-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])(\.([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])){3}$'
+
+  validate_string($ensure)
+  validate_re($ensure, ['present', 'absent'],
+              "\$ensure must be either 'present' or 'absent', got '${ensure}'")
+  validate_string($broadcast)
+  validate_re($broadcast, $ip_re)
+  validate_string($netmask)
+  validate_array($routers)
+  validate_string($subnet_mask)
+  validate_string($domain_name)
+  validate_bool($is_shared)
 
   concat {"${dhcp::params::config_dir}/hosts.d/${name}.conf":
     owner => root,
@@ -48,34 +61,30 @@ define dhcp::subnet(
     ensure  => $ensure,
     owner   => root,
     group   => root,
-    content => template('dhcp/subnet.conf.erb'),
+    content => template("${module_name}/subnet.conf.erb"),
     notify  => Service['dhcpd'],
   }
 
-  if ! $is_shared {
-    concat::fragment {"dhcp.${name}":
-      ensure  => $ensure,
-      target  => "${dhcp::params::config_dir}/dhcpd.conf",
-      content => "include \"${dhcp::params::config_dir}/subnets/${name}.conf\";\n",
-    }
-  } else {
-    concat::fragment {"dhcp.${name}":
-      ensure  => absent,
-      target  => "${dhcp::params::config_dir}/dhcpd.conf",
-      content => "include \"${dhcp::params::config_dir}/subnets/${name}.conf\";\n",
-    }
-
+  $ensure_shared = $is_shared ? {
+    true  => $ensure,
+    false => 'absent',
+  }
+  concat::fragment {"dhcp.subnet.${name}":
+    ensure  => $ensure_shared,
+    target  => "${dhcp::params::config_dir}/dhcpd.conf",
+    content => "include \"${dhcp::params::config_dir}/subnets/${name}.conf\";\n",
   }
 
-  concat::fragment {"subnet.${name}.hosts":
+  concat::fragment {"dhcp.subnet.${name}.hosts":
     ensure  => $ensure,
     target  => "${dhcp::params::config_dir}/dhcpd.conf",
     content => "include \"${dhcp::params::config_dir}/hosts.d/${name}.conf\";\n",
   }
 
-  concat::fragment {"00.dhcp.${name}.base":
+  concat::fragment {"dhcp.subnet.${name}.base":
     ensure  => $ensure,
     target  => "${dhcp::params::config_dir}/hosts.d/${name}.conf",
     content => "# File managed by puppet\n",
+    order   => '00',
   }
 }

diff --git a/spec/defines/dhcp_subnet_spec.rb b/spec/defines/dhcp_subnet_spec.rb
new file mode 100644 (file)
index 0000000..48ab71b
--- /dev/null
+++ b/spec/defines/dhcp_subnet_spec.rb
@@ -0,0 +1,252 @@
+require 'spec_helper'
+
+describe 'dhcp::subnet' do
+  let (:title) { '1.2.3.4' }
+  let (:facts) { {
+    :operatingsystem => 'Debian',
+    :osfamily        => 'Debian',
+    :lsbdistcodename => 'squeeze',
+    :netmask_eth0    => '255.255.255.0',
+    :domain          => 'example.com',
+  } }
+
+  context 'when passing wrong value for ensure' do
+    let (:params) { {
+      :ensure    => 'running',
+      :broadcast => '1.2.3.4',
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /\$ensure must be either 'present' or 'absent', got 'running'/)
+    end
+  end
+
+  context 'when not passing broadcast' do
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /Must pass broadcast to Dhcp::Subnet/)
+    end
+  end
+
+  context 'when passing wrong type for broadcast' do
+    let (:params) { {
+      :broadcast => true,
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /true is not a string\./)
+    end
+  end
+
+  context 'when passing wrong value for broadcast' do
+    let (:params) { {
+      :broadcast => 'foo',
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /"foo" does not match/)
+    end
+  end
+
+  context 'when passing wrong type for netmask' do
+    let (:params) { {
+      :broadcast => '1.2.3.4',
+      :netmask   => true,
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /true is not a string\./)
+    end
+  end
+
+  context 'when passing wrong value for netmask' do
+    let (:params) { {
+      :broadcast => '1.2.3.4',
+      :netmask   => 'foo',
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /"foo" does not match/)
+    end
+  end
+
+  context 'when passing wrong type for routers' do
+    let (:params) { {
+      :broadcast => '1.2.3.4',
+      :routers   => true,
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /true is not an Array\./)
+    end
+  end
+
+  context 'when passing wrong type for subnet_mask' do
+    let (:params) { {
+      :broadcast   => '1.2.3.4',
+      :subnet_mask => true,
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /true is not a string\./)
+    end
+  end
+
+  context 'when passing wrong type for domain_name' do
+    let (:params) { {
+      :broadcast   => '1.2.3.4',
+      :domain_name => true,
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /true is not a string\./)
+    end
+  end
+
+  context 'when passing wrong type for is_shared' do
+    let (:params) { {
+      :broadcast => '1.2.3.4',
+      :is_shared => 'foo',
+    } }
+
+    it 'should fail' do
+      expect {
+        should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf')
+      }.to raise_error(Puppet::Error, /"foo" is not a boolean\./)
+    end
+  end
+
+  context 'when using defaults' do
+    let (:params) { {
+      :broadcast => '1.2.3.4',
+    } }
+
+    it { should contain_concat('/etc/dhcp/hosts.d/1.2.3.4.conf').with(
+      :owner => 'root',
+      :group => 'root',
+      :mode  => '0644'
+    ) }
+
+    it { should contain_file('/etc/dhcp/subnets/1.2.3.4.conf').with(
+        :ensure  => 'present',
+        :owner   => 'root',
+        :group   => 'root'
+      ).with_content(
+        /subnet 1\.2\.3\.4 netmask 255\.255\.255\.0 \{\n/
+      ).with_content(
+        /option routers 255\.255\.255\.0;\n/
+      ).with_content(
+        /option subnet-mask 255\.255\.255\.0;\n/
+      ).with_content(
+        /option broadcast-address 1\.2\.3\.4;\n/
+      ).with_content(
+        /option domain-name "example\.com";\n/
+      ) }
+
+    it { should contain_concat__fragment('dhcp.subnet.1.2.3.4').with(
+      :ensure  => 'absent'
+    ) }
+
+    it { should contain_concat__fragment('dhcp.subnet.1.2.3.4.hosts').with(
+      :ensure  => 'present',
+      :target  => '/etc/dhcp/dhcpd.conf',
+      :content => "include \"/etc/dhcp/hosts.d/1\.2\.3\.4\.conf\";\n"
+    ) }
+
+    it { should contain_concat__fragment('dhcp.subnet.1.2.3.4.base').with(
+      :ensure  => 'present',
+      :target  => '/etc/dhcp/hosts.d/1.2.3.4.conf',
+      :content => "# File managed by puppet\n",
+      :order   => '00'
+    ) }
+  end
+
+  context 'when is_shared is true' do
+    let (:params) { {
+      :broadcast => '1.2.3.4',
+      :is_shared => true,
+    } }
+
+    it { should contain_concat__fragment('dhcp.subnet.1.2.3.4').with(
+      :ensure  => 'present',
+      :target  => '/etc/dhcp/dhcpd.conf',
+      :content => "include \"/etc/dhcp/subnets/1.2.3.4.conf\";\n"
+    ) }
+  end
+
+  context 'when passing other_opts as array' do
+    let (:params) { {
+      :broadcast   => '1.2.3.4',
+      :other_opts  => ['foo', 'bar'],
+    } }
+
+    it { should contain_file('/etc/dhcp/subnets/1.2.3.4.conf').with(
+        :ensure  => 'present',
+        :owner   => 'root',
+        :group   => 'root'
+      ).with_content(
+        /  foo;\n  bar;\n/
+      ) }
+  end
+
+  context 'when passing other_opts as string' do
+    let (:params) { {
+      :broadcast   => '1.2.3.4',
+      :other_opts  => 'bar',
+    } }
+
+    it { should contain_file('/etc/dhcp/subnets/1.2.3.4.conf').with(
+        :ensure  => 'present',
+        :owner   => 'root',
+        :group   => 'root'
+      ).with_content(
+        /  bar;\n/
+      ) }
+  end
+
+  context 'when overriding all parameters' do
+    let (:params) { {
+      :broadcast   => '1.2.3.4',
+      :netmask     => '255.1.2.0',
+      :routers     => ['2.3.4.5', '3.4.5.6'],
+      :subnet_mask => '255.255.1.0',
+      :domain_name => 'foo.io',
+      :other_opts  => ['foo', 'bar'],
+    } }
+
+    it { should contain_file('/etc/dhcp/subnets/1.2.3.4.conf').with(
+        :ensure  => 'present',
+        :owner   => 'root',
+        :group   => 'root'
+      ).with_content(
+        /subnet 1\.2\.3\.4 netmask 255\.1\.2\.0 \{\n/
+      ).with_content(
+        /option routers 2\.3\.4\.5,3\.4\.5\.6;\n/
+      ).with_content(
+        /option subnet-mask 255\.255\.1\.0;\n/
+      ).with_content(
+        /option broadcast-address 1\.2\.3\.4;\n/
+      ).with_content(
+        /option domain-name "foo\.io";\n/
+      ).with_content(
+        /  foo;\n  bar;\n/
+      ) }
+  end
+end

diff --git a/templates/subnet.conf.erb b/templates/subnet.conf.erb
index 89ca82fb697905035979e0d699a650e3faedd9c1..11f9ed4f910d68dda61f6e1b558f73f4cf385fc9 100644 (file)
--- a/templates/subnet.conf.erb
+++ b/templates/subnet.conf.erb
@@ -1,33 +1,28 @@
 # File managed by puppet
 
-<% if netmask -%>
-subnet <%=name%> netmask <%=netmask%> {
-<% else -%>
-subnet <%=name%> netmask <%=netmask_eth0%> {
-<% end -%>
-<% if routers and not routers.empty? -%>
-  option routers <%= routers.collect! {|i| "#{i}" }.join(",") %>;
-<% else -%>
-  option routers <%=network_eth0%>;
-<% end -%>
-<% if subnet_mask -%>
-  option subnet-mask <%=subnet_mask%>;
-<% elsif netmask -%>
-  option subnet-mask <%=netmask%>;
-<% else -%>
-  option subnet-mask <%=netmask_eth0%>;
-<% end -%>
-  option broadcast-address <%=broadcast%>;
-<% if domain_name -%>
-  option domain-name "<%=domain_name%>";
-<% else -%>
-  option domain-name "<%=domain%>";
-<% end -%>
-<% if other_opts and not other_opts.empty? -%>
-  <% if other_opts.is_a?(Array) -%>
-<%= other_opts.collect! {|i| "  #{i};"}.join("\n") %>
+<%-
+  _netmask = @netmask || scope.lookupvar('::netmask_eth0')
+  scope.function_validate_re([_netmask, @ip_re]) -%>
+subnet <%= @name %> netmask <%= _netmask %> {
+<%-
+  _routers = @routers unless @routers.empty?
+  _routers ||= [scope.lookupvar('::netmask_eth0')]
+  scope.function_validate_array([_routers]) -%>
+  option routers <%= _routers.collect! {|i| "#{i}" }.join(',') %>;
+<%-
+  _subnet_mask = @subnet_mask || _netmask
+  scope.function_validate_re([_subnet_mask, @ip_re]) -%>
+  option subnet-mask <%= _subnet_mask %>;
+  option broadcast-address <%= @broadcast %>;
+<%-
+  _domain_name = @domain_name || scope.lookupvar('::domain')
+  scope.function_validate_re([_domain_name, '^\S+$']) -%>
+  option domain-name "<%= _domain_name %>";
+<% unless @other_opts.empty? -%>
+  <% if @other_opts.is_a?(Array) -%>
+<%= @other_opts.collect! {|i| "  #{i};"}.join("\n") %>
   <% else -%>
-  <%=other_opts%>;
+  <%= @other_opts %>;
   <% end -%>
 <% end -%>
 }