diff --git a/changelogs/fragments/8029-iptables-state-restore-check-mode.yml b/changelogs/fragments/8029-iptables-state-restore-check-mode.yml new file mode 100644 index 0000000000..900ea50988 --- /dev/null +++ b/changelogs/fragments/8029-iptables-state-restore-check-mode.yml @@ -0,0 +1,2 @@ +bugfixes: + - iptables_state - fix idempotency issues when restoring incomplete iptables dumps (https://github.com/ansible-collections/community.general/issues/8029). diff --git a/plugins/modules/iptables_state.py b/plugins/modules/iptables_state.py index 79c0e26c48..b0cc3bd3f6 100644 --- a/plugins/modules/iptables_state.py +++ b/plugins/modules/iptables_state.py @@ -207,7 +207,9 @@ saved: "# Completed" ] tables: - description: The iptables we have interest for when module starts. + description: + - The iptables on the system before the module has run, separated by table. + - If the option O(table) is used, only this table is included. type: dict contains: table: @@ -346,20 +348,27 @@ def filter_and_format_state(string): return lines -def per_table_state(command, state): +def parse_per_table_state(all_states_dump): ''' Convert raw iptables-save output into usable datastructure, for reliable comparisons between initial and final states. ''' + lines = filter_and_format_state(all_states_dump) tables = dict() - for t in TABLES: - COMMAND = list(command) - if '*%s' % t in state.splitlines(): - COMMAND.extend(['--table', t]) - dummy, out, dummy = module.run_command(COMMAND, check_rc=True) - out = re.sub(r'(^|\n)(# Generated|# Completed|[*]%s|COMMIT)[^\n]*' % t, r'', out) - out = re.sub(r' *\[[0-9]+:[0-9]+\] *', r'', out) - tables[t] = [tt for tt in out.splitlines() if tt != ''] + current_table = '' + current_list = list() + for line in lines: + if re.match(r'^[*](filter|mangle|nat|raw|security)$', line): + current_table = line[1:] + continue + if line == 'COMMIT': + tables[current_table] = current_list + current_table = '' + current_list = list() + continue + if line.startswith('# '): + continue + current_list.append(line) return tables @@ -486,7 +495,7 @@ def main(): # Depending on the value of 'table', initref_state may differ from # initial_state. (rc, stdout, stderr) = module.run_command(SAVECOMMAND, check_rc=True) - tables_before = per_table_state(SAVECOMMAND, stdout) + tables_before = parse_per_table_state(stdout) initref_state = filter_and_format_state(stdout) if state == 'saved': @@ -583,14 +592,17 @@ def main(): (rc, stdout, stderr) = module.run_command(SAVECOMMAND, check_rc=True) restored_state = filter_and_format_state(stdout) - + tables_after = parse_per_table_state('\n'.join(restored_state)) if restored_state not in (initref_state, initial_state): - if module.check_mode: - changed = True - else: - tables_after = per_table_state(SAVECOMMAND, stdout) - if tables_after != tables_before: + for table_name, table_content in tables_after.items(): + if table_name not in tables_before: + # Would initialize a table, which doesn't exist yet changed = True + break + if tables_before[table_name] != table_content: + # Content of some table changes + changed = True + break if _back is None or module.check_mode: module.exit_json( @@ -633,7 +645,7 @@ def main(): os.remove(b_back) (rc, stdout, stderr) = module.run_command(SAVECOMMAND, check_rc=True) - tables_rollback = per_table_state(SAVECOMMAND, stdout) + tables_rollback = parse_per_table_state(stdout) msg = ( "Failed to confirm state restored from %s after %ss. " diff --git a/tests/integration/targets/iptables_state/tasks/main.yml b/tests/integration/targets/iptables_state/tasks/main.yml index a74e74df48..d550070677 100644 --- a/tests/integration/targets/iptables_state/tasks/main.yml +++ b/tests/integration/targets/iptables_state/tasks/main.yml @@ -29,6 +29,12 @@ when: - xtables_lock is undefined + - name: include tasks to test partial restore files + include_tasks: tests/02-partial-restore.yml + when: + - xtables_lock is undefined + + - name: include tasks to test rollbacks include_tasks: tests/10-rollback.yml when: diff --git a/tests/integration/targets/iptables_state/tasks/tests/02-partial-restore.yml b/tests/integration/targets/iptables_state/tasks/tests/02-partial-restore.yml new file mode 100644 index 0000000000..6da4814af0 --- /dev/null +++ b/tests/integration/targets/iptables_state/tasks/tests/02-partial-restore.yml @@ -0,0 +1,66 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: "Create initial rule set to use" + copy: + dest: "{{ iptables_tests }}" + content: | + *filter + :INPUT ACCEPT [0:0] + :FORWARD ACCEPT [0:0] + :OUTPUT ACCEPT [0:0] + -A INPUT -m state --state NEW,ESTABLISHED -j ACCEPT + COMMIT + *nat + :PREROUTING ACCEPT [151:17304] + :INPUT ACCEPT [151:17304] + :OUTPUT ACCEPT [151:17304] + :POSTROUTING ACCEPT [151:17304] + -A POSTROUTING -o eth0 -j MASQUERADE + COMMIT + +- name: "Restore initial state" + iptables_state: + path: "{{ iptables_tests }}" + state: restored + async: "{{ ansible_timeout }}" + poll: 0 + +- name: "Create partial ruleset only specifying input" + copy: + dest: "{{ iptables_tests }}" + content: | + *filter + :INPUT ACCEPT [0:0] + :FORWARD ACCEPT [0:0] + :OUTPUT ACCEPT [0:0] + -A INPUT -m state --state NEW,ESTABLISHED -j ACCEPT + COMMIT + +- name: "Check restoring partial state" + iptables_state: + path: "{{ iptables_tests }}" + state: restored + check_mode: true + register: iptables_state + + +- name: "assert that no changes are detected in check mode" + assert: + that: + - iptables_state is not changed + +- name: "Restore partial state" + iptables_state: + path: "{{ iptables_tests }}" + state: restored + register: iptables_state + async: "{{ ansible_timeout }}" + poll: 0 + +- name: "assert that no changes are made" + assert: + that: + - iptables_state is not changed \ No newline at end of file