work.

7 files changed, 103 insertions, 165 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index 31e8248..eb66290 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -4,7 +4,7 @@ require:
 - rubocop-rspec
 AllCops:
   DisplayCopNames: true
-  TargetRubyVersion: '2.5'
+  TargetRubyVersion: '2.7'
   Include:
   - "**/*.rb"
   Exclude:
diff --git a/lib/puppet/provider/dns_record2/named.rb b/lib/puppet/provider/dns_record2/named.rb
index 61f4a96..13fe5ee 100644
--- a/lib/puppet/provider/dns_record2/named.rb
+++ b/lib/puppet/provider/dns_record2/named.rb
@@ -1,39 +1,16 @@
-# require 'resolv'
-
 Puppet::Type.type(:dns_record2).provide(:named) do
   def self.instances
-    objects = []
-     # `named-checkconf -l`.split("\n").each do |zone|
-     #   zonename, cls, view, _ = zone.split(' ')
-     #   path = `rndc zonestatus #{zonename} #{cls} #{view} 2>/dev/null | awk -F' ' '/^files:/ { print $2 }'`.strip;
-     #   `named-checkzone -q -D #{zonename} /var/named/#{path} | sed 's/[[:space:]]\+/ /g'`.split("\n").each do |record|
-     #     key, ttl, cls, type, *value = record.split(' ')
-     #     value = value.join(' ')
-
-     #     name = "#{zonename} #{cls} #{view} #{key} #{type} #{value}"
-
-     #     objects << new(:name  => name,
-     #                    :key   => key,
-     #                    :ttl   => ttl,
-     #                    :class => cls,
-     #                    :type  => type,
-     #                    :value => value)
-     #   end
-     # end
-    objects
+    []
   end
 
-  def create
-    # print("Create\n")
-  end
+  # Create and destroy don't do anything on their own,
+  # See comment in type
+  def create; end
 
-  def destroy
-    # print("Destroy\n")
-  end
+  def destroy; end
 
   def exists?
-    # print("record = [#{record().inspect}]\n")
-    record() != nil
+    record != nil
   end
 
   def type
@@ -41,8 +18,7 @@ Puppet::Type.type(:dns_record2).provide(:named) do
   end
 
   def value
-    # print("Get old value (#{resource[:value]}, #{get(:value)})\n")
-    t = record()
+    t = record
     case t
     when NilClass
       :absent
@@ -53,10 +29,8 @@ Puppet::Type.type(:dns_record2).provide(:named) do
     end
   end
 
-  def value=(v)
-    # print("Set new value (#{v})\n")
-    # resource[:value] = v
-  end
+  # TODO: should we have this?
+  def value=(v); end
 
   def key
     resource[:key]
@@ -80,49 +54,17 @@ Puppet::Type.type(:dns_record2).provide(:named) do
     end
   end
 
-  # def zone=(v)
-  #   # set(:zone, v)
-  #   resource[:zone] = v
-  # end
-
   def record
-    # dns = Resolv::DNS.new(nameserver: ['localhost'])
-    # type = {
-    #   A:       Resolv::DNS::Resource::IN::A,
-    #   AAAA:    Resolv::DNS::Resource::IN::AAAA,
-    #   CNAME:   Resolv::DNS::Resource::IN::CNAME,
-    #   HINFO:   Resolv::DNS::Resource::IN::HINFO,
-    #   MINFO:   Resolv::DNS::Resource::IN::MINFO,
-    #   MX:      Resolv::DNS::Resource::IN::MX,
-    #   NS:      Resolv::DNS::Resource::IN::NS,
-    #   PTR:     Resolv::DNS::Resource::IN::PTR,
-    #   SOA:     Resolv::DNS::Resource::IN::SOA,
-    #   TXT:     Resolv::DNS::Resource::IN::TXT,
-    #   WKS:     Resolv::DNS::Resource::IN::WKS,
-    # }[resource[:type]]
-
-    # if type == nil
-    #   nil
-    # else
-    #   rs = dns.getresources(full_key, type)
-    #   if rs.length == 0
-    #     nil
-    #   else
-    #     rs[0]
-    #   end
-    # end
     lines = `named-checkzone -j -q -D #{zone} /var/named/zones/#{zone}db`
-      .split("\n")
-      .map {|line| line.gsub(/[[:space:]]+/, ' ').split(' ') }
-
-    matches = lines
-      .filter {|line|
-        line[0] == full_key and line[3].to_sym == resource[:type]
-      }
-        .map {|line| line[(4..)].join(' ') }
+            .split("\n")
+            .map { |line| line.gsub(%r{[[:space:]]+}, ' ').split(' ') }
 
+    matches =
+      lines
+      .filter { |line| line[0] == full_key && line[3].to_sym == resource[:type] }
+      .map { |line| line[(4..)].join(' ') }
 
-    if matches.length == 0
+    if matches.empty?
       nil
     elsif matches.length == 1
       matches[0]
diff --git a/lib/puppet/provider/dns_zone2/named.rb b/lib/puppet/provider/dns_zone2/named.rb
index e249be6..d929322 100644
--- a/lib/puppet/provider/dns_zone2/named.rb
+++ b/lib/puppet/provider/dns_zone2/named.rb
@@ -1,8 +1,8 @@
 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(' ');
-      new(:name => name, :cls => cls, :view => view, :type => type)
+      name, cls, view, type = record.split(' ')
+      new(name: name, cls: cls, view: view, type: type)
     end
   end
 
@@ -12,7 +12,7 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
   end
 
   def destroy
-      print("Remove #{resource[:name]}\n")
+    print("Remove #{resource[:name]}\n")
   end
 
   def refresh(records)
@@ -25,8 +25,9 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
     # resource[:ensure] == :present
     `named-checkconf -l`
       .split("\n")
-      .grep(/#{resource[:origin]} /)
-      .length > 0
+      .grep(%r{#{resource[:origin]} })
+      .empty?
+      .!
   end
 
   def filename
@@ -34,53 +35,38 @@ Puppet::Type.type(:dns_zone2).provide(:named) do
   end
 
   def zone_content(records)
-      content = <<~EOF
-      ; File managed by Puppet.
-      ; Local changes WILL be overwritten
-      ; File last generated #{Time.now}
+    content = <<~EOF
+    ; File managed by Puppet.
+    ; Local changes WILL be overwritten
+    ; File last generated #{Time.now}
 
-      $ORIGIN #{resource[:origin]}
-      $TTL #{resource[:default_ttl]}
+    $ORIGIN #{resource[:origin]}
+    $TTL #{resource[:default_ttl]}
 
-      @	#{resource[:soa_ttl]}	IN	SOA	#{resource[:mname]} #{resource[:rname]} (
-      				#{serial+1}	; serial
-      				#{resource[:refresh]}	; refresh
-      				#{resource[:retry]}	; retry
-      				#{resource[:expire]}	; expire
-      				#{resource[:negative_ttl]}	; Negative TTL
-                    )
-      EOF
+    @	#{resource[:soa_ttl]}	IN	SOA	#{resource[:mname]} #{resource[:rname]} (
+                  #{serial + 1}	; serial
+                  #{resource[:refresh]}	; refresh
+                  #{resource[:retry]}	; retry
+                  #{resource[:expire]}	; expire
+                  #{resource[:negative_ttl]}	; Negative TTL
+                  )
+    EOF
 
-      records
-        .filter {|r| r[:zone] == resource[:name] }
-        .group_by {|r| r[:type]}
-        .sort_by {|(type, _)|
-          # Bit of a hack, but ensures that SOA is always first,
-          # NS is after
-          # And the rest have stable order
-          {
-            SOA:     0,
-            NS:      1,
-            A:       2,
-            AAAA:    3,
-            CNAME:   4,
-            HINFO:   5,
-            MINFO:   6,
-            MX:      7,
-            PTR:     8,
-            TXT:     9,
-            WKS:    10,
-          }[type]
-        }.each{|(type, values)|
+    records
+      .filter { |r| r[:zone] == resource[:name] }
+      .group_by { |r| r[:type] }
+      .sort # Sort to ensure stable order in output, actual order doesn't matter.
+      .each do |(type, values)|
+        content += <<~EOF
+        ; #{type} Records
+        EOF
+        values.each do |val|
           content += <<~EOF
-          ; #{type} Records
+          #{val[:key]} #{val[:ttl]}  IN  #{val[:type]} #{val[:value]}
           EOF
-          values.each {|val|
-            content += <<~EOF
-            #{val[:key]} #{val[:ttl]}  IN  #{val[:type]} #{val[:value]}
-            EOF
-          }
-        }
+        end
+      end
+
     content
   end
 
diff --git a/lib/puppet/type/dns_record2.rb b/lib/puppet/type/dns_record2.rb
index 2074782..98eaadb 100644
--- a/lib/puppet/type/dns_record2.rb
+++ b/lib/puppet/type/dns_record2.rb
@@ -1,8 +1,11 @@
 Puppet::Type.newtype(:dns_record2) do
-  @doc = %q{
-  }
+  @doc = <<~EOF
+  A single DNS record.
 
-  #"named-checkconf -j -D -F raw $zonename $zonefile"
+  Doesn't have any effect on the system by itself, but instead
+  registers data to the current catalog, which is later gathered by a
+  coresponding dns_zone2 resource.
+  EOF
 
   newproperty(:ensure) do
     newvalue(:present) do
@@ -29,45 +32,41 @@ Puppet::Type.newtype(:dns_record2) do
   end
 
   newparam(:name) do
+    desc <<~EOF
+    Explicitly not used as the key, since each key can contain many
+    EOF
   end
 
   newproperty(:type) do
-    desc %q{
+    desc <<~EOF
     Resource Record type, such as A, AAAA, ...
-
-    Only a few types are supported, due to the DNS library only supporting these.
-    }
-    newvalues(:A,
-              :AAAA,
-              :CNAME,
-              :HINFO,
-              :MINFO,
-              :MX,
-              :NS,
-              :PTR,
-              :SOA,
-              :TXT,
-              :WKS,
-             )
+    EOF
+    munge { |v| v.to_sym }
   end
 
   newproperty(:value) do
-    desc %q{
+    desc <<~EOF
     DNS payload. For example an IP address.
-    }
+      EOF
   end
 
   newproperty(:key) do
-    desc %q{
+    desc <<~EOF
     DNS key.
     Such as 'www'
-    }
+
+    Will be treated as an absolute key if ending in a period, and
+    relative to the zone otherwise.
+    (NOTE this is handled by the provider)
+
+    '@' is the "empty" key.
+    EOF
   end
 
   newproperty(:zone) do
-    desc %q{
-    Zone this record belongs to
-    }
+    desc <<~EOF
+    Zone this record belongs to.
+    EOF
 
     munge do |value|
       if value[-1] == '.'
@@ -79,15 +78,20 @@ Puppet::Type.newtype(:dns_record2) do
   end
 
   newproperty(:cls) do
-    desc %q{
-    }
+    desc <<~EOF
+    DNS class of this record.
+    Currently only IN works.
+    EOF
   end
 
   newproperty(:ttl) do
+    desc <<~EOF
+    TTL of this record.
+    EOF
   end
 
   # autobefore('dns::zone') { value(:zone) }
   autonotify('dns_zone2') { [value(:zone)] }
   # autobefore('dns_zone2') { [value(:zone)] }
-  # TODO view
+  # TODO views
 end
diff --git a/lib/puppet/type/dns_zone2.rb b/lib/puppet/type/dns_zone2.rb
index 6cee6f2..61fe8ce 100644
--- a/lib/puppet/type/dns_zone2.rb
+++ b/lib/puppet/type/dns_zone2.rb
@@ -1,10 +1,17 @@
-Puppet::Type.newtype(:dns_zone2, :self_refresh => true) do
-  @doc = %q{
-  }
+Puppet::Type.newtype(:dns_zone2, self_refresh: true) do
+  @doc = <<~EOF
+  A complete DNS zonefile.
+
+  The SOA record comes backed in, this for two reasons:
+  1. A SOA record is REQUIRED in all zonefiles, so it doesn't make
+     sense to allow it to be emitted.
+  2. This is the only way to ensure that our serial is only updated
+     when something else has actually changed in the zone.
+  EOF
 
   newproperty(:ensure) do
     newvalue(:present) do
-      provider.write_zone (resource.should_content)
+      provider.write_zone(resource.should_content)
     end
 
     # This should ideally remove the zone. This is however managed
@@ -32,7 +39,7 @@ Puppet::Type.newtype(:dns_zone2, :self_refresh => true) do
     provider.write_zone(should_content)
   end
 
-  newparam(:origin, :namevar => true) do
+  newparam(:origin, namevar: true) do
     munge do |value|
       if value[-1] == '.'
         value
@@ -58,7 +65,6 @@ Puppet::Type.newtype(:dns_zone2, :self_refresh => true) do
     defaultto :master
   end
 
-
   newparam(:default_ttl) do
     defaultto '300'
   end
@@ -112,10 +118,11 @@ Puppet::Type.newtype(:dns_zone2, :self_refresh => true) do
 
   # This prevents the directory purge on /var/named/zones from deleting us.
   def generate
-    [Puppet::Type.type(:file).new({
+    params = {
       ensure: :present,
       path: provider.filename,
-    })]
+    }
+    [Puppet::Type.type(:file).new(params)]
   end
 
   def should_content
diff --git a/manifests/record.pp b/manifests/record.pp
index 2ecb52f..1b36a94 100644
--- a/manifests/record.pp
+++ b/manifests/record.pp
@@ -1,10 +1,7 @@
 # @param type
 #   Record type (A, AAAA, ...)
-# @param class
+# @param cls
 #   DNS class type (IN, HS, CH, HS)
-# @param dns_name
-#   Name of record (example.com.)
-#   Note that the trailing period **IS** significant
 # @param ttl
 #   TTL for record.
 # @param duplicate
@@ -20,6 +17,10 @@
 #   Some record types have extra processing.
 #   TXT splits data into chunks of 255 characters (TODO shouldn't
 #   this be bytes) and the surrounds each chunk with quotation marks.
+# @param key
+#   DNS key. Will be treated as absolute if ending with a period, or
+#   relative to the zone if not. '@' for the "empty" key.
+#   TODO tests for above?
 define dns::record (
   String $zone,
   Dns::Rr $type,
diff --git a/manifests/zone.pp b/manifests/zone.pp
index 33c2b0d..1fee3d5 100644
--- a/manifests/zone.pp
+++ b/manifests/zone.pp
@@ -33,8 +33,6 @@
 # @param soa_ttl
 #   TTL of SOA record.
 define dns::zone (
-  Boolean $manage_soa = true,
-
   String $rname = undef,
   String $mname = $ns[0],
   Dns::Ttl $refresh = '24h',