diff --git a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb index 447fcd38..a9548cf6 100644 --- a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb +++ b/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 diff --git a/spec/acceptance/00_default_spec.rb b/spec/acceptance/00_default_spec.rb index 3c82167c..47106468 100644 --- a/spec/acceptance/00_default_spec.rb +++ b/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 { diff --git a/spec/unit/puppet/provider/firewalld_zone_spec.rb b/spec/unit/puppet/provider/firewalld_zone_spec.rb index 4bf2bac6..6fa7850f 100644 --- a/spec/unit/puppet/provider/firewalld_zone_spec.rb +++ b/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']) diff --git a/spec/unit/puppet/type/firewalld_zone_spec.rb b/spec/unit/puppet/type/firewalld_zone_spec.rb index dd6dd0b2..0d895abd 100644 --- a/spec/unit/puppet/type/firewalld_zone_spec.rb +++ b/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])