diff --git a/changelogs/fragments/154-ip-dhcp-client-dhcp-options.yml b/changelogs/fragments/154-ip-dhcp-client-dhcp-options.yml new file mode 100644 index 00000000..016eb155 --- /dev/null +++ b/changelogs/fragments/154-ip-dhcp-client-dhcp-options.yml @@ -0,0 +1,3 @@ +bugfixes: + - "api_info, api_modify - fix default and remove behavior for ``dhcp-options`` in path ``ip dhcp-client`` (https://github.com/ansible-collections/community.routeros/issues/148, https://github.com/ansible-collections/community.routeros/pull/154)." + - "api_modify - fix handling of disabled keys on creation (https://github.com/ansible-collections/community.routeros/pull/154)." diff --git a/plugins/module_utils/_api_data.py b/plugins/module_utils/_api_data.py index 8d785886..0d3e82f7 100644 --- a/plugins/module_utils/_api_data.py +++ b/plugins/module_utils/_api_data.py @@ -1314,7 +1314,7 @@ def join_path(path): 'add-default-route': KeyInfo(default=True), 'comment': KeyInfo(can_disable=True, remove_value=''), 'default-route-distance': KeyInfo(default=1), - 'dhcp-options': KeyInfo(default='hostname,clientid'), + 'dhcp-options': KeyInfo(default='hostname,clientid', can_disable=True, remove_value=''), 'disabled': KeyInfo(default=False), 'interface': KeyInfo(), 'script': KeyInfo(can_disable=True), diff --git a/plugins/modules/api_modify.py b/plugins/modules/api_modify.py index 029fcd03..0bf2c719 100644 --- a/plugins/modules/api_modify.py +++ b/plugins/modules/api_modify.py @@ -542,6 +542,22 @@ def get_api_data(api_path, path_info): return entries +def prepare_for_add(entry, path_info): + new_entry = {} + for k, v in entry.items(): + if k.startswith('!'): + real_k = k[1:] + remove_value = path_info.fields[real_k].remove_value + if remove_value is not None: + k = real_k + v = remove_value + else: + if v is None: + v = path_info.fields[k].remove_value + new_entry[k] = v + return new_entry + + def sync_list(module, api, path, path_info): handle_absent_entries = module.params['handle_absent_entries'] handle_entries_content = module.params['handle_entries_content'] @@ -661,7 +677,7 @@ def match(current_entry): ) for entry in create_list: try: - entry['.id'] = api_path.add(**entry) + entry['.id'] = api_path.add(**prepare_for_add(entry, path_info)) except (LibRouterosError, UnicodeEncodeError) as e: module.fail_json( msg='Error while creating entry: {error}'.format( @@ -850,7 +866,7 @@ def sync_with_primary_keys(module, api, path, path_info): ) for entry in create_list: try: - entry['.id'] = api_path.add(**entry) + entry['.id'] = api_path.add(**prepare_for_add(entry, path_info)) # Store ID for primary keys pks = tuple(entry[primary_key] for primary_key in primary_keys) id_by_key[pks] = entry['.id'] diff --git a/tests/unit/plugins/modules/fake_api.py b/tests/unit/plugins/modules/fake_api.py index 37fae3b3..a5ddb318 100644 --- a/tests/unit/plugins/modules/fake_api.py +++ b/tests/unit/plugins/modules/fake_api.py @@ -118,9 +118,9 @@ def str_return(self): return repr(self.args) -def _normalize_entry(entry, path_info): +def _normalize_entry(entry, path_info, on_create=False): for key, data in path_info.fields.items(): - if key not in entry and data.default is not None and not data.can_disable: + if key not in entry and data.default is not None and (not data.can_disable or on_create): entry[key] = data.default if data.can_disable: if key in entry and entry[key] in (None, data.remove_value): @@ -188,7 +188,7 @@ def add(self, **kwargs): '.id': id, } entry.update(kwargs) - _normalize_entry(entry, self._path_info) + _normalize_entry(entry, self._path_info, on_create=True) self._values.append(entry) return id diff --git a/tests/unit/plugins/modules/test_api_modify.py b/tests/unit/plugins/modules/test_api_modify.py index d70cf1f5..78979733 100644 --- a/tests/unit/plugins/modules/test_api_modify.py +++ b/tests/unit/plugins/modules/test_api_modify.py @@ -89,7 +89,48 @@ START_IP_ADDRESS_OLD_DATA = massage_expected_result_data(START_IP_ADDRESS, ('ip', 'address')) -START_IP_DHCP_SEVER_LEASE = [ +START_IP_DHCP_CLIENT = [ + { + "!comment": None, + "!script": None, + ".id": "*1", + "add-default-route": True, + "default-route-distance": 1, + "dhcp-options": "hostname,clientid", + "disabled": False, + "interface": "ether1", + "use-peer-dns": True, + "use-peer-ntp": True, + }, + { + "!comment": None, + "!dhcp-options": None, + "!script": None, + ".id": "*2", + "add-default-route": True, + "default-route-distance": 1, + "disabled": False, + "interface": "ether2", + "use-peer-dns": True, + "use-peer-ntp": True, + }, + { + "!comment": None, + "!script": None, + ".id": "*3", + "add-default-route": True, + "default-route-distance": 1, + "dhcp-options": "hostname", + "disabled": False, + "interface": "ether3", + "use-peer-dns": True, + "use-peer-ntp": True, + }, +] + +START_IP_DHCP_CLIENT_OLD_DATA = massage_expected_result_data(START_IP_DHCP_CLIENT, ('ip', 'dhcp-client')) + +START_IP_DHCP_SERVER_LEASE = [ { '.id': '*1', 'address': '192.168.88.2', @@ -155,7 +196,7 @@ }, ] -START_IP_DHCP_SEVER_LEASE_OLD_DATA = massage_expected_result_data(START_IP_DHCP_SEVER_LEASE, ('ip', 'dhcp-server', 'lease')) +START_IP_DHCP_SERVER_LEASE_OLD_DATA = massage_expected_result_data(START_IP_DHCP_SERVER_LEASE, ('ip', 'dhcp-server', 'lease')) START_INTERFACE_LIST = [ { @@ -1729,7 +1770,7 @@ def test_sync_primary_key_cru_reorder_check(self): ]) @patch('ansible_collections.community.routeros.plugins.modules.api_modify.compose_api_path', - new=create_fake_path(('ip', 'dhcp-server', 'lease'), START_IP_DHCP_SEVER_LEASE, read_only=True)) + new=create_fake_path(('ip', 'dhcp-server', 'lease'), START_IP_DHCP_SERVER_LEASE, read_only=True)) def test_absent_value(self): with self.assertRaises(AnsibleExitJson) as exc: args = self.config_module_args.copy() @@ -1769,8 +1810,103 @@ def test_absent_value(self): result = exc.exception.args[0] self.assertEqual(result['changed'], False) - self.assertEqual(result['old_data'], START_IP_DHCP_SEVER_LEASE_OLD_DATA) - self.assertEqual(result['new_data'], START_IP_DHCP_SEVER_LEASE_OLD_DATA) + self.assertEqual(result['old_data'], START_IP_DHCP_SERVER_LEASE_OLD_DATA) + self.assertEqual(result['new_data'], START_IP_DHCP_SERVER_LEASE_OLD_DATA) + + @patch('ansible_collections.community.routeros.plugins.modules.api_modify.compose_api_path', + new=create_fake_path(('ip', 'dhcp-client'), START_IP_DHCP_CLIENT, read_only=True)) + def test_default_remove_combination_idempotent(self): + with self.assertRaises(AnsibleExitJson) as exc: + args = self.config_module_args.copy() + args.update({ + 'path': 'ip dhcp-client', + 'data': [ + { + 'interface': 'ether1', + }, + { + 'interface': 'ether2', + 'dhcp-options': None, + }, + { + 'interface': 'ether3', + 'dhcp-options': 'hostname', + }, + ], + 'handle_absent_entries': 'remove', + 'handle_entries_content': 'remove', + 'ensure_order': True, + }) + set_module_args(args) + self.module.main() + + result = exc.exception.args[0] + self.assertEqual(result['changed'], False) + self.assertEqual(result['old_data'], START_IP_DHCP_CLIENT_OLD_DATA) + self.assertEqual(result['new_data'], START_IP_DHCP_CLIENT_OLD_DATA) + + @patch('ansible_collections.community.routeros.plugins.modules.api_modify.compose_api_path', + new=create_fake_path(('ip', 'dhcp-client'), [])) + def test_default_remove_combination_create(self): + with self.assertRaises(AnsibleExitJson) as exc: + args = self.config_module_args.copy() + args.update({ + 'path': 'ip dhcp-client', + 'data': [ + { + 'interface': 'ether1', + }, + { + 'interface': 'ether2', + 'dhcp-options': None, + }, + { + 'interface': 'ether3', + 'dhcp-options': 'hostname', + }, + ], + 'handle_absent_entries': 'remove', + 'handle_entries_content': 'remove', + 'ensure_order': True, + }) + set_module_args(args) + self.module.main() + + result = exc.exception.args[0] + self.assertEqual(result['changed'], True) + self.assertEqual(result['old_data'], []) + self.assertEqual(result['new_data'], [ + { + ".id": "*NEW1", + "add-default-route": True, + "default-route-distance": 1, + "dhcp-options": "hostname,clientid", + "disabled": False, + "interface": "ether1", + "use-peer-dns": True, + "use-peer-ntp": True, + }, + { + # "!dhcp-options": None, + ".id": "*NEW2", + "add-default-route": True, + "default-route-distance": 1, + "disabled": False, + "interface": "ether2", + "use-peer-dns": True, + "use-peer-ntp": True, + }, + { + ".id": "*NEW3", + "add-default-route": True, + "default-route-distance": 1, + "dhcp-options": "hostname", + "disabled": False, + "interface": "ether3", + "use-peer-dns": True, + "use-peer-ntp": True, + }, + ]) @patch('ansible_collections.community.routeros.plugins.modules.api_modify.compose_api_path', new=create_fake_path(('interface', 'list'), START_INTERFACE_LIST, read_only=True))