Acceptance test of icmp_block_inversion

lib/puppet/provider/firewalld_zone/firewall_cmd.rb

@@ -23,6 +23,7 @@ def create
     self.sources = (@resource[:sources]) if @resource[:sources]
     self.interfaces = @resource[:interfaces]
     self.icmp_blocks = (@resource[:icmp_blocks]) if @resource[:icmp_blocks]
+    self.icmp_block_inversion = (@resource[:icmp_block_inversion]) if @resource[:icmp_block_inversion]
     self.description = (@resource[:description]) if @resource[:description]
     self.short = (@resource[:short]) if @resource[:short]
   end
@@ -143,7 +144,7 @@ def icmp_blocks=(new_icmp_blocks)
     else
       raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!'
     end
-    unless remove_blocks.empty?
+    unless remove_blocks.empty? # should this list the zone explicitly
       remove_blocks.each do |block|
         execute_firewall_cmd(['--remove-icmp-block', block])
       end
@@ -156,7 +157,7 @@ def icmp_blocks=(new_icmp_blocks)
   end
 
   def icmp_block_inversion
-    if execute_firewall_cmd(['--query-icmp-block-inversion'], @resource[:name], true, false).chomp == 'no'
+    if execute_firewall_cmd(['--query-icmp-block-inversion'], @resource[:name]).chomp == 'no'
       :false
     else
       :true
@@ -166,9 +167,11 @@ def icmp_block_inversion
   def icmp_block_inversion=(bool)
     case bool
     when :true
-      execute_firewall_cmd(['--add-icmp-block-inversion'], @resource[:name], true, false)
+      debug("adding icmp block inversion for zone #{@resource[:name]}")
+      execute_firewall_cmd(['--add-icmp-block-inversion'], @resource[:name])
     when :false
-      execute_firewall_cmd(['--remove-icmp-block-inversion'], @resource[:name], true, false)
+      debug("removing icmp block inversion for zone #{@resource[:name]}")
+      execute_firewall_cmd(['--remove-icmp-block-inversion'], @resource[:name])
     end
   end
 

spec/acceptance/00_default_spec.rb

@@ -29,11 +29,12 @@ class ssh_test {
             }
 
             firewalld_zone{ 'test':
-              ensure           => 'present',
-              purge_rich_rules => true,
-              purge_services   => true,
-              purge_ports      => true,
-              target           => 'DROP'
+              ensure               => 'present',
+              purge_rich_rules     => true,
+              purge_services       => true,
+              purge_ports          => true,
+              target               => 'DROP',
+              icmp_block_inversion => true,
             }
 
             class other_service {

spec/unit/puppet/provider/firewalld_zone_spec.rb

@@ -31,6 +31,7 @@
         resource.expects(:[]).with(:sources).returns(nil).at_least_once
         resource.expects(:[]).with(:interfaces).returns(['eth0']).at_least_once
         resource.expects(:[]).with(:icmp_blocks).returns(nil).at_least_once
+        resource.expects(:[]).with(:icmp_block_inversion).returns(false).at_least_once
         resource.expects(:[]).with(:description).returns(nil).at_least_once
         resource.expects(:[]).with(:short).returns('little description').at_least_once
         provider.expects(:execute_firewall_cmd).with(['--list-interfaces'])
@@ -50,6 +51,7 @@
         resource.expects(:[]).with(:sources).returns(nil).at_least_once
         resource.expects(:[]).with(:interfaces).returns(['eth0']).at_least_once
         resource.expects(:[]).with(:icmp_blocks).returns(nil).at_least_once
+        resource.expects(:[]).with(:icmp_block_inversion).returns(false).at_least_once
         resource.expects(:[]).with(:description).returns(nil).at_least_once
         resource.expects(:[]).with(:short).returns('little description').at_least_once
         provider.expects(:execute_firewall_cmd).with(['--list-interfaces'])

spec/unit/puppet/type/firewalld_zone_spec.rb

@@ -70,6 +70,7 @@
         provider.expects(:execute_firewall_cmd).with(['--set-target', '%%REJECT%%'])
 
         provider.expects(:icmp_blocks=).with(%w[redirect router-advertisment])
+        provider.expects(:icmp_block_inversion=).with(:true)
 
         provider.expects(:sources).returns([])
         provider.expects(:execute_firewall_cmd).with(['--add-source', '192.168.2.2'])
@@ -120,16 +121,21 @@
       end
 
       it 'sets icmp_block_inversion' do
-        provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'restricted', true, false).returns('no')
-        provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], 'restricted', true, false)
-        expect(provider.icmp_block_inversion).to eq(:false)
+        provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'restricted').returns('no')
+        provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], 'restricted')
       end
 
       it 'gets icmp_blocks' do
+        # shouldn't this list the zone?
         provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks']).returns('val')
         expect(provider.icmp_blocks).to eq(['val'])
       end
 
+      it 'sets icmp_blocks' do
+        # shouldn't this list the zone?
+        provider.expects(:execute_firewall_cmd).with(['--add-icmp-blocks', 'redirect,router-advertisment'])
+      end
+
       it 'lists icmp types' do
         provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns('echo-reply echo-request')
         expect(provider.get_icmp_types).to eq(%w[echo-reply echo-request])