From bce38be54f439f114113923ae3d7fbfef178b4d0 Mon Sep 17 00:00:00 2001 From: Ben Magistro Date: Fri, 24 Jun 2022 13:57:57 +0000 Subject: [PATCH 1/2] #193 [tests] add failing unit test for action + type Signed-off-by: Ben Magistro --- .../puppet/type/firewalld_rich_rule_spec.rb | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb index 6fd166e4..cfdb0fdc 100644 --- a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb @@ -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"', } From 19f4b15cb1caca0ec7f0cf7fce9443d3c4d01104 Mon Sep 17 00:00:00 2001 From: Ben Magistro Date: Sun, 26 Jun 2022 17:23:22 -0400 Subject: [PATCH 2/2] #193 fix rich rule typed action Fixes: #193 Replaces: #194 Signed-off-by: Ben Magistro --- lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb | 4 ++-- lib/puppet/type/firewalld_rich_rule.rb | 4 ++-- spec/unit/puppet/provider/firewalld_rich_rule_spec.rb | 2 +- spec/unit/puppet/type/firewalld_rich_rule_spec.rb | 7 ++----- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb index 3112f55b..21a58bc3 100644 --- a/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb +++ b/lib/puppet/provider/firewalld_rich_rule/firewall_cmd.rb @@ -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 diff --git a/lib/puppet/type/firewalld_rich_rule.rb b/lib/puppet/type/firewalld_rich_rule.rb index 447f3da8..13156c3d 100644 --- a/lib/puppet/type/firewalld_rich_rule.rb +++ b/lib/puppet/type/firewalld_rich_rule.rb @@ -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 diff --git a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb index 1f671607..380ed1ca 100644 --- a/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/provider/firewalld_rich_rule_spec.rb @@ -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 diff --git a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb index cfdb0fdc..4790e823 100644 --- a/spec/unit/puppet/type/firewalld_rich_rule_spec.rb +++ b/spec/unit/puppet/type/firewalld_rich_rule_spec.rb @@ -42,7 +42,7 @@ 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 @@ -50,7 +50,7 @@ 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 @@ -219,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)