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.

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

Fix rich rule with typed action #329

Merged
merged 2 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def eval_action
return [] unless (action = @resource[:action])
args = []
if action.is_a?(Hash)
args << action[:action]
args << quote_keyval('type', action[:type])
args << action['action']
args << quote_keyval('type', action['type'])
else
args << action
end
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/type/firewalld_rich_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ def _validate_action(value)
end
validate do |value|
if value.is_a?(Hash)
if value.keys.sort != [:action, :type]
if value.keys.sort != ['action', 'type']
raise Puppet::Error, "Rule action hash should contain `action` and `type` keys. Use a string if you only want to declare the action to be `accept` or `reject`. Got #{value}"
end
_validate_action(value[:action])
_validate_action(value['action'])
elsif value.is_a?(String)
_validate_action(value)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/puppet/provider/firewalld_rich_rule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
resource.expects(:[]).with(:log).returns(nil)
resource.expects(:[]).with(:audit).returns(nil)
resource.expects(:[]).with(:raw_rule).returns(nil)
resource.expects(:[]).with(:action).returns(action: 'reject', type: 'icmp-admin-prohibited')
resource.expects(:[]).with(:action).returns('action' => 'reject', 'type' => 'icmp-admin-prohibited')
expect(provider.build_rich_rule).to eq('rule family="ipv4" destination address="192.168.0.1/32" service name="ssh" reject type="icmp-admin-prohibited"')
end
end
Expand Down
33 changes: 27 additions & 6 deletions spec/unit/puppet/type/firewalld_rich_rule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@
expect do
described_class.new(
title: 'SSH from barny',
action: { type: 'accepted', foo: 'bar' }
action: { 'type' => 'accepted', 'foo' => 'bar' }
)
end.to raise_error(%r{Rule action hash should contain `action` and `type` keys. Use a string if you only want to declare the action to be `accept` or `reject`})
end
it 'raises an error if wrong action hash values' do
expect do
described_class.new(
title: 'SSH from barny',
action: { type: 'icmp-admin-prohibited', action: 'accepted' }
action: { 'type' => 'icmp-admin-prohibited', 'action' => 'accepted' }
)
end.to raise_error(%r{Authorized action values are `accept`, `reject`, `drop` or `mark`})
end
Expand Down Expand Up @@ -184,7 +184,31 @@
icmp_type: 'echo',
log: { 'level' => 'debug' },
action: 'accept'
} => 'rule family="ipv4" destination address="10.0.1.2/24" icmp-type name="echo" log level="debug" accept'
} => 'rule family="ipv4" destination address="10.0.1.2/24" icmp-type name="echo" log level="debug" accept',

## test reject
{
name: 'reject ssh',
ensure: 'present',
family: 'ipv4',
zone: 'restricted',
source: { 'address' => '10.0.1.2/24' },
service: 'ssh',
log: { 'level' => 'debug' },
action: 'reject'
} => 'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" reject',

## test reject + type (#193)
{
name: 'reject ssh tcp reset',
ensure: 'present',
family: 'ipv4',
zone: 'restricted',
source: { 'address' => '10.0.1.2/24' },
service: 'ssh',
log: { 'level' => 'debug' },
action: { 'action' => 'reject', 'type' => 'tcp-reset' }
} => 'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" reject type="tcp-reset"',

}

Expand All @@ -195,9 +219,6 @@
end
let(:fakeclass) { Class.new }
let(:provider) { resource.provider }
let(:rawrule) do
'rule family="ipv4" source address="10.0.1.2/24" service name="ssh" log level="debug" accept'
end

it 'queries the status' do
fakeclass.stubs(:exitstatus).returns(0)
Expand Down