Permit overriding global forwarders with an empty list (per zone)

manifests/key.pp

@@ -15,7 +15,6 @@
     command     => "/usr/sbin/dnssec-keygen -a HMAC-MD5 -r /dev/urandom -b 128 -n USER ${name}",
     cwd         => "${cfg_dir}/bind.keys.d",
     require     => [
-      Package['dnssec-tools'],
       File["${cfg_dir}/bind.keys.d"],
     ],
     refreshonly => true,

manifests/server/options.pp

@@ -108,7 +108,11 @@
 #    forwarders => [ '8.8.8.8', '8.8.4.4' ],
 #   }
 #
-include dns::server::params
+
+# This is causing 'Server Error: This Function Call is unacceptable as a top level construct in this location' errors.
+# It's not clear whether this has to be moved, or whether it can simply be removed.
+#include dns::server::params
+
 define dns::server::options (
   $allow_query = [],
   $allow_recursion = [],

manifests/server/params.pp

@@ -18,14 +18,7 @@
       $default_template   = 'default.debian.erb'
       $default_dnssec_enable     = true
       $default_dnssec_validation = 'auto'
-      case $::operatingsystemmajrelease {
-        '8': {
-          $necessary_packages = ['bind9']
-        }
-        default: {
-          $necessary_packages = [ 'bind9', 'dnssec-tools' ]
-        }
-      }
+      $necessary_packages = ['bind9']
     }
     'RedHat': {
       $cfg_dir            = '/etc/named'

manifests/tsig.pp

@@ -28,11 +28,12 @@
   $cfg_dir   = $dns::server::params::cfg_dir # Used in a template
   validate_string($name)
 
-  concat::fragment { "named.conf.local.tsig.${name}.include":
-    ensure  => $ensure,
-    target  => "${cfg_dir}/named.conf.local",
-    order   => 4,
-    content => template("${module_name}/tsig.erb"),
+  if $ensure == present {
+    concat::fragment { "named.conf.local.tsig.${name}.include":
+      target  => "${cfg_dir}/named.conf.local",
+      order   => 4,
+      content => template("${module_name}/tsig.erb"),
+    }
   }
 
 }

manifests/zone.pp

@@ -166,7 +166,7 @@
   $serial = false,
   $zone_type = 'master',
   $allow_transfer = [],
-  $allow_forwarder = [],
+  $allow_forwarder = undef,
   $allow_query =[],
   $allow_update =[],
   $forward_policy = 'first',
@@ -180,7 +180,9 @@
   $cfg_dir = $dns::server::params::cfg_dir
 
   validate_array($allow_transfer)
-  validate_array($allow_forwarder)
+  if $allow_forwarder != undef {
+    validate_array($allow_forwarder)
+  }
   if !member(['first', 'only'], $forward_policy) {
     fail('The forward policy can only be set to either first or only')
   }

spec/classes/dns__server__install_spec.rb

@@ -8,7 +8,7 @@
   context "on a Debian OS with default params" do
     let(:facts) {{ :osfamily => 'Debian' }}
     it { should contain_class('dns::server::params') }
-    ['bind9', 'dnssec-tools'].each do |package|
+    ['bind9'].each do |package|
         it do
           should contain_package(package).with({
             'ensure' => 'latest',
@@ -21,7 +21,7 @@
     let(:facts)  {{ :osfamily        => 'Debian'  }}
     let(:params) {{ :ensure_packages => 'present' }}
     it { should contain_class('dns::server::params') }
-    ['bind9', 'dnssec-tools'].each do |package|
+    ['bind9'].each do |package|
         it do
           should contain_package(package).with({
             'ensure' => 'present',

spec/classes/server/default_spec.rb

@@ -32,10 +32,10 @@
         it { should contain_file('/etc/default/bind9').with_content(/OPTIONS="-u bind -6"/) }
       end
 
-      context "requires bind9 and dnssec-tools package" do
+      context "requires bind9 package" do
         it do
           should contain_file('/etc/default/bind9').with({
-            'require' => ['Package[bind9]', 'Package[dnssec-tools]'],
+            'require' => ['Package[bind9]'],
           })
         end
       end

templates/zone.erb

@@ -20,29 +20,33 @@ zone "<%= @zone %>" {
     masters { <%= @slave_masters %>;};
 <%- end -%>
 <% elsif @zone_type == 'master' -%>
-    <%- if @allow_transfer.is_a?(Array) and @allow_transfer.size != 0 -%>
-        allow-transfer {
-        <%- @allow_transfer.each do |ip| -%>
-            <%= ip %>;
-        <%- end -%>
-        };
+<%- if @allow_transfer.is_a?(Array) and @allow_transfer.size != 0 -%>
+    allow-transfer {
+    <%- @allow_transfer.each do |ip| -%>
+        <%= ip %>;
     <%- end -%>
+    };
+<%- end -%>
 <% end -%>
-<% if !@allow_forwarder.empty? and ( @zone_type == 'master' or  @zone_type == 'forward') -%>
+<%- if ['master','slave','stub','forward'].include? @zone_type and @allow_forwarder.is_a?(Array) -%>
+<%- if @allow_forwarder.size != 0 -%>
     forward <%= @forward_policy %>;
     forwarders {
     <%- @allow_forwarder.each do |ip| -%>
         <%= ip %>;
     <%- end -%>
     };
-<% end -%>
-    <%- if @allow_query.size != 0 %>
-        allow-query {
-        <%- @allow_query.each do |ip| -%>
-            <%= ip %>;
-        <%- end -%>
-        };
+<%- else -%>
+    forwarders {};
+<%- end -%>
+<%- end -%>
+<%- if @allow_query.size != 0 %>
+    allow-query {
+    <%- @allow_query.each do |ip| -%>
+        <%= ip %>;
     <%- end -%>
+    };
+<%- end -%>
 <%- if @allow_update.size != 0 %>
     allow-update {
     <%- @allow_update.each do |ip| -%>