ansible/community.general
1
0
Fork 0
mirror of https://github.com/ansible-collections/community.general.git synced 2024-09-14 20:13:21 +02:00
Code Releases Activity

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

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

Follow-up https://github.com/ansible-collections/community.general/pull/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
This commit is contained in:
Chih-Hsuan Yen 2022-09-03 18:02:03 +08:00 committed by GitHub
parent 7ffe6539c0
commit 6ff594b524
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
changelogs/fragments/5126-nmcli-remove-diffs.yml Normal file
View file

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

11
plugins/modules/net_tools/nmcli.py
View file

@ -923,7 +923,6 @@ options:
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 @@ options:
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 @@ EXAMPLES = r'''
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 @@ class Nmcli(object):
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 @@ class Nmcli(object):
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

14
tests/unit/plugins/modules/net_tools/test_nmcli.py
View file

@ -1201,7 +1201,7 @@ TESTCASE_VPN_L2TP = [
'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.autoconnect: no
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 @@ TESTCASE_VPN_PPTP = [
'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.autoconnect: no
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