salt-bootstrap icon indicating copy to clipboard operation
salt-bootstrap copied to clipboard

Add support for specifying a directory other than /tmp for installation execution

Open edlitmus opened this issue 1 year ago • 5 comments

What does this PR do?

Adds an option to specify something other than /tmp for installation execution.

What issues does this PR fix or reference?

Systems that have been hardened to CIS-2 benchmarks should have /tmp and /var/tmp mounted with the noexec option. This causes installations to fail when trying to execute anything from /tmp.

New Behavior

Using the new option allows installations to proceed as expected. An example using the ubuntu user home directory:

bash /home/ubuntu/bootstrap-salt.sh -X -U -A localhost -T /home/ubuntu onedir "$SALT_VERSION"

edlitmus avatar Dec 04 '24 21:12 edlitmus

@edlitmus You need to refresh, released v2024.12.12 today, fix the lint errors by running the following in your work directory pre-commit install, this way you will catch lint errors, etc when you git commit.

And lastly, you need to add tests to ensure your changes to code are tested.

dmurphy18 avatar Dec 12 '24 20:12 dmurphy18

@edlitmus You need to refresh, released v2024.12.12 today, fix the lint errors by running the following in your work directory pre-commit install, this way you will catch lint errors, etc when you git commit.

Thanks, I refreshed and fixed the lint errors.

And lastly, you need to add tests to ensure your changes to code are tested.

Any guides on adding tests for this? I'm not sure where to start on that.

edlitmus avatar Dec 12 '24 21:12 edlitmus

@edlitmus Adding tests using pytest, similar to Salt itself, except don't have to use nox. Take a look at https://github.com/saltstack/salt-bootstrap/blob/develop/tests/integration/test_installation.py and also Salt test and pytest documentation

  • https://docs.saltproject.io/en/latest/topics/development/tests/index.html
  • https://docs.saltproject.io/en/latest/topics/tutorials/writing_tests.html
  • https://docs.pytest.org/en/stable/
  • https://www.tutorialspoint.com/pytest/index.htm

Lastly, if you add a feature to Linux, you cannot ignore Windows, needs to be added there too, that is, bootstrap-salt.ps1 needs updating too.

dmurphy18 avatar Dec 13 '24 17:12 dmurphy18

@dmurphy18 Sorry for the delay on this, I was taking time off, but I did some digging into the Windows side of this, and it doesn't look like the options for the Windows script are a match for the shell script. Also, I don't think it makes sense to add the same feature to the Windows script as I do not believe that noexec exists for NTFS, so it wouldn't be something that would be required for the installation on Windows systems.

As for the tests, I still need to look into how to add something to do validations.

edlitmus avatar Jan 06 '25 15:01 edlitmus

Sorry for the lack of additional tests but I didn't see how to add any that would be effective. FWIW we have been using this version in production since January.

ed-silva-eb avatar Apr 04 '25 13:04 ed-silva-eb

Would you mind rebasing and fixing the conflicts?

twangboy avatar Sep 03 '25 19:09 twangboy

Would you mind rebasing and fixing the conflicts?

I was able to merge cleanly info my fork from the upstream develop branch with no conflicts.

ed-silva-eb avatar Sep 03 '25 19:09 ed-silva-eb