API Changes for RunAsGroup and Implementation and e2e

pkg/kubelet/apis/cri/runtime/v1alpha2/api.proto

@@ -235,6 +235,8 @@ message LinuxSandboxSecurityContext {
     SELinuxOption selinux_options = 2;
     // UID to run sandbox processes as, when applicable.
     Int64Value run_as_user = 3;
+    // GID to run sandbox processes as, when applicable.
+    Int64Value run_as_group = 8;
     // If set, the root filesystem of the sandbox is read-only.
     bool readonly_rootfs = 4;
     // List of groups applied to the first process run in the sandbox, in
@@ -551,6 +553,9 @@ message LinuxContainerSecurityContext {
     // UID to run the container process as. Only one of run_as_user and
     // run_as_username can be specified at a time.
     Int64Value run_as_user = 5;
+    // GID to run the container process as. Only one of run_as_group and
+    // run_as_groupname can be specified at a time.
+    Int64Value run_as_group = 12;
     // User name to run the container process as. If specified, the user MUST
     // exist in the container image (i.e. in the /etc/passwd inside the image),
     // and be resolved there by the runtime; otherwise, the runtime MUST error.

pkg/kubelet/dockershim/security_context.go

@@ -39,13 +39,18 @@ func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *d
 		sc = &runtimeapi.LinuxContainerSecurityContext{
 			SupplementalGroups: lc.SecurityContext.SupplementalGroups,
 			RunAsUser:          lc.SecurityContext.RunAsUser,
+			RunAsGroup:         lc.SecurityContext.RunAsGroup,
 			ReadonlyRootfs:     lc.SecurityContext.ReadonlyRootfs,
 			SelinuxOptions:     lc.SecurityContext.SelinuxOptions,
 			NamespaceOptions:   lc.SecurityContext.NamespaceOptions,
 		}
 	}
 
-	modifyContainerConfig(sc, config)
+	err := modifyContainerConfig(sc, config)
+	if err != nil {
+		return err
+	}
+
 	if err := modifyHostConfig(sc, hc, separator); err != nil {
 		return err
 	}
@@ -59,7 +64,10 @@ func applyContainerSecurityContext(lc *runtimeapi.LinuxContainerConfig, podSandb
 		return nil
 	}
 
-	modifyContainerConfig(lc.SecurityContext, config)
+	err := modifyContainerConfig(lc.SecurityContext, config)
+	if err != nil {
+		return err
+	}
 	if err := modifyHostConfig(lc.SecurityContext, hc, separator); err != nil {
 		return err
 	}
@@ -68,9 +76,9 @@ func applyContainerSecurityContext(lc *runtimeapi.LinuxContainerConfig, podSandb
 }
 
 // modifyContainerConfig applies container security context config to dockercontainer.Config.
-func modifyContainerConfig(sc *runtimeapi.LinuxContainerSecurityContext, config *dockercontainer.Config) {
+func modifyContainerConfig(sc *runtimeapi.LinuxContainerSecurityContext, config *dockercontainer.Config) error {
 	if sc == nil {
-		return
+		return nil
 	}
 	if sc.RunAsUser != nil {
 		config.User = strconv.FormatInt(sc.GetRunAsUser().Value, 10)
@@ -78,6 +86,18 @@ func modifyContainerConfig(sc *runtimeapi.LinuxContainerSecurityContext, config
 	if sc.RunAsUsername != "" {
 		config.User = sc.RunAsUsername
 	}
+
+	user := config.User
+	if sc.RunAsGroup != nil {
+		if user == "" {
+			return fmt.Errorf("runAsGroup is specified without a runAsUser.")
+		}
+		user = fmt.Sprintf("%s:%d", config.User, sc.GetRunAsGroup().Value)
+	}
+
+	config.User = user
+
+	return nil
 }
 
 // modifyHostConfig applies security context config to dockercontainer.HostConfig.

pkg/kubelet/dockershim/security_context_test.go

@@ -31,11 +31,13 @@ import (
 func TestModifyContainerConfig(t *testing.T) {
 	var uid int64 = 123
 	var username = "testuser"
+	var gid int64 = 423
 
 	cases := []struct {
 		name     string
 		sc       *runtimeapi.LinuxContainerSecurityContext
 		expected *dockercontainer.Config
+		isErr    bool
 	}{
 		{
 			name: "container.SecurityContext.RunAsUser set",
@@ -45,6 +47,7 @@ func TestModifyContainerConfig(t *testing.T) {
 			expected: &dockercontainer.Config{
 				User: strconv.FormatInt(uid, 10),
 			},
+			isErr: false,
 		},
 		{
 			name: "container.SecurityContext.RunAsUsername set",
@@ -54,18 +57,54 @@ func TestModifyContainerConfig(t *testing.T) {
 			expected: &dockercontainer.Config{
 				User: username,
 			},
+			isErr: false,
 		},
 		{
 			name:     "no RunAsUser value set",
 			sc:       &runtimeapi.LinuxContainerSecurityContext{},
 			expected: &dockercontainer.Config{},
+			isErr:    false,
+		},
+		{
+			name: "RunAsUser value set, RunAsGroup set",
+			sc: &runtimeapi.LinuxContainerSecurityContext{
+				RunAsUser:  &runtimeapi.Int64Value{Value: uid},
+				RunAsGroup: &runtimeapi.Int64Value{Value: gid},
+			},
+			expected: &dockercontainer.Config{
+				User: "123:423",
+			},
+			isErr: false,
+		},
+		{
+			name: "RunAsUsername value set, RunAsGroup set",
+			sc: &runtimeapi.LinuxContainerSecurityContext{
+				RunAsUsername: username,
+				RunAsGroup:    &runtimeapi.Int64Value{Value: gid},
+			},
+			expected: &dockercontainer.Config{
+				User: "testuser:423",
+			},
+			isErr: false,
+		},
+		{
+			name: "RunAsUser/RunAsUsername not set, RunAsGroup set",
+			sc: &runtimeapi.LinuxContainerSecurityContext{
+				RunAsGroup: &runtimeapi.Int64Value{Value: gid},
+			},
+			isErr: true,
 		},
 	}
 
 	for _, tc := range cases {
 		dockerCfg := &dockercontainer.Config{}
-		modifyContainerConfig(tc.sc, dockerCfg)
-		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
+		err := modifyContainerConfig(tc.sc, dockerCfg)
+		if tc.isErr {
+			assert.NotNil(t, err)
+		} else {
+			assert.Nil(t, err)
+			assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
+		}
 	}
 }