diff --git a/pkg/security/podsecuritypolicy/BUILD b/pkg/security/podsecuritypolicy/BUILD index abfd148dc41..02668dc0486 100644 --- a/pkg/security/podsecuritypolicy/BUILD +++ b/pkg/security/podsecuritypolicy/BUILD @@ -49,8 +49,6 @@ go_test( "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", - "//vendor/github.com/davecgh/go-spew/spew:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", ], diff --git a/pkg/security/podsecuritypolicy/provider_test.go b/pkg/security/podsecuritypolicy/provider_test.go index a833da80102..8f7046b74e3 100644 --- a/pkg/security/podsecuritypolicy/provider_test.go +++ b/pkg/security/podsecuritypolicy/provider_test.go @@ -20,10 +20,8 @@ import ( "fmt" "reflect" "strconv" - "strings" "testing" - "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -85,13 +83,9 @@ func TestMutatePodNonmutating(t *testing.T) { psp := createPSP() provider, err := NewSimpleProvider(psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Fatalf("unable to create provider %v", err) - } + require.NoError(t, err, "unable to create provider") err = provider.MutatePod(pod) - if err != nil { - t.Fatalf("unable to create psc %v", err) - } + require.NoError(t, err, "unable to modify pod") // Creating the provider or the security context should not have mutated the psp or pod // since all the strategies were permissive @@ -160,13 +154,9 @@ func TestMutateContainerNonmutating(t *testing.T) { psp := createPSP() provider, err := NewSimpleProvider(psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Fatalf("unable to create provider %v", err) - } + require.NoError(t, err, "unable to create provider") err = provider.MutatePod(pod) - if err != nil { - t.Fatalf("unable to create container security context %v", err) - } + require.NoError(t, err, "unable to modify pod") // Creating the provider or the security context should not have mutated the psp or pod // since all the strategies were permissive @@ -443,19 +433,14 @@ func TestValidatePodFailures(t *testing.T) { expectedError: "Flexvolume driver is not allowed to be used", }, } - for k, v := range errorCases { - provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Fatalf("unable to create provider %v", err) - } - errs := provider.ValidatePod(v.pod) - if len(errs) == 0 { - t.Errorf("%s expected validation failure but did not receive errors", k) - continue - } - if !strings.Contains(errs[0].Error(), v.expectedError) { - t.Errorf("%s received unexpected error %v", k, errs) - } + for name, test := range errorCases { + t.Run(name, func(t *testing.T) { + provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "unable to create provider") + errs := provider.ValidatePod(test.pod) + require.NotEmpty(t, errs, "expected validation failure but did not receive errors") + assert.Contains(t, errs[0].Error(), test.expectedError, "received unexpected error") + }) } } @@ -618,20 +603,13 @@ func TestValidateContainerFailures(t *testing.T) { }, } - for k, v := range errorCases { - t.Run(k, func(t *testing.T) { - provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Fatalf("unable to create provider %v", err) - } - errs := provider.ValidatePod(v.pod) - if len(errs) == 0 { - t.Errorf("expected validation failure but did not receive errors") - return - } - if !strings.Contains(errs[0].Error(), v.expectedError) { - t.Errorf("unexpected error %v\nexpected: %s", errs, v.expectedError) - } + for name, test := range errorCases { + t.Run(name, func(t *testing.T) { + provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "unable to create provider") + errs := provider.ValidatePod(test.pod) + require.NotEmpty(t, errs, "expected validation failure but did not receive errors") + assert.Contains(t, errs[0].Error(), test.expectedError, "unexpected error") }) } } @@ -909,16 +887,13 @@ func TestValidatePodSuccess(t *testing.T) { }, } - for k, v := range successCases { - provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Fatalf("unable to create provider %v", err) - } - errs := provider.ValidatePod(v.pod) - if len(errs) != 0 { - t.Errorf("%s expected validation pass but received errors %v", k, errs) - continue - } + for name, test := range successCases { + t.Run(name, func(t *testing.T) { + provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "unable to create provider") + errs := provider.ValidatePod(test.pod) + assert.Empty(t, errs, "expected validation pass but received errors") + }) } } @@ -1076,16 +1051,12 @@ func TestValidateContainerSuccess(t *testing.T) { }, } - for k, v := range successCases { - t.Run(k, func(t *testing.T) { - provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Fatalf("unable to create provider %v", err) - } - errs := provider.ValidatePod(v.pod) - if len(errs) != 0 { - t.Errorf("%s expected validation pass but received errors %v\n%s", k, errs, spew.Sdump(v.pod.ObjectMeta)) - } + for name, test := range successCases { + t.Run(name, func(t *testing.T) { + provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "unable to create provider") + errs := provider.ValidatePod(test.pod) + assert.Empty(t, errs, "expected validation pass but received errors") }) } } @@ -1144,29 +1115,21 @@ func TestGenerateContainerSecurityContextReadOnlyRootFS(t *testing.T) { }, } - for k, v := range tests { - provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Errorf("%s unable to create provider %v", k, err) - continue - } - err = provider.MutatePod(v.pod) - if err != nil { - t.Errorf("%s unable to create container security context %v", k, err) - continue - } - - sc := v.pod.Spec.Containers[0].SecurityContext - if v.expected == nil && sc.ReadOnlyRootFilesystem != nil { - t.Errorf("%s expected a nil ReadOnlyRootFilesystem but got %t", k, *sc.ReadOnlyRootFilesystem) - } - if v.expected != nil && sc.ReadOnlyRootFilesystem == nil { - t.Errorf("%s expected a non nil ReadOnlyRootFilesystem but received nil", k) - } - if v.expected != nil && sc.ReadOnlyRootFilesystem != nil && (*v.expected != *sc.ReadOnlyRootFilesystem) { - t.Errorf("%s expected a non nil ReadOnlyRootFilesystem set to %t but got %t", k, *v.expected, *sc.ReadOnlyRootFilesystem) - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "unable to create provider") + err = provider.MutatePod(test.pod) + require.NoError(t, err, "unable to mutate container") + sc := test.pod.Spec.Containers[0].SecurityContext + if test.expected == nil { + assert.Nil(t, sc.ReadOnlyRootFilesystem, "expected a nil ReadOnlyRootFilesystem") + } else { + require.NotNil(t, sc.ReadOnlyRootFilesystem, "expected a non nil ReadOnlyRootFilesystem") + assert.Equal(t, *test.expected, *sc.ReadOnlyRootFilesystem) + } + }) } } @@ -1256,55 +1219,42 @@ func TestValidateAllowedVolumes(t *testing.T) { // reflectively create the volume source fieldVal := val.Type().Field(i) - volumeSource := api.VolumeSource{} - volumeSourceVolume := reflect.New(fieldVal.Type.Elem()) + t.Run(fieldVal.Name, func(t *testing.T) { + volumeSource := api.VolumeSource{} + volumeSourceVolume := reflect.New(fieldVal.Type.Elem()) - reflect.ValueOf(&volumeSource).Elem().FieldByName(fieldVal.Name).Set(volumeSourceVolume) - volume := api.Volume{VolumeSource: volumeSource} + reflect.ValueOf(&volumeSource).Elem().FieldByName(fieldVal.Name).Set(volumeSourceVolume) + volume := api.Volume{VolumeSource: volumeSource} - // sanity check before moving on - fsType, err := psputil.GetVolumeFSType(volume) - if err != nil { - t.Errorf("error getting FSType for %s: %s", fieldVal.Name, err.Error()) - continue - } + // sanity check before moving on + fsType, err := psputil.GetVolumeFSType(volume) + require.NoError(t, err, "error getting FSType") - // add the volume to the pod - pod := defaultPod() - pod.Spec.Volumes = []api.Volume{volume} + // add the volume to the pod + pod := defaultPod() + pod.Spec.Volumes = []api.Volume{volume} - // create a PSP that allows no volumes - psp := defaultPSP() + // create a PSP that allows no volumes + psp := defaultPSP() - provider, err := NewSimpleProvider(psp, "namespace", NewSimpleStrategyFactory()) - if err != nil { - t.Errorf("error creating provider for %s: %s", fieldVal.Name, err.Error()) - continue - } + provider, err := NewSimpleProvider(psp, "namespace", NewSimpleStrategyFactory()) + require.NoError(t, err, "error creating provider") - // expect a denial for this PSP and test the error message to ensure it's related to the volumesource - errs := provider.ValidatePod(pod) - if len(errs) != 1 { - t.Errorf("expected exactly 1 error for %s but got %v", fieldVal.Name, errs) - } else { - if !strings.Contains(errs.ToAggregate().Error(), fmt.Sprintf("%s volumes are not allowed to be used", fsType)) { - t.Errorf("did not find the expected error, received: %v", errs) - } - } + // expect a denial for this PSP and test the error message to ensure it's related to the volumesource + errs := provider.ValidatePod(pod) + require.Len(t, errs, 1, "expected exactly 1 error") + assert.Contains(t, errs.ToAggregate().Error(), fmt.Sprintf("%s volumes are not allowed to be used", fsType), "did not find the expected error") - // now add the fstype directly to the psp and it should validate - psp.Spec.Volumes = []policy.FSType{fsType} - errs = provider.ValidatePod(pod) - if len(errs) != 0 { - t.Errorf("directly allowing volume expected no errors for %s but got %v", fieldVal.Name, errs) - } + // now add the fstype directly to the psp and it should validate + psp.Spec.Volumes = []policy.FSType{fsType} + errs = provider.ValidatePod(pod) + assert.Empty(t, errs, "directly allowing volume expected no errors") - // now change the psp to allow any volumes and the pod should still validate - psp.Spec.Volumes = []policy.FSType{policy.All} - errs = provider.ValidatePod(pod) - if len(errs) != 0 { - t.Errorf("wildcard volume expected no errors for %s but got %v", fieldVal.Name, errs) - } + // now change the psp to allow any volumes and the pod should still validate + psp.Spec.Volumes = []policy.FSType{policy.All} + errs = provider.ValidatePod(pod) + assert.Empty(t, errs, "wildcard volume expected no errors") + }) } }