Hi @mbaynton. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

@mbaynton thanks for catching this!

I can confirm (I've just verified it) that before #10607 the userns was created by runc and setgroups was "allow". crun does the same. There is no field in CRI nor runtime-spec to toggle this, so adding it unconditionally here as this PR does, seems the right thing to me. I've also read the manpage, the security considerations stated there don't really apply for our usage (probably why runc and crun default to true too).

@mbaynton can you amend the commits (not add a commit that later removes the change), sign-off the commits (CI is failing due to that) and make sure CI passes fine? Right now most test haven't run because of those failures.

cc @fuweid

It would be great to add this to the 2.0 milestone too.

None of these commits have a Signed-off-by: .. so CI is not running. Can you squash the commits into one and add a Signed-off-by: First Last <email@address.com> as the last line of the commit? Thanks!

/ok-to-test

Thanks @rata and @estesp for your pointers and help with getting this moving.

Sorry on the signoff, I trusted the bot that the next thing required was someone needed to mark ok-to-test, and was probably too pessimistic for my own good that the PR would be noticed :)