zabbly / incus

Incus package repository
176 stars 14 forks source link

Create incus user and manage userns subuid/gid #16

Closed f-bn closed 7 months ago

f-bn commented 7 months ago

Closes #14, closes #15

stgraber commented 7 months ago

Looks good, thanks!

stgraber commented 7 months ago

Hmm, actually, I wonder if there's a way to not require uidmap?

The behavior when it's not present tends to be better. It makes sense to have the allocations in place for when it is present, but I don't think we should force users to get it.

The main case I can think of is incus-user as that allows individual users to map their own uid/gid into the container. This won't work with subuid/subgid without adding another root allocation for the user's own uid and gid.

f-bn commented 7 months ago

I'm not the most confident with these configurations, so I can add a simple condition to manage these subuid/subgid based on uidmap package presence. Something like :

if ! dpkg -l | grep -q uidmap; then
    # manage subuid/gid
fi

I can also remove this part if not strictly needed and can cause issues. In this case, we may need to clarify the documentation about it, I don't have a strong opinion on this one.

WDYT ?

stgraber commented 7 months ago

usermod should still work fine even if uidmap isn't installed so I'd just not make it a dependency

stgraber commented 7 months ago

I've confirmed that the subuid/subgid usermod logic works perfectly fine when the uidmap package isn't installed. So the logic can stay as it is and just remove the uidmap dependency from debian/control will give us what we want.

f-bn commented 7 months ago

@stgraber requested changes pushed 👍