feature: support RunAsGroup of CRI Manager

Signed-off-by: Starnop <starnop@163.com>

cri/v1alpha2/cri_utils.go

@@ -205,14 +205,20 @@ func applySandboxSecurityContext(lc *runtime.LinuxPodSandboxConfig, config *apit
 		sc = &runtime.LinuxContainerSecurityContext{
 			SupplementalGroups: lc.SecurityContext.SupplementalGroups,
 			RunAsUser:          lc.SecurityContext.RunAsUser,
+			RunAsGroup:         lc.SecurityContext.RunAsGroup,
+			RunAsUsername:      "",
 			ReadonlyRootfs:     lc.SecurityContext.ReadonlyRootfs,
 			SelinuxOptions:     lc.SecurityContext.SelinuxOptions,
 			NamespaceOptions:   lc.SecurityContext.NamespaceOptions,
 		}
 	}
 
-	modifyContainerConfig(sc, config)
-	err := modifyHostConfig(sc, hc)
+	err := modifyContainerConfig(sc, config)
+	if err != nil {
+		return err
+	}
+
+	err = modifyHostConfig(sc, hc)
 	if err != nil {
 		return err
 	}
@@ -673,28 +679,53 @@ func modifyHostConfig(sc *runtime.LinuxContainerSecurityContext, hostConfig *api
 }
 
 // modifyContainerConfig applies container security context config to pouch's Config.
