CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## [6.2.0](https://github.com/theforeman/puppet-dns/tree/6.2.0) (2019-07-18)
+
+[Full Changelog](https://github.com/theforeman/puppet-dns/compare/6.1.0...6.2.0)
+
+**Implemented enhancements:**
+
+- Validate named.conf and zones.conf using named-checkconf [\#144](https://github.com/theforeman/puppet-dns/pull/144) ([antaflos](https://github.com/antaflos))
+- Allow setting service restart command [\#143](https://github.com/theforeman/puppet-dns/pull/143) ([antaflos](https://github.com/antaflos))
+- Don't set forbidden zone options for zone type 'forward' [\#142](https://github.com/theforeman/puppet-dns/pull/142) ([antaflos](https://github.com/antaflos))
+
 ## [6.1.0](https://github.com/theforeman/puppet-dns/tree/6.1.0) (2019-06-12)
 
 [Full Changelog](https://github.com/theforeman/puppet-dns/compare/6.0.0...6.1.0)

manifests/config.pp

@@ -6,9 +6,10 @@
   }
 
   concat { $dns::publicviewpath:
-    owner => root,
-    group => $dns::params::group,
-    mode  => '0640',
+    owner        => root,
+    group        => $dns::params::group,
+    mode         => '0640',
+    validate_cmd => "${dns::named_checkconf} %",
   }
 
   if $dns::enable_views {
@@ -19,13 +20,24 @@
       mode   => '0755',
     }
   }
+
   concat::fragment { 'dns_zones+01-header.dns':
     target  => $dns::publicviewpath,
     content => ' ',
     order   => '01',
   }
 
-  concat { [$dns::namedconf_path, $dns::optionspath]:
+  concat { $dns::namedconf_path:
+    owner        => root,
+    group        => $dns::params::group,
+    mode         => '0640',
+    require      => Concat[$dns::optionspath],
+    validate_cmd => "${dns::named_checkconf} %",
+  }
+
+  # This file cannot be checked by named-checkconf because its content is only
+  # valid inside an "options { };" directive.
+  concat { $dns::optionspath:
     owner => root,
     group => $dns::params::group,
     mode  => '0640',

manifests/init.pp

@@ -93,6 +93,14 @@
 #   The ensure attribute on the service
 # @param service_enable
 #   Whether to enable the service (start at boot)
+# @param service_restart_command
+#   Custom command to use when the service will be restarted (notified by
+#   configuration changes). Will be passed directly to the restart parameter of
+#   the contained service resource. This is useful when you want BIND to reload
+#   its configuration instead of restarting the whole process, for example by
+#   setting `service_restart_command` to `/usr/sbin/service bind9 reload` or
+#   `/usr/sbin/rndc reload` or even `/usr/bin/systemctl try-reload-or-restart bind9`.
+#   Default is 'undef' so the service resource default is used.
 # @param additional_options
 #   Additional options
 # @param additional_directives
@@ -143,6 +151,7 @@
   Hash[String, Hash[String, Data]] $controls                        = $dns::params::controls,
   Variant[Enum['running', 'stopped'], Boolean] $service_ensure      = $dns::params::service_ensure,
   Boolean $service_enable                                           = $dns::params::service_enable,
+  Optional[String[1]] $service_restart_command                      = $dns::params::service_restart_command,
   Hash[String, Data] $additional_options                            = $dns::params::additional_options,
   Array[String] $additional_directives                              = $dns::params::additional_directives,
   Boolean $enable_views                                             = $dns::params::enable_views,

manifests/params.pp

@@ -16,6 +16,7 @@
       $user               = 'bind'
       $group              = 'bind'
       $rndcconfgen        = '/usr/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/sbin/named-checkconf'
       $sysconfig_file     = '/etc/default/bind9'
       $sysconfig_template = "dns/sysconfig.${facts['osfamily']}.erb"
       $sysconfig_startup_options = '-u bind'
@@ -38,6 +39,7 @@
       $user               = 'named'
       $group              = 'named'
       $rndcconfgen        = '/usr/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/sbin/named-checkconf'
       $sysconfig_file     = '/etc/sysconfig/named'
       $sysconfig_template = "dns/sysconfig.${facts['osfamily']}.erb"
       $sysconfig_startup_options = undef
@@ -60,6 +62,7 @@
       $user               = 'bind'
       $group              = 'bind'
       $rndcconfgen        = '/usr/local/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/local/sbin/named-checkconf'
       # The sysconfig settings are not relevant for FreeBSD
       $sysconfig_file     = undef
       $sysconfig_template = undef
@@ -81,6 +84,7 @@
       $user               = 'named'
       $group              = 'named'
       $rndcconfgen        = '/usr/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/sbin/named-checkconf'
       # The sysconfig settings are not relevant for ArchLinux
       $sysconfig_file     = undef
       $sysconfig_template = undef
@@ -96,6 +100,11 @@
   # This module will manage the system group by default
   $group_manage = true
 
+  # Don't set any restart command by default, let Puppet use the
+  # platform-dependent service resource default when handling the service
+  # restart.
+  $service_restart_command = undef
+
   $namedconf_template    = 'dns/named.conf.erb'
   $optionsconf_template  = 'dns/options.conf.erb'
 

manifests/service.pp

@@ -6,5 +6,6 @@
     enable     => $dns::service_enable,
     hasstatus  => true,
     hasrestart => true,
+    restart    => $dns::service_restart_command,
   }
 }

manifests/view.pp

@@ -36,6 +36,7 @@
     group  => $dns::params::group,
     mode   => '0640',
     notify => Service[$dns::namedservicename],
+    before => Concat[$dns::publicviewpath],
   }
 
   concat::fragment { "dns_view_header_${title}.dns":

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "theforeman-dns",
-  "version": "6.1.0",
+  "version": "6.2.0",
   "author": "theforeman",
   "summary": "Manage the ISC BIND daemon",
   "license": "Apache-2.0",

spec/classes/dns_init_spec.rb

@@ -34,7 +34,8 @@
           'allow-recursion { localnets; localhost; };'
       ])}
 
