admission: don't update psp annotation on update

This commit is contained in:
Dr. Stefan Schimanski 2017-11-10 15:58:37 +01:00
parent 1a4a019e5f
commit 3d5849fd54
2 changed files with 25 additions and 19 deletions

View File

@ -119,11 +119,16 @@ func (c *PodSecurityPolicyPlugin) Admit(a admission.Attributes) error {
return nil return nil
} }
// only mutate if this is a CREATE request. On updates we only validate.
// TODO(liggitt): allow spec mutation during initializing updates?
if a.GetOperation() != admission.Create {
return nil
}
pod := a.GetObject().(*api.Pod) pod := a.GetObject().(*api.Pod)
// compute the context. If the current security context is valid, this call won't change it. // compute the context
allowMutation := a.GetOperation() == admission.Create // TODO(liggitt): allow spec mutation during initializing updates? allowedPod, pspName, validationErrs, err := c.computeSecurityContext(a, pod, true)
allowedPod, pspName, validationErrs, err := c.computeSecurityContext(a, pod, allowMutation)
if err != nil { if err != nil {
return admission.NewForbidden(a, err) return admission.NewForbidden(a, err)
} }
@ -134,10 +139,6 @@ func (c *PodSecurityPolicyPlugin) Admit(a admission.Attributes) error {
if pod.ObjectMeta.Annotations == nil { if pod.ObjectMeta.Annotations == nil {
pod.ObjectMeta.Annotations = map[string]string{} pod.ObjectMeta.Annotations = map[string]string{}
} }
// set annotation to mark this as passed. Note, that the actual value is not important, the
// validating PSP might even change later-on. Also not that pspName can be the empty string
// if failOnNoPolicies is false.
// TODO: if failOnNoPolicies is toggled from false to true, we will never update the annotation anymore. Is this desired?
pod.ObjectMeta.Annotations[psputil.ValidatedPSPAnnotation] = pspName pod.ObjectMeta.Annotations[psputil.ValidatedPSPAnnotation] = pspName
return nil return nil
} }
@ -156,7 +157,7 @@ func (c *PodSecurityPolicyPlugin) Validate(a admission.Attributes) error {
pod := a.GetObject().(*api.Pod) pod := a.GetObject().(*api.Pod)
// compute the context. If the current security context is valid, this call won't change it. // compute the context. Mutation is not allowed.
allowedPod, _, validationErrs, err := c.computeSecurityContext(a, pod, false) allowedPod, _, validationErrs, err := c.computeSecurityContext(a, pod, false)
if err != nil { if err != nil {
return admission.NewForbidden(a, err) return admission.NewForbidden(a, err)

View File

@ -329,6 +329,11 @@ func TestAdmitPreferNonmutating(t *testing.T) {
changedPod := unprivilegedRunAsAnyPod.DeepCopy() changedPod := unprivilegedRunAsAnyPod.DeepCopy()
changedPod.Spec.Containers[0].Image = "myimage2" changedPod.Spec.Containers[0].Image = "myimage2"
podWithSC := unprivilegedRunAsAnyPod.DeepCopy()
podWithSC.Annotations = map[string]string{psputil.ValidatedPSPAnnotation: privilegedPSP.Name}
changedPodWithSC := changedPod.DeepCopy()
changedPodWithSC.Annotations = map[string]string{psputil.ValidatedPSPAnnotation: privilegedPSP.Name}
gcChangedPod := unprivilegedRunAsAnyPod.DeepCopy() gcChangedPod := unprivilegedRunAsAnyPod.DeepCopy()
gcChangedPod.OwnerReferences = []metav1.OwnerReference{{Kind: "Foo", Name: "bar"}} gcChangedPod.OwnerReferences = []metav1.OwnerReference{{Kind: "Foo", Name: "bar"}}
gcChangedPod.Finalizers = []string{"foo"} gcChangedPod.Finalizers = []string{"foo"}
@ -336,7 +341,7 @@ func TestAdmitPreferNonmutating(t *testing.T) {
tests := map[string]struct { tests := map[string]struct {
operation kadmission.Operation operation kadmission.Operation
pod *kapi.Pod pod *kapi.Pod
oldPod *kapi.Pod podBeforeUpdate *kapi.Pod
psps []*extensions.PodSecurityPolicy psps []*extensions.PodSecurityPolicy
shouldPassAdmit bool shouldPassAdmit bool
shouldPassValidate bool shouldPassValidate bool
@ -380,8 +385,8 @@ func TestAdmitPreferNonmutating(t *testing.T) {
}, },
"pod should prefer non-mutating PSP on update": { "pod should prefer non-mutating PSP on update": {
operation: kadmission.Update, operation: kadmission.Update,
pod: unprivilegedRunAsAnyPod.DeepCopy(), pod: changedPodWithSC.DeepCopy(),
oldPod: changedPod.DeepCopy(), podBeforeUpdate: podWithSC.DeepCopy(),
psps: []*extensions.PodSecurityPolicy{mutating2, mutating1, privilegedPSP}, psps: []*extensions.PodSecurityPolicy{mutating2, mutating1, privilegedPSP},
shouldPassAdmit: true, shouldPassAdmit: true,
shouldPassValidate: true, shouldPassValidate: true,
@ -390,12 +395,12 @@ func TestAdmitPreferNonmutating(t *testing.T) {
expectedContainerUser: nil, expectedContainerUser: nil,
expectedPSP: privilegedPSP.Name, expectedPSP: privilegedPSP.Name,
}, },
"pod should not allow mutation on update": { "pod should not mutate on update, but fail validation": {
operation: kadmission.Update, operation: kadmission.Update,
pod: unprivilegedRunAsAnyPod.DeepCopy(), pod: changedPod.DeepCopy(),
oldPod: changedPod.DeepCopy(), podBeforeUpdate: unprivilegedRunAsAnyPod.DeepCopy(),
psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, psps: []*extensions.PodSecurityPolicy{mutating2, mutating1},
shouldPassAdmit: false, shouldPassAdmit: true,
shouldPassValidate: false, shouldPassValidate: false,
expectMutation: false, expectMutation: false,
expectedPodUser: nil, expectedPodUser: nil,
@ -405,7 +410,7 @@ func TestAdmitPreferNonmutating(t *testing.T) {
"pod should be allowed if completely unchanged on update": { "pod should be allowed if completely unchanged on update": {
operation: kadmission.Update, operation: kadmission.Update,
pod: unprivilegedRunAsAnyPod.DeepCopy(), pod: unprivilegedRunAsAnyPod.DeepCopy(),
oldPod: unprivilegedRunAsAnyPod.DeepCopy(), podBeforeUpdate: unprivilegedRunAsAnyPod.DeepCopy(),
psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, psps: []*extensions.PodSecurityPolicy{mutating2, mutating1},
shouldPassAdmit: true, shouldPassAdmit: true,
shouldPassValidate: true, shouldPassValidate: true,
@ -416,8 +421,8 @@ func TestAdmitPreferNonmutating(t *testing.T) {
}, },
"pod should be allowed if unchanged on update except finalizers,ownerrefs": { "pod should be allowed if unchanged on update except finalizers,ownerrefs": {
operation: kadmission.Update, operation: kadmission.Update,
pod: unprivilegedRunAsAnyPod.DeepCopy(), pod: gcChangedPod.DeepCopy(),
oldPod: gcChangedPod.DeepCopy(), podBeforeUpdate: unprivilegedRunAsAnyPod.DeepCopy(),
psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, psps: []*extensions.PodSecurityPolicy{mutating2, mutating1},
shouldPassAdmit: true, shouldPassAdmit: true,
shouldPassValidate: true, shouldPassValidate: true,
@ -429,7 +434,7 @@ func TestAdmitPreferNonmutating(t *testing.T) {
} }
for k, v := range tests { for k, v := range tests {
testPSPAdmitAdvanced(k, v.operation, v.psps, v.pod, v.oldPod, v.shouldPassAdmit, v.shouldPassValidate, v.expectMutation, v.expectedPSP, t) testPSPAdmitAdvanced(k, v.operation, v.psps, v.pod, v.podBeforeUpdate, v.shouldPassAdmit, v.shouldPassValidate, v.expectMutation, v.expectedPSP, t)
if v.shouldPassAdmit { if v.shouldPassAdmit {
actualPodUser := (*int64)(nil) actualPodUser := (*int64)(nil)