(CAT-1224) firewall type/provider code updates

- Update boolean attributes to always return a value.
- Update `dst_type` and `src_type` to accept unique values.
- Update `nflog_prefix` to correctly quote passed in variables and expect quotes when retrieving them.
- Update `source` and `destination` to add a mask when needed.
- Update `sport` and `dport` to allow a `-` when submitting a range.
- Purge tests disabled pending merge and release of resource_api change: puppetlabs/puppet-resource_api#316
- Set type of `nflog_prefix` to String as refining it to String[1] or String[1, 64] causes errors on Puppet 8.

lib/puppet/provider/firewall/firewall.rb

@@ -385,12 +385,16 @@ def self.rule_to_hash(_context, rule, table_name, protocol)
     $resource_map.each do |key, value|
       if $known_booleans.include?(key)
         # check for flag with regex, add a space/line end to ensure accuracy with the more simplistic flags; i.e. `-f`, `--random`
-        rule_hash[key] = true if rule[0].match(Regexp.new("#{value}(\\s|$)"))
+        rule_hash[key] = if rule[0].match(Regexp.new("#{value}(\\s|$)"))
+                           true
+                         else
+                           false
+                         end
         next
       end
 
       case key
-      when :name, :string, :string_hex, :bytecode, :u32, :log_prefix
+      when :name, :string, :string_hex, :bytecode, :u32, :nflog_prefix, :log_prefix
         # When :name/:string/:string_hex/:bytecode, return everything inside the double quote pair following the key value
         # When only a single word comment is returned no quotes are given, so we must check for this as well
         # First find if flag is present, add a space to ensure accuracy with the more simplistic flags; i.e. `-i`
@@ -399,7 +403,7 @@ def self.rule_to_hash(_context, rule, table_name, protocol)
           key_value = rule[0].scan(value_regex)[0]
           # Combine the returned values and remove and trailing or leading whitespace
           key_value[1] = [key_value[0], key_value[1], key_value[2]].join('')
-          rule_hash[key] = key_value[1] if key_value
+          rule_hash[key] = key_value[1] if key_value[1]
         end
       when :sport, :dport
         split_value_regex = value[0].split(%r{ })
@@ -514,7 +518,7 @@ def self.rule_to_hash(_context, rule, table_name, protocol)
       else # :chain, stat_mode, stat_every, stat_packet, stat_probability, socket, ipsec_dir, ipsec_policy, :ctdir,
         # :limit, :burst, :length, :rseconds, :rhitcount, :rname, :mask, :string_algo, :string_from, :string_to,
         # :jump, :goto, :clusterip_hashmode, :clusterip_clustermac, :clusterip_total_nodes, :clusterip_local_node,
-        # :clusterip_hash_init, :queue_num, :nflog_group, :nflog_prefix, :nflog_range, :nflog_size, :nflog_threshold,
+        # :clusterip_hash_init, :queue_num, :nflog_group, :nflog_range, :nflog_size, :nflog_threshold,
         # :gateway, :set_mss, :set_dscp, :set_dscp_class, :todest, :tosource, :toports, :to, :log_level,
         # :reject, :set_mark, :connlimit_upto, :connlimit_above, :connlimit_mask, :time_start, :time_stop, :date_start,
         # :date_stop, :src_cc, :dst_cc, :hashlimit_upto, :hashlimit_above, :hashlimit_name, :hashlimit_burst, :hashlimit_mode,
@@ -676,6 +680,13 @@ def self.validate_input(is, should)
     ipv6_only.each do |ipv6|
       raise "Parameter `#{ipv6}` is specific to the `IPv6` protocol" if should[ipv6] && !(should[:protocol] == 'IPv6' || should[:protocol] == 'ip6tables')
     end
+    ## Array elements must be unique
+    [:dst_type, :src_type].each do |key|
+      next unless should[key] && should[key].is_a?(Array)
+      raise "#{key} elements must be unique" if should[key].map { |type| type.to_s.gsub(%r{--limit-iface-(in|out)}, '') }.uniq.length != should[key].length
+    end
+    # Log prefix size is limited
+    raise 'Parameter `nflog_prefix`` must be less than 64 characters' if should[:nflog_prefix] && should[:nflog_prefix].length > 64
   end
 
   # Certain attributes need processed in ways that can vary between IPv4 and IPv6
@@ -684,7 +695,11 @@ def self.process_input(should)
     # Jump values should always be uppercase
     should[:jump] = should[:jump].upcase if should[:jump]
 
-    # ct attributes must be put through host_to_mask
+    # :source and :destination must be put through host_to_mask
+    should[:source] = PuppetX::Firewall::Utility.host_to_mask(should[:source], should[:protocol]) if should[:source]
+    should[:destination] = PuppetX::Firewall::Utility.host_to_mask(should[:destination], should[:protocol]) if should[:destination]
+
+    # ct attributes must be put through host_to_mask with certain masks then being removed
     ct = [:ctorigsrc, :ctorigdst, :ctreplsrc, :ctrepldst]
     ct.each do |c|
       break unless should[c]
@@ -715,6 +730,12 @@ def self.process_input(should)
       should[t] = "#{should[t]}:00" if %r{^([0-9]|0[0-9]|1[0-9]|2[0-3]):[0-5][0-9]$}.match?(should[t])
     end
 
+    # If range has been passed with `-`, replace with `:`
+    should[:sport] = should[:sport].gsub(%r{-}, ':') if should[:sport] && should[:sport].is_a?(String)
+    should[:dport] = should[:dport].gsub(%r{-}, ':') if should[:dport] && should[:dport].is_a?(String)
+    should[:sport] = should[:sport].map { |port| port.to_s.gsub(%r{-}, ':') } if should[:sport] && should[:sport].is_a?(Array)
+    should[:dport] = should[:dport].map { |port| port.to_s.gsub(%r{-}, ':') } if should[:dport] && should[:dport].is_a?(Array)
+
     should
   end
 
@@ -751,7 +772,7 @@ def self.hash_to_rule(_context, _name, rule)
       # check for existence, retrieve string that follows if it is
       # certain resources may need special rules
       case key
-      when :name, :string, :string_hex, :bytecode, :u32, :log_prefix
+      when :name, :string, :string_hex, :bytecode, :u32, :nflog_prefix, :log_prefix
         arguments += " #{[$resource_map[key], "'#{rule[key]}'"].join(' ')}" if rule[key].match?(%r{^[^!]}) # if standard
         arguments += " #{['!', $resource_map[key], "'#{rule[key].gsub(%r{^!\s?}, '')}'"].join(' ')}" if rule[key].match?(%r{^!}) # if negated
       when :sport, :dport
@@ -823,7 +844,7 @@ def self.hash_to_rule(_context, _name, rule)
       else # :chain, stat_mode, stat_every, stat_packet, stat_probability, socket, ipsec_dir, ipsec_policy, :ctdir,
         # :limit, :burst, :length, :rseconds, :rhitcount, :rname, :mask, :string_algo, :string_from, :string_to,
         # :jump, :goto, :clusterip_hashmode, :clusterip_clustermac, :clusterip_total_nodes, :clusterip_local_node,
-        # :clusterip_hash_init, :queue_num, :nflog_group, :nflog_prefix, :nflog_range, :nflog_size, :nflog_threshold,
+        # :clusterip_hash_init, :queue_num, :nflog_group, :nflog_range, :nflog_size, :nflog_threshold,
         # :gateway, :set_mss, :set_dscp, :set_dscp_class, :todest, :tosource, :toports, :to, :log_level,
         # :reject, :set_mark, :connlimit_upto, :connlimit_above, :connlimit_mask, :time_start, :time_stop, :date_start,
         # :date_stop, :src_cc, :dst_cc, :hashlimit_upto, :hashlimit_above, :hashlimit_name, :hashlimit_burst, :hashlimit_mode,
@@ -900,6 +921,9 @@ def insync?(context, _name, property_name, is_hash, should_hash)
       should = 'IPv6' if should == 'ip6tables'
 
       is == should
+    when :source, :destination
+      # Ensure source/destination has it's valid mask before you compare it
+      is_hash[property_name] == PuppetX::Firewall::Utility.host_to_mask(should_hash[property_name], should_hash[:protocol])
     when :tcp_flags
       # Custom logic to account for `ALL` being returned as `FIN,SYN,RST,PSH,ACK,URG`
       is = is_hash[property_name].split
@@ -987,6 +1011,10 @@ def insync?(context, _name, property_name, is_hash, should_hash)
     when :jump
       # Compare values as uppercase
       is_hash[property_name].upcase == should_hash[property_name].upcase
+    when :dport, :sport
+      # Range can be passed as `-` but will always be set/returned as `:`
+      is_hash[property_name] == should_hash[property_name].gsub(%r{-}, ':') if should_hash[property_name].is_a?(String)
+      is_hash[property_name] == should_hash[property_name].map { |port| port.to_s.gsub(%r{-}, ':') } if should_hash[property_name].is_a?(Array)
     else
       # Ensure that if both values are arrays, that they are sorted prior to comparison
       return nil unless is_hash[property_name].is_a?(Array) && should_hash[property_name].is_a?(Array)

lib/puppet/type/firewall.rb

@@ -420,7 +420,7 @@
       DESC
     },
     sport: {
-      type: 'Optional[Variant[Array[Pattern[/^(?:!\s)?\d+(?:\:\d+)?$/]],Pattern[/^(?:!\s)?\d+(?:\:\d+)?$/]]]',
+      type: 'Optional[Variant[Array[Pattern[/^(?:!\s)?\d+(?:(?:\:|-)\d+)?$/]],Pattern[/^(?:!\s)?\d+(?:(?:\:|-)\d+)?$/]]]',
       desc: <<-DESC
       The source port to match for this filter (if the protocol supports
       ports). Will accept a single element or an array.
@@ -444,7 +444,7 @@
       DESC
     },
     dport: {
-      type: 'Optional[Variant[Array[Pattern[/^(?:!\s)?\d+(?:\:\d+)?$/]],Pattern[/^(?:!\s)?\d+(?:\:\d+)?$/]]]',
+      type: 'Optional[Variant[Array[Pattern[/^(?:!\s)?\d+(?:(?:\:|-)\d+)?$/]],Pattern[/^(?:!\s)?\d+(?:(?:\:|-)\d+)?$/]]]',
       desc: <<-DESC
       The source port to match for this filter (if the protocol supports
       ports). Will accept a single element or an array.
@@ -1101,7 +1101,7 @@
       DESC
     },
     nflog_prefix: {
-      type: 'Optional[String[1, 64]]',
+      type: 'Optional[String]',
       desc: <<-DESC
       Used with the jump target NFLOG.
       A prefix string to include in the log message, up to 64 characters long,