mount,cache: allow setting `--mount=type=cache` ownership using `uid`,`gid`,`U` and `chown`

[flouthoc]

• Adds support for chown option to --mount=type=cache
• Conflict option uid or gid with U or chown.
• Allow correctly chowing the permission of central cache using uid and gid.

Verify cases where user inside container is not root, so following Container file which was not functional before should work now.

Further tests cases are added to verify all the possible behaviour.

FROM ubi8/ubi

ARG UID=0
ARG GID=0

USER ${UID}:${GID}
RUN --mount=type=cache,uid=${UID},gid=${GID},target=/mycache,z touch /mycache/test.txt
RUN --mount=type=cache,uid=${UID},gid=${GID},target=/mycache,z mkdir -p /mycache/foo/bar

Closes: Bugzilla#2111275

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

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~~ [flouthoc]

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

[rhatdan]

LGTM @giuseppe @vrothberg @nalind @umohnani8 @ashley-cui PTAL

[rhatdan]

@giuseppe @vrothberg @nalind @umohnani8 @ashley-cui PTAL

[flouthoc]

@containers/buildah-maintainers @nalind PTAL again.

[TomSweeneyRedHat]

LGTM but would @nalind to review to make sure his concerns were covered.

[nalind]

Hang on a minute. I'm looking at this, and the bug report, and when I build

FROM busybox
RUN --mount=type=cache,uid=0,target=/var/tmp/.cache ls -lR /var/tmp/.cache; touch /var/tmp/.cache/uid=0; ls -lR /var/tmp/.cache
RUN --mount=type=cache,gid=0,target=/var/tmp/.cache ls -lR /var/tmp/.cache; touch /var/tmp/.cache/gid=0; ls -lR /var/tmp/.cache
RUN --mount=type=cache,uid=1000,target=/var/tmp/.cache ls -lR /var/tmp/.cache ; touch /var/tmp/.cache/uid=1000 ; ls -lR /var/tmp/.cache
RUN --mount=type=cache,gid=1000,target=/var/tmp/.cache ls -lR /var/tmp/.cache; touch /var/tmp/.cache/gid=1000; ls -lR /var/tmp/.cache
RUN --mount=type=cache,uid=1000,gid=1000,target=/var/tmp/.cache ls -lR /var/tmp/.cache; touch /var/tmp/.cache/uid=gid=1000; ls -lR /var/tmp/.cache
RUN --mount=type=cache,target=/var/tmp/.cache ls -lR /var/tmp/.cache

with buildkit, the third and fourth RUN instructions don't see the content that was placed there by the first two RUNs, nor the content that was placed by each other, and no content is seen by the sixth RUN. Are we sure that cache is supposed to be shared among different UIDs/GIDs, with ownership for it all being changed each time it's mounted, the way we're doing it?

In the linked bugzilla bug, would defaulting to assuming "z" for cache mounts when SELinux is enabled have been enough?

[flouthoc]

In the linked bugzilla bug, would defaulting to assuming "z" for cache mounts when SELinux is enabled have been enough?

Yes i was already planning for this in a different PR since this was discussed before as well on offline channel, created a PR now: https://github.com/containers/buildah/pull/4192

with buildkit, the third and fourth RUN instructions don't see the content that was placed there by the first two RUNs, nor the content that was placed by each other, and no content is seen by the sixth RUN. Are we sure that cache is supposed to be shared among different UIDs/GIDs, with ownership for it all being changed each time it's mounted, the way we're doing it?

This is a very different behavior with what is documented (https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/reference.md#run---mounttypecache ) afaics only id is used a separator for cache but looks like buildkit is splitting cache on uid,gid and their combinations as well. I am not sure if this a bug or a feature at buildkit and also can't see why cache with different uid and gid can't be shared if we have a option to modify uid and gid, I'll open issue at buildkit repo to get a some more detail on this.

[flouthoc]

Created an issue for confirming this behavior : https://github.com/moby/buildkit/issues/3041

[TomSweeneyRedHat]

@flouthoc I'm not sure, but you may need to rebase.

[flouthoc]

@flouthoc I'm not sure, but you may need to rebase.

@TomSweeneyRedHat I am waiting for some sort of confirmation here: https://github.com/moby/buildkit/issues/3041 .

[openshift-merge-robot]

@flouthoc: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[github-actions[bot]]

A friendly reminder that this PR had no activity for 30 days.

[rhatdan]

@flouthoc Ping

[flouthoc]

Buildkit folks confirmed the behavior so I'll amend the PR.

[github-actions[bot]]

A friendly reminder that this PR had no activity for 30 days.

[rhatdan]

@flouthoc Reminder...

[flouthoc]

@flouthoc I'm not sure, but you may need to rebase.

@TomSweeneyRedHat I am waiting for some sort of confirmation here: moby/buildkit#3041 .

After this following PR needs a bigger refactor I'll open a new one instead with better context and link back to discussion happened here