-func modifyContainerConfig(sc *runtime.LinuxContainerSecurityContext, config *apitypes.ContainerConfig) {
+func modifyContainerConfig(sc *runtime.LinuxContainerSecurityContext, config *apitypes.ContainerConfig) error {
 	if sc == nil {
-		return
+		return nil
 	}
+
+	var userStr, groupStr string
+
 	if sc.RunAsUser != nil {
-		config.User = strconv.FormatInt(sc.GetRunAsUser().Value, 10)
+		userStr = strconv.FormatInt(sc.RunAsUser.GetValue(), 10)
 	}
 	if sc.RunAsUsername != "" {
-		config.User = sc.RunAsUsername
+		userStr = sc.RunAsUsername
 	}
+	if sc.RunAsGroup != nil {
+		groupStr = strconv.FormatInt(sc.RunAsGroup.GetValue(), 10)
+	}
+
+	if userStr == "" {
+		// run_as_group should only be specified when run_as_user or run_as_username is specified;
+		// otherwise, the runtime MUST error.
+		if groupStr != "" {
+			return fmt.Errorf("user group %q is specified without user", groupStr)
+		}
+		return nil
+	}
+	if groupStr != "" {
+		userStr = userStr + ":" + groupStr
+	}
+
+	config.User = userStr
+	return nil
 }
 
 // applyContainerSecurityContext updates pouch container options according to security context.
 func applyContainerSecurityContext(lc *runtime.LinuxContainerConfig, podSandboxID string, config *apitypes.ContainerConfig, hc *apitypes.HostConfig) error {
-	modifyContainerConfig(lc.SecurityContext, config)
+	sc := lc.SecurityContext
+	err := modifyContainerConfig(sc, config)
+	if err != nil {
+		return err
+	}
 
-	err := modifyHostConfig(lc.SecurityContext, hc)
+	err = modifyHostConfig(sc, hc)
 	if err != nil {
 		return err
 	}
 
-	modifyContainerNamespaceOptions(lc.SecurityContext.GetNamespaceOptions(), podSandboxID, hc)
+	modifyContainerNamespaceOptions(sc.GetNamespaceOptions(), podSandboxID, hc)
 
 	return nil
 }

cri/v1alpha2/cri_utils_test.go

@@ -937,7 +937,8 @@ func Test_modifyHostConfig(t *testing.T) {
 
 func Test_modifyContainerConfig(t *testing.T) {
 	runAsUser := &runtime.Int64Value{int64(1)}
-	configUser := strconv.FormatInt(1, 10)
+	runAsGroup := &runtime.Int64Value{int64(1)}
+	formatResult := strconv.FormatInt(1, 10)
 
 	type args struct {
 		sc     *runtime.LinuxContainerSecurityContext
@@ -947,56 +948,79 @@ func Test_modifyContainerConfig(t *testing.T) {
 		name       string
 		args       args
 		wantConfig *apitypes.ContainerConfig
+		wantErr    bool
 	}{
 		{
-			name: "Normal Test",
+			name: "No nil Test",
 			args: args{
 				sc: &runtime.LinuxContainerSecurityContext{
 					RunAsUser:     runAsUser,
 					RunAsUsername: "foo",
+					RunAsGroup:    runAsGroup,
 				},
 				config: &apitypes.ContainerConfig{},
 			},
 			wantConfig: &apitypes.ContainerConfig{
-				User: "foo",
+				User: "foo" + ":" + formatResult,
 			},
+			wantErr: false,
 		},
 		{
 			name: "RunAsUser Nil Test",
 			args: args{
 				sc: &runtime.LinuxContainerSecurityContext{
 					RunAsUsername: "foo",
+					RunAsGroup:    runAsGroup,
 				},
 				config: &apitypes.ContainerConfig{},
 			},
 			wantConfig: &apitypes.ContainerConfig{
-				User: "foo",
+				User: "foo" + ":" + formatResult,
 			},
+			wantErr: false,
 		},
 		{
 			name: "RunAsUsername Empty Test",
 			args: args{
 				sc: &runtime.LinuxContainerSecurityContext{
 					RunAsUser:     runAsUser,
 					RunAsUsername: "",
+					RunAsGroup:    runAsGroup,
 				},
 				config: &apitypes.ContainerConfig{},
 			},
 			wantConfig: &apitypes.ContainerConfig{
-				User: configUser,
+				User: formatResult + ":" + formatResult,
 			},
+			wantErr: false,
+		},
+		{
+			name: "RunAsUser And RunAsUsername All Empty Test",
+			args: args{
+				sc: &runtime.LinuxContainerSecurityContext{
+					RunAsUsername: "",
+					RunAsGroup:    runAsGroup,
+				},
+				config: &apitypes.ContainerConfig{},
+			},
+			wantConfig: &apitypes.ContainerConfig{},
+			wantErr:    true,
 		},
 		{
 			name: "Nil Test",
 			args: args{
 				config: &apitypes.ContainerConfig{},
 			},
 			wantConfig: &apitypes.ContainerConfig{},
+			wantErr:    false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			modifyContainerConfig(tt.args.sc, tt.args.config)
+			err := modifyContainerConfig(tt.args.sc, tt.args.config)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("modifyContainerConfig() error = %v, wantErr %v", err, tt.wantErr)
+			}
 			if !reflect.DeepEqual(tt.args.config, tt.wantConfig) {
 				t.Errorf("modifyContainerConfig() config = %v, wantConfig %v", tt.args.config, tt.wantConfig)
 				return

hack/testing/run_daemon_cri_integration.sh

@@ -18,11 +18,9 @@ export PATH="${REPO_BASE}/bin:${PATH}"
 export PATH="${GOPATH}/bin:${PATH}"
 
 # CRI_SKIP skips the test to skip.
-DEFAULT_CRI_SKIP="RunAsUserName|seccomp localhost"
+DEFAULT_CRI_SKIP="seccomp localhost"
 DEFAULT_CRI_SKIP="${DEFAULT_CRI_SKIP}|should error on create with wrong options"
 DEFAULT_CRI_SKIP="${DEFAULT_CRI_SKIP}|runtime should support reopening container log"
-DEFAULT_CRI_SKIP="${DEFAULT_CRI_SKIP}|runtime should support RunAsGroup"
-DEFAULT_CRI_SKIP="${DEFAULT_CRI_SKIP}|runtime should return error if RunAsGroup is set without RunAsUser"
 CRI_SKIP="${CRI_SKIP:-"${DEFAULT_CRI_SKIP}"}"
 
 # CRI_FOCUS focuses the test to run.