From 23396e62dc75bb4a9a16f0d1b29d85fb66d93725 Mon Sep 17 00:00:00 2001 From: Maxopoly Date: Sun, 24 Mar 2024 18:02:48 +0100 Subject: [PATCH] Fix check mode in iptables_state for incomplete iptables-save files along with integration tests (#8029) * Implement integration test to reproduce #7463 * Make new iptables_state checks async * Add missing commit to iptable_state integration test * Remove async when using checkmode in iptables_state integration tests * Do per table comparison in check mode for iptables_state * Calculate changes of iptables state per table based on result * Output target iptables state in checkmode * Refactor calculation of invidual table states in iptables_state * Add missing return for table calculation * Add missing arg to regex check * Remove leftover debug output for target iptable state * Parse per table state from raw state string * Join restored state for extration of table specific rules * Switch arguments for joining restored iptable state * Output final ip table state * Compare content of tables * Complete iptables partial tables test cases * Correct order of test iptables data * Update docu for iptables tables_after * Add changelog fragment * Appease the linting gods for iptables_state * Adjust spelling and remove tables_after from return values --- ...8029-iptables-state-restore-check-mode.yml | 2 + plugins/modules/iptables_state.py | 48 +++++++++----- .../targets/iptables_state/tasks/main.yml | 6 ++ .../tasks/tests/02-partial-restore.yml | 66 +++++++++++++++++++ 4 files changed, 104 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/8029-iptables-state-restore-check-mode.yml create mode 100644 tests/integration/targets/iptables_state/tasks/tests/02-partial-restore.yml 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