From b45b809f4cd0cd9d128cc7bcfec956b6c64e02a8 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Thu, 5 Oct 2017 15:55:53 -0400
Subject: [PATCH 01/12] PodSecurityPolicy: Do not mutate nil privileged field
 to false

---
 pkg/security/podsecuritypolicy/provider.go    |  7 +---
 .../podsecuritypolicy/admission_test.go       | 41 ++++++++++++++-----
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
index 1c608ef81da..c659addf4ec 100644
--- a/pkg/security/podsecuritypolicy/provider.go
+++ b/pkg/security/podsecuritypolicy/provider.go
@@ -157,11 +157,6 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 		return nil, nil, err
 	}
 
-	if sc.Privileged == nil {
-		priv := false
-		sc.Privileged = &priv
-	}
-
 	// if we're using the non-root strategy set the marker that this container should not be
 	// run as root which will signal to the kubelet to do a final check either on the runAsUser
 	// or, if runAsUser is not set, the image UID will be checked.
@@ -284,7 +279,7 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
 	allErrs = append(allErrs, s.strategies.AppArmorStrategy.Validate(pod, container)...)
 	allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidateContainer(pod, container)...)
 
-	if !s.psp.Spec.Privileged && *sc.Privileged {
+	if !s.psp.Spec.Privileged && sc.Privileged != nil && *sc.Privileged {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *sc.Privileged, "Privileged containers are not allowed"))
 	}
 
diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
index b88df3deefd..b6bafdbbb82 100644
--- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
+++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
@@ -204,37 +204,54 @@ func TestAdmitPrivileged(t *testing.T) {
 	privilegedPSP.Name = "priv"
 	privilegedPSP.Spec.Privileged = true
 
+	trueValue := true
+	falseValue := false
+
 	tests := map[string]struct {
 		pod          *kapi.Pod
 		psps         []*extensions.PodSecurityPolicy
 		shouldPass   bool
-		expectedPriv bool
+		expectedPriv *bool
 		expectedPSP  string
 	}{
-		"pod without priv request allowed under non priv PSP": {
+		"pod with priv=nil allowed under non priv PSP": {
 			pod:          goodPod(),
 			psps:         []*extensions.PodSecurityPolicy{nonPrivilegedPSP},
 			shouldPass:   true,
-			expectedPriv: false,
+			expectedPriv: nil,
 			expectedPSP:  nonPrivilegedPSP.Name,
 		},
-		"pod without priv request allowed under priv PSP": {
+		"pod with priv=nil allowed under priv PSP": {
 			pod:          goodPod(),
 			psps:         []*extensions.PodSecurityPolicy{privilegedPSP},
 			shouldPass:   true,
-			expectedPriv: false,
+			expectedPriv: nil,
 			expectedPSP:  privilegedPSP.Name,
 		},
-		"pod with priv request denied by non priv PSP": {
+		"pod with priv=false allowed under non priv PSP": {
+			pod:          createPodWithPriv(false),
+			psps:         []*extensions.PodSecurityPolicy{nonPrivilegedPSP},
+			shouldPass:   true,
+			expectedPriv: &falseValue,
+			expectedPSP:  nonPrivilegedPSP.Name,
+		},
+		"pod with priv=false allowed under priv PSP": {
+			pod:          createPodWithPriv(false),
+			psps:         []*extensions.PodSecurityPolicy{privilegedPSP},
+			shouldPass:   true,
+			expectedPriv: &falseValue,
+			expectedPSP:  privilegedPSP.Name,
+		},
+		"pod with priv=true denied by non priv PSP": {
 			pod:        createPodWithPriv(true),
 			psps:       []*extensions.PodSecurityPolicy{nonPrivilegedPSP},
 			shouldPass: false,
 		},
-		"pod with priv request allowed by priv PSP": {
+		"pod with priv=true allowed by priv PSP": {
 			pod:          createPodWithPriv(true),
 			psps:         []*extensions.PodSecurityPolicy{nonPrivilegedPSP, privilegedPSP},
 			shouldPass:   true,
-			expectedPriv: true,
+			expectedPriv: &trueValue,
 			expectedPSP:  privilegedPSP.Name,
 		},
 	}
@@ -243,9 +260,11 @@ func TestAdmitPrivileged(t *testing.T) {
 		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
 
 		if v.shouldPass {
-			if v.pod.Spec.Containers[0].SecurityContext.Privileged == nil ||
-				*v.pod.Spec.Containers[0].SecurityContext.Privileged != v.expectedPriv {
-				t.Errorf("%s expected privileged to be %t", k, v.expectedPriv)
+			priv := v.pod.Spec.Containers[0].SecurityContext.Privileged
+			if (priv == nil) != (v.expectedPriv == nil) {
+				t.Errorf("%s expected privileged to be %v, got %v", k, v.expectedPriv, priv)
+			} else if priv != nil && *priv != *v.expectedPriv {
+				t.Errorf("%s expected privileged to be %v, got %v", k, *v.expectedPriv, *priv)
 			}
 		}
 	}

From 59510caaf3a4af203a2c0ee4afae2052dcdbba0c Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Fri, 6 Oct 2017 17:08:24 -0400
Subject: [PATCH 02/12] PodSecurityPolicy: only set runAsNonRoot when runAsUser
 is nil

---
 pkg/security/podsecuritypolicy/provider.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
index c659addf4ec..ad43fa6d367 100644
--- a/pkg/security/podsecuritypolicy/provider.go
+++ b/pkg/security/podsecuritypolicy/provider.go
@@ -160,7 +160,7 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 	// if we're using the non-root strategy set the marker that this container should not be
 	// run as root which will signal to the kubelet to do a final check either on the runAsUser
 	// or, if runAsUser is not set, the image UID will be checked.
-	if sc.RunAsNonRoot == nil && s.psp.Spec.RunAsUser.Rule == extensions.RunAsUserStrategyMustRunAsNonRoot {
+	if sc.RunAsNonRoot == nil && sc.RunAsUser == nil && s.psp.Spec.RunAsUser.Rule == extensions.RunAsUserStrategyMustRunAsNonRoot {
 		nonRoot := true
 		sc.RunAsNonRoot = &nonRoot
 	}

From cfb490e3a199b48f88b325de6e49b721daa2a250 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Mon, 9 Oct 2017 17:30:13 -0400
Subject: [PATCH 03/12] PodSecurityPolicy: avoid unnecessary mutation of
 container capabilities

---
 .../capabilities/mustrunas.go                 | 18 +++++---
 .../capabilities/mustrunas_test.go            | 44 +++++++++++++++----
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/capabilities/mustrunas.go b/pkg/security/podsecuritypolicy/capabilities/mustrunas.go
index d8efe066db2..ed88883d8ad 100644
--- a/pkg/security/podsecuritypolicy/capabilities/mustrunas.go
+++ b/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,17 +66,17 @@ 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
 }
 
diff --git a/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go b/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go
index 7e9bfa51551..8f002d4008f 100644
--- a/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go
+++ b/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 dedupes": {
-			defaultAddCaps: []api.Capability{"foo", "foo", "foo", "foo"},
+		"generation does not mutate unnecessarily": {
+			defaultAddCaps: []api.Capability{"foo", "bar"},
 			containerCaps: &api.Capabilities{
-				Add: []api.Capability{"foo", "foo", "foo"},
+				Add: []api.Capability{"foo", "foo", "bar", "baz"},
 			},
 			expectedCaps: &api.Capabilities{
-				Add: []api.Capability{"foo"},
+				Add: []api.Capability{"foo", "foo", "bar", "baz"},
+			},
+		},
+		"generation dedupes": {
+			defaultAddCaps: []api.Capability{"foo", "bar"},
+			containerCaps: &api.Capabilities{
+				Add: []api.Capability{"foo", "baz"},
+			},
+			expectedCaps: &api.Capabilities{
+				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": {

From abc7c077e190e11ff93278a5f009a6512fc9e937 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Sun, 15 Oct 2017 22:48:36 -0400
Subject: [PATCH 04/12] PodSecurityPolicy: avoid unnecessary mutation of
 supplemental groups

---
 pkg/security/podsecuritypolicy/group/runasany.go                | 2 +-
 pkg/security/podsecuritypolicy/provider.go                      | 2 +-
 .../pkg/admission/security/podsecuritypolicy/admission_test.go  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/group/runasany.go b/pkg/security/podsecuritypolicy/group/runasany.go
index 0d3f1182e09..aff046d50b5 100644
--- a/pkg/security/podsecuritypolicy/group/runasany.go
+++ b/pkg/security/podsecuritypolicy/group/runasany.go
@@ -34,7 +34,7 @@ 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
+	return nil, nil
 }
 
 // Generate a single value to be applied.  This is used for FSGroup.  This strategy returns nil.
diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
index ad43fa6d367..a3bfa0d7747 100644
--- a/pkg/security/podsecuritypolicy/provider.go
+++ b/pkg/security/podsecuritypolicy/provider.go
@@ -80,7 +80,7 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit
 	}
 	annotations := maps.CopySS(pod.Annotations)
 
-	if len(sc.SupplementalGroups) == 0 {
+	if sc.SupplementalGroups == nil {
 		supGroups, err := s.strategies.SupplementalGroupStrategy.Generate(pod)
 		if err != nil {
 			return nil, nil, err
diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
index b6bafdbbb82..585f2f8f10d 100644
--- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
+++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
@@ -993,7 +993,7 @@ func TestAdmitSupplementalGroups(t *testing.T) {
 			pod:               goodPod(),
 			psps:              []*extensions.PodSecurityPolicy{runAsAny},
 			shouldPass:        true,
-			expectedSupGroups: []int64{},
+			expectedSupGroups: nil,
 			expectedPSP:       runAsAny.Name,
 		},
 		"runAsAny pod request": {

From 9e34f2b968d85ad9728686e1baf8681b3f050398 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Fri, 6 Oct 2017 16:29:19 -0400
Subject: [PATCH 05/12] PodSecurityPolicy: pass effective capabilities to
 validation interface

---
 .../capabilities/mustrunas.go                 | 19 ++++++-------------
 .../capabilities/mustrunas_test.go            | 16 ++--------------
 .../podsecuritypolicy/capabilities/types.go   |  2 +-
 pkg/security/podsecuritypolicy/provider.go    |  2 +-
 4 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/capabilities/mustrunas.go b/pkg/security/podsecuritypolicy/capabilities/mustrunas.go
index ed88883d8ad..6ab75c214b9 100644
--- a/pkg/security/podsecuritypolicy/capabilities/mustrunas.go
+++ b/pkg/security/podsecuritypolicy/capabilities/mustrunas.go
@@ -81,17 +81,10 @@ func (s *defaultCapabilities) Generate(pod *api.Pod, container *api.Container) (
 }
 
 // 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.
@@ -101,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
 	}
@@ -116,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"))
@@ -124,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)))
 		}
 	}
diff --git a/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go b/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go
index 8f002d4008f..21044e924c7 100644
--- a/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go
+++ b/pkg/security/podsecuritypolicy/capabilities/mustrunas_test.go
@@ -324,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
@@ -391,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
diff --git a/pkg/security/podsecuritypolicy/capabilities/types.go b/pkg/security/podsecuritypolicy/capabilities/types.go
index 558cfd683d9..c936d938679 100644
--- a/pkg/security/podsecuritypolicy/capabilities/types.go
+++ b/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
 }
diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
index a3bfa0d7747..b578fde98a0 100644
--- a/pkg/security/podsecuritypolicy/provider.go
+++ b/pkg/security/podsecuritypolicy/provider.go
@@ -283,7 +283,7 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *sc.Privileged, "Privileged containers are not allowed"))
 	}
 
-	allErrs = append(allErrs, s.strategies.CapabilitiesStrategy.Validate(pod, container)...)
+	allErrs = append(allErrs, s.strategies.CapabilitiesStrategy.Validate(pod, container, sc.Capabilities)...)
 
 	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"))

From 5dc4da7c6a97b1dd0b1eededf5b0aec21c5161e9 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Fri, 6 Oct 2017 16:29:57 -0400
Subject: [PATCH 06/12] PodSecurityPolicy: limit validation to provided groups

---
 .../podsecuritypolicy/group/mustrunas.go        | 11 +++--------
 .../podsecuritypolicy/group/mustrunas_test.go   | 17 +----------------
 .../podsecuritypolicy/group/runasany.go         |  6 +++---
 3 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/group/mustrunas.go b/pkg/security/podsecuritypolicy/group/mustrunas.go
index 6413ed2d4d4..b2182afaa3f 100644
--- a/pkg/security/podsecuritypolicy/group/mustrunas.go
+++ b/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"))
 	}
diff --git a/pkg/security/podsecuritypolicy/group/mustrunas_test.go b/pkg/security/podsecuritypolicy/group/mustrunas_test.go
index 554e8a19a2d..d3075a8862a 100644
--- a/pkg/security/podsecuritypolicy/group/mustrunas_test.go
+++ b/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)
 		}
diff --git a/pkg/security/podsecuritypolicy/group/runasany.go b/pkg/security/podsecuritypolicy/group/runasany.go
index aff046d50b5..071f2e08ecd 100644
--- a/pkg/security/podsecuritypolicy/group/runasany.go
+++ b/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) {
+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{}
 
 }

From e34a00d14f43e041886095309d9cbe19fc1537bb Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Fri, 6 Oct 2017 16:31:59 -0400
Subject: [PATCH 07/12] PodSecurityPolicy: pass effective selinux options to
 validate

---
 pkg/security/podsecuritypolicy/provider.go    | 11 ++-----
 .../podsecuritypolicy/provider_test.go        |  6 ++--
 .../podsecuritypolicy/selinux/mustrunas.go    | 32 +++++++------------
 .../selinux/mustrunas_test.go                 | 15 +++------
 .../podsecuritypolicy/selinux/runasany.go     |  2 +-
 .../selinux/runasany_test.go                  |  4 +--
 .../podsecuritypolicy/selinux/types.go        |  2 +-
 7 files changed, 26 insertions(+), 46 deletions(-)

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
 }

From fef3b03188edf97f007fa419855e3ba4c9eabd89 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Fri, 6 Oct 2017 16:40:49 -0400
Subject: [PATCH 08/12] PodSecurityPolicy: pass effective runAsNonRoot and
 runAsUser to user validation interface

