From 7a93a644f766cbdd16d23107c495accfa14dd913 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Wed, 20 Nov 2024 15:30:02 -0500 Subject: [PATCH] Make TestRemoteUser more robust against slow trust establishment (#49223) The assertions made in the test did not allow for any delay in the CAs being propagated in response to updating the trust relationship. This caused failures in cases where the remote CA did no yet exist, or did not contain the updated role mapping when the remote client attempted requests. Closes https://github.com/gravitational/teleport/issues/47628. --- lib/auth/tls_test.go | 48 +++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index 5c1597f35b6d7..e8f047fe02fce 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -1234,36 +1234,60 @@ func TestRemoteUser(t *testing.T) { certPool, err := testSrv.CertPool() require.NoError(t, err) - remoteClient, err := remoteServer.NewRemoteClient( - TestUser(remoteUser.GetName()), testSrv.Addr(), certPool) + remoteClient, err := remoteServer.NewRemoteClient(TestUser(remoteUser.GetName()), testSrv.Addr(), certPool) require.NoError(t, err) // User is not authorized to perform any actions - // as local cluster does not trust the remote cluster yet + // as local cluster does not trust the remote cluster yet. _, err = remoteClient.GetDomainName(ctx) - require.True(t, trace.IsConnectionProblem(err)) + require.True(t, trace.IsConnectionProblem(err), "expected a connection problem error, got %v", err) - // Establish trust, the request will still fail, there is - // no role mapping set up + // Establish trust, the request will still fail, since there is + // no role mapping set up yet. err = testSrv.AuthServer.Trust(ctx, remoteServer, nil) require.NoError(t, err) - // Create fresh client now trust is established - remoteClient, err = remoteServer.NewRemoteClient( - TestUser(remoteUser.GetName()), testSrv.Addr(), certPool) + // Create a fresh client now that trust is established. + remoteClient, err = remoteServer.NewRemoteClient(TestUser(remoteUser.GetName()), testSrv.Addr(), certPool) require.NoError(t, err) + + // Validate that the client is not permitted to perform RPCs without a role map. + // The requests are attempted several times since it may take some time + // for the trust relationship to be established and CAs to be propagated. _, err = remoteClient.GetDomainName(ctx) - require.True(t, trace.IsAccessDenied(err)) + assert.Error(t, err) + if !trace.IsAccessDenied(err) { + require.EventuallyWithT(t, func(t *assert.CollectT) { + _, err = remoteClient.GetDomainName(ctx) + assert.True(t, trace.IsAccessDenied(err), "expected an access denied error, got %v", err) + }, 15*time.Second, 100*time.Millisecond) + } - // Establish trust and map remote role to local admin role + // Establish trust and map the remote role to the local admin role. _, localRole, err := CreateUserAndRole(testSrv.Auth(), "local-user", []string{"local-role"}, nil) require.NoError(t, err) err = testSrv.AuthServer.Trust(ctx, remoteServer, types.RoleMap{{Remote: remoteRole.GetName(), Local: []string{localRole.GetName()}}}) require.NoError(t, err) + // Validate that the client is now permitted to perform RPCs now that + // the role map was created. The requests are attempted several times since it + // may take some time for the trust relationship to be updated and CAs to be propagated. _, err = remoteClient.GetDomainName(ctx) - require.NoError(t, err) + if err == nil { + return + } + + // The only acceptable error here is AccessDenied caused by the role map + // not being propagated yet. Any other errors should fail the test. + if !trace.IsAccessDenied(err) { + require.NoError(t, err) + } + + require.EventuallyWithT(t, func(t *assert.CollectT) { + _, err = remoteClient.GetDomainName(ctx) + assert.NoError(t, err) + }, 15*time.Second, 100*time.Millisecond) } // TestNopUser tests user with no permissions except