Merge pull request #129 from mattmoyer/test-fixes

Test fixes and hardening

test/integration/client_test.go

@@ -9,7 +9,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1"
 
 	"go.pinniped.dev/internal/client"
 	"go.pinniped.dev/internal/here"
@@ -71,8 +73,13 @@ func TestClient(t *testing.T) {
 	// Using the CA bundle and host from the current (admin) kubeconfig, do the token exchange.
 	clientConfig := library.NewClientConfig(t)
 
-	resp, err := client.ExchangeToken(ctx, namespace, idp, token, string(clientConfig.CAData), clientConfig.Host)
+	var resp *clientauthenticationv1beta1.ExecCredential
+	assert.Eventually(t, func() bool {
+		resp, err = client.ExchangeToken(ctx, namespace, idp, token, string(clientConfig.CAData), clientConfig.Host)
+		return err == nil
+	}, 10*time.Second, 500*time.Millisecond)
 	require.NoError(t, err)
+
 	require.NotNil(t, resp.Status.ExpirationTimestamp)
 	require.InDelta(t, time.Until(resp.Status.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute))
 

test/integration/common_test.go

@@ -21,6 +21,11 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
+const (
+	accessRetryInterval = 250 * time.Millisecond
+	accessRetryTimeout  = 10 * time.Second
+)
+
 // accessAsUserTest runs a generic test in which a clientUnderTest operating with username
 // testUsername tries to auth to the kube API (i.e., list namespaces).
 //
@@ -42,7 +47,7 @@ func accessAsUserTest(
 			listNamespaceResponse, err = clientUnderTest.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
 			return err == nil
 		}
-		assert.Eventually(t, canListNamespaces, 3*time.Second, 250*time.Millisecond)
+		assert.Eventually(t, canListNamespaces, accessRetryTimeout, accessRetryInterval)
 		require.NoError(t, err) // prints out the error and stops the test in case of failure
 		require.NotEmpty(t, listNamespaceResponse.Items)
 	}
@@ -66,7 +71,7 @@ func accessAsUserWithKubectlTest(
 			return err == nil
 		}
 
-		assert.Eventually(t, canListNamespaces, 3*time.Second, 250*time.Millisecond)
+		assert.Eventually(t, canListNamespaces, accessRetryTimeout, accessRetryInterval)
 		require.NoError(t, err) // prints out the error and stops the test in case of failure
 		require.Contains(t, kubectlCommandOutput, expectedNamespace)
 	}
@@ -93,7 +98,7 @@ func accessAsGroupTest(
 			listNamespaceResponse, err = clientUnderTest.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
 			return err == nil
 		}
-		assert.Eventually(t, canListNamespaces, 3*time.Second, 250*time.Millisecond)
+		assert.Eventually(t, canListNamespaces, accessRetryTimeout, accessRetryInterval)
 		require.NoError(t, err) // prints out the error and stops the test in case of failure
 		require.NotEmpty(t, listNamespaceResponse.Items)
 	}
@@ -117,7 +122,7 @@ func accessAsGroupWithKubectlTest(
 			return err == nil
 		}
 
-		assert.Eventually(t, canListNamespaces, 3*time.Second, 250*time.Millisecond)
+		assert.Eventually(t, canListNamespaces, accessRetryTimeout, accessRetryInterval)
 		require.NoError(t, err) // prints out the error and stops the test in case of failure
 		require.Contains(t, kubectlCommandOutput, expectedNamespace)
 	}

test/integration/credentialrequest_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -50,9 +51,14 @@ func TestSuccessfulCredentialRequest(t *testing.T) {
 
 	testWebhook := library.CreateTestWebhookIDP(ctx, t)
 
-	response, err := makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, testWebhook))
-	require.NoError(t, err)
-
+	var response *loginv1alpha1.TokenCredentialRequest
+	successfulResponse := func() bool {
+		var err error
+		response, err = makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, testWebhook))
+		require.NoError(t, err, "the request should never fail at the HTTP level")
+		return response.Status.Credential != nil
+	}
+	assert.Eventually(t, successfulResponse, 10*time.Second, 500*time.Millisecond)
 	require.NotNil(t, response.Status.Credential)
 	require.Empty(t, response.Status.Message)
 	require.Empty(t, response.Spec)

test/library/client.go

@@ -156,10 +156,12 @@ func CreateTestWebhookIDP(ctx context.Context, t *testing.T) corev1.TypedLocalOb
 
 	createContext, cancel := context.WithTimeout(ctx, 5*time.Second)
 	defer cancel()
+
 	idp, err := webhooks.Create(createContext, &idpv1alpha1.WebhookIdentityProvider{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "test-webhook-",
-			Labels:       map[string]string{"pinniped.dev/test": t.Name()},
+			Labels:       map[string]string{"pinniped.dev/test": ""},
+			Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
 		},
 		Spec: idpv1alpha1.WebhookIdentityProviderSpec{
 			Endpoint: endpoint,