Skip to content

T8205 Ensure the vyos_user module works when user properties are defined in…#445

Merged
dmbaturin merged 10 commits intovyos:mainfrom
gideon-kuijt-northwave:patch-1
Apr 7, 2026
Merged

T8205 Ensure the vyos_user module works when user properties are defined in…#445
dmbaturin merged 10 commits intovyos:mainfrom
gideon-kuijt-northwave:patch-1

Conversation

@gideon-kuijt-northwave
Copy link
Copy Markdown
Contributor

@gideon-kuijt-northwave gideon-kuijt-northwave commented Jan 16, 2026

Ensure the vyos_user module works when user properties are defined in aggregate. Previously the value variable is not filled when a property is configured as a property of a user. This gives a python error when the value variable is called.

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T8205

Related PR(s)

Component(s) name

vyos_user

Proposed changes

Ensuring that the value variable will always be defined in the get_param_value function regardless of whether it is created inside or outside the aggregate of the vyos_user module.

How to test

Version information:

$ ansible-playbook --version
ansible-playbook [core 2.20.1]
  config file = None
  configured module search path = ['$HOME/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/homebrew/Cellar/ansible/13.2.0_1/libexec/lib/python3.14/site-packages/ansible
  ansible collection location = $HOME/.ansible/collections:/usr/share/ansible/collections
  executable location = /opt/homebrew/bin/ansible-playbook
  python version = 3.14.2 (main, Dec  5 2025, 16:49:16) [Clang 17.0.0 (clang-1700.4.4.1)] (/opt/homebrew/Cellar/ansible/13.2.0_1/libexec/bin/python)
  jinja version = 3.1.6
  pyyaml version = 6.0.3 (with libyaml v0.2.5)

Ran a playbook with the following configuration:

- vyos_user:
    aggregate:
      - name: user1
        configured_password: "password"
      - name: user2
        configured_password: "password"

This results in the following error:
"msg": "Task failed: cannot access local variable 'value' where it is not associated with a value"

Test results

  • Sanity tests passed
    (had to create an ignore file for ansible 2.20 for action-plugin-docs)
$ ansible-test sanity                                         
Running sanity test "action-plugin-docs"
Running sanity test "ansible-doc"
Running sanity test "changelog"
WARNING: Skipping sanity test "compile" on Python 3.9 because it could not be found.
WARNING: Skipping sanity test "compile" on Python 3.10 because it could not be found.
Running sanity test "compile" on Python 3.11
Running sanity test "compile" on Python 3.12
Running sanity test "compile" on Python 3.13
Running sanity test "compile" on Python 3.14
Running sanity test "empty-init"
Running sanity test "ignores"
WARNING: Skipping sanity test "import" on Python 3.9 because it could not be found.
WARNING: Skipping sanity test "import" on Python 3.10 because it could not be found.
Running sanity test "import" on Python 3.11
Running sanity test "import" on Python 3.12
Running sanity test "import" on Python 3.13
Running sanity test "import" on Python 3.14
Running sanity test "line-endings"
Running sanity test "no-assert"
Running sanity test "no-get-exception"
Running sanity test "no-illegal-filenames"
Running sanity test "no-smart-quotes"
Running sanity test "pep8"
Running sanity test "pslint"
Running sanity test "pylint"
Running sanity test "replace-urlopen"
Running sanity test "runtime-metadata"
Running sanity test "shebang"
Running sanity test "shellcheck"
Running sanity test "symlinks"
Running sanity test "use-argspec-type-path"
Running sanity test "use-compat-six"
Running sanity test "validate-modules"
Running sanity test "yamllint"
WARNING: Reviewing previous 4 warning(s):
WARNING: Skipping sanity test "compile" on Python 3.9 because it could not be found.
WARNING: Skipping sanity test "compile" on Python 3.10 because it could not be found.
WARNING: Skipping sanity test "import" on Python 3.9 because it could not be found.
WARNING: Skipping sanity test "import" on Python 3.10 because it could not be found.
  • Unit tests passed
$ ansible-test units --venv
WARNING: Skipping unit tests on Python 3.11 because it is only supported by module/module_utils unit tests. No module/module_utils unit tests were selected.
WARNING: Skipping unit tests on Python 3.11 because it is only supported by module/module_utils unit tests. No module/module_utils unit tests were selected.
Installing requirements for Python 3.12 [venv]
Installing requirements for Python 3.13 [venv]
Installing requirements for Python 3.14 [venv]
Unit test controller with Python 3.12
============================= test session starts ==============================
platform darwin -- Python 3.12.12, pytest-9.0.2, pluggy-1.6.0
rootdir: $HOME/.ansible/collections/ansible_collections/vyos/vyos
configfile: ../../../../../../../var/folders/8m/jwqbrn6n6z7gnd66ml9jg7wc0000gn/T/ansible-test-shcyl9q6/ansible_test/_data/pytest/config/default.ini
plugins: mock-3.15.1, xdist-3.8.0
created: 14/14 workers
14 workers [374 items]

........................................................................ [ 19%]
........................................................................ [ 38%]
........................................................................ [ 57%]
........................................................................ [ 77%]
........................................................................ [ 96%]
..............                                                           [100%]
- generated xml file: $HOME/.ansible/collections/ansible_collections/vyos/vyos/tests/output/junit/python3.12-controller-units.xml -
============================= 374 passed in 3.64s ==============================
Unit test controller with Python 3.13
============================= test session starts ==============================
platform darwin -- Python 3.13.11, pytest-9.0.2, pluggy-1.6.0
rootdir: /$HOME/.ansible/collections/ansible_collections/vyos/vyos
configfile: ../../../../../../../var/folders/8m/jwqbrn6n6z7gnd66ml9jg7wc0000gn/T/ansible-test-shcyl9q6/ansible_test/_data/pytest/config/default.ini
plugins: mock-3.15.1, xdist-3.8.0
created: 14/14 workers
14 workers [374 items]

........................................................................ [ 19%]
........................................................................ [ 38%]
........................................................................ [ 57%]
........................................................................ [ 77%]
........................................................................ [ 96%]
..............                                                           [100%]
- generated xml file: $HOME/.ansible/collections/ansible_collections/vyos/vyos/tests/output/junit/python3.13-controller-units.xml -
============================= 374 passed in 3.54s ==============================
Unit test controller with Python 3.14
============================= test session starts ==============================
platform darwin -- Python 3.14.2, pytest-9.0.2, pluggy-1.6.0
rootdir: $HOME/.ansible/collections/ansible_collections/vyos/vyos
configfile: ../../../../../../../var/folders/8m/jwqbrn6n6z7gnd66ml9jg7wc0000gn/T/ansible-test-shcyl9q6/ansible_test/_data/pytest/config/default.ini
plugins: mock-3.15.1, xdist-3.8.0
created: 14/14 workers
14 workers [374 items]

........................................................................ [ 19%]
........................................................................ [ 38%]
........................................................................ [ 57%]
........................................................................ [ 77%]
........................................................................ [ 96%]
..............                                                           [100%]
- generated xml file: $HOME/.ansible/collections/ansible_collections/vyos/vyos/tests/output/junit/python3.14-controller-units.xml -
============================= 374 passed in 3.58s ==============================
WARNING: Reviewing previous 1 warning(s):
WARNING: Skipping unit tests on Python 3.11 because it is only supported by module/module_utils unit tests. No module/module_utils unit tests were selected.
WARNING: Reviewing previous 1 warning(s):
WARNING: Skipping unit tests on Python 3.11 because it is only supported by module/module_utils unit tests. No module/module_utils unit tests were selected.

Tested against VyOS versions:

  • 1.4.3

It seems that currently there are no unit tests for aggregate scenario's hence this error slipped through.

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the ansible sanity and unit tests
  • My commit headlines contain a valid Task id
  • [] My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added unit tests to cover my changes
  • I have added a file to changelogs/fragments to describe the changes

… aggregate

Ensure the vyos_user module works when user properties are defined in aggregate. Previously the value variable is not filled when a property is configured as a property of a user. This gives a python error when the value variable is called.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 16, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@omnom62
Copy link
Copy Markdown
Contributor

omnom62 commented Jan 20, 2026

@gideon-kuijt-northwave thanks for your contribution!
Can you please sign CLA? Just simply create a comment with wording "I have read the CLA Document and I hereby sign the CLA", then I should be able to review and kick-off tests.
Thanks again!

Copy link
Copy Markdown
Contributor

@omnom62 omnom62 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gideon-kuijt-northwave thanks!
is it possible to sign CIA, add changelog and possibly - unit and/or integration test to demo the fix?

@gideon-kuijt-northwave
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

vyosbot added a commit to vyos/vyos-cla-signatures that referenced this pull request Jan 22, 2026
@gideon-kuijt-northwave
Copy link
Copy Markdown
Contributor Author

@omnom62 I've signed the CLA, and added a changelog fragment. Also added a unit test for the aggregate function, this generates the same error as before my change.

@gaige
Copy link
Copy Markdown
Contributor

gaige commented Jan 25, 2026

I added a task for this: T8205. @gideon-kuijt-northwave please prepend to the title and internal references and then check the box for the task ID.

gaige
gaige previously requested changes Jan 25, 2026
Copy link
Copy Markdown
Contributor

@gaige gaige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go once we have the task information updated.

@gideon-kuijt-northwave gideon-kuijt-northwave changed the title Ensure the vyos_user module works when user properties are defined in… T8205: Ensure the vyos_user module works when user properties are defined in… Jan 26, 2026
@gideon-kuijt-northwave gideon-kuijt-northwave changed the title T8205: Ensure the vyos_user module works when user properties are defined in… T8205 Ensure the vyos_user module works when user properties are defined in… Jan 26, 2026
@gideon-kuijt-northwave
Copy link
Copy Markdown
Contributor Author

gideon-kuijt-northwave commented Jan 26, 2026

@gaige Thanks for the review. I've updated the title, change log file and checkboxes.

@gaige
Copy link
Copy Markdown
Contributor

gaige commented Jan 26, 2026

@gaige Thanks for the review. I've updated the title, change log file and checkboxes.

Sorry for the lack of clarity, I meant the ones in the PR, so you should be good as soon as Ansible Galaxy is back up and running and a re-test is forced as long as @omnom62 is good with the changes.

Copy link
Copy Markdown
Contributor

@omnom62 omnom62 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@omnom62 omnom62 requested a review from gaige February 28, 2026 20:21
Copy link
Copy Markdown
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no issues here as far as my knowledge allows, and I trust @omnom62's judgement.

@dmbaturin dmbaturin dismissed gaige’s stale review March 5, 2026 09:03

As I understand, this change request no longer applies

Comment thread changelogs/fragments/T8205_vyos_user_aggregate_fix.yml
…_fix.yml

Remove unintended whitespace in changelog fragment: T8205_vyos_user_aggregate_fix.yml
Copy link
Copy Markdown
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes adressed

@c-po c-po enabled auto-merge (squash) March 5, 2026 15:33
@c-po c-po disabled auto-merge April 2, 2026 14:21
@dmbaturin dmbaturin merged commit 16e6f21 into vyos:main Apr 7, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants