Skip to content

Commit 4d6b4b3

Browse files
shubhamshinde360shubhamshinde360
committed
(MODULES-11476) Fix non-idempotency of firewall table creation
The 'iptables-save' command behaves differently on rpm and deb based platforms. rpm based: If an iptable has been interacted with, it will show up in the command even if it is empty. deb based: If an iptable has been interacted with, it may not show up in the command if it is empty. If we add and then delete a rule, it shows up. But if we create the table using "iptables -N" it doesn't. Specific tables always show up with "iptables -t #{table_name}". Since, the tables are always already created and the module supports a specific set of them. This commit tries to check if they exist one by one using the "iptables -t" command. Also, since the tables are being checked like this, all tables show up in the output, so adjusted the rpsec according to that.
1 parent 8d23172 commit 4d6b4b3

File tree

2 files changed

+71
-9
lines changed

2 files changed

+71
-9
lines changed

‎lib/puppet/provider/firewallchain/firewallchain.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class Puppet::Provider::Firewallchain::Firewallchain
1717
$table_regex = %r{(\*(?:nat|mangle|filter|raw|rawpost|broute|security)[^*]+)}
1818
# Regex used to retrieve table name
1919
$table_name_regex = %r{^\*(nat|mangle|filter|raw|rawpost|broute|security)}
20+
$supported_tables = ['nat', 'mangle', 'filter', 'raw', 'rawpost', 'broute', 'security']
2021
# Regex used to retrieve Chains
2122
$chain_regex = %r{\n:(INPUT|FORWARD|OUTPUT|(?:\S+))(?:\s(ACCEPT|DROP|QEUE|RETURN|PREROUTING|POSTROUTING))?}
2223
# Base commands for the protocols, including table affixes
@@ -49,11 +50,10 @@ def get(_context)
4950
chains = []
5051
# Scan String to retrieve all Chains and Policies
5152
['IPv4', 'IPv6'].each do |protocol|
52-
# Retrieve String containing all IPv4 information
53-
iptables_list = Puppet::Provider.execute($list_command[protocol])
54-
iptables_list.scan($table_regex).each do |table|
55-
table_name = table[0].scan($table_name_regex)[0][0]
56-
table[0].scan($chain_regex).each do |chain|
53+
# Go through each supported table and retrieve its chains if it exists.
54+
$supported_tables.each do |table_name|
55+
cmd_output = Puppet::Provider.execute([$list_command[protocol], '-t', table_name].join(' '), failonfail: false)
56+
cmd_output.scan($chain_regex).each do |chain|
5757
# Create the base hash
5858
chain_hash = {
5959
name: "#{chain[0]}:#{table_name}:#{protocol}",

‎spec/unit/puppet/provider/firewallchain/firewallchain_spec.rb

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,83 @@
4545
'
4646
end
4747
let(:returned_data) do
48-
[{ name: 'INPUT:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
48+
[{ name: 'INPUT:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
49+
{ name: 'FORWARD:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
50+
{ name: 'OUTPUT:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
51+
{ name: 'TEST_ONE:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
52+
{ name: 'PREROUTING:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
53+
{ name: 'OUTPUT:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
54+
{ name: 'TEST_TWO:nat:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
55+
{ name: 'INPUT:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
56+
{ name: 'FORWARD:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
57+
{ name: 'OUTPUT:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
58+
{ name: 'TEST_ONE:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
59+
{ name: 'PREROUTING:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
60+
{ name: 'OUTPUT:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
61+
{ name: 'TEST_TWO:mangle:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
62+
{ name: 'INPUT:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
4963
{ name: 'FORWARD:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
5064
{ name: 'OUTPUT:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
5165
{ name: 'TEST_ONE:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
66+
{ name: 'PREROUTING:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
67+
{ name: 'OUTPUT:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
68+
{ name: 'TEST_TWO:filter:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
69+
{ name: 'INPUT:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
70+
{ name: 'FORWARD:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
71+
{ name: 'OUTPUT:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
72+
{ name: 'TEST_ONE:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
5273
{ name: 'PREROUTING:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
5374
{ name: 'OUTPUT:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
5475
{ name: 'TEST_TWO:raw:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
76+
{ name: 'INPUT:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
77+
{ name: 'FORWARD:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
78+
{ name: 'OUTPUT:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
79+
{ name: 'TEST_ONE:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
80+
{ name: 'PREROUTING:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
81+
{ name: 'OUTPUT:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
82+
{ name: 'TEST_TWO:rawpost:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
83+
{ name: 'INPUT:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
84+
{ name: 'FORWARD:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
85+
{ name: 'OUTPUT:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
86+
{ name: 'TEST_ONE:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
87+
{ name: 'PREROUTING:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
88+
{ name: 'OUTPUT:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
89+
{ name: 'TEST_TWO:broute:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
90+
{ name: 'INPUT:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
91+
{ name: 'FORWARD:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
92+
{ name: 'OUTPUT:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
93+
{ name: 'TEST_ONE:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
94+
{ name: 'PREROUTING:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
95+
{ name: 'OUTPUT:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
96+
{ name: 'TEST_TWO:security:IPv4', purge: false, ignore_foreign: false, ensure: 'present' },
97+
{ name: 'INPUT:nat:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
98+
{ name: 'FORWARD:nat:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
99+
{ name: 'OUTPUT:nat:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
100+
{ name: 'INPUT:mangle:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
101+
{ name: 'FORWARD:mangle:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
102+
{ name: 'OUTPUT:mangle:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
55103
{ name: 'INPUT:filter:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
56104
{ name: 'FORWARD:filter:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
57-
{ name: 'OUTPUT:filter:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' }]
105+
{ name: 'OUTPUT:filter:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
106+
{ name: 'INPUT:raw:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
107+
{ name: 'FORWARD:raw:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
108+
{ name: 'OUTPUT:raw:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
109+
{ name: 'INPUT:rawpost:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
110+
{ name: 'FORWARD:rawpost:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
111+
{ name: 'OUTPUT:rawpost:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
112+
{ name: 'INPUT:broute:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
113+
{ name: 'FORWARD:broute:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
114+
{ name: 'OUTPUT:broute:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
115+
{ name: 'INPUT:security:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
116+
{ name: 'FORWARD:security:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' },
117+
{ name: 'OUTPUT:security:IPv6', purge: false, ignore_foreign: false, ensure: 'present', policy: 'accept' }]
58118
end
59119

60120
it 'processes the resource' do
61-
allow(Puppet::Util::Execution).to receive(:execute).with('iptables-save').and_return(iptables)
62-
allow(Puppet::Util::Execution).to receive(:execute).with('ip6tables-save').and_return(ip6tables)
121+
['nat', 'mangle', 'filter', 'raw', 'rawpost', 'broute', 'security'].each do |table|
122+
allow(Puppet::Provider).to receive(:execute).with("iptables-save -t #{table}", any_args).and_return(iptables)
123+
allow(Puppet::Provider).to receive(:execute).with("ip6tables-save -t #{table}", any_args).and_return(ip6tables)
124+
end
63125

64126
expect(provider.get(context)).to eq(returned_data)
65127
end

0 commit comments

Comments
 (0)