Modernize PSP unit test

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",
     ],

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 {
+	require.NoError(t, err, "unable to create provider")
-		t.Fatalf("unable to create provider %v", err)
-	}
 	err = provider.MutatePod(pod)
-	if err != nil {
+	require.NoError(t, err, "unable to modify pod")
-		t.Fatalf("unable to create psc %v", err)
-	}
 
 	// 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 {
+		require.NoError(t, err, "unable to create provider")
-			t.Fatalf("unable to create provider %v", err)
-		}
 		err = provider.MutatePod(pod)
-		if err != nil {
+		require.NoError(t, err, "unable to modify pod")
-			t.Fatalf("unable to create container security context %v", err)
-		}
 
 		// 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 {
+	for name, test := range errorCases {
-		provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
+		t.Run(name, func(t *testing.T) {
-		if err != nil {
+			provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory())
-			t.Fatalf("unable to create provider %v", err)
+			require.NoError(t, err, "unable to create provider")
-		}
+			errs := provider.ValidatePod(test.pod)
-		errs := provider.ValidatePod(v.pod)
+			require.NotEmpty(t, errs, "expected validation failure but did not receive errors")
-		if len(errs) == 0 {
+			assert.Contains(t, errs[0].Error(), test.expectedError, "received unexpected error")
-			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)
-		}
 	}
 }
 
@@ -618,20 +603,13 @@ func TestValidateContainerFailures(t *testing.T) {
 		},
 	}
 
-	for k, v := range errorCases {
+	for name, test := range errorCases {
-		t.Run(k, func(t *testing.T) {
+		t.Run(name, func(t *testing.T) {
-			provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
+			provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory())
-			if err != nil {
+			require.NoError(t, err, "unable to create provider")
-				t.Fatalf("unable to create provider %v", err)
+			errs := provider.ValidatePod(test.pod)
-			}
+			require.NotEmpty(t, errs, "expected validation failure but did not receive errors")
-			errs := provider.ValidatePod(v.pod)
+			assert.Contains(t, errs[0].Error(), test.expectedError, "unexpected error")
-			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)
-			}
 		})
 	}
 }
@@ -909,16 +887,13 @@ func TestValidatePodSuccess(t *testing.T) {
 		},
 	}
 
-	for k, v := range successCases {
+	for name, test := range successCases {
-		provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
+		t.Run(name, func(t *testing.T) {
-		if err != nil {
+			provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory())
-			t.Fatalf("unable to create provider %v", err)
+			require.NoError(t, err, "unable to create provider")
-		}
+			errs := provider.ValidatePod(test.pod)
-		errs := provider.ValidatePod(v.pod)
+			assert.Empty(t, errs, "expected validation pass but received errors")
-		if len(errs) != 0 {
+		})
-			t.Errorf("%s expected validation pass but received errors %v", k, errs)
-			continue
-		}
 	}
 }
 
@@ -1076,16 +1051,12 @@ func TestValidateContainerSuccess(t *testing.T) {
 		},
 	}
 
-	for k, v := range successCases {
+	for name, test := range successCases {
-		t.Run(k, func(t *testing.T) {
+		t.Run(name, func(t *testing.T) {
-			provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
+			provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory())
-			if err != nil {
+			require.NoError(t, err, "unable to create provider")
-				t.Fatalf("unable to create provider %v", err)
+			errs := provider.ValidatePod(test.pod)
-			}
+			assert.Empty(t, errs, "expected validation pass but received errors")
-			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))
-			}
 		})
 	}
 }
@@ -1144,29 +1115,21 @@ func TestGenerateContainerSecurityContextReadOnlyRootFS(t *testing.T) {
 		},
 	}
 
-	for k, v := range tests {
+	for name, test := range tests {
-		provider, err := NewSimpleProvider(v.psp, "namespace", NewSimpleStrategyFactory())
+		t.Run(name, func(t *testing.T) {
-		if err != nil {
+			provider, err := NewSimpleProvider(test.psp, "namespace", NewSimpleStrategyFactory())
-			t.Errorf("%s unable to create provider %v", k, err)
+			require.NoError(t, err, "unable to create provider")
-			continue
+			err = provider.MutatePod(test.pod)
-		}
+			require.NoError(t, err, "unable to mutate container")
-		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
+			sc := test.pod.Spec.Containers[0].SecurityContext
-		if v.expected == nil && sc.ReadOnlyRootFilesystem != nil {
+			if test.expected == nil {
-			t.Errorf("%s expected a nil ReadOnlyRootFilesystem but got %t", k, *sc.ReadOnlyRootFilesystem)
+				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)
 			}
-		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)
-		}
-
 	}
 }
 
@@ -1256,6 +1219,7 @@ func TestValidateAllowedVolumes(t *testing.T) {
 		// reflectively create the volume source
 		fieldVal := val.Type().Field(i)
 
+		t.Run(fieldVal.Name, func(t *testing.T) {
 			volumeSource := api.VolumeSource{}
 			volumeSourceVolume := reflect.New(fieldVal.Type.Elem())
 
@@ -1264,10 +1228,7 @@ func TestValidateAllowedVolumes(t *testing.T) {
 
 			// sanity check before moving on
 			fsType, err := psputil.GetVolumeFSType(volume)
-		if err != nil {
+			require.NoError(t, err, "error getting FSType")
-			t.Errorf("error getting FSType for %s: %s", fieldVal.Name, err.Error())
-			continue
-		}
 
 			// add the volume to the pod
 			pod := defaultPod()
@@ -1277,34 +1238,23 @@ func TestValidateAllowedVolumes(t *testing.T) {
 			psp := defaultPSP()
 
 			provider, err := NewSimpleProvider(psp, "namespace", NewSimpleStrategyFactory())
-		if err != nil {
+			require.NoError(t, err, "error creating provider")
-			t.Errorf("error creating provider for %s: %s", fieldVal.Name, err.Error())
-			continue
-		}
 
 			// 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 {
+			require.Len(t, errs, 1, "expected exactly 1 error")
-			t.Errorf("expected exactly 1 error for %s but got %v", fieldVal.Name, errs)
+			assert.Contains(t, errs.ToAggregate().Error(), fmt.Sprintf("%s volumes are not allowed to be used", fsType), "did not find the expected error")
-		} 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)
-			}
-		}
 
 			// 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 {
+			assert.Empty(t, errs, "directly allowing volume expected no errors")
-			t.Errorf("directly allowing 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)
-		if len(errs) != 0 {
+			assert.Empty(t, errs, "wildcard volume expected no errors")
-			t.Errorf("wildcard volume expected no errors for %s but got %v", fieldVal.Name, errs)
+		})
-		}
 	}
 }