Merge pull request #1238 from span786/CAT-2088-match-marks-with-mask

(CAT-2088): Add ability to set match_mark|connmark as a mark/mask value

Author: span786

REFERENCE.md

@@ -364,10 +364,10 @@ Data type: `Optional[Integer]`
 
 ##### `connmark`
 
-Data type: `Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+$/]]`
+Data type: `Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+(?:\/[a-fA-F0-9x]+)?$/]]`
 _*this data type contains a regex that may not be accurately reflected in generated documentation_
 
-      Match the Netfilter mark value associated with the packet, accepts a mark.
+      Match the Netfilter mark value associated with the packet. Accepts either of mark/mask or mark.
       This value will be converted to hex if it is not already.
       This value can be negated by adding a space seperated `!` to the beginning.
 
@@ -1039,10 +1039,10 @@ _*this data type contains a regex that may not be accurately reflected in genera
 
 ##### `match_mark`
 
-Data type: `Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+$/]]`
+Data type: `Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+(?:\/[a-fA-F0-9x]+)?$/]]`
 _*this data type contains a regex that may not be accurately reflected in generated documentation_
 
-      Match the Netfilter mark value associated with the packet, accepts a mark.
+      Match the Netfilter mark value associated with the packet. Accepts either of mark/mask or mark.
       This value will be converted to hex if it is not already.
       This value can be negated by adding a space seperated `!` to the beginning.
 

lib/puppet/provider/firewall/firewall.rb

@@ -394,16 +394,11 @@ def insync?(context, _name, property_name, is_hash, should_hash)
       is = PuppetX::Firewall::Utility.log_level_name_to_number(is_hash[property_name])
       should = PuppetX::Firewall::Utility.log_level_name_to_number(should_hash[property_name])
       is == should
-    when :set_mark
+    when :set_mark, :match_mark, :connmark
       # Ensure that the values are compared to eachother in hexidecimal format
       is = PuppetX::Firewall::Utility.mark_mask_to_hex(is_hash[property_name])
       should = PuppetX::Firewall::Utility.mark_mask_to_hex(should_hash[property_name])
       is == should
-    when :match_mark, :connmark
-      # Ensure that the values are compared to eachother in hexidecimal format
-      is = PuppetX::Firewall::Utility.mark_to_hex(is_hash[property_name])
-      should = PuppetX::Firewall::Utility.mark_to_hex(should_hash[property_name])
-      is == should
     when :time_start, :time_stop
       # Ensure the values are compared in full `00:00:00` format
       is = is_hash[property_name]
@@ -893,8 +888,8 @@ def self.process_input(should)
 
     # `set_mark`, `match_mark` and `connmark` must be applied in hexidecimal format
     should[:set_mark] = PuppetX::Firewall::Utility.mark_mask_to_hex(should[:set_mark]) if should[:set_mark]
-    should[:match_mark] = PuppetX::Firewall::Utility.mark_to_hex(should[:match_mark]) if should[:match_mark]
-    should[:connmark] = PuppetX::Firewall::Utility.mark_to_hex(should[:connmark]) if should[:connmark]
+    should[:match_mark] = PuppetX::Firewall::Utility.mark_mask_to_hex(should[:match_mark]) if should[:match_mark]
+    should[:connmark] = PuppetX::Firewall::Utility.mark_mask_to_hex(should[:connmark]) if should[:connmark]
 
     # `time_start` and `time_stop` must be applied in full HH:MM:SS format
     time = [:time_start, :time_stop]

lib/puppet/type/firewall.rb

@@ -1278,9 +1278,9 @@
       DESC
     },
     match_mark: {
-      type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+$/]]',
+      type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+(?:\/[a-fA-F0-9x]+)?$/]]',
       desc: <<-DESC
-      Match the Netfilter mark value associated with the packet, accepts a mark.
+      Match the Netfilter mark value associated with the packet. Accepts either of mark/mask or mark.
       This value will be converted to hex if it is not already.
       This value can be negated by adding a space seperated `!` to the beginning.
       DESC
@@ -1313,9 +1313,9 @@
       DESC
     },
     connmark: {
-      type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+$/]]',
+      type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+(?:\/[a-fA-F0-9x]+)?$/]]',
       desc: <<-DESC
-      Match the Netfilter mark value associated with the packet, accepts a mark.
+      Match the Netfilter mark value associated with the packet. Accepts either of mark/mask or mark.
       This value will be converted to hex if it is not already.
       This value can be negated by adding a space seperated `!` to the beginning.
       DESC

lib/puppet_x/puppetlabs/firewall/utility.rb

@@ -216,25 +216,16 @@ def self.to_hex32(value)
 
     # Accepts a valid mark or mark/mask and returns them in the valid
     # hexidecimal format.
-    # USed for set_mark
+    # Used for set_mark, match_mark, connmark
     def self.mark_mask_to_hex(value)
-      match = value.to_s.match(%r{([a-fA-F0-9x]+)/?([a-fA-F0-9x]+)?})
-      mark = PuppetX::Firewall::Utility.to_hex32(match[1])
-      return "#{mark}/0xffffffff" if match[2].nil?
+      match = value.to_s.match(%r{^(!\s)?([a-fA-F0-9x]+)\/?([a-fA-F0-9x]+)?})
+      negation = '! '
+      negation = '' if match[1].nil?
+      mark = PuppetX::Firewall::Utility.to_hex32(match[2])
+      return "#{negation}#{mark}/0xffffffff" if match[3].nil?
 
-      mask = PuppetX::Firewall::Utility.to_hex32(match[2])
-      "#{mark}/#{mask}"
-    end
-
-    # Accepts a valid mark and returns them in the valid hexidecimal format.
-    # Accounts for negation.
-    # Used for match_mark / connmark
-    def self.mark_to_hex(value)
-      match = value.to_s.match(%r{^(!\s)?([a-fA-F0-9x]+)})
-      mask = PuppetX::Firewall::Utility.to_hex32(match[2])
-      return mask if match[1].nil?
-
-      "! #{mask}"
+      mask = PuppetX::Firewall::Utility.to_hex32(match[3])
+      "#{negation}#{mark}/#{mask}"
     end
 
     # Converts a given number to its protocol keyword

spec/acceptance/firewall_attributes_exceptions_spec.rb

@@ -1163,6 +1163,11 @@ class { '::firewall': }
               match_mark => '0x1',
               jump       => reject,
             }
+            firewall { '504 match_mark - test with mask':
+              proto      => 'all',
+              match_mark => '0x1/0x2000',
+              jump       => reject,
+            }
       PUPPETCODE
       it 'applies' do
         apply_manifest(pp1, catch_failures: true)
@@ -1171,6 +1176,7 @@ class { '::firewall': }
       it 'contains the rule' do
         run_shell('iptables-save') do |r|
           expect(r.stdout).to match(%r{-A INPUT -m mark --mark 0x1 -m comment --comment "503 match_mark - test" -j REJECT --reject-with icmp-port-unreachable})
+          expect(r.stdout).to match(%r{-A INPUT -m mark --mark 0x1/0x2000 -m comment --comment "504 match_mark - test with mask" -j REJECT --reject-with icmp-port-unreachable})
         end
       end
     end

spec/unit/puppet/provider/firewall/firewall_private_set_spec.rb

@@ -302,9 +302,9 @@
           result: { log_level: '1' }
         },
         {
-          process: '`set_mark`, `match_mark` and `connmark` must be put through mark_mask_to_hex/mark_to_hex',
+          process: '`set_mark`, `match_mark` and `connmark` must be put through mark_mask_to_hex',
           should: { set_mark: '42', match_mark: '42', connmark: '42' },
-          result: { set_mark: '0x2a/0xffffffff', match_mark: '0x2a', connmark: '0x2a' }
+          result: { set_mark: '0x2a/0xffffffff', match_mark: '0x2a/0xffffffff', connmark: '0x2a/0xffffffff' }
         },
         {
           process: '`time_start` and `time_stop` must be applied in full HH:MM:SS format',

spec/unit/puppet_x/puppetlabs/firewall/utility_spec.rb

@@ -192,13 +192,6 @@
     it { expect(utility.mark_mask_to_hex('4294967295/42')).to eql '0xffffffff/0x2a' }
   end
 
-  describe '#mark_to_hex' do
-    it { expect(utility.mark_to_hex('0')).to eql '0x0' }
-    it { expect(utility.mark_to_hex('! 0x32')).to eql '! 0x32' }
-    it { expect(utility.mark_to_hex('42')).to eql '0x2a' }
-    it { expect(utility.mark_to_hex('! 4294967295')).to eql '! 0xffffffff' }
-  end
-
   describe '#proto_number_to_name' do
     it { expect(utility.proto_number_to_name('1')).to eql 'icmp' }
     it { expect(utility.proto_number_to_name('2')).to eql 'igmp' }