Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/1.7] CRI: Support Linux usernames for !linux platforms #9015

Merged

Conversation

dcantah
Copy link
Member

@dcantah dcantah commented Aug 26, 2023

Backport: #8464

The oci.WithUser option was being applied in container_create_linux.go instead of the cross plat buildLinuxSpec method. There's been recent work to try and make every spec option that can be applied on any platform able to do so, and this falls under that. However, WithUser on linux platforms relies on the containers SnapshotKey being filled out, which means the spec option needs to be applied during container creation.

To make this a little more generic, I've created a new platformSpecOpts method that handles any spec opts that rely on runtime state (rootfs mounted for example) for some platforms, or just platform options that we still don't have workarounds for to be able to specify them for other platforms (apparmor, seccomp etc.) by internally calling the already existing containerSpecOpts method.

(cherry picked from commit 66307d0)

The oci.WithUser option was being applied in container_create_linux.go
instead of the cross plat buildLinuxSpec method. There's been recent
work to try and make every spec option that can be applied on any platform
able to do so, and this falls under that. However, WithUser on linux platforms
relies on the containers SnapshotKey being filled out, which means the spec
option needs to be applied during container creation.

To make this a little more generic, I've created a new platformSpecOpts
method that handles any spec opts that rely on runtime state (rootfs mounted
for example) for some platforms, or just platform options that we still don't
have workarounds for to be able to specify them for other platforms
(apparmor, seccomp etc.) by internally calling the already existing
containerSpecOpts method.

Signed-off-by: Danny Canter <danny@dcantah.dev>
(cherry picked from commit 66307d0)
Signed-off-by: Danny Canter <danny@dcantah.dev>
@samuelkarp samuelkarp added the area/cri Container Runtime Interface (CRI) label Aug 26, 2023
@samuelkarp
Copy link
Member

Looks like this is just in sbserver. Does this need to be ported to the old CRI server package as well?

@dcantah
Copy link
Member Author

dcantah commented Aug 26, 2023

@samuelkarp In the original change I'd only done it for the sandbox server as it uses the sandbox's Platform rpc to know whether to run some logic: https://github.com/containerd/containerd/pull/9015/files#diff-15877ae3602deacafa14a8688676328a93c85215593babe9f4b8a7bd46c2803bR483-R503. I could try and adapt for the regular CRI plugin but I don't think it's as useful there. One use case being this essentially adds support for any operating system that wants to virtualize linux to be able to get the linux username filled out, as if the sandbox is ultimately going to run a Linux container it will let us know via controller.Platform. All we've got is GOOS to key off of in the regular CRI plugin, so we don't have the hint

@dcantah
Copy link
Member Author

dcantah commented Aug 28, 2023

/retest

@dcantah
Copy link
Member Author

dcantah commented Sep 1, 2023

@samuelkarp ptal

@samuelkarp samuelkarp merged commit 86dc86e into containerd:release/1.7 Sep 2, 2023
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cri Container Runtime Interface (CRI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants