Improve uid shifting via chown. #44
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rodnymolina
reviewed
Nov 29, 2021
utils.go
Outdated
| @@ -723,28 +723,13 @@ func mntSrcUidShiftNeeded(mntSrc string, uid, gid uint32) (bool, uint32, uint32, | |||
| mntSrcUid = st.Uid | |||
| mntSrcGid = st.Gid | |||
|
|
|||
| // If the host uid assigned to the container is higher than the uid | |||
| // of the dir being mounted into the container, then we perform uid | |||
| // If the host uid assigned to the container's root user differs from the the | |||
rodnymolina
reviewed
Nov 29, 2021
utils.go
Outdated
|
|
||
| needChown = false | ||
| break | ||
| if uid != st.Uid && gid != st.Gid { |
There was a problem hiding this comment.
Good point, I changed it to OR. In practice it won't make a difference, because the chown happens on host dirs that get mounted into the container's special dirs (e.g., /var/lib/docker) and for those we always expect the uid:gid to match. But it's clearer / more correct to do the OR instead of AND.
Sysbox performs uid-shifting using chown for host dirs that are mounted into certain special dirs in the container (e.g., /var/lib/docker, /var/lib/kubelet, etc.) Prior to this change, the uid shifting via chown worked well when the host dir was initially owned by root:root. In this case, when the container started, Sysbox would shift uids to match the container's root user (e.g., 165536:165536); later when the container stopped, Sysbox would revert the uids back to the original one (i.e, root:root). However, we've seen situations where the uid revert may not occur because the host dir gets detached from the host. In this case, a subsequent creation of a container where said host dir is again mounted into a special dir in the container, would result in Sysbox not performing or mishandling the uid shifting. This commit fixes this by ensuring that Sysbox always performs the uid shifting regardless of the original uid associated with the host dir being mounted into the container. Signed-off-by: Cesar Talledo <ctalledo@nestybox.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sysbox performs uid-shifting using chown for host dirs that are mounted into
certain special dirs in the container (e.g., /var/lib/docker, /var/lib/kubelet,
etc.)
Prior to this change, the uid shifting via chown worked well when the host dir
was initially owned by root:root. In this case, when the container started,
Sysbox would shift uids to match the container's root user (e.g.,
165536:165536); later when the container stopped, Sysbox would revert the uids
back to the original one (i.e, root:root).
However, we've seen situations where the uid revert may not occur because the
host dir gets detached from the host. In this case, a subsequent creation of a
container where said host dir is again mounted into a special dir in the
container, would result in Sysbox not performing or mishandling the uid
shifting.
This commit fixes this by ensuring that Sysbox always performs the uid
shifting regardless of the original uid associated with the host dir
being mounted into the container.
Signed-off-by: Cesar Talledo ctalledo@nestybox.com