Add RunAsGroup support. #710

Random-Liu · 2018-03-30T08:36:03Z

Based on containerd/containerd#2257.

RunAsGroup is added in Kubernetes 1.10 kubernetes/kubernetes#52077. We should support it.

Please hold this PR until I add CRI validation test. kubernetes-sigs/cri-tools#280

Signed-off-by: Lantao Liu lantaol@google.com

mikebrow

See comments and questions.

mikebrow · 2018-03-30T15:28:59Z

pkg/server/container_create.go

@@ -225,6 +226,18 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	if username := securityContext.GetRunAsUsername(); username != "" {
 		specOpts = append(specOpts, oci.WithUsername(username))
 	}
+	if gid := securityContext.GetRunAsGroup(); gid != nil {
+		if securityContext.GetRunAsUser() == nil || securityContext.GetRunAsUsername() == "" {


Above comment block should be modified to mention groupid

uid and username variables should be promoted in scope so they can be used here in this if nil check

mikebrow · 2018-03-30T15:34:24Z

pkg/server/container_create.go

@@ -225,6 +226,18 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	if username := securityContext.GetRunAsUsername(); username != "" {
 		specOpts = append(specOpts, oci.WithUsername(username))
 	}
+	if gid := securityContext.GetRunAsGroup(); gid != nil {
+		if securityContext.GetRunAsUser() == nil || securityContext.GetRunAsUsername() == "" {
+			return nil, errors.New("user group is specified without user")


maybe output uid and username here so we can see if one or both are nil...

My mistake, it should be &&.

mikebrow · 2018-03-30T15:41:15Z

pkg/server/container_create.go

+		if securityContext.GetRunAsUser() == nil || securityContext.GetRunAsUsername() == "" {
+			return nil, errors.New("user group is specified without user")
+		}
+		// TODO(random-liu): Add WithGroupID in containerd client.


and add WithUserID/WithUsername?

We already have those helpers.

mikebrow · 2018-03-30T15:44:56Z

pkg/server/container_create.go

@@ -225,6 +226,18 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 	if username := securityContext.GetRunAsUsername(); username != "" {
 		specOpts = append(specOpts, oci.WithUsername(username))
 	}
+	if gid := securityContext.GetRunAsGroup(); gid != nil {


FYI... oci.WithUserID() and oci.WithUsername() are already looking up the gid and may already be setting it. Course a user can be in more than one group.. so I'm not sure how well the current assignment is working. Thoughts?

We set GID after WithUserID and WithUsername which overwrites the gid.

per discussion let's todo a helper that does uid,username, and gid at the same time..

Random-Liu · 2018-03-30T17:34:41Z

@mikebrow Addressed comments.

mikebrow

/LGTM

Random-Liu · 2018-03-30T19:44:58Z

@mikebrow Added a helper in containerd, which makes the code much cleaner.

I'll send a PR to containerd soon.

Signed-off-by: Lantao Liu <lantaol@google.com>

Random-Liu · 2018-03-30T22:27:47Z

@mikebrow Updated the PR to use the newly added containerd helper oci.WithUser.

mikebrow

/LGTM

Random-Liu · 2018-03-31T00:21:06Z

I added the test kubernetes-sigs/cri-tools#282. This PR passed the test:

$ make test-cri FOCUS=RunAsGroup
/home/lantaol/workspace/bin/critest
Running Suite: CRI validation
=============================
Random Seed: 1522455630 - Will randomize all specs
Will run 70 specs

Running in parallel across 8 nodes

SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
[BeforeEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:50
[BeforeEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:52
[It] runtime should return error if RunAsGroup is set without RunAsUser
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:376
STEP: create pod
STEP: create container with invalid RunAsGroup
STEP: create invalid RunAsGroup container
STEP: create a container with RunAsGroup without RunAsUser
STEP: Get image status for image: busybox:1.26
STEP: Create container.
E0331 00:20:31.185110   16991 remote_runtime.go:187] CreateContainer in sandbox "a6fab5752333e398cc3710f560f0dc5bfa90eec0d61a8f68156ee9c0b4544e3d" from runtime service failed: rpc error: code = Unknown desc = failed to generate user string: user group "1002" is specified without user
[AfterEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:51
[AfterEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:57
STEP: stop PodSandbox
STEP: delete PodSandbox

•
------------------------------
[BeforeEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:50
[BeforeEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:52
[It] runtime should support RunAsGroup
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:358
STEP: create pod
STEP: create a PodSandbox with log directory
STEP: create container for security context RunAsGroup
STEP: create RunAsGroup container
STEP: create a container with RunAsUser and RunAsGroup
STEP: Get image status for image: busybox:1.26
STEP: Create container.
Mar 31 00:20:32.237: INFO: Created container "8e303ff0a02c4243a6f0a8597f2454555e97cba1f65311567eb662c1daecc4df"

STEP: start container
STEP: Start container for containerID: 8e303ff0a02c4243a6f0a8597f2454555e97cba1f65311567eb662c1daecc4df
Mar 31 00:20:32.352: INFO: Started container "8e303ff0a02c4243a6f0a8597f2454555e97cba1f65311567eb662c1daecc4df"

STEP: Get container status for containerID: 8e303ff0a02c4243a6f0a8597f2454555e97cba1f65311567eb662c1daecc4df
STEP: Get container status for containerID: 8e303ff0a02c4243a6f0a8597f2454555e97cba1f65311567eb662c1daecc4df
STEP: verify RunAsGroup for container
STEP: check container output
STEP: verify log contents
Mar 31 00:20:36.354: INFO: Open log file /tmp/podLogTest084028383/PodSandbox-with-log-directory-522fde28-3479-11e8-856a-42010af00002/container-with-RunAsGroup-test-52f0c7d1-3479-11e8-856a-42010af00002.log
Mar 31 00:20:36.354: INFO: Parse container log succeed
[AfterEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:51
[AfterEach] [k8s.io] Security Context
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:57
STEP: stop PodSandbox
STEP: delete PodSandbox


• [SLOW TEST:5.701 seconds]
[k8s.io] Security Context
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:72
  bucket
  /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:288
    runtime should support RunAsGroup
    /home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:358
------------------------------

Ran 2 of 70 Specs in 5.720 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 68 Skipped 

Ginkgo ran 1 suite in 5.761128797s
Test Suite Passed
PASS
./hack/test-utils.sh: line 53: 16920 Terminated              keepalive "sudo ${ROOT}/_output/containerd ${CONTAINERD_FLAGS}" ${RESTART_WAIT_PERIOD} &> ${report_dir}/containerd.log

Random-Liu assigned mikebrow Mar 30, 2018

k8s-ci-robot added the size/S label Mar 30, 2018

Random-Liu added this to the v1.0.0-rc.1 milestone Mar 30, 2018

Random-Liu added the hold label Mar 30, 2018

mikebrow reviewed Mar 30, 2018

View reviewed changes

Random-Liu force-pushed the support-run-as-group branch from ae0ed2f to 8482a98 Compare March 30, 2018 17:34

k8s-ci-robot added size/M and removed size/S labels Mar 30, 2018

mikebrow approved these changes Mar 30, 2018

View reviewed changes

k8s-ci-robot added the lgtm label Mar 30, 2018

Random-Liu force-pushed the support-run-as-group branch from 8482a98 to 0ab24f2 Compare March 30, 2018 19:42

k8s-ci-robot added size/L and removed lgtm size/M labels Mar 30, 2018

Random-Liu force-pushed the support-run-as-group branch from 0ab24f2 to b50b5ca Compare March 30, 2018 19:45

Random-Liu mentioned this pull request Mar 30, 2018

Add oci.WithUser helper function. containerd/containerd#2257

Merged

Add RunAsGroup support.

ed20174

Signed-off-by: Lantao Liu <lantaol@google.com>

Random-Liu force-pushed the support-run-as-group branch from b50b5ca to ed20174 Compare March 30, 2018 22:26

k8s-ci-robot added the lgtm label Mar 31, 2018

mikebrow approved these changes Mar 31, 2018

View reviewed changes

Random-Liu removed the hold label Mar 31, 2018

Random-Liu merged commit ed92bef into containerd:master Mar 31, 2018

Random-Liu deleted the support-run-as-group branch March 31, 2018 00:21

krmayankk mentioned this pull request Nov 9, 2018

Provide RunAsGroup feature for Containers in a Pod kubernetes/enhancements#213

Closed

5 tasks

krmayankk mentioned this pull request Jan 18, 2019

Promote RunAsGroup to Beta kubernetes/kubernetes#73007

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RunAsGroup support. #710

Add RunAsGroup support. #710

Random-Liu commented Mar 30, 2018 •

edited

Loading

mikebrow left a comment

mikebrow Mar 30, 2018

mikebrow Mar 30, 2018

Random-Liu Mar 30, 2018

mikebrow Mar 30, 2018

Random-Liu Mar 30, 2018 •

edited

Loading

mikebrow Mar 30, 2018

Random-Liu Mar 30, 2018

mikebrow Mar 30, 2018

Random-Liu Mar 30, 2018

mikebrow Mar 30, 2018

Random-Liu commented Mar 30, 2018

mikebrow left a comment

Random-Liu commented Mar 30, 2018

Random-Liu commented Mar 30, 2018

mikebrow left a comment

Random-Liu commented Mar 31, 2018

Add RunAsGroup support. #710

Add RunAsGroup support. #710

Conversation

Random-Liu commented Mar 30, 2018 • edited Loading

mikebrow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Random-Liu Mar 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Random-Liu commented Mar 30, 2018

mikebrow left a comment

Choose a reason for hiding this comment

Random-Liu commented Mar 30, 2018

Random-Liu commented Mar 30, 2018

mikebrow left a comment

Choose a reason for hiding this comment

Random-Liu commented Mar 31, 2018

Random-Liu commented Mar 30, 2018 •

edited

Loading

Random-Liu Mar 30, 2018 •

edited

Loading