From 638d9235a27694c48eddea168db358f7cae44285 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Thu, 22 Apr 2021 10:25:44 -0500 Subject: [PATCH] Remove unneeded OIDC-related sleeps in tests. Now that we have the fix from https://github.com/kubernetes/kubernetes/pull/97693, we no longer need these sleeps. The underlying authenticator initialization is still asynchronous, but should happen within a few milliseconds. Signed-off-by: Matt Moyer --- .../jwtcachefiller/jwtcachefiller_test.go | 29 ++++++++++++------- test/integration/e2e_test.go | 5 ---- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 5a43751e..19eb1731 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -16,6 +16,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -25,6 +26,7 @@ import ( "gopkg.in/square/go-jose.v2/jwt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" @@ -345,15 +347,6 @@ func TestController(t *testing.T) { return // end of test unless we wanted to run tests on the resulting authenticator from the cache } - // The implementation of AuthenticateToken() that we use waits 10 seconds after creation to - // perform OIDC discovery. Therefore, the JWTAuthenticator is not functional for the first 10 - // seconds. We sleep for 13 seconds in this unit test to give a little bit of cushion to that 10 - // second delay. - // - // We should get rid of this 10 second delay. See - // https://github.com/vmware-tanzu/pinniped/issues/260. - time.Sleep(time.Second * 13) - // We expected the cache to have an entry, so pull that entry from the cache and test it. expectedCacheKey := authncache.Key{ APIGroup: auth1alpha1.GroupName, @@ -428,7 +421,17 @@ func TestController(t *testing.T) { tt.wantUsernameClaim, username, ) - rsp, authenticated, err := cachedAuthenticator.AuthenticateToken(context.Background(), jwt) + + // Loop for a while here to allow the underlying OIDC authenticator to initialize itself asynchronously. + var ( + rsp *authenticator.Response + authenticated bool + err error + ) + _ = wait.PollImmediate(10*time.Millisecond, 5*time.Second, func() (bool, error) { + rsp, authenticated, err = cachedAuthenticator.AuthenticateToken(context.Background(), jwt) + return !isNotInitialized(err), nil + }) if test.wantErrorRegexp != "" { require.Error(t, err) require.Regexp(t, test.wantErrorRegexp, err.Error()) @@ -443,6 +446,12 @@ func TestController(t *testing.T) { } } +// isNotInitialized checks if the error is the internally-defined "oidc: authenticator not initialized" error from +// the underlying OIDC authenticator, which is initialized asynchronously. +func isNotInitialized(err error) bool { + return err != nil && strings.Contains(err.Error(), "authenticator not initialized") +} + func testTableForAuthenticateTokenTests( t *testing.T, goodRSASigningKey *rsa.PrivateKey, diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 2cb207e7..b70a5053 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -151,11 +151,6 @@ func TestE2EFullIntegration(t *testing.T) { kubeconfigPath := filepath.Join(tempDir, "kubeconfig.yaml") require.NoError(t, ioutil.WriteFile(kubeconfigPath, []byte(kubeconfigYAML), 0600)) - // Wait 10 seconds for the JWTAuthenticator to become initialized. - // TODO: remove this sleep once we have fixed the initialization problem. - t.Log("sleeping 10s to wait for JWTAuthenticator to become initialized") - time.Sleep(10 * time.Second) - // Run "kubectl get namespaces" which should trigger a browser login via the plugin. start := time.Now() kubectlCmd := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath)