(Fix #63636) terraform roster to use explicit priv value if defined in state

[ricekab]

What does this PR do?

Fixes terraform roster from overriding explicit priv attributes with default behaviour.

This PR does not include test cases yet as I'm not sure how to do this for Salt.

What issues does this PR fix or reference?

Fixes: #63636

Previous Behavior

Explicit priv values from terraform state was overriden with default values.

New Behavior

If an explicit value for priv is defined, the default value determination is skipped.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes

[welcome]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Community Wiki
• Salt’s Contributor Guide
• Join our Community Slack
• IRC on LiberaChat
• Salt Project YouTube channel
• Salt Project Twitch channel

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

[ricekab]

Could I get feedback on this? I'd like to work on this PR to get it into a mergeable state.

[cmcmarrow]

@ricekab thx for the PR. I would add a unit test here https://github.com/saltstack/salt/blob/master/tests/pytests/unit/roster/test_terraform.py

You would need to test _add_ssh_key(ret) and make sure that you get the right ret into ret data. So one test might have ret = {"priv": some temp file}. Another test would check the behavior if ret = {}.

[ricekab]

Thanks for the feedback! I'll look into implementing this in the coming week.

[ricekab]

Hi @cmcmarrow , ended up taking me a while before I could come back around to it - I've added a test case for it to the PR. I opted to use a separate tfstate file since the other ones were testing the correct usage of default values (which this would override).

Tested on my setup using a Python 3.9 venv:

$ python -m nox -e "test-3(coverage=False)" -- tests/pytests/unit/roster/test_terraform.py
nox > Running session test-3(coverage=False)
nox > Re-using existing virtual environment at .nox/test-3-coverage-false.
nox > Session test-3(coverage=False) was successful.
nox > Running session test-parametrized-3(crypto=None, transport='zeromq', coverage=False)
nox > Re-using existing virtual environment at .nox/test-parametrized-3-crypto-none-transport-zeromq-coverage-false.
nox > python -m pip install --progress-bar=off -U 'pip>=20.2.4,<21.2' 'setuptools!=50.*,!=51.*,!=52.*,<59'
nox > python -m pip install --progress-bar=off -r requirements/static/ci/py3.9/linux.txt
nox > python -m pytest --rootdir /home/localadmin/salt --log-file-level=debug --show-capture=no -ra -s -vv --showlocals --log-file=/home/localadmin/salt/artifacts/logs/runtests-20230807145051.163828.log --transport=zeromq tests/pytests/unit/roster/test_terraform.py
======================================== test session starts =========================================
platform linux -- Python 3.9.15, pytest-7.3.2, pluggy-1.0.0 -- /home/localadmin/salt/.nox/test-parametrized-3-crypto-none-transport-zeromq-coverage-false/bin/python
cachedir: .pytest_cache
max open files; soft: 3072; hard: 4096
rootdir: /home/localadmin/salt
configfile: pytest.ini
plugins: httpserver-1.0.8, shell-utilities-1.8.0, salt-factories-1.0.0rc25, skip-markers-1.4.1, timeout-2.1.0, flaky-3.7.0, system-statistics-1.0.2, helpers-namespace-2021.12.29, custom-exit-code-0.3.0, subtests-0.11.0, anyio-3.7.0
collected 7 items

tests/pytests/unit/roster/test_terraform.py::test_default_output PASSED
tests/pytests/unit/roster/test_terraform.py::test_default_matching PASSED
tests/pytests/unit/roster/test_terraform.py::test_defaults_new PASSED
tests/pytests/unit/roster/test_terraform.py::test_defaults_new_matching PASSED
tests/pytests/unit/roster/test_terraform.py::test_specified_priv PASSED
tests/pytests/unit/roster/test_terraform.py::test_correct_handler_called_old PASSED
tests/pytests/unit/roster/test_terraform.py::test_correct_handler_called_new PASSED

========================================= 7 passed in 1.04s ==========================================
nox > Session test-parametrized-3(crypto=None, transport='zeromq', coverage=False) was successful.
nox > Ran multiple sessions:
nox > * test-3(coverage=False): success
nox > * test-parametrized-3(crypto=None, transport='zeromq', coverage=False): success

[twangboy]

This module and its associated states and tests have been transitioned to community support and are no longer maintained by the Salt Core team. The code has been removed from the Salt code-base and can be found in this repository: https://github.com/salt-extensions/community-extensions-holding/

There is currently discussion and work being done on the salt-extensions Discord channel (https://discordapp.com/channels/1200072194781368340/1208165123240370197) to document and build the infrastructure for community-supported salt extensions. There is also a Salt-Extensions Working Group that takes place on the 1st and 3rd Thursday of every month to coordinate salt-extension efforts.