Skip to content

Commit

Permalink
Fixup ICMP tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcpunk
jcpunk committed Sep 19, 2023
1 parent fa1f9c0 commit 47e60f9
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 27 deletions.
25 changes: 12 additions & 13 deletions lib/puppet/provider/firewalld_zone/firewall_cmd.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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(', ')
Expand All @@ -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(', ')
Expand All @@ -144,23 +140,26 @@ 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

unless remove_blocks.empty? # rubocop:disable Style/GuardClause
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
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
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

Expand Down Expand Up @@ -200,7 +199,7 @@ 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
Expand Down
1 change: 1 addition & 0 deletions lib/puppet/type/firewalld_zone.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
152 changes: 138 additions & 14 deletions spec/unit/puppet/type/firewalld_zone_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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%%'])
Expand Down Expand Up @@ -120,25 +180,26 @@
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.create
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

Expand All @@ -155,12 +216,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

Expand All @@ -174,8 +235,71 @@
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, /is not a valid icmp type/)
provider.create
end
end
end


context 'autorequires' do
# rubocop:disable RSpec/InstanceVariable
before do
Expand Down

0 comments on commit 47e60f9

Please sign in to comment.