Add forwarders to dns::zone; deprecate allow_forwarder

manifests/zone.pp

@@ -47,12 +47,14 @@
 #   the array that matches the requester's address will be used.
 #
 # [*allow_forwarder*]
+#   > **DEPRECATED** in favor of the `forwarders` parameter.  This
+#   > parameter will be removed in the next major release.
 #   An array of IP addresses and optional port numbers to which queries
 #   for this zone will be forwarded (based on the *forward_policy*
 #   setting).  If the optional port number is included, it must be
 #   separated from the IP address by the word `port` - for example, `[
 #   '192.168.100.102 port 1234' ]`.  Defaults to an empty array, which
-#   means no forwarding will be done.
+#   means the global forwarders options will be used.
 #
 # [*allow_query*]
 #   An array of IP addresses from which queries should be allowed
@@ -89,6 +91,19 @@
 #   returns a not-found response, the DNS server will attempt to answer
 #   the request itself.
 #
+# [*forwarders*]
+#   An array of IP addresses and optional port numbers to which queries
+#   for this zone will be forwarded (based on the *forward_policy*
+#   setting).  If the optional port number is included, it must be
+#   separated from the IP address by the word `port` - for example,
+#   `[ '192.168.100.102 port 1234' ]`.  If passed an empty array or the
+#   boolean value `false`, the zone will not forward.  If passed `true`
+#   or left undefined, the zone will use the global forwarders defined
+#   in `dns::server::options`.
+#   *Note* - this parameter deprecates and should be used in place of
+#   the *allow_forwarder* parameter.  If both parameters are passed in,
+#   only *forwarders* will take effect.
+#
 # [*nameservers*]
 #   An array containing the FQDN's of each name server for this zone.
 #   This will be used to create the `NS` records for the zone file.
@@ -176,7 +191,6 @@
   $serial = false,
   $zone_type = 'master',
   $allow_transfer = [],
-  $allow_forwarder = [],
   $allow_query =[],
   $allow_update =[],
   $forward_policy = 'first',
@@ -187,12 +201,44 @@
   $data_dir = $::dns::server::params::data_dir,
   $view = undef,
   $default_zone = false,
+  $forwarders = undef,
+# DEPRECATED, to be removed in the next major release
+  $allow_forwarder = [],
 ) {
 
   $cfg_dir = $dns::server::params::cfg_dir
 
   validate_array($allow_transfer)
+
   validate_array($allow_forwarder)
+  # deprecation notice for allow_forwarder
+  if size($allow_forwarder) > 0 {
+    warning('dns::zone parameter `allow_forwarder` deprecated in favor of `forwarders`')
+    notify { 'dns::zone parameter `allow_forwarder` deprecated in favor of `forwarders`': }
+  }
+
+  # assign $zone_forwarders to the list of forwarders to define for the
+  # zone.  an empty list means *no forwarders*.  set $zone_forwarders to
+  # undef to not define the forwarders list at all (and thereby default
+  # to the forwarders list defined in the global options).
+
+  if $forwarders != undef {
+    if is_bool($forwarders) {
+      if $forwarders {
+        $zone_forwarders = undef
+      } else {
+        $zone_forwarders = []
+      }
+    } else {
+      validate_array($forwarders)
+      $zone_forwarders = $forwarders
+    }
+  } elsif size($allow_forwarder) > 0 {
+    $zone_forwarders = $allow_forwarder
+  } else {
+    $zone_forwarders = undef
+  }
+
   if !member(['first', 'only'], $forward_policy) {
     fail('The forward policy can only be set to either first or only')
   }

spec/defines/dns__zone_spec.rb

@@ -390,5 +390,54 @@
           with_content(/2001:db8::\/32/)
     }
   end
+
+  describe '$allow_forwarder should issue a deprecation notice' do
+    let(:params) {{ :allow_forwarder => ['192.0.2.0'] }}
+    it { should contain_notify('dns::zone parameter `allow_forwarder` deprecated in favor of `forwarders`') }
+  end
+
+  describe 'passing an array to $forwarders' do
+    let(:params) {{ :forwarders => ['192.0.2.0'] }}
+    it 'should have a forwarders entry' do
+        should contain_concat__fragment('named.conf.local.test.com.include').
+                       with_content(/forwarders *{/).
+                       with_content(/192\.0\.2\.0;/)
+    end
+  end
+
+  describe 'passing an empty array to $forwarders' do
+    let(:params) {{ :forwarders => [] }}
+    it 'should have an empty forwarders entry' do
+        should contain_concat__fragment('named.conf.local.test.com.include').
+                       with_content(/forwarders *{[ \n]*}/)
+    end
+  end
+
+  describe 'passing `false` to $forwarders' do
+    let(:params) {{ :forwarders => false }}
+    it 'should have an empty forwarders entry' do
+        should contain_concat__fragment('named.conf.local.test.com.include').
+                       with_content(/forwarders *{[ \n]*}/)
+    end
+  end
+
+  describe 'passing `true` to $forwarders' do
+    let(:params) {{ :forwarders => true }}
+    it 'should not have a forwarders entry' do
+        should contain_concat__fragment('named.conf.local.test.com.include').
+                       without_content(/forwarders *{/)
+    end
+  end
+
+  describe 'passing an array to $forwarders and $allow_forwarder' do
+    let(:params) {{ :forwarders => ['192.0.2.0'], :allow_forwarder => ['192.0.2.1'] }}
+    it 'should have the forwarders entries from $forwarders' do
+        should contain_concat__fragment('named.conf.local.test.com.include').
+                       with_content(/forwarders *{/).
+                       with_content(/192\.0\.2\.0;/).
+                       without_content(/192\.0\.2\.1;/)
+    end
+  end
+
 end
 

templates/zone.erb

@@ -28,10 +28,10 @@ zone "<%= @zone %>" {
         };
     <%- end -%>
 <% end -%>
-<% if !@allow_forwarder.empty? and ( @zone_type == 'master' or  @zone_type == 'forward') -%>
+<% if !@zone_forwarders.nil? and ( @zone_type == 'master' or  @zone_type == 'forward') -%>
     forward <%= @forward_policy %>;
     forwarders {
-    <%- @allow_forwarder.each do |ip| -%>
+    <%- @zone_forwarders.each do |ip| -%>
         <%= ip %>;
     <%- end -%>
     };