initContainer: Only remove password when needed #533
Conversation
When a newly created container has already the current user created
inside of it then the user initialization section of command
'init-container' is not triggered. The user init section currently takes
care of:
1) symlinking /home to /var/home if called with option --home-link
2a) creating user set by --user, --uid, --home and --shell
2b) adds the user to the sudoers group (either sudo or wheel)
3) removes password of user and of root
This commit does the following:
- moves 1) before the user init section and makes calling it
conditional depending on /home being a symlink or not
- moves 3) after the user init section and depending on the output of
'passwd --status' (that is expected to be NP; more in 'man
passwd(1)') calls the said sections
containers#533
HarryMichal
force-pushed
the
restructure-user-init
branch
from
a335eff to
f14771a
Compare
HarryMichal
added
3. Bugfix
|
Build failed.
|
There was a problem hiding this comment.
Oops! Sorry, I failed to notice this PR. :(
Is it still relevant after #547 ?
I'll have to adjust it but I'd say "yes". This PR does not touch |
HarryMichal
force-pushed
the
restructure-user-init
branch
from
f14771a to
a3e575b
Compare
HarryMichal
changed the title
|
In the end, I only took one part from the original PR: making password removal conditional to prevent log cluttering. What do you think about it, @debarshiray? |
|
Build failed.
|
When looking into logs of toolboxes using 'podman logs', one can notice that every "startup" log mentions removing the password for the user and root. These lines could be considered as bloat because what they're saying is actually not happening because appart from the first run the password are usually already gone. This makes the removal of password for the user and root conditional based on the output of 'passwd --status <username>' where value "NP" (meaning "no password") will be considered the only value that does not trigger the removal. Any other value will trigger the removal.
HarryMichal
force-pushed
the
restructure-user-init
branch
from
a3e575b to
c8fbaa8
Compare
|
Build failed.
|
There was a problem hiding this comment.
Umm... I am a bit torn about this.
Only deleting the password when some spurious password is set seems like a reasonable thing to do, but it happens only once at container start-up so we don't actually save much. Moreover, the fork and exec to run the passwd process, which is relatively costly, still happens and in the worst case will happen twice.
So, not sure. Seems like more lines of code for no real benefit. :)
|
Fair enough. Let's close this. |
When a newly created container has already the current user created
inside of it then the user initialization section of command
'init-container' is not triggered. The user init section currently takes
care of:
symlinking /home to /var/home if called with option --home-link
creating user set by --user, --uid, --home and --shell
adds the user to the sudoers group (either sudo or wheel)
removes password of user and of root
This commit does the following:
conditional depending on /home being a symlink or not
'passwd --status' (that is expected to be NP; more in 'man
passwd(1)') calls the said sections
Partially fixes #523