diff --git a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb index 81eff8fe..f8973210 100644 --- a/lib/puppet/provider/firewalld_zone/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_zone/firewall_cmd.rb @@ -104,55 +104,26 @@ def icmp_blocks end def icmp_blocks=(new_icmp_blocks) - set_blocks = [] - remove_blocks = [] + new_icmp_blocks = new_icmp_blocks.split(%r{\s+}) if new_icmp_blocks.is_a?(String) + raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' unless new_icmp_blocks.is_a?(Array) icmp_types = get_icmp_types + invalid_blocks = new_icmp_blocks - icmp_types + raise Puppet::Error, "Invalid ICMP types: '#{invalid_blocks.join(', ')}'! Valid types are: '#{icmp_types.join(', ')}'" unless invalid_blocks.empty? - case new_icmp_blocks - when Array - get_icmp_blocks.each do |remove_block| - remove_blocks.push(remove_block) unless new_icmp_blocks.include?(remove_block) - end - - new_icmp_blocks.each do |block| - raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' unless block.is_a?(String) - - if icmp_types.include?(block) - set_blocks.push(block) - else - valid_types = icmp_types.join(', ') - raise Puppet::Error, "#{block} is not a valid icmp type on this system! Valid types are: #{valid_types}" - end - end - when String - get_icmp_blocks.reject { |x| x == new_icmp_blocks }.each do |remove_block| - remove_blocks.push(remove_block) - end - if icmp_types.include?(new_icmp_blocks) - set_blocks.push(new_icmp_blocks) - else - valid_types = icmp_types.join(', ') - raise Puppet::Error, "#{new_icmp_blocks} is not a valid icmp type on this system! Valid types are: #{valid_types}" - end - else - raise Puppet::Error, 'parameter icmp_blocks must be a string or array of strings!' - end + icmp_blocks = get_icmp_blocks + + set_blocks = new_icmp_blocks - icmp_blocks + remove_blocks = icmp_blocks - new_icmp_blocks - # rubocop:disable Style/GuardClause - unless remove_blocks.empty? - remove_blocks.each do |block| - debug("removing block #{remove_block} from zone #{@resource[:name]}") - execute_firewall_cmd(['--remove-icmp-block', block], @resource[:name]) - end + Array(remove_blocks).each do |block| + debug("removing block #{block} from zone #{@resource[:name]}") + execute_firewall_cmd(['--remove-icmp-block', block], @resource[:name]) end - unless set_blocks.empty? - set_blocks.each do |block| - debug("adding block #{new_icmp_blocks} to zone #{@resource[:name]}") - execute_firewall_cmd(['--add-icmp-block', block], @resource[:name]) - end + Array(set_blocks).each do |block| + debug("adding block #{new_icmp_blocks} to zone #{@resource[:name]}") + execute_firewall_cmd(['--add-icmp-block', block], @resource[:name]) end - # rubocop:enable Style/GuardClause end def icmp_block_inversion diff --git a/spec/unit/puppet/type/firewalld_zone_spec.rb b/spec/unit/puppet/type/firewalld_zone_spec.rb index 67f44c7c..12aaf078 100644 --- a/spec/unit/puppet/type/firewalld_zone_spec.rb +++ b/spec/unit/puppet/type/firewalld_zone_spec.rb @@ -7,6 +7,56 @@ Puppet::Provider::Firewalld.any_instance.stubs(:state).returns(:true) # rubocop:disable RSpec/AnyInstance end + let(:icmptypes) do + %w[ + address-unreachable + bad-header + beyond-scope + communication-prohibited + destination-unreachable + echo-reply + echo-request + failed-policy + fragmentation-needed + host-precedence-violation + host-prohibited + host-redirect + host-unknown + host-unreachable + ip-header-bad + neighbour-advertisement + neighbour-solicitation + network-prohibited + network-redirect + network-unknown + network-unreachable + no-route + packet-too-big + parameter-problem + port-unreachable + precedence-cutoff + protocol-unreachable + redirect + reject-route + required-option-missing + router-advertisement + router-solicitation + source-quench + source-route-failed + time-exceeded + timestamp-reply + timestamp-request + tos-host-redirect + tos-host-unreachable + tos-network-redirect + tos-network-unreachable + ttl-zero-during-reassembly + ttl-zero-during-transit + unknown-header-type + unknown-option + ] + end + describe 'type' do context 'with no params' do describe 'when validating attributes' do @@ -186,19 +236,21 @@ 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]) + 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]) end it 'gets icmp_blocks' do - provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'restricted').returns('val') - expect(provider.icmp_blocks).to eq(['val']) + provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'restricted').returns('redirect router-advertisement') + expect(provider.icmp_blocks).to eq(%w[redirect router-advertisement]) end it 'sets icmp_blocks' do - 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') + provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'restricted').returns('redirect router-advertisement') + provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns(icmptypes.join(' ')) + provider.expects(:execute_firewall_cmd).with(['--add-icmp-block', 'bad-header'], 'restricted') + provider.expects(:execute_firewall_cmd).with(['--remove-icmp-block', 'router-advertisement'], 'restricted') + provider.icmp_blocks = %w[redirect bad-header] end end @@ -268,11 +320,11 @@ 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') + it 'sets icmp_blocks' do + provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks'], 'public').returns('') + provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns(icmptypes.join(' ')) 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 + provider.icmp_blocks = %w[echo-request] end end @@ -290,10 +342,8 @@ 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 + provider.expects(:execute_firewall_cmd).with(['--get-icmptypes'], nil).returns(icmptypes.join(' ')) + expect { provider.icmp_blocks = 'banana' }.to raise_error(Puppet::Error, %r{Invalid ICMP types}) end end end