diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index 09ca844ed8..712eca1715 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/pkg/security/podsecuritypolicy/provider.go @@ -267,7 +267,7 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe } sc := container.SecurityContext - allErrs = append(allErrs, s.strategies.RunAsUserStrategy.Validate(pod, container)...) + allErrs = append(allErrs, s.strategies.RunAsUserStrategy.Validate(fldPath.Child("securityContext"), pod, container, sc.RunAsNonRoot, sc.RunAsUser)...) allErrs = append(allErrs, s.strategies.SELinuxStrategy.Validate(fldPath.Child("seLinuxOptions"), pod, container, sc.SELinuxOptions)...) allErrs = append(allErrs, s.strategies.AppArmorStrategy.Validate(pod, container)...) allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidateContainer(pod, container)...) diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index 7c7cca7e4a..d61dc96247 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -455,7 +455,7 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) { "failUserPSP": { pod: failUserPod, psp: failUserPSP, - expectedError: "does not match required range", + expectedError: "runAsUser: Invalid value", }, "failSELinuxPSP": { pod: failSELinuxPod, diff --git a/pkg/security/podsecuritypolicy/user/mustrunas.go b/pkg/security/podsecuritypolicy/user/mustrunas.go index abc631e280..df89163357 100644 --- a/pkg/security/podsecuritypolicy/user/mustrunas.go +++ b/pkg/security/podsecuritypolicy/user/mustrunas.go @@ -49,27 +49,17 @@ func (s *mustRunAs) Generate(pod *api.Pod, container *api.Container) (*int64, er } // Validate ensures that the specified values fall within the range of the strategy. -func (s *mustRunAs) Validate(pod *api.Pod, container *api.Container) field.ErrorList { +func (s *mustRunAs) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList { allErrs := field.ErrorList{} - securityContextPath := field.NewPath("securityContext") - if container.SecurityContext == nil { - detail := fmt.Sprintf("unable to validate nil security context for container %s", container.Name) - allErrs = append(allErrs, field.Invalid(securityContextPath, container.SecurityContext, detail)) - return allErrs - } - if container.SecurityContext.RunAsUser == nil { - detail := fmt.Sprintf("unable to validate nil RunAsUser for container %s", container.Name) - allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), container.SecurityContext.RunAsUser, detail)) + if runAsUser == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("runAsUser"), "")) return allErrs } - if !s.isValidUID(*container.SecurityContext.RunAsUser) { - detail := fmt.Sprintf("UID on container %s does not match required range. Found %d, allowed: %v", - container.Name, - *container.SecurityContext.RunAsUser, - s.opts.Ranges) - allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), *container.SecurityContext.RunAsUser, detail)) + if !s.isValidUID(*runAsUser) { + detail := fmt.Sprintf("must be in the ranges: %v", s.opts.Ranges) + allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *runAsUser, detail)) } return allErrs } diff --git a/pkg/security/podsecuritypolicy/user/mustrunas_test.go b/pkg/security/podsecuritypolicy/user/mustrunas_test.go index 02edf0e4a8..9121882063 100644 --- a/pkg/security/podsecuritypolicy/user/mustrunas_test.go +++ b/pkg/security/podsecuritypolicy/user/mustrunas_test.go @@ -98,19 +98,13 @@ func TestValidate(t *testing.T) { }, }, }, - "nil security context": { - container: &api.Container{ - SecurityContext: nil, - }, - expectedMsg: "unable to validate nil security context for container", - }, "nil run as user": { container: &api.Container{ SecurityContext: &api.SecurityContext{ RunAsUser: nil, }, }, - expectedMsg: "unable to validate nil RunAsUser for container", + expectedMsg: "runAsUser: Required", }, "invalid id": { container: &api.Container{ @@ -118,7 +112,7 @@ func TestValidate(t *testing.T) { RunAsUser: &invalidID, }, }, - expectedMsg: "does not match required range", + expectedMsg: "runAsUser: Invalid", }, } @@ -128,7 +122,7 @@ func TestValidate(t *testing.T) { t.Errorf("unexpected error initializing NewMustRunAs for testcase %s: %#v", name, err) continue } - errs := mustRunAs.Validate(nil, tc.container) + errs := mustRunAs.Validate(nil, nil, nil, tc.container.SecurityContext.RunAsNonRoot, tc.container.SecurityContext.RunAsUser) //should've passed but didn't if len(tc.expectedMsg) == 0 && len(errs) > 0 { t.Errorf("%s expected no errors but received %v", name, errs) diff --git a/pkg/security/podsecuritypolicy/user/nonroot.go b/pkg/security/podsecuritypolicy/user/nonroot.go index f53880a9a6..2a9624fc0b 100644 --- a/pkg/security/podsecuritypolicy/user/nonroot.go +++ b/pkg/security/podsecuritypolicy/user/nonroot.go @@ -17,8 +17,6 @@ limitations under the License. package user import ( - "fmt" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" @@ -43,22 +41,18 @@ func (s *nonRoot) Generate(pod *api.Pod, container *api.Container) (*int64, erro // or if the UID is set it is not root. Validation will fail if RunAsNonRoot is set to false. // In order to work properly this assumes that the kubelet performs a final check on runAsUser // or the image UID when runAsUser is nil. -func (s *nonRoot) Validate(pod *api.Pod, container *api.Container) field.ErrorList { +func (s *nonRoot) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList { allErrs := field.ErrorList{} - securityContextPath := field.NewPath("securityContext") - if container.SecurityContext == nil { - detail := fmt.Sprintf("unable to validate nil security context for container %s", container.Name) - allErrs = append(allErrs, field.Invalid(securityContextPath, container.SecurityContext, detail)) + if runAsNonRoot == nil && runAsUser == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("runAsNonRoot"), "must be true")) return allErrs } - if container.SecurityContext.RunAsNonRoot != nil && *container.SecurityContext.RunAsNonRoot == false { - detail := fmt.Sprintf("RunAsNonRoot must be true for container %s", container.Name) - allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsNonRoot"), *container.SecurityContext.RunAsNonRoot, detail)) + if runAsNonRoot != nil && *runAsNonRoot == false { + allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsNonRoot"), *runAsNonRoot, "must be true")) return allErrs } - if container.SecurityContext.RunAsUser != nil && *container.SecurityContext.RunAsUser == 0 { - detail := fmt.Sprintf("running with the root UID is forbidden by the pod security policy for container %s", container.Name) - allErrs = append(allErrs, field.Invalid(securityContextPath.Child("runAsUser"), *container.SecurityContext.RunAsUser, detail)) + if runAsUser != nil && *runAsUser == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("runAsUser"), *runAsUser, "running with the root UID is forbidden")) return allErrs } return allErrs diff --git a/pkg/security/podsecuritypolicy/user/nonroot_test.go b/pkg/security/podsecuritypolicy/user/nonroot_test.go index d2ec55ae06..7822ccf126 100644 --- a/pkg/security/podsecuritypolicy/user/nonroot_test.go +++ b/pkg/security/podsecuritypolicy/user/nonroot_test.go @@ -102,17 +102,17 @@ func TestNonRootValidate(t *testing.T) { { container: &api.Container{ SecurityContext: &api.SecurityContext{ - RunAsNonRoot: &unfalse, - RunAsUser: &badUID, + RunAsNonRoot: nil, + RunAsUser: nil, }, }, expectedErr: true, - msg: "in test case %d, expected errors from root uid but got %v", + msg: "in test case %d, expected errors from nil runAsNonRoot and nil runAsUser but got %v", }, } for i, tc := range tests { - errs := s.Validate(nil, tc.container) + errs := s.Validate(nil, nil, nil, tc.container.SecurityContext.RunAsNonRoot, tc.container.SecurityContext.RunAsUser) if (len(errs) == 0) == tc.expectedErr { t.Errorf(tc.msg, i, errs) } diff --git a/pkg/security/podsecuritypolicy/user/runasany.go b/pkg/security/podsecuritypolicy/user/runasany.go index ffee679320..729201bf64 100644 --- a/pkg/security/podsecuritypolicy/user/runasany.go +++ b/pkg/security/podsecuritypolicy/user/runasany.go @@ -38,6 +38,6 @@ func (s *runAsAny) Generate(pod *api.Pod, container *api.Container) (*int64, err } // Validate ensures that the specified values fall within the range of the strategy. -func (s *runAsAny) Validate(pod *api.Pod, container *api.Container) field.ErrorList { +func (s *runAsAny) Validate(fldPath *field.Path, _ *api.Pod, _ *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList { return field.ErrorList{} } diff --git a/pkg/security/podsecuritypolicy/user/runasany_test.go b/pkg/security/podsecuritypolicy/user/runasany_test.go index d2a2915bb7..9a3181b486 100644 --- a/pkg/security/podsecuritypolicy/user/runasany_test.go +++ b/pkg/security/podsecuritypolicy/user/runasany_test.go @@ -52,7 +52,7 @@ func TestRunAsAnyValidate(t *testing.T) { if err != nil { t.Fatalf("unexpected error initializing NewRunAsAny %v", err) } - errs := s.Validate(nil, nil) + errs := s.Validate(nil, nil, nil, nil, nil) if len(errs) != 0 { t.Errorf("unexpected errors validating with ") } diff --git a/pkg/security/podsecuritypolicy/user/types.go b/pkg/security/podsecuritypolicy/user/types.go index 8e754c32f6..8df0c766d5 100644 --- a/pkg/security/podsecuritypolicy/user/types.go +++ b/pkg/security/podsecuritypolicy/user/types.go @@ -26,5 +26,5 @@ type RunAsUserStrategy interface { // Generate creates the uid based on policy rules. Generate(pod *api.Pod, container *api.Container) (*int64, error) // Validate ensures that the specified values fall within the range of the strategy. - Validate(pod *api.Pod, container *api.Container) field.ErrorList + Validate(fldPath *field.Path, pod *api.Pod, container *api.Container, runAsNonRoot *bool, runAsUser *int64) field.ErrorList }