Drop effective and permitted cap for non-root user. #1397
Conversation
|
@Random-Liu: GitHub didn't allow me to request PR reviews from the following users: michaelbannister, cheftako, BenTheElder. Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Random-Liu
force-pushed
the
fix-non-root-cap
branch
from
07604f5 to
bdbc2ab
Compare
Signed-off-by: Lantao Liu <[email protected]>
Random-Liu
force-pushed
the
fix-non-root-cap
branch
from
bdbc2ab to
f19395a
Compare
|
The new test passed with runc 1.0.0-rc9, but failed with runc 1.0.0-rc10. I guess this runc change is related: opencontainers/runc#2086 The test expectation matches moby/moby#36587. If the expectation of moby/moby#36587 is correct, I believe opencontainers/runc#2086 is a regression. |
|
@Random-Liu: GitHub didn't allow me to request PR reviews from the following users: justincormack. Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| docker build -t $(IMAGE) . | ||
|
|
||
| push: | ||
| gcloud docker -- push $(IMAGE) |
There was a problem hiding this comment.
Suggest adding a todo or open an issue to convert to using test images via local image cache... vs round tripping them through gcr.io. Would also be nice to set the test image name via make so we don't have to hard code it in the source file if possible.
| // if the user is non-root. | ||
| // See https://github.com/moby/moby/pull/36587. | ||
| // | ||
| // This option should be behind options for setting users and capabilities. |
| uid int64 | ||
| startOK bool | ||
| }{ | ||
| "shouldn't be able to run container with a private workdir only accessible to nobody as a non-root user": { |
There was a problem hiding this comment.
suggest: "shouldn't be able to run test container, with a private workdir that is accessible to nobody, as non-root user"
| uid: 1234, | ||
| startOK: false, | ||
| }, | ||
| "shouldn be able to run container with a private workdir only accessible to nobody as root": { |
There was a problem hiding this comment.
suggest: "should be able to run test container, with a private workdir that is accessible to nobody, as root user"
| assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img})) | ||
| }() | ||
|
|
||
| t.Log("Create a container to print capabilities") |
|
Gentle poke -- Let me know if I can help out here 😅 |
|
LGTM. |
|
gentle ping, we've got users that are disappointed to see things pass in CI that would've failed if the node runtime was dockerd 😅 ... would love to help get this in a running containerd, let me know if I can do anything. |
|
@BenTheElder looks like @Random-Liu has the pen now since there are a few things left from @mikebrow 's review. |
|
@Random-Liu just a couple nits.. |
|
@Random-Liu: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
* upstream/master: *: move to klog v2 Add support for ignore-paths Update README.md *: cut v0.7.0 update to use user ID 65532 as workrround for containerd/cri#1397 update the manifests make image be rootless upgrade to use go 1.13 and remove the vendors upgrade to use go 1.13 and remove the vendors
* upstream/master: *: move to klog v2 Add support for ignore-paths Update README.md *: cut v0.7.0 update to use user ID 65532 as workrround for containerd/cri#1397 update the manifests make image be rootless upgrade to use go 1.13 and remove the vendors upgrade to use go 1.13 and remove the vendors
|
carrying forward in containerd/containerd#4669 |
See moby/moby#36587.
Ref kubernetes-sigs/kind#1331.
We may want to cherrypick into supported branches.
/cc @mikebrow @BenTheElder @michaelbannister @cheftako
Signed-off-by: Lantao Liu [email protected]