From e056bbb881a05ea1c8d345444e5cc41371d587c6 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 11:39:32 +0100 Subject: [PATCH 01/11] adding support for api fields that can be disabled and have default value at the same time Signed-off-by: Tomas Herfert --- plugins/module_utils/_api_data.py | 5 +++-- plugins/modules/api_modify.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index 5bac5341..996389ca 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -49,8 +49,9 @@ class KeyInfo(object): def __init__(self, _dummy=None, can_disable=False, remove_value=None, absent_value=None, default=None, required=False, automatically_computed_from=None): if _dummy is not None: raise ValueError('KeyInfo() does not have positional arguments') - if sum([required, default is not None, automatically_computed_from is not None, can_disable]) > 1: - raise ValueError('required, default, automatically_computed_from, and can_disable are mutually exclusive') + sm = sum([required, default is not None, automatically_computed_from is not None, can_disable]) + if sm > 1 and sm > sum([default is not None, can_disable]): + raise ValueError('required, default, automatically_computed_from, and can_disable are mutually exclusive besides default and can_disable which can be set together') if not can_disable and remove_value is not None: raise ValueError('remove_value can only be specified if can_disable=True') if absent_value is not None and any([default is not None, automatically_computed_from is not None, can_disable]): diff --git a/plugins/modules/api_modify.py b/plugins/modules/api_modify.py index 633836b6..7e078a7d 100644 --- a/plugins/modules/api_modify.py +++ b/plugins/modules/api_modify.py @@ -336,7 +336,7 @@ def find_modifications(old_entry, new_entry, path_info, module, for_text='', ret modifications['!%s' % disabled_k] = '' del updated_entry[disabled_k] continue - if k not in old_entry and path_info.fields[k].default == v: + if k not in old_entry and path_info.fields[k].default == v and not path_info.fields[k].can_disable: continue if k not in old_entry or old_entry[k] != v: modifications[k] = v @@ -352,7 +352,9 @@ def find_modifications(old_entry, new_entry, path_info, module, for_text='', ret if field_info.remove_value is not None and field_info.remove_value == old_entry[k]: continue if field_info.can_disable: - if field_info.remove_value is not None: + if field_info.default is not None: + modifications[k] = field_info.default + elif field_info.remove_value is not None: modifications[k] = field_info.remove_value else: modifications['!%s' % k] = '' @@ -364,6 +366,11 @@ def find_modifications(old_entry, new_entry, path_info, module, for_text='', ret if return_none_instead_of_fail: return None, None module.fail_json(msg='Key "{key}" cannot be removed{for_text}.'.format(key=k, for_text=for_text)) + for k in path_info.fields: + field_info = path_info.fields[k] + if k not in old_entry and k not in new_entry and field_info.can_disable and field_info.default is not None: + modifications[k] = field_info.default + updated_entry[k] = field_info.default return modifications, updated_entry From 65f9aa70c17aa714d7dc38532c92b9a524e39d11 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 11:40:22 +0100 Subject: [PATCH 02/11] api path support: interface gre Signed-off-by: Tomas Herfert --- plugins/module_utils/_api_data.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index 996389ca..5d61e4f4 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -146,6 +146,24 @@ def join_path(path): 'tx-flow-control': KeyInfo(default='off'), }, ), + ('interface', 'gre'): APIData( + fully_understood=True, + primary_keys=('name', ), + fields={ + 'allow-fast-path': KeyInfo(default=True), + 'clamp-tcp-mss': KeyInfo(default=True), + 'comment': KeyInfo(can_disable=True, remove_value=''), + 'disabled': KeyInfo(default=False), + 'dont-fragment': KeyInfo(default=False), + 'dscp': KeyInfo(default='inherit'), + 'ipsec-secret': KeyInfo(can_disable=True), + 'keepalive': KeyInfo(default='10s,10', can_disable=True), + 'local-address': KeyInfo(default='0.0.0.0'), + 'mtu': KeyInfo(default='auto'), + 'name': KeyInfo(), + 'remote-address': KeyInfo(required=True), + }, + ), ('interface', 'list'): APIData( primary_keys=('name', ), fully_understood=True, From 498d3453eb50e1f163ce8c44e98f455ab4ad3b5a Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 11:41:11 +0100 Subject: [PATCH 03/11] docs Signed-off-by: Tomas Herfert --- plugins/modules/api_info.py | 1 + plugins/modules/api_modify.py | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/modules/api_info.py b/plugins/modules/api_info.py index b8d6aa69..a46ffed3 100644 --- a/plugins/modules/api_info.py +++ b/plugins/modules/api_info.py @@ -60,6 +60,7 @@ - interface ethernet - interface ethernet switch - interface ethernet switch port + - interface gre - interface l2tp-server server - interface list - interface list member diff --git a/plugins/modules/api_modify.py b/plugins/modules/api_modify.py index 7e078a7d..528f433a 100644 --- a/plugins/modules/api_modify.py +++ b/plugins/modules/api_modify.py @@ -65,6 +65,7 @@ - interface ethernet - interface ethernet switch - interface ethernet switch port + - interface gre - interface l2tp-server server - interface list - interface list member From 514ff39f2a6b59486752949692815c792b3301c6 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 12:10:58 +0100 Subject: [PATCH 04/11] unit test update & yamlling fix Signed-off-by: Tomas Herfert --- plugins/module_utils/_api_data.py | 4 +++- tests/unit/plugins/module_utils/test__api_data.py | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index 5d61e4f4..61aa9ce7 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -51,7 +51,9 @@ def __init__(self, _dummy=None, can_disable=False, remove_value=None, absent_val raise ValueError('KeyInfo() does not have positional arguments') sm = sum([required, default is not None, automatically_computed_from is not None, can_disable]) if sm > 1 and sm > sum([default is not None, can_disable]): - raise ValueError('required, default, automatically_computed_from, and can_disable are mutually exclusive besides default and can_disable which can be set together') + raise ValueError( + 'required, default, automatically_computed_from, and can_disable are mutually exclusive ' + + 'besides default and can_disable which can be set together') if not can_disable and remove_value is not None: raise ValueError('remove_value can only be specified if can_disable=True') if absent_value is not None and any([default is not None, automatically_computed_from is not None, can_disable]): diff --git a/tests/unit/plugins/module_utils/test__api_data.py b/tests/unit/plugins/module_utils/test__api_data.py index 67b5b1b4..dced66fc 100644 --- a/tests/unit/plugins/module_utils/test__api_data.py +++ b/tests/unit/plugins/module_utils/test__api_data.py @@ -63,11 +63,20 @@ def test_key_info_errors(): ('can_disable', True), ] + values_allowed_together = [ + ('default', ''), + ('can_disable', True), + ] + for index, (param, param_value) in enumerate(values): for param2, param2_value in values[index + 1:]: + if param in values_allowed_together and param2 in values_allowed_together: + continue with pytest.raises(ValueError) as exc: KeyInfo(**{param: param_value, param2: param2_value}) - assert exc.value.args[0] == 'required, default, automatically_computed_from, and can_disable are mutually exclusive' + assert exc.value.args[0] == ( + 'required, default, automatically_computed_from, and can_disable are mutually exclusive ' + + 'besides default and can_disable which can be set together') with pytest.raises(ValueError) as exc: KeyInfo('foo') From 1b393572819004c20994fb1c127e5c1e250ccfd2 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 12:37:33 +0100 Subject: [PATCH 05/11] test fix Signed-off-by: Tomas Herfert --- tests/unit/plugins/module_utils/test__api_data.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/plugins/module_utils/test__api_data.py b/tests/unit/plugins/module_utils/test__api_data.py index dced66fc..ac56d75b 100644 --- a/tests/unit/plugins/module_utils/test__api_data.py +++ b/tests/unit/plugins/module_utils/test__api_data.py @@ -63,20 +63,20 @@ def test_key_info_errors(): ('can_disable', True), ] - values_allowed_together = [ - ('default', ''), - ('can_disable', True), + params_allowed_together = [ + 'default', + 'can_disable', ] for index, (param, param_value) in enumerate(values): for param2, param2_value in values[index + 1:]: - if param in values_allowed_together and param2 in values_allowed_together: + if param in params_allowed_together and param2 in params_allowed_together: continue with pytest.raises(ValueError) as exc: KeyInfo(**{param: param_value, param2: param2_value}) assert exc.value.args[0] == ( - 'required, default, automatically_computed_from, and can_disable are mutually exclusive ' + - 'besides default and can_disable which can be set together') + 'required, default, automatically_computed_from, and can_disable are mutually exclusive ' + + 'besides default and can_disable which can be set together') with pytest.raises(ValueError) as exc: KeyInfo('foo') From a5f24a1b14671255a01e3d497faefdf42dc72e02 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 12:55:21 +0100 Subject: [PATCH 06/11] sanity fix Signed-off-by: Tomas Herfert --- tests/unit/plugins/module_utils/test__api_data.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/plugins/module_utils/test__api_data.py b/tests/unit/plugins/module_utils/test__api_data.py index ac56d75b..6220e292 100644 --- a/tests/unit/plugins/module_utils/test__api_data.py +++ b/tests/unit/plugins/module_utils/test__api_data.py @@ -68,15 +68,14 @@ def test_key_info_errors(): 'can_disable', ] + emsg = 'required, default, automatically_computed_from, and can_disable are mutually exclusive besides default and can_disable which can be set together' for index, (param, param_value) in enumerate(values): for param2, param2_value in values[index + 1:]: if param in params_allowed_together and param2 in params_allowed_together: continue with pytest.raises(ValueError) as exc: KeyInfo(**{param: param_value, param2: param2_value}) - assert exc.value.args[0] == ( - 'required, default, automatically_computed_from, and can_disable are mutually exclusive ' + - 'besides default and can_disable which can be set together') + assert exc.value.args[0] == emsg with pytest.raises(ValueError) as exc: KeyInfo('foo') From 11fa41a06a4e0c0a4b43a62d2eba8f93aa13b42d Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Sun, 13 Nov 2022 13:03:52 +0100 Subject: [PATCH 07/11] changelog Signed-off-by: Tomas Herfert --- changelogs/fragments/128-api.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/128-api.yml diff --git a/changelogs/fragments/128-api.yml b/changelogs/fragments/128-api.yml new file mode 100644 index 00000000..8860e9ae --- /dev/null +++ b/changelogs/fragments/128-api.yml @@ -0,0 +1,3 @@ +minor_changes: + - api_modify, api_info - support for API fields that can be disabled and have default value at the same time, support API path ``interface gre`` + (https://github.com/ansible-collections/community.routeros/pull/128). From 4f20cea709f5b8c34688a8b155e5783954881c78 Mon Sep 17 00:00:00 2001 From: Tomas Herfert <68421396+therfert@users.noreply.github.com> Date: Sun, 13 Nov 2022 18:08:01 +0100 Subject: [PATCH 08/11] Update per suggestion Co-authored-by: Felix Fontein --- plugins/module_utils/_api_data.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index 61aa9ce7..b7b78a2f 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -49,8 +49,7 @@ class KeyInfo(object): def __init__(self, _dummy=None, can_disable=False, remove_value=None, absent_value=None, default=None, required=False, automatically_computed_from=None): if _dummy is not None: raise ValueError('KeyInfo() does not have positional arguments') - sm = sum([required, default is not None, automatically_computed_from is not None, can_disable]) - if sm > 1 and sm > sum([default is not None, can_disable]): + if sum([required, default is not None or can_disable, automatically_computed_from is not None]) > 1: raise ValueError( 'required, default, automatically_computed_from, and can_disable are mutually exclusive ' + 'besides default and can_disable which can be set together') From 014b09a250c0fc6b72eaf2f4c1489b347a60f553 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Wed, 16 Nov 2022 21:31:21 +0100 Subject: [PATCH 09/11] api path support: interface eoip Signed-off-by: Tomas Herfert --- changelogs/fragments/128-api.yml | 2 +- plugins/module_utils/_api_data.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/changelogs/fragments/128-api.yml b/changelogs/fragments/128-api.yml index 8860e9ae..6bcc4b7e 100644 --- a/changelogs/fragments/128-api.yml +++ b/changelogs/fragments/128-api.yml @@ -1,3 +1,3 @@ minor_changes: - - api_modify, api_info - support for API fields that can be disabled and have default value at the same time, support API path ``interface gre`` + - api_modify, api_info - support for API fields that can be disabled and have default value at the same time, support API paths ``interface gre``, ``interface eoip`` (https://github.com/ansible-collections/community.routeros/pull/128). diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index b7b78a2f..8aa31693 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -108,6 +108,31 @@ def join_path(path): 'vlan-filtering': KeyInfo(default=False), }, ), + ('interface', 'eoip'): APIData( + fully_understood=True, + primary_keys=('name',), + fields={ + 'allow-fast-path': KeyInfo(default=True), + 'arp': KeyInfo(default='enabled'), + 'arp-timeout': KeyInfo(default='auto'), + 'clamp-tcp-mss': KeyInfo(default=True), + 'comment': KeyInfo(can_disable=True, remove_value=''), + 'disabled': KeyInfo(default=False), + 'dont-fragment': KeyInfo(default=False), + 'dscp': KeyInfo(default='inherit'), + 'ipsec-secret': KeyInfo(can_disable=True), + 'keepalive': KeyInfo(default='10s,10', can_disable=True), + 'local-address': KeyInfo(default='0.0.0.0'), + 'loop-protect': KeyInfo(default='default'), + 'loop-protect-disable-time': KeyInfo(default='5m'), + 'loop-protect-send-interval': KeyInfo(default='5s'), + 'mac-address': KeyInfo(), + 'mtu': KeyInfo(default='auto'), + 'name': KeyInfo(), + 'remote-address': KeyInfo(required=True), + 'tunnel-id': KeyInfo(required=True), + }, + ), ('interface', 'ethernet'): APIData( fixed_entries=True, fully_understood=True, From 7c40459901ca728d376b849466f3584630618f04 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Wed, 16 Nov 2022 21:33:31 +0100 Subject: [PATCH 10/11] docs Signed-off-by: Tomas Herfert --- plugins/modules/api_info.py | 1 + plugins/modules/api_modify.py | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/modules/api_info.py b/plugins/modules/api_info.py index a46ffed3..f070c121 100644 --- a/plugins/modules/api_info.py +++ b/plugins/modules/api_info.py @@ -57,6 +57,7 @@ - interface bridge settings - interface bridge vlan - interface detect-internet + - interface eoip - interface ethernet - interface ethernet switch - interface ethernet switch port diff --git a/plugins/modules/api_modify.py b/plugins/modules/api_modify.py index 528f433a..74320c45 100644 --- a/plugins/modules/api_modify.py +++ b/plugins/modules/api_modify.py @@ -62,6 +62,7 @@ - interface bridge settings - interface bridge vlan - interface detect-internet + - interface eoip - interface ethernet - interface ethernet switch - interface ethernet switch port From 41bbf4444f350288b62c19ea6c06ebf628248043 Mon Sep 17 00:00:00 2001 From: Tomas Herfert Date: Thu, 17 Nov 2022 08:36:45 +0100 Subject: [PATCH 11/11] apply suggestion from code review Signed-off-by: Tomas Herfert --- tests/unit/plugins/modules/fake_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/plugins/modules/fake_api.py b/tests/unit/plugins/modules/fake_api.py index b17c088d..f50c996c 100644 --- a/tests/unit/plugins/modules/fake_api.py +++ b/tests/unit/plugins/modules/fake_api.py @@ -120,7 +120,7 @@ def str_return(self): def _normalize_entry(entry, path_info): for key, data in path_info.fields.items(): - if key not in entry and data.default is not None: + if key not in entry and data.default is not None and not data.can_disable: entry[key] = data.default if data.can_disable: if key in entry and entry[key] in (None, data.remove_value):