From c88f5ec19e2b418df80b4e3a92d2dde6864f021d Mon Sep 17 00:00:00 2001
From: Andreas Ntaflos <andreas.ntaflos@rise-world.com>
Date: Wed, 8 May 2019 18:00:51 +0200
Subject: [PATCH] Validate named.conf and zones.conf using named-checkconf

Make use of the `validate_cmd` parameter in concat to run
named-checkconf on the target file. This way the validity of to-be-updated
configuration files named.conf or zones.conf is checked before actually
writing the new configuration and restarting the named service. This
prevents named from loading invalid configuration settings that would
result in named failing to start.

The file named.conf.options itself cannot be checked with
named-checkconf because its content is only valid inside the
"options { };" directive.
---
 manifests/config.pp           | 20 ++++++++++++++++----
 manifests/params.pp           |  4 ++++
 manifests/view.pp             |  1 +
 spec/classes/dns_init_spec.rb |  6 ++++--
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/manifests/config.pp b/manifests/config.pp
index fda55e75..787335dc 100644
--- a/manifests/config.pp
+++ b/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::params::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::params::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',
diff --git a/manifests/params.pp b/manifests/params.pp
index a18f370e..08ad07ba 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -16,6 +16,7 @@
       $user               = 'bind'
       $group              = 'bind'
       $rndcconfgen        = '/usr/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/sbin/named-checkconf'
     }
     'RedHat': {
       $dnsdir             = '/etc'
@@ -31,6 +32,7 @@
       $user               = 'named'
       $group              = 'named'
       $rndcconfgen        = '/usr/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/sbin/named-checkconf'
     }
     /^(FreeBSD|DragonFly)$/: {
       $dnsdir             = '/usr/local/etc/namedb'
@@ -46,6 +48,7 @@
       $user               = 'bind'
       $group              = 'bind'
       $rndcconfgen        = '/usr/local/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/local/sbin/named-checkconf'
     }
     'Archlinux': {
       $dnsdir             = '/etc'
@@ -61,6 +64,7 @@
       $user               = 'named'
       $group              = 'named'
       $rndcconfgen        = '/usr/sbin/rndc-confgen'
+      $named_checkconf    = '/usr/sbin/named-checkconf'
     }
     default: {
       fail ("Unsupported operating system family ${facts['osfamily']}")
diff --git a/manifests/view.pp b/manifests/view.pp
index 03ce4e9b..1670544c 100644
--- a/manifests/view.pp
+++ b/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":
diff --git a/spec/classes/dns_init_spec.rb b/spec/classes/dns_init_spec.rb
index 86dbf26e..bc1c0a86 100644
--- a/spec/classes/dns_init_spec.rb
+++ b/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";',
@@ -281,7 +282,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";',