Quadlet - Support setting UID and GID for --userns=keep-id

[ygalblum]

Does this PR introduce a user-facing change?

Yes

Quadlet - Support setting UID and GID for --userns=keep-id

Resolves: #17908

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ygalblum

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 [ygalblum]

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

[vrothberg]

LGTM
@giuseppe @rhatdan PTAL

[rhatdan]

Why not just support
RemapUsers=keep-id:uid=100:gid200?

I would rather just see us support this syntax then adding the new Remap classes.

@giuseppe WDYT?

[ygalblum]

Why not just support
RemapUsers=keep-id:uid=100:gid200?

Initially that's what I implemented. But, then I looked at how RemapUsers=auto is implemented (using KeyRemapUID and KeyRemapGID) and decided to align with the same convention

[vrothberg]

Why not just support
RemapUsers=keep-id:uid=100:gid200?

Initially that's what I implemented. But, then I looked at how RemapUsers=auto is implemented (using KeyRemapUID and KeyRemapGID) and decided to align with the same convention

SGTM +1

[giuseppe]

Initially that's what I implemented. But, then I looked at how RemapUsers=auto is implemented (using KeyRemapUID and KeyRemapGID) and decided to align with the same convention

I am fine this way if you prefer it, my only concern is that it makes it more difficult to extend the feature in the future if we decide to add more options to keep-id. For example, userns=auto supports already quite a few knobs, would these need to be exposed as well?

[ygalblum]

For example, userns=auto supports already quite a few knobs, would these need to be exposed as well?

Quadlet does not have a key that is a one to one assignment to the userns argument. So, yes, additional nobs would require new keys.

Keep in mind that the initial thought was that Quadlet would not provide a dedicated key for each and every podman run argument or value. Instead, the Container section supports a PodmanArgs key that is passes as is to podman run.

[rhatdan]

Right my concern is we see an explosion of keys, which is what we are starting to see.

[rhatdan]

/lgtm

[vrothberg]

Should we revisit this design decision and just add a UserNS key and deprecate/remove the other ones?

The --userns flag is already something very advanced, so it may be more complicated for users having to decompose the --userns arguments into multiple Quadlet fields.

[giuseppe]

I agree, let's avoid another abstraction

[ygalblum]

Let's continue the discussion in the new issue I opened: #17984