From 52402634a78132eca0edfb55f55ec0d8a61c7aec Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Thu, 27 Feb 2025 10:05:10 -0500 Subject: [PATCH] Fix test flakes related to Kubernetes user cert generation (#52527) #52109 added a dependency on the unified resource cache to user cert generation to reduce resource consumption. A number of tests that exercise generating Kubernetes user certs were either not waiting for the Kubernetes resources to exist prior to authentication and getting lucky, or checking that the resources were in the auth cache, but not the unified resource cache. This attempts to cover any tests which generate Kubernetes user certificates to verify that the unified resource cache contains the expected cluster before proceeding. Fixes #52157. Fixes #52441. --- e2e/aws/eks_test.go | 16 +++++++++------- lib/auth/auth_test.go | 10 ++++++++++ tool/tsh/common/kube_test.go | 17 ++++++++++++----- tool/tsh/common/tsh_helper_test.go | 5 ++--- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/e2e/aws/eks_test.go b/e2e/aws/eks_test.go index 9a936894c8b78..ebcb44181aa1f 100644 --- a/e2e/aws/eks_test.go +++ b/e2e/aws/eks_test.go @@ -91,6 +91,7 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) { ) // Get the auth server. authC := teleport.Process.GetAuthServer() + expectedClusterName := os.Getenv(eksClusterNameEnv) // Wait for the discovery service to discover the cluster and create a // KubernetesCluster resource. // Discovery service will scan the AWS account each minutes. @@ -105,7 +106,7 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) { // Fail fast if the discovery service creates more than one cluster. assert.Len(t, clusters, 1) // Fail fast if the discovery service creates a cluster with a different name. - assert.Equal(t, os.Getenv(eksClusterNameEnv), clusters[0].GetName()) + assert.Equal(t, expectedClusterName, clusters[0].GetName()) return true }, 3*time.Minute, 10*time.Second, "wait for the discovery service to create a cluster") @@ -116,13 +117,14 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - kubeServers, err := authC.GetKubernetesServers(ctx) - return err == nil && len(kubeServers) == 1 + for ks := range authC.UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) { + if ks.GetCluster().GetName() == expectedClusterName { + return true + } + } + return false }, 2*time.Minute, time.Second, "wait for the kubernetes service to create a KubernetesServer") - clusters, err := authC.GetKubernetesClusters(context.Background()) - require.NoError(t, err) - // kubeClient is a Kubernetes client for the user created above // that will be used to verify that the user can access the cluster and // the permissions are correct. @@ -131,7 +133,7 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) { Username: hostUser, KubeUsers: kubeUsers, KubeGroups: kubeGroups, - KubeCluster: clusters[0].GetName(), + KubeCluster: expectedClusterName, }) require.NoError(t, err) diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index 061f3d0c0dcb1..65799d2b4969e 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -739,6 +739,16 @@ func TestAuthenticateSSHUser(t *testing.T) { _, err = s.a.UpsertKubernetesServer(ctx, kubeServer) require.NoError(t, err) + // Wait for cache propagation of the kubernetes resources before proceeding with the tests. + require.Eventually(t, func() bool { + for ks := range s.a.UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) { + if ks.GetCluster().GetName() == kubeCluster.GetName() { + return true + } + } + return false + }, 10*time.Second, 100*time.Millisecond) + // Login specifying a valid kube cluster. It should appear in the TLS cert. resp, err = s.a.AuthenticateSSHUser(ctx, authclient.AuthenticateSSHRequest{ AuthenticateUserRequest: authclient.AuthenticateUserRequest{ diff --git a/tool/tsh/common/kube_test.go b/tool/tsh/common/kube_test.go index ae0890e8b8bbe..8a399c976d254 100644 --- a/tool/tsh/common/kube_test.go +++ b/tool/tsh/common/kube_test.go @@ -166,11 +166,18 @@ func setupKubeTestPack(t *testing.T, withMultiplexMode bool) *kubeTestPack { }, ), withValidationFunc(func(s *suite) bool { - rootClusters, err := s.root.GetAuthServer().GetKubernetesServers(ctx) - require.NoError(t, err) - leafClusters, err := s.leaf.GetAuthServer().GetKubernetesServers(ctx) - require.NoError(t, err) - return len(rootClusters) >= 2 && len(leafClusters) >= 1 + // Wait for cache propagation of the kubernetes resources before proceeding with the tests. + var foundRoot1, foundRoot2, foundLeaf bool + for ks := range s.root.GetAuthServer().UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) { + foundRoot1 = foundRoot1 || ks.GetCluster().GetName() == rootKubeCluster1 + foundRoot2 = foundRoot2 || ks.GetCluster().GetName() == rootKubeCluster2 + } + + for ks := range s.leaf.GetAuthServer().UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) { + foundLeaf = foundLeaf || ks.GetCluster().GetName() == leafKubeCluster + } + + return foundRoot1 && foundRoot2 && foundLeaf }), ) diff --git a/tool/tsh/common/tsh_helper_test.go b/tool/tsh/common/tsh_helper_test.go index 85ec86097b3bd..bd9584d46b3eb 100644 --- a/tool/tsh/common/tsh_helper_test.go +++ b/tool/tsh/common/tsh_helper_test.go @@ -49,6 +49,7 @@ import ( "github.com/gravitational/teleport/lib/config" "github.com/gravitational/teleport/lib/service" "github.com/gravitational/teleport/lib/service/servicecfg" + "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/utils" ) @@ -482,10 +483,8 @@ func mustRegisterKubeClusters(t *testing.T, ctx context.Context, authSrv *auth.S require.NoError(t, wg.Wait()) require.EventuallyWithT(t, func(c *assert.CollectT) { - servers, err := authSrv.GetKubernetesServers(ctx) - assert.NoError(c, err) gotNames := map[string]struct{}{} - for _, ks := range servers { + for ks := range authSrv.UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) { gotNames[ks.GetName()] = struct{}{} } for _, name := range wantNames {