From 4edda802e5c6e6c00e57be837f8a3f4ab3465d59 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 22 Sep 2020 11:23:34 -0500 Subject: [PATCH 1/3] 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/3] 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) From d574fe05ba8565476b63d826610254b157702d67 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 22 Sep 2020 14:15:14 -0500 Subject: [PATCH 3/3] Add a section about our CLA. Signed-off-by: Matt Moyer --- doc/contributing.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/contributing.md b/doc/contributing.md index 2f81bbb3..4d9c438f 100644 --- a/doc/contributing.md +++ b/doc/contributing.md @@ -58,6 +58,15 @@ Also check to see if any open issues are labeled with ["good first issue"](https://github.com/vmware-tanzu/pinniped/labels/good%20first%20issue) or ["help wanted"](https://github.com/vmware-tanzu/pinniped/labels/help%20wanted). +## CLA + +We welcome contributions from everyone but we can only accept them if you sign +our Contributor License Agreement (CLA). If you would like to contribute and you +have not signed it, our CLA-bot will walk you through the process when you open +a Pull Request. For questions about the CLA process, see the +[FAQ](https://cla.vmware.com/faq) or submit a question through the GitHub issue +tracker. + ## Building The [Dockerfile](../Dockerfile) at the root of the repo can be used to build and