Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to fixup failing ICMP tests #356

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Try to fixup failing ICMP tests #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 14 additions & 43 deletions lib/puppet/provider/firewalld_zone/firewall_cmd.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
80 changes: 65 additions & 15 deletions spec/unit/puppet/type/firewalld_zone_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down