---
 pkg/security/podsecuritypolicy/provider.go    |  2 +-
 .../podsecuritypolicy/provider_test.go        |  2 +-
 .../podsecuritypolicy/user/mustrunas.go       | 22 +++++--------------
 .../podsecuritypolicy/user/mustrunas_test.go  | 12 +++-------
 .../podsecuritypolicy/user/nonroot.go         | 20 ++++++-----------
 .../podsecuritypolicy/user/nonroot_test.go    |  8 +++----
 .../podsecuritypolicy/user/runasany.go        |  2 +-
 .../podsecuritypolicy/user/runasany_test.go   |  2 +-
 pkg/security/podsecuritypolicy/user/types.go  |  2 +-
 9 files changed, 25 insertions(+), 47 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
index 09ca844ed83..712eca17156 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 7c7cca7e4aa..d61dc962472 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 abc631e2803..df891633571 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 02edf0e4a8f..9121882063e 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 f53880a9a68..2a9624fc0b9 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 d2ec55ae06d..7822ccf126f 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 ffee679320d..729201bf645 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 d2a2915bb73..9a3181b4865 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 8e754c32f6c..8df0c766d5c 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
 }

From 34ed25cf52c91b2fb201e8211dbafcd81911bcce Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Thu, 5 Oct 2017 22:43:08 -0400
Subject: [PATCH 09/12] GC: Add check for nil interface

---
 pkg/registry/rbac/helpers.go      |  6 ++++++
 pkg/registry/rbac/helpers_test.go | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/pkg/registry/rbac/helpers.go b/pkg/registry/rbac/helpers.go
