Merge pull request #56503 from php-coder/fail_non_root_verification

Automatic merge from submit-queue (batch tested with PRs 56589, 56503). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

MustRunAsNonRoot should reject a pod if it has non-numeric USER

**What this PR does / why we need it**:
This PR modifies kubelet behavior to reject pods with non-numeric USER instead of showing a warning.

**Special notes for your reviewer**:
Related discussion: https://github.com/kubernetes/community/pull/756#discussion_r143694443

**Release note**:
```release-note
kubelet: fix bug where `runAsUser: MustRunAsNonRoot` strategy didn't reject a pod with a non-numeric `USER`.
```

PTAL @pweil- @tallclair @liggitt @Random-Liu
CC @simo5 @adelton

pkg/kubelet/kuberuntime/kuberuntime_container.go

@@ -183,13 +183,10 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Contai
 	if err != nil {
 		return nil, err
 	}
-	if uid != nil {
-		// Verify RunAsNonRoot. Non-root verification only supports numeric user.
-		if err := verifyRunAsNonRoot(pod, container, *uid); err != nil {
-			return nil, err
-		}
-	} else if username != "" {
-		glog.Warningf("Non-root verification doesn't support non-numeric user (%s)", username)
+
+	// Verify RunAsNonRoot. Non-root verification only supports numeric user.
+	if err := verifyRunAsNonRoot(pod, container, uid, username); err != nil {
+		return nil, err
 	}
 
 	command, args := kubecontainer.ExpandContainerCommandAndArgs(container, opts.Envs)

pkg/kubelet/kuberuntime/kuberuntime_container_test.go

@@ -236,7 +236,7 @@ func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerInde
 }
 
 func TestGenerateContainerConfig(t *testing.T) {
-	_, _, m, err := createTestRuntimeManager()
+	_, imageService, m, err := createTestRuntimeManager()
 	assert.NoError(t, err)
 
 	pod := &v1.Pod{
@@ -290,6 +290,18 @@ func TestGenerateContainerConfig(t *testing.T) {
 
 	_, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image)
 	assert.Error(t, err)
+
+	imageId, _ := imageService.PullImage(&runtimeapi.ImageSpec{Image: "busybox"}, nil)
+	image, _ := imageService.ImageStatus(&runtimeapi.ImageSpec{Image: imageId})
+
+	image.Uid = nil
+	image.Username = "test"
+
+	podWithContainerSecurityContext.Spec.Containers[0].SecurityContext.RunAsUser = nil
+	podWithContainerSecurityContext.Spec.Containers[0].SecurityContext.RunAsNonRoot = &runAsNonRootTrue
+
+	_, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image)
+	assert.Error(t, err, "RunAsNonRoot should fail for non-numeric username")
 }
 
 func TestLifeCycleHook(t *testing.T) {

pkg/kubelet/kuberuntime/security_context.go

@@ -75,7 +75,7 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po
 }
 
 // verifyRunAsNonRoot verifies RunAsNonRoot.
-func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid int64) error {
+func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid *int64, username string) error {
 	effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container)
 	// If the option is not set, or if running as root is allowed, return nil.
 	if effectiveSc == nil || effectiveSc.RunAsNonRoot == nil || !*effectiveSc.RunAsNonRoot {
@@ -89,11 +89,14 @@ func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid int64) error {
 		return nil
 	}
 
-	if uid == 0 {
+	switch {
+	case uid != nil && *uid == 0:
 		return fmt.Errorf("container has runAsNonRoot and image will run as root")
+	case uid == nil && len(username) > 0:
+		return fmt.Errorf("container has runAsNonRoot and image has non-numeric user (%s), cannot verify user is non-root", username)
+	default:
+		return nil
 	}
-
-	return nil
 }
 
 // convertToRuntimeSecurityContext converts v1.SecurityContext to runtimeapi.SecurityContext.

pkg/kubelet/kuberuntime/security_context_test.go

@@ -105,7 +105,8 @@ func TestVerifyRunAsNonRoot(t *testing.T) {
 		},
 	} {
 		pod.Spec.Containers[0].SecurityContext = test.sc
-		err := verifyRunAsNonRoot(pod, &pod.Spec.Containers[0], int64(0))
+		uid := int64(0)
+		err := verifyRunAsNonRoot(pod, &pod.Spec.Containers[0], &uid, "")
 		if test.fail {
 			assert.Error(t, err, test.desc)
 		} else {