Skip to content

Extend the functionality of ssh_pre_flight with ssh_pre_flight_args #61715

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

Merged
merged 16 commits into from
Apr 18, 2022

Conversation

vzhestkov
Copy link
Contributor

What does this PR do?

What issues does this PR fix or reference?

Make it possible to pass extra parameters to salt-ssh pre flight script with ssh_pre_flight_args from the roster.

Previous Behavior

No way to pass any parameters to pre flight script.

New Behavior

Pre flight script can be executed with extra arguments specified in ssh_pre_flight_args

Merge requirements satisfied?

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

Commits signed with GPG?

Yes/No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@vzhestkov vzhestkov requested a review from a team as a code owner February 24, 2022 12:11
@vzhestkov vzhestkov requested review from Ch3LL and removed request for a team February 24, 2022 12:11
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

This will require a changelog and tests. One test I was hoping to add in addition to just ensuring the functionality works, was to ensure someone cannot pass shell injections when using ssh_pre_flight_args. For example piping another command or using ; to then run a different command.

@vzhestkov
Copy link
Contributor Author

@Ch3LL there was a change from @meaksh with the injection prevention fix and additionally different types of tests for it as well as changelog entry.

The only point left is the documentation for the new roster parameter. Could you please give a hint where the documentation regarding the roster file parameters shold be appended?

@vzhestkov vzhestkov requested a review from Ch3LL February 28, 2022 16:02
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 3, 2022

doc/topics/ssh/roster.rst

Ch3LL
Ch3LL previously approved these changes Mar 7, 2022
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 7, 2022

I just want to get another review on this one. Ping @dwoz can you review please?

@Ch3LL Ch3LL requested a review from dwoz March 7, 2022 20:45
@Ch3LL Ch3LL added the Phosphorus v3005.0 Release code name and version label Mar 7, 2022
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 8, 2022

There are some test failures on windows that are related to this fix

@Ch3LL Ch3LL merged commit 36afe59 into saltstack:master Apr 18, 2022
agraul pushed a commit to agraul/salt that referenced this pull request Jan 27, 2025
* Add salt-ssh support with venv-salt-minion

* Add some comments and drop the commented line

* Fix return in check_venv_hash_file

* Convert all script parameters to strings

* Reduce the size of minion response

Minion response contains SSH_PY_CODE wrapped to base64.
This fix reduces the size of the response in DEBUG logging

* Make VENV_HASH_FILE global

* Pass the context to roster modules

* Avoid race condition on loading roster modules

* Prevent simultaneous to salt-ssh minion

* Make ssh session grace time configurable

* Prevent possible segfault by GC

* Revert "Avoid race condition on loading roster modules"

This reverts commit 8ff822a162cc494d3528184aef983ad20e09f4e2.

* Prevent deadlocks with importlib on using LazyLoader

* Make logging on salt-ssh errors more informative

* Add comments about using salt.loader.LOAD_LOCK

* Fix test_loader test

* Prevent deadlocks on using logging

* Use collections.deque instead of list for salt-ssh

Suggested by @agraul

* Get proper exitstatus from salt.utils.vt.Terminal

to prevent empty event returns due to improperly detecting
the child process as failed

* Do not run pre flight script for raw_shell

BACKPORT-UPSTREAM=saltstack#61715
DOWNSTREAM-REF=openSUSE/salt#493
DOWNSTREAM-REF=openSUSE/salt#497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants