Merge pull request #31927 from timstclair/psp

Automatic merge from submit-queue

Fix PSP volumes error message

Was:
```
Error from server: error when creating "pause-pod.yaml": pods "pause" is forbidden: unable to validate against any pod security policy: [spec.containers[0].securityContext.volumes[0]: Invalid value: "secret": secret volumes are not allowed to be used]
```
Now:
```
Error from server: error when creating "pause-pod.yaml": pods "pause" is forbidden: unable to validate against any pod security policy: [spec.volumes[0]: Invalid value: "secret": secret volumes are not allowed to be used]
```

Also, only perform the validation once (by moving it from `ValidateContainerSecurityContext` to `ValidatePodSecurityContext`).

---

1.4 Justification:

- Risk: low, this is just altering an error message
- Rollback: nothing should depend on this functionality
- Cost: the old error message didn't make any sense (there are no volumes on a container SecurityContext). This is fixing a bug.

pkg/security/podsecuritypolicy/provider.go

@@ -212,6 +212,25 @@ func (s *simpleProvider) ValidatePodSecurityContext(pod *api.Pod, fldPath *field
 
 	allErrs = append(allErrs, s.strategies.SysctlsStrategy.Validate(pod)...)
 
+	// TODO(timstclair): ValidatePodSecurityContext should be renamed to ValidatePod since its scope
+	// is not limited to the PodSecurityContext.
+	if len(pod.Spec.Volumes) > 0 && !psputil.PSPAllowsAllVolumes(s.psp) {
+		allowedVolumes := psputil.FSTypeToStringSet(s.psp.Spec.Volumes)
+		for i, v := range pod.Spec.Volumes {
+			fsType, err := psputil.GetVolumeFSType(v)
+			if err != nil {
+				allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "volumes").Index(i), string(fsType), err.Error()))
+				continue
+			}
+
+			if !allowedVolumes.Has(string(fsType)) {
+				allErrs = append(allErrs, field.Invalid(
+					field.NewPath("spec", "volumes").Index(i), string(fsType),
+					fmt.Sprintf("%s volumes are not allowed to be used", string(fsType))))
+			}
+		}
+	}
+
 	return allErrs
 }
 
@@ -235,23 +254,6 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
 
 	allErrs = append(allErrs, s.strategies.CapabilitiesStrategy.Validate(pod, container)...)
 
-	if len(pod.Spec.Volumes) > 0 && !psputil.PSPAllowsAllVolumes(s.psp) {
-		allowedVolumes := psputil.FSTypeToStringSet(s.psp.Spec.Volumes)
-		for i, v := range pod.Spec.Volumes {
-			fsType, err := psputil.GetVolumeFSType(v)
-			if err != nil {
-				allErrs = append(allErrs, field.Invalid(fldPath.Child("volumes").Index(i), string(fsType), err.Error()))
-				continue
-			}
-
-			if !allowedVolumes.Has(string(fsType)) {
-				allErrs = append(allErrs, field.Invalid(
-					fldPath.Child("volumes").Index(i), string(fsType),
-					fmt.Sprintf("%s volumes are not allowed to be used", string(fsType))))
-			}
-		}
-	}
-
 	if !s.psp.Spec.HostNetwork && pod.Spec.SecurityContext.HostNetwork {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostNetwork"), pod.Spec.SecurityContext.HostNetwork, "Host network is not allowed to be used"))
 	}

pkg/security/podsecuritypolicy/provider_test.go

@@ -216,6 +216,16 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
 		Level: "bar",
 	}
 
+	failHostDirPod := defaultPod()
+	failHostDirPod.Spec.Volumes = []api.Volume{
+		{
+			Name: "bad volume",
+			VolumeSource: api.VolumeSource{
+				HostPath: &api.HostPathVolumeSource{},
+			},
+		},
+	}
+
 	errorCases := map[string]struct {
 		pod           *api.Pod
 		psp           *extensions.PodSecurityPolicy
@@ -266,6 +276,11 @@ func TestValidatePodSecurityContextFailures(t *testing.T) {
 			psp:           failSELinuxPSP,
 			expectedError: "does not match required level.  Found bar, wanted foo",
 		},
+		"failHostDirPSP": {
+			pod:           failHostDirPod,
+			psp:           defaultPSP(),
+			expectedError: "hostPath volumes are not allowed to be used",
+		},
 	}
 	for k, v := range errorCases {
 		provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
@@ -325,16 +340,6 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) {
 		Add: []api.Capability{"foo"},
 	}
 
-	failHostDirPod := defaultPod()
-	failHostDirPod.Spec.Volumes = []api.Volume{
-		{
-			Name: "bad volume",
-			VolumeSource: api.VolumeSource{
-				HostPath: &api.HostPathVolumeSource{},
-			},
-		},
-	}
-
 	failHostPortPod := defaultPod()
 	failHostPortPod.Spec.Containers[0].Ports = []api.ContainerPort{{HostPort: 1}}
 
@@ -380,11 +385,6 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) {
 			psp:           defaultPSP(),
 			expectedError: "capability may not be added",
 		},
-		"failHostDirPSP": {
-			pod:           failHostDirPod,
-			psp:           defaultPSP(),
-			expectedError: "hostPath volumes are not allowed to be used",
-		},
 		"failHostPortPSP": {
 			pod:           failHostPortPod,
 			psp:           defaultPSP(),
@@ -832,7 +832,7 @@ func TestValidateAllowedVolumes(t *testing.T) {
 		}
 
 		// expect a denial for this PSP and test the error message to ensure it's related to the volumesource
-		errs := provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[0], field.NewPath(""))
+		errs := provider.ValidatePodSecurityContext(pod, field.NewPath(""))
 		if len(errs) != 1 {
 			t.Errorf("expected exactly 1 error for %s but got %v", fieldVal.Name, errs)
 		} else {
@@ -843,14 +843,14 @@ func TestValidateAllowedVolumes(t *testing.T) {
 
 		// now add the fstype directly to the psp and it should validate
 		psp.Spec.Volumes = []extensions.FSType{fsType}
-		errs = provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[0], field.NewPath(""))
+		errs = provider.ValidatePodSecurityContext(pod, field.NewPath(""))
 		if len(errs) != 0 {
 			t.Errorf("directly allowing volume expected no errors for %s but got %v", fieldVal.Name, errs)
 		}
 
 		// now change the psp to allow any volumes and the pod should still validate
 		psp.Spec.Volumes = []extensions.FSType{extensions.All}
-		errs = provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[0], field.NewPath(""))
+		errs = provider.ValidatePodSecurityContext(pod, field.NewPath(""))
 		if len(errs) != 0 {
 			t.Errorf("wildcard volume expected no errors for %s but got %v", fieldVal.Name, errs)
 		}