diff --git a/cri/v1alpha2/cri_utils.go b/cri/v1alpha2/cri_utils.go index 8d8cb1ef40..8c0f08107a 100644 --- a/cri/v1alpha2/cri_utils.go +++ b/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 } diff --git a/cri/v1alpha2/cri_utils_test.go b/cri/v1alpha2/cri_utils_test.go index 41726d73c1..905eb79704 100644 --- a/cri/v1alpha2/cri_utils_test.go +++ b/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,31 +948,36 @@ 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", @@ -979,12 +985,26 @@ func Test_modifyContainerConfig(t *testing.T) { 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", @@ -992,11 +1012,15 @@ func Test_modifyContainerConfig(t *testing.T) { 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 diff --git a/hack/testing/run_daemon_cri_integration.sh b/hack/testing/run_daemon_cri_integration.sh index 488ee41795..85b3aec244 100755 --- a/hack/testing/run_daemon_cri_integration.sh +++ b/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.