work

7 files changed, 295 insertions, 153 deletions

diff --git a/lib/puppet/provider/dns_record2/named.rb b/lib/puppet/provider/dns_record2/named.rb
index 13fe5ee..88bc73e 100644
--- a/lib/puppet/provider/dns_record2/named.rb
+++ b/lib/puppet/provider/dns_record2/named.rb
@@ -1,14 +1,8 @@
 Puppet::Type.type(:dns_record2).provide(:named) do
   def self.instances
-    []
+    raise _('Listing DNS records not supported.')
   end
 
-  # Create and destroy don't do anything on their own,
-  # See comment in type
-  def create; end
-
-  def destroy; end
-
   def exists?
     record != nil
   end
@@ -25,21 +19,14 @@ Puppet::Type.type(:dns_record2).provide(:named) do
     when String
       t
     else
-      "INVALID VALUE (#{t.class}, #{t})"
+      raise "INVALID VALUE (#{t.class}, #{t})"
     end
   end
 
-  # TODO: should we have this?
-  def value=(v); end
-
   def key
     resource[:key]
   end
 
-  # def key=(v)
-  #   resource[:key] = v
-  # end
-
   def zone
     resource[:zone]
   end
diff --git a/lib/puppet/provider/dns_zone2/named.rb b/lib/puppet/provider/dns_zone2/named.rb
index c1e39c5..3a59c81 100644
--- a/lib/puppet/provider/dns_zone2/named.rb
+++ b/lib/puppet/provider/dns_zone2/named.rb
@@ -1,5 +1,4 @@
 Puppet::Type.type(:dns_zone2).provide(:named) do
-
   def self.instances
     `named-checkconf -l`.split("\n").map do |record|
       name, cls, view, type = record.split(' ')
@@ -7,20 +6,10 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
     end
   end
 
-  def create(records)
-    print("Create #{resource[:name]}\n")
-  end
-
-  def destroy
-    print("Remove #{resource[:name]}\n")
-  end
-
   def exists?
-    # instances.find { |r| r.name == resource[:name] }
-    # resource[:ensure] == :present
     `named-checkconf -l`
       .split("\n")
-      .grep(%r{#{resource[:origin]} })
+      .grep(%r{^#{origin_rx} })
       .empty?
       .!
   end
@@ -29,7 +18,7 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
     "/var/named/zones/#{resource[:name]}db"
   end
 
-  def zone_content(records)
+  def zone_content(records, serial_change)
     content = <<~EOF
     ; File managed by Puppet.
     ; Local changes WILL be overwritten
@@ -39,7 +28,7 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
     $TTL #{resource[:default_ttl]}
 
     @	#{resource[:soa_ttl]}	IN	SOA	#{resource[:mname]} #{resource[:rname]} (
-                  #{serial + 1}	; serial
+                  #{serial + serial_change}	; serial
                   #{resource[:refresh]}	; refresh
                   #{resource[:retry]}	; retry
                   #{resource[:expire]}	; expire
@@ -65,7 +54,13 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
     content
   end
 
+  # The current (before updating) serial of the zone
   def serial
     `rndc zonestatus #{name} | awk -F' ' '/^serial:/ { print $2 }'`.to_i
   end
+
+  # A regex matching the origin
+  def origin_rx
+    resource[:origin].gsub(%r{[.]}, '[.]')
+  end
 end
diff --git a/lib/puppet/type/dns_record2.rb b/lib/puppet/type/dns_record2.rb
index 1aceaf9..098d27d 100644
--- a/lib/puppet/type/dns_record2.rb
+++ b/lib/puppet/type/dns_record2.rb
@@ -1,3 +1,4 @@
+# TODO: views
 Puppet::Type.newtype(:dns_record2) do
   @doc = <<~EOF
   A single DNS record.
@@ -7,25 +8,9 @@ Puppet::Type.newtype(:dns_record2) do
   coresponding dns_zone2 resource.
   EOF
 
-  newproperty(:ensure) do
-    newvalue(:present) do
-      provider.create
-    end
-
-    newvalue(:absent) do
-      provider.destroy
-    end
-
-    defaultto :present
+  autonotify(:dns_zone2) { || [value(:zone)] }
 
-    def retrieve
-      if provider.exists?
-        :present
-      else
-        :absent
-      end
-    end
-  end
+  ensurable
 
   def exists?
     provider.exists?
@@ -89,8 +74,4 @@ Puppet::Type.newtype(:dns_record2) do
     TTL of this record.
     EOF
   end
-
-  autonotify(:dns_zone2) { || [value(:zone)] }
-
-  # TODO views
 end
diff --git a/lib/puppet/type/dns_zone2.rb b/lib/puppet/type/dns_zone2.rb
index ff0cf02..e21af6d 100644
--- a/lib/puppet/type/dns_zone2.rb
+++ b/lib/puppet/type/dns_zone2.rb
@@ -1,3 +1,5 @@
+require 'puppet/type/file'
+
 Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
   @doc = <<~EOF
   A complete DNS zonefile.
@@ -9,40 +11,81 @@ Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
      when something else has actually changed in the zone.
   EOF
 
-  newproperty(:ensure) do
-    newvalue(:present) do
-      # @resource.content = resource.should_content
-      # provider.write_zone(resource.should_content)
-      print("Present resource\n")
+  ensurable
+
+  def refresh
+    catalog.resource("File[/var/named/zones/#{self[:name]}db]")[:content] = should_content(1)
+  end
+
+  # List of all DNS records (at all)
+  #
+  # Should ideally be limited to records in this zone here instead of
+  # further down.
+  def records
+    catalog.resources.filter do |r|
+      r.is_a?(Puppet::Type.type(:dns_record2))
     end
+  end
+
+  # Create the file resource for us.
+  # We also propagate some parameters and metaparameters to the
+  # underlying file here
+  def generate
+    params = {
+      ensure: self[:ensure],
+      path: provider.filename,
+      # notify: self[:notify],
+      validate_cmd: "/usr/sbin/named-checkzone #{self[:origin]} %"
+    }
 
-    # This should ideally remove the zone. This is however managed
-    # "higher" up in the puppet code.
-    # dns::init has a purge on the directory, and the named
-    # configuration file is fully managed by us.
-    # This means that decomissioning is as simple as removing the zone
-    # from the puppet environment.
-    newvalue(:absent) do
-      # provider.destroy
+    [:owner,
+     :group,
+     :backup,
+     :selinux_ignore_defaults,
+     :selrange,
+     :selrole,
+     :seltype,
+     :seluser,
+     :show_diff].each do |param|
+      params[param] = self[param] unless self[param].nil?
     end
 
-    defaultto :present
+    excluded_metaparams = [:before, :notify, :require, :subscribe, :tag]
 
-    def retrieve
-      if provider.exists?
-        :present
-      else
-        :absent
+    Puppet::Type.metaparams.each do |metaparam|
+      unless self[metaparam].nil? || excluded_metaparams.include?(metaparam)
+        params[metaparam] = self[metaparam]
       end
     end
+
+    [Puppet::Type.type(:file).new(params)]
   end
 
-  def refresh
-    print("Refreshing zone\n")
-    catalog.resource("File[/var/named/zones/#{self[:name]}db]")[:content] = should_content
+  # Returning "our" file resource causes errors on the file to
+  # propagate out from us (which is NEEDED for `validate_cmd` to
+  # work).
+  #
+  # This always sets the content, but is run is *before* refresh
+  # eventns. This means that this works for creation events, but not
+  # refresh events. For refresh events we simply generate the content
+  # a second time, overriding this instance.
+  #
+  # Ideally we wouldn't generate content here, but instead in
+  # `create`. But that method isn't called
+  def eval_generate
+    catalog.resource("File[/var/named/zones/#{self[:name]}db]")[:content] = should_content(0)
+    [catalog.resource("File[#{provider.filename}]")]
+  end
+
+  def should_content(serial_change)
+    provider.zone_content(records, serial_change)
   end
 
   newparam(:origin, namevar: true) do
+    desc <<-EOF
+    Origin parameter of the source.
+    Should almost always be the same as the name of the zone.
+    EOF
     munge do |value|
       if value[-1] == '.'
         value
@@ -53,14 +96,27 @@ Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
   end
 
   newparam(:cls) do
+    desc <<-EOF
+    DNS class of the zone.
+    EOF
     defaultto :IN
   end
 
   newparam(:view) do
+    desc <<-EOF
+    Named view this zone belongs to.
+    TODO: support views
+    EOF
     defaultto '_default'
   end
 
   newparam(:type) do
+    desc <<-EOF
+    The type of the zone.
+    TODO: support types
+    Currently only master is tested.
+    EOF
+
     newvalues(:master, :slave,
               :mirror, :hint, :stub, :static_stub, :forward, :redirect)
     aliasvalue :primary, :master
@@ -69,10 +125,18 @@ Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
   end
 
   newparam(:default_ttl) do
+    desc <<-EOF
+    TTL for all records in zone without an explicit TTL set.
+    EOF
     defaultto '300'
   end
 
   newparam(:mname) do
+    desc <<-EOF
+    Domain name of zone master.
+
+    Part of the SOA record.
+    EOF
     munge do |value|
       if value[-1] == '.'
         value
@@ -83,6 +147,11 @@ Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
   end
 
   newparam(:rname) do
+    desc <<-EOF
+    Email to the zone admin, but with the '@' replaced with a period ('.')
+
+    Part of the SOA record.
+    EOF
     munge do |value|
       if value[-1] == '.'
         value
@@ -93,45 +162,120 @@ Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
   end
 
   newparam(:soa_ttl) do
+    desc <<-EOF
+    TTL of the SOA record.
+    EOF
   end
 
   newparam(:refresh) do
+    desc <<-EOF
+    Refresh value in the SOA record.
+    EOF
   end
 
   newparam(:retry) do
+    desc <<-EOF
+    Retry value in the SOA record.
+    EOF
   end
 
   newparam(:expire) do
+    desc <<-EOF
+    Expire value in the SOA record.
+    EOF
   end
 
   newparam(:negative_ttl) do
+    desc <<-EOF
+    Negative TTL value in the SOA value.
+
+    This file is sometimes call Minimum TTL due to historical reasons.
+    EOF
   end
 
-  # List of all DNS records (at all)
-  #
-  # Should ideally be limited to records in this zone here instead of
-  # further down.
-  def records
-    catalog.resources.filter do |r|
-      r.is_a?(Puppet::Type.type(:dns_record2))
+  newparam(:owner, parent: Puppet::Type::File::Owner) do
+    desc <<-EOF
+    Owner of the zonefile.
+    EOF
+  end
+
+  newparam(:group, parent: Puppet::Type::File::Group) do
+    desc <<-EOF
+    Group of the zonefile.
+    EOF
+  end
+
+  newparam(:backup) do
+    desc <<-EOF
+    Should the pre-existing file be backed up.
+
+    See the file resource's documentation for details.
+    EOF
+
+    validate do |value|
+      unless [TrueClass, FalseClass, String].include?(value.class)
+        raise ArgumentError, _('Backup must be a Boolean or String')
+      end
     end
   end
 
-  # Create the file resource for us.
-  def generate
-    print("Generate\n")
-    params = {
-      ensure: :present,
-      path: provider.filename,
-      # notify: self[:notify],
-    }
+  newparam(:selinux_ignore_defaults, boolean: true, parent: Puppet::Parameter::Boolean) do
+    desc <<-EOF
+    See the file resource's documentation for details.
+    EOF
+  end
 
-    [Puppet::Type.type(:file).new(params)]
+  newparam(:selrange) do
+    desc <<-EOF
+    See the file resource's documentation for details.
+    EOF
+
+    validate do |value|
+      unless value.is_a?(String)
+        raise ArgumentError, _('Selrange must be a String')
+      end
+    end
   end
 
-  def should_content
-    print("Getting content\n")
-    provider.zone_content(records)
+  newparam(:selrole) do
+    desc <<-EOF
+    See the file resource's documentation for details.
+    EOF
+
+    validate do |value|
+      unless value.is_a?(String)
+        raise ArgumentError, _('Selrole must be a String')
+      end
+    end
   end
 
+  newparam(:seltype) do
+    desc <<-EOF
+    See the file resource's documentation for details.
+    EOF
+
+    validate do |value|
+      unless value.is_a?(String)
+        raise ArgumentError, _('Seltype must be a String')
+      end
+    end
+  end
+
+  newparam(:seluser) do
+    desc <<-EOF
+    See the file resource's documentation for details.
+    EOF
+
+    validate do |value|
+      unless value.is_a?(String)
+        raise ArgumentError, _('Seluser must be a String')
+      end
+    end
+  end
+
+  newparam(:show_diff, boolean: true, parent: Puppet::Parameter::Boolean) do
+    desc <<-EOF
+    See the file resource's documentation for details.
+    EOF
+  end
 end
diff --git a/manifests/init.pp b/manifests/init.pp
index a3e073a..d80cea3 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -104,11 +104,17 @@ class dns (
     ensure => file,
   }
 
+  $warn = @(EOF)
+    #
+    # File managed by Puppet. Local changes WILL be overwritter',
+    #
+    | EOF
+
   concat { $config_file:
     ensure_newline => true,
-    warn           => '# File managed by Puppet. Local changes WILL be overwritter',
     validate_cmd   => "${checkconf} %",
     notify         => Service[$servicename],
+    warn           => $warn,
   }
 
   concat::fragment { 'named.conf main configuration':
diff --git a/manifests/zone.pp b/manifests/zone.pp
index 828b887..bf35a52 100644
--- a/manifests/zone.pp
+++ b/manifests/zone.pp
@@ -41,6 +41,28 @@
 #   Defaults to true if an update_policy is set.
 # @param soa_ttl
 #   TTL of SOA record.
+#
+# @param owner
+#   Owner of zonefile.
+# @param group
+#   Group of zonefile.
+# @param backup
+#   Should a backup be created. See file resources documentation.
+# @param selinux_ignore_defaults
+#   See file resources documentation.
+# @param selrange
+#   See file resources documentation.
+# @param selrole
+#   See file resources documentation.
+# @param seltype
+#   See file resources documentation.
+# @param seluser
+#   See file resources documentation.
+# @param show_diff
+#   Should a diff be shown.
+#
+# @param ensure
+#   Should this zone be present
 define dns::zone (
   String $rname = undef,
   String $mname = $ns[0],
@@ -61,73 +83,81 @@ define dns::zone (
 
   Optional[String] $update_policy = undef,
   Boolean $dynamic = $update_policy != undef,
+
+  Optional[Variant[String, Integer]] $owner = undef,
+  Optional[Variant[String, Integer]] $group = undef,
+  Variant[Boolean, String] $backup = false,
+  Optional[Boolean] $selinux_ignore_defaults = undef,
+  Optional[String] $selrange = undef,
+  Optional[String] $selrole = undef,
+  Optional[String] $seltype = undef,
+  Optional[String] $seluser = undef,
+  Boolean $show_diff = true,
+
+  Enum['present', 'absent'] $ensure = 'present',
 ) {
   $zone_ = dns::ensure_ending_period($zone)
 
   $zone_serial = $facts.get("dns_zone_serial.'${zone_}'", 0)
 
-  concat::fragment { "Dns::Zone - ${zone_}":
-    target  => $dns::config_file,
-    content => epp("${module_name}/zoneconf.epp", {
-        zone          => $zone_,
-        type          => $type,
-        update_policy => $update_policy,
-    }),
-  }
-
-  # $ns.each |$ns| {
-  #   dns::record { "Dns::Zone - record - ${zone} NS ${ns}":
-  #     key   => '@',
-  #     type  => 'NS',
-  #     zone  => $zone_,
-  #     value => $ns,
-  #   }
-  # }
-
-  # $fixed_records = $records.map |$record| {
-  #   ["Dns::Zone - record - ${zone_} - ${record['class']} ${record['type']} ${record['key']} ${record['value']}",
-  #   $record + { key => $record['key'] } - ['key']]
-  # }.convert_to(Hash)
-
-  # create_resources(dns::record, $fixed_records, {
-  #     zone => $zone_,
-  # })
-
   $params = {
-    'rname'        => $rname,
-    'mname'        => $mname,
-    'refresh'      => $refresh,
-    'expire'       => $expire,
-    'negative_ttl' => $negative_ttl,
-    'soa_ttl'      => $soa_ttl,
-    'retry'        => $retry,
+    'rname'                   => $rname,
+    'mname'                   => $mname,
+    'refresh'                 => $refresh,
+    'expire'                  => $expire,
+    'negative_ttl'            => $negative_ttl,
+    'soa_ttl'                 => $soa_ttl,
+    'retry'                   => $retry,
+    'owner'                   => $owner,
+    'group'                   => $group,
+    'backup'                  => $backup,
+    'selinux_ignore_defaults' => $selinux_ignore_defaults,
+    'selrange'                => $selrange,
+    'selrole'                 => $selrole,
+    'seltype'                 => $seltype,
+    'seluser'                 => $seluser,
+    'show_diff'               => $show_diff,
+    'ensure'                  => $ensure,
   }
 
-  if $dynamic {
-    dns_zone2 { $zone:
-      require => Exec["Dns::zone freeze ${zone_}"],
-      *       => $params,
-    }
-
-    exec { "Dns::zone freeze ${zone_}":
-      command     => [$dns::rndc, 'freeze', $zone_],
-      refreshonly => true,
-    }
-
-    exec { "Dns::zone thaw ${zone_}":
-      command     => [$dns::rndc, 'thaw', $zone_],
-      refreshonly => true,
-      subscribe   => Dns_zone2[$zone_],
+  if $ensure == 'present' {
+    if $dynamic {
+      exec { "Dns::zone freeze ${zone_}":
+        command     => [$dns::rndc, 'freeze', $zone_],
+        refreshonly => true,
+      }
+      ~> dns_zone2 { $zone_:
+        * => $params,
+      }
+      ~> exec { "Dns::zone thaw ${zone_}":
+        command     => [$dns::rndc, 'thaw', $zone_],
+        refreshonly => true,
+        subscribe   => Dns_zone2[$zone_],
+      }
+    } else {
+      dns_zone2 { $zone_:
+        * => $params,
+      }
+      ~> exec { "Dns::zone reload ${zone_}":
+        command     => [$dns::rndc, 'reload', $zone_],
+        refreshonly => true,
+      }
     }
   } else {
-    dns_zone2 { $zone:
-      notify => Exec["Dns::zone reload ${zone_}"],
-      *      => $params,
+    dns_zone2 { $zone_:
+      ensure => 'absent',
     }
+  }
 
-    exec { "Dns::zone reload ${zone_}":
-      command     => [$dns::rndc, 'reload', $zone_],
-      refreshonly => true,
+  if $ensure == 'present' {
+    concat::fragment { "Dns::Zone - ${zone_}":
+      target  => $dns::config_file,
+      content => epp("${module_name}/zoneconf.epp", {
+          zone          => $zone_,
+          type          => $type,
+          update_policy => $update_policy,
+      }),
+      require => Dns_zone2[$zone_],
     }
   }
 }
diff --git a/templates/zoneconf.epp b/templates/zoneconf.epp
index af0d6af..2d85b5c 100644
--- a/templates/zoneconf.epp
+++ b/templates/zoneconf.epp
@@ -7,14 +7,13 @@ zone "<%= $zone %>" {
   type <%= $type %>;
   file "zones/<%= $zone %>db";
 
-  <%- if $type == 'master' or $type == 'slave' { %>
+  <%- if $type == 'master' or $type == 'slave' { -%>
   journal "journal/<%= $zone %>jnl";
-  <%- } %>
+  <%- } -%>
 
-
-  <%- if $update_policy { %>
+  <%- if $update_policy { -%>
   update-policy {
     <%= $update_policy %>;
   };
-  <%- } %>
+  <%- } -%>
 };