PodSecurityPolicy: Do not mutate nil privileged field to false

pkg/security/podsecuritypolicy/provider.go

@@ -157,11 +157,6 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 		return nil, nil, err
 	}
 
-	if sc.Privileged == nil {
-		priv := false
-		sc.Privileged = &priv
-	}
-
 	// if we're using the non-root strategy set the marker that this container should not be
 	// run as root which will signal to the kubelet to do a final check either on the runAsUser
 	// or, if runAsUser is not set, the image UID will be checked.
@@ -284,7 +279,7 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
 	allErrs = append(allErrs, s.strategies.AppArmorStrategy.Validate(pod, container)...)
 	allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidateContainer(pod, container)...)
 
-	if !s.psp.Spec.Privileged && *sc.Privileged {
+	if !s.psp.Spec.Privileged && sc.Privileged != nil && *sc.Privileged {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *sc.Privileged, "Privileged containers are not allowed"))
 	}
 

plugin/pkg/admission/security/podsecuritypolicy/admission_test.go

@@ -204,37 +204,54 @@ func TestAdmitPrivileged(t *testing.T) {
 	privilegedPSP.Name = "priv"
 	privilegedPSP.Spec.Privileged = true
 
+	trueValue := true
+	falseValue := false
+
 	tests := map[string]struct {
 		pod          *kapi.Pod
 		psps         []*extensions.PodSecurityPolicy
 		shouldPass   bool
-		expectedPriv bool
+		expectedPriv *bool
 		expectedPSP  string
 	}{
-		"pod without priv request allowed under non priv PSP": {
+		"pod with priv=nil allowed under non priv PSP": {
 			pod:          goodPod(),
 			psps:         []*extensions.PodSecurityPolicy{nonPrivilegedPSP},
 			shouldPass:   true,
-			expectedPriv: false,
+			expectedPriv: nil,
 			expectedPSP:  nonPrivilegedPSP.Name,
 		},
-		"pod without priv request allowed under priv PSP": {
+		"pod with priv=nil allowed under priv PSP": {
 			pod:          goodPod(),
 			psps:         []*extensions.PodSecurityPolicy{privilegedPSP},
 			shouldPass:   true,
-			expectedPriv: false,
+			expectedPriv: nil,
 			expectedPSP:  privilegedPSP.Name,
 		},
-		"pod with priv request denied by non priv PSP": {
+		"pod with priv=false allowed under non priv PSP": {
+			pod:          createPodWithPriv(false),
+			psps:         []*extensions.PodSecurityPolicy{nonPrivilegedPSP},
+			shouldPass:   true,
+			expectedPriv: &falseValue,
+			expectedPSP:  nonPrivilegedPSP.Name,
+		},
+		"pod with priv=false allowed under priv PSP": {
+			pod:          createPodWithPriv(false),
+			psps:         []*extensions.PodSecurityPolicy{privilegedPSP},
+			shouldPass:   true,
+			expectedPriv: &falseValue,
+			expectedPSP:  privilegedPSP.Name,
+		},
+		"pod with priv=true denied by non priv PSP": {
 			pod:        createPodWithPriv(true),
 			psps:       []*extensions.PodSecurityPolicy{nonPrivilegedPSP},
 			shouldPass: false,
 		},
-		"pod with priv request allowed by priv PSP": {
+		"pod with priv=true allowed by priv PSP": {
 			pod:          createPodWithPriv(true),
 			psps:         []*extensions.PodSecurityPolicy{nonPrivilegedPSP, privilegedPSP},
 			shouldPass:   true,
-			expectedPriv: true,
+			expectedPriv: &trueValue,
 			expectedPSP:  privilegedPSP.Name,
 		},
 	}
@@ -243,9 +260,11 @@ func TestAdmitPrivileged(t *testing.T) {
 		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
 
 		if v.shouldPass {
-			if v.pod.Spec.Containers[0].SecurityContext.Privileged == nil ||
+			priv := v.pod.Spec.Containers[0].SecurityContext.Privileged
-				*v.pod.Spec.Containers[0].SecurityContext.Privileged != v.expectedPriv {
+			if (priv == nil) != (v.expectedPriv == nil) {
-				t.Errorf("%s expected privileged to be %t", k, v.expectedPriv)
+				t.Errorf("%s expected privileged to be %v, got %v", k, v.expectedPriv, priv)
+			} else if priv != nil && *priv != *v.expectedPriv {
+				t.Errorf("%s expected privileged to be %v, got %v", k, *v.expectedPriv, *priv)
 			}
 		}
 	}