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 6bfa6e40f4)

Co-authored-by: David Hummel <6109326+hummeltech@users.noreply.github.com>

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).

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)
 

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):
     """