From 2b95212ad339e00de1cffd49a0de67ce6058ddd0 Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Fri, 24 Nov 2017 17:10:39 +0100 Subject: [PATCH 1/2] admission_test.go(TestAdmitPreferNonmutating): simplify test by replacing expectedPodUser by a constant value. --- .../security/podsecuritypolicy/admission_test.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go index 2b99c0b4c29..0aae6fafec0 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go @@ -352,7 +352,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit bool shouldPassValidate bool expectMutation bool - expectedPodUser *int64 expectedContainerUser *int64 expectedPSP string }{ @@ -363,7 +362,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, - expectedPodUser: nil, expectedContainerUser: nil, expectedPSP: privilegedPSP.Name, }, @@ -374,7 +372,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, - expectedPodUser: nil, expectedContainerUser: nil, expectedPSP: privilegedPSP.Name, }, @@ -385,7 +382,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: true, expectMutation: true, - expectedPodUser: nil, expectedContainerUser: &mutating1.Spec.RunAsUser.Ranges[0].Min, expectedPSP: mutating1.Name, }, @@ -397,7 +393,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, - expectedPodUser: nil, expectedContainerUser: nil, expectedPSP: privilegedPSP.Name, }, @@ -409,7 +404,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: false, expectMutation: false, - expectedPodUser: nil, expectedContainerUser: nil, expectedPSP: "", }, @@ -421,7 +415,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, - expectedPodUser: nil, expectedContainerUser: nil, expectedPSP: "", }, @@ -433,7 +426,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, - expectedPodUser: nil, expectedContainerUser: nil, expectedPSP: "", }, @@ -447,10 +439,8 @@ func TestAdmitPreferNonmutating(t *testing.T) { 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) + if actualPodUser != nil { + t.Errorf("%s expected pod user nil, got %v", k, *actualPodUser) } actualContainerUser := (*int64)(nil) From b1ae1d67b209f445a1254eb7a53a14bdcabebf3d Mon Sep 17 00:00:00 2001 From: Slava Semushin Date: Fri, 24 Nov 2017 17:11:51 +0100 Subject: [PATCH 2/2] admission_test.go(TestAdmitPreferNonmutating): simplify test by replacing shouldPassAdmit by a constant value. --- .../podsecuritypolicy/admission_test.go | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go index 0aae6fafec0..8b2c2b701ed 100644 --- a/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go +++ b/plugin/pkg/admission/security/podsecuritypolicy/admission_test.go @@ -349,7 +349,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { pod *kapi.Pod podBeforeUpdate *kapi.Pod psps []*extensions.PodSecurityPolicy - shouldPassAdmit bool shouldPassValidate bool expectMutation bool expectedContainerUser *int64 @@ -359,7 +358,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { operation: kadmission.Create, pod: unprivilegedRunAsAnyPod.DeepCopy(), psps: []*extensions.PodSecurityPolicy{privilegedPSP}, - shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, expectedContainerUser: nil, @@ -369,7 +367,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { operation: kadmission.Create, pod: unprivilegedRunAsAnyPod.DeepCopy(), psps: []*extensions.PodSecurityPolicy{mutating2, mutating1, privilegedPSP}, - shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, expectedContainerUser: nil, @@ -379,7 +376,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { operation: kadmission.Create, pod: unprivilegedRunAsAnyPod.DeepCopy(), psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, - shouldPassAdmit: true, shouldPassValidate: true, expectMutation: true, expectedContainerUser: &mutating1.Spec.RunAsUser.Ranges[0].Min, @@ -390,7 +386,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { pod: changedPodWithSC.DeepCopy(), podBeforeUpdate: podWithSC.DeepCopy(), psps: []*extensions.PodSecurityPolicy{mutating2, mutating1, privilegedPSP}, - shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, expectedContainerUser: nil, @@ -401,7 +396,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { pod: changedPod.DeepCopy(), podBeforeUpdate: unprivilegedRunAsAnyPod.DeepCopy(), psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, - shouldPassAdmit: true, shouldPassValidate: false, expectMutation: false, expectedContainerUser: nil, @@ -412,7 +406,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { pod: unprivilegedRunAsAnyPod.DeepCopy(), podBeforeUpdate: unprivilegedRunAsAnyPod.DeepCopy(), psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, - shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, expectedContainerUser: nil, @@ -423,7 +416,6 @@ func TestAdmitPreferNonmutating(t *testing.T) { pod: gcChangedPod.DeepCopy(), podBeforeUpdate: unprivilegedRunAsAnyPod.DeepCopy(), psps: []*extensions.PodSecurityPolicy{mutating2, mutating1}, - shouldPassAdmit: true, shouldPassValidate: true, expectMutation: false, expectedContainerUser: nil, @@ -432,26 +424,24 @@ func TestAdmitPreferNonmutating(t *testing.T) { } for k, v := range tests { - testPSPAdmitAdvanced(k, v.operation, v.psps, nil, &user.DefaultInfo{}, v.pod, v.podBeforeUpdate, v.shouldPassAdmit, v.shouldPassValidate, v.expectMutation, v.expectedPSP, t) + testPSPAdmitAdvanced(k, v.operation, v.psps, nil, &user.DefaultInfo{}, v.pod, v.podBeforeUpdate, true, v.shouldPassValidate, v.expectMutation, v.expectedPSP, t) - if v.shouldPassAdmit { - actualPodUser := (*int64)(nil) - if v.pod.Spec.SecurityContext != nil { - actualPodUser = v.pod.Spec.SecurityContext.RunAsUser - } - if actualPodUser != nil { - t.Errorf("%s expected pod user nil, got %v", k, *actualPodUser) - } + actualPodUser := (*int64)(nil) + if v.pod.Spec.SecurityContext != nil { + actualPodUser = v.pod.Spec.SecurityContext.RunAsUser + } + if actualPodUser != nil { + t.Errorf("%s expected pod user nil, got %v", k, *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) - } + 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) } } }