index ca0b7188569..0e10a65b855 100644
--- a/pkg/registry/rbac/helpers.go
+++ b/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)
diff --git a/pkg/registry/rbac/helpers_test.go b/pkg/registry/rbac/helpers_test.go
index 523f2de12a6..a371c7964c5 100644
--- a/pkg/registry/rbac/helpers_test.go
+++ b/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 {

From b6a750c1f6ed6b6b3f4672460ab8d096dc51eaa1 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Sat, 14 Oct 2017 15:07:02 -0400
Subject: [PATCH 10/12] SecurityContext: Add accessors/mutators for effective
 container security context

---
 pkg/securitycontext/BUILD             |  12 +-
 pkg/securitycontext/accessors.go      | 407 +++++++++++++++
 pkg/securitycontext/accessors_test.go | 726 ++++++++++++++++++++++++++
 3 files changed, 1143 insertions(+), 2 deletions(-)
 create mode 100644 pkg/securitycontext/accessors.go
 create mode 100644 pkg/securitycontext/accessors_test.go

diff --git a/pkg/securitycontext/BUILD b/pkg/securitycontext/BUILD
index 9d8d2e42c08..10611402801 100644
--- a/pkg/securitycontext/BUILD
+++ b/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(
diff --git a/pkg/securitycontext/accessors.go b/pkg/securitycontext/accessors.go
new file mode 100644
index 00000000000..f7abb22a25c
--- /dev/null
+++ b/pkg/securitycontext/accessors.go
@@ -0,0 +1,407 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package securitycontext
+
+import (
+	"reflect"
+
+	"k8s.io/kubernetes/pkg/api"
+)
+
+// PodSecurityContextAccessor allows reading the values of a PodSecurityContext object
+type PodSecurityContextAccessor interface {
+	HostNetwork() bool
+	HostPID() bool
+	HostIPC() bool
+	SELinuxOptions() *api.SELinuxOptions
+	RunAsUser() *int64
+	RunAsNonRoot() *bool
+	SupplementalGroups() []int64
+	FSGroup() *int64
+}
+
+// PodSecurityContextMutator allows reading and writing the values of a PodSecurityContext object
+type PodSecurityContextMutator interface {
+	PodSecurityContextAccessor
+
+	SetHostNetwork(bool)
+	SetHostPID(bool)
+	SetHostIPC(bool)
+	SetSELinuxOptions(*api.SELinuxOptions)
+	SetRunAsUser(*int64)
+	SetRunAsNonRoot(*bool)
+	SetSupplementalGroups([]int64)
+	SetFSGroup(*int64)
+
+	// PodSecurityContext returns the current PodSecurityContext object
+	PodSecurityContext() *api.PodSecurityContext
+}
+
+// NewPodSecurityContextAccessor returns an accessor for the given pod security context.
+// May be initialized with a nil PodSecurityContext.
+func NewPodSecurityContextAccessor(podSC *api.PodSecurityContext) PodSecurityContextAccessor {
+	return &podSecurityContextWrapper{podSC: podSC}
+}
+
+// NewPodSecurityContextMutator returns a mutator for the given pod security context.
+// May be initialized with a nil PodSecurityContext.
+func NewPodSecurityContextMutator(podSC *api.PodSecurityContext) PodSecurityContextMutator {
+	return &podSecurityContextWrapper{podSC: podSC}
+}
+
+type podSecurityContextWrapper struct {
+	podSC *api.PodSecurityContext
+}
+
+func (w *podSecurityContextWrapper) PodSecurityContext() *api.PodSecurityContext {
+	return w.podSC
+}
+
+func (w *podSecurityContextWrapper) ensurePodSC() {
+	if w.podSC == nil {
+		w.podSC = &api.PodSecurityContext{}
+	}
+}
+
+func (w *podSecurityContextWrapper) HostNetwork() bool {
+	if w.podSC == nil {
+		return false
+	}
+	return w.podSC.HostNetwork
+}
+func (w *podSecurityContextWrapper) SetHostNetwork(v bool) {
+	if w.podSC == nil && v == false {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.HostNetwork = v
+}
+func (w *podSecurityContextWrapper) HostPID() bool {
+	if w.podSC == nil {
+		return false
+	}
+	return w.podSC.HostPID
+}
+func (w *podSecurityContextWrapper) SetHostPID(v bool) {
+	if w.podSC == nil && v == false {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.HostPID = v
+}
+func (w *podSecurityContextWrapper) HostIPC() bool {
+	if w.podSC == nil {
+		return false
+	}
+	return w.podSC.HostIPC
+}
+func (w *podSecurityContextWrapper) SetHostIPC(v bool) {
+	if w.podSC == nil && v == false {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.HostIPC = v
+}
+func (w *podSecurityContextWrapper) SELinuxOptions() *api.SELinuxOptions {
+	if w.podSC == nil {
+		return nil
+	}
+	return w.podSC.SELinuxOptions
+}
+func (w *podSecurityContextWrapper) SetSELinuxOptions(v *api.SELinuxOptions) {
+	if w.podSC == nil && v == nil {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.SELinuxOptions = v
+}
+func (w *podSecurityContextWrapper) RunAsUser() *int64 {
+	if w.podSC == nil {
+		return nil
+	}
+	return w.podSC.RunAsUser
+}
+func (w *podSecurityContextWrapper) SetRunAsUser(v *int64) {
+	if w.podSC == nil && v == nil {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.RunAsUser = v
+}
+func (w *podSecurityContextWrapper) RunAsNonRoot() *bool {
+	if w.podSC == nil {
+		return nil
+	}
+	return w.podSC.RunAsNonRoot
+}
+func (w *podSecurityContextWrapper) SetRunAsNonRoot(v *bool) {
+	if w.podSC == nil && v == nil {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.RunAsNonRoot = v
+}
+func (w *podSecurityContextWrapper) SupplementalGroups() []int64 {
+	if w.podSC == nil {
+		return nil
+	}
+	return w.podSC.SupplementalGroups
+}
+func (w *podSecurityContextWrapper) SetSupplementalGroups(v []int64) {
+	if w.podSC == nil && len(v) == 0 {
+		return
+	}
+	w.ensurePodSC()
+	if len(v) == 0 && len(w.podSC.SupplementalGroups) == 0 {
+		return
+	}
+	w.podSC.SupplementalGroups = v
+}
+func (w *podSecurityContextWrapper) FSGroup() *int64 {
+	if w.podSC == nil {
+		return nil
+	}
+	return w.podSC.FSGroup
+}
+func (w *podSecurityContextWrapper) SetFSGroup(v *int64) {
+	if w.podSC == nil && v == nil {
+		return
+	}
+	w.ensurePodSC()
+	w.podSC.FSGroup = v
+}
+
+type ContainerSecurityContextAccessor interface {
+	Capabilities() *api.Capabilities
+	Privileged() *bool
+	SELinuxOptions() *api.SELinuxOptions
+	RunAsUser() *int64
+	RunAsNonRoot() *bool
+	ReadOnlyRootFilesystem() *bool
+	AllowPrivilegeEscalation() *bool
+}
+
+type ContainerSecurityContextMutator interface {
+	ContainerSecurityContextAccessor
+
+	ContainerSecurityContext() *api.SecurityContext
+
+	SetCapabilities(*api.Capabilities)
+	SetPrivileged(*bool)
+	SetSELinuxOptions(*api.SELinuxOptions)
+	SetRunAsUser(*int64)
+	SetRunAsNonRoot(*bool)
+	SetReadOnlyRootFilesystem(*bool)
+	SetAllowPrivilegeEscalation(*bool)
+}
+
+func NewContainerSecurityContextAccessor(containerSC *api.SecurityContext) ContainerSecurityContextAccessor {
+	return &containerSecurityContextWrapper{containerSC: containerSC}
+}
+
+func NewContainerSecurityContextMutator(containerSC *api.SecurityContext) ContainerSecurityContextMutator {
+	return &containerSecurityContextWrapper{containerSC: containerSC}
+}
+
+type containerSecurityContextWrapper struct {
+	containerSC *api.SecurityContext
+}
+
+func (w *containerSecurityContextWrapper) ContainerSecurityContext() *api.SecurityContext {
+	return w.containerSC
+}
+
+func (w *containerSecurityContextWrapper) ensureContainerSC() {
+	if w.containerSC == nil {
+		w.containerSC = &api.SecurityContext{}
+	}
+}
+
+func (w *containerSecurityContextWrapper) Capabilities() *api.Capabilities {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.Capabilities
+}
+func (w *containerSecurityContextWrapper) SetCapabilities(v *api.Capabilities) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.Capabilities = v
+}
+func (w *containerSecurityContextWrapper) Privileged() *bool {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.Privileged
+}
+func (w *containerSecurityContextWrapper) SetPrivileged(v *bool) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.Privileged = v
+}
+func (w *containerSecurityContextWrapper) SELinuxOptions() *api.SELinuxOptions {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.SELinuxOptions
+}
+func (w *containerSecurityContextWrapper) SetSELinuxOptions(v *api.SELinuxOptions) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.SELinuxOptions = v
+}
+func (w *containerSecurityContextWrapper) RunAsUser() *int64 {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.RunAsUser
+}
+func (w *containerSecurityContextWrapper) SetRunAsUser(v *int64) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.RunAsUser = v
+}
+func (w *containerSecurityContextWrapper) RunAsNonRoot() *bool {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.RunAsNonRoot
+}
+func (w *containerSecurityContextWrapper) SetRunAsNonRoot(v *bool) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.RunAsNonRoot = v
+}
+func (w *containerSecurityContextWrapper) ReadOnlyRootFilesystem() *bool {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.ReadOnlyRootFilesystem
+}
+func (w *containerSecurityContextWrapper) SetReadOnlyRootFilesystem(v *bool) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.ReadOnlyRootFilesystem = v
+}
+func (w *containerSecurityContextWrapper) AllowPrivilegeEscalation() *bool {
+	if w.containerSC == nil {
+		return nil
+	}
+	return w.containerSC.AllowPrivilegeEscalation
+}
+func (w *containerSecurityContextWrapper) SetAllowPrivilegeEscalation(v *bool) {
+	if w.containerSC == nil && v == nil {
+		return
+	}
+	w.ensureContainerSC()
+	w.containerSC.AllowPrivilegeEscalation = v
+}
+
+func NewEffectiveContainerSecurityContextAccessor(podSC PodSecurityContextAccessor, containerSC ContainerSecurityContextMutator) ContainerSecurityContextAccessor {
+	return &effectiveContainerSecurityContextWrapper{podSC: podSC, containerSC: containerSC}
+}
+
+func NewEffectiveContainerSecurityContextMutator(podSC PodSecurityContextAccessor, containerSC ContainerSecurityContextMutator) ContainerSecurityContextMutator {
+	return &effectiveContainerSecurityContextWrapper{podSC: podSC, containerSC: containerSC}
+}
+
+type effectiveContainerSecurityContextWrapper struct {
+	podSC       PodSecurityContextAccessor
+	containerSC ContainerSecurityContextMutator
+}
+
+func (w *effectiveContainerSecurityContextWrapper) ContainerSecurityContext() *api.SecurityContext {
+	return w.containerSC.ContainerSecurityContext()
+}
+
+func (w *effectiveContainerSecurityContextWrapper) Capabilities() *api.Capabilities {
+	return w.containerSC.Capabilities()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetCapabilities(v *api.Capabilities) {
+	if !reflect.DeepEqual(w.Capabilities(), v) {
+		w.containerSC.SetCapabilities(v)
+	}
+}
+func (w *effectiveContainerSecurityContextWrapper) Privileged() *bool {
+	return w.containerSC.Privileged()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetPrivileged(v *bool) {
+	if !reflect.DeepEqual(w.Privileged(), v) {
+		w.containerSC.SetPrivileged(v)
+	}
+}
+func (w *effectiveContainerSecurityContextWrapper) SELinuxOptions() *api.SELinuxOptions {
+	if v := w.containerSC.SELinuxOptions(); v != nil {
+		return v
+	}
+	return w.podSC.SELinuxOptions()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetSELinuxOptions(v *api.SELinuxOptions) {
+	if !reflect.DeepEqual(w.SELinuxOptions(), v) {
+		w.containerSC.SetSELinuxOptions(v)
+	}
+}
+func (w *effectiveContainerSecurityContextWrapper) RunAsUser() *int64 {
+	if v := w.containerSC.RunAsUser(); v != nil {
+		return v
+	}
+	return w.podSC.RunAsUser()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetRunAsUser(v *int64) {
+	if !reflect.DeepEqual(w.RunAsUser(), v) {
+		w.containerSC.SetRunAsUser(v)
+	}
+}
+func (w *effectiveContainerSecurityContextWrapper) RunAsNonRoot() *bool {
+	if v := w.containerSC.RunAsNonRoot(); v != nil {
+		return v
+	}
+	return w.podSC.RunAsNonRoot()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetRunAsNonRoot(v *bool) {
+	if !reflect.DeepEqual(w.RunAsNonRoot(), v) {
+		w.containerSC.SetRunAsNonRoot(v)
+	}
+}
+func (w *effectiveContainerSecurityContextWrapper) ReadOnlyRootFilesystem() *bool {
+	return w.containerSC.ReadOnlyRootFilesystem()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetReadOnlyRootFilesystem(v *bool) {
+	if !reflect.DeepEqual(w.ReadOnlyRootFilesystem(), v) {
+		w.containerSC.SetReadOnlyRootFilesystem(v)
+	}
+}
+func (w *effectiveContainerSecurityContextWrapper) AllowPrivilegeEscalation() *bool {
+	return w.containerSC.AllowPrivilegeEscalation()
+}
+func (w *effectiveContainerSecurityContextWrapper) SetAllowPrivilegeEscalation(v *bool) {
+	if !reflect.DeepEqual(w.AllowPrivilegeEscalation(), v) {
+		w.containerSC.SetAllowPrivilegeEscalation(v)
+	}
+}
diff --git a/pkg/securitycontext/accessors_test.go b/pkg/securitycontext/accessors_test.go
new file mode 100644
index 00000000000..7f6e73cd0eb
--- /dev/null
+++ b/pkg/securitycontext/accessors_test.go
@@ -0,0 +1,726 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package securitycontext
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/kubernetes/pkg/api"
+)
+
+func TestPodSecurityContextAccessor(t *testing.T) {
+	fsGroup := int64(2)
+	runAsUser := int64(1)
+	runAsNonRoot := true
+
+	testcases := []*api.PodSecurityContext{
+		nil,
+		{},
+		{FSGroup: &fsGroup},
+		{HostIPC: true},
+		{HostNetwork: true},
+		{HostPID: true},
+		{RunAsNonRoot: &runAsNonRoot},
+		{RunAsUser: &runAsUser},
+		{SELinuxOptions: &api.SELinuxOptions{User: "bob"}},
+		{SupplementalGroups: []int64{1, 2, 3}},
+	}
+
+	for i, tc := range testcases {
+		expected := tc
+		if expected == nil {
+			expected = &api.PodSecurityContext{}
+		}
+
+		a := NewPodSecurityContextAccessor(tc)
+
+		if v := a.FSGroup(); !reflect.DeepEqual(expected.FSGroup, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.FSGroup, v)
+		}
+		if v := a.HostIPC(); !reflect.DeepEqual(expected.HostIPC, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.HostIPC, v)
+		}
+		if v := a.HostNetwork(); !reflect.DeepEqual(expected.HostNetwork, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.HostNetwork, v)
+		}
+		if v := a.HostPID(); !reflect.DeepEqual(expected.HostPID, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.HostPID, v)
+		}
+		if v := a.RunAsNonRoot(); !reflect.DeepEqual(expected.RunAsNonRoot, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.RunAsNonRoot, v)
+		}
+		if v := a.RunAsUser(); !reflect.DeepEqual(expected.RunAsUser, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.RunAsUser, v)
+		}
+		if v := a.SELinuxOptions(); !reflect.DeepEqual(expected.SELinuxOptions, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.SELinuxOptions, v)
+		}
+		if v := a.SupplementalGroups(); !reflect.DeepEqual(expected.SupplementalGroups, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.SupplementalGroups, v)
+		}
+	}
+}
+
+func TestPodSecurityContextMutator(t *testing.T) {
+	testcases := map[string]struct {
+		newSC func() *api.PodSecurityContext
+	}{
+		"nil": {
+			newSC: func() *api.PodSecurityContext { return nil },
+		},
+		"zero": {
+			newSC: func() *api.PodSecurityContext { return &api.PodSecurityContext{} },
+		},
+		"populated": {
+			newSC: func() *api.PodSecurityContext {
+				return &api.PodSecurityContext{
+					HostNetwork:        true,
+					HostIPC:            true,
+					HostPID:            true,
+					SELinuxOptions:     &api.SELinuxOptions{},
+					RunAsUser:          nil,
+					RunAsNonRoot:       nil,
+					SupplementalGroups: nil,
+					FSGroup:            nil,
+				}
+			},
+		},
+	}
+
+	nonNilSC := func(sc *api.PodSecurityContext) *api.PodSecurityContext {
+		if sc == nil {
+			return &api.PodSecurityContext{}
+		}
+		return sc
+	}
+
+	for k, tc := range testcases {
+		{
+			sc := tc.newSC()
+			originalSC := tc.newSC()
+			m := NewPodSecurityContextMutator(sc)
+
+			// no-op sets should not modify the object
+			m.SetFSGroup(m.FSGroup())
+			m.SetHostNetwork(m.HostNetwork())
+			m.SetHostIPC(m.HostIPC())
+			m.SetHostPID(m.HostPID())
+			m.SetRunAsNonRoot(m.RunAsNonRoot())
+			m.SetRunAsUser(m.RunAsUser())
+			m.SetSELinuxOptions(m.SELinuxOptions())
+			m.SetSupplementalGroups(m.SupplementalGroups())
+			if !reflect.DeepEqual(sc, originalSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, sc, originalSC)
+			}
+			if !reflect.DeepEqual(m.PodSecurityContext(), originalSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, m.PodSecurityContext(), originalSC)
+			}
+		}
+
+		// FSGroup
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			i := int64(1123)
+			modifiedSC.FSGroup = &i
+			m.SetFSGroup(&i)
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// HostNetwork
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			modifiedSC.HostNetwork = !modifiedSC.HostNetwork
+			m.SetHostNetwork(!m.HostNetwork())
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// HostIPC
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			modifiedSC.HostIPC = !modifiedSC.HostIPC
+			m.SetHostIPC(!m.HostIPC())
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// HostPID
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			modifiedSC.HostPID = !modifiedSC.HostPID
+			m.SetHostPID(!m.HostPID())
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// RunAsNonRoot
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			b := true
+			modifiedSC.RunAsNonRoot = &b
+			m.SetRunAsNonRoot(&b)
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// RunAsUser
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			i := int64(1123)
+			modifiedSC.RunAsUser = &i
+			m.SetRunAsUser(&i)
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// SELinuxOptions
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			modifiedSC.SELinuxOptions = &api.SELinuxOptions{User: "bob"}
+			m.SetSELinuxOptions(&api.SELinuxOptions{User: "bob"})
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+
+		// SupplementalGroups
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewPodSecurityContextMutator(tc.newSC())
+			modifiedSC.SupplementalGroups = []int64{1, 1, 2, 3}
+			m.SetSupplementalGroups([]int64{1, 1, 2, 3})
+			if !reflect.DeepEqual(m.PodSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.PodSecurityContext()))
+				continue
+			}
+		}
+	}
+}
+
+func TestContainerSecurityContextAccessor(t *testing.T) {
+	privileged := true
+	runAsUser := int64(1)
+	runAsNonRoot := true
+	readOnlyRootFilesystem := true
+	allowPrivilegeEscalation := true
+
+	testcases := []*api.SecurityContext{
+		nil,
+		{},
+		{Capabilities: &api.Capabilities{Drop: []api.Capability{"test"}}},
+		{Privileged: &privileged},
+		{SELinuxOptions: &api.SELinuxOptions{User: "bob"}},
+		{RunAsUser: &runAsUser},
+		{RunAsNonRoot: &runAsNonRoot},
+		{ReadOnlyRootFilesystem: &readOnlyRootFilesystem},
+		{AllowPrivilegeEscalation: &allowPrivilegeEscalation},
+	}
+
+	for i, tc := range testcases {
+		expected := tc
+		if expected == nil {
+			expected = &api.SecurityContext{}
+		}
+
+		a := NewContainerSecurityContextAccessor(tc)
+
+		if v := a.Capabilities(); !reflect.DeepEqual(expected.Capabilities, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.Capabilities, v)
+		}
+		if v := a.Privileged(); !reflect.DeepEqual(expected.Privileged, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.Privileged, v)
+		}
+		if v := a.RunAsNonRoot(); !reflect.DeepEqual(expected.RunAsNonRoot, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.RunAsNonRoot, v)
+		}
+		if v := a.RunAsUser(); !reflect.DeepEqual(expected.RunAsUser, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.RunAsUser, v)
+		}
+		if v := a.SELinuxOptions(); !reflect.DeepEqual(expected.SELinuxOptions, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.SELinuxOptions, v)
+		}
+		if v := a.ReadOnlyRootFilesystem(); !reflect.DeepEqual(expected.ReadOnlyRootFilesystem, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.ReadOnlyRootFilesystem, v)
+		}
+		if v := a.AllowPrivilegeEscalation(); !reflect.DeepEqual(expected.AllowPrivilegeEscalation, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.AllowPrivilegeEscalation, v)
+		}
+	}
+}
+
+func TestContainerSecurityContextMutator(t *testing.T) {
+	testcases := map[string]struct {
+		newSC func() *api.SecurityContext
+	}{
+		"nil": {
+			newSC: func() *api.SecurityContext { return nil },
+		},
+		"zero": {
+			newSC: func() *api.SecurityContext { return &api.SecurityContext{} },
+		},
+		"populated": {
+			newSC: func() *api.SecurityContext {
+				return &api.SecurityContext{
+					Capabilities:   &api.Capabilities{Drop: []api.Capability{"test"}},
+					SELinuxOptions: &api.SELinuxOptions{},
+				}
+			},
+		},
+	}
+
+	nonNilSC := func(sc *api.SecurityContext) *api.SecurityContext {
+		if sc == nil {
+			return &api.SecurityContext{}
+		}
+		return sc
+	}
+
+	for k, tc := range testcases {
+		{
+			sc := tc.newSC()
+			originalSC := tc.newSC()
+			m := NewContainerSecurityContextMutator(sc)
+
+			// no-op sets should not modify the object
+			m.SetAllowPrivilegeEscalation(m.AllowPrivilegeEscalation())
+			m.SetCapabilities(m.Capabilities())
+			m.SetPrivileged(m.Privileged())
+			m.SetReadOnlyRootFilesystem(m.ReadOnlyRootFilesystem())
+			m.SetRunAsNonRoot(m.RunAsNonRoot())
+			m.SetRunAsUser(m.RunAsUser())
+			m.SetSELinuxOptions(m.SELinuxOptions())
+			if !reflect.DeepEqual(sc, originalSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, sc, originalSC)
+			}
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), originalSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, m.ContainerSecurityContext(), originalSC)
+			}
+		}
+
+		// AllowPrivilegeEscalation
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			b := true
+			modifiedSC.AllowPrivilegeEscalation = &b
+			m.SetAllowPrivilegeEscalation(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// Capabilities
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			modifiedSC.Capabilities = &api.Capabilities{Drop: []api.Capability{"test2"}}
+			m.SetCapabilities(&api.Capabilities{Drop: []api.Capability{"test2"}})
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// Privileged
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			b := true
+			modifiedSC.Privileged = &b
+			m.SetPrivileged(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// ReadOnlyRootFilesystem
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			b := true
+			modifiedSC.ReadOnlyRootFilesystem = &b
+			m.SetReadOnlyRootFilesystem(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// RunAsNonRoot
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			b := true
+			modifiedSC.RunAsNonRoot = &b
+			m.SetRunAsNonRoot(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// RunAsUser
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			i := int64(1123)
+			modifiedSC.RunAsUser = &i
+			m.SetRunAsUser(&i)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// SELinuxOptions
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewContainerSecurityContextMutator(tc.newSC())
+			modifiedSC.SELinuxOptions = &api.SELinuxOptions{User: "bob"}
+			m.SetSELinuxOptions(&api.SELinuxOptions{User: "bob"})
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+	}
+}
+
+func TestEffectiveContainerSecurityContextAccessor(t *testing.T) {
+	privileged := true
+	runAsUser := int64(1)
+	runAsUserPod := int64(12)
+	runAsNonRoot := true
+	runAsNonRootPod := false
+	readOnlyRootFilesystem := true
+	allowPrivilegeEscalation := true
+
+	testcases := []struct {
+		PodSC     *api.PodSecurityContext
+		SC        *api.SecurityContext
+		Effective *api.SecurityContext
+	}{
+		{
+			PodSC:     nil,
+			SC:        nil,
+			Effective: nil,
+		},
+		{
+			PodSC:     &api.PodSecurityContext{},
+			SC:        &api.SecurityContext{},
+			Effective: &api.SecurityContext{},
+		},
+		{
+			PodSC: &api.PodSecurityContext{
+				SELinuxOptions: &api.SELinuxOptions{User: "bob"},
+				RunAsUser:      &runAsUser,
+				RunAsNonRoot:   &runAsNonRoot,
+			},
+			SC: nil,
+			Effective: &api.SecurityContext{
+				SELinuxOptions: &api.SELinuxOptions{User: "bob"},
+				RunAsUser:      &runAsUser,
+				RunAsNonRoot:   &runAsNonRoot,
+			},
+		},
+		{
+			PodSC: &api.PodSecurityContext{
+				SELinuxOptions: &api.SELinuxOptions{User: "bob"},
+				RunAsUser:      &runAsUserPod,
+				RunAsNonRoot:   &runAsNonRootPod,
+			},
+			SC: &api.SecurityContext{},
+			Effective: &api.SecurityContext{
+				SELinuxOptions: &api.SELinuxOptions{User: "bob"},
+				RunAsUser:      &runAsUserPod,
+				RunAsNonRoot:   &runAsNonRootPod,
+			},
+		},
+		{
+			PodSC: &api.PodSecurityContext{
+				SELinuxOptions: &api.SELinuxOptions{User: "bob"},
+				RunAsUser:      &runAsUserPod,
+				RunAsNonRoot:   &runAsNonRootPod,
+			},
+			SC: &api.SecurityContext{
+				AllowPrivilegeEscalation: &allowPrivilegeEscalation,
+				Capabilities:             &api.Capabilities{Drop: []api.Capability{"test"}},
+				Privileged:               &privileged,
+				ReadOnlyRootFilesystem:   &readOnlyRootFilesystem,
+				RunAsUser:                &runAsUser,
+				RunAsNonRoot:             &runAsNonRoot,
+				SELinuxOptions:           &api.SELinuxOptions{User: "bob"},
+			},
+			Effective: &api.SecurityContext{
+				AllowPrivilegeEscalation: &allowPrivilegeEscalation,
+				Capabilities:             &api.Capabilities{Drop: []api.Capability{"test"}},
+				Privileged:               &privileged,
+				ReadOnlyRootFilesystem:   &readOnlyRootFilesystem,
+				RunAsUser:                &runAsUser,
+				RunAsNonRoot:             &runAsNonRoot,
+				SELinuxOptions:           &api.SELinuxOptions{User: "bob"},
+			},
+		},
+	}
+
+	for i, tc := range testcases {
+		expected := tc.Effective
+		if expected == nil {
+			expected = &api.SecurityContext{}
+		}
+
+		a := NewEffectiveContainerSecurityContextAccessor(
+			NewPodSecurityContextAccessor(tc.PodSC),
+			NewContainerSecurityContextMutator(tc.SC),
+		)
+
+		if v := a.Capabilities(); !reflect.DeepEqual(expected.Capabilities, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.Capabilities, v)
+		}
+		if v := a.Privileged(); !reflect.DeepEqual(expected.Privileged, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.Privileged, v)
+		}
+		if v := a.RunAsNonRoot(); !reflect.DeepEqual(expected.RunAsNonRoot, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.RunAsNonRoot, v)
+		}
+		if v := a.RunAsUser(); !reflect.DeepEqual(expected.RunAsUser, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.RunAsUser, v)
+		}
+		if v := a.SELinuxOptions(); !reflect.DeepEqual(expected.SELinuxOptions, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.SELinuxOptions, v)
+		}
+		if v := a.ReadOnlyRootFilesystem(); !reflect.DeepEqual(expected.ReadOnlyRootFilesystem, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.ReadOnlyRootFilesystem, v)
+		}
+		if v := a.AllowPrivilegeEscalation(); !reflect.DeepEqual(expected.AllowPrivilegeEscalation, v) {
+			t.Errorf("%d: expected %#v, got %#v", i, expected.AllowPrivilegeEscalation, v)
+		}
+	}
+}
+
+func TestEffectiveContainerSecurityContextMutator(t *testing.T) {
+	runAsNonRootPod := false
+	runAsUserPod := int64(12)
+
+	testcases := map[string]struct {
+		newPodSC func() *api.PodSecurityContext
+		newSC    func() *api.SecurityContext
+	}{
+		"nil": {
+			newPodSC: func() *api.PodSecurityContext { return nil },
+			newSC:    func() *api.SecurityContext { return nil },
+		},
+		"zero": {
+			newPodSC: func() *api.PodSecurityContext { return &api.PodSecurityContext{} },
+			newSC:    func() *api.SecurityContext { return &api.SecurityContext{} },
+		},
+		"populated pod sc": {
+			newPodSC: func() *api.PodSecurityContext {
+				return &api.PodSecurityContext{
+					SELinuxOptions: &api.SELinuxOptions{User: "poduser"},
+					RunAsNonRoot:   &runAsNonRootPod,
+					RunAsUser:      &runAsUserPod,
+				}
+			},
+			newSC: func() *api.SecurityContext {
+				return &api.SecurityContext{}
+			},
+		},
+		"populated sc": {
+			newPodSC: func() *api.PodSecurityContext { return nil },
+			newSC: func() *api.SecurityContext {
+				return &api.SecurityContext{
+					Capabilities:   &api.Capabilities{Drop: []api.Capability{"test"}},
+					SELinuxOptions: &api.SELinuxOptions{},
+				}
+			},
+		},
+	}
+
+	nonNilSC := func(sc *api.SecurityContext) *api.SecurityContext {
+		if sc == nil {
+			return &api.SecurityContext{}
+		}
+		return sc
+	}
+
+	for k, tc := range testcases {
+		{
+			podSC := tc.newPodSC()
+			sc := tc.newSC()
+			originalPodSC := tc.newPodSC()
+			originalSC := tc.newSC()
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(podSC),
+				NewContainerSecurityContextMutator(sc),
+			)
+
+			// no-op sets should not modify the object
+			m.SetAllowPrivilegeEscalation(m.AllowPrivilegeEscalation())
+			m.SetCapabilities(m.Capabilities())
+			m.SetPrivileged(m.Privileged())
+			m.SetReadOnlyRootFilesystem(m.ReadOnlyRootFilesystem())
+			m.SetRunAsNonRoot(m.RunAsNonRoot())
+			m.SetRunAsUser(m.RunAsUser())
+			m.SetSELinuxOptions(m.SELinuxOptions())
+			if !reflect.DeepEqual(podSC, originalPodSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, podSC, originalPodSC)
+			}
+			if !reflect.DeepEqual(sc, originalSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, sc, originalSC)
+			}
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), originalSC) {
+				t.Errorf("%s: unexpected mutation: %#v, %#v", k, m.ContainerSecurityContext(), originalSC)
+			}
+		}
+
+		// AllowPrivilegeEscalation
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			b := true
+			modifiedSC.AllowPrivilegeEscalation = &b
+			m.SetAllowPrivilegeEscalation(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// Capabilities
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			modifiedSC.Capabilities = &api.Capabilities{Drop: []api.Capability{"test2"}}
+			m.SetCapabilities(&api.Capabilities{Drop: []api.Capability{"test2"}})
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// Privileged
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			b := true
+			modifiedSC.Privileged = &b
+			m.SetPrivileged(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// ReadOnlyRootFilesystem
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			b := true
+			modifiedSC.ReadOnlyRootFilesystem = &b
+			m.SetReadOnlyRootFilesystem(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// RunAsNonRoot
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			b := true
+			modifiedSC.RunAsNonRoot = &b
+			m.SetRunAsNonRoot(&b)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// RunAsUser
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			i := int64(1123)
+			modifiedSC.RunAsUser = &i
+			m.SetRunAsUser(&i)
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+
+		// SELinuxOptions
+		{
+			modifiedSC := nonNilSC(tc.newSC())
+			m := NewEffectiveContainerSecurityContextMutator(
+				NewPodSecurityContextAccessor(tc.newPodSC()),
+				NewContainerSecurityContextMutator(tc.newSC()),
+			)
+			modifiedSC.SELinuxOptions = &api.SELinuxOptions{User: "bob"}
+			m.SetSELinuxOptions(&api.SELinuxOptions{User: "bob"})
+			if !reflect.DeepEqual(m.ContainerSecurityContext(), modifiedSC) {
+				t.Errorf("%s: unexpected object:\n%s", k, diff.ObjectGoPrintSideBySide(modifiedSC, m.ContainerSecurityContext()))
+				continue
+			}
+		}
+	}
+}

From a5f722e181420cec5ef5533a111487034a713d7b Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Sat, 14 Oct 2017 15:07:09 -0400
Subject: [PATCH 11/12] PodSecurityPolicy: avoid unnecessary securitycontext
 mutation

---
 pkg/security/podsecuritypolicy/BUILD          |   1 +
 pkg/security/podsecuritypolicy/provider.go    | 145 ++++++++----------
 .../podsecuritypolicy/provider_test.go        |  44 ++----
 3 files changed, 74 insertions(+), 116 deletions(-)

diff --git a/pkg/security/podsecuritypolicy/BUILD b/pkg/security/podsecuritypolicy/BUILD
index 9216455dcf2..55a68dec3bf 100644
--- a/pkg/security/podsecuritypolicy/BUILD
+++ b/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",
diff --git a/pkg/security/podsecuritypolicy/provider.go b/pkg/security/podsecuritypolicy/provider.go
index 712eca17156..fadacec1dd9 100644
--- a/pkg/security/podsecuritypolicy/provider.go
+++ b/pkg/security/podsecuritypolicy/provider.go
@@ -24,6 +24,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"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"
 )
 
