Add U volume flag to chown source volumes

[EduardoVega]

What type of PR is this?

/kind feature

What this PR does / why we need it:

It adds the U volume flag to chown source volumes based on the uid, gid within the container.

When a new container is created with a user namespace, we want to get the uid,gid that has been assigned outside the container to chown source volumes. This will isolate the filesystem (source volumes) from other containers (users, groups) since they will be owned by the same user and group within the container.

This is required by podman, so it can parse the U volume flag.
Podman Issue: 7778

How to verify it

There is an integration test to validate this functionality.
Example:

# Create source volume
mkdir -p /var/tmp/foo

# Create container with user namespace and mount volume using U flag
buildah from --userns-uid-map 0:1000:1024 --userns-gid-map 0:1000:1024 --volume /var/tmp/foo:/testdata:z,U --name container-foo alpine

# Ensure owner of the volume is 0:0
buildah run container-foo stat -c "%u:%g" /testdata

# Ensure source volume has been chowned based on user namespace mapping
stat -c "%u:%g" /var/tmp/foo

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Signed-off-by: Eduardo Vega [email protected]

[TomSweeneyRedHat]

@EduardoVega thanks for the PR. The tests don't look happy and it looks like you need to rebase.
@giuseppe PTAL

[TomSweeneyRedHat]

Need man page changes.

[EduardoVega]

I have updated docs.

[rhatdan]

@giuseppe PTAL

[giuseppe]

LGTM once the comment with --user is addressed

[TomSweeneyRedHat]

A few doc nits and a rebase is needed. Otherwise LGTM
Thanks @EduardoVega !

[TomSweeneyRedHat]

@EduardoVega thanks for the updates. Hover it looks like you need to rebase and beyond that, it looks like one of the tests flaked and the rebase will hopefully clear that too.

[rhatdan]

Is there a reason to not allow Overlay options with the U Flag? In SELinux the kernel will prevent this. Might not be a good idea because it will trigger a copy-up, but it might be the case where content is intended to be used for the run of the container temporarily but not written to the host. Of course this would mean the chown would need to happen on each run of the container.

[EduardoVega]

Thanks. Let me work on this.

[EduardoVega]

Currently buildah will chown the source volume and then use the processUID and processGID to create the overlay temp dir, would this be the expected behaviour? Initially I tried to chown the overlay temp directory only, in the case of rootless containers the UID/GID were added correctly, but in the case of rootful containers the content of the source volume was never present on the overlay temp dir so the correct UID/GID were never added to sub dirs/files.

[rhatdan]

If you chown the files after the volume is mounted, it should create new INODES in the upper directory with the correct ownership.

[EduardoVega]

It seems to work with rootless containers since the volume is mounted using fuse-overlayfs here https://github.com/containers/buildah/blob/master/pkg/overlay/overlay.go#L64, but for containers created by root the overlay volume is mounted by crun using the native overlay type.

If I am not wrong there's no way to specify uid/gid as an option for overlay native mount type. The only way I can see this working is if the real source volume is chown just like volumes without the O flag. So the chown would be permanent but any other operation would be temporary.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EduardoVega, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

@EduardoVega Please rebase to get past the ubuntu failures.

[rhatdan]

/lgtm
Thanks @EduardoVega