From 9d8bec14c09cc79448b925784b3e8347d78bb17e Mon Sep 17 00:00:00 2001 From: genofire Date: Thu, 6 Jul 2023 21:06:24 +0200 Subject: [PATCH] nmcli: fix empty list to replace / remove values (#6769) --- .../fragments/6769-nmcli-fix-empty-list.yml | 2 + plugins/modules/nmcli.py | 7 ++- tests/unit/plugins/modules/test_nmcli.py | 62 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/6769-nmcli-fix-empty-list.yml diff --git a/changelogs/fragments/6769-nmcli-fix-empty-list.yml b/changelogs/fragments/6769-nmcli-fix-empty-list.yml new file mode 100644 index 0000000000..0220178a11 --- /dev/null +++ b/changelogs/fragments/6769-nmcli-fix-empty-list.yml @@ -0,0 +1,2 @@ + bugfixes: + - nmcli - fix support for empty list (in compare and scrape) (https://github.com/ansible-collections/community.general/pull/6769). diff --git a/plugins/modules/nmcli.py b/plugins/modules/nmcli.py index c077c4cbf2..921fd1fc9c 100644 --- a/plugins/modules/nmcli.py +++ b/plugins/modules/nmcli.py @@ -2183,7 +2183,10 @@ class Nmcli(object): if key and len(pair) > 1: raw_value = pair[1].lstrip() if raw_value == '--': - conn_info[key] = None + if key_type == list: + conn_info[key] = [] + else: + conn_info[key] = None elif key == 'bond.options': # Aliases such as 'miimon', 'downdelay' are equivalent to the +bond.options 'option=value' syntax. opts = raw_value.split(',') @@ -2270,7 +2273,7 @@ class Nmcli(object): # We can't just do `if not value` because then if there's a value # of 0 specified as an integer it'll be interpreted as empty when # it actually isn't. - if value != 0 and not value: + if value not in (0, []) and not value: continue if key in conn_info: diff --git a/tests/unit/plugins/modules/test_nmcli.py b/tests/unit/plugins/modules/test_nmcli.py index ccb6ececcd..3231d4f331 100644 --- a/tests/unit/plugins/modules/test_nmcli.py +++ b/tests/unit/plugins/modules/test_nmcli.py @@ -262,6 +262,25 @@ ipv4.routes: { ip = 192.168.200.0/24, nh = 192.168.1. ipv4.route-metric: 10 """ +TESTCASE_ETHERNET_MOD_IPV4_INT_WITH_ROUTE_AND_METRIC_CLEAR = [ + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'routes4': [], + 'state': 'present', + '_ansible_check_mode': False, + '_ansible_diff': True, + }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'routes4_extended': [], + 'state': 'present', + '_ansible_check_mode': False, + '_ansible_diff': True, + }, +] + TESTCASE_ETHERNET_MOD_IPV6_INT_WITH_ROUTE_AND_METRIC = [ { 'type': 'ethernet', @@ -1672,6 +1691,17 @@ def mocked_ethernet_connection_with_ipv4_static_address_static_route_metric_modi )) +@pytest.fixture +def mocked_ethernet_connection_with_ipv4_static_address_static_route_metric_clear(mocker): + mocker_set(mocker, + connection_exists=True, + execute_return=None, + execute_side_effect=( + (0, TESTCASE_ETHERNET_MOD_IPV4_INT_WITH_ROUTE_AND_METRIC_SHOW_OUTPUT, ""), + (0, "", ""), + )) + + @pytest.fixture def mocked_ethernet_connection_with_ipv6_static_address_static_route_metric_modify(mocker): mocker_set(mocker, @@ -2992,6 +3022,38 @@ def test_ethernet_connection_static_ipv4_address_static_route_with_metric_modify assert not results.get('failed') +@pytest.mark.parametrize('patch_ansible_module', TESTCASE_ETHERNET_MOD_IPV4_INT_WITH_ROUTE_AND_METRIC_CLEAR, indirect=['patch_ansible_module']) +def test_ethernet_connection_static_ipv4_address_static_route_with_metric_clear( + mocked_ethernet_connection_with_ipv4_static_address_static_route_metric_clear, capfd): + """ + Test : Modify ethernet connection with static IPv4 address and static route + """ + with pytest.raises(SystemExit): + nmcli.main() + + arg_list = nmcli.Nmcli.execute_command.call_args_list + add_args, add_kw = arg_list[1] + + assert add_args[0][0] == '/usr/bin/nmcli' + assert add_args[0][1] == 'con' + assert add_args[0][2] == 'modify' + assert add_args[0][3] == 'non_existent_nw_device' + + add_args_text = list(map(to_text, add_args[0])) + + for param in ['ipv4.routes', '']: + assert param in add_args_text + + out, err = capfd.readouterr() + results = json.loads(out) + + assert 'ipv4.routes' in results['diff']['before'] + assert 'ipv4.routes' in results['diff']['after'] + + assert results.get('changed') is True + assert not results.get('failed') + + @pytest.mark.parametrize('patch_ansible_module', TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_ROUTE, indirect=['patch_ansible_module']) def test_ethernet_connection_static_ipv6_address_static_route_create(mocked_ethernet_connection_with_ipv6_static_address_static_route_create, capfd): """