From 4edda802e5c6e6c00e57be837f8a3f4ab3465d59 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 22 Sep 2020 11:23:34 -0500 Subject: [PATCH 1/2] Avoid a bug where long test names overflow the max label length. Annotations do not have this restriction, so we can put it there instead. This only currently occurs on clusters without the cluster signing capability (GKE). Signed-off-by: Matt Moyer --- test/library/client.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/library/client.go b/test/library/client.go index ec14c8cd..73c10b1c 100644 --- a/test/library/client.go +++ b/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, From adf263b566353f364d5156ef3e8423f237d3f150 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 22 Sep 2020 11:50:00 -0500 Subject: [PATCH 2/2] Harden some tests against slow IDP controllers using `Eventually()`. Signed-off-by: Matt Moyer --- test/integration/client_test.go | 9 ++++++++- test/integration/common_test.go | 13 +++++++++---- test/integration/credentialrequest_test.go | 12 +++++++++--- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 1482e278..1c5da211 100644 --- a/test/integration/client_test.go +++ b/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)) diff --git a/test/integration/common_test.go b/test/integration/common_test.go index 7e529ac7..97a09f4b 100644 --- a/test/integration/common_test.go +++ b/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) } diff --git a/test/integration/credentialrequest_test.go b/test/integration/credentialrequest_test.go index d27f652e..481eba9a 100644 --- a/test/integration/credentialrequest_test.go +++ b/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)