Skip to content

(GH-585/CONT-998) Fix for safe_directory logic #605

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

david22swan
Copy link
Member

@david22swan david22swan commented May 19, 2023

Due to mistake in the logic unsafe directory was previously removed on every other run. The logic previously checked whether the unsafe directory needed to be added, removing it if this was false without taking into account when it was already set but we wanted it to be left in place.

In addition: safe directory is now set at a system wide level rather than at a global user level in order to ensure it is as effective as possible.

@david22swan david22swan requested a review from a team as a code owner May 19, 2023 09:23
Due to  mistake in the logic unsafe directory was previously removed on every other run.
The logic previously checked whether the unsafe directory needed to be added, removing it if this was false without taking into account when it was already set but we wanted it to be left in place.
@david22swan david22swan force-pushed the GH-585--CONT-998/main/unsafe_directoryfix branch from d3b519a to dcf9ac6 Compare May 19, 2023 09:25
@pmcmaw
Copy link

pmcmaw commented May 19, 2023

Can we get some sort of test on this logic please? 😄

@david22swan
Copy link
Member Author

There already is one that should have caught this. Not sure why it didn't

@david22swan david22swan force-pushed the GH-585--CONT-998/main/unsafe_directoryfix branch 4 times, most recently from e02932a to 6db82af Compare May 19, 2023 13:11
@david22swan david22swan force-pushed the GH-585--CONT-998/main/unsafe_directoryfix branch from 6db82af to 04d5e68 Compare May 19, 2023 13:25
…stem level

Safe directory is now set at a system wide level rather than at a global user level in order to ensure it is as effective as possible.
@david22swan david22swan force-pushed the GH-585--CONT-998/main/unsafe_directoryfix branch from 7da43e5 to 64586c3 Compare May 19, 2023 15:23
@david22swan
Copy link
Member Author

Test added/updated

Copy link

@LukasAud LukasAud left a comment

Choose a reason for hiding this comment

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

LGTM

@LukasAud LukasAud merged commit f962d15 into puppetlabs:main May 19, 2023
@david22swan david22swan deleted the GH-585--CONT-998/main/unsafe_directoryfix branch May 19, 2023 17:04
@hdeadman
Copy link

hdeadman commented Aug 30, 2023

Doing this at system level is a problem when running puppet as non-root, which I am doing in a container. Non-root users can't write to /etc/gitconfig. I will see if I can create that file in the containers /etc folder to be writable. - Update: I had to make /etc/ directory in the container writable by my non-root user, making the gitconfig file writable wasn't enough.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants