concierge_impersonation_proxy_test.go: Test all the verbs

Also:
- Shut down the informer correctly in
  concierge_impersonation_proxy_test.go
- Remove the t.Failed() checks which avoid cleaning up after failed
  tests. This was inconsistent with how most of the tests work, and
  left cruft on clusters when a test failed.

Signed-off-by: Ryan Richard <richardry@vmware.com>

test/integration/concierge_impersonation_proxy_test.go

@@ -15,8 +15,10 @@ import (
 	v1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/types"
 	k8sinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -172,57 +174,103 @@ func TestImpersonationProxy(t *testing.T) {
 		stopChannel := make(chan struct{})
 		informerFactory.Start(stopChannel)
 		t.Cleanup(func() {
-			stopChannel <- struct{}{}
+			// Shut down the informer.
+			close(stopChannel)
 		})
 		informerFactory.WaitForCacheSync(ctx.Done())
 
-		// Test "create" verb.
+		// Test "create" verb through the impersonation proxy.
-		_, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Create(
+		_, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Create(ctx,
-			ctx,
 			&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "configmap-1"}},
 			metav1.CreateOptions{},
 		)
 		require.NoError(t, err)
-		require.Eventually(t, func() bool {
+		_, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Create(ctx,
-			_, err = informer.Lister().ConfigMaps(namespace.Name).Get("configmap-1")
-			return err == nil
-		}, 10*time.Second, 500*time.Millisecond)
-
-		_, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Create(
-			ctx,
 			&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "configmap-2"}},
 			metav1.CreateOptions{},
 		)
 		require.NoError(t, err)
-		require.Eventually(t, func() bool {
+		_, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Create(ctx,
-			_, err = informer.Lister().ConfigMaps(namespace.Name).Get("configmap-2")
-			return err == nil
-		}, 10*time.Second, 500*time.Millisecond)
-
-		_, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Create(
-			ctx,
 			&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "configmap-3"}},
 			metav1.CreateOptions{},
 		)
 		require.NoError(t, err)
-		require.Eventually(t, func() bool {
-			_, err = informer.Lister().ConfigMaps(namespace.Name).Get("configmap-3")
-			return err == nil
-		}, 10*time.Second, 500*time.Millisecond)
 
+		// Make sure that all of the created ConfigMaps show up in the informer's cache to
+		// demonstrate that the informer's "watch" verb is working through the impersonation proxy.
 		require.Eventually(t, func() bool {
-			configmaps, err := informer.Lister().ConfigMaps(namespace.Name).List(labels.Everything())
+			_, err1 := informer.Lister().ConfigMaps(namespace.Name).Get("configmap-1")
-			return err == nil && len(configmaps) == 3
+			_, err2 := informer.Lister().ConfigMaps(namespace.Name).Get("configmap-2")
-		}, 10*time.Second, 500*time.Millisecond)
+			_, err3 := informer.Lister().ConfigMaps(namespace.Name).Get("configmap-3")
+			return err1 == nil && err2 == nil && err3 == nil
+		}, 10*time.Second, 50*time.Millisecond)
 
-		// TODO, test more verbs
+		// Test "get" verb through the impersonation proxy.
-		// "get" one them.
+		configMap3, err := impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Get(ctx, "configmap-3", metav1.GetOptions{})
-		// "list" them all.
+		require.NoError(t, err)
-		// "update" one of them.
+
-		// "patch" one of them.
+		// Test "list" verb through the impersonation proxy.
-		// "delete" one of them.
+		listResult, err := impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).List(ctx, metav1.ListOptions{})
-		// "deletecollection" all of them.
+		require.NoError(t, err)
-		// Make sure the watch sees all of those actions.
+		require.Len(t, listResult.Items, 3)
+
+		// Test "update" verb through the impersonation proxy.
+		configMap3.Data = map[string]string{"foo": "bar"}
+		updateResult, err := impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Update(ctx, configMap3, metav1.UpdateOptions{})
+		require.NoError(t, err)
+		require.Equal(t, "bar", updateResult.Data["foo"])
+
+		// Make sure that the updated ConfigMap shows up in the informer's cache to
+		// demonstrate that the informer's "watch" verb is working through the impersonation proxy.
+		require.Eventually(t, func() bool {
+			configMap, err := informer.Lister().ConfigMaps(namespace.Name).Get("configmap-3")
+			return err == nil && configMap.Data["foo"] == "bar"
+		}, 10*time.Second, 50*time.Millisecond)
+
+		// Test "patch" verb through the impersonation proxy.
+		patchResult, err := impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Patch(ctx,
+			"configmap-3",
+			types.MergePatchType,
+			[]byte(`{"data":{"baz":"42"}}`),
+			metav1.PatchOptions{},
+		)
+		require.NoError(t, err)
+		require.Equal(t, "bar", patchResult.Data["foo"])
+		require.Equal(t, "42", patchResult.Data["baz"])
+
+		// Make sure that the patched ConfigMap shows up in the informer's cache to
+		// demonstrate that the informer's "watch" verb is working through the impersonation proxy.
+		require.Eventually(t, func() bool {
+			configMap, err := informer.Lister().ConfigMaps(namespace.Name).Get("configmap-3")
+			return err == nil && configMap.Data["foo"] == "bar" && configMap.Data["baz"] == "42"
+		}, 10*time.Second, 50*time.Millisecond)
+
+		// Test "delete" verb through the impersonation proxy.
+		err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).Delete(ctx, "configmap-3", metav1.DeleteOptions{})
+		require.NoError(t, err)
+
+		// Make sure that the deleted ConfigMap shows up in the informer's cache to
+		// demonstrate that the informer's "watch" verb is working through the impersonation proxy.
+		require.Eventually(t, func() bool {
+			_, getErr := informer.Lister().ConfigMaps(namespace.Name).Get("configmap-3")
+			list, listErr := informer.Lister().ConfigMaps(namespace.Name).List(labels.Everything())
+			return k8serrors.IsNotFound(getErr) && listErr == nil && len(list) == 2
+		}, 10*time.Second, 50*time.Millisecond)
+
+		// Test "deletecollection" verb through the impersonation proxy.
+		err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{})
+		require.NoError(t, err)
+
+		// Make sure that the deleted ConfigMaps shows up in the informer's cache to
+		// demonstrate that the informer's "watch" verb is working through the impersonation proxy.
+		require.Eventually(t, func() bool {
+			list, listErr := informer.Lister().ConfigMaps(namespace.Name).List(labels.Everything())
+			return listErr == nil && len(list) == 0
+		}, 10*time.Second, 50*time.Millisecond)
+
+		listResult, err = impersonationProxyClient.CoreV1().ConfigMaps(namespace.Name).List(ctx, metav1.ListOptions{})
+		require.NoError(t, err)
+		require.Len(t, listResult.Items, 0)
 	})
 
 	// Update configuration to force the proxy to disabled mode

test/integration/kubeclient_test.go

@@ -42,13 +42,12 @@ func TestKubeClientOwnerRef(t *testing.T) {
 	)
 	require.NoError(t, err)
 
-	defer func() {
+	t.Cleanup(func() {
-		if t.Failed() {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
-			return
+		defer cancel()
-		}
 		err := namespaces.Delete(ctx, namespace.Name, metav1.DeleteOptions{})
 		require.NoError(t, err)
-	}()
+	})
 
 	// create something that we can point to
 	parentSecret, err := regularClient.CoreV1().Secrets(namespace.Name).Create(
@@ -91,13 +90,15 @@ func TestKubeClientOwnerRef(t *testing.T) {
 		metav1.CreateOptions{},
 	)
 	require.NoError(t, err)
-	defer func() {
+	t.Cleanup(func() {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+		defer cancel()
 		err := regularAggregationClient.ApiregistrationV1().APIServices().Delete(ctx, parentAPIService.Name, metav1.DeleteOptions{})
 		if errors.IsNotFound(err) {
 			return
 		}
 		require.NoError(t, err)
-	}()
+	})
 
 	// work around stupid behavior of WithoutVersionDecoder.Decode
 	parentAPIService.APIVersion, parentAPIService.Kind = apiregistrationv1.SchemeGroupVersion.WithKind("APIService").ToAPIVersionAndKind()

test/library/client.go

@@ -168,12 +168,6 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
 
 	t.Cleanup(func() {
 		t.Helper()
-
-		if t.Failed() {
-			t.Logf("skipping deletion of test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name)
-			return
-		}
-
 		t.Logf("cleaning up test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name)
 		deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()
@@ -237,12 +231,6 @@ func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T, spec auth1alp
 
 	t.Cleanup(func() {
 		t.Helper()
-
-		if t.Failed() {
-			t.Logf("skipping deletion of test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name)
-			return
-		}
-
 		t.Logf("cleaning up test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name)
 		deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()