From a4a3c7938a9241210160f7a7500fadb049ade338 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Fri, 10 Nov 2017 18:23:26 +0100 Subject: [PATCH] CreateContainerSecurityContext: rename; modify its arguments intead of returning a copy. --- pkg/security/podsecuritypolicy/BUILD | 1 - pkg/security/podsecuritypolicy/provider.go | 26 +++++++++---------- .../podsecuritypolicy/provider_test.go | 11 ++++---- pkg/security/podsecuritypolicy/types.go | 6 ++--- .../security/podsecuritypolicy/admission.go | 9 ++----- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/pkg/security/podsecuritypolicy/BUILD b/pkg/security/podsecuritypolicy/BUILD index 282d256aae..e76448d470 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 ccf897ec75..0c6bc1c71f 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 @@ -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 -// 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) } @@ -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. diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index 1c740842d8..ee1afcd308 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/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) } diff --git a/pkg/security/podsecuritypolicy/types.go b/pkg/security/podsecuritypolicy/types.go index 405ac65df7..1cb7b025b4 100644 --- a/pkg/security/podsecuritypolicy/types.go +++ b/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 - // of Pod annotations for alpha feature support. - 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. + 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 c7b545ec99..a2f8c3d6b7 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission.go +++ b/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"))...) }