Deprecate zone update_policy_rules by more generic update_policy

theforeman · May 11, 2020 · a197ad1 · a197ad1
1 parent 6c5f3c2
commit a197ad1
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 7 deletions.
diff --git a/manifests/zone.pp b/manifests/zone.pp
@@ -49,11 +49,23 @@
   Enum['first', 'only'] $forward                        = 'first',
   Array $forwarders                                     = [],
   Optional[Enum['yes', 'no', 'explicit']] $dns_notify   = undef,
-  Hash[String, Hash[String, Data]] $update_policy_rules = {},
+  Hash[String, Hash[String, Data]] $update_policy_rules = {}, # deprecated
+  Optional[Dns::UpdatePolicy] $update_policy            = undef,
 ) {
 
   $_contact = pick($contact, "root.${zone}.")
 
+  if $update_policy == undef {
+    if $update_policy_rules.length > 0 {
+      warning('update_policy_rules are deprecated in favour of update_policy')
+    }
+    $real_update_policy = $update_policy_rules + {
+      'rndc-key' => {'matchtype' => 'zonesub', 'rr' => 'ANY'}
+    }
+  } else {
+    $real_update_policy = $update_policy
+  }
+
   $zonefilename = "${zonefilepath}/${filename}"
 
   if $dns::enable_views {

diff --git a/spec/defines/dns_zone_spec.rb b/spec/defines/dns_zone_spec.rb
@@ -330,9 +330,63 @@
 
       end
 
-      context 'update_policy_rules is set' do
+      context 'update_policy with multiple declarations' do
+        let(:params) { {
+          :update_policy => {
+            'foreman_key' => {
+              'matchtype' => 'zonesub',
+              'rr'        => 'ANY'
+            },
+            'goreman_key' => {
+              'action'    => 'deny',
+              'matchtype' => 'subdomain',
+              'rr'        => 'ANY'
+            },
+          }
+        } }
+
+        it "should have valid zone configuration" do
+          verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+          'zone "example.com" {',
+          '    type master;',
+          "    file \"#{zonefilepath}/db.example.com\";",
+          '    update-policy {',
+          '            grant foreman_key zonesub ANY;',
+          '            deny goreman_key subdomain ANY;',
+          '    };',
+          '};',
+        ])
+        end
+      end
+
+      context 'deprecated update_policy_rules' do
         let(:params) { {
           :update_policy_rules => {
+            'foreman_key' => {
+              'action'    => 'grant',
+              'matchtype' => 'zonesub',
+              'rr'        => 'ANY'
+            },
+          }
+        } }
+
+        it "should have valid zone configuration" do
+          verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+          'zone "example.com" {',
+          '    type master;',
+          "    file \"#{zonefilepath}/db.example.com\";",
+          '    update-policy {',
+          '            grant rndc-key zonesub ANY;',
+          '            grant foreman_key zonesub ANY;',
+          '    };',
+          '};',
+        ])
+        end
+      end
+
+      context 'update_policy uses non-default key' do
+        let(:params) { {
+          :update_policy => {
             'foreman_key' => {
               'matchtype' => 'zonesub',
               'tname' => '*',
@@ -341,8 +395,47 @@
           }
         } }
 
-        it "should have valid slave zone configuration" do
-          is_expected.to compile
+        it "should have valid zone configuration" do
+          verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+          'zone "example.com" {',
+          '    type master;',
+          "    file \"#{zonefilepath}/db.example.com\";",
+          '    update-policy {',
+          '            grant foreman_key zonesub * ANY;',
+          '    };',
+          '};',
+        ])
+        end
+      end
+
+      context 'update_policy set to local' do
+        let(:params) { {
+          :update_policy => 'local',
+        } }
+
+        it "should have valid zone configuration" do
+          verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+          'zone "example.com" {',
+          '    type master;',
+          "    file \"#{zonefilepath}/db.example.com\";",
+          '    update-policy local;',
+          '};',
+        ])
+        end
+      end
+
+      context 'update_policy set to none' do
+        let(:params) { {
+          :update_policy => 'none',
+        } }
+
+        it "should have valid zone configuration" do
+          verify_concat_fragment_exact_contents(catalogue, 'dns_zones+10__GLOBAL__example.com.dns', [
+          'zone "example.com" {',
+          '    type master;',
+          "    file \"#{zonefilepath}/db.example.com\";",
+          '};',
+        ])
         end
       end
     end

diff --git a/templates/named.zone.erb b/templates/named.zone.erb
@@ -10,12 +10,17 @@ zone "<%= @zone %>" {
     file "<%= @zonefilename %>";
 <% end -%>
 <% if @zonetype == 'master' -%>
+<%   if @real_update_policy.is_a? String -%>
+<%     if @real_update_policy != 'none' -%>
+    update-policy <%= @real_update_policy %>;
+<%     end -%>
+<%   else -%>
     update-policy {
-            grant rndc-key zonesub ANY;
-            <%- @update_policy_rules.sort_by {|k, v| k}.each do |key, key_hash| -%>
-            grant <%= key %> <%= key_hash['matchtype'] %> <% if key_hash['tname'] %><%= key_hash['tname'] %> <% end %><% if key_hash['rr'] %><%= key_hash['rr'] %><% end %>;
+            <%- @real_update_policy.sort_by {|k, v| k}.each do |key, key_hash| -%>
+            <%= key_hash['action'] || 'grant' %> <%= key %> <%= key_hash['matchtype'] %> <% if key_hash['tname'] %><%= key_hash['tname'] %> <% end %><%= key_hash['rr'] %>;
             <%- end -%>
     };
+<%   end -%>
 <% end -%>
 <% unless @zonetype == 'forward' -%>
 <% unless @allow_transfer.empty? -%>

diff --git a/types/updatepolicy.pp b/types/updatepolicy.pp
@@ -0,0 +1,30 @@
+# Validate update-policy parameter
+type Dns::UpdatePolicy = Variant[
+  Enum['none', 'local'],
+  Hash[
+    String,
+    Struct[{
+      Optional[action] => Enum['deny', 'grant'],
+      Optional[tname]  => String,
+      rr               => String,
+      matchtype        => Enum[
+        '6to4-self',
+        'external',
+        'krb5-self',
+        'krb5-selfsub',
+        'krb5-subdomain',
+        'ms-self',
+        'ms-selfsub',
+        'ms-subdomain',
+        'name',
+        'self',
+        'selfsub',
+        'selfwild',
+        'subdomain',
+        'tcp-self',
+        'wildcard',
+        'zonesub',
+      ],
+    }],
+  ],
+]