Skip to content

Conversation

@edlitmus
Copy link

@edlitmus edlitmus commented Dec 4, 2024

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"

@dmurphy18
Copy link
Contributor

@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.

@edlitmus
Copy link
Author

@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.

@dmurphy18
Copy link
Contributor

dmurphy18 commented Dec 13, 2024

@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

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.

@edlitmus
Copy link
Author

edlitmus commented Jan 6, 2025

@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.

@ed-silva-eb
Copy link
Contributor

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.

@scriptautomate-bc scriptautomate-bc removed their assignment Jun 3, 2025
@twangboy
Copy link
Contributor

twangboy commented Sep 3, 2025

Would you mind rebasing and fixing the conflicts?

@ed-silva-eb
Copy link
Contributor

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.

@twangboy twangboy enabled auto-merge September 3, 2025 20:47
@twangboy twangboy merged commit 5ce98f1 into saltstack:develop Sep 3, 2025
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants