diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go index b578fde98a0..09ca844ed83 100644 --- a/pkg/security/podsecuritypolicy/provider.go +++ b/pkg/security/podsecuritypolicy/provider.go @@ -209,14 +209,7 @@ func (s *simpleProvider) ValidatePodSecurityContext(pod *api.Pod, fldPath *field allErrs = append(allErrs, s.strategies.SupplementalGroupStrategy.Validate(pod, pod.Spec.SecurityContext.SupplementalGroups)...) allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidatePod(pod)...) - // make a dummy container context to reuse the selinux strategies - container := &api.Container{ - Name: pod.Name, - SecurityContext: &api.SecurityContext{ - SELinuxOptions: pod.Spec.SecurityContext.SELinuxOptions, - }, - } - allErrs = append(allErrs, s.strategies.SELinuxStrategy.Validate(pod, container)...) + allErrs = append(allErrs, s.strategies.SELinuxStrategy.Validate(fldPath.Child("seLinuxOptions"), pod, nil, pod.Spec.SecurityContext.SELinuxOptions)...) if !s.psp.Spec.HostNetwork && pod.Spec.SecurityContext.HostNetwork { allErrs = append(allErrs, field.Invalid(fldPath.Child("hostNetwork"), pod.Spec.SecurityContext.HostNetwork, "Host network is not allowed to be used")) @@ -275,7 +268,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.SELinuxStrategy.Validate(pod, container)...) + 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 48846848b75..7c7cca7e4aa 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -323,12 +323,12 @@ func TestValidatePodSecurityContextFailures(t *testing.T) { "failNilSELinux": { pod: failNilSELinuxPod, psp: failSELinuxPSP, - expectedError: "unable to validate nil seLinuxOptions", + expectedError: "seLinuxOptions: Required", }, "failInvalidSELinux": { pod: failInvalidSELinuxPod, psp: failSELinuxPSP, - expectedError: "does not match required level. Found bar, wanted foo", + expectedError: "seLinuxOptions.level: Invalid value", }, "failHostDirPSP": { pod: failHostDirPod, @@ -460,7 +460,7 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) { "failSELinuxPSP": { pod: failSELinuxPod, psp: failSELinuxPSP, - expectedError: "does not match required level", + expectedError: "seLinuxOptions.level: Invalid value", }, "failNilAppArmor": { pod: failNilAppArmorPod, diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas.go b/pkg/security/podsecuritypolicy/selinux/mustrunas.go index 684f7bb9669..4f13272bda1 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas.go +++ b/pkg/security/podsecuritypolicy/selinux/mustrunas.go @@ -43,41 +43,33 @@ func NewMustRunAs(options *extensions.SELinuxStrategyOptions) (SELinuxStrategy, } // Generate creates the SELinuxOptions based on constraint rules. -func (s *mustRunAs) Generate(pod *api.Pod, container *api.Container) (*api.SELinuxOptions, error) { +func (s *mustRunAs) Generate(_ *api.Pod, _ *api.Container) (*api.SELinuxOptions, error) { return s.opts.SELinuxOptions, nil } // 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, seLinux *api.SELinuxOptions) field.ErrorList { allErrs := field.ErrorList{} - if container.SecurityContext == nil { - detail := fmt.Sprintf("unable to validate nil security context for %s", container.Name) - allErrs = append(allErrs, field.Invalid(field.NewPath("securityContext"), container.SecurityContext, detail)) + if seLinux == nil { + allErrs = append(allErrs, field.Required(fldPath, "")) return allErrs } - if container.SecurityContext.SELinuxOptions == nil { - detail := fmt.Sprintf("unable to validate nil seLinuxOptions for %s", container.Name) - allErrs = append(allErrs, field.Invalid(field.NewPath("seLinuxOptions"), container.SecurityContext.SELinuxOptions, detail)) - return allErrs - } - seLinuxOptionsPath := field.NewPath("seLinuxOptions") - seLinux := container.SecurityContext.SELinuxOptions if seLinux.Level != s.opts.SELinuxOptions.Level { - detail := fmt.Sprintf("seLinuxOptions.level on %s does not match required level. Found %s, wanted %s", container.Name, seLinux.Level, s.opts.SELinuxOptions.Level) - allErrs = append(allErrs, field.Invalid(seLinuxOptionsPath.Child("level"), seLinux.Level, detail)) + detail := fmt.Sprintf("must be %s", s.opts.SELinuxOptions.Level) + allErrs = append(allErrs, field.Invalid(fldPath.Child("level"), seLinux.Level, detail)) } if seLinux.Role != s.opts.SELinuxOptions.Role { - detail := fmt.Sprintf("seLinuxOptions.role on %s does not match required role. Found %s, wanted %s", container.Name, seLinux.Role, s.opts.SELinuxOptions.Role) - allErrs = append(allErrs, field.Invalid(seLinuxOptionsPath.Child("role"), seLinux.Role, detail)) + detail := fmt.Sprintf("must be %s", s.opts.SELinuxOptions.Role) + allErrs = append(allErrs, field.Invalid(fldPath.Child("role"), seLinux.Role, detail)) } if seLinux.Type != s.opts.SELinuxOptions.Type { - detail := fmt.Sprintf("seLinuxOptions.type on %s does not match required type. Found %s, wanted %s", container.Name, seLinux.Type, s.opts.SELinuxOptions.Type) - allErrs = append(allErrs, field.Invalid(seLinuxOptionsPath.Child("type"), seLinux.Type, detail)) + detail := fmt.Sprintf("must be %s", s.opts.SELinuxOptions.Type) + allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), seLinux.Type, detail)) } if seLinux.User != s.opts.SELinuxOptions.User { - detail := fmt.Sprintf("seLinuxOptions.user on %s does not match required user. Found %s, wanted %s", container.Name, seLinux.User, s.opts.SELinuxOptions.User) - allErrs = append(allErrs, field.Invalid(seLinuxOptionsPath.Child("user"), seLinux.User, detail)) + detail := fmt.Sprintf("must be %s", s.opts.SELinuxOptions.User) + allErrs = append(allErrs, field.Invalid(fldPath.Child("user"), seLinux.User, detail)) } return allErrs diff --git a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go index b7a48bf626d..bd57b8a9892 100644 --- a/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go +++ b/pkg/security/podsecuritypolicy/selinux/mustrunas_test.go @@ -100,19 +100,19 @@ func TestMustRunAsValidate(t *testing.T) { }{ "invalid role": { seLinux: role, - expectedMsg: "does not match required role", + expectedMsg: "role: Invalid value", }, "invalid user": { seLinux: user, - expectedMsg: "does not match required user", + expectedMsg: "user: Invalid value", }, "invalid level": { seLinux: level, - expectedMsg: "does not match required level", + expectedMsg: "level: Invalid value", }, "invalid type": { seLinux: seType, - expectedMsg: "does not match required type", + expectedMsg: "type: Invalid value", }, "valid": { seLinux: newValidOpts(), @@ -130,13 +130,8 @@ func TestMustRunAsValidate(t *testing.T) { t.Errorf("unexpected error initializing NewMustRunAs for testcase %s: %#v", name, err) continue } - container := &api.Container{ - SecurityContext: &api.SecurityContext{ - SELinuxOptions: tc.seLinux, - }, - } - errs := mustRunAs.Validate(nil, container) + errs := mustRunAs.Validate(nil, nil, nil, tc.seLinux) //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/selinux/runasany.go b/pkg/security/podsecuritypolicy/selinux/runasany.go index 711336fc595..62fd9083b12 100644 --- a/pkg/security/podsecuritypolicy/selinux/runasany.go +++ b/pkg/security/podsecuritypolicy/selinux/runasany.go @@ -38,6 +38,6 @@ func (s *runAsAny) Generate(pod *api.Pod, container *api.Container) (*api.SELinu } // 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, options *api.SELinuxOptions) field.ErrorList { return field.ErrorList{} } diff --git a/pkg/security/podsecuritypolicy/selinux/runasany_test.go b/pkg/security/podsecuritypolicy/selinux/runasany_test.go index 0eab0ad81b8..d31550034b6 100644 --- a/pkg/security/podsecuritypolicy/selinux/runasany_test.go +++ b/pkg/security/podsecuritypolicy/selinux/runasany_test.go @@ -58,7 +58,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) if len(errs) != 0 { t.Errorf("unexpected errors validating with ") } @@ -66,7 +66,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) if len(errs) != 0 { t.Errorf("unexpected errors validating %v", errs) } diff --git a/pkg/security/podsecuritypolicy/selinux/types.go b/pkg/security/podsecuritypolicy/selinux/types.go index 043a3efe020..8f312e64cbc 100644 --- a/pkg/security/podsecuritypolicy/selinux/types.go +++ b/pkg/security/podsecuritypolicy/selinux/types.go @@ -26,5 +26,5 @@ type SELinuxStrategy interface { // Generate creates the SELinuxOptions based on constraint rules. Generate(pod *api.Pod, container *api.Container) (*api.SELinuxOptions, 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, options *api.SELinuxOptions) field.ErrorList }