Merge pull request #45 from kubashin-a/zone-serial-sed

Update zone-serial only on changing zone-records (sed version)

manifests/record.pp

@@ -9,10 +9,12 @@
   $order = 9
 ) {
 
-  $zone_file = "/etc/bind/zones/db.${zone}"
+  $cfg_dir = $dns::server::params::cfg_dir
+
+  $zone_file_stage = "${cfg_dir}/zones/db.${zone}.stage"
 
   concat::fragment{"db.${zone}.${name}.record":
-    target  => $zone_file,
+    target  => $zone_file_stage,
     order   => $order,
     content => template("${module_name}/zone_record.erb")
   }

manifests/zone.pp

@@ -1,7 +1,6 @@
 define dns::zone (
   $soa = "${::fqdn}.",
   $soa_email = "root.${::fqdn}.",
-  $serial = false,
   $zone_ttl = '604800',
   $zone_refresh = '604800',
   $zone_retry = '86400',
@@ -18,52 +17,67 @@
   $ensure = present
 ) {
 
+  $cfg_dir = $dns::server::params::cfg_dir
+
   validate_array($allow_transfer)
   validate_array($allow_forwarder)
-  if $dns::options::forwarder and $allow_forwarder {
+  if $dns::server::options::forwarder and $allow_forwarder {
     fatal("You cannot specify a global forwarder and \
     a zone forwarder for zone ${soa}")
   }
   if !member(['first', 'only'], $forward_policy) {
     error('The forward policy can only be set to either first or only')
   }
 
-  $zone_serial = $serial ? {
-    false   => inline_template('<%= Time.now.to_i %>'),
-    default => $serial
-  }
-
   $zone = $reverse ? {
     true    => "${name}.in-addr.arpa",
     default => $name
   }
 
-  $zone_file = "/etc/bind/zones/db.${name}"
+  $zone_file = "${cfg_dir}/zones/db.${name}"
+  $zone_file_stage = "${zone_file}.stage"
 
   if $ensure == absent {
     file { $zone_file:
       ensure => absent,
     }
   } else {
     # Zone Database
-    concat { $zone_file:
+
+    # Create "fake" zone file without zone-serial
+    concat { $zone_file_stage:
       owner   => 'bind',
       group   => 'bind',
       mode    => '0644',
       require => [Class['concat::setup'], Class['dns::server']],
-      notify  => Class['dns::server::service']
+      notify  => Exec["bump-${zone}-serial"]
     }
     concat::fragment{"db.${name}.soa":
-      target  => $zone_file,
+      target  => $zone_file_stage,
       order   => 1,
       content => template("${module_name}/zone_file.erb")
     }
+
+    # Generate real zone from stage file through replacement _SERIAL_ template
+    # to current timestamp. A real zone file will be updated only at change of
+    # the stage file, thanks to this serial is updated only in case of need.
+    $zone_serial = inline_template('<%= Time.now.to_i %>')
+    exec { "bump-${zone}-serial":
+      command     => "sed '8s/_SERIAL_/${zone_serial}/' ${zone_file_stage} > ${zone_file}",
+      path        => ['/bin', '/sbin', '/usr/bin', '/usr/sbin'],
+      refreshonly => true,
+      provider    => posix,
+      user        => 'bind',
+      group       => 'bind',
+      require     => Class['dns::server::install'],
+      notify      => Class['dns::server::service'],
+    }
   }
 
   # Include Zone in named.conf.local
   concat::fragment{"named.conf.local.${name}.include":
     ensure  => $ensure,
-    target  => '/etc/bind/named.conf.local',
+    target  => "${cfg_dir}/named.conf.local",
     order   => 3,
     content => template("${module_name}/zone.erb")
   }

spec/defines/dns_zone_spec.rb

@@ -1,10 +1,12 @@
 require 'spec_helper'
 
 describe 'dns::zone' do
+  let(:pre_condition) { 'include dns::server::params' }
+
   let(:title) { 'test.com' }
 
   context 'passing something other than an array' do
-    let :facts  do { :concat_basedir => '/dne',  } end
+    let :facts  do { :osfamily => 'Debian', :concat_basedir => '/dne' } end
     let :params do { :allow_transfer => '127.0.0.1' } end
 
     it 'should fail input validation' do
@@ -13,7 +15,7 @@
   end
 
   context 'passing an array to data' do
-    let :facts do { :concat_basedir => '/dne',  } end
+    let :facts  do { :osfamily => 'Debian', :concat_basedir => '/dne' } end
     let :params do
       { :allow_transfer => [ '192.0.2.0', '2001:db8::/32' ],
         :allow_forwarder => ['8.8.8.8', '208.67.222.222']
@@ -53,10 +55,23 @@
       should contain_concat__fragment('named.conf.local.test.com.include').
       with_content(/2001:db8::\/32/)
     }
+
+    it { 
+      should contain_concat('/etc/bind/zones/db.test.com.stage')
+    }
+
+    it { should contain_concat__fragment('db.test.com.soa').
+      with_content(/_SERIAL_/)
+    }
+
+    it {
+      should contain_exec('bump-test.com-serial').
+      with_refreshonly('true')
+    }
   end
 
   context 'when ask to have a only forward policy' do
-    let :facts do { :concat_basedir => '/dne',  } end
+    let :facts  do { :osfamily => 'Debian', :concat_basedir => '/dne' } end
     let :params do
       { :allow_transfer => [],
         :allow_forwarder => ['8.8.8.8', '208.67.222.222'],
@@ -70,7 +85,7 @@
   end
 
   context 'In the default case with no explicit forward policy or forwarder' do
-    let :facts do { :concat_basedir => '/dne',  } end
+    let :facts  do { :osfamily => 'Debian', :concat_basedir => '/dne' } end
     let :params do
       { :allow_transfer => [ '192.0.2.0', '2001:db8::/32' ],
       }

templates/zone_file.erb

@@ -5,7 +5,7 @@
 $ORIGIN	<%= @zone %>.
 $TTL	<%= @zone_ttl %>
 @	IN	SOA	<%= @soa %>. <%= @soa_email %>. (
-		<%= @zone_serial %>		; Serial
+		_SERIAL_		; Serial<%# Be careful at change of number of this line. It is used in zone.pp/Exec[bump-${zone}-serial]. %>
 		<%= @zone_refresh %>		; Refresh
 		<%= @zone_retry %>		; Retry
 		<%= @zone_expire %>		; Expire