diff --git a/REFERENCE.md b/REFERENCE.md index 4232555e..6f7176d0 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -1232,6 +1232,8 @@ Valid values: `true`, `false` Can be set to true or false, specifies whether to set icmp_block_inversion from the zone +Default value: `false` + ##### `icmp_blocks` Specify the icmp-blocks for the zone. Can be a single string specifying one icmp type, diff --git a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb index a9548cf6..5a54fa05 100644 --- a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb @@ -93,9 +93,9 @@ def masquerade def masquerade=(bool) case bool when :true - execute_firewall_cmd(['--add-masquerade']) + execute_firewall_cmd(['--add-masquerade'], @resource[:name]) when :false - execute_firewall_cmd(['--remove-masquerade']) + execute_firewall_cmd(['--remove-masquerade'], @resource[:name]) end end @@ -113,7 +113,6 @@ def icmp_blocks=(new_icmp_blocks) when Array get_icmp_blocks.each do |remove_block| unless new_icmp_blocks.include?(remove_block) - debug("removing block #{remove_block} from zone #{@resource[:name]}") remove_blocks.push(remove_block) end end @@ -122,7 +121,6 @@ def icmp_blocks=(new_icmp_blocks) raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' unless block.is_a?(String) if icmp_types.include?(block) - debug("adding block #{block} to zone #{@resource[:name]}") set_blocks.push(block) else valid_types = icmp_types.join(', ') @@ -131,11 +129,9 @@ def icmp_blocks=(new_icmp_blocks) end when String get_icmp_blocks.reject { |x| x == new_icmp_blocks }.each do |remove_block| - debug("removing block #{remove_block} from zone #{@resource[:name]}") remove_blocks.push(remove_block) end if icmp_types.include?(new_icmp_blocks) - debug("adding block #{new_icmp_blocks} to zone #{@resource[:name]}") set_blocks.push(new_icmp_blocks) else valid_types = icmp_types.join(', ') @@ -144,23 +140,28 @@ 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? # should this list the zone explicitly + + # rubocop:disable Style/GuardClause + unless remove_blocks.empty? remove_blocks.each do |block| - execute_firewall_cmd(['--remove-icmp-block', block]) + debug("removing block #{remove_block} from zone #{@resource[:name]}") + execute_firewall_cmd(['--remove-icmp-block', block], @resource[:name]) end end - unless set_blocks.empty? # rubocop:disable Style/GuardClause + unless set_blocks.empty? set_blocks.each do |block| - execute_firewall_cmd(['--add-icmp-block', block]) + debug("adding block #{new_icmp_blocks} to zone #{@resource[:name]}") + execute_firewall_cmd(['--add-icmp-block', block], @resource[:name]) end end + # rubocop:enable Style/GuardClause end def icmp_block_inversion - if execute_firewall_cmd(['--query-icmp-block-inversion'], @resource[:name]).chomp == 'no' - :false - else + if execute_firewall_cmd(['--query-icmp-block-inversion'], @resource[:name], true, false).chomp == 'yes' :true + else + :false end end @@ -175,7 +176,7 @@ def icmp_block_inversion=(bool) end end - # rubocop:disable Style/AccessorMethodName + # rubocop:disable Naming/AccessorMethodName def get_rules perm = execute_firewall_cmd(['--list-rich-rules']).split(%r{\n}) curr = execute_firewall_cmd(['--list-rich-rules'], @resource[:name], false).split(%r{\n}) @@ -200,13 +201,13 @@ def get_ports end def get_icmp_blocks - execute_firewall_cmd(['--list-icmp-blocks']).split.sort + execute_firewall_cmd(['--list-icmp-blocks'], @resource[:name]).split.sort end def get_icmp_types execute_firewall_cmd(['--get-icmptypes'], nil).split end - # rubocop:enable Style/AccessorMethodName + # rubocop:enable Naming/AccessorMethodName def description execute_firewall_cmd(['--get-description'], @resource[:name], true, false) diff --git a/lib/puppet/type/firewalld_zone.rb b/lib/puppet/type/firewalld_zone.rb index a611afc6..3cda734e 100644 --- a/lib/puppet/type/firewalld_zone.rb +++ b/lib/puppet/type/firewalld_zone.rb @@ -128,6 +128,7 @@ def insync?(is) newproperty(:icmp_block_inversion) do desc 'Can be set to true or false, specifies whether to set icmp_block_inversion from the zone' + defaultto :false newvalue(:true) newvalue(:false) end diff --git a/spec/unit/puppet/type/firewalld_zone_spec.rb b/spec/unit/puppet/type/firewalld_zone_spec.rb index 0d895abd..67f44c7c 100644 --- a/spec/unit/puppet/type/firewalld_zone_spec.rb +++ b/spec/unit/puppet/type/firewalld_zone_spec.rb @@ -30,6 +30,61 @@ ## Provider tests for the firewalld_zone type # describe 'provider' do + context 'with minimal parameters' do + let(:resource) do + described_class.new( + name: 'restricted', + target: '%%REJECT%%', + interfaces: ['eth0'] + ) + end + let(:provider) do + resource.provider + end + + it 'checks if it exists' do + provider.expects(:execute_firewall_cmd).with(['--get-zones'], nil).returns('public restricted') + expect(provider).to be_exists + end + + it 'evalulates target' do + provider.expects(:execute_firewall_cmd).with(['--get-target']).returns('%%REJECT%%') + expect(provider.target).to eq('%%REJECT%%') + end + + it 'gets masquerading state as false when not set' do + provider.expects(:execute_firewall_cmd).with(['--query-masquerade'], 'restricted', true, false).returns("no\n") + expect(provider.masquerade).to eq(:false) + end + + it 'sets target' do + provider.expects(:execute_firewall_cmd).with(['--set-target', '%%REJECT%%']) + provider.target = '%%REJECT%%' + end + + it 'gets interfaces' do + provider.expects(:execute_firewall_cmd).with(['--list-interfaces']).returns('') + provider.interfaces + end + + it 'sets interfaces' do + provider.expects(:interfaces).returns(['eth1']) + provider.expects(:execute_firewall_cmd).with(['--add-interface', 'eth0']) + provider.expects(:execute_firewall_cmd).with(['--remove-interface', 'eth1']) + provider.interfaces = ['eth0'] + end + + it 'creates' do + provider.expects(:execute_firewall_cmd).with(['--new-zone', 'restricted'], nil) + provider.expects(:execute_firewall_cmd).with(['--set-target', '%%REJECT%%']) + provider.expects(:execute_firewall_cmd).with(['--remove-icmp-block-inversion'], 'restricted') + + provider.expects(:interfaces).returns([]) + provider.expects(:execute_firewall_cmd).with(['--add-interface', 'eth0']) + provider.create + end + end + context 'with standard parameters' do let(:resource) do described_class.new( @@ -65,6 +120,11 @@ expect(provider.target).to eq('%%REJECT%%') end + it 'gets masquerading state as false when not set' do + provider.expects(:execute_firewall_cmd).with(['--query-masquerade'], 'restricted', true, false).returns("no\n") + expect(provider.masquerade).to eq(:false) + end + it 'creates' do provider.expects(:execute_firewall_cmd).with(['--new-zone', 'restricted'], nil) provider.expects(:execute_firewall_cmd).with(['--set-target', '%%REJECT%%']) @@ -120,25 +180,25 @@ provider.sources = ['valy'] end - it 'sets icmp_block_inversion' do - provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'restricted').returns('no') - provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], 'restricted') + it 'gets icmp_block_inversion' do + provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'restricted', true, false).returns("no\n") + expect(provider.icmp_block_inversion).to eq(:false) + end + + it 'lists icmp types' do + provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns('echo-reply echo-request val') + expect(provider.get_icmp_types).to eq(%w[echo-reply echo-request val]) end it 'gets icmp_blocks' do - # shouldn't this list the zone? - provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks']).returns('val') + provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'restricted').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]) + provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'restricted').returns('val') + provider.expects(:execute_firewall_cmd).with(['--add-icmp-blocks', 'redirect,router-advertisment'], 'restricted') + provider.expects(:execute_firewall_cmd).with(['--remove-icmp-block', 'val'], 'public') end end @@ -155,12 +215,12 @@ end it 'sets masquerading' do - provider.expects(:execute_firewall_cmd).with(['--add-masquerade']) + provider.expects(:execute_firewall_cmd).with(['--add-masquerade'], 'public') provider.masquerade = :true end it 'disables masquerading' do - provider.expects(:execute_firewall_cmd).with(['--remove-masquerade']) + provider.expects(:execute_firewall_cmd).with(['--remove-masquerade'], 'public') provider.masquerade = :false end @@ -174,6 +234,68 @@ expect(provider.masquerade).to eq(:true) end end + + context 'when specifiying a single icmp block' do + let(:resource) do + described_class.new( + name: 'public', + ensure: :present, + sources: '192.168.2.2', + icmp_blocks: 'echo-request' + ) + end + let(:provider) do + resource.provider + end + + it 'sets icmp_block_inversion' do + provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], 'public') + provider.icmp_block_inversion = :true + end + + it 'disables icmp_block_inversion' do + provider.expects(:execute_firewall_cmd).with(['--remove-icmp-block-inversion'], 'public') + provider.icmp_block_inversion = :false + end + + it 'gets icmp_block_inversion state as false when not set' do + provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'public', true, false).returns("no\n") + expect(provider.icmp_block_inversion).to eq(:false) + end + + it 'gets icmp_block_inversion state as true when set' do + provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'public', true, false).returns("yes\n") + expect(provider.icmp_block_inversion).to eq(:true) + end + + it 'sets icmp blocks' do + provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'public').returns('val') + provider.expects(:execute_firewall_cmd).with(['--add-icmp-block', 'echo-request'], 'public') + provider.expects(:execute_firewall_cmd).with(['--remove-icmp-block', 'val'], 'public') + provider.create + end + end + + context 'when specifiying a bad icmp block' do + let(:resource) do + described_class.new( + name: 'public', + ensure: :present, + sources: '192.168.2.2', + icmp_blocks: 'invalid-request' + ) + end + let(:provider) do + resource.provider + end + + it 'errors out' do + provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns('echo-reply echo-request val') + expect(provider.get_icmp_types).to eq(%w[echo-reply echo-request val]) + expect(provider).to raise_error(Puppet::Error, %r{is not a valid icmp type}) + provider.create + end + end end context 'autorequires' do