From 67640e54318aa2091781d7a6ad98dc21d7a3d576 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Sat, 7 Aug 2021 15:39:34 +0200 Subject: [PATCH] nmcli: Fix change detection for Wi-Fi security options (#3136) (#3166) * Fixed `wifi_sec` option changes are not detected Also updated `docs` URL and formatting to match that of the `wifi` option * Removed extraneous `appends` to `cmd` in `connection_update` These really should have only been added to `connection_options` whose return values get `extended` onto `cmd` (cherry picked from commit 6bfa6e40f4c421e3781ccaf80743424c5b60ec0a) Co-authored-by: David Hummel <6109326+hummeltech@users.noreply.github.com> --- ...i-sec-change-detection-to-nmcli-module.yml | 3 ++ plugins/modules/net_tools/nmcli.py | 32 +++++------- .../plugins/modules/net_tools/test_nmcli.py | 51 +++++++++++++++++++ 3 files changed, 66 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/3136-add-wifi-sec-change-detection-to-nmcli-module.yml diff --git a/changelogs/fragments/3136-add-wifi-sec-change-detection-to-nmcli-module.yml b/changelogs/fragments/3136-add-wifi-sec-change-detection-to-nmcli-module.yml new file mode 100644 index 0000000000..6cc5e7630d --- /dev/null +++ b/changelogs/fragments/3136-add-wifi-sec-change-detection-to-nmcli-module.yml @@ -0,0 +1,3 @@ +minor_changes: + - nmcli - add ``wifi-sec`` option change detection to support managing secure Wi-Fi connections + (https://github.com/ansible-collections/community.general/pull/3136). diff --git a/plugins/modules/net_tools/nmcli.py b/plugins/modules/net_tools/nmcli.py index bbc1b4770f..92d1e65ef7 100644 --- a/plugins/modules/net_tools/nmcli.py +++ b/plugins/modules/net_tools/nmcli.py @@ -332,10 +332,10 @@ options: version_added: 2.0.0 wifi_sec: description: - - 'The security configuration of the Wifi connection. The valid attributes are listed on:' - - 'U(https://developer.gnome.org/NetworkManager/stable/settings-802-11-wireless-security.html)' - - 'For instance to use common WPA-PSK auth with a password:' - - '- C({key-mgmt: wpa-psk, psk: my_password})' + - 'The security configuration of the WiFi connection. The valid attributes are listed on: + U(https://networkmanager.dev/docs/api/latest/settings-802-11-wireless-security.html).' + - 'For instance to use common WPA-PSK auth with a password: + C({key-mgmt: wpa-psk, psk: my_password}).' type: dict version_added: 3.0.0 ssid: @@ -345,9 +345,9 @@ options: version_added: 3.0.0 wifi: description: - - 'The configuration of the Wifi connection. The valid attributes are listed on: + - 'The configuration of the WiFi connection. The valid attributes are listed on: U(https://networkmanager.dev/docs/api/latest/settings-802-11-wireless.html).' - - 'For instance to create a hidden AP mode Wifi connection: + - 'For instance to create a hidden AP mode WiFi connection: C({hidden: true, mode: ap}).' type: dict version_added: 3.5.0 @@ -915,6 +915,11 @@ class Nmcli(object): options.update({ '802-11-wireless.%s' % name: value }) + if self.wifi_sec: + for name, value in self.wifi_sec.items(): + options.update({ + '802-11-wireless-security.%s' % name: value + }) # Convert settings values based on the situation. for setting, value in options.items(): setting_type = self.settings_type(setting) @@ -1065,19 +1070,6 @@ class Nmcli(object): else: ifname = self.ifname - if self.type == "wifi": - cmd.append('ssid') - cmd.append(self.ssid) - if self.wifi: - for name, value in self.wifi.items(): - # Disallow setting 'ssid' via 'wifi.ssid' - if name == 'ssid': - continue - cmd += ['802-11-wireless.%s' % name, value] - if self.wifi_sec: - for name, value in self.wifi_sec.items(): - cmd += ['wifi-sec.%s' % name, value] - options = { 'connection.interface-name': ifname, } @@ -1116,7 +1108,7 @@ class Nmcli(object): return self.connection_update('modify') def show_connection(self): - cmd = [self.nmcli_bin, 'con', 'show', self.conn_name] + cmd = [self.nmcli_bin, '--show-secrets', 'con', 'show', self.conn_name] (rc, out, err) = self.execute_command(cmd) diff --git a/tests/unit/plugins/modules/net_tools/test_nmcli.py b/tests/unit/plugins/modules/net_tools/test_nmcli.py index b2307f245a..c1b3e93ed4 100644 --- a/tests/unit/plugins/modules/net_tools/test_nmcli.py +++ b/tests/unit/plugins/modules/net_tools/test_nmcli.py @@ -491,6 +491,22 @@ TESTCASE_WIRELESS = [ } ] +TESTCASE_SECURE_WIRELESS = [ + { + 'type': 'wifi', + 'conn_name': 'non_existent_nw_device', + 'ifname': 'wireless_non_existant', + 'ip4': '10.10.10.10/24', + 'ssid': 'Brittany', + 'wifi_sec': { + 'key-mgmt': 'wpa-psk', + 'psk': 'VERY_SECURE_PASSWORD', + }, + 'state': 'present', + '_ansible_check_mode': False, + } +] + TESTCASE_DUMMY_STATIC = [ { 'type': 'dummy', @@ -1630,6 +1646,41 @@ def test_create_wireless(mocked_generic_connection_create, capfd): assert results['changed'] +@pytest.mark.parametrize('patch_ansible_module', TESTCASE_SECURE_WIRELESS, indirect=['patch_ansible_module']) +def test_create_secure_wireless(mocked_generic_connection_create, capfd): + """ + Test : Create secure wireless connection + """ + + with pytest.raises(SystemExit): + nmcli.main() + + assert nmcli.Nmcli.execute_command.call_count == 1 + arg_list = nmcli.Nmcli.execute_command.call_args_list + add_args, add_kw = arg_list[0] + + assert add_args[0][0] == '/usr/bin/nmcli' + assert add_args[0][1] == 'con' + assert add_args[0][2] == 'add' + assert add_args[0][3] == 'type' + assert add_args[0][4] == 'wifi' + assert add_args[0][5] == 'con-name' + assert add_args[0][6] == 'non_existent_nw_device' + + add_args_text = list(map(to_text, add_args[0])) + for param in ['connection.interface-name', 'wireless_non_existant', + 'ipv4.addresses', '10.10.10.10/24', + '802-11-wireless.ssid', 'Brittany', + '802-11-wireless-security.key-mgmt', 'wpa-psk', + '802-11-wireless-security.psk', 'VERY_SECURE_PASSWORD']: + assert param in add_args_text + + out, err = capfd.readouterr() + results = json.loads(out) + assert not results.get('failed') + assert results['changed'] + + @pytest.mark.parametrize('patch_ansible_module', TESTCASE_DUMMY_STATIC, indirect=['patch_ansible_module']) def test_create_dummy_static(mocked_generic_connection_create, capfd): """