nmcli: Disallow Wi-Fi options not supported by nmcli #3141

hummeltech · 2021-08-04T03:43:30Z

SUMMARY

Since both the wifi & wifi_sec options do not have a static list of suboptions (it depends on nmcli's version), we should query nmcli to determine which options are available on the client. Currently, when applying changes containing invalid/unsupported options, only the first option will be shown, show all of them instead. Also added new option to allow the play to display a warning and continue when invalid/unsupported options are present, the default is to fail.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

net_tools/nmcli.py

ADDITIONAL INFORMATION

When specifying invalid/unsupported Wi-Fi options in the wifi and/or wifi_sec dict.

I.E.

ssid was specified via wifi.ssid:

type: wifi
ssid: SSID_SHOULD_BE_SPECIFIED_HERE
wifi:
  ssid: SSID_SHOULD_NOT_BE_SPECIFIED_HERE

Before:
- No error message is given, so the user is unaware of any potential issue/disparity.

After:

[WARNING]: Ignoring option 'wifi.ssid', it must be specified with option 'ssid'

Invalid option was specified via wifi:

type: wifi
ssid: SSID
wifi:
  INVALID_1: This is a bad option
  INVALID_2: This is a bad option
  INVALID_3: This is a bad option

Before (note only the first invalid/unsupported option is detected):

fatal: [router]: FAILED! => {"changed": false, "msg": "Error: invalid property 'INVALID_1': 'INVALID_1' not among [ssid, mode, band, channel, bssid, rate, tx-power, mac-address, cloned-mac-address, generate-mac-address-mask, mac-address-blacklist, mac-address-randomization, mtu, seen-bssids, hidden, powersave, wake-on-wlan, ap-isolation].\n", "name": "WLAN", "rc": 2}

After:

fatal: [router]: FAILED! => {"changed": false, "msg": "Invalid or unsupported option(s): \"wifi.INVALID_1\", \"wifi.INVALID_2\", \"wifi.INVALID_3\""}

Invalid option was specified via wifi with ignore_unsupported_suboptions enabled:

ignore_unsupported_suboptions: yes
type: wifi
ssid: SSID
wifi:
  INVALID_1: This is a bad option
  INVALID_2: This is a bad option
  INVALID_3: This is a bad option

Before (note only the first invalid/unsupported option is detected):

fatal: [router]: FAILED! => {"changed": false, "msg": "Error: invalid property 'INVALID_1': 'INVALID_1' not among [ssid, mode, band, channel, bssid, rate, tx-power, mac-address, cloned-mac-address, generate-mac-address-mask, mac-address-blacklist, mac-address-randomization, mtu, seen-bssids, hidden, powersave, wake-on-wlan, ap-isolation].\n", "name": "WLAN", "rc": 2}

After:

[WARNING]: Invalid or unsupported option(s): 'wifi.INVALID_1', 'wifi.INVALID_2', 'wifi.INVALID_3'

ansibullbot · 2021-08-04T03:47:20Z

cc @alcamie101
click here for bot help

felixfontein · 2021-08-08T10:51:15Z

Since this contains #3160, let's keep this WIP until that's merged and this PR has been rebased.

By querying nmcli directly

Co-authored-by: Felix Fontein <felix@fontein.de>

Ajpantuso

Just a few minor items to consider.

plugins/modules/net_tools/nmcli.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

Ajpantuso

LGTM, thanks!

patchback · 2021-08-20T19:45:34Z

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/8a62b79ef2e902116575f1fe266bbce5def6f9e8/pr-3141

Backported as #3249

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* nmcli: Disallow Wi-Fi options not supported by nmcli By querying nmcli directly * Added changelog fragment * Added tests * Simplify `get_available_options()` * Update changelogs/fragments/3141-disallow-options-unsupported-by-nmcli.yml Co-authored-by: Felix Fontein <felix@fontein.de> * Remove redundant `802-11-wireless` settings from test show outputs * Update `mocked_wireless_create(mocker)` * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Ajpantuso <ajpantuso@gmail.com> * Address comment re. creating function & use nmcli naming conventions I.E. `setting`.`property` = `value` ``` nmcli> help set set [<setting>.<prop> <value>] :: set property value This command sets property value. Example: nmcli> set con.id My connection ``` * Added `ignore_unsupported_suboptions` option & improved `wifi(_sec)` doc * Corrected pep8 issues ``` ERROR: Found 2 pep8 issue(s) which need to be resolved: ERROR: plugins/modules/net_tools/nmcli.py:342:161: E501: line too long (236 > 160 characters) ERROR: plugins/modules/net_tools/nmcli.py:359:161: E501: line too long (237 > 160 characters) ``` * Fixed remaining sanity check issues and added even more docs * No need to split Note * Update plugins/modules/net_tools/nmcli.py 3.5.0 has already been released. Co-authored-by: Felix Fontein <felix@fontein.de> * Followed uniformity guideline for format macros from Ansible's dev guide * Addressed comment #3141 (comment) * Documentation cleanup continuation * Replace `NM_SETTING_*`s having a description with their numeric value * Splitting up long paragraphs. Also removed `wifi`.`seen-bssids` as it "`is only meant for reading`" * Addressed remaining comments and clarified `wake-on-lan` note * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Finishing addressing documentation comments. * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Ajpantuso <ajpantuso@gmail.com> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Ajpantuso <ajpantuso@gmail.com> * Update nmcli.py * Added wifi-related `list` type options to `settings_type` method * Moved `edit_commands` `execution` logic into its own method * Move `unsupported_property` deletion into `main` function * Missing `.items()` * Resolved missing proper `nmcli conn edit` arguments * Resolve pylint issue `dangerous-default-value` Co-authored-by: Felix Fontein <felix@fontein.de> Co-authored-by: Ajpantuso <ajpantuso@gmail.com> Co-authored-by: David Hummel <dhummel@Fingerling> (cherry picked from commit 8a62b79)

felixfontein · 2021-08-20T19:45:49Z

@hummeltech thanks for implementing this!
@Ajpantuso thanks for reviewing!

* nmcli: Disallow Wi-Fi options not supported by nmcli By querying nmcli directly * Added changelog fragment * Added tests * Simplify `get_available_options()` * Update changelogs/fragments/3141-disallow-options-unsupported-by-nmcli.yml Co-authored-by: Felix Fontein <felix@fontein.de> * Remove redundant `802-11-wireless` settings from test show outputs * Update `mocked_wireless_create(mocker)` * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Ajpantuso <ajpantuso@gmail.com> * Address comment re. creating function & use nmcli naming conventions I.E. `setting`.`property` = `value` ``` nmcli> help set set [<setting>.<prop> <value>] :: set property value This command sets property value. Example: nmcli> set con.id My connection ``` * Added `ignore_unsupported_suboptions` option & improved `wifi(_sec)` doc * Corrected pep8 issues ``` ERROR: Found 2 pep8 issue(s) which need to be resolved: ERROR: plugins/modules/net_tools/nmcli.py:342:161: E501: line too long (236 > 160 characters) ERROR: plugins/modules/net_tools/nmcli.py:359:161: E501: line too long (237 > 160 characters) ``` * Fixed remaining sanity check issues and added even more docs * No need to split Note * Update plugins/modules/net_tools/nmcli.py 3.5.0 has already been released. Co-authored-by: Felix Fontein <felix@fontein.de> * Followed uniformity guideline for format macros from Ansible's dev guide * Addressed comment #3141 (comment) * Documentation cleanup continuation * Replace `NM_SETTING_*`s having a description with their numeric value * Splitting up long paragraphs. Also removed `wifi`.`seen-bssids` as it "`is only meant for reading`" * Addressed remaining comments and clarified `wake-on-lan` note * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Felix Fontein <felix@fontein.de> * Finishing addressing documentation comments. * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Ajpantuso <ajpantuso@gmail.com> * Update plugins/modules/net_tools/nmcli.py Co-authored-by: Ajpantuso <ajpantuso@gmail.com> * Update nmcli.py * Added wifi-related `list` type options to `settings_type` method * Moved `edit_commands` `execution` logic into its own method * Move `unsupported_property` deletion into `main` function * Missing `.items()` * Resolved missing proper `nmcli conn edit` arguments * Resolve pylint issue `dangerous-default-value` Co-authored-by: Felix Fontein <felix@fontein.de> Co-authored-by: Ajpantuso <ajpantuso@gmail.com> Co-authored-by: David Hummel <dhummel@Fingerling> (cherry picked from commit 8a62b79) Co-authored-by: David Hummel <6109326+hummeltech@users.noreply.github.com>

hummeltech marked this pull request as ready for review August 4, 2021 07:27

ansibullbot added feature This issue/PR relates to a feature request needs_maintainer labels Aug 4, 2021

hummeltech marked this pull request as draft August 6, 2021 01:40

hummeltech marked this pull request as ready for review August 7, 2021 16:11

felixfontein added backport-3 check-before-release PR will be looked at again shortly before release and merged if possible. labels Aug 8, 2021

ansibullbot removed the needs_triage label Aug 8, 2021

felixfontein changed the title ~~nmcli: Disallow Wi-Fi options not supported by nmcli~~ [WIP] nmcli: Disallow Wi-Fi options not supported by nmcli Aug 8, 2021

ansibullbot added the WIP Work in progress label Aug 8, 2021

hummeltech added 3 commits August 8, 2021 10:28

nmcli: Disallow Wi-Fi options not supported by nmcli

966790f

By querying nmcli directly

Added changelog fragment

d1f9e6a

Added tests

a74aabd

Update plugins/modules/net_tools/nmcli.py

bda5e3d

Co-authored-by: Felix Fontein <felix@fontein.de>

ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 17, 2021

Finishing addressing documentation comments.

df2016c

felixfontein approved these changes Aug 17, 2021

View reviewed changes

felixfontein requested a review from Ajpantuso August 17, 2021 07:51

ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Aug 17, 2021

Ajpantuso requested changes Aug 17, 2021

View reviewed changes

plugins/modules/net_tools/nmcli.py Outdated Show resolved Hide resolved

plugins/modules/net_tools/nmcli.py Outdated Show resolved Hide resolved

plugins/modules/net_tools/nmcli.py Outdated Show resolved Hide resolved

Update plugins/modules/net_tools/nmcli.py

9148e38

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 17, 2021

hummeltech and others added 8 commits August 17, 2021 07:55

Update plugins/modules/net_tools/nmcli.py

41b51a2

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

Update nmcli.py

13a51f3

Added wifi-related list type options to settings_type method

f575479

Moved edit_commands execution logic into its own method

b46eb00

Move unsupported_property deletion into main function

c169fe4

Missing .items()

94c546a

Resolved missing proper nmcli conn edit arguments

9c07790

Resolve pylint issue dangerous-default-value

c8658a2

Ajpantuso approved these changes Aug 20, 2021

View reviewed changes

felixfontein merged commit 8a62b79 into ansible-collections:main Aug 20, 2021

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Aug 20, 2021

patchback bot mentioned this pull request Aug 20, 2021

[PR #3141/8a62b79e backport][stable-3] nmcli: Disallow Wi-Fi options not supported by nmcli #3249

Merged

hummeltech deleted the nmcli-wifi-options-check branch August 21, 2021 03:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nmcli: Disallow Wi-Fi options not supported by nmcli #3141

nmcli: Disallow Wi-Fi options not supported by nmcli #3141

hummeltech commented Aug 4, 2021 •

edited

Loading

ansibullbot commented Aug 4, 2021

felixfontein commented Aug 8, 2021

Ajpantuso left a comment

Ajpantuso left a comment

patchback bot commented Aug 20, 2021 •

edited

Loading

felixfontein commented Aug 20, 2021

nmcli: Disallow Wi-Fi options not supported by nmcli #3141

nmcli: Disallow Wi-Fi options not supported by nmcli #3141

Conversation

hummeltech commented Aug 4, 2021 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

I.E.

ansibullbot commented Aug 4, 2021

felixfontein commented Aug 8, 2021

Ajpantuso left a comment

Choose a reason for hiding this comment

Ajpantuso left a comment

Choose a reason for hiding this comment

patchback bot commented Aug 20, 2021 • edited Loading

Backport to stable-3: 💚 backport PR created

felixfontein commented Aug 20, 2021

hummeltech commented Aug 4, 2021 •

edited

Loading

patchback bot commented Aug 20, 2021 •

edited

Loading