@@ -66,42 +67,32 @@ func NewSimpleProvider(psp *extensions.PodSecurityPolicy, namespace string, stra
 // 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.
-//
-// NOTE: this method works on a copy of the PodSecurityContext.  It is up to the caller to
-// apply the PSC if validation passes.
 func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurityContext, map[string]string, error) {
-	var sc *api.PodSecurityContext = nil
-	if pod.Spec.SecurityContext != nil {
-		// work with a copy
-		copy := *pod.Spec.SecurityContext
-		sc = &copy
-	} else {
-		sc = &api.PodSecurityContext{}
-	}
+	sc := securitycontext.NewPodSecurityContextMutator(pod.Spec.SecurityContext)
 	annotations := maps.CopySS(pod.Annotations)
 
-	if sc.SupplementalGroups == nil {
+	if sc.SupplementalGroups() == nil {
 		supGroups, err := s.strategies.SupplementalGroupStrategy.Generate(pod)
 		if err != nil {
 			return nil, nil, err
 		}
-		sc.SupplementalGroups = supGroups
+		sc.SetSupplementalGroups(supGroups)
 	}
 
-	if sc.FSGroup == nil {
+	if sc.FSGroup() == nil {
 		fsGroup, err := s.strategies.FSGroupStrategy.GenerateSingle(pod)
 		if err != nil {
 			return nil, nil, err
 		}
-		sc.FSGroup = fsGroup
+		sc.SetFSGroup(fsGroup)
 	}
 
-	if sc.SELinuxOptions == nil {
+	if sc.SELinuxOptions() == nil {
 		seLinux, err := s.strategies.SELinuxStrategy.Generate(pod, nil)
 		if err != nil {
 			return nil, nil, err
 		}
-		sc.SELinuxOptions = seLinux
+		sc.SetSELinuxOptions(seLinux)
 	}
 
 	// This is only generated on the pod level.  Containers inherit the pod's profile.  If the
@@ -116,40 +107,34 @@ func (s *simpleProvider) CreatePodSecurityContext(pod *api.Pod) (*api.PodSecurit
 		}
 		annotations[api.SeccompPodAnnotationKey] = seccompProfile
 	}
-	return sc, annotations, nil
+	return sc.PodSecurityContext(), annotations, 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.
-//
-// NOTE: this method works on a copy of the SC of the container.  It is up to the caller to apply
-// the SC if validation passes.
 func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container *api.Container) (*api.SecurityContext, map[string]string, error) {
-	var sc *api.SecurityContext = nil
-	if container.SecurityContext != nil {
-		// work with a copy of the original
-		copy := *container.SecurityContext
-		sc = &copy
-	} else {
-		sc = &api.SecurityContext{}
-	}
+	sc := securitycontext.NewEffectiveContainerSecurityContextMutator(
+		securitycontext.NewPodSecurityContextAccessor(pod.Spec.SecurityContext),
+		securitycontext.NewContainerSecurityContextMutator(container.SecurityContext),
+	)
+
 	annotations := maps.CopySS(pod.Annotations)
 
-	if sc.RunAsUser == nil {
+	if sc.RunAsUser() == nil {
 		uid, err := s.strategies.RunAsUserStrategy.Generate(pod, container)
 		if err != nil {
 			return nil, nil, err
 		}
-		sc.RunAsUser = uid
+		sc.SetRunAsUser(uid)
 	}
 
-	if sc.SELinuxOptions == nil {
+	if sc.SELinuxOptions() == nil {
 		seLinux, err := s.strategies.SELinuxStrategy.Generate(pod, container)
 		if err != nil {
 			return nil, nil, err
 		}
-		sc.SELinuxOptions = seLinux
+		sc.SetSELinuxOptions(seLinux)
 	}
 
 	annotations, err := s.strategies.AppArmorStrategy.Generate(annotations, container)
@@ -160,67 +145,64 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container
 	// if we're using the non-root strategy set the marker that this container should not be
 	// run as root which will signal to the kubelet to do a final check either on the runAsUser
 	// or, if runAsUser is not set, the image UID will be checked.
-	if sc.RunAsNonRoot == nil && sc.RunAsUser == nil && s.psp.Spec.RunAsUser.Rule == extensions.RunAsUserStrategyMustRunAsNonRoot {
+	if sc.RunAsNonRoot() == nil && sc.RunAsUser() == nil && s.psp.Spec.RunAsUser.Rule == extensions.RunAsUserStrategyMustRunAsNonRoot {
 		nonRoot := true
-		sc.RunAsNonRoot = &nonRoot
+		sc.SetRunAsNonRoot(&nonRoot)
 	}
 
 	caps, err := s.strategies.CapabilitiesStrategy.Generate(pod, container)
 	if err != nil {
 		return nil, nil, err
 	}
-	sc.Capabilities = caps
+	sc.SetCapabilities(caps)
 
 	// if the PSP requires a read only root filesystem and the container has not made a specific
 	// request then default ReadOnlyRootFilesystem to true.
-	if s.psp.Spec.ReadOnlyRootFilesystem && sc.ReadOnlyRootFilesystem == nil {
+	if s.psp.Spec.ReadOnlyRootFilesystem && sc.ReadOnlyRootFilesystem() == nil {
 		readOnlyRootFS := true
-		sc.ReadOnlyRootFilesystem = &readOnlyRootFS
+		sc.SetReadOnlyRootFilesystem(&readOnlyRootFS)
 	}
 
 	// if the PSP sets DefaultAllowPrivilegeEscalation and the container security context
 	// allowPrivilegeEscalation is not set, then default to that set by the PSP.
-	if s.psp.Spec.DefaultAllowPrivilegeEscalation != nil && sc.AllowPrivilegeEscalation == nil {
-		sc.AllowPrivilegeEscalation = s.psp.Spec.DefaultAllowPrivilegeEscalation
+	if s.psp.Spec.DefaultAllowPrivilegeEscalation != nil && sc.AllowPrivilegeEscalation() == nil {
+		sc.SetAllowPrivilegeEscalation(s.psp.Spec.DefaultAllowPrivilegeEscalation)
 	}
 
 	// if the PSP sets psp.AllowPrivilegeEscalation to false set that as the default
-	if !s.psp.Spec.AllowPrivilegeEscalation && sc.AllowPrivilegeEscalation == nil {
-		sc.AllowPrivilegeEscalation = &s.psp.Spec.AllowPrivilegeEscalation
+	if !s.psp.Spec.AllowPrivilegeEscalation && sc.AllowPrivilegeEscalation() == nil {
+		sc.SetAllowPrivilegeEscalation(&s.psp.Spec.AllowPrivilegeEscalation)
 	}
 
-	return sc, annotations, nil
+	return sc.ContainerSecurityContext(), annotations, nil
 }
 
 // Ensure a pod's SecurityContext is in compliance with the given constraints.
 func (s *simpleProvider) ValidatePodSecurityContext(pod *api.Pod, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if pod.Spec.SecurityContext == nil {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("securityContext"), pod.Spec.SecurityContext, "No security context is set"))
-		return allErrs
-	}
+	sc := securitycontext.NewPodSecurityContextAccessor(pod.Spec.SecurityContext)
 
 	fsGroups := []int64{}
-	if pod.Spec.SecurityContext.FSGroup != nil {
-		fsGroups = append(fsGroups, *pod.Spec.SecurityContext.FSGroup)
+	if fsGroup := sc.FSGroup(); fsGroup != nil {
+		fsGroups = append(fsGroups, *fsGroup)
 	}
 	allErrs = append(allErrs, s.strategies.FSGroupStrategy.Validate(pod, fsGroups)...)
-	allErrs = append(allErrs, s.strategies.SupplementalGroupStrategy.Validate(pod, pod.Spec.SecurityContext.SupplementalGroups)...)
+	allErrs = append(allErrs, s.strategies.SupplementalGroupStrategy.Validate(pod, sc.SupplementalGroups())...)
 	allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidatePod(pod)...)
 
-	allErrs = append(allErrs, s.strategies.SELinuxStrategy.Validate(fldPath.Child("seLinuxOptions"), pod, nil, pod.Spec.SecurityContext.SELinuxOptions)...)
+	allErrs = append(allErrs, s.strategies.SELinuxStrategy.Validate(fldPath.Child("seLinuxOptions"), pod, nil, sc.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"))
+	if !s.psp.Spec.HostNetwork && sc.HostNetwork() {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostNetwork"), sc.HostNetwork(), "Host network is not allowed to be used"))
 	}
 
-	if !s.psp.Spec.HostPID && pod.Spec.SecurityContext.HostPID {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostPID"), pod.Spec.SecurityContext.HostPID, "Host PID is not allowed to be used"))
+	if !s.psp.Spec.HostPID && sc.HostPID() {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostPID"), sc.HostPID(), "Host PID is not allowed to be used"))
 	}
 
-	if !s.psp.Spec.HostIPC && pod.Spec.SecurityContext.HostIPC {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostIPC"), pod.Spec.SecurityContext.HostIPC, "Host IPC is not allowed to be used"))
+	if !s.psp.Spec.HostIPC && sc.HostIPC() {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostIPC"), sc.HostIPC(), "Host IPC is not allowed to be used"))
 	}
 
 	allErrs = append(allErrs, s.strategies.SysctlsStrategy.Validate(pod)...)
@@ -261,25 +243,23 @@ func (s *simpleProvider) ValidatePodSecurityContext(pod *api.Pod, fldPath *field
 func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, container *api.Container, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if container.SecurityContext == nil {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("securityContext"), container.SecurityContext, "No security context is set"))
-		return allErrs
-	}
+	podSC := securitycontext.NewPodSecurityContextAccessor(pod.Spec.SecurityContext)
+	sc := securitycontext.NewEffectiveContainerSecurityContextAccessor(podSC, securitycontext.NewContainerSecurityContextMutator(container.SecurityContext))
 
-	sc := container.SecurityContext
-	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.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)...)
 
-	if !s.psp.Spec.Privileged && sc.Privileged != nil && *sc.Privileged {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *sc.Privileged, "Privileged containers are not allowed"))
+	privileged := sc.Privileged()
+	if !s.psp.Spec.Privileged && privileged != nil && *privileged {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *privileged, "Privileged containers are not allowed"))
 	}
 
-	allErrs = append(allErrs, s.strategies.CapabilitiesStrategy.Validate(pod, container, sc.Capabilities)...)
+	allErrs = append(allErrs, s.strategies.CapabilitiesStrategy.Validate(pod, container, sc.Capabilities())...)
 
-	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"))
+	if !s.psp.Spec.HostNetwork && podSC.HostNetwork() {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostNetwork"), podSC.HostNetwork(), "Host network is not allowed to be used"))
 	}
 
 	containersPath := fldPath.Child("containers")
@@ -294,29 +274,30 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
 		allErrs = append(allErrs, s.hasInvalidHostPort(&c, idxPath)...)
 	}
 
-	if !s.psp.Spec.HostPID && pod.Spec.SecurityContext.HostPID {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostPID"), pod.Spec.SecurityContext.HostPID, "Host PID is not allowed to be used"))
+	if !s.psp.Spec.HostPID && podSC.HostPID() {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostPID"), podSC.HostPID(), "Host PID is not allowed to be used"))
 	}
 
-	if !s.psp.Spec.HostIPC && pod.Spec.SecurityContext.HostIPC {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostIPC"), pod.Spec.SecurityContext.HostIPC, "Host IPC is not allowed to be used"))
+	if !s.psp.Spec.HostIPC && podSC.HostIPC() {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("hostIPC"), podSC.HostIPC(), "Host IPC is not allowed to be used"))
 	}
 
 	if s.psp.Spec.ReadOnlyRootFilesystem {
-		if sc.ReadOnlyRootFilesystem == nil {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("readOnlyRootFilesystem"), sc.ReadOnlyRootFilesystem, "ReadOnlyRootFilesystem may not be nil and must be set to true"))
-		} else if !*sc.ReadOnlyRootFilesystem {
-			allErrs = append(allErrs, field.Invalid(fldPath.Child("readOnlyRootFilesystem"), *sc.ReadOnlyRootFilesystem, "ReadOnlyRootFilesystem must be set to true"))
+		readOnly := sc.ReadOnlyRootFilesystem()
+		if readOnly == nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("readOnlyRootFilesystem"), readOnly, "ReadOnlyRootFilesystem may not be nil and must be set to true"))
+		} else if !*readOnly {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("readOnlyRootFilesystem"), *readOnly, "ReadOnlyRootFilesystem must be set to true"))
 		}
 	}
 
-	if !s.psp.Spec.AllowPrivilegeEscalation && sc.AllowPrivilegeEscalation == nil {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("allowPrivilegeEscalation"), sc.AllowPrivilegeEscalation, "Allowing privilege escalation for containers is not allowed"))
-
+	allowEscalation := sc.AllowPrivilegeEscalation()
+	if !s.psp.Spec.AllowPrivilegeEscalation && allowEscalation == nil {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("allowPrivilegeEscalation"), allowEscalation, "Allowing privilege escalation for containers is not allowed"))
 	}
 
-	if !s.psp.Spec.AllowPrivilegeEscalation && sc.AllowPrivilegeEscalation != nil && *sc.AllowPrivilegeEscalation {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("allowPrivilegeEscalation"), *sc.AllowPrivilegeEscalation, "Allowing privilege escalation for containers is not allowed"))
+	if !s.psp.Spec.AllowPrivilegeEscalation && allowEscalation != nil && *allowEscalation {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("allowPrivilegeEscalation"), *allowEscalation, "Allowing privilege escalation for containers is not allowed"))
 	}
 
 	return allErrs
diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go
index d61dc962472..cfba7786c67 100644
--- a/pkg/security/podsecuritypolicy/provider_test.go
+++ b/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)

From 8c5b01376a4967dcda3650517bfa058fd26db8f5 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Thu, 5 Oct 2017 15:56:12 -0400
Subject: [PATCH 12/12] PodSecurityPolicy: Order by name, prefer non-mutating
 policies, require *api.Pod, allow GC updates

---
 .../security/podsecuritypolicy/BUILD          |   6 +-
 .../security/podsecuritypolicy/admission.go   | 129 ++--
 .../podsecuritypolicy/admission_test.go       | 662 +++++++++++++-----
 3 files changed, 550 insertions(+), 247 deletions(-)

diff --git a/plugin/pkg/admission/security/podsecuritypolicy/BUILD b/plugin/pkg/admission/security/podsecuritypolicy/BUILD
index f6bbf4a2ded..782b75eb790 100644
--- a/plugin/pkg/admission/security/podsecuritypolicy/BUILD
+++ b/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",
diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission.go b/plugin/pkg/admission/security/podsecuritypolicy/admission.go
index e0eb705a4e1..19de55b93e1 100644
--- a/plugin/pkg/admission/security/podsecuritypolicy/admission.go
+++ b/plugin/pkg/admission/security/podsecuritypolicy/admission.go
@@ -19,10 +19,12 @@ package podsecuritypolicy
 import (
 	"fmt"
 	"io"
+	"sort"
 	"strings"
 
 	"github.com/golang/glog"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
@@ -34,11 +36,10 @@ import (
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	extensionslisters "k8s.io/kubernetes/pkg/client/listers/extensions/internalversion"
 	kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
+	rbacregistry "k8s.io/kubernetes/pkg/registry/rbac"
 	psp "k8s.io/kubernetes/pkg/security/podsecuritypolicy"
 	psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
-	sc "k8s.io/kubernetes/pkg/securitycontext"
 	"k8s.io/kubernetes/pkg/serviceaccount"
-	"k8s.io/kubernetes/pkg/util/maps"
 )
 
 const (
@@ -120,8 +121,17 @@ func (c *podSecurityPolicyPlugin) Admit(a admission.Attributes) error {
 	}
 
 	pod, ok := a.GetObject().(*api.Pod)
-	// if we can't convert then we don't handle this object so just return
+	// if we can't convert then fail closed since we've already checked that this is supposed to be a pod object.
+	// this shouldn't normally happen during admission but could happen if an integrator passes a versioned
+	// pod object rather than an internal object.
 	if !ok {
+		return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
+	}
+
+	// if this is an update, see if we are only updating the ownerRef/finalizers.  Garbage collection does this
+	// and we should allow it in general, since you had the power to update and the power to delete.
+	// The worst that happens is that you delete something, but you aren't controlling the privileged object itself
+	if a.GetOperation() == admission.Update && rbacregistry.IsOnlyMutatingGCFields(a.GetObject(), a.GetOldObject(), apiequality.Semantic) {
 		return nil
 	}
 
@@ -143,32 +153,70 @@ func (c *podSecurityPolicyPlugin) Admit(a admission.Attributes) error {
 		return nil
 	}
 
+	// sort by name to make order deterministic
+	// TODO(liggitt): add priority field to allow admins to bucket differently
+	sort.SliceStable(matchedPolicies, func(i, j int) bool {
+		return strings.Compare(matchedPolicies[i].Name, matchedPolicies[j].Name) < 0
+	})
+
 	providers, errs := c.createProvidersFromPolicies(matchedPolicies, pod.Namespace)
-	logProviders(pod, providers, errs)
+	logProviders(a, pod, providers, errs)
 
 	if len(providers) == 0 {
 		return admission.NewForbidden(a, fmt.Errorf("no providers available to validate pod request"))
 	}
 
+	// TODO(liggitt): allow spec mutation during initializing updates?
+	specMutationAllowed := a.GetOperation() == admission.Create
+
 	// all containers in a single pod must validate under a single provider or we will reject the request
 	validationErrs := field.ErrorList{}
+	var (
+		allowedPod       *api.Pod
+		allowingProvider psp.Provider
+	)
+
+loop:
 	for _, provider := range providers {
-		if errs := assignSecurityContext(provider, pod, field.NewPath(fmt.Sprintf("provider %s: ", provider.GetPSPName()))); len(errs) > 0 {
+		podCopy := pod.DeepCopy()
+
+		if errs := assignSecurityContext(provider, podCopy, field.NewPath(fmt.Sprintf("provider %s: ", provider.GetPSPName()))); len(errs) > 0 {
 			validationErrs = append(validationErrs, errs...)
 			continue
 		}
 
-		// the entire pod validated, annotate and accept the pod
-		glog.V(4).Infof("pod %s (generate: %s) validated against provider %s", pod.Name, pod.GenerateName, provider.GetPSPName())
+		// the entire pod validated
+
+		switch {
+		case apiequality.Semantic.DeepEqual(pod, podCopy):
+			// if it validated without mutating anything, use this result
+			allowedPod = podCopy
+			allowingProvider = provider
+			break loop
+		case specMutationAllowed && allowedPod == nil:
+			// if mutation is allowed and this is the first PSP to allow the pod, remember it,
+			// but continue to see if another PSP allows without mutating
+			allowedPod = podCopy
+			allowingProvider = provider
+			glog.V(6).Infof("pod %s (generate: %s) in namespace %s validated against provider %s with mutation", pod.Name, pod.GenerateName, a.GetNamespace(), provider.GetPSPName())
+		case !specMutationAllowed:
+			glog.V(6).Infof("pod %s (generate: %s) in namespace %s validated against provider %s, but required mutation, skipping", pod.Name, pod.GenerateName, a.GetNamespace(), provider.GetPSPName())
+		}
+	}
+
+	if allowedPod != nil {
+		*pod = *allowedPod
+		// annotate and accept the pod
+		glog.V(4).Infof("pod %s (generate: %s) in namespace %s validated against provider %s", pod.Name, pod.GenerateName, a.GetNamespace(), allowingProvider.GetPSPName())
 		if pod.ObjectMeta.Annotations == nil {
 			pod.ObjectMeta.Annotations = map[string]string{}
 		}
-		pod.ObjectMeta.Annotations[psputil.ValidatedPSPAnnotation] = provider.GetPSPName()
+		pod.ObjectMeta.Annotations[psputil.ValidatedPSPAnnotation] = allowingProvider.GetPSPName()
 		return nil
 	}
 
 	// we didn't validate against any provider, reject the pod and give the errors for each attempt
-	glog.V(4).Infof("unable to validate pod %s (generate: %s) against any pod security policy: %v", pod.Name, pod.GenerateName, validationErrs)
+	glog.V(4).Infof("unable to validate pod %s (generate: %s) in namespace %s against any pod security policy: %v", pod.Name, pod.GenerateName, a.GetNamespace(), validationErrs)
 	return admission.NewForbidden(a, fmt.Errorf("unable to validate against any pod security policy: %v", validationErrs))
 }
 
@@ -176,82 +224,43 @@ func (c *podSecurityPolicyPlugin) Admit(a admission.Attributes) error {
 // and validates that the sc falls within the psp constraints.  All containers must validate against
 // the same psp or is not considered valid.
 func assignSecurityContext(provider psp.Provider, pod *api.Pod, fldPath *field.Path) field.ErrorList {
-	generatedSCs := make([]*api.SecurityContext, len(pod.Spec.Containers))
-	var generatedInitSCs []*api.SecurityContext
-
 	errs := field.ErrorList{}
 
 	psc, pscAnnotations, err := provider.CreatePodSecurityContext(pod)
 	if err != nil {
 		errs = append(errs, field.Invalid(field.NewPath("spec", "securityContext"), pod.Spec.SecurityContext, err.Error()))
 	}
-
-	// save the original PSC and validate the generated PSC.  Leave the generated PSC
-	// set for container generation/validation.  We will reset to original post container
-	// validation.
-	originalPSC := pod.Spec.SecurityContext
 	pod.Spec.SecurityContext = psc
-	originalAnnotations := maps.CopySS(pod.Annotations)
 	pod.Annotations = pscAnnotations
+
 	errs = append(errs, provider.ValidatePodSecurityContext(pod, field.NewPath("spec", "securityContext"))...)
 
-	// Note: this is not changing the original container, we will set container SCs later so long
-	// as all containers validated under the same PSP.
-	for i, containerCopy := range pod.Spec.InitContainers {
-		// We will determine the effective security context for the container and validate against that
-		// since that is how the sc provider will eventually apply settings in the runtime.
-		// This results in an SC that is based on the Pod's PSC with the set fields from the container
-		// overriding pod level settings.
-		containerCopy.SecurityContext = sc.InternalDetermineEffectiveSecurityContext(pod, &containerCopy)
-
-		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
+	for i := range pod.Spec.InitContainers {
+		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.InitContainers[i])
 		if err != nil {
 			errs = append(errs, field.Invalid(field.NewPath("spec", "initContainers").Index(i).Child("securityContext"), "", err.Error()))
 			continue
 		}
-		generatedInitSCs = append(generatedInitSCs, sc)
-
-		containerCopy.SecurityContext = sc
+		pod.Spec.InitContainers[i].SecurityContext = sc
 		pod.Annotations = scAnnotations
-		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &containerCopy, field.NewPath("spec", "initContainers").Index(i).Child("securityContext"))...)
+		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &pod.Spec.InitContainers[i], field.NewPath("spec", "initContainers").Index(i).Child("securityContext"))...)
 	}
 
-	// Note: this is not changing the original container, we will set container SCs later so long
-	// as all containers validated under the same PSP.
-	for i, containerCopy := range pod.Spec.Containers {
-		// We will determine the effective security context for the container and validate against that
-		// since that is how the sc provider will eventually apply settings in the runtime.
-		// This results in an SC that is based on the Pod's PSC with the set fields from the container
-		// overriding pod level settings.
-		containerCopy.SecurityContext = sc.InternalDetermineEffectiveSecurityContext(pod, &containerCopy)
-
-		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &containerCopy)
+	for i := range pod.Spec.Containers {
+		sc, scAnnotations, err := provider.CreateContainerSecurityContext(pod, &pod.Spec.Containers[i])
 		if err != nil {
 			errs = append(errs, field.Invalid(field.NewPath("spec", "containers").Index(i).Child("securityContext"), "", err.Error()))
 			continue
 		}
-		generatedSCs[i] = sc
 
-		containerCopy.SecurityContext = sc
+		pod.Spec.Containers[i].SecurityContext = sc
 		pod.Annotations = scAnnotations
-		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &containerCopy, field.NewPath("spec", "containers").Index(i).Child("securityContext"))...)
+		errs = append(errs, provider.ValidateContainerSecurityContext(pod, &pod.Spec.Containers[i], field.NewPath("spec", "containers").Index(i).Child("securityContext"))...)
 	}
 
 	if len(errs) > 0 {
-		// ensure psc is not mutated if there are errors
-		pod.Spec.SecurityContext = originalPSC
-		pod.Annotations = originalAnnotations
 		return errs
 	}
-
-	// if we've reached this code then we've generated and validated an SC for every container in the
-	// pod so let's apply what we generated.  Note: the psc is already applied.
-	for i, sc := range generatedInitSCs {
-		pod.Spec.InitContainers[i].SecurityContext = sc
-	}
-	for i, sc := range generatedSCs {
-		pod.Spec.Containers[i].SecurityContext = sc
-	}
 	return nil
 }
 
@@ -328,13 +337,13 @@ func buildAttributes(info user.Info, namespace string, policy *extensions.PodSec
 
 // logProviders logs what providers were found for the pod as well as any errors that were encountered
 // while creating providers.
-func logProviders(pod *api.Pod, providers []psp.Provider, providerCreationErrs []error) {
+func logProviders(a admission.Attributes, pod *api.Pod, providers []psp.Provider, providerCreationErrs []error) {
 	for _, err := range providerCreationErrs {
 		glog.V(4).Infof("provider creation error: %v", err)
 	}
 
 	if len(providers) == 0 {
-		glog.V(4).Infof("unable to validate pod %s (generate: %s) against any provider.", pod.Name, pod.GenerateName)
+		glog.V(4).Infof("unable to validate pod %s (generate: %s) in namespace %s against any provider.", pod.Name, pod.GenerateName, a.GetNamespace())
 		return
 	}
 
@@ -342,5 +351,5 @@ func logProviders(pod *api.Pod, providers []psp.Provider, providerCreationErrs [
 	for i, p := range providers {
 		names[i] = p.GetPSPName()
 	}
-	glog.V(4).Infof("validating pod %s (generate: %s) against providers: %s", pod.Name, pod.GenerateName, strings.Join(names, ","))
+	glog.V(4).Infof("validating pod %s (generate: %s) in namespace %s against providers: %s", pod.Name, pod.GenerateName, a.GetNamespace(), strings.Join(names, ","))
 }
diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
index 585f2f8f10d..c43c7fb29f8 100644
--- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
+++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go
@@ -24,6 +24,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -48,7 +50,7 @@ const defaultContainerName = "test-c"
 // an authorizer that always returns true.
 func NewTestAdmission(lister extensionslisters.PodSecurityPolicyLister) kadmission.Interface {
 	return &podSecurityPolicyPlugin{
-		Handler:         kadmission.NewHandler(kadmission.Create),
+		Handler:         kadmission.NewHandler(kadmission.Create, kadmission.Update),
 		strategyFactory: kpsp.NewSimpleStrategyFactory(),
 		pspMatcher:      getMatchingPolicies,
 		authz:           &TestAuthorizer{},
@@ -270,6 +272,173 @@ func TestAdmitPrivileged(t *testing.T) {
 	}
 }
 
+func defaultPod(t *testing.T, pod *kapi.Pod) *kapi.Pod {
+	v1Pod := &v1.Pod{}
+	if err := kapi.Scheme.Convert(pod, v1Pod, nil); err != nil {
+		t.Fatal(err)
+	}
+	kapi.Scheme.Default(v1Pod)
+	apiPod := &kapi.Pod{}
+	if err := kapi.Scheme.Convert(v1Pod, apiPod, nil); err != nil {
+		t.Fatal(err)
+	}
+	return apiPod
+}
+
+func TestAdmitPreferNonmutating(t *testing.T) {
+	mutating1 := restrictivePSP()
+	mutating1.Name = "mutating1"
+	mutating1.Spec.RunAsUser.Ranges = []extensions.UserIDRange{{Min: int64(1), Max: int64(1)}}
+
+	mutating2 := restrictivePSP()
+	mutating2.Name = "mutating2"
+	mutating2.Spec.RunAsUser.Ranges = []extensions.UserIDRange{{Min: int64(2), Max: int64(2)}}
+
+	privilegedPSP := permissivePSP()
+	privilegedPSP.Name = "privileged"
+
+	unprivilegedRunAsAnyPod := defaultPod(t, &kapi.Pod{
+		ObjectMeta: metav1.ObjectMeta{},
+		Spec: kapi.PodSpec{
+			ServiceAccountName: "default",
+			Containers:         []kapi.Container{{Name: "mycontainer", Image: "myimage"}},
+		},
+	})
+	changedPod := unprivilegedRunAsAnyPod.DeepCopy()
+	changedPod.Spec.Containers[0].Image = "myimage2"
+
+	gcChangedPod := unprivilegedRunAsAnyPod.DeepCopy()
+	gcChangedPod.OwnerReferences = []metav1.OwnerReference{{Kind: "Foo", Name: "bar"}}
+	gcChangedPod.Finalizers = []string{"foo"}
+
+	tests := map[string]struct {
+		operation             kadmission.Operation
+		pod                   *kapi.Pod
+		oldPod                *kapi.Pod
+		psps                  []*extensions.PodSecurityPolicy
+		shouldPass            bool
+		expectMutation        bool
+		expectedPodUser       *int64
+		expectedContainerUser *int64
+		expectedPSP           string
+	}{
+		"pod should not be mutated by allow-all strategies": {
+			operation:             kadmission.Create,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{privilegedPSP},
+			shouldPass:            true,
+			expectMutation:        false,
+			expectedPodUser:       nil,
+			expectedContainerUser: nil,
+			expectedPSP:           privilegedPSP.Name,
+		},
+		"pod should prefer non-mutating PSP on create": {
+			operation:             kadmission.Create,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{mutating2, mutating1, privilegedPSP},
+			shouldPass:            true,
+			expectMutation:        false,
+			expectedPodUser:       nil,
+			expectedContainerUser: nil,
+			expectedPSP:           privilegedPSP.Name,
+		},
+		"pod should use deterministic mutating PSP on create": {
+			operation:             kadmission.Create,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{mutating2, mutating1},
+			shouldPass:            true,
+			expectMutation:        true,
+			expectedPodUser:       nil,
+			expectedContainerUser: &mutating1.Spec.RunAsUser.Ranges[0].Min,
+			expectedPSP:           mutating1.Name,
+		},
+		"pod should prefer non-mutating PSP on update": {
+			operation:             kadmission.Update,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			oldPod:                changedPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{mutating2, mutating1, privilegedPSP},
+			shouldPass:            true,
+			expectMutation:        false,
+			expectedPodUser:       nil,
+			expectedContainerUser: nil,
+			expectedPSP:           privilegedPSP.Name,
+		},
+		"pod should not allow mutation on update": {
+			operation:             kadmission.Update,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			oldPod:                changedPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{mutating2, mutating1},
+			shouldPass:            false,
+			expectMutation:        false,
+			expectedPodUser:       nil,
+			expectedContainerUser: nil,
+			expectedPSP:           "",
+		},
+		"pod should be allowed if completely unchanged on update": {
+			operation:             kadmission.Update,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			oldPod:                unprivilegedRunAsAnyPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{mutating2, mutating1},
+			shouldPass:            true,
+			expectMutation:        false,
+			expectedPodUser:       nil,
+			expectedContainerUser: nil,
+			expectedPSP:           "",
+		},
+		"pod should be allowed if unchanged on update except finalizers,ownerrefs": {
+			operation:             kadmission.Update,
+			pod:                   unprivilegedRunAsAnyPod.DeepCopy(),
+			oldPod:                gcChangedPod.DeepCopy(),
+			psps:                  []*extensions.PodSecurityPolicy{mutating2, mutating1},
+			shouldPass:            true,
+			expectMutation:        false,
+			expectedPodUser:       nil,
+			expectedContainerUser: nil,
+			expectedPSP:           "",
+		},
+	}
+
+	for k, v := range tests {
+		testPSPAdmitAdvanced(k, v.operation, v.psps, v.pod, v.oldPod, v.shouldPass, v.expectMutation, v.expectedPSP, t)
+
+		if v.shouldPass {
+			actualPodUser := (*int64)(nil)
+			if v.pod.Spec.SecurityContext != nil {
+				actualPodUser = v.pod.Spec.SecurityContext.RunAsUser
+			}
+			if (actualPodUser == nil) != (v.expectedPodUser == nil) {
+				t.Errorf("%s expected pod user %v, got %v", k, v.expectedPodUser, actualPodUser)
+			} else if actualPodUser != nil && *actualPodUser != *v.expectedPodUser {
+				t.Errorf("%s expected pod user %v, got %v", k, *v.expectedPodUser, *actualPodUser)
+			}
+
+			actualContainerUser := (*int64)(nil)
+			if v.pod.Spec.Containers[0].SecurityContext != nil {
+				actualContainerUser = v.pod.Spec.Containers[0].SecurityContext.RunAsUser
+			}
+			if (actualContainerUser == nil) != (v.expectedContainerUser == nil) {
+				t.Errorf("%s expected container user %v, got %v", k, v.expectedContainerUser, actualContainerUser)
+			} else if actualContainerUser != nil && *actualContainerUser != *v.expectedContainerUser {
+				t.Errorf("%s expected container user %v, got %v", k, *v.expectedContainerUser, *actualContainerUser)
+			}
+		}
+	}
+}
+
+func TestFailClosedOnInvalidPod(t *testing.T) {
+	plugin := NewTestAdmission(nil)
+	pod := &v1.Pod{}
+	attrs := kadmission.NewAttributesRecord(pod, nil, kapi.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, kapi.Resource("pods").WithVersion("version"), "", kadmission.Create, &user.DefaultInfo{})
+	err := plugin.Admit(attrs)
+
+	if err == nil {
+		t.Fatalf("expected versioned pod object to fail admission")
+	}
+	if !strings.Contains(err.Error(), "unexpected type") {
+		t.Errorf("expected type error but got: %v", err)
+	}
+}
+
 func TestAdmitCaps(t *testing.T) {
 	createPodWithCaps := func(caps *kapi.Capabilities) *kapi.Pod {
 		pod := goodPod()
@@ -693,66 +862,125 @@ func TestAdmitHostIPC(t *testing.T) {
 	}
 }
 
-func TestAdmitSELinux(t *testing.T) {
-	createPodWithSELinux := func(opts *kapi.SELinuxOptions) *kapi.Pod {
-		pod := goodPod()
-		// doesn't matter if we set it here or on the container, the
-		// admission controller uses DetermineEffectiveSC to get the defaulting
-		// behavior so it can validate what will be applied at runtime
-		pod.Spec.SecurityContext.SELinuxOptions = opts
-		return pod
-	}
+func createPodWithSecurityContexts(podSC *kapi.PodSecurityContext, containerSC *kapi.SecurityContext) *kapi.Pod {
+	pod := goodPod()
+	pod.Spec.SecurityContext = podSC
+	pod.Spec.Containers[0].SecurityContext = containerSC
+	return pod
+}
 
-	runAsAny := restrictivePSP()
+func TestAdmitSELinux(t *testing.T) {
+	runAsAny := permissivePSP()
 	runAsAny.Name = "runAsAny"
 	runAsAny.Spec.SELinux.Rule = extensions.SELinuxStrategyRunAsAny
+	runAsAny.Spec.SELinux.SELinuxOptions = nil
 
-	mustRunAs := restrictivePSP()
+	mustRunAs := permissivePSP()
 	mustRunAs.Name = "mustRunAs"
+	mustRunAs.Spec.SELinux.Rule = extensions.SELinuxStrategyMustRunAs
+	mustRunAs.Spec.SELinux.SELinuxOptions = &kapi.SELinuxOptions{}
 	mustRunAs.Spec.SELinux.SELinuxOptions.Level = "level"
 	mustRunAs.Spec.SELinux.SELinuxOptions.Role = "role"
 	mustRunAs.Spec.SELinux.SELinuxOptions.Type = "type"
 	mustRunAs.Spec.SELinux.SELinuxOptions.User = "user"
 
 	tests := map[string]struct {
-		pod             *kapi.Pod
-		psps            []*extensions.PodSecurityPolicy
-		shouldPass      bool
-		expectedSELinux *kapi.SELinuxOptions
-		expectedPSP     string
+		pod                 *kapi.Pod
+		psps                []*extensions.PodSecurityPolicy
+		shouldPass          bool
+		expectedPodSC       *kapi.PodSecurityContext
+		expectedContainerSC *kapi.SecurityContext
+		expectedPSP         string
 	}{
-		"runAsAny with no pod request": {
-			pod:             goodPod(),
-			psps:            []*extensions.PodSecurityPolicy{runAsAny},
-			shouldPass:      true,
-			expectedSELinux: nil,
-			expectedPSP:     runAsAny.Name,
+		"runAsAny with no request": {
+			pod:                 createPodWithSecurityContexts(nil, nil),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: nil,
+			expectedPSP:         runAsAny.Name,
 		},
+		"runAsAny with empty pod request": {
+			pod:                 createPodWithSecurityContexts(&kapi.PodSecurityContext{}, nil),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       &kapi.PodSecurityContext{},
+			expectedContainerSC: nil,
+			expectedPSP:         runAsAny.Name,
+		},
+		"runAsAny with empty container request": {
+			pod:                 createPodWithSecurityContexts(nil, &kapi.SecurityContext{}),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: &kapi.SecurityContext{},
+			expectedPSP:         runAsAny.Name,
+		},
+
 		"runAsAny with pod request": {
-			pod:             createPodWithSELinux(&kapi.SELinuxOptions{User: "foo"}),
-			psps:            []*extensions.PodSecurityPolicy{runAsAny},
-			shouldPass:      true,
-			expectedSELinux: &kapi.SELinuxOptions{User: "foo"},
-			expectedPSP:     runAsAny.Name,
+			pod:                 createPodWithSecurityContexts(&kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}}, nil),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       &kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}},
+			expectedContainerSC: nil,
+			expectedPSP:         runAsAny.Name,
 		},
+		"runAsAny with container request": {
+			pod:                 createPodWithSecurityContexts(nil, &kapi.SecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}}),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: &kapi.SecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}},
+			expectedPSP:         runAsAny.Name,
+		},
+		"runAsAny with pod and container request": {
+			pod:                 createPodWithSecurityContexts(&kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "bar"}}, &kapi.SecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}}),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       &kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "bar"}},
+			expectedContainerSC: &kapi.SecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}},
+			expectedPSP:         runAsAny.Name,
+		},
+
 		"mustRunAs with bad pod request": {
-			pod:        createPodWithSELinux(&kapi.SELinuxOptions{User: "foo"}),
+			pod:        createPodWithSecurityContexts(&kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}}, nil),
 			psps:       []*extensions.PodSecurityPolicy{mustRunAs},
 			shouldPass: false,
 		},
-		"mustRunAs with no pod request": {
-			pod:             goodPod(),
-			psps:            []*extensions.PodSecurityPolicy{mustRunAs},
-			shouldPass:      true,
-			expectedSELinux: mustRunAs.Spec.SELinux.SELinuxOptions,
-			expectedPSP:     mustRunAs.Name,
+		"mustRunAs with bad container request": {
+			pod:        createPodWithSecurityContexts(nil, &kapi.SecurityContext{SELinuxOptions: &kapi.SELinuxOptions{User: "foo"}}),
+			psps:       []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass: false,
+		},
+		"mustRunAs with no request": {
+			pod:                 createPodWithSecurityContexts(nil, nil),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       &kapi.PodSecurityContext{SELinuxOptions: mustRunAs.Spec.SELinux.SELinuxOptions},
+			expectedContainerSC: nil,
+			expectedPSP:         mustRunAs.Name,
 		},
 		"mustRunAs with good pod request": {
-			pod:             createPodWithSELinux(&kapi.SELinuxOptions{Level: "level", Role: "role", Type: "type", User: "user"}),
-			psps:            []*extensions.PodSecurityPolicy{mustRunAs},
-			shouldPass:      true,
-			expectedSELinux: mustRunAs.Spec.SELinux.SELinuxOptions,
-			expectedPSP:     mustRunAs.Name,
+			pod: createPodWithSecurityContexts(
+				&kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{Level: "level", Role: "role", Type: "type", User: "user"}},
+				nil,
+			),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       &kapi.PodSecurityContext{SELinuxOptions: mustRunAs.Spec.SELinux.SELinuxOptions},
+			expectedContainerSC: nil,
+			expectedPSP:         mustRunAs.Name,
+		},
+		"mustRunAs with good container request": {
+			pod: createPodWithSecurityContexts(
+				&kapi.PodSecurityContext{SELinuxOptions: &kapi.SELinuxOptions{Level: "level", Role: "role", Type: "type", User: "user"}},
+				nil,
+			),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       &kapi.PodSecurityContext{SELinuxOptions: mustRunAs.Spec.SELinux.SELinuxOptions},
+			expectedContainerSC: nil,
+			expectedPSP:         mustRunAs.Name,
 		},
 	}
 
@@ -760,20 +988,11 @@ func TestAdmitSELinux(t *testing.T) {
 		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
 
 		if v.shouldPass {
-			if v.pod.Spec.Containers[0].SecurityContext.SELinuxOptions == nil && v.expectedSELinux == nil {
-				// ok, don't need to worry about identifying specific diffs
-				continue
+			if !reflect.DeepEqual(v.expectedPodSC, v.pod.Spec.SecurityContext) {
+				t.Errorf("%s unexpected diff:\n%s", k, diff.ObjectGoPrintSideBySide(v.expectedPodSC, v.pod.Spec.SecurityContext))
 			}
-			if v.pod.Spec.Containers[0].SecurityContext.SELinuxOptions == nil && v.expectedSELinux != nil {
-				t.Errorf("%s expected selinux to be: %v but found nil", k, v.expectedSELinux)
-				continue
-			}
-			if v.pod.Spec.Containers[0].SecurityContext.SELinuxOptions != nil && v.expectedSELinux == nil {
-				t.Errorf("%s expected selinux to be nil but found: %v", k, *v.pod.Spec.Containers[0].SecurityContext.SELinuxOptions)
-				continue
-			}
-			if !reflect.DeepEqual(*v.expectedSELinux, *v.pod.Spec.Containers[0].SecurityContext.SELinuxOptions) {
-				t.Errorf("%s expected selinux to be: %v but found %v", k, *v.expectedSELinux, *v.pod.Spec.Containers[0].SecurityContext.SELinuxOptions)
+			if !reflect.DeepEqual(v.expectedContainerSC, v.pod.Spec.Containers[0].SecurityContext) {
+				t.Errorf("%s unexpected diff:\n%s", k, diff.ObjectGoPrintSideBySide(v.expectedContainerSC, v.pod.Spec.Containers[0].SecurityContext))
 			}
 		}
 	}
@@ -859,85 +1078,139 @@ func TestAdmitAppArmor(t *testing.T) {
 }
 
 func TestAdmitRunAsUser(t *testing.T) {
-	createPodWithRunAsUser := func(user int64) *kapi.Pod {
-		pod := goodPod()
-		// doesn't matter if we set it here or on the container, the
-		// admission controller uses DetermineEffectiveSC to get the defaulting
-		// behavior so it can validate what will be applied at runtime
-		userID := int64(user)
-		pod.Spec.SecurityContext.RunAsUser = &userID
-		return pod
+	podSC := func(user *int64) *kapi.PodSecurityContext {
+		return &kapi.PodSecurityContext{RunAsUser: user}
+	}
+	containerSC := func(user *int64) *kapi.SecurityContext {
+		return &kapi.SecurityContext{RunAsUser: user}
 	}
 
-	runAsAny := restrictivePSP()
+	runAsAny := permissivePSP()
 	runAsAny.Name = "runAsAny"
 	runAsAny.Spec.RunAsUser.Rule = extensions.RunAsUserStrategyRunAsAny
 
-	mustRunAs := restrictivePSP()
+	mustRunAs := permissivePSP()
 	mustRunAs.Name = "mustRunAs"
+	mustRunAs.Spec.RunAsUser.Rule = extensions.RunAsUserStrategyMustRunAs
+	mustRunAs.Spec.RunAsUser.Ranges = []extensions.UserIDRange{
+		{Min: int64(999), Max: int64(1000)},
+	}
 
-	runAsNonRoot := restrictivePSP()
+	runAsNonRoot := permissivePSP()
 	runAsNonRoot.Name = "runAsNonRoot"
 	runAsNonRoot.Spec.RunAsUser.Rule = extensions.RunAsUserStrategyMustRunAsNonRoot
 
+	trueValue := true
+
 	tests := map[string]struct {
-		pod               *kapi.Pod
-		psps              []*extensions.PodSecurityPolicy
-		shouldPass        bool
-		expectedRunAsUser *int64
-		expectedPSP       string
+		pod                 *kapi.Pod
+		psps                []*extensions.PodSecurityPolicy
+		shouldPass          bool
+		expectedPodSC       *kapi.PodSecurityContext
+		expectedContainerSC *kapi.SecurityContext
+		expectedPSP         string
 	}{
 		"runAsAny no pod request": {
-			pod:               goodPod(),
-			psps:              []*extensions.PodSecurityPolicy{runAsAny},
-			shouldPass:        true,
-			expectedRunAsUser: nil,
-			expectedPSP:       runAsAny.Name,
+			pod:                 createPodWithSecurityContexts(nil, nil),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: nil,
+			expectedPSP:         runAsAny.Name,
 		},
 		"runAsAny pod request": {
-			pod:               createPodWithRunAsUser(1),
-			psps:              []*extensions.PodSecurityPolicy{runAsAny},
-			shouldPass:        true,
-			expectedRunAsUser: userIDPtr(1),
-			expectedPSP:       runAsAny.Name,
+			pod:                 createPodWithSecurityContexts(podSC(userIDPtr(1)), nil),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       podSC(userIDPtr(1)),
+			expectedContainerSC: nil,
+			expectedPSP:         runAsAny.Name,
 		},
+		"runAsAny container request": {
+			pod:                 createPodWithSecurityContexts(nil, containerSC(userIDPtr(1))),
+			psps:                []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: containerSC(userIDPtr(1)),
+			expectedPSP:         runAsAny.Name,
+		},
+
 		"mustRunAs pod request out of range": {
-			pod:        createPodWithRunAsUser(1),
+			pod:        createPodWithSecurityContexts(podSC(userIDPtr(1)), nil),
 			psps:       []*extensions.PodSecurityPolicy{mustRunAs},
 			shouldPass: false,
 		},
+		"mustRunAs container request out of range": {
+			pod:        createPodWithSecurityContexts(podSC(userIDPtr(999)), containerSC(userIDPtr(1))),
+			psps:       []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass: false,
+		},
+
 		"mustRunAs pod request in range": {
-			pod:               createPodWithRunAsUser(999),
-			psps:              []*extensions.PodSecurityPolicy{mustRunAs},
-			shouldPass:        true,
-			expectedRunAsUser: &mustRunAs.Spec.RunAsUser.Ranges[0].Min,
-			expectedPSP:       mustRunAs.Name,
+			pod:                 createPodWithSecurityContexts(podSC(userIDPtr(999)), nil),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       podSC(&mustRunAs.Spec.RunAsUser.Ranges[0].Min),
+			expectedContainerSC: nil,
+			expectedPSP:         mustRunAs.Name,
 		},
-		"mustRunAs no pod request": {
-			pod:               goodPod(),
-			psps:              []*extensions.PodSecurityPolicy{mustRunAs},
-			shouldPass:        true,
-			expectedRunAsUser: &mustRunAs.Spec.RunAsUser.Ranges[0].Min,
-			expectedPSP:       mustRunAs.Name,
+		"mustRunAs container request in range": {
+			pod:                 createPodWithSecurityContexts(nil, containerSC(userIDPtr(999))),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: containerSC(&mustRunAs.Spec.RunAsUser.Ranges[0].Min),
+			expectedPSP:         mustRunAs.Name,
 		},
-		"runAsNonRoot no pod request": {
-			pod:               goodPod(),
-			psps:              []*extensions.PodSecurityPolicy{runAsNonRoot},
-			shouldPass:        true,
-			expectedRunAsUser: nil,
-			expectedPSP:       runAsNonRoot.Name,
+		"mustRunAs pod and container request in range": {
+			pod:                 createPodWithSecurityContexts(podSC(userIDPtr(999)), containerSC(userIDPtr(1000))),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       podSC(userIDPtr(999)),
+			expectedContainerSC: containerSC(userIDPtr(1000)),
+			expectedPSP:         mustRunAs.Name,
+		},
+		"mustRunAs no request": {
+			pod:                 createPodWithSecurityContexts(nil, nil),
+			psps:                []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: containerSC(&mustRunAs.Spec.RunAsUser.Ranges[0].Min),
+			expectedPSP:         mustRunAs.Name,
+		},
+
+		"runAsNonRoot no request": {
+			pod:                 createPodWithSecurityContexts(nil, nil),
+			psps:                []*extensions.PodSecurityPolicy{runAsNonRoot},
+			shouldPass:          true,
+			expectedPodSC:       nil,
+			expectedContainerSC: &kapi.SecurityContext{RunAsNonRoot: &trueValue},
+			expectedPSP:         runAsNonRoot.Name,
 		},
 		"runAsNonRoot pod request root": {
-			pod:        createPodWithRunAsUser(0),
+			pod:        createPodWithSecurityContexts(podSC(userIDPtr(0)), nil),
 			psps:       []*extensions.PodSecurityPolicy{runAsNonRoot},
 			shouldPass: false,
 		},
 		"runAsNonRoot pod request non-root": {
-			pod:               createPodWithRunAsUser(1),
-			psps:              []*extensions.PodSecurityPolicy{runAsNonRoot},
-			shouldPass:        true,
-			expectedRunAsUser: userIDPtr(1),
-			expectedPSP:       runAsNonRoot.Name,
+			pod:           createPodWithSecurityContexts(podSC(userIDPtr(1)), nil),
+			psps:          []*extensions.PodSecurityPolicy{runAsNonRoot},
+			shouldPass:    true,
+			expectedPodSC: podSC(userIDPtr(1)),
+			expectedPSP:   runAsNonRoot.Name,
+		},
+		"runAsNonRoot container request root": {
+			pod:        createPodWithSecurityContexts(podSC(userIDPtr(1)), containerSC(userIDPtr(0))),
+			psps:       []*extensions.PodSecurityPolicy{runAsNonRoot},
+			shouldPass: false,
+		},
+		"runAsNonRoot container request non-root": {
+			pod:                 createPodWithSecurityContexts(podSC(userIDPtr(1)), containerSC(userIDPtr(2))),
+			psps:                []*extensions.PodSecurityPolicy{runAsNonRoot},
+			shouldPass:          true,
+			expectedPodSC:       podSC(userIDPtr(1)),
+			expectedContainerSC: containerSC(userIDPtr(2)),
+			expectedPSP:         runAsNonRoot.Name,
 		},
 	}
 
@@ -945,82 +1218,83 @@ func TestAdmitRunAsUser(t *testing.T) {
 		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
 
 		if v.shouldPass {
-			if v.pod.Spec.Containers[0].SecurityContext.RunAsUser == nil && v.expectedRunAsUser == nil {
-				// ok, don't need to worry about identifying specific diffs
-				continue
+			if !reflect.DeepEqual(v.expectedPodSC, v.pod.Spec.SecurityContext) {
+				t.Errorf("%s unexpected pod sc diff:\n%s", k, diff.ObjectGoPrintSideBySide(v.expectedPodSC, v.pod.Spec.SecurityContext))
 			}
-			if v.pod.Spec.Containers[0].SecurityContext.RunAsUser == nil && v.expectedRunAsUser != nil {
-				t.Errorf("%s expected RunAsUser to be: %v but found nil", k, v.expectedRunAsUser)
-				continue
-			}
-			if v.pod.Spec.Containers[0].SecurityContext.RunAsUser != nil && v.expectedRunAsUser == nil {
-				t.Errorf("%s expected RunAsUser to be nil but found: %v", k, *v.pod.Spec.Containers[0].SecurityContext.RunAsUser)
-				continue
-			}
-			if *v.expectedRunAsUser != *v.pod.Spec.Containers[0].SecurityContext.RunAsUser {
-				t.Errorf("%s expected RunAsUser to be: %v but found %v", k, *v.expectedRunAsUser, *v.pod.Spec.Containers[0].SecurityContext.RunAsUser)
+			if !reflect.DeepEqual(v.expectedContainerSC, v.pod.Spec.Containers[0].SecurityContext) {
+				t.Errorf("%s unexpected container sc diff:\n%s", k, diff.ObjectGoPrintSideBySide(v.expectedContainerSC, v.pod.Spec.Containers[0].SecurityContext))
 			}
 		}
 	}
 }
 
 func TestAdmitSupplementalGroups(t *testing.T) {
-	createPodWithSupGroup := func(group int64) *kapi.Pod {
-		pod := goodPod()
-		// doesn't matter if we set it here or on the container, the
-		// admission controller uses DetermineEffectiveSC to get the defaulting
-		// behavior so it can validate what will be applied at runtime
-		groupID := int64(group)
-		pod.Spec.SecurityContext.SupplementalGroups = []int64{groupID}
-		return pod
+	podSC := func(group int64) *kapi.PodSecurityContext {
+		return &kapi.PodSecurityContext{SupplementalGroups: []int64{group}}
 	}
 
-	runAsAny := restrictivePSP()
+	runAsAny := permissivePSP()
 	runAsAny.Name = "runAsAny"
 	runAsAny.Spec.SupplementalGroups.Rule = extensions.SupplementalGroupsStrategyRunAsAny
 
-	mustRunAs := restrictivePSP()
+	mustRunAs := permissivePSP()
 	mustRunAs.Name = "mustRunAs"
+	mustRunAs.Spec.SupplementalGroups.Rule = extensions.SupplementalGroupsStrategyMustRunAs
+	mustRunAs.Spec.SupplementalGroups.Ranges = []extensions.GroupIDRange{{Min: int64(999), Max: int64(1000)}}
 
 	tests := map[string]struct {
-		pod               *kapi.Pod
-		psps              []*extensions.PodSecurityPolicy
-		shouldPass        bool
-		expectedSupGroups []int64
-		expectedPSP       string
+		pod           *kapi.Pod
+		psps          []*extensions.PodSecurityPolicy
+		shouldPass    bool
+		expectedPodSC *kapi.PodSecurityContext
+		expectedPSP   string
 	}{
 		"runAsAny no pod request": {
-			pod:               goodPod(),
-			psps:              []*extensions.PodSecurityPolicy{runAsAny},
-			shouldPass:        true,
-			expectedSupGroups: nil,
-			expectedPSP:       runAsAny.Name,
+			pod:           createPodWithSecurityContexts(nil, nil),
+			psps:          []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:    true,
+			expectedPodSC: nil,
+			expectedPSP:   runAsAny.Name,
+		},
+		"runAsAny empty pod request": {
+			pod:           createPodWithSecurityContexts(&kapi.PodSecurityContext{}, nil),
+			psps:          []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:    true,
+			expectedPodSC: &kapi.PodSecurityContext{},
+			expectedPSP:   runAsAny.Name,
+		},
+		"runAsAny empty pod request empty supplemental groups": {
+			pod:           createPodWithSecurityContexts(&kapi.PodSecurityContext{SupplementalGroups: []int64{}}, nil),
+			psps:          []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:    true,
+			expectedPodSC: &kapi.PodSecurityContext{SupplementalGroups: []int64{}},
+			expectedPSP:   runAsAny.Name,
 		},
 		"runAsAny pod request": {
-			pod:               createPodWithSupGroup(1),
-			psps:              []*extensions.PodSecurityPolicy{runAsAny},
-			shouldPass:        true,
-			expectedSupGroups: []int64{1},
-			expectedPSP:       runAsAny.Name,
+			pod:           createPodWithSecurityContexts(podSC(1), nil),
+			psps:          []*extensions.PodSecurityPolicy{runAsAny},
+			shouldPass:    true,
+			expectedPodSC: &kapi.PodSecurityContext{SupplementalGroups: []int64{1}},
+			expectedPSP:   runAsAny.Name,
 		},
 		"mustRunAs no pod request": {
-			pod:               goodPod(),
-			psps:              []*extensions.PodSecurityPolicy{mustRunAs},
-			shouldPass:        true,
-			expectedSupGroups: []int64{mustRunAs.Spec.SupplementalGroups.Ranges[0].Min},
-			expectedPSP:       mustRunAs.Name,
+			pod:           createPodWithSecurityContexts(nil, nil),
+			psps:          []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:    true,
+			expectedPodSC: podSC(mustRunAs.Spec.SupplementalGroups.Ranges[0].Min),
+			expectedPSP:   mustRunAs.Name,
 		},
 		"mustRunAs bad pod request": {
-			pod:        createPodWithSupGroup(1),
+			pod:        createPodWithSecurityContexts(podSC(1), nil),
 			psps:       []*extensions.PodSecurityPolicy{mustRunAs},
 			shouldPass: false,
 		},
 		"mustRunAs good pod request": {
-			pod:               createPodWithSupGroup(999),
-			psps:              []*extensions.PodSecurityPolicy{mustRunAs},
-			shouldPass:        true,
-			expectedSupGroups: []int64{999},
-			expectedPSP:       mustRunAs.Name,
+			pod:           createPodWithSecurityContexts(podSC(999), nil),
+			psps:          []*extensions.PodSecurityPolicy{mustRunAs},
+			shouldPass:    true,
+			expectedPodSC: podSC(999),
+			expectedPSP:   mustRunAs.Name,
 		},
 	}
 
@@ -1028,16 +1302,8 @@ func TestAdmitSupplementalGroups(t *testing.T) {
 		testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
 
 		if v.shouldPass {
-			if v.pod.Spec.SecurityContext.SupplementalGroups == nil && v.expectedSupGroups != nil {
-				t.Errorf("%s expected SupplementalGroups to be: %v but found nil", k, v.expectedSupGroups)
-				continue
-			}
-			if v.pod.Spec.SecurityContext.SupplementalGroups != nil && v.expectedSupGroups == nil {
-				t.Errorf("%s expected SupplementalGroups to be nil but found: %v", k, v.pod.Spec.SecurityContext.SupplementalGroups)
-				continue
-			}
-			if !reflect.DeepEqual(v.expectedSupGroups, v.pod.Spec.SecurityContext.SupplementalGroups) {
-				t.Errorf("%s expected SupplementalGroups to be: %v but found %v", k, v.expectedSupGroups, v.pod.Spec.SecurityContext.SupplementalGroups)
+			if !reflect.DeepEqual(v.expectedPodSC, v.pod.Spec.SecurityContext) {
+				t.Errorf("%s unexpected pod sc diff:\n%s", k, diff.ObjectGoPrintSideBySide(v.expectedPodSC, v.pod.Spec.SecurityContext))
 			}
 		}
 	}
@@ -1372,6 +1638,10 @@ func TestAdmitSysctls(t *testing.T) {
 }
 
 func testPSPAdmit(testCaseName string, psps []*extensions.PodSecurityPolicy, pod *kapi.Pod, shouldPass bool, expectedPSP string, t *testing.T) {
+	testPSPAdmitAdvanced(testCaseName, kadmission.Create, psps, pod, nil, shouldPass, true, expectedPSP, t)
+}
+
+func testPSPAdmitAdvanced(testCaseName string, op kadmission.Operation, psps []*extensions.PodSecurityPolicy, pod, oldPod *kapi.Pod, shouldPass bool, canMutate bool, expectedPSP string, t *testing.T) {
 	informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc())
 	store := informerFactory.Extensions().InternalVersion().PodSecurityPolicies().Informer().GetStore()
 
@@ -1379,9 +1649,11 @@ func testPSPAdmit(testCaseName string, psps []*extensions.PodSecurityPolicy, pod
 		store.Add(psp)
 	}
 
+	originalPod := pod.DeepCopy()
+
 	plugin := NewTestAdmission(informerFactory.Extensions().InternalVersion().PodSecurityPolicies().Lister())
 
-	attrs := kadmission.NewAttributesRecord(pod, nil, kapi.Kind("Pod").WithVersion("version"), "namespace", "", kapi.Resource("pods").WithVersion("version"), "", kadmission.Create, &user.DefaultInfo{})
+	attrs := kadmission.NewAttributesRecord(pod, oldPod, kapi.Kind("Pod").WithVersion("version"), "namespace", "", kapi.Resource("pods").WithVersion("version"), "", op, &user.DefaultInfo{})
 	err := plugin.Admit(attrs)
 
 	if shouldPass && err != nil {
@@ -1392,6 +1664,18 @@ func testPSPAdmit(testCaseName string, psps []*extensions.PodSecurityPolicy, pod
 		if pod.Annotations[psputil.ValidatedPSPAnnotation] != expectedPSP {
 			t.Errorf("%s: expected to validate under %q PSP but found %q", testCaseName, expectedPSP, pod.Annotations[psputil.ValidatedPSPAnnotation])
 		}
+
+		if !canMutate {
+			podWithoutPSPAnnotation := pod.DeepCopy()
+			delete(podWithoutPSPAnnotation.Annotations, psputil.ValidatedPSPAnnotation)
+
+			originalPodWithoutPSPAnnotation := originalPod.DeepCopy()
+			delete(originalPodWithoutPSPAnnotation.Annotations, psputil.ValidatedPSPAnnotation)
+
+			if !apiequality.Semantic.DeepEqual(originalPodWithoutPSPAnnotation.Spec, podWithoutPSPAnnotation.Spec) {
+				t.Errorf("%s: expected no mutation, got %s", testCaseName, diff.ObjectGoPrintSideBySide(originalPodWithoutPSPAnnotation.Spec, podWithoutPSPAnnotation.Spec))
+			}
+		}
 	}
 
 	if !shouldPass && err == nil {
@@ -1415,10 +1699,6 @@ func TestAssignSecurityContext(t *testing.T) {
 		}
 	}
 
-	// these are set up such that the containers always have a nil uid.  If the case should not
-	// validate then the uids should not have been updated by the strategy.  If the case should
-	// validate then uids should be set.  This is ensuring that we're hanging on to the old SC
-	// as we generate/validate and only updating the original container if the entire pod validates
 	testCases := map[string]struct {
 		pod            *kapi.Pod
 		shouldValidate bool
@@ -1464,24 +1744,6 @@ func TestAssignSecurityContext(t *testing.T) {
 			t.Errorf("%s expected validation errors but received none", k)
 			continue
 		}
-
-		// if we shouldn't have validated ensure that uid is not set on the containers
-		if !v.shouldValidate {
-			for _, c := range v.pod.Spec.Containers {
-				if c.SecurityContext.RunAsUser != nil {
-					t.Errorf("%s had non-nil UID %d.  UID should not be set on test cases that don't validate", k, *c.SecurityContext.RunAsUser)
-				}
-			}
-		}
-
-		// if we validated then the pod sc should be updated now with the defaults from the psp
-		if v.shouldValidate {
-			for _, c := range v.pod.Spec.Containers {
-				if *c.SecurityContext.RunAsUser != 999 {
-					t.Errorf("%s expected uid to be defaulted to 999 but found %v", k, *c.SecurityContext.RunAsUser)
-				}
-			}
-		}
 	}
 }
 
@@ -1765,6 +2027,36 @@ func restrictivePSP() *extensions.PodSecurityPolicy {
 	}
 }
 
+func permissivePSP() *extensions.PodSecurityPolicy {
+	return &extensions.PodSecurityPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "privileged",
+			Annotations: map[string]string{},
+		},
+		Spec: extensions.PodSecurityPolicySpec{
+			AllowPrivilegeEscalation: true,
+			HostIPC:                  true,
+			HostNetwork:              true,
+			HostPID:                  true,
+			HostPorts:                []extensions.HostPortRange{{Min: 0, Max: 65536}},
+			Volumes:                  []extensions.FSType{extensions.All},
+			AllowedCapabilities:      []kapi.Capability{extensions.AllowAllCapabilities},
+			RunAsUser: extensions.RunAsUserStrategyOptions{
+				Rule: extensions.RunAsUserStrategyRunAsAny,
+			},
+			SELinux: extensions.SELinuxStrategyOptions{
+				Rule: extensions.SELinuxStrategyRunAsAny,
+			},
+			FSGroup: extensions.FSGroupStrategyOptions{
+				Rule: extensions.FSGroupStrategyRunAsAny,
+			},
+			SupplementalGroups: extensions.SupplementalGroupsStrategyOptions{
+				Rule: extensions.SupplementalGroupsStrategyRunAsAny,
+			},
+		},
+	}
+}
+
 func createNamespaceForTest() *kapi.Namespace {
 	return &kapi.Namespace{
 		ObjectMeta: metav1.ObjectMeta{