Fix check mode in iptables_state for incomplete iptables-save files along with integration tests

[Maxopoly]

SUMMARY

Fixes https://github.com/ansible-collections/community.general/issues/7463

ISSUE TYPE

• Bugfix Pull Request
• Test Pull Request

COMPONENT NAME

iptables_state

ADDITIONAL INFORMATION

This PR first attempted to reproduce the behavior observed in https://github.com/ansible-collections/community.general/issues/7463 through integration tests. iptables-save doesn't work in docker, so this was tested against the azure pipeline through this PR, please excuse the commit spam.

https://github.com/ansible-collections/community.general/commit/a58bc16f2fb9ef4e5e60346abad4474db6d4933a was able reproduce the issue in https://github.com/ansible-collections/community.general/issues/7463 based on incomplete iptables-save files. Initially, all tables are empty and uninitialized, which will make iptables-save return nothing for them.

For example when creating a nat rule and then deleting it, an output of iptables-save may look like this:

        "*filter",
        ":INPUT ACCEPT [0:0]",
        ":FORWARD DROP [0:0]",
        ":OUTPUT ACCEPT [0:0]",
        "COMMIT",
        "*nat",
        ":PREROUTING ACCEPT [0:0]",
        ":INPUT ACCEPT [0:0]",
        ":OUTPUT ACCEPT [0:0]",
        ":POSTROUTING ACCEPT [0:0]",
        "COMMIT",

while one on a new container/machine which has never had a nat rule will look like this:

        "*filter",
        ":INPUT ACCEPT [0:0]",
        ":FORWARD DROP [0:0]",
        ":OUTPUT ACCEPT [0:0]",
        "COMMIT",

This broke the existing check_mode checks, which did simple string comparison. Most likely there were also edge cases outside of check mode in regards to uninitialized tables of existing iptables-save dumps, which would report changes when restoring them, despite nothing changing. This is only the case for iptables-save files created through means other than this module.

To circument this I implemented parsing of iptables-save dumps into a simple data structure based on preexisting code. If a table currently has some kind of state (including rules) but is not referenced in the iptables-save dump, it will not be touched by iptables-restore and now handled properly by this module.

I also added the tables tables_after after restoring as a data structure, similar to the already existing state tables before running the module

[Maxopoly]

Ok, https://github.com/ansible-collections/community.general/pull/8029/commits/a58bc16f2fb9ef4e5e60346abad4474db6d4933a can reproduce the issue described. Incomplete iptables save files are not handled properly in check mode.

[ansibullbot]

cc @quidame click here for bot help

[ansibullbot]

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:22: W291: trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:22: W291: trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:21: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:21: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:22: W291: trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:21: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:22: W291: trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/iptables_state.py:592:21: trailing-whitespace: Trailing whitespace

click here for bot help

[ansibullbot]

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/iptables_state.py:356:18: E225: missing whitespace around operator
plugins/modules/iptables_state.py:357:17: E225: missing whitespace around operator
plugins/modules/iptables_state.py:360:37: W291: trailing whitespace
plugins/modules/iptables_state.py:363:49: W291: trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/iptables_state.py:356:18: E225: missing whitespace around operator
plugins/modules/iptables_state.py:357:17: E225: missing whitespace around operator
plugins/modules/iptables_state.py:360:37: W291: trailing whitespace
plugins/modules/iptables_state.py:363:49: W291: trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

plugins/modules/iptables_state.py:360:36: trailing-whitespace: Trailing whitespace
plugins/modules/iptables_state.py:363:48: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

plugins/modules/iptables_state.py:360:36: trailing-whitespace: Trailing whitespace
plugins/modules/iptables_state.py:363:48: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/iptables_state.py:356:18: E225: missing whitespace around operator
plugins/modules/iptables_state.py:357:17: E225: missing whitespace around operator
plugins/modules/iptables_state.py:360:37: W291: trailing whitespace
plugins/modules/iptables_state.py:363:49: W291: trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/iptables_state.py:356:18: E225: missing whitespace around operator
plugins/modules/iptables_state.py:357:17: E225: missing whitespace around operator
plugins/modules/iptables_state.py:360:37: W291: trailing whitespace
plugins/modules/iptables_state.py:363:49: W291: trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

plugins/modules/iptables_state.py:360:36: trailing-whitespace: Trailing whitespace
plugins/modules/iptables_state.py:363:48: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

plugins/modules/iptables_state.py:360:36: trailing-whitespace: Trailing whitespace
plugins/modules/iptables_state.py:363:48: trailing-whitespace: Trailing whitespace

click here for bot help

[ansibullbot]

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

plugins/modules/iptables_state.py:210:15: W291: trailing whitespace
plugins/modules/iptables_state.py:239:15: W291: trailing whitespace

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

plugins/modules/iptables_state.py:0:0: invalid-documentation-markup: Directive "O(tables)" contains a non-existing option "tables"
plugins/modules/iptables_state.py:0:0: invalid-documentation-markup: Directive "O(tables_after)" contains a non-existing option "tables_after"

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

plugins/modules/iptables_state.py:210:15: W291: trailing whitespace
plugins/modules/iptables_state.py:239:15: W291: trailing whitespace

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

plugins/modules/iptables_state.py:0:0: invalid-documentation-markup: Directive "O(tables)" contains a non-existing option "tables"
plugins/modules/iptables_state.py:0:0: invalid-documentation-markup: Directive "O(tables_after)" contains a non-existing option "tables_after"

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

plugins/modules/iptables_state.py:210:15: W291: trailing whitespace
plugins/modules/iptables_state.py:239:15: W291: trailing whitespace

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

plugins/modules/iptables_state.py:0:0: invalid-documentation-markup: Directive "O(tables)" contains a non-existing option "tables"
plugins/modules/iptables_state.py:0:0: invalid-documentation-markup: Directive "O(tables_after)" contains a non-existing option "tables_after"

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

plugins/modules/iptables_state.py:210:15: W291: trailing whitespace
plugins/modules/iptables_state.py:239:15: W291: trailing whitespace

The test extra-docs failed with 2 errors:

plugins/modules/iptables_state.py:0:0: RETURN -> tables_after -> description[3]: O(tables): option name does not reference to an existing option of the module community.general.iptables_state
plugins/modules/iptables_state.py:0:0: RETURN -> tables_after -> description[3]: O(tables_after): option name does not reference to an existing option of the module community.general.iptables_state

click here for bot help

[Maxopoly]

Fixed as suggested and removed the additional return value introduced. I'll make an additional PR against master to readd that once this PR is merged, as it depends on the changes made here.

[patchback[bot]]

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/23396e62dc75bb4a9a16f0d1b29d85fb66d93725/pr-8029

Backported as https://github.com/ansible-collections/community.general/pull/8136

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

[felixfontein]

@Maxopoly thanks for your contribution!

[patchback[bot]]

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/23396e62dc75bb4a9a16f0d1b29d85fb66d93725/pr-8029

Backported as https://github.com/ansible-collections/community.general/pull/8137

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.