Merge pull request #75680 from tallclair/psp-refactor

pkg/security/podsecuritypolicy/BUILD

@@ -49,8 +49,6 @@ go_test(
         "//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
-        "//vendor/github.com/davecgh/go-spew/spew:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/github.com/stretchr/testify/require:go_default_library",
     ],

pkg/security/podsecuritypolicy/provider.go

@@ -59,10 +59,10 @@ func NewSimpleProvider(psp *policy.PodSecurityPolicy, namespace string, strategy
 	}, nil
 }
 
-// DefaultPodSecurityContext sets the default values of the required but not filled fields.
-// It modifies the SecurityContext and annotations of the provided pod. Validation should be
-// used after the context is defaulted to ensure it complies with the required restrictions.
-func (s *simpleProvider) DefaultPodSecurityContext(pod *api.Pod) error {
+// MutatePod sets the default values of the required but not filled fields.
+// Validation should be used after the context is defaulted to ensure it
+// complies with the required restrictions.
+func (s *simpleProvider) MutatePod(pod *api.Pod) error {
 	sc := securitycontext.NewPodSecurityContextMutator(pod.Spec.SecurityContext)
 
 	if sc.SupplementalGroups() == nil {
@@ -104,13 +104,25 @@ func (s *simpleProvider) DefaultPodSecurityContext(pod *api.Pod) error {
 
 	pod.Spec.SecurityContext = sc.PodSecurityContext()
 
+	for i := range pod.Spec.InitContainers {
+		if err := s.mutateContainer(pod, &pod.Spec.InitContainers[i]); err != nil {
+			return err
+		}
+	}
+
+	for i := range pod.Spec.Containers {
+		if err := s.mutateContainer(pod, &pod.Spec.Containers[i]); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
-// DefaultContainerSecurityContext sets the default values of the required but not filled fields.
+// mutateContainer sets the default values of the required but not filled fields.
 // It modifies the SecurityContext of the container and annotations of the pod. Validation should
 // be used after the context is defaulted to ensure it complies with the required restrictions.
-func (s *simpleProvider) DefaultContainerSecurityContext(pod *api.Pod, container *api.Container) error {
+func (s *simpleProvider) mutateContainer(pod *api.Pod, container *api.Container) error {
 	sc := securitycontext.NewEffectiveContainerSecurityContextMutator(
 		securitycontext.NewPodSecurityContextAccessor(pod.Spec.SecurityContext),
 		securitycontext.NewContainerSecurityContextMutator(container.SecurityContext),
@@ -282,11 +294,22 @@ func (s *simpleProvider) ValidatePod(pod *api.Pod) field.ErrorList {
 			}
 		}
 	}
+
+	fldPath := field.NewPath("spec", "initContainers")
+	for i := range pod.Spec.InitContainers {
+		allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.InitContainers[i], fldPath.Index(i))...)
+	}
+
+	fldPath = field.NewPath("spec", "containers")
+	for i := range pod.Spec.Containers {
+		allErrs = append(allErrs, s.validateContainer(pod, &pod.Spec.Containers[i], fldPath.Index(i))...)
+	}
+
 	return allErrs
 }
 
 // Ensure a container's SecurityContext is in compliance with the given constraints
-func (s *simpleProvider) ValidateContainer(pod *api.Pod, container *api.Container, containerPath *field.Path) field.ErrorList {
+func (s *simpleProvider) validateContainer(pod *api.Pod, container *api.Container, containerPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	podSC := securitycontext.NewPodSecurityContextAccessor(pod.Spec.SecurityContext)

pkg/security/podsecuritypolicy/types.go

@@ -32,16 +32,12 @@ import (
 // Provider provides the implementation to generate a new security
 // context based on constraints or validate an existing security context against constraints.
 type Provider interface {
-	// DefaultPodSecurityContext sets the default values of the required but not filled fields.
-	// It modifies the SecurityContext and annotations of the provided pod.
-	DefaultPodSecurityContext(pod *api.Pod) error
-	// DefaultContainerSecurityContext sets the default values of the required but not filled fields.
-	// It modifies the SecurityContext of the container and annotations of the pod.
-	DefaultContainerSecurityContext(pod *api.Pod, container *api.Container) error
-	// Ensure a pod is in compliance with the given constraints.
+	// MutatePod sets the default values of the required but not filled fields of the pod and all
+	// containers in the pod.
+	MutatePod(pod *api.Pod) error
+	// ValidatePod ensures a pod and all its containers are in compliance with the given constraints.
+	// ValidatePod MUST NOT mutate the pod.
 	ValidatePod(pod *api.Pod) field.ErrorList
-	// Ensure a container's SecurityContext is in compliance with the given constraints.
-	ValidateContainer(pod *api.Pod, container *api.Container, containerPath *field.Path) field.ErrorList
 	// Get the name of the PSP that this provider was initialized with.
 	GetPSPName() string
 }

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

@@ -306,35 +306,14 @@ func (c *PodSecurityPolicyPlugin) computeSecurityContext(a admission.Attributes,
 func assignSecurityContext(provider psp.Provider, pod *api.Pod) field.ErrorList {
 	errs := field.ErrorList{}
 
-	err := provider.DefaultPodSecurityContext(pod)
-	if err != nil {
-		errs = append(errs, field.Invalid(field.NewPath("spec", "securityContext"), pod.Spec.SecurityContext, err.Error()))
+	if err := provider.MutatePod(pod); err != nil {
+		// TODO(tallclair): MutatePod should return a field.ErrorList
+		errs = append(errs, field.Invalid(field.NewPath(""), pod, err.Error()))
 	}
 
 	errs = append(errs, provider.ValidatePod(pod)...)
 
-	for i := range pod.Spec.InitContainers {
-		err := provider.DefaultContainerSecurityContext(pod, &pod.Spec.InitContainers[i])
-		if err != nil {
-			errs = append(errs, field.Invalid(field.NewPath("spec", "initContainers").Index(i).Child("securityContext"), "", err.Error()))
-			continue
-		}
-		errs = append(errs, provider.ValidateContainer(pod, &pod.Spec.InitContainers[i], field.NewPath("spec", "initContainers").Index(i))...)
-	}
-
-	for i := range pod.Spec.Containers {
-		err := provider.DefaultContainerSecurityContext(pod, &pod.Spec.Containers[i])
-		if err != nil {
-			errs = append(errs, field.Invalid(field.NewPath("spec", "containers").Index(i).Child("securityContext"), "", err.Error()))
-			continue
-		}
-		errs = append(errs, provider.ValidateContainer(pod, &pod.Spec.Containers[i], field.NewPath("spec", "containers").Index(i))...)
-	}
-
-	if len(errs) > 0 {
-		return errs
-	}
-	return nil
+	return errs
 }
 
 // createProvidersFromPolicies creates providers from the constraints supplied.