-      it { should contain_concat('/etc/named.conf') }
+      it { should contain_concat('/etc/named/zones.conf').with_validate_cmd('/usr/sbin/named-checkconf %') }
+      it { should contain_concat('/etc/named.conf').with_validate_cmd('/usr/sbin/named-checkconf %') }
       it { verify_concat_fragment_exact_contents(catalogue, 'named.conf+10-main.dns', [
           '// named.conf',
           'include "/etc/rndc.key";',
@@ -83,7 +84,7 @@
         )
       }
 
-      it { should contain_service('named').with_ensure('running').with_enable(true) }
+      it { should contain_service('named').with_ensure('running').with_enable(true).with_restart(nil) }
     end
 
     describe 'with unmanaged localzonepath' do
@@ -212,6 +213,16 @@
       it { should contain_service('named').with_ensure('running').with_enable(false) }
     end
 
+    describe 'with service_restart_command set to "/usr/sbin/service bind9 reload' do
+      let(:params) { {:service_restart_command => '/usr/sbin/service bind9 reload'} }
+      it {
+        should contain_service('named')
+          .with_ensure('running')
+          .with_enable(true)
+          .with_restart('/usr/sbin/service bind9 reload')
+      }
+    end
+
     describe 'with group_manage false' do
       let(:params) { {:group_manage => false} }
       it { should_not contain_group('named') }
@@ -406,7 +417,8 @@
            'allow-recursion { localnets; localhost; };'
       ])}
 
-      it { should contain_concat('/usr/local/etc/namedb/named.conf') }
+      it { should contain_concat('/usr/local/etc/namedb/zones.conf').with_validate_cmd('/usr/local/sbin/named-checkconf %') }
+      it { should contain_concat('/usr/local/etc/namedb/named.conf').with_validate_cmd('/usr/local/sbin/named-checkconf %') }
       it { verify_concat_fragment_exact_contents(catalogue, 'named.conf+10-main.dns', [
           '// named.conf',
           'include "/usr/local/etc/namedb/rndc.key";',

spec/defines/dns_zone_spec.rb

@@ -263,6 +263,45 @@
 
   end
 
+  context 'when zonetype => forward' do
+    let(:params) {{ :zonetype => 'forward', :forward => 'only', :forwarders => ['192.168.3.4', '192.168.5.6'] }}
+
+    it "should have valid forward zone configuration" do
+      verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+        'zone "example.com" {',
+        '    type forward;',
+        '    forward only;',
+        '    forwarders { 192.168.3.4; 192.168.5.6; };',
+        '};',
+      ])
+    end
+
+    context 'when parameters are set that are not allowed options for zonetype => forward' do
+      let(:params) {{
+        :zonetype => 'forward',
+        :forwarders => ['192.168.3.4', '192.168.5.6'],
+        :manage_file => true,
+        :manage_file_name => true,
+        :masters  => ['192.168.1.1', '192.168.1.2'],
+        :allow_transfer => ['192.168.100.1'],
+        :allow_query => ['192.168.100.1'],
+        :also_notify => ['192.168.100.1'],
+        :dns_notify => 'explicit'
+      }}
+
+      it "should have valid forward zone configuration without options that are not allowed" do
+        verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+          'zone "example.com" {',
+          '    type forward;',
+          '    forward first;',
+          '    forwarders { 192.168.3.4; 192.168.5.6; };',
+          '};',
+        ])
+      end
+    end
+
+  end
+
   context 'update_policy_rules is set' do
     let(:params) { {
       :update_policy_rules => {

templates/named.zone.erb

@@ -6,7 +6,7 @@ zone "<%= @zone %>" {
     forwarders { <%= @forwarders.join('; ') %>; };
 <% end -%>
 <% end -%>
-<% if @manage_file or @manage_file_name -%>
+<% if (@manage_file || @manage_file_name) && @zonetype != 'forward' -%>
     file "<%= @zonefilename %>";
 <% end -%>
 <% if @zonetype == 'master' -%>
@@ -17,6 +17,7 @@ zone "<%= @zone %>" {
             <%- end -%>
     };
 <% end -%>
+<% unless @zonetype == 'forward' -%>
 <% unless @allow_transfer.empty? -%>
     allow-transfer { <%= @allow_transfer.join('; ') %>; };
 <% end -%>
@@ -32,4 +33,5 @@ zone "<%= @zone %>" {
 <% if @_dns_notify -%>
     notify <%= @_dns_notify %>;
 <% end -%>
+<% end -%>
 };