Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PR #5126/6ff594b5 backport][stable-5] nmcli: avoid changed status for most cases with VPN connections #5220

Conversation

patchback[bot]
Copy link

@patchback patchback bot commented Sep 3, 2022

This is a backport of PR #5126 as merged into main (6ff594b).

SUMMARY

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

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nmcli

ADDITIONAL INFORMATION

N/A

* 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)
@felixfontein felixfontein merged commit 1cddae2 into stable-5 Sep 3, 2022
@felixfontein felixfontein deleted the patchback/backports/stable-5/6ff594b524a9180a66a5d98b465f2de1d75086a8/pr-5126 branch September 3, 2022 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant