Skip to content
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

feat(osutil.Mkdir): add the chmod flag #423

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

IronCore864
Copy link
Contributor

@IronCore864 IronCore864 commented Jun 6, 2024

Add a new Chmod flag to MkdirOptions so that an explicit chmod command is executed on all directories created by osutil.Mkdir.

Closes #372.

Manual Tests

Comparison

Tests Before After
pebble mkdir ~/normal -m 777 umask set to 0022, result is 0755, not the same as what the user specifies because of umask umask set to 0022, result is 0777, not affected by umask
pebble mkdir -p ~/nested/folder -m 777 umask set to 0022, result is 0755 for both parents and children, not the same as what the user specifies because of umask umask set to 0022, result is 0777 for both parents and children, not affected by umask

Before

Permission is affected by umask settings:

# umask 0022
$ pebble mkdir ~/normal -m 777
$ ll normal/
total 8
drwxr-xr-x  2 ubuntu ubuntu 4096 Jun  6 02:30 ./
$ pebble mkdir -p ~/nested/folder -m 777
$ ll nested/
total 12
drwxr-xr-x  3 ubuntu ubuntu 4096 Jun  6 02:34 ./
drwxr-x--- 19 ubuntu ubuntu 4096 Jun  6 02:34 ../
drwxr-xr-x  2 ubuntu ubuntu 4096 Jun  6 02:34 folder/

After

Permission isn't affected by umask anymore:

# umask 0022
$ pebble mkdir ~/normal -m 777
$ ll normal/
total 8
drwxrwxrwx  2 ubuntu ubuntu 4096 Jun  6 02:31 ./
$ pebble mkdir -p ~/nested/folder -m 777
$ ll nested/
total 12
drwxrwxrwx  3 ubuntu ubuntu 4096 Jun  6 02:32 ./
drwxr-x--- 19 ubuntu ubuntu 4096 Jun  6 02:32 ../
drwxrwxrwx  2 ubuntu ubuntu 4096 Jun  6 02:32 folder/

Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

This looks good, thanks!

internals/osutil/mkdir_test.go Show resolved Hide resolved
@benhoyt
Copy link
Contributor

benhoyt commented Jun 6, 2024

I've tested this manually locally too, and it seems to work nicely.

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.

pebble make-dirs permission is masked by pebble daemon's umask
2 participants