executor: switch to docker seccomp profile #1807
Conversation
| // TODO: Can LCOW support seccomp? Does that even make sense? | ||
| return []oci.SpecOpts{seccomp.WithDefaultProfile()}, nil |
There was a problem hiding this comment.
To answer this question; https://github.com/containerd/containerd/blob/v1.4.1/contrib/seccomp/seccomp_default_unsupported.go#L1-L26
// +build !linuxfunc DefaultProfile(sp *specs.Spec) *specs.LinuxSeccomp {
return &specs.LinuxSeccomp{}
}
There was a problem hiding this comment.
This was more of a forward-looking pondering. I know it doesn't work now, but I figured if at some point it was implemented, that was the place to hook it up. Although system.SeccompSupported() doesn't really provide a way to indicate whether it's LCOW or not, so we'd still need some more plumbing to activate that.
The whole LCOW thing seems to be somewhat up-in-the-air anyway, I was going to ignore it until after I had WCOW working, if even then.
| var err error | ||
| s.Linux.Seccomp, err = seccomp.GetDefaultProfile(s) | ||
| return err |
There was a problem hiding this comment.
I think this should to the equivalent of what the containerd code does. Probably should add a function to the docker package to directly apply the profile instead of returning it
|
@tonistiigi @tiborvass thought I'd try to replace the containerd profile so that buildkit and docker would remain in sync; let me know if you think this makes sense. |
While we try to keep the containerd and docker seccomp profiles in sync, they may not always be; this switches the executor to use the docker seccomp profile, so that buildkit (when vendored in docker) will use the same default seccomp profile as is used for containers. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
switch_seccomp
branch
from
dd9c3b1 to
a1a85da
Compare
|
If the Docker and containerd seccomp profiles differ, will this change cause problems for the containerd executor when running against plain containerd (outside Docker)? The Edit: Ah, I think I see. |
"technically" they're fully separate, however, I try to make sure that whenever a change is merged in moby, to also open a pull request in containerd (and vice-versa). So the "syncing" is fully manual 😞 however, I did some preparation / refactor work recently (https://github.com/moby/moby/pulls?q=is%3Apr+author%3Athajeztah+is%3Aclosed+seccomp+in%3Atitle+milestone%3A20.10.0+) to make the seccomp package in moby self contained. Might still need a bit of cleaning up, but possibly we can move it to a separate repository, and make containerd use that as a dependency. That way we can update the profile, and have both containerd and moby/docker use the same profile |
|
"fully separate" -> the profile in containerd is a fork/copy of the one in docker |
|
Yeah, that makes sense, and sounds like a good way forward. |
|
I know the containerd maintainers are not always happy with adding more dependencies, so I may need to try to convince them (and have that package in a clean shape; some code is (currently) docker specific, because docker allows specifying a custom seccomp profile as a JSON, whereas containerd does not have that functionality (only "default" profile, or provide a custom profile through a golang struct) |
While we try to keep the containerd and docker seccomp profiles in sync, they may not always be; this switches the executor to use the docker seccomp profile, so that buildkit (when vendored in docker) will use the same default seccomp profile as is used for containers.