-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
fix impermanence in combination with sysusers/userborn #223
base: master
Are you sure you want to change the base?
Conversation
39ac7c7
to
4b34e65
Compare
af08f1b
to
0bc0d03
Compare
Both remove user creation from the action phase to a systemd service. This is hopefully also easier to debug, since we now can look at a systemd service instead for log output.
d5f21cf
to
8495848
Compare
No idea how this ever worked, but for me all directories in /persistent never have the write user/group permissions when freshly created. This now also ensures that permissions will change if the configuration changes, which is a nice side effect.
f1d0251
to
32b1094
Compare
# synchronize perms between source and target | ||
chown --reference="$realSource" "$target" | ||
chmod --reference="$realSource" "$target" | ||
chown "$user:$group" "$realSource" "$target" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See commit message for reasoning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit skeptical of this, since it would apply mode changes to already existing directories, whether a mode is supplied for the directory or not. I'm not really sure what's not working for you, though. Are the initial directories created with the incorrect permissions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I guess this isn't critical for the main purpose of this PR, so maybe it could be moved to its own?
Can be tested with
In the vm do:
to see that permissions and owners are correct now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you're moving a lot of code from one scope to another, but it's intermixed with your actual changes. Please split out the move to its own commit, so that it's easier to review the changes.
# synchronize perms between source and target | ||
chown --reference="$realSource" "$target" | ||
chmod --reference="$realSource" "$target" | ||
chown "$user:$group" "$realSource" "$target" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit skeptical of this, since it would apply mode changes to already existing directories, whether a mode is supplied for the directory or not. I'm not really sure what's not working for you, though. Are the initial directories created with the incorrect permissions?
Both remove user creation from the action phase to a systemd service. This is hopefully also easier to debug, since we now can look at a systemd service instead for log output.