CreateContainerSecurityContext: rename; modify its arguments intead of returning a copy.

pkg/security/podsecuritypolicy/BUILD

@@ -27,7 +27,6 @@ go_library(
         "//pkg/security/podsecuritypolicy/user:go_default_library",
         "//pkg/security/podsecuritypolicy/util:go_default_library",
         "//pkg/securitycontext:go_default_library",
-        "//pkg/util/maps:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],

pkg/security/podsecuritypolicy/provider.go

@@ -25,7 +25,6 @@ import (
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
 	"k8s.io/kubernetes/pkg/securitycontext"
-	"k8s.io/kubernetes/pkg/util/maps"
 )
 
 // used to pass in the field being validated for reusable group strategies so they
@@ -112,21 +111,19 @@ func (s *simpleProvider) DefaultPodSecurityContext(pod *api.Pod) error {
 	return nil
 }
 
-// Create a SecurityContext based on the given constraints.  If a setting is already set on the
+// DefaultContainerSecurityContext sets the default values of the required but not filled fields.
-// container's security context then it will not be changed.  Validation should be used after
+// It modifies the SecurityContext of the container and annotations of the pod. Validation should
-// the context is created to ensure it complies with the required restrictions.
+// be used after the context is defaulted to ensure it complies with the required restrictions.
-func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, error) {
+func (s *simpleProvider) DefaultContainerSecurityContext(pod *api.Pod, container *api.Container) error {
 	sc := securitycontext.NewEffectiveContainerSecurityContextMutator(
 		securitycontext.NewPodSecurityContextAccessor(pod.Spec.SecurityContext),
 		securitycontext.NewContainerSecurityContextMutator(container.SecurityContext),
 	)
 
-	annotations := maps.CopySS(pod.Annotations)
-
 	if sc.RunAsUser() == nil {
 		uid, err := s.strategies.RunAsUserStrategy.Generate(pod, container)
 		if err != nil {
-			return nil, nil, err
+			return err
 		}
 		sc.SetRunAsUser(uid)
 	}
@@ -134,14 +131,14 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 	if sc.SELinuxOptions() == nil {
 		seLinux, err := s.strategies.SELinuxStrategy.Generate(pod, container)
 		if err != nil {
-			return nil, nil, err
+			return err
 		}
 		sc.SetSELinuxOptions(seLinux)
 	}
 
-	annotations, err := s.strategies.AppArmorStrategy.Generate(annotations, container)
+	annotations, err := s.strategies.AppArmorStrategy.Generate(pod.Annotations, container)
 	if err != nil {
-		return nil, nil, err
+		return err
 	}
 
 	// if we're using the non-root strategy set the marker that this container should not be
@@ -154,7 +151,7 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 
 	caps, err := s.strategies.CapabilitiesStrategy.Generate(pod, container)
 	if err != nil {
-		return nil, nil, err
+		return err
 	}
 	sc.SetCapabilities(caps)
 
@@ -176,7 +173,10 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 		sc.SetAllowPrivilegeEscalation(&s.psp.Spec.AllowPrivilegeEscalation)
 	}
 
-	return sc.ContainerSecurityContext(), annotations, nil
+	pod.Annotations = annotations
+	container.SecurityContext = sc.ContainerSecurityContext()
+
+	return nil
 }
 
 // Ensure a pod's SecurityContext is in compliance with the given constraints.

pkg/security/podsecuritypolicy/provider_test.go

@@ -98,7 +98,7 @@ func TestDefaultPodSecurityContextNonmutating(t *testing.T) {
 	}
 }
 
-func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
+func TestDefaultContainerSecurityContextNonmutating(t *testing.T) {
 	untrue := false
 	tests := []struct {
 		security *api.SecurityContext
@@ -154,7 +154,7 @@ func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unable to create provider %v", err)
 		}
-		_, _, err = provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[0])
+		err = provider.DefaultContainerSecurityContext(pod, &pod.Spec.Containers[0])
 		if err != nil {
 			t.Fatalf("unable to create container security context %v", err)
 		}
@@ -163,10 +163,10 @@ func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
 		// since all the strategies were permissive
 		if !reflect.DeepEqual(createPod(), pod) {
 			diffs := diff.ObjectDiff(createPod(), pod)
-			t.Errorf("pod was mutated by CreateContainerSecurityContext. diff:\n%s", diffs)
+			t.Errorf("pod was mutated by DefaultContainerSecurityContext. diff:\n%s", diffs)
 		}
 		if !reflect.DeepEqual(createPSP(), psp) {
-			t.Error("psp was mutated by CreateContainerSecurityContext")
+			t.Error("psp was mutated by DefaultContainerSecurityContext")
 		}
 	}
 }
@@ -893,12 +893,13 @@ func TestGenerateContainerSecurityContextReadOnlyRootFS(t *testing.T) {
 			t.Errorf("%s unable to create provider %v", k, err)
 			continue
 		}
-		sc, _, err := provider.CreateContainerSecurityContext(v.pod, &v.pod.Spec.Containers[0])
+		err = provider.DefaultContainerSecurityContext(v.pod, &v.pod.Spec.Containers[0])
 		if err != nil {
 			t.Errorf("%s unable to create container security context %v", k, err)
 			continue
 		}
 
+		sc := v.pod.Spec.Containers[0].SecurityContext
 		if v.expected == nil && sc.ReadOnlyRootFilesystem != nil {
 			t.Errorf("%s expected a nil ReadOnlyRootFilesystem but got %t", k, *sc.ReadOnlyRootFilesystem)
 		}

pkg/security/podsecuritypolicy/types.go

@@ -35,9 +35,9 @@ 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
-	// Create a container SecurityContext based on the given constraints. Also returns an updated set
+	// DefaultContainerSecurityContext sets the default values of the required but not filled fields.
-	// of Pod annotations for alpha feature support.
+	// It modifies the SecurityContext of the container and annotations of the pod.
-	CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, error)
+	DefaultContainerSecurityContext(pod *api.Pod, container *api.Container) error
 	// Ensure a pod's SecurityContext is in compliance with the given constraints.
 	ValidatePodSecurityContext(pod *api.Pod, fldPath *field.Path) field.ErrorList
 	// Ensure a container's SecurityContext is in compliance with the given constraints

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

@@ -281,25 +281,20 @@ func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.P
 	errs = append(errs, provider.ValidatePodSecurityContext(pod, field.NewPath("spec", "securityContext"))...)
 
 	for i := range pod.Spec.InitContainers {
-		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.InitContainers[i])
+		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
 		}
-		pod.Spec.InitContainers[i].SecurityContext = sc
-		pod.Annotations = scAnnotations
 		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &pod.Spec.InitContainers[i], field.NewPath("spec", "initContainers").Index(i).Child("securityContext"))...)
 	}
 
 	for i := range pod.Spec.Containers {
-		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[i])
+		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
 		}
-
-		pod.Spec.Containers[i].SecurityContext = sc
-		pod.Annotations = scAnnotations
 		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[i], field.NewPath("spec", "containers").Index(i).Child("securityContext"))...)
 	}