[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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

The pull request process is described here

@AdamWill PTAL
@nalind @giuseppe @lsm5 @TomSweeneyRedHat @flouthoc PTAL

I'm skeptical that this will work for manifest push.

@AdamWill could you see if buildah manifest push fails when not in the user namespace. Since this follows the similar path to buildah push, most likely @nalind is correct.

$ ./bin/buildah manifest create quay.io/rhatdan/mymanifest
ff08b4da30d4c9474dcbebde7d61e03864f404541c44f5b2890798d7dbcae3ca
$ ./bin/buildah manifest push quay.io/rhatdan/mymanifest
Getting image list signatures
Copying 0 images generated from 0 images in list
Writing manifest list to image destination
Storing list signatures

This worked.

I'm doubtful that many of the relevant code paths were invoked there, since there was no image rootfs being pushed.

I'm not sure this is safe. login used to be in the list, but you took it out in #3278 because of #3259 .

If adding all these other things to the list means #3259 won't be an issue any more, great, but I'm worried maybe that's not the case :)

Well it looks like login, logout and manifest are likely to require UserNS then.

well the thinking behind my suggestion (make it an arg) was that what you can't do is "cross the streams", right? run some things in userns but some things outside it. but (at least as I understood it) it should in theory be okay to run only non-container-build-y commands, all outside userns. so my idea was to provide a somewhat-hidden way to do that, for this specific use case. if a command-line argument is too prominent it could be an env var or something, I guess.

I am fine with adding a hidden option --unshare and forcing the user to need to deal with it.

@nalind Thoughts?

Would it actually solve the problem? Attempting to push layers without configuring a user namespace can very easily trip over permissions problems.

I can try and test my patch later, I didn't have time when I wrote it.

I'd be curious to hear @giuseppe 's opinion here. The code change looks fine for what you want to do, I'll let others decide if that's an appropriate approach. The hidden --unshare option is appealing.

Looks like a rebase is needed.

I'd be curious to hear @giuseppe 's opinion here. The code change looks fine for what you want to do, I'll let others decide if that's an appropriate approach. The hidden --unshare option is appealing.

if we are accessing the storage as @nalind pointed out, it won't work without a user namespace since we need CAP_DAC_OVERRIDE to access some of the files in an exploded rootfs

sorry for not testing this; after some extensive discussion we decided to try having the image uploader produce its own multi-arch manifest and push it with skopeo, to avoid the need for buildah or podman. I'm just finishing up that code now (adapted from https://pagure.io/koji-flatpak/blob/main/f/koji_flatpak/plugins/flatpak_builder_plugin.py#_586 ). If it works, we'll probably not care about this so much (though it'd still be nice if this could be addressed somehow in skopeo or buildah or podman so we could drop the manifest creation code from our tool).

Update - we changed our uploader tool to build the manifest itself, and it works, so we'll just go with that for now. Bit more code than we really want to carry/maintain, but hey. If there's ever a way to use podman or buildah or skopeo to construct the manifest that doesn't run through this userns stuff, we'll try it.