diff --git a/pkg/security/podsecuritypolicy/BUILD b/pkg/security/podsecuritypolicy/BUILD index 282d256aaef..e76448d470d 100644 --- a/pkg/security/podsecuritypolicy/BUILD +++ b/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", ], diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index b10aa15555c..cc5132654b4 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/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 @@ -64,17 +63,16 @@ func NewSimpleProvider(psp *extensions.PodSecurityPolicy, namespace string, stra }, nil } -// Create a PodSecurityContext based on the given constraints. If a setting is already set -// on the PodSecurityContext it will not be changed. Validate should be used after the context -// is created to ensure it complies with the required restrictions. -func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, map[string]string, error) { +// 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 { sc := securitycontext.NewPodSecurityContextMutator(pod.Spec.SecurityContext) - annotations := maps.CopySS(pod.Annotations) if sc.SupplementalGroups() == nil { supGroups, err := s.strategies.SupplementalGroupStrategy.Generate(pod) if err != nil { - return nil, nil, err + return err } sc.SetSupplementalGroups(supGroups) } @@ -82,7 +80,7 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit if sc.FSGroup() == nil { fsGroup, err := s.strategies.FSGroupStrategy.GenerateSingle(pod) if err != nil { - return nil, nil, err + return err } sc.SetFSGroup(fsGroup) } @@ -90,41 +88,42 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit if sc.SELinuxOptions() == nil { seLinux, err := s.strategies.SELinuxStrategy.Generate(pod, nil) if err != nil { - return nil, nil, err + return err } sc.SetSELinuxOptions(seLinux) } // This is only generated on the pod level. Containers inherit the pod's profile. If the // container has a specific profile set then it will be caught in the validation step. - seccompProfile, err := s.strategies.SeccompStrategy.Generate(annotations, pod) + seccompProfile, err := s.strategies.SeccompStrategy.Generate(pod.Annotations, pod) if err != nil { - return nil, nil, err + return err } if seccompProfile != "" { - if annotations == nil { - annotations = map[string]string{} + if pod.Annotations == nil { + pod.Annotations = map[string]string{} } - annotations[api.SeccompPodAnnotationKey] = seccompProfile + pod.Annotations[api.SeccompPodAnnotationKey] = seccompProfile } - return sc.PodSecurityContext(), annotations, nil + + pod.Spec.SecurityContext = sc.PodSecurityContext() + + return nil } -// Create a SecurityContext based on the given constraints. If a setting is already set on the -// container's security context then it will not be changed. Validation should be used after -// the context is created to ensure it complies with the required restrictions. -func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, 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. 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 { 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) } @@ -132,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 @@ -152,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) @@ -174,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. diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index 6a112b67446..5d05b19c41d 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -38,7 +38,7 @@ import ( const defaultContainerName = "test-c" -func TestCreatePodSecurityContextNonmutating(t *testing.T) { +func TestDefaultPodSecurityContextNonmutating(t *testing.T) { // Create a pod with a security context that needs filling in createPod := func() *api.Pod { return &api.Pod{ @@ -82,7 +82,7 @@ func TestCreatePodSecurityContextNonmutating(t *testing.T) { if err != nil { t.Fatalf("unable to create provider %v", err) } - _, _, err = provider.CreatePodSecurityContext(pod) + err = provider.DefaultPodSecurityContext(pod) if err != nil { t.Fatalf("unable to create psc %v", err) } @@ -91,14 +91,14 @@ func TestCreatePodSecurityContextNonmutating(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 CreatePodSecurityContext. diff:\n%s", diffs) + t.Errorf("pod was mutated by DefaultPodSecurityContext. diff:\n%s", diffs) } if !reflect.DeepEqual(createPSP(), psp) { - t.Error("psp was mutated by CreatePodSecurityContext") + t.Error("psp was mutated by DefaultPodSecurityContext") } } -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") } } } @@ -965,12 +965,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) } diff --git a/pkg/security/podsecuritypolicy/types.go b/pkg/security/podsecuritypolicy/types.go index 31fcc5484d8..1cb7b025b43 100644 --- a/pkg/security/podsecuritypolicy/types.go +++ b/pkg/security/podsecuritypolicy/types.go @@ -32,12 +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 { - // Create a PodSecurityContext based on the given constraints. Also returns an updated set - // of Pod annotations for alpha feature support. - CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, map[string]string, error) - // Create a container SecurityContext based on the given constraints. Also returns an updated set - // of Pod annotations for alpha feature support. - CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, error) + // 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'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 diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission.go b/plugin/pkg/admission/security/podsecuritypolicy/admission.go index 15b94df75a0..a8d80515535 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission.go @@ -287,35 +287,28 @@ func (c *PodSecurityPolicyPlugin) computeSecurityContext(a admission.Attributes, func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.Path) field.ErrorList { errs := field.ErrorList{} - psc, pscAnnotations, err := provider.CreatePodSecurityContext(pod) + err := provider.DefaultPodSecurityContext(pod) if err != nil { errs = append(errs, field.Invalid(field.NewPath("spec", "securityContext"), pod.Spec.SecurityContext, err.Error())) } - pod.Spec.SecurityContext = psc - pod.Annotations = pscAnnotations 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"))...) }