Merge pull request #52849 from liggitt/psp-defaulting-order

pkg/registry/rbac/helpers.go

@@ -17,6 +17,8 @@ limitations under the License.
 package rbac
 
 import (
+	"reflect"
+
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/conversion"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -25,6 +27,10 @@ import (
 // IsOnlyMutatingGCFields checks finalizers and ownerrefs which GC manipulates
 // and indicates that only those fields are changing
 func IsOnlyMutatingGCFields(obj, old runtime.Object, equalities conversion.Equalities) bool {
+	if old == nil || reflect.ValueOf(old).IsNil() {
+		return false
+	}
+
 	// make a copy of the newObj so that we can stomp for comparison
 	copied := obj.DeepCopyObject()
 	copiedMeta, err := meta.Accessor(copied)

pkg/registry/rbac/helpers_test.go

@@ -128,6 +128,19 @@ func TestIsOnlyMutatingGCFields(t *testing.T) {
 			},
 			expected: false,
 		},
+		{
+			name: "and nil",
+			obj: func() runtime.Object {
+				obj := newPod()
+				obj.OwnerReferences = append(obj.OwnerReferences, metav1.OwnerReference{Name: "foo"})
+				obj.Spec.RestartPolicy = kapi.RestartPolicyAlways
+				return obj
+			},
+			old: func() runtime.Object {
+				return (*kapi.Pod)(nil)
+			},
+			expected: false,
+		},
 	}
 
 	for _, tc := range tests {

pkg/security/podsecuritypolicy/BUILD

@@ -26,6 +26,7 @@ go_library(
         "//pkg/security/podsecuritypolicy/sysctl:go_default_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/capabilities/mustrunas.go

@@ -48,13 +48,17 @@ func NewDefaultCapabilities(defaultAddCapabilities, requiredDropCapabilities, al
 // 1.  a capabilities.Add set containing all the required adds (unless the
 // 		container specifically is dropping the cap) and container requested adds
 // 2.  a capabilities.Drop set containing all the required drops and container requested drops
+//
+// Returns the original container capabilities if no changes are required.
 func (s *defaultCapabilities) Generate(pod *api.Pod, container *api.Container) (*api.Capabilities, error) {
 	defaultAdd := makeCapSet(s.defaultAddCapabilities)
 	requiredDrop := makeCapSet(s.requiredDropCapabilities)
 	containerAdd := sets.NewString()
 	containerDrop := sets.NewString()
 
+	var containerCapabilities *api.Capabilities
 	if container.SecurityContext != nil && container.SecurityContext.Capabilities != nil {
+		containerCapabilities = container.SecurityContext.Capabilities
 		containerAdd = makeCapSet(container.SecurityContext.Capabilities.Add)
 		containerDrop = makeCapSet(container.SecurityContext.Capabilities.Drop)
 	}
@@ -62,32 +66,25 @@ func (s *defaultCapabilities) Generate(pod *api.Pod, container *api.Container) (
 	// remove any default adds that the container is specifically dropping
 	defaultAdd = defaultAdd.Difference(containerDrop)
 
-	combinedAdd := defaultAdd.Union(containerAdd).List()
-	combinedDrop := requiredDrop.Union(containerDrop).List()
+	combinedAdd := defaultAdd.Union(containerAdd)
+	combinedDrop := requiredDrop.Union(containerDrop)
 
-	// nothing generated?  return nil
-	if len(combinedAdd) == 0 && len(combinedDrop) == 0 {
-		return nil, nil
+	// no changes? return the original capabilities
+	if (len(combinedAdd) == len(containerAdd)) && (len(combinedDrop) == len(containerDrop)) {
+		return containerCapabilities, nil
 	}
 
 	return &api.Capabilities{
-		Add:  capabilityFromStringSlice(combinedAdd),
-		Drop: capabilityFromStringSlice(combinedDrop),
+		Add:  capabilityFromStringSlice(combinedAdd.List()),
+		Drop: capabilityFromStringSlice(combinedDrop.List()),
 	}, nil
 }
 
 // Validate ensures that the specified values fall within the range of the strategy.
-func (s *defaultCapabilities) Validate(pod *api.Pod, container *api.Container) field.ErrorList {
+func (s *defaultCapabilities) Validate(pod *api.Pod, container *api.Container, capabilities *api.Capabilities) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	// if the security context isn't set then we haven't generated correctly.  Shouldn't get here
-	// if using the provider correctly
-	if container.SecurityContext == nil {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("securityContext"), container.SecurityContext, "no security context is set"))
-		return allErrs
-	}
-
-	if container.SecurityContext.Capabilities == nil {
+	if capabilities == nil {
 		// if container.SC.Caps is nil then nothing was defaulted by the strategy or requested by the pod author
 		// if there are no required caps on the strategy and nothing is requested on the pod
 		// then we can safely return here without further validation.
@@ -97,7 +94,7 @@ func (s *defaultCapabilities) Validate(pod *api.Pod, container *api.Container) f
 
 		// container has no requested caps but we have required caps.  We should have something in
 		// at least the drops on the container.
-		allErrs = append(allErrs, field.Invalid(field.NewPath("capabilities"), container.SecurityContext.Capabilities,
+		allErrs = append(allErrs, field.Invalid(field.NewPath("capabilities"), capabilities,
 			"required capabilities are not set on the securityContext"))
 		return allErrs
 	}
@@ -112,7 +109,7 @@ func (s *defaultCapabilities) Validate(pod *api.Pod, container *api.Container) f
 	// validate that anything being added is in the default or allowed sets
 	defaultAdd := makeCapSet(s.defaultAddCapabilities)
 
-	for _, cap := range container.SecurityContext.Capabilities.Add {
+	for _, cap := range capabilities.Add {
 		sCap := string(cap)
 		if !defaultAdd.Has(sCap) && !allowedAdd.Has(sCap) {
 			allErrs = append(allErrs, field.Invalid(field.NewPath("capabilities", "add"), sCap, "capability may not be added"))
@@ -120,12 +117,12 @@ func (s *defaultCapabilities) Validate(pod *api.Pod, container *api.Container) f
 	}
 
 	// validate that anything that is required to be dropped is in the drop set
-	containerDrops := makeCapSet(container.SecurityContext.Capabilities.Drop)
+	containerDrops := makeCapSet(capabilities.Drop)
 
 	for _, requiredDrop := range s.requiredDropCapabilities {
 		sDrop := string(requiredDrop)
 		if !containerDrops.Has(sDrop) {
-			allErrs = append(allErrs, field.Invalid(field.NewPath("capabilities", "drop"), container.SecurityContext.Capabilities.Drop,
+			allErrs = append(allErrs, field.Invalid(field.NewPath("capabilities", "drop"), capabilities.Drop,
 				fmt.Sprintf("%s is required to be dropped but was not found", sDrop)))
 		}
 	}

pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go

@@ -31,6 +31,10 @@ func TestGenerateAdds(t *testing.T) {
 		expectedCaps   *api.Capabilities
 	}{
 		"no required, no container requests": {},
+		"no required, no container requests, non-nil": {
+			containerCaps: &api.Capabilities{},
+			expectedCaps:  &api.Capabilities{},
+		},
 		"required, no container requests": {
 			defaultAddCaps: []api.Capability{"foo"},
 			expectedCaps: &api.Capabilities{
@@ -64,13 +68,22 @@ func TestGenerateAdds(t *testing.T) {
 				Add: []api.Capability{"bar", "foo"},
 			},
 		},
+		"generation does not mutate unnecessarily": {
+			defaultAddCaps: []api.Capability{"foo", "bar"},
+			containerCaps: &api.Capabilities{
+				Add: []api.Capability{"foo", "foo", "bar", "baz"},
+			},
+			expectedCaps: &api.Capabilities{
+				Add: []api.Capability{"foo", "foo", "bar", "baz"},
+			},
+		},
 		"generation dedupes": {
-			defaultAddCaps: []api.Capability{"foo", "foo", "foo", "foo"},
+			defaultAddCaps: []api.Capability{"foo", "bar"},
 			containerCaps: &api.Capabilities{
-				Add: []api.Capability{"foo", "foo", "foo"},
+				Add: []api.Capability{"foo", "baz"},
 			},
 			expectedCaps: &api.Capabilities{
-				Add: []api.Capability{"foo"},
+				Add: []api.Capability{"bar", "baz", "foo"},
 			},
 		},
 		"generation is case sensitive - will not dedupe": {
@@ -121,6 +134,10 @@ func TestGenerateDrops(t *testing.T) {
 		"no required, no container requests": {
 			expectedCaps: nil,
 		},
+		"no required, no container requests, non-nil": {
+			containerCaps: &api.Capabilities{},
+			expectedCaps:  &api.Capabilities{},
+		},
 		"required drops are defaulted": {
 			requiredDropCaps: []api.Capability{"foo"},
 			expectedCaps: &api.Capabilities{
@@ -128,12 +145,21 @@ func TestGenerateDrops(t *testing.T) {
 			},
 		},
 		"required drops are defaulted when making container requests": {
-			requiredDropCaps: []api.Capability{"foo"},
+			requiredDropCaps: []api.Capability{"baz"},
 			containerCaps: &api.Capabilities{
 				Drop: []api.Capability{"foo", "bar"},
 			},
 			expectedCaps: &api.Capabilities{
-				Drop: []api.Capability{"bar", "foo"},
+				Drop: []api.Capability{"bar", "baz", "foo"},
+			},
+		},
+		"required drops do not mutate unnecessarily": {
+			requiredDropCaps: []api.Capability{"baz"},
+			containerCaps: &api.Capabilities{
+				Drop: []api.Capability{"foo", "bar", "baz"},
+			},
+			expectedCaps: &api.Capabilities{
+				Drop: []api.Capability{"foo", "bar", "baz"},
 			},
 		},
 		"can drop a required add": {
@@ -167,12 +193,12 @@ func TestGenerateDrops(t *testing.T) {
 			},
 		},
 		"generation dedupes": {
-			requiredDropCaps: []api.Capability{"bar", "bar", "bar", "bar"},
+			requiredDropCaps: []api.Capability{"baz", "foo"},
 			containerCaps: &api.Capabilities{
-				Drop: []api.Capability{"bar", "bar", "bar"},
+				Drop: []api.Capability{"bar", "foo"},
 			},
 			expectedCaps: &api.Capabilities{
-				Drop: []api.Capability{"bar"},
+				Drop: []api.Capability{"bar", "baz", "foo"},
 			},
 		},
 		"generation is case sensitive - will not dedupe": {
@@ -298,18 +324,12 @@ func TestValidateAdds(t *testing.T) {
 	}
 
 	for k, v := range tests {
-		container := &api.Container{
-			SecurityContext: &api.SecurityContext{
-				Capabilities: v.containerCaps,
-			},
-		}
-
 		strategy, err := NewDefaultCapabilities(v.defaultAddCaps, nil, v.allowedCaps)
 		if err != nil {
 			t.Errorf("%s failed: %v", k, err)
 			continue
 		}
-		errs := strategy.Validate(nil, container)
+		errs := strategy.Validate(nil, nil, v.containerCaps)
 		if v.expectedError == "" && len(errs) > 0 {
 			t.Errorf("%s should have passed but had errors %v", k, errs)
 			continue
@@ -365,18 +385,12 @@ func TestValidateDrops(t *testing.T) {
 	}
 
 	for k, v := range tests {
-		container := &api.Container{
-			SecurityContext: &api.SecurityContext{
-				Capabilities: v.containerCaps,
-			},
-		}
-
 		strategy, err := NewDefaultCapabilities(nil, v.requiredDropCaps, nil)
 		if err != nil {
 			t.Errorf("%s failed: %v", k, err)
 			continue
 		}
-		errs := strategy.Validate(nil, container)
+		errs := strategy.Validate(nil, nil, v.containerCaps)
 		if v.expectedError == "" && len(errs) > 0 {
 			t.Errorf("%s should have passed but had errors %v", k, errs)
 			continue

pkg/security/podsecuritypolicy/capabilities/types.go

@@ -26,5 +26,5 @@ type Strategy interface {
 	// Generate creates the capabilities based on policy rules.
 	Generate(pod *api.Pod, container *api.Container) (*api.Capabilities, error)
 	// Validate ensures that the specified values fall within the range of the strategy.
-	Validate(pod *api.Pod, container *api.Container) field.ErrorList
+	Validate(pod *api.Pod, container *api.Container, capabilities *api.Capabilities) field.ErrorList
 }

pkg/security/podsecuritypolicy/group/mustrunas.go

@@ -46,13 +46,13 @@ func NewMustRunAs(ranges []extensions.GroupIDRange, field string) (GroupStrategy
 
 // Generate creates the group based on policy rules.  By default this returns the first group of the
 // first range (min val).
-func (s *mustRunAs) Generate(pod *api.Pod) ([]int64, error) {
+func (s *mustRunAs) Generate(_ *api.Pod) ([]int64, error) {
 	return []int64{s.ranges[0].Min}, nil
 }
 
 // Generate a single value to be applied.  This is used for FSGroup.  This strategy will return
 // the first group of the first range (min val).
-func (s *mustRunAs) GenerateSingle(pod *api.Pod) (*int64, error) {
+func (s *mustRunAs) GenerateSingle(_ *api.Pod) (*int64, error) {
 	single := new(int64)
 	*single = s.ranges[0].Min
 	return single, nil
@@ -61,14 +61,9 @@ func (s *mustRunAs) GenerateSingle(pod *api.Pod) (*int64, error) {
 // Validate ensures that the specified values fall within the range of the strategy.
 // Groups are passed in here to allow this strategy to support multiple group fields (fsgroup and
 // supplemental groups).
-func (s *mustRunAs) Validate(pod *api.Pod, groups []int64) field.ErrorList {
+func (s *mustRunAs) Validate(_ *api.Pod, groups []int64) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if pod.Spec.SecurityContext == nil {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("securityContext"), pod.Spec.SecurityContext, "unable to validate nil security context"))
-		return allErrs
-	}
-
 	if len(groups) == 0 && len(s.ranges) > 0 {
 		allErrs = append(allErrs, field.Invalid(field.NewPath(s.field), groups, "unable to validate empty groups against required ranges"))
 	}

pkg/security/podsecuritypolicy/group/mustrunas_test.go

@@ -109,14 +109,6 @@ func TestGenerate(t *testing.T) {
 }
 
 func TestValidate(t *testing.T) {
-	validPod := func() *api.Pod {
-		return &api.Pod{
-			Spec: api.PodSpec{
-				SecurityContext: &api.PodSecurityContext{},
-			},
-		}
-	}
-
 	tests := map[string]struct {
 		ranges []extensions.GroupIDRange
 		pod    *api.Pod
@@ -124,19 +116,16 @@ func TestValidate(t *testing.T) {
 		pass   bool
 	}{
 		"nil security context": {
-			pod: &api.Pod{},
 			ranges: []extensions.GroupIDRange{
 				{Min: 1, Max: 3},
 			},
 		},
 		"empty groups": {
-			pod: validPod(),
 			ranges: []extensions.GroupIDRange{
 				{Min: 1, Max: 3},
 			},
 		},
 		"not in range": {
-			pod:    validPod(),
 			groups: []int64{5},
 			ranges: []extensions.GroupIDRange{
 				{Min: 1, Max: 3},
@@ -144,7 +133,6 @@ func TestValidate(t *testing.T) {
 			},
 		},
 		"in range 1": {
-			pod:    validPod(),
 			groups: []int64{2},
 			ranges: []extensions.GroupIDRange{
 				{Min: 1, Max: 3},
@@ -152,7 +140,6 @@ func TestValidate(t *testing.T) {
 			pass: true,
 		},
 		"in range boundry min": {
-			pod:    validPod(),
 			groups: []int64{1},
 			ranges: []extensions.GroupIDRange{
 				{Min: 1, Max: 3},
@@ -160,7 +147,6 @@ func TestValidate(t *testing.T) {
 			pass: true,
 		},
 		"in range boundry max": {
-			pod:    validPod(),
 			groups: []int64{3},
 			ranges: []extensions.GroupIDRange{
 				{Min: 1, Max: 3},
@@ -168,7 +154,6 @@ func TestValidate(t *testing.T) {
 			pass: true,
 		},
 		"singular range": {
-			pod:    validPod(),
 			groups: []int64{4},
 			ranges: []extensions.GroupIDRange{
 				{Min: 4, Max: 4},
@@ -182,7 +167,7 @@ func TestValidate(t *testing.T) {
 		if err != nil {
 			t.Errorf("error creating strategy for %s: %v", k, err)
 		}
-		errs := s.Validate(v.pod, v.groups)
+		errs := s.Validate(nil, v.groups)
 		if v.pass && len(errs) > 0 {
 			t.Errorf("unexpected errors for %s: %v", k, errs)
 		}

pkg/security/podsecuritypolicy/group/runasany.go

@@ -33,17 +33,17 @@ func NewRunAsAny() (GroupStrategy, error) {
 }
 
 // Generate creates the group based on policy rules.  This strategy returns an empty slice.
-func (s *runAsAny) Generate(pod *api.Pod) ([]int64, error) {
-	return []int64{}, nil
+func (s *runAsAny) Generate(_ *api.Pod) ([]int64, error) {
+	return nil, nil
 }
 
 // Generate a single value to be applied.  This is used for FSGroup.  This strategy returns nil.
-func (s *runAsAny) GenerateSingle(pod *api.Pod) (*int64, error) {
+func (s *runAsAny) GenerateSingle(_ *api.Pod) (*int64, error) {
 	return nil, nil
 }
 
 // Validate ensures that the specified values fall within the range of the strategy.
-func (s *runAsAny) Validate(pod *api.Pod, groups []int64) field.ErrorList {
+func (s *runAsAny) Validate(_ *api.Pod, groups []int64) field.ErrorList {
 	return field.ErrorList{}
 
 }

pkg/security/podsecuritypolicy/provider_test.go

@@ -55,30 +55,21 @@ func TestCreatePodSecurityContextNonmutating(t *testing.T) {
 				Name: "psp-sa",
 				Annotations: map[string]string{
 					seccomp.AllowedProfilesAnnotationKey: "*",
-					seccomp.DefaultProfileAnnotationKey:  "foo",
 				},
 			},
 			Spec: extensions.PodSecurityPolicySpec{
-				DefaultAddCapabilities:   []api.Capability{"foo"},
-				RequiredDropCapabilities: []api.Capability{"bar"},
+				AllowPrivilegeEscalation: true,
 				RunAsUser: extensions.RunAsUserStrategyOptions{
 					Rule: extensions.RunAsUserStrategyRunAsAny,
 				},
 				SELinux: extensions.SELinuxStrategyOptions{
 					Rule: extensions.SELinuxStrategyRunAsAny,
 				},
-				// these are pod mutating strategies that are tested above
 				FSGroup: extensions.FSGroupStrategyOptions{
-					Rule: extensions.FSGroupStrategyMustRunAs,
-					Ranges: []extensions.GroupIDRange{
-						{Min: 1, Max: 1},
-					},
+					Rule: extensions.FSGroupStrategyRunAsAny,
 				},
 				SupplementalGroups: extensions.SupplementalGroupsStrategyOptions{
-					Rule: extensions.SupplementalGroupsStrategyMustRunAs,
-					Ranges: []extensions.GroupIDRange{
-						{Min: 1, Max: 1},
-					},
+					Rule: extensions.SupplementalGroupsStrategyRunAsAny,
 				},
 			},
 		}
@@ -91,17 +82,13 @@ func TestCreatePodSecurityContextNonmutating(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unable to create provider %v", err)
 	}
-	sc, _, err := provider.CreatePodSecurityContext(pod)
+	_, _, err = provider.CreatePodSecurityContext(pod)
 	if err != nil {
 		t.Fatalf("unable to create psc %v", err)
 	}
 
-	// The generated security context should have filled in missing options, so they should differ
-	if reflect.DeepEqual(sc, &pod.Spec.SecurityContext) {
-		t.Error("expected created security context to be different than container's, but they were identical")
-	}
-
 	// Creating the provider or the security context should not have mutated the psp or pod
+	// 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)
@@ -134,7 +121,6 @@ func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
 
 		// Create a PSP with strategies that will populate a blank security context
 		createPSP := func() *extensions.PodSecurityPolicy {
-			uid := int64(1)
 			return &extensions.PodSecurityPolicy{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "psp-sa",
@@ -144,25 +130,19 @@ func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
 					},
 				},
 				Spec: extensions.PodSecurityPolicySpec{
-					DefaultAddCapabilities:   []api.Capability{"foo"},
-					RequiredDropCapabilities: []api.Capability{"bar"},
+					AllowPrivilegeEscalation: true,
 					RunAsUser: extensions.RunAsUserStrategyOptions{
-						Rule:   extensions.RunAsUserStrategyMustRunAs,
-						Ranges: []extensions.UserIDRange{{Min: uid, Max: uid}},
+						Rule: extensions.RunAsUserStrategyRunAsAny,
 					},
 					SELinux: extensions.SELinuxStrategyOptions{
-						Rule:           extensions.SELinuxStrategyMustRunAs,
-						SELinuxOptions: &api.SELinuxOptions{User: "you"},
+						Rule: extensions.SELinuxStrategyRunAsAny,
 					},
-					// these are pod mutating strategies that are tested above
 					FSGroup: extensions.FSGroupStrategyOptions{
 						Rule: extensions.FSGroupStrategyRunAsAny,
 					},
 					SupplementalGroups: extensions.SupplementalGroupsStrategyOptions{
 						Rule: extensions.SupplementalGroupsStrategyRunAsAny,
 					},
-					// mutates the container SC by defaulting to true if container sets nil
-					ReadOnlyRootFilesystem: true,
 				},
 			}
 		}
@@ -174,17 +154,13 @@ func TestCreateContainerSecurityContextNonmutating(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unable to create provider %v", err)
 		}
-		sc, _, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[0])
+		_, _, err = provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[0])
 		if err != nil {
 			t.Fatalf("unable to create container security context %v", err)
 		}
 
-		// The generated security context should have filled in missing options, so they should differ
-		if reflect.DeepEqual(sc, &pod.Spec.Containers[0].SecurityContext) {
-			t.Error("expected created security context to be different than container's, but they were identical")
-		}
-
 		// Creating the provider or the security context should not have mutated the psp or pod
+		// 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)
@@ -323,12 +299,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,
@@ -455,12 +431,12 @@ func TestValidateContainerSecurityContextFailures(t *testing.T) {
 		"failUserPSP": {
 			pod:           failUserPod,
 			psp:           failUserPSP,
-			expectedError: "does not match required range",
+			expectedError: "runAsUser: Invalid value",
 		},
 		"failSELinuxPSP": {
 			pod:           failSELinuxPod,
 			psp:           failSELinuxPSP,
-			expectedError: "does not match required level",
+			expectedError: "seLinuxOptions.level: Invalid value",
 		},
 		"failNilAppArmor": {
 			pod:           failNilAppArmorPod,

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

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)

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{}
 }

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)
 	}

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
 }

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
 }

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)

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

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)
 		}

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{}
 }

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 ")
 	}

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
 }

pkg/securitycontext/BUILD

@@ -9,6 +9,7 @@ load(
 go_library(
     name = "go_default_library",
     srcs = [
+        "accessors.go",
         "doc.go",
         "fake.go",
         "util.go",
@@ -22,10 +23,17 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["util_test.go"],
+    srcs = [
+        "accessors_test.go",
+        "util_test.go",
+    ],
     importpath = "k8s.io/kubernetes/pkg/securitycontext",
     library = ":go_default_library",
-    deps = ["//vendor/k8s.io/api/core/v1:go_default_library"],
+    deps = [
+        "//pkg/api:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
+    ],
 )
 
 filegroup(

plugin/pkg/admission/security/podsecuritypolicy/BUILD

@@ -16,12 +16,12 @@ go_library(
         "//pkg/client/informers/informers_generated/internalversion:go_default_library",
         "//pkg/client/listers/extensions/internalversion:go_default_library",
         "//pkg/kubeapiserver/admission:go_default_library",
+        "//pkg/registry/rbac:go_default_library",
         "//pkg/security/podsecuritypolicy:go_default_library",
         "//pkg/security/podsecuritypolicy/util:go_default_library",
-        "//pkg/securitycontext:go_default_library",
         "//pkg/serviceaccount:go_default_library",
-        "//pkg/util/maps:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
@@ -48,6 +48,8 @@ go_test(
         "//pkg/security/podsecuritypolicy/seccomp:go_default_library",
         "//pkg/security/podsecuritypolicy/util:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",