nmcli: avoid changed status for most cases with VPN connections (#5126)

* nmcli: avoid changed status for most cases with VPN connections

Follow-up #4746

* `nmcli connection show` includes vpn.service-type but not vpn-type.
  Switching to vpn.service-type removes unneeded diffs while keeping
  the same functionality, as vpn-type is an alias of vpn.service-type
  per nm-settings-nmcli(1).

  NetworkManager also adds `org.freedesktop.NetworkManager.` prefix for
  known VPN types [1]. The logic is non-trivial so I didn't implement it
  in this commit. If a user specifies `service-type: l2tp`, changed will
  be always be True:

    -    "vpn.service-type": "org.freedesktop.NetworkManager.l2tp"
    +    "vpn.service-type": "l2tp"

* The vpn.data field from `nmcli connection show` is sorted by keys and
  there are spaces around equal signs. I added codes for parsing such
  data.

Tests are also updated to match outputs of nmcli commands.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/1.38.4/src/libnm-core-impl/nm-vpn-plugin-info.c#L619

* Add changelog

* Some suggested changes

* Make space stripping more flexible - works for cases without equal
  signs.
* Keep vpn.data in a test case with no spaces

* nmcli: allow any string for vpn service-type

(cherry picked from commit 6ff594b)

changelogs/fragments/5126-nmcli-remove-diffs.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "nmcli - avoid changed status for most cases with VPN connections (https://github.com/ansible-collections/community.general/pull/5126)."

plugins/modules/net_tools/nmcli.py

@@ -923,7 +923,6 @@
                 description: This defines the service type of connection.
                 type: str
                 required: true
-                choices: [ pptp, l2tp ]
             gateway:
                 description: The gateway to connection. It can be an IP address (for example C(192.0.2.1))
                     or a FQDN address (for example C(vpn.example.com)).
@@ -949,7 +948,7 @@
             ipsec-enabled:
                 description:
                     - Enable or disable IPSec tunnel to L2TP host.
-                    - This option is need when C(service-type) is C(l2tp).
+                    - This option is need when C(service-type) is C(org.freedesktop.NetworkManager.l2tp).
                 type: bool
                 choices: [ yes, no ]
             ipsec-psk:
@@ -1350,7 +1349,7 @@
     conn_name: my-vpn-connection
     vpn:
         permissions: "{{ ansible_user }}"
-        service-type: l2tp
+        service-type: org.freedesktop.NetworkManager.l2tp
         gateway: vpn.example.com
         password-flags: 2
         user: brittany
@@ -1670,7 +1669,7 @@ def connection_options(self, detect_change=False):
                 for name, value in self.vpn.items():
                     if name == 'service-type':
                         options.update({
-                            'vpn-type': value,
+                            'vpn.service-type': value,
                         })
                     elif name == 'permissions':
                         options.update({
@@ -2100,8 +2099,8 @@ def _compare_conn_params(self, conn_info, options):
                 if key == self.mtu_setting and self.mtu is None:
                     self.mtu = 0
                 if key == 'vpn.data':
-                    current_value = list(map(str.strip, current_value.split(',')))
-                    value = list(map(str.strip, value.split(',')))
+                    current_value = sorted(re.sub(r'\s*=\s*', '=', part.strip(), count=1) for part in current_value.split(','))
+                    value = sorted(part.strip() for part in value.split(','))
             else:
                 # parameter does not exist
                 current_value = None

tests/unit/plugins/modules/net_tools/test_nmcli.py

@@ -1201,7 +1201,7 @@
         'conn_name': 'vpn_l2tp',
         'vpn': {
             'permissions': 'brittany',
-            'service-type': 'l2tp',
+            'service-type': 'org.freedesktop.NetworkManager.l2tp',
             'gateway': 'vpn.example.com',
             'password-flags': '2',
             'user': 'brittany',
@@ -1221,9 +1221,8 @@
 connection.permissions:                 brittany
 ipv4.method:                            auto
 ipv6.method:                            auto
-vpn-type:                               l2tp
 vpn.service-type:                       org.freedesktop.NetworkManager.l2tp
-vpn.data:                               gateway=vpn.example.com, password-flags=2, user=brittany, ipsec-enabled=true, ipsec-psk=QnJpdHRhbnkxMjM=
+vpn.data:                               gateway = vpn.example.com, ipsec-enabled = true, ipsec-psk = QnJpdHRhbnkxMjM=, password-flags = 2, user = brittany
 vpn.secrets:                            ipsec-psk = QnJpdHRhbnkxMjM=
 vpn.persistent:                         no
 vpn.timeout:                            0
@@ -1235,7 +1234,7 @@
         'conn_name': 'vpn_pptp',
         'vpn': {
             'permissions': 'brittany',
-            'service-type': 'pptp',
+            'service-type': 'org.freedesktop.NetworkManager.pptp',
             'gateway': 'vpn.example.com',
             'password-flags': '2',
             'user': 'brittany',
@@ -1253,9 +1252,8 @@
 connection.permissions:                 brittany
 ipv4.method:                            auto
 ipv6.method:                            auto
-vpn-type:                               pptp
 vpn.service-type:                       org.freedesktop.NetworkManager.pptp
-vpn.data:                               password-flags=2, gateway=vpn.example.com, user=brittany
+vpn.data:                               gateway=vpn.example.com, password-flags=2, user=brittany
 """
 
 
@@ -3630,7 +3628,7 @@ def test_create_vpn_l2tp(mocked_generic_connection_create, capfd):
 
     for param in ['connection.autoconnect', 'no',
                   'connection.permissions', 'brittany',
-                  'vpn.data', 'vpn-type', 'l2tp',
+                  'vpn.data', 'vpn.service-type', 'org.freedesktop.NetworkManager.l2tp',
                   ]:
         assert param in add_args_text
 
@@ -3670,7 +3668,7 @@ def test_create_vpn_pptp(mocked_generic_connection_create, capfd):
 
     for param in ['connection.autoconnect', 'no',
                   'connection.permissions', 'brittany',
-                  'vpn.data', 'vpn-type', 'pptp',
+                  'vpn.data', 'vpn.service-type', 'org.freedesktop.NetworkManager.pptp',
                   ]:
         assert param in add_args_text