From ad6faf275d71038863d24c46cbb54b02d57ff26f Mon Sep 17 00:00:00 2001 From: derdeagle Date: Sun, 3 Dec 2023 13:02:08 +0100 Subject: [PATCH] Fixes #236: Add missing parameters for the "ip firewall" subpaths and set the default value for the "disabled" parameter for most paths (#237) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixes #236: Add missing parameters The parameters "address-list", "address-list-timeout", and "realm" were missing for some subpaths of "ip firewall" and are now added. Additionally the default value of "False" for the "disabled" parameter has been set so that an e.g. firewall rule, which was disabled (disabled=True) is enabled (disabled=False) after removing the "disabled" argument in the data. Some more parameters can now be removed, e.g. "jump-target", "log", and "log-prefix", which are not mandatory. * Add missing changes for #236 Additionally fixed the PR id in the changelog fragment. * Update changelogs/fragments/237-add-missing-ip-firewall-attributes.yml Full stop added at the end of the changelog fragment Co-authored-by: Felix Fontein * Update changelogs/fragments/237-add-missing-ip-firewall-attributes.yml Full stop added at the end of the changelog fragment Co-authored-by: Felix Fontein * Fix unit tests --------- Co-authored-by: Johannes Münch Co-authored-by: Felix Fontein --- ...237-add-missing-ip-firewall-attributes.yml | 3 + plugins/module_utils/_api_data.py | 77 +++++++++++-------- .../modules/test_api_find_and_modify.py | 7 ++ tests/unit/plugins/modules/test_api_info.py | 6 ++ 4 files changed, 59 insertions(+), 34 deletions(-) create mode 100644 changelogs/fragments/237-add-missing-ip-firewall-attributes.yml diff --git a/changelogs/fragments/237-add-missing-ip-firewall-attributes.yml b/changelogs/fragments/237-add-missing-ip-firewall-attributes.yml new file mode 100644 index 00000000..677fef50 --- /dev/null +++ b/changelogs/fragments/237-add-missing-ip-firewall-attributes.yml @@ -0,0 +1,3 @@ +minor_changes: + - api_info, api_modify - add missing parameters ``address-list``, ``address-list-timeout``, ``randomise-ports``, and ``realm`` to subpaths of the ``ip firewall`` path (https://github.com/ansible-collections/community.routeros/issues/236, https://github.com/ansible-collections/community.routeros/pull/237). + - api_info, api_modify - set the default value to ``false`` for the ``disabled`` parameter in some more paths where it can be seen in the documentation (https://github.com/ansible-collections/community.routeros/pull/237). diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index f786de8c..747017a8 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -959,7 +959,7 @@ def join_path(path): fields={ 'name': KeyInfo(required=True), 'comment': KeyInfo(can_disable=True, remove_value=''), - 'disabled': KeyInfo(can_disable=True), + 'disabled': KeyInfo(default=False), 'fib': KeyInfo(), }, )), @@ -1007,7 +1007,7 @@ def join_path(path): 'client-to-client-forwarding': KeyInfo(can_disable=True), 'client-tx-limit': KeyInfo(can_disable=True), 'comment': KeyInfo(can_disable=True, remove_value=''), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'interface': KeyInfo(can_disable=True), 'mac-address': KeyInfo(can_disable=True), 'mac-address-mask': KeyInfo(can_disable=True), @@ -1941,6 +1941,8 @@ def join_path(path): stratify_keys=('chain', ), fields={ 'action': KeyInfo(), + 'address-list': KeyInfo(can_disable=True), + 'address-list-timeout': KeyInfo(can_disable=True), 'chain': KeyInfo(), 'comment': KeyInfo(can_disable=True, remove_value=''), 'connection-bytes': KeyInfo(can_disable=True), @@ -1951,7 +1953,7 @@ def join_path(path): 'connection-state': KeyInfo(can_disable=True), 'connection-type': KeyInfo(can_disable=True), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -1969,11 +1971,11 @@ def join_path(path): 'ingress-priority': KeyInfo(can_disable=True), 'ipsec-policy': KeyInfo(can_disable=True), 'ipv4-options': KeyInfo(can_disable=True), - 'jump-target': KeyInfo(), + 'jump-target': KeyInfo(can_disable=True), 'layer7-protocol': KeyInfo(can_disable=True), 'limit': KeyInfo(can_disable=True), - 'log': KeyInfo(), - 'log-prefix': KeyInfo(), + 'log': KeyInfo(can_disable=True), + 'log-prefix': KeyInfo(can_disable=True), 'nth': KeyInfo(can_disable=True), 'out-bridge-port': KeyInfo(can_disable=True), 'out-bridge-port-list': KeyInfo(can_disable=True), @@ -1988,7 +1990,8 @@ def join_path(path): 'protocol': KeyInfo(can_disable=True), 'psd': KeyInfo(can_disable=True), 'random': KeyInfo(can_disable=True), - 'reject-with': KeyInfo(), + 'realm': KeyInfo(can_disable=True), + 'reject-with': KeyInfo(can_disable=True), 'routing-mark': KeyInfo(can_disable=True), 'routing-table': KeyInfo(can_disable=True), 'src-address': KeyInfo(can_disable=True), @@ -2010,6 +2013,8 @@ def join_path(path): stratify_keys=('chain', ), fields={ 'action': KeyInfo(), + 'address-list': KeyInfo(can_disable=True), + 'address-list-timeout': KeyInfo(can_disable=True), 'chain': KeyInfo(), 'comment': KeyInfo(can_disable=True, remove_value=''), 'connection-bytes': KeyInfo(can_disable=True), @@ -2020,7 +2025,7 @@ def join_path(path): 'connection-state': KeyInfo(can_disable=True), 'connection-type': KeyInfo(can_disable=True), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2037,11 +2042,11 @@ def join_path(path): 'ingress-priority': KeyInfo(can_disable=True), 'ipsec-policy': KeyInfo(can_disable=True), 'ipv4-options': KeyInfo(can_disable=True), - 'jump-target': KeyInfo(), + 'jump-target': KeyInfo(can_disable=True), 'layer7-protocol': KeyInfo(can_disable=True), 'limit': KeyInfo(can_disable=True), - 'log': KeyInfo(), - 'log-prefix': KeyInfo(), + 'log': KeyInfo(can_disable=True), + 'log-prefix': KeyInfo(can_disable=True), 'new-connection-mark': KeyInfo(can_disable=True), 'new-dscp': KeyInfo(can_disable=True), 'new-mss': KeyInfo(can_disable=True), @@ -2064,6 +2069,7 @@ def join_path(path): 'protocol': KeyInfo(can_disable=True), 'psd': KeyInfo(can_disable=True), 'random': KeyInfo(can_disable=True), + 'realm': KeyInfo(can_disable=True), 'route-dst': KeyInfo(can_disable=True), 'routing-mark': KeyInfo(can_disable=True), 'routing-table': KeyInfo(can_disable=True), @@ -2089,8 +2095,8 @@ def join_path(path): stratify_keys=('chain', ), fields={ 'action': KeyInfo(), - 'address-list': KeyInfo(), - 'address-list-timeout': KeyInfo(), + 'address-list': KeyInfo(can_disable=True), + 'address-list-timeout': KeyInfo(can_disable=True), 'chain': KeyInfo(), 'comment': KeyInfo(can_disable=True, remove_value=''), 'connection-bytes': KeyInfo(can_disable=True), @@ -2099,7 +2105,7 @@ def join_path(path): 'connection-rate': KeyInfo(can_disable=True), 'connection-type': KeyInfo(can_disable=True), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2116,11 +2122,11 @@ def join_path(path): 'ingress-priority': KeyInfo(can_disable=True), 'ipsec-policy': KeyInfo(can_disable=True), 'ipv4-options': KeyInfo(can_disable=True), - 'jump-target': KeyInfo(), + 'jump-target': KeyInfo(can_disable=True), 'layer7-protocol': KeyInfo(can_disable=True), 'limit': KeyInfo(can_disable=True), - 'log': KeyInfo(), - 'log-prefix': KeyInfo(), + 'log': KeyInfo(can_disable=True), + 'log-prefix': KeyInfo(can_disable=True), 'nth': KeyInfo(can_disable=True), 'out-bridge-port': KeyInfo(can_disable=True), 'out-bridge-port-list': KeyInfo(can_disable=True), @@ -2134,9 +2140,10 @@ def join_path(path): 'protocol': KeyInfo(can_disable=True), 'psd': KeyInfo(can_disable=True), 'random': KeyInfo(can_disable=True), + 'randomise-ports': KeyInfo(can_disable=True), 'realm': KeyInfo(can_disable=True), 'routing-mark': KeyInfo(can_disable=True), - 'same-not-by-dst': KeyInfo(), + 'same-not-by-dst': KeyInfo(can_disable=True), 'src-address': KeyInfo(can_disable=True), 'src-address-list': KeyInfo(can_disable=True), 'src-address-type': KeyInfo(can_disable=True), @@ -2157,12 +2164,12 @@ def join_path(path): stratify_keys=('chain',), fields={ 'action': KeyInfo(), - 'address-list': KeyInfo(), - 'address-list-timeout': KeyInfo(), + 'address-list': KeyInfo(can_disable=True), + 'address-list-timeout': KeyInfo(can_disable=True), 'chain': KeyInfo(), 'comment': KeyInfo(can_disable=True, remove_value=''), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2179,10 +2186,10 @@ def join_path(path): 'ingress-priority': KeyInfo(can_disable=True), 'ipsec-policy': KeyInfo(can_disable=True), 'ipv4-options': KeyInfo(can_disable=True), - 'jump-target': KeyInfo(), + 'jump-target': KeyInfo(can_disable=True), 'limit': KeyInfo(can_disable=True), - 'log': KeyInfo(), - 'log-prefix': KeyInfo(), + 'log': KeyInfo(can_disable=True), + 'log-prefix': KeyInfo(can_disable=True), 'nth': KeyInfo(can_disable=True), 'out-bridge-port': KeyInfo(can_disable=True), 'out-bridge-port-list': KeyInfo(can_disable=True), @@ -2503,6 +2510,8 @@ def join_path(path): stratify_keys=('chain', ), fields={ 'action': KeyInfo(), + 'address-list': KeyInfo(can_disable=True), + 'address-list-timeout': KeyInfo(can_disable=True), 'chain': KeyInfo(), 'comment': KeyInfo(can_disable=True, remove_value=''), 'connection-bytes': KeyInfo(can_disable=True), @@ -2512,7 +2521,7 @@ def join_path(path): 'connection-state': KeyInfo(can_disable=True), 'connection-type': KeyInfo(can_disable=True), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2528,10 +2537,10 @@ def join_path(path): 'in-interface-list': KeyInfo(can_disable=True), 'ingress-priority': KeyInfo(can_disable=True), 'ipsec-policy': KeyInfo(can_disable=True), - 'jump-target': KeyInfo(), + 'jump-target': KeyInfo(can_disable=True), 'limit': KeyInfo(can_disable=True), - 'log': KeyInfo(), - 'log-prefix': KeyInfo(), + 'log': KeyInfo(can_disable=False), + 'log-prefix': KeyInfo(can_disable=False), 'nth': KeyInfo(can_disable=True), 'out-bridge-port': KeyInfo(can_disable=True), 'out-bridge-port-list': KeyInfo(can_disable=True), @@ -2573,7 +2582,7 @@ def join_path(path): 'connection-state': KeyInfo(can_disable=True), 'connection-type': KeyInfo(can_disable=True), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2647,7 +2656,7 @@ def join_path(path): 'connection-state': KeyInfo(can_disable=True), 'connection-type': KeyInfo(can_disable=True), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2703,7 +2712,7 @@ def join_path(path): 'chain': KeyInfo(), 'comment': KeyInfo(can_disable=True, remove_value=''), 'content': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'dscp': KeyInfo(can_disable=True), 'dst-address': KeyInfo(can_disable=True), 'dst-address-list': KeyInfo(can_disable=True), @@ -2796,7 +2805,7 @@ def join_path(path): 'blackhole': KeyInfo(can_disable=True), 'check-gateway': KeyInfo(can_disable=True), 'comment': KeyInfo(can_disable=True, remove_value=''), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'distance': KeyInfo(default=1), 'dst-address': KeyInfo(), 'gateway': KeyInfo(), @@ -3581,7 +3590,7 @@ def join_path(path): 'cluster-id': KeyInfo(), 'comment': KeyInfo(), 'connect': KeyInfo(default=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'hold-time': KeyInfo(), 'input.accept-communities': KeyInfo(), 'input.accept-ext-communities': KeyInfo(), @@ -3640,7 +3649,7 @@ def join_path(path): 'client-to-client-reflection': KeyInfo(), 'cluster-id': KeyInfo(can_disable=True), 'confederation': KeyInfo(can_disable=True), - 'disabled': KeyInfo(), + 'disabled': KeyInfo(default=False), 'ignore-as-path-len': KeyInfo(), 'name': KeyInfo(), 'out-filter': KeyInfo(), diff --git a/tests/unit/plugins/modules/test_api_find_and_modify.py b/tests/unit/plugins/modules/test_api_find_and_modify.py index 7587107d..2f47af4f 100644 --- a/tests/unit/plugins/modules/test_api_find_and_modify.py +++ b/tests/unit/plugins/modules/test_api_find_and_modify.py @@ -619,24 +619,28 @@ def test_change_remove_generic(self): 'chain': 'input', 'comment': 'defconf', 'protocol': 'icmp', + 'disabled': False, }, { '.id': '*3', 'action': 'accept', 'chain': 'input', 'comment': 'defconf', + 'disabled': False, }, { '.id': '*4', 'action': 'accept', 'chain': 'input', 'comment': 'defconf', + 'disabled': False, }, { '.id': '*7', 'action': 'drop', 'chain': 'input', 'comment': 'defconf', + 'disabled': False, 'in-interface': 'wan', }, { @@ -645,6 +649,7 @@ def test_change_remove_generic(self): 'chain': 'forward', 'comment': 'defconf', 'connection-state': 'established', + 'disabled': False, }, { '.id': '*9', @@ -652,6 +657,7 @@ def test_change_remove_generic(self): 'chain': 'forward', 'comment': 'defconf', 'connection-state': 'related', + 'disabled': False, }, { '.id': '*A', @@ -659,6 +665,7 @@ def test_change_remove_generic(self): 'chain': 'forward', 'comment': 'defconf', 'connection-status': 'invalid', + 'disabled': False, }, ]) self.assertEqual(result['match_count'], 3) diff --git a/tests/unit/plugins/modules/test_api_info.py b/tests/unit/plugins/modules/test_api_info.py index cd83d8ac..3564668e 100644 --- a/tests/unit/plugins/modules/test_api_info.py +++ b/tests/unit/plugins/modules/test_api_info.py @@ -198,6 +198,8 @@ def test_disabled_exclamation(self, mock_compose_api_path): 'chain': 'input', 'in-interface-list': 'LAN', '!action': None, + '!address-list': None, + '!address-list-timeout': None, '!comment': None, '!connection-bytes': None, '!connection-limit': None, @@ -243,6 +245,7 @@ def test_disabled_exclamation(self, mock_compose_api_path): '!protocol': None, '!psd': None, '!random': None, + '!realm': None, '!reject-with': None, '!routing-mark': None, '!routing-table': None, @@ -284,6 +287,8 @@ def test_disabled_null_value(self, mock_compose_api_path): 'chain': 'input', 'in-interface-list': 'LAN', 'action': None, + 'address-list': None, + 'address-list-timeout': None, 'comment': None, 'connection-bytes': None, 'connection-limit': None, @@ -329,6 +334,7 @@ def test_disabled_null_value(self, mock_compose_api_path): 'protocol': None, 'psd': None, 'random': None, + 'realm': None, 'reject-with': None, 'routing-mark': None, 'routing